Commit bb1d3819 authored by Aishwarya Subramanian's avatar Aishwarya Subramanian Committed by Mayra Cabrera

Send expired SSH key notification to users

Sends notification to users when their
SSH key has expired.
Cron job runs daily at 2 am UTC.
parent 7ffc8a86
...@@ -74,6 +74,18 @@ module Emails ...@@ -74,6 +74,18 @@ module Emails
end end
end end
def ssh_key_expired_email(user, fingerprints)
return unless user && user.active?
@user = user
@fingerprints = fingerprints
@target_url = profile_keys_url
Gitlab::I18n.with_locale(@user.preferred_language) do
mail(to: @user.notification_email, subject: subject(_("Your SSH key has expired")))
end
end
def unknown_sign_in_email(user, ip, time) def unknown_sign_in_email(user, ip, time)
@user = user @user = user
@ip = ip @ip = ip
......
...@@ -43,6 +43,7 @@ class Key < ApplicationRecord ...@@ -43,6 +43,7 @@ class Key < ApplicationRecord
scope :preload_users, -> { preload(:user) } scope :preload_users, -> { preload(:user) }
scope :for_user, -> (user) { where(user: user) } scope :for_user, -> (user) { where(user: user) }
scope :order_last_used_at_desc, -> { reorder(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) } scope :order_last_used_at_desc, -> { reorder(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) }
scope :expired_today_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) }
def self.regular_keys def self.regular_keys
where(type: ['Key', nil]) where(type: ['Key', nil])
......
...@@ -103,6 +103,7 @@ class User < ApplicationRecord ...@@ -103,6 +103,7 @@ class User < ApplicationRecord
# Profile # Profile
has_many :keys, -> { regular_keys }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :keys, -> { regular_keys }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :expired_today_and_unnotified_keys, -> { expired_today_and_not_notified }, class_name: 'Key'
has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent
has_many :group_deploy_keys has_many :group_deploy_keys
has_many :gpg_keys has_many :gpg_keys
...@@ -398,6 +399,14 @@ class User < ApplicationRecord ...@@ -398,6 +399,14 @@ class User < ApplicationRecord
.without_impersonation .without_impersonation
.expired_today_and_not_notified) .expired_today_and_not_notified)
end end
scope :with_ssh_key_expired_today, -> do
includes(:expired_today_and_unnotified_keys)
.where('EXISTS (?)',
::Key
.select(1)
.where('keys.user_id = users.id')
.expired_today_and_not_notified)
end
scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) } scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) }
scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) } scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) }
scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) } scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) }
......
# frozen_string_literal: true
module Keys
class ExpiryNotificationService < ::Keys::BaseService
attr_accessor :keys
def initialize(user, params)
@keys = params[:keys]
super
end
def execute
return unless user.can?(:receive_notifications)
notification_service.ssh_key_expired(user, keys.map(&:fingerprint))
keys.update_all(expiry_notification_delivered_at: Time.current.utc)
end
end
end
...@@ -79,6 +79,13 @@ class NotificationService ...@@ -79,6 +79,13 @@ class NotificationService
mailer.access_token_expired_email(user).deliver_later mailer.access_token_expired_email(user).deliver_later
end end
# Notify the user when at least one of their ssh key has expired today
def ssh_key_expired(user, fingerprints)
return unless user.can?(:receive_notifications)
mailer.ssh_key_expired_email(user, fingerprints).deliver_later
end
# Notify a user when a previously unknown IP or device is used to # Notify a user when a previously unknown IP or device is used to
# sign in to their account # sign in to their account
def unknown_sign_in(user, ip, time) def unknown_sign_in(user, ip, time)
......
%p
= _('Hi %{username}!') % { username: sanitize_name(@user.name) }
%p
= _('Your SSH keys with the following fingerprints has expired:')
%table
%tbody
- @fingerprints.each do |fingerprint|
%tr
%td= fingerprint
%p
- ssh_key_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: @target_url }
= html_escape(_('You can create a new one or check them in your %{ssh_key_link_start}SSH keys%{ssh_key_link_end} settings.')) % { ssh_key_link_start: ssh_key_link_start, ssh_key_link_end: '</a>'.html_safe }
<%= _('Hi %{username}!') % { username: sanitize_name(@user.name) } %>
<%= _('Your SSH keys with the following fingerprints has expired:') %>
<% @fingerprints.each do |fingerprint| %>
- <%= fingerprint %>
<% end %>
<%= _('You can create a new one or check them in your SSH keys settings %{ssh_key_link}.') % { ssh_key_link: @target_url } %>
...@@ -443,6 +443,14 @@ ...@@ -443,6 +443,14 @@
:weight: 1 :weight: 1
:idempotent: :idempotent:
:tags: [] :tags: []
- :name: cronjob:ssh_keys_expired_notification
:feature_category: :compliance_management
:has_external_dependencies:
:urgency: :low
:resource_boundary: :unknown
:weight: 1
:idempotent: true
:tags: []
- :name: cronjob:stuck_ci_jobs - :name: cronjob:stuck_ci_jobs
:feature_category: :continuous_integration :feature_category: :continuous_integration
:has_external_dependencies: :has_external_dependencies:
......
# frozen_string_literal: true
module SshKeys
class ExpiredNotificationWorker
include ApplicationWorker
include CronjobQueue
feature_category :compliance_management
idempotent!
def perform
return unless ::Feature.enabled?(:ssh_key_expiration_email_notification, default_enabled: :yaml)
User.with_ssh_key_expired_today.find_each do |user|
with_context(user: user) do
Gitlab::AppLogger.info "#{self.class}: Notifying User #{user.id} about expired ssh key(s)"
keys = user.expired_today_and_unnotified_keys
Keys::ExpiryNotificationService.new(user, { keys: keys }).execute
end
end
end
end
end
---
title: Send email notification on SSH key expiration
merge_request: 56888
author:
type: added
---
name: ssh_key_expiration_email_notification
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56888
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326386
milestone: '13.11'
type: development
group: group::compliance
default_enabled: false
...@@ -560,6 +560,9 @@ Settings.cron_jobs['manage_evidence_worker']['job_class'] = 'Releases::ManageEvi ...@@ -560,6 +560,9 @@ Settings.cron_jobs['manage_evidence_worker']['job_class'] = 'Releases::ManageEvi
Settings.cron_jobs['user_status_cleanup_batch_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['user_status_cleanup_batch_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['user_status_cleanup_batch_worker']['cron'] ||= '* * * * *' Settings.cron_jobs['user_status_cleanup_batch_worker']['cron'] ||= '* * * * *'
Settings.cron_jobs['user_status_cleanup_batch_worker']['job_class'] = 'UserStatusCleanup::BatchWorker' Settings.cron_jobs['user_status_cleanup_batch_worker']['job_class'] = 'UserStatusCleanup::BatchWorker'
Settings.cron_jobs['ssh_keys_expired_notification_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['ssh_keys_expired_notification_worker']['cron'] ||= '0 2 * * *'
Settings.cron_jobs['ssh_keys_expired_notification_worker']['job_class'] = 'SshKeys::ExpiredNotificationWorker'
Gitlab.com do Gitlab.com do
Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({})
......
# frozen_string_literal: true
class AddExpiryNotificationDeliveredToKeys < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :keys, :expiry_notification_delivered_at, :datetime_with_timezone
end
end
# frozen_string_literal: true
class AddIndexToKeysOnExpiresAtAndExpiryNotificationUndelivered < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
INDEX_NAME = 'index_keys_on_expires_at_and_expiry_notification_undelivered'
disable_ddl_transaction!
def up
add_concurrent_index :keys,
"date(timezone('UTC', expires_at)), expiry_notification_delivered_at",
where: 'expiry_notification_delivered_at IS NULL', name: INDEX_NAME
end
def down
remove_concurrent_index_by_name(:keys, INDEX_NAME)
end
end
dfb88ea7a213da1e56bef532255f01a284d7b9be9ec8a6b9dd0e95ec04d0f524
\ No newline at end of file
79ad2de15faef8edb8752c2a9c89f1739a805af999c86db6e73482a613c4f9d1
\ No newline at end of file
...@@ -14023,7 +14023,8 @@ CREATE TABLE keys ( ...@@ -14023,7 +14023,8 @@ CREATE TABLE keys (
public boolean DEFAULT false NOT NULL, public boolean DEFAULT false NOT NULL,
last_used_at timestamp without time zone, last_used_at timestamp without time zone,
fingerprint_sha256 bytea, fingerprint_sha256 bytea,
expires_at timestamp with time zone expires_at timestamp with time zone,
expiry_notification_delivered_at timestamp with time zone
); );
CREATE SEQUENCE keys_id_seq CREATE SEQUENCE keys_id_seq
...@@ -22974,6 +22975,8 @@ CREATE INDEX index_jira_imports_on_user_id ON jira_imports USING btree (user_id) ...@@ -22974,6 +22975,8 @@ CREATE INDEX index_jira_imports_on_user_id ON jira_imports USING btree (user_id)
CREATE INDEX index_jira_tracker_data_on_service_id ON jira_tracker_data USING btree (service_id); CREATE INDEX index_jira_tracker_data_on_service_id ON jira_tracker_data USING btree (service_id);
CREATE INDEX index_keys_on_expires_at_and_expiry_notification_undelivered ON keys USING btree (date(timezone('UTC'::text, expires_at)), expiry_notification_delivered_at) WHERE (expiry_notification_delivered_at IS NULL);
CREATE UNIQUE INDEX index_keys_on_fingerprint ON keys USING btree (fingerprint); CREATE UNIQUE INDEX index_keys_on_fingerprint ON keys USING btree (fingerprint);
CREATE INDEX index_keys_on_fingerprint_sha256 ON keys USING btree (fingerprint_sha256); CREATE INDEX index_keys_on_fingerprint_sha256 ON keys USING btree (fingerprint_sha256);
...@@ -218,6 +218,7 @@ To use SSH with GitLab, copy your public key to your GitLab account. ...@@ -218,6 +218,7 @@ To use SSH with GitLab, copy your public key to your GitLab account.
The expiration date is informational only, and does not prevent you from using The expiration date is informational only, and does not prevent you from using
the key. However, administrators can view expiration dates and the key. However, administrators can view expiration dates and
use them for guidance when [deleting keys](../user/admin_area/credentials_inventory.md#delete-a-users-ssh-key). use them for guidance when [deleting keys](../user/admin_area/credentials_inventory.md#delete-a-users-ssh-key).
GitLab checks all SSH keys at 02:00 AM UTC every day. It emails an expiration notice for all SSH keys that expire on the current date. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/322637) in GitLab 13.11.)
1. Select **Add key**. 1. Select **Add key**.
## Verify that you can connect ## Verify that you can connect
......
...@@ -34776,6 +34776,12 @@ msgstr "" ...@@ -34776,6 +34776,12 @@ msgstr ""
msgid "You can create a new one or check them in your %{pat_link_start}personal access tokens%{pat_link_end} settings" msgid "You can create a new one or check them in your %{pat_link_start}personal access tokens%{pat_link_end} settings"
msgstr "" msgstr ""
msgid "You can create a new one or check them in your %{ssh_key_link_start}SSH keys%{ssh_key_link_end} settings."
msgstr ""
msgid "You can create a new one or check them in your SSH keys settings %{ssh_key_link}."
msgstr ""
msgid "You can create a new one or check them in your personal access tokens settings %{pat_link}" msgid "You can create a new one or check them in your personal access tokens settings %{pat_link}"
msgstr "" msgstr ""
...@@ -35271,12 +35277,18 @@ msgstr "" ...@@ -35271,12 +35277,18 @@ msgstr ""
msgid "Your Public Email will be displayed on your public profile." msgid "Your Public Email will be displayed on your public profile."
msgstr "" msgstr ""
msgid "Your SSH key has expired"
msgstr ""
msgid "Your SSH key was deleted" msgid "Your SSH key was deleted"
msgstr "" msgstr ""
msgid "Your SSH keys (%{count})" msgid "Your SSH keys (%{count})"
msgstr "" msgstr ""
msgid "Your SSH keys with the following fingerprints has expired:"
msgstr ""
msgid "Your To-Do List" msgid "Your To-Do List"
msgstr "" msgstr ""
......
...@@ -212,6 +212,57 @@ RSpec.describe Emails::Profile do ...@@ -212,6 +212,57 @@ RSpec.describe Emails::Profile do
end end
end end
describe 'notification email for expired ssh key' do
let_it_be(:user) { create(:user) }
let_it_be(:fingerprints) { ["aa:bb:cc:dd:ee:zz"] }
context 'when valid' do
subject { Notify.ssh_key_expired_email(user, fingerprints) }
it_behaves_like 'an email sent from GitLab'
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like 'a user cannot unsubscribe through footer link'
it 'is sent to the user' do
is_expected.to deliver_to user.email
end
it 'has the correct subject' do
is_expected.to have_subject /Your SSH key has expired/
end
it 'mentions the ssh keu has expired' do
is_expected.to have_body_text /Your SSH keys with the following fingerprints has expired/
end
it 'includes a link to ssh key page' do
is_expected.to have_body_text /#{profile_keys_url}/
end
it 'includes the email reason' do
is_expected.to have_body_text /You're receiving this email because of your account on localhost/
end
end
context 'when invalid' do
context 'when user does not exist' do
it do
expect { Notify.ssh_key_expired_email(nil) }.not_to change { ActionMailer::Base.deliveries.count }
end
end
context 'when user is not active' do
before do
user.block!
end
it do
expect { Notify.ssh_key_expired_email(user) }.not_to change { ActionMailer::Base.deliveries.count }
end
end
end
end
describe 'user unknown sign in email' do describe 'user unknown sign in email' do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:ip) { '169.0.0.1' } let_it_be(:ip) { '169.0.0.1' }
......
...@@ -75,6 +75,18 @@ RSpec.describe Key, :mailer do ...@@ -75,6 +75,18 @@ RSpec.describe Key, :mailer do
.to eq([key_3, key_1, key_2]) .to eq([key_3, key_1, key_2])
end end
end end
describe '.expired_today_and_not_notified' do
let_it_be(:user) { create(:user) }
let_it_be(:expired_today_not_notified) { create(:key, expires_at: Time.current, user: user) }
let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user, expiry_notification_delivered_at: Time.current) }
let_it_be(:expired_yesterday) { create(:key, expires_at: 1.day.ago, user: user) }
let_it_be(:future_expiry) { create(:key, expires_at: 1.day.from_now, user: user) }
it 'returns tokens that have expired today' do
expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified)
end
end
end end
context "validation of uniqueness (based on fingerprint uniqueness)" do context "validation of uniqueness (based on fingerprint uniqueness)" do
......
...@@ -85,6 +85,7 @@ RSpec.describe User do ...@@ -85,6 +85,7 @@ RSpec.describe User do
it { is_expected.to have_many(:group_members) } it { is_expected.to have_many(:group_members) }
it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:groups) }
it { is_expected.to have_many(:keys).dependent(:destroy) } it { is_expected.to have_many(:keys).dependent(:destroy) }
it { is_expected.to have_many(:expired_today_and_unnotified_keys) }
it { is_expected.to have_many(:deploy_keys).dependent(:nullify) } it { is_expected.to have_many(:deploy_keys).dependent(:nullify) }
it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:group_deploy_keys) }
it { is_expected.to have_many(:events).dependent(:delete_all) } it { is_expected.to have_many(:events).dependent(:delete_all) }
...@@ -1000,6 +1001,18 @@ RSpec.describe User do ...@@ -1000,6 +1001,18 @@ RSpec.describe User do
end end
end end
describe '.with_ssh_key_expired_today' do
let_it_be(:user1) { create(:user) }
let_it_be(:expired_today_not_notified) { create(:key, expires_at: Time.current, user: user1) }
let_it_be(:user2) { create(:user) }
let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user2, expiry_notification_delivered_at: Time.current) }
it 'returns users whose token has expired today' do
expect(described_class.with_ssh_key_expired_today).to contain_exactly(user1)
end
end
describe '.active_without_ghosts' do describe '.active_without_ghosts' do
let_it_be(:user1) { create(:user, :external) } let_it_be(:user1) { create(:user, :external) }
let_it_be(:user2) { create(:user, state: 'blocked') } let_it_be(:user2) { create(:user, state: 'blocked') }
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Keys::ExpiryNotificationService do
let_it_be_with_reload(:user) { create(:user) }
let_it_be_with_reload(:expired_key) { create(:key, expires_at: Time.current, user: user) }
let(:params) { { keys: keys } }
subject { described_class.new(user, params) }
context 'with expired key', :mailer do
let(:keys) { user.keys }
it 'sends a notification' do
perform_enqueued_jobs do
subject.execute
end
should_email(user)
end
it 'uses notification service to send email to the user' do
expect_next_instance_of(NotificationService) do |notification_service|
expect(notification_service).to receive(:ssh_key_expired).with(expired_key.user, [expired_key.fingerprint])
end
subject.execute
end
it 'updates notified column' do
expect { subject.execute }.to change { expired_key.reload.expiry_notification_delivered_at }
end
context 'when user does not have permission to receive notification' do
before do
user.block!
end
it 'does not send notification' do
perform_enqueued_jobs do
subject.execute
end
should_not_email(user)
end
it 'does not update notified column' do
expect { subject.execute }.not_to change { expired_key.reload.expiry_notification_delivered_at }
end
end
end
end
...@@ -288,6 +288,27 @@ RSpec.describe NotificationService, :mailer do ...@@ -288,6 +288,27 @@ RSpec.describe NotificationService, :mailer do
end end
end end
end end
describe '#ssh_key_expired' do
let_it_be(:user) { create(:user) }
let_it_be(:fingerprints) { ["aa:bb:cc:dd:ee:zz"] }
subject { notification.ssh_key_expired(user, fingerprints) }
it 'sends email to the token owner' do
expect { subject }.to have_enqueued_email(user, fingerprints, mail: "ssh_key_expired_email")
end
context 'when user is not allowed to receive notifications' do
before do
user.block!
end
it 'does not send email to the token owner' do
expect { subject }.not_to have_enqueued_email(user, fingerprints, mail: "ssh_key_expired_email")
end
end
end
end end
describe '#unknown_sign_in' do describe '#unknown_sign_in' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe SshKeys::ExpiredNotificationWorker, type: :worker do
subject(:worker) { described_class.new }
it 'uses a cronjob queue' do
expect(worker.sidekiq_options_hash).to include(
'queue' => 'cronjob:ssh_keys_expired_notification',
'queue_namespace' => :cronjob
)
end
describe '#perform' do
let_it_be(:user) { create(:user) }
context 'with expiring key today' do
let_it_be_with_reload(:expired_today) { create(:key, expires_at: Time.current, user: user) }
it 'invoke the notification service' do
expect_next_instance_of(Keys::ExpiryNotificationService) do |expiry_service|
expect(expiry_service).to receive(:execute)
end
worker.perform
end
it 'updates notified column' do
expect { worker.perform }.to change { expired_today.reload.expiry_notification_delivered_at }
end
include_examples 'an idempotent worker' do
subject do
perform_multiple(worker: worker)
end
end
context 'when feature is not enabled' do
before do
stub_feature_flags(ssh_key_expiration_email_notification: false)
end
it 'does not update notified column' do
expect { worker.perform }.not_to change { expired_today.reload.expiry_notification_delivered_at }
end
end
end
context 'when key has expired in the past' do
let_it_be(:expired_past) { create(:key, expires_at: 1.day.ago, user: user) }
it 'does not update notified column' do
expect { worker.perform }.not_to change { expired_past.reload.expiry_notification_delivered_at }
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment