FEATURE: notify admins about old credentials (#9918)

FEATURE: notify admins about old credentials (#9918)

  • FEATURE: notify admins about old credentials

Security and API keys should be renewed periodically. This additional notification should help admins keep their Discourse safe and secure.

diff --git a/app/jobs/scheduled/old_keys_reminder.rb b/app/jobs/scheduled/old_keys_reminder.rb
new file mode 100644
index 0000000..8689c60
--- /dev/null
+++ b/app/jobs/scheduled/old_keys_reminder.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+module Jobs
+  class OldKeysReminder < ::Jobs::Scheduled
+    every 1.month
+
+    OLD_CREDENTIALS_PERIOD = 2.years
+
+    def execute(_args)
+      return if SiteSetting.send_old_credential_reminder_days.to_i == 0
+      return if message_exists?
+      return if old_site_settings_keys.blank? && old_api_keys.blank?
+      PostCreator.create!(
+        Discourse.system_user,
+        title: title,
+        raw: body,
+        archetype: Archetype.private_message,
+        target_usernames: admins.map(&:username),
+        validate: false
+      )
+    end
+
+    private
+
+    def old_site_settings_keys
+      @old_site_settings_keys ||= SiteSetting.secret_settings.each_with_object([]) do |secret_name, old_keys|
+        site_setting = SiteSetting.find_by(name: secret_name)
+        next if site_setting&.value.blank?
+        next if site_setting.updated_at + OLD_CREDENTIALS_PERIOD > Time.zone.now
+        old_keys << site_setting
+      end.sort_by { |key| key.updated_at }
+    end
+
+    def old_api_keys
+      @old_api_keys ||= ApiKey.all.order(created_at: :asc).each_with_object([]) do |api_key, old_keys|
+        next if api_key.created_at + OLD_CREDENTIALS_PERIOD > Time.zone.now
+        old_keys << api_key
+      end
+    end
+
+    def admins
+      User.real.admins
+    end
+
+    def message_exists?
+      message = Topic.private_messages.with_deleted.find_by(title: title)
+      message && message.created_at + SiteSetting.send_old_credential_reminder_days.to_i.days > Time.zone.now
+    end
+
+    def title
+      I18n.t('old_keys_reminder.title')
+    end
+
+    def body
+      I18n.t('old_keys_reminder.body', keys: keys_list)
+    end
+
+    def keys_list
+      messages = old_site_settings_keys.map { |key| "#{key.name} - #{key.updated_at.to_date.to_s(:db)}" }
+      old_api_keys.each_with_object(messages) { |key, array| array << "#{[key.description, key.user&.username, key.created_at.to_date.to_s(:db)].compact.join(" - ")}" }
+      messages.join("\n")
+    end
+  end
+end
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 09e9a70..7c043dc 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1582,6 +1582,7 @@ en:
     moderators_view_emails: "Allow moderators to view user emails"
     prioritize_username_in_ux: "Show username first on user page, user card and posts (when disabled name is shown first)"
     enable_rich_text_paste: "Enable automatic HTML to Markdown conversion when pasting text into the composer. (Experimental)"
+    send_old_credential_reminder_days: "Remind about old credentials after days"
 
     email_token_valid_hours: "Forgot password / activate account tokens are valid for (n) hours."
 
@@ -4820,3 +4821,14 @@ en:
 
   discord:
     not_in_allowed_guild: "Authentication failed. You are not a member of a permitted Discord guild."
+
+  old_keys_reminder:
+    title: "Reminder about old credentials"
+    body: |
+      Hello! This is a routine yearly security reminder from your Discourse instance.
+      
+      As a courtesy, we wanted to let you know that the following credentials used on your Discourse instance have not been updated in more than two years:
+      
+      %{keys}
+      
+      No action is required at this time, however, it is considered good security practice to cycle all your important credentials every few years.
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 4676660..5901d6d 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1454,6 +1454,9 @@ security:
   allow_embedding_site_in_an_iframe:
     default: false
     hidden: true
+  send_old_credential_reminder_days:
+    default: 0
+    hidden: true
 
 onebox:
   enable_flash_video_onebox: false
diff --git a/spec/jobs/old_keys_reminder_spec.rb b/spec/jobs/old_keys_reminder_spec.rb
new file mode 100644
index 0000000..fe68070
--- /dev/null
+++ b/spec/jobs/old_keys_reminder_spec.rb
@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe Jobs::OldKeysReminder do
+  let!(:google_secret) { SiteSetting.create!(name: 'google_oauth2_client_secret', value: '123', data_type: 1) }
+  let!(:instagram_secret) { SiteSetting.create!(name: 'instagram_consumer_secret', value: '123', data_type: 1) }
+  let!(:api_key) { Fabricate(:api_key, description: 'api key description') }
+  let!(:admin) { Fabricate(:admin) }
+  let!(:another_admin) { Fabricate(:admin) }
+
+  let!(:recent_twitter_secret) { SiteSetting.create!(name: 'twitter_consumer_secret', value: '123', data_type: 1, updated_at: 2.years.from_now) }
+  let!(:recent_api_key) { Fabricate(:api_key, description: 'recent api key description', created_at: 2.years.from_now, user_id: admin.id) }
+
+  it 'is disabled be default' do
+    freeze_time 2.years.from_now
+    expect { described_class.new.execute({}) }.not_to change { Post.count }
+  end
+
+  it 'sends message to admin with old credentials' do
+    SiteSetting.send_old_credential_reminder_days = '365'
+    freeze_time 2.years.from_now
+    expect { described_class.new.execute({}) }.to change { Post.count }.by(1)
+    post = Post.last
+    expect(post.archetype).to eq(Archetype.private_message)
+    expect(post.topic.topic_allowed_users.map(&:user_id).sort).to eq([Discourse.system_user.id, admin.id, another_admin.id].sort)
+    expect(post.topic.title).to eq('Reminder about old credentials')
+    expect(post.raw).to eq(<<-MSG.rstrip)
+Hello! This is a routine yearly security reminder from your Discourse instance.
+
+As a courtesy, we wanted to let you know that the following credentials used on your Discourse instance have not been updated in more than two years:
+
+google_oauth2_client_secret - #{google_secret.updated_at.to_date.to_s(:db)}
+instagram_consumer_secret - #{instagram_secret.updated_at.to_date.to_s(:db)}
+api key description - #{api_key.created_at.to_date.to_s(:db)}
+
+No action is required at this time, however, it is considered good security practice to cycle all your important credentials every few years.
+    MSG
+
+    post.topic.destroy
+    freeze_time 4.years.from_now
+    described_class.new.execute({})
+    post = Post.last
+    expect(post.topic.title).to eq('Reminder about old credentials')
+    expect(post.raw).to eq(<<-MSG.rstrip)
+Hello! This is a routine yearly security reminder from your Discourse instance.
+
+As a courtesy, we wanted to let you know that the following credentials used on your Discourse instance have not been updated in more than two years:
+
+google_oauth2_client_secret - #{google_secret.updated_at.to_date.to_s(:db)}
+instagram_consumer_secret - #{instagram_secret.updated_at.to_date.to_s(:db)}
+twitter_consumer_secret - #{recent_twitter_secret.updated_at.to_date.to_s(:db)}
+api key description - #{api_key.created_at.to_date.to_s(:db)}
+recent api key description - #{admin.username} - #{recent_api_key.created_at.to_date.to_s(:db)}
+
+No action is required at this time, however, it is considered good security practice to cycle all your important credentials every few years.
+    MSG
+  end
+
+  it 'does not send message when send_old_credential_reminder_days is set to 0 or no old keys' do
+    expect { described_class.new.execute({}) }.to change { Post.count }.by(0)
+    SiteSetting.send_old_credential_reminder_days = '0'
+    freeze_time 2.years.from_now
+    expect { described_class.new.execute({}) }.to change { Post.count }.by(0)
+  end
+
+  it 'does not send a message if already exists' do
+    SiteSetting.send_old_credential_reminder_days = '367'
+    freeze_time 2.years.from_now
+    expect { described_class.new.execute({}) }.to change { Post.count }.by(1)
+    Topic.last.trash!

[... diff too long, it was truncated ...]

GitHub sha: 9a6ef807

1 Like

This commit appears in #9918 which was approved by eviltrout. It was merged by lis2.