Skip to content

Expiry email notification for SSH keys [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Aishwarya Subramanian requested to merge ssh_key_expiration into master

What does this MR do?

This MR adds changes to notify users via email when their SSH key has expired (irrespective of whether expiration is enforced or not). The cron job is set to run at 2:00am UTC everyday.

In order to make the operation of notifying users idempotent, a new column on_expiry_notification_delivered_at is introduced in the keys table to track when a user gets the expiration notification.

Omnibus changes: omnibus-gitlab!5119 (merged)

Mentions #322637 (closed)

Database migrations

Up
== 20210317192943 AddExpiryNotificationDeliveredToKeys: migrating =============
-- add_column(:keys, :expiry_notification_delivered_at, :datetime_with_timezone)
   -> 0.0027s
== 20210317192943 AddExpiryNotificationDeliveredToKeys: migrated (0.0028s) ====

== 20210322182751 AddIndexToKeysOnExpiresAtAndExpiryNotificationUndelivered: migrating
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:keys, "date(timezone('UTC', expires_at)), expiry_notification_delivered_at", {:where=>"expiry_notification_delivered_at IS NULL", :name=>"index_keys_on_expires_at_and_expiry_notification_undelivered", :algorithm=>:concurrently})
   -> 0.0046s
-- execute("SET statement_timeout TO 0")
   -> 0.0007s
-- add_index(:keys, "date(timezone('UTC', expires_at)), expiry_notification_delivered_at", {:where=>"expiry_notification_delivered_at IS NULL", :name=>"index_keys_on_expires_at_and_expiry_notification_undelivered", :algorithm=>:concurrently})
   -> 0.0039s
-- execute("RESET ALL")
   -> 0.0007s
== 20210322182751 AddIndexToKeysOnExpiresAtAndExpiryNotificationUndelivered: migrated (0.0111s)
Down
== 20210317192943 AddExpiryNotificationDeliveredToKeys: reverting =============
-- remove_column(:keys, :expiry_notification_delivered_at, :datetime_with_timezone)
   -> 0.0035s
== 20210317192943 AddExpiryNotificationDeliveredToKeys: reverted (0.0072s) ====

== 20210322182751 AddIndexToKeysOnExpiresAtAndExpiryNotificationUndelivered: reverting
-- transaction_open?()
   -> 0.0000s
-- indexes(:keys)
   -> 0.0053s
-- remove_index(:keys, {:algorithm=>:concurrently, :name=>"index_keys_on_expires_at_and_expiry_notification_undelivered"})
   -> 0.0034s
== 20210322182751 AddIndexToKeysOnExpiresAtAndExpiryNotificationUndelivered: reverted (0.0101s)

Database queries

User.with_ssh_key_expired_today

Query:

SELECT
    "users".*
FROM
    "users"
WHERE (EXISTS (
        SELECT
            1
        FROM
            "keys"
        WHERE (keys.user_id = users.id)
        AND (date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE
            AND on_expiry_notification_delivered_at IS NULL)))

Details and visualization:

Before index After index
Link Link

Execution time:

Time: 15.720 ms
  - planning: 11.998 ms
  - execution: 3.722 ms
    - I/O read: 1.187 ms
    - I/O write: N/A

Shared buffers:
  - hits: 397 (~3.10 MiB) from the buffer pool
  - reads: 12 (~96.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0
Key.expired_today_and_not_notified

Query:

SELECT
    "keys".*
FROM
    "keys"
WHERE (date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE
    AND on_expiry_notification_delivered_at IS NULL)

Details and visualizations:

Before index After index
Link Link

Execution time:

Time: 3.433 ms
  - planning: 2.234 ms
  - execution: 1.199 ms
    - I/O read: N/A
    - I/O write: N/A

Shared buffers:
  - hits: 85 (~680.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Screenshots (strongly suggested)

Email copy:

Screen_Shot_2021-03-29_at_11.43.14_PM

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Mayra Cabrera

Merge request reports