diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb
index 0734f1786400a8bfda6c0da9bd55938ae2acbfca..f967323f849cdc2f2790fad7ce3868df5cb9cc91 100644
--- a/app/mailers/emails/profile.rb
+++ b/app/mailers/emails/profile.rb
@@ -75,7 +75,7 @@ def access_token_expired_email(user)
end
def ssh_key_expired_email(user, fingerprints)
- return unless user && user.active?
+ return unless user&.active?
@user = user
@fingerprints = fingerprints
@@ -86,6 +86,18 @@ def ssh_key_expired_email(user, fingerprints)
end
end
+ def ssh_key_expiring_soon_email(user, fingerprints)
+ return unless 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 is expiring soon.")))
+ end
+ end
+
def unknown_sign_in_email(user, ip, time)
@user = user
@ip = ip
diff --git a/app/models/key.rb b/app/models/key.rb
index d9da700376aefd89b99f08845c783c19fd6a5bab..131416d1bee38206a5786cfcf63452b557cfda2d 100644
--- a/app/models/key.rb
+++ b/app/models/key.rb
@@ -44,6 +44,7 @@ class Key < ApplicationRecord
scope :for_user, -> (user) { where(user: user) }
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"]) }
+ scope :expiring_soon_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') > CURRENT_DATE AND date(expires_at AT TIME ZONE 'UTC') < ? AND before_expiry_notification_delivered_at IS NULL", DAYS_TO_EXPIRE.days.from_now.to_date]) }
def self.regular_keys
where(type: ['Key', nil])
diff --git a/app/models/user.rb b/app/models/user.rb
index de7bb1ccf49b674971ec066cc69c8652ca8505d5..a0b23429f9fb0cc69ccba6f226736d95c65888c6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -104,6 +104,7 @@ def update_tracked_fields!(request)
# Profile
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 :expiring_soon_and_unnotified_keys, -> { expiring_soon_and_not_notified }, class_name: 'Key'
has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent
has_many :group_deploy_keys
has_many :gpg_keys
@@ -402,6 +403,14 @@ def blocked?
.where('keys.user_id = users.id')
.expired_today_and_not_notified)
end
+ scope :with_ssh_key_expiring_soon, -> do
+ includes(:expiring_soon_and_unnotified_keys)
+ .where('EXISTS (?)',
+ ::Key
+ .select(1)
+ .where('keys.user_id = users.id')
+ .expiring_soon_and_not_notified)
+ end
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_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) }
diff --git a/app/services/keys/expiry_notification_service.rb b/app/services/keys/expiry_notification_service.rb
index 4ba896477d29017ff6ecdd0421271c10a86882f6..b486f77ced2339dc2f522a6ddf6df5f84167e94d 100644
--- a/app/services/keys/expiry_notification_service.rb
+++ b/app/services/keys/expiry_notification_service.rb
@@ -2,17 +2,38 @@
module Keys
class ExpiryNotificationService < ::Keys::BaseService
- attr_accessor :keys
+ attr_accessor :keys, :expiring_soon
def initialize(user, params)
@keys = params[:keys]
+ @expiring_soon = params[:expiring_soon]
super
end
def execute
- return unless user.can?(:receive_notifications)
+ return unless allowed?
+ if expiring_soon
+ trigger_expiring_soon_notification
+ else
+ trigger_expired_notification
+ end
+ end
+
+ private
+
+ def allowed?
+ user.can?(:receive_notifications)
+ end
+
+ def trigger_expiring_soon_notification
+ notification_service.ssh_key_expiring_soon(user, keys.map(&:fingerprint))
+
+ keys.update_all(before_expiry_notification_delivered_at: Time.current.utc)
+ end
+
+ def trigger_expired_notification
notification_service.ssh_key_expired(user, keys.map(&:fingerprint))
keys.update_all(expiry_notification_delivered_at: Time.current.utc)
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 6e014c0a6709e4efef47b5798da095a50d5bb5f6..6f1f3309ad99eb9c31b7ce5b9d8e3c52b0659c9b 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -86,6 +86,13 @@ def ssh_key_expired(user, fingerprints)
mailer.ssh_key_expired_email(user, fingerprints).deliver_later
end
+ # Notify the user when at least one of their ssh key is expiring soon
+ def ssh_key_expiring_soon(user, fingerprints)
+ return unless user.can?(:receive_notifications)
+
+ mailer.ssh_key_expiring_soon_email(user, fingerprints).deliver_later
+ end
+
# Notify a user when a previously unknown IP or device is used to
# sign in to their account
def unknown_sign_in(user, ip, time)
diff --git a/app/views/notify/ssh_key_expiring_soon.text.erb b/app/views/notify/ssh_key_expiring_soon.text.erb
new file mode 100644
index 0000000000000000000000000000000000000000..2a7c0cafe83f8e6d4e0bd469e6451c522732fef5
--- /dev/null
+++ b/app/views/notify/ssh_key_expiring_soon.text.erb
@@ -0,0 +1,9 @@
+<%= _('Hi %{username}!') % { username: sanitize_name(@user.name) } %>
+
+<%= _('Your SSH keys with the following fingerprints are scheduled to expire soon:') %>
+
+<% @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 } %>
diff --git a/app/views/notify/ssh_key_expiring_soon_email.html.haml b/app/views/notify/ssh_key_expiring_soon_email.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..f4aee9c5fded1f034225e09770bb2af3f6562573
--- /dev/null
+++ b/app/views/notify/ssh_key_expiring_soon_email.html.haml
@@ -0,0 +1,13 @@
+%p
+ = _('Hi %{username}!') % { username: sanitize_name(@user.name) }
+%p
+ = _('Your SSH keys with the following fingerprints are scheduled to expire soon:')
+%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 }
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 359eb5499c760fa4e34858afd4cdbab23c29e502..24c96095ccc16a7fa8e6dbe09f9f0b0e87d080bd 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -451,6 +451,14 @@
:weight: 1
:idempotent: true
:tags: []
+- :name: cronjob:ssh_keys_expiring_soon_notification
+ :feature_category: :compliance_management
+ :has_external_dependencies:
+ :urgency: :low
+ :resource_boundary: :unknown
+ :weight: 1
+ :idempotent: true
+ :tags: []
- :name: cronjob:stuck_ci_jobs
:feature_category: :continuous_integration
:has_external_dependencies:
diff --git a/app/workers/ssh_keys/expired_notification_worker.rb b/app/workers/ssh_keys/expired_notification_worker.rb
index 57a9060b5734914ad2b51f040a5d32473eb762fd..ab6d1998773536d7fd60e853ca95800c67729984 100644
--- a/app/workers/ssh_keys/expired_notification_worker.rb
+++ b/app/workers/ssh_keys/expired_notification_worker.rb
@@ -17,7 +17,7 @@ def perform
keys = user.expired_today_and_unnotified_keys
- Keys::ExpiryNotificationService.new(user, { keys: keys }).execute
+ Keys::ExpiryNotificationService.new(user, { keys: keys, expiring_soon: false }).execute
end
end
end
diff --git a/app/workers/ssh_keys/expiring_soon_notification_worker.rb b/app/workers/ssh_keys/expiring_soon_notification_worker.rb
new file mode 100644
index 0000000000000000000000000000000000000000..3214cd7a24253836b59f1178c3f5bdd59aab4952
--- /dev/null
+++ b/app/workers/ssh_keys/expiring_soon_notification_worker.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module SshKeys
+ class ExpiringSoonNotificationWorker
+ 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_expiring_soon.find_each do |user|
+ with_context(user: user) do
+ Gitlab::AppLogger.info "#{self.class}: Notifying User #{user.id} about expiring soon ssh key(s)"
+
+ keys = user.expiring_soon_and_unnotified_keys
+
+ Keys::ExpiryNotificationService.new(user, { keys: keys, expiring_soon: true }).execute
+ end
+ end
+ end
+ end
+end
diff --git a/changelogs/unreleased/expiring-ssh-key-notification.yml b/changelogs/unreleased/expiring-ssh-key-notification.yml
new file mode 100644
index 0000000000000000000000000000000000000000..d1e14249df3d90b6b47e57246e02da5eb5fa5179
--- /dev/null
+++ b/changelogs/unreleased/expiring-ssh-key-notification.yml
@@ -0,0 +1,5 @@
+---
+title: User notification when SSH key is set to expire soon
+merge_request: 58171
+author:
+type: added
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 7e0c2778e854371e4be143ab84af81fe3a4f895d..c14df942ed886fa0c25594a7faff0a83b8150128 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -569,6 +569,9 @@
Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['namespaces_in_product_marketing_emails_worker']['cron'] ||= '0 9 * * *'
Settings.cron_jobs['namespaces_in_product_marketing_emails_worker']['job_class'] = 'Namespaces::InProductMarketingEmailsWorker'
+Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker']['cron'] ||= '0 1 * * *'
+Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker']['job_class'] = 'SshKeys::ExpiringSoonNotificationWorker'
Gitlab.com do
Settings.cron_jobs['batched_background_migrations_worker'] ||= Settingslogic.new({})
diff --git a/db/migrate/20210401175134_add_before_expiry_notification_delivered_to_keys.rb b/db/migrate/20210401175134_add_before_expiry_notification_delivered_to_keys.rb
new file mode 100644
index 0000000000000000000000000000000000000000..6a2ea0e738c5b743b6141173bba3810b1c79898d
--- /dev/null
+++ b/db/migrate/20210401175134_add_before_expiry_notification_delivered_to_keys.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddBeforeExpiryNotificationDeliveredToKeys < ActiveRecord::Migration[6.0]
+ DOWNTIME = false
+
+ def change
+ add_column :keys, :before_expiry_notification_delivered_at, :datetime_with_timezone
+ end
+end
diff --git a/db/migrate/20210401192808_add_index_to_keys_on_expires_at_and_before_expiry_notification_undelivered.rb b/db/migrate/20210401192808_add_index_to_keys_on_expires_at_and_before_expiry_notification_undelivered.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ff792d2e6e6c4ef6c8b0e9b95c1dabb60a21f948
--- /dev/null
+++ b/db/migrate/20210401192808_add_index_to_keys_on_expires_at_and_before_expiry_notification_undelivered.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddIndexToKeysOnExpiresAtAndBeforeExpiryNotificationUndelivered < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+ INDEX_NAME = 'idx_keys_expires_at_and_before_expiry_notification_undelivered'
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index :keys,
+ "date(timezone('UTC', expires_at)), before_expiry_notification_delivered_at",
+ where: 'before_expiry_notification_delivered_at IS NULL', name: INDEX_NAME
+ end
+
+ def down
+ remove_concurrent_index_by_name(:keys, INDEX_NAME)
+ end
+end
diff --git a/db/schema_migrations/20210401175134 b/db/schema_migrations/20210401175134
new file mode 100644
index 0000000000000000000000000000000000000000..8b93c13bfdc473ffa58f7cda4b66cdafd37db03a
--- /dev/null
+++ b/db/schema_migrations/20210401175134
@@ -0,0 +1 @@
+07d527134f776dbed2199f1717c34b3a6c41caadcaa3c50e6e5866f2cfad31b0
\ No newline at end of file
diff --git a/db/schema_migrations/20210401192808 b/db/schema_migrations/20210401192808
new file mode 100644
index 0000000000000000000000000000000000000000..fd005c98732a83f725602307adad07e1ce0073ab
--- /dev/null
+++ b/db/schema_migrations/20210401192808
@@ -0,0 +1 @@
+1cd4799ed7df41bfb9d96a7d18faaa9cbb2dc03f2a804c2bc3c1a6bba15d6d3d
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 1dd216b52da91d61ba66f8883e9322121d3da284..7bd7a5bfb26f5693e995190f91dc90e90c1363f7 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -13969,7 +13969,8 @@ CREATE TABLE keys (
last_used_at timestamp without time zone,
fingerprint_sha256 bytea,
expires_at timestamp with time zone,
- expiry_notification_delivered_at timestamp with time zone
+ expiry_notification_delivered_at timestamp with time zone,
+ before_expiry_notification_delivered_at timestamp with time zone
);
CREATE SEQUENCE keys_id_seq
@@ -21729,6 +21730,8 @@ CREATE INDEX idx_jira_connect_subscriptions_on_installation_id ON jira_connect_s
CREATE UNIQUE INDEX idx_jira_connect_subscriptions_on_installation_id_namespace_id ON jira_connect_subscriptions USING btree (jira_connect_installation_id, namespace_id);
+CREATE INDEX idx_keys_expires_at_and_before_expiry_notification_undelivered ON keys USING btree (date(timezone('UTC'::text, expires_at)), before_expiry_notification_delivered_at) WHERE (before_expiry_notification_delivered_at IS NULL);
+
CREATE INDEX idx_members_created_at_user_id_invite_token ON members USING btree (created_at) WHERE ((invite_token IS NOT NULL) AND (user_id IS NULL));
CREATE INDEX idx_merge_requests_on_id_and_merge_jid ON merge_requests USING btree (id, merge_jid) WHERE ((merge_jid IS NOT NULL) AND (state_id = 4));
diff --git a/doc/ssh/README.md b/doc/ssh/README.md
index 38356878822ba9bcd75dac72ef58be0d3dd7e173..87213f72534d00cecde50e47d48453130e472ca4 100644
--- a/doc/ssh/README.md
+++ b/doc/ssh/README.md
@@ -218,7 +218,8 @@ 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 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).
- 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.)
+ - 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.)
+ - GitLab checks all SSH keys at 01:00 AM UTC every day. It emails an expiration notice for all SSH keys that are scheduled to expire seven days from now. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/322637) in GitLab 13.11.)
1. Select **Add key**.
## Verify that you can connect
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8feaf10d5d8873a96c5949569387375be9e035da..ee7c82a3d4f7bacc244a9d1a112ff434354634d7 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -35592,12 +35592,18 @@ msgstr ""
msgid "Your SSH key has expired"
msgstr ""
+msgid "Your SSH key is expiring soon."
+msgstr ""
+
msgid "Your SSH key was deleted"
msgstr ""
msgid "Your SSH keys (%{count})"
msgstr ""
+msgid "Your SSH keys with the following fingerprints are scheduled to expire soon:"
+msgstr ""
+
msgid "Your SSH keys with the following fingerprints has expired:"
msgstr ""
diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb
index 764186dc226437e204c87837c25163c7933b8464..8ac1f15d67ebbc8be8f6124e1e269d95072cdebe 100644
--- a/spec/mailers/emails/profile_spec.rb
+++ b/spec/mailers/emails/profile_spec.rb
@@ -212,52 +212,101 @@
end
end
- describe 'notification email for expired ssh key' do
- let_it_be(:user) { create(:user) }
+ describe 'SSH key notification' do
+ let_it_be_with_reload(: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) }
+ shared_examples 'is sent to the user' do
+ it { is_expected.to deliver_to user.email }
+ end
+
+ shared_examples 'has the correct subject' do |subject_text|
+ it { is_expected.to have_subject subject_text }
+ end
+
+ shared_examples 'has the correct body text' do |body_text|
+ it { is_expected.to have_body_text body_text }
+ end
+
+ shared_examples 'includes a link to ssh key page' do
+ it { is_expected.to have_body_text /#{profile_keys_url}/ }
+ end
+
+ shared_examples 'includes the email reason' do
+ it { is_expected.to have_body_text /You're receiving this email because of your account on localhost/ }
+ end
+ shared_examples 'valid use case' do
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_behaves_like 'is sent to the user'
+ it_behaves_like 'includes a link to ssh key page'
+ it_behaves_like 'includes the email reason'
+ end
- it 'is sent to the user' do
- is_expected.to deliver_to user.email
+ shared_examples 'does not send email' do
+ it do
+ expect { subject }.not_to change { ActionMailer::Base.deliveries.count }
end
+ end
- it 'has the correct subject' do
- is_expected.to have_subject /Your SSH key has expired/
+ shared_context 'block user' do
+ before do
+ user.block!
end
+ 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
+ context 'notification email for expired ssh key' do
+ context 'when valid' do
+ subject { Notify.ssh_key_expired_email(user, fingerprints) }
- it 'includes a link to ssh key page' do
- is_expected.to have_body_text /#{profile_keys_url}/
+ include_examples 'valid use case'
+
+ it_behaves_like 'has the correct subject', /Your SSH key has expired/
+ it_behaves_like 'has the correct body text', /Your SSH keys with the following fingerprints has expired/
end
- it 'includes the email reason' do
- is_expected.to have_body_text /You're receiving this email because of your account on localhost/
+ context 'when invalid' do
+ context 'when user does not exist' do
+ subject { Notify.ssh_key_expired_email(nil, fingerprints) }
+
+ it_behaves_like 'does not send email'
+ end
+
+ context 'when user is not active' do
+ subject { Notify.ssh_key_expired_email(user, fingerprints) }
+
+ include_context 'block user'
+
+ it_behaves_like 'does not send email'
+ end
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
+ context 'notification email for expiring ssh key' do
+ context 'when valid' do
+ subject { Notify.ssh_key_expiring_soon_email(user, fingerprints) }
+
+ include_examples 'valid use case'
+
+ it_behaves_like 'has the correct subject', /Your SSH key is expiring soon/
+ it_behaves_like 'has the correct body text', /Your SSH keys with the following fingerprints are scheduled to expire soon/
end
- context 'when user is not active' do
- before do
- user.block!
+ context 'when invalid' do
+ context 'when user does not exist' do
+ subject { Notify.ssh_key_expiring_soon_email(nil, fingerprints) }
+
+ it_behaves_like 'does not send email'
end
- it do
- expect { Notify.ssh_key_expired_email(user) }.not_to change { ActionMailer::Base.deliveries.count }
+ context 'when user is not active' do
+ subject { Notify.ssh_key_expiring_soon_email(user, fingerprints) }
+
+ include_context 'block user'
+
+ it_behaves_like 'does not send email'
end
end
end
diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb
index 195ec1fa83b9ed87a6c57c1f742333c5499ffc8d..0cb20efcb0a7c5350aaf95892558cf63e9ea1ac3 100644
--- a/spec/models/key_spec.rb
+++ b/spec/models/key_spec.rb
@@ -76,15 +76,25 @@
end
end
- describe '.expired_today_and_not_notified' do
+ context 'expiration scopes' 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) }
+ let_it_be(:expiring_soon_unotified) { create(:key, expires_at: 3.days.from_now, user: user) }
+ let_it_be(:expiring_soon_notified) { create(:key, expires_at: 4.days.from_now, user: user, before_expiry_notification_delivered_at: Time.current) }
+ let_it_be(:future_expiry) { create(:key, expires_at: 1.month.from_now, user: user) }
+
+ describe '.expired_today_and_not_notified' do
+ it 'returns keys that expire today' do
+ expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified)
+ end
+ end
- it 'returns tokens that have expired today' do
- expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified)
+ describe '.expiring_soon_and_not_notified' do
+ it 'returns keys that will expire soon' do
+ expect(described_class.expiring_soon_and_not_notified).to contain_exactly(expiring_soon_unotified)
+ end
end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index ee67afcd50b334fa8f8a5e426367bbc12d109fef..520f616bfa049c3d0473b2afe1e569a1a81ccb88 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1001,15 +1001,24 @@
end
end
- describe '.with_ssh_key_expired_today' do
+ context 'SSH key expiration scopes' 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_not_notified) { create(:key, expires_at: Time.current, user: user1) }
let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user2, expiry_notification_delivered_at: Time.current) }
+ let_it_be(:expiring_soon_not_notified) { create(:key, expires_at: 2.days.from_now, user: user2) }
+ let_it_be(:expiring_soon_notified) { create(:key, expires_at: 2.days.from_now, user: user1, before_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)
+ describe '.with_ssh_key_expired_today' do
+ it 'returns users whose key has expired today' do
+ expect(described_class.with_ssh_key_expired_today).to contain_exactly(user1)
+ end
+ end
+
+ describe '.with_ssh_key_expiring_soon' do
+ it 'returns users whose keys will expire soon' do
+ expect(described_class.with_ssh_key_expiring_soon).to contain_exactly(user2)
+ end
end
end
diff --git a/spec/services/keys/expiry_notification_service_spec.rb b/spec/services/keys/expiry_notification_service_spec.rb
index 4cfd576a15f21041442b62e88cdfe7fa96c18d30..1d1da179cf78c2ff3b893f11fa0735921861e067 100644
--- a/spec/services/keys/expiry_notification_service_spec.rb
+++ b/spec/services/keys/expiry_notification_service_spec.rb
@@ -4,48 +4,93 @@
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 } }
+ let(:params) { { keys: user.keys, expiring_soon: expiring_soon } }
subject { described_class.new(user, params) }
- context 'with expired key', :mailer do
- let(:keys) { user.keys }
-
- it 'sends a notification' do
+ shared_examples 'sends a notification' do
+ it do
perform_enqueued_jobs do
subject.execute
end
should_email(user)
end
+ end
- it 'uses notification service to send email to the user' do
+ shared_examples 'uses notification service to send email to the user' do |notification_method|
+ it do
expect_next_instance_of(NotificationService) do |notification_service|
- expect(notification_service).to receive(:ssh_key_expired).with(expired_key.user, [expired_key.fingerprint])
+ expect(notification_service).to receive(notification_method).with(key.user, [key.fingerprint])
end
subject.execute
end
+ end
- it 'updates notified column' do
- expect { subject.execute }.to change { expired_key.reload.expiry_notification_delivered_at }
+ shared_examples 'does not send notification' do
+ it do
+ perform_enqueued_jobs do
+ subject.execute
+ end
+ should_not_email(user)
end
+ end
- context 'when user does not have permission to receive notification' do
- before do
- user.block!
+ shared_context 'block user' do
+ before do
+ user.block!
+ end
+ end
+
+ context 'with key expiring today', :mailer do
+ let_it_be_with_reload(:key) { create(:key, expires_at: Time.current, user: user) }
+
+ let(:expiring_soon) { false }
+
+ context 'when user has permission to receive notification' do
+ it_behaves_like 'sends a notification'
+
+ it_behaves_like 'uses notification service to send email to the user', :ssh_key_expired
+
+ it 'updates notified column' do
+ expect { subject.execute }.to change { key.reload.expiry_notification_delivered_at }
end
+ end
- it 'does not send notification' do
- perform_enqueued_jobs do
- subject.execute
- end
- should_not_email(user)
+ context 'when user does NOT have permission to receive notification' do
+ include_context 'block user'
+
+ it_behaves_like 'does not send notification'
+
+ it 'does not update notified column' do
+ expect { subject.execute }.not_to change { key.reload.expiry_notification_delivered_at }
+ end
+ end
+ end
+
+ context 'with key expiring soon', :mailer do
+ let_it_be_with_reload(:key) { create(:key, expires_at: 3.days.from_now, user: user) }
+
+ let(:expiring_soon) { true }
+
+ context 'when user has permission to receive notification' do
+ it_behaves_like 'sends a notification'
+
+ it_behaves_like 'uses notification service to send email to the user', :ssh_key_expiring_soon
+
+ it 'updates notified column' do
+ expect { subject.execute }.to change { key.reload.before_expiry_notification_delivered_at }
end
+ end
+
+ context 'when user does NOT have permission to receive notification' do
+ include_context 'block user'
+
+ it_behaves_like 'does not send notification'
it 'does not update notified column' do
- expect { subject.execute }.not_to change { expired_key.reload.expiry_notification_delivered_at }
+ expect { subject.execute }.not_to change { key.reload.before_expiry_notification_delivered_at }
end
end
end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 63bfc090a0dffcb956498f335f2648f9a3c8e7de..6eff768eac29c634a2d6e58ec52a34bb18a0d2df 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -288,11 +288,19 @@
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"] }
+ describe 'SSH Keys' do
+ let_it_be_with_reload(:user) { create(:user) }
+ let_it_be(:fingerprints) { ["aa:bb:cc:dd:ee:zz"] }
+ shared_context 'block user' do
+ before do
+ user.block!
+ end
+ end
+
+ describe '#ssh_key_expired' do
subject { notification.ssh_key_expired(user, fingerprints) }
it 'sends email to the token owner' do
@@ -300,15 +308,29 @@
end
context 'when user is not allowed to receive notifications' do
- before do
- user.block!
- end
+ include_context 'block user'
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
+
+ describe '#ssh_key_expiring_soon' do
+ subject { notification.ssh_key_expiring_soon(user, fingerprints) }
+
+ it 'sends email to the token owner' do
+ expect { subject }.to have_enqueued_email(user, fingerprints, mail: "ssh_key_expiring_soon_email")
+ end
+
+ context 'when user is not allowed to receive notifications' do
+ include_context 'block user'
+
+ it 'does not send email to the token owner' do
+ expect { subject }.not_to have_enqueued_email(user, fingerprints, mail: "ssh_key_expiring_soon_email")
+ end
+ end
+ end
end
describe '#unknown_sign_in' do
diff --git a/spec/workers/ssh_keys/expiring_soon_notification_worker_spec.rb b/spec/workers/ssh_keys/expiring_soon_notification_worker_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..f9276c86cdf2866d2cf5b2ca596e3c2c2d6ac8df
--- /dev/null
+++ b/spec/workers/ssh_keys/expiring_soon_notification_worker_spec.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SshKeys::ExpiringSoonNotificationWorker, 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_expiring_soon_notification',
+ 'queue_namespace' => :cronjob
+ )
+ end
+
+ describe '#perform' do
+ let_it_be(:user) { create(:user) }
+
+ context 'with key expiring soon' do
+ let_it_be_with_reload(:expiring_soon) { create(:key, expires_at: 6.days.from_now, 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 { expiring_soon.reload.before_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 { expiring_soon.reload.before_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.before_expiry_notification_delivered_at }
+ end
+ end
+
+ context 'when key is not expiring soon' do
+ let_it_be(:expires_future) { create(:key, expires_at: 8.days.from_now, user: user) }
+
+ it 'does not update notified column' do
+ expect { worker.perform }.not_to change { expires_future.reload.before_expiry_notification_delivered_at }
+ end
+ end
+ end
+end