Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: notify admins about old credentials #9854

Merged
merged 3 commits into from May 26, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 70 additions & 0 deletions app/jobs/scheduled/old_keys_reminder.rb
@@ -0,0 +1,70 @@
# frozen_string_literal: true

module Jobs
class OldKeysReminder < ::Jobs::Scheduled
every 1.month

def execute(_args)
return if SiteSetting.notify_about_secrets_older_than == 'never'
return if old_site_settings_keys.blank? && old_api_keys.blank?
admins.each do |admin|
PostCreator.create!(
Discourse.system_user,
title: title,
raw: body,
archetype: Archetype.private_message,
target_usernames: admin.username,
validate: false
)
end
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 unless site_setting
lis2 marked this conversation as resolved.
Show resolved Hide resolved
next if site_setting.value.blank?
next if site_setting.updated_at + calculate_period > Time.zone.now
old_keys << site_setting
end
end

def old_api_keys
@old_api_keys ||= ApiKey.all.each_with_object([]) do |api_key, old_keys|
next if api_key.created_at + calculate_period > Time.zone.now
old_keys << api_key
end
end

def calculate_period
case SiteSetting.notify_about_secrets_older_than
lis2 marked this conversation as resolved.
Show resolved Hide resolved
when '1 year'
1.year
when '2 years'
2.years
when '3 years'
3.years
end
end

def admins
User.real.where(admin: true)
lis2 marked this conversation as resolved.
Show resolved Hide resolved
end

def title
I18n.t('old_keys_reminder.title', keys_count: old_site_settings_keys.count + old_api_keys.count)
end

def body
I18n.t('old_keys_reminder.body', keys: keys_list)
end

def keys_list
setting_key_messages = old_site_settings_keys.map { |key| "#{key.name} - #{key.updated_at}" }
api_key_messages = old_api_keys.map { |key| "#{[key.description, key.user&.username, key.created_at].compact.join(" - ")}" }
[setting_key_messages | api_key_messages].join("\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor but instead of two intermediate arrays, because we can just push into an existing array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! nice catch, fixed and I will merge that stuff tomorrow in the morning :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw I left a small comment in dev about the design of this feature in https://dev.discourse.org/t/automatic-reminder-to-reset-old-machine-generated-secrets/16311/9

end
end
end
10 changes: 10 additions & 0 deletions config/locales/server.en.yml
Expand Up @@ -1578,6 +1578,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)"
notify_about_secrets_older_than: "Notify about credentials older than"

email_token_valid_hours: "Forgot password / activate account tokens are valid for (n) hours."

Expand Down Expand Up @@ -4812,3 +4813,12 @@ en:

discord:
not_in_allowed_guild: "Authentication failed. You are not a member of a permitted Discord guild."

old_keys_reminder:
title: "You have %{keys_count} old credentials"
body: |
We have detected you have the following credentials that are 2 years or older

%{keys}

These credentials are sensitive and we recommend resetting them every 2 years to avoid impact of any future data breaches
8 changes: 8 additions & 0 deletions config/site_settings.yml
Expand Up @@ -1448,6 +1448,14 @@ security:
allow_embedding_site_in_an_iframe:
default: false
hidden: true
notify_about_secrets_older_than:
default: "2 years"
type: enum
choices:
- never
- "1 year"
- "2 years"
- "3 years"

onebox:
enable_flash_video_onebox: false
Expand Down
57 changes: 57 additions & 0 deletions spec/jobs/old_keys_reminder_spec.rb
@@ -0,0 +1,57 @@
# 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) }

it "detects old keys" do
SiteSetting.notify_about_secrets_older_than = "2 years"
freeze_time 2.years.from_now
expect(described_class.new.send(:old_site_settings_keys).sort).to eq([google_secret, instagram_secret].sort)
lis2 marked this conversation as resolved.
Show resolved Hide resolved
expect(described_class.new.send(:old_api_keys)).to eq([api_key])
SiteSetting.notify_about_secrets_older_than = "3 years"
expect(described_class.new.send(:old_site_settings_keys)).to eq([])
expect(described_class.new.send(:old_api_keys)).to eq([])
end

it 'has correct title' do
SiteSetting.notify_about_secrets_older_than = "2 years"
freeze_time 2.years.from_now
expect(described_class.new.send(:title)).to eq("You have 3 old credentials")
end

it 'has correct body' do
SiteSetting.notify_about_secrets_older_than = "2 years"
freeze_time 2.years.from_now
expect(described_class.new.send(:body)).to eq(<<-MSG)
We have detected you have the following credentials that are 2 years or older

google_oauth2_client_secret - #{google_secret.updated_at}
instagram_consumer_secret - #{instagram_secret.updated_at}
api key description - #{api_key.created_at}

These credentials are sensitive and we recommend resetting them every 2 years to avoid impact of any future data breaches
MSG
end

it 'sends message to admin' do
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].sort)
end

it 'does not send message when notification set to never or no old keys' do
SiteSetting.notify_about_secrets_older_than = "never"
freeze_time 2.years.from_now
expect { described_class.new.execute({}) }.to change { Post.count }.by(0)
SiteSetting.notify_about_secrets_older_than = "never"
freeze_time 2.years.from_now
expect { described_class.new.execute({}) }.to change { Post.count }.by(0)
end
end