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.
This commit is contained in:
Krzysztof Kotlarek 2020-06-01 13:49:27 +10:00 committed by GitHub
parent b0b37bf5a3
commit 9a6ef80739
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 157 additions and 0 deletions

View File

@ -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

View File

@ -1582,6 +1582,7 @@ en:
moderators_view_emails: "Allow moderators to view user emails" 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)" 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)" 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." email_token_valid_hours: "Forgot password / activate account tokens are valid for (n) hours."
@ -4820,3 +4821,14 @@ en:
discord: discord:
not_in_allowed_guild: "Authentication failed. You are not a member of a permitted Discord guild." 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.

View File

@ -1454,6 +1454,9 @@ security:
allow_embedding_site_in_an_iframe: allow_embedding_site_in_an_iframe:
default: false default: false
hidden: true hidden: true
send_old_credential_reminder_days:
default: 0
hidden: true
onebox: onebox:
enable_flash_video_onebox: false enable_flash_video_onebox: false

View File

@ -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!
expect { described_class.new.execute({}) }.to change { Post.count }.by(0)
freeze_time 1.years.from_now
expect { described_class.new.execute({}) }.to change { Post.count }.by(0)
freeze_time 3.days.from_now
expect { described_class.new.execute({}) }.to change { Post.count }.by(1)
end
end