Personal Access Token Expiration Notification email

What does this MR do?

It adds a worker (cron) that runs every day at 1AM that will notify the user that some of its tokens are about to be expired.

Database checklist

When adding migrations:

• Updated db/schema.rb
• Added a down method so the migration can be reverted
• Added the output of the migration(s) to the MR body

Related issues

• Omnibus related changes omnibus-gitlab!3679 (merged)
• Related to #3649 (closed)

Approximated numbers for GitLab.com on how many tokens do we have and on which state they are

explain select count(*) from personal_access_tokens where revoked = false AND expires_at >= '2019-11-04'

 Aggregate  (cost=4796.77..4796.78 rows=1 width=8) (actual time=30.555..30.556 rows=1 loops=1)
   Buffers: shared hit=13293
   ->  Index Only Scan using index_personal_access_tokens_on_expires_at_and_revoked_false on public.personal_access_tokens  (cost=0.43..4526.18 rows=108236 width=0) (actual time=0.055..22.774 rows=88994 loops=1)
         Index Cond: (personal_access_tokens.expires_at >= '2019-11-04'::date)
         Heap Fetches: 8234
         Buffers: shared hit=13293

explain select count(*) from personal_access_tokens where revoked = false AND expires_at IS NULL

 Aggregate  (cost=50235.68..50235.69 rows=1 width=8) (actual time=489.295..489.295 rows=1 loops=1)
   Buffers: shared dirtied=104 hit=1353 read=5178
   ->  Index Only Scan using index_personal_access_tokens_on_expires_at_and_revoked_false on public.personal_access_tokens  (cost=0.43..45612.86 rows=1849128 width=0) (actual time=0.274..337.365 rows=1894773 loops=1)
         Index Cond: (personal_access_tokens.expires_at IS NULL)
         Heap Fetches: 37608
         Buffers: shared dirtied=104 hit=1353 read=5178

So it looks like we have 88994 tokens that are not revoked with an expires_at in the future and 1894773 with no expires_at.

And as for the users with tokens that are expiring soon (note that expire_notification_delivered_at will help to reduce the extra notifications and the amount of notified users each day) for today Nov 4th we have the following

explain SELECT count(DISTINCT "users".*) FROM "users" INNER JOIN "personal_access_tokens" ON "personal_access_tokens"."user_id" = "users"."id" WHERE (revoked = false AND expires_at >= NOW() AND expires_at <= '2019-11-11 13:18:21.047503')

 Aggregate  (cost=1906.20..1906.21 rows=1 width=8) (actual time=954.845..954.845 rows=1 loops=1)
   Buffers: shared dirtied=3 hit=1264 read=869
   ->  Nested Loop  (cost=0.86..1905.40 rows=320 width=1664) (actual time=3.379..945.024 rows=423 loops=1)
         Buffers: shared dirtied=3 hit=1245 read=865
         ->  Index Scan using index_personal_access_tokens_on_expires_at_and_revoked_false on public.personal_access_tokens  (cost=0.43..480.20 rows=320 width=4) (actual time=0.211..5.181 rows=423 loops=1)
               Index Cond: ((personal_access_tokens.expires_at >= now()) AND (personal_access_tokens.expires_at <= '2019-11-11'::date))
               Buffers: shared dirtied=2 hit=414 read=4
         ->  Index Scan using users_pkey on public.users  (cost=0.43..4.44 rows=1 width=1668) (actual time=2.218..2.219 rows=1 loops=423)
               Index Cond: (users.id = personal_access_tokens.user_id)
               Buffers: shared dirtied=1 hit=831 read=861

We have for today around 423 that would have received the notification about their personal tokens expiring soon.

explain SELECT count(DISTINCT "users".*) FROM "users" INNER JOIN "personal_access_tokens" ON "personal_access_tokens"."user_id" = "users"."id" WHERE (revoked = false AND expires_at >= NOW() AND expires_at <= '2019-12-04')

 Aggregate  (cost=19335.76..19335.77 rows=1 width=8) (actual time=89.438..89.438 rows=1 loops=1)
   Buffers: shared hit=15709
   ->  Nested Loop  (cost=0.86..19327.40 rows=3345 width=1664) (actual time=0.078..37.259 rows=3161 loops=1)
         Buffers: shared hit=15686
         ->  Index Scan using index_personal_access_tokens_on_expires_at_and_revoked_false on public.personal_access_tokens  (cost=0.43..4854.70 rows=3345 width=4) (actual time=0.052..10.623 rows=3161 loops=1)
               Index Cond: ((personal_access_tokens.expires_at >= now()) AND (personal_access_tokens.expires_at <= '2019-12-04'::date))
               Buffers: shared hit=3023
         ->  Index Scan using users_pkey on public.users  (cost=0.43..4.32 rows=1 width=1668) (actual time=0.008..0.008 rows=1 loops=3161)
               Index Cond: (users.id = personal_access_tokens.user_id)
               Buffers: shared hit=12663

This result shows around 3.1k users for a month so after the first past of the worker, will be notifying around 100 users per day about their tokens

Query plans

 User.with_expiring_and_not_notified_personal_access_tokens(7.days.from_now).limit(1000)

Raw sql

SELECT "users".* FROM "users" 
  WHERE 
  (EXISTS 
    (SELECT 1 FROM "personal_access_tokens"
     WHERE 
        (personal_access_tokens.user_id = users.id) AND 
        (revoked = false AND 
         expire_notification_delivered = false AND 
         expires_at >= CURRENT_DATE AND 
         expires_at <= '2019-11-21 20:54:15.241017'))) LIMIT 1000

Plan with execution:

 Limit  (cost=42369.78..42378.28 rows=2 width=1645) (actual time=58.167..61.633 rows=408 loops=1)
   Buffers: shared hit=11048
   ->  Nested Loop  (cost=42369.78..42378.28 rows=2 width=1645) (actual time=58.165..61.589 rows=408 loops=1)
         Buffers: shared hit=11048
         ->  Unique  (cost=42369.35..42369.36 rows=2 width=4) (actual time=58.131..58.246 rows=408 loops=1)
               Buffers: shared hit=9412
               ->  Sort  (cost=42369.35..42369.36 rows=2 width=4) (actual time=58.130..58.181 rows=432 loops=1)
                     Sort Key: personal_access_tokens.user_id
                     Sort Method: quicksort  Memory: 45kB
                     Buffers: shared hit=9412
                     ->  Index Scan using index_personal_access_tokens_on_user_id_and_expires_at on public.personal_access_tokens  (cost=0.43..42369.34 rows=2 width=4) (actual time=0.487..57.921 rows=432 loops=1)
                           Index Cond: ((personal_access_tokens.expires_at >= ('now'::cstring)::date) AND (personal_access_tokens.expires_at <= '2019-11-21'::date))
                           Filter: ((NOT personal_access_tokens.revoked) AND (personal_access_tokens.expire_notification_delivered = false))
                           Rows Removed by Filter: 180
                           Buffers: shared hit=9409
         ->  Index Scan using users_pkey on public.users  (cost=0.43..4.45 rows=1 width=1645) (actual time=0.007..0.008 rows=1 loops=408)
               Index Cond: (users.id = personal_access_tokens.user_id)
               Buffers: shared hit=1636

changed milestone to %12.5

added personal access tokens backend databasereview pending devopsmanage groupauthentication and authorization [DEPRECATED] typefeature labels

added 1 commit

• 15ebeecd - Add locale/gitlab.pot changes

Compare with previous version

added 1 commit

• dabfcec0 - Add changelog entry

Compare with previous version

changed the description

mentioned in merge request omnibus-gitlab!3679 (merged)

added database label

	1 Warning
	This merge request includes more than 10 commits. Please rebase these commits into a smaller number of commits.

	1 Message
	This merge request adds or changes files that require a review from the Database team.

This merge request requires a database review. To make sure these changes are reviewed, take the following steps:

1. Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
2. Use the Database changes checklist template or add the appropriate items to the MR description.
3. Assign and mention the database reviewer suggested by Reviewer Roulette.

The following files require a review from the Database team:

• db/migrate/20191014123159_add_expire_notification_delivered_to_personal_access_tokens.rb
• db/schema.rb

Reviewer roulette

Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.

To spread load more evenly across eligible reviewers, Danger has randomly picked a candidate for each review slot. Feel free to override this selection if you think someone else would be better-suited, or the chosen person is unavailable.

Once you've decided who will review this merge request, mention them as you normally would! Danger does not (yet?) automatically notify them for you.

Category	Reviewer	Maintainer
backend	Felipe Artur (@felipe_artur)	Rémy Coutable (@rymai)
frontend	Ragnar Hardarson (@rhardarson)	Phil Hughes (@iamphill)
database	Toon Claes (@toon)	Mayra Cabrera (@mayra-cabrera)

Generated by Danger

added 4 commits

• 92e7b3dd - Add worker pat expiring worker
• 27ffe518 - Cronjob configuration for PersonalAccessTokenExpiringWorker
• eb266716 - Add locale/gitlab.pot changes
• 5d161053 - Add changelog entry

Compare with previous version

added 5 commits

• d32bc7bc - Add expiring_and_not_notified scopes to PAT
• 7d3d3e84 - Add worker pat expiring worker
• c609cc85 - Cronjob configuration for PersonalAccessTokenExpiringWorker
• ed21e213 - Add locale/gitlab.pot changes
• 87adc3b7 - Add changelog entry

Compare with previous version

mentioned in merge request !17344 (merged)

added 1 commit

• f35ba6e4 - Move query logic outside PAT expiring worker to user scopes

Compare with previous version

changed the description

added 8 commits

• 9475c7e1 - Add mailer for access_token_about_to_expire_email
• 241aa9f1 - Add pat about to expire to notification service
• d690ec07 - Add expiring_and_not_notified scopes to PAT
• da9fdcb5 - Add worker pat expiring worker
• 5acd0349 - Cronjob configuration for PersonalAccessTokenExpiringWorker
• ce3e759e - Add locale/gitlab.pot changes
• 70d4ef5b - Add changelog entry
• 44882ab8 - Move query logic outside PAT expiring worker to user scopes

Compare with previous version

added 7 commits

• b40c9f0c - Add pat about to expire to notification service
• f0049293 - Add expiring_and_not_notified scopes to PAT
• 065d97d8 - Add worker pat expiring worker
• 35b2d60b - Cronjob configuration for PersonalAccessTokenExpiringWorker
• 570abee0 - Add locale/gitlab.pot changes
• 23f3f4c9 - Add changelog entry
• 4cd22c99 - Move query logic outside PAT expiring worker to user scopes

Compare with previous version

assigned to @igor.drozdov, @dennis, and @a_akgun

The pipeline is failing at rspec ./spec/workers/personal_access_token_expiring_worker_spec.rb:20 with:

       expected `pat.reload.expire_notification_delivered_at` to have changed to 2019-10-29 16:10:55.015589723 +0000, but is now 2019-10-29 16:10:55.015589000 +0000

even though it is in a Timecop.freeze block, am I missing something? !19296 (diffs)

unassigned @dennis