Skip to content

Add Personal access token expiry policy

Sebastián Arcila Valenzuela requested to merge 3649-pat-expiry-policy into master

What does this MR do?

  1. It adds max_personal_access_token_lifetime to the admin sections. Screenshot_2019-10-14_at_17.50.18
  2. Everything starts if the user changes the value of max_personal_access_token_lifetime on the admin sections.
  3. A worker (PersonalAccessTokenPolicyWorker) is scheduled 3 hours into the future and:
    • Revoke all PersonalAccessToken.active with expires_at higher than the maximum lifetime and with expires_at not set.
    • Notify the users about the revoked tokens

Related issues

Related to #3649 (closed)

Migration output

== 20191112105448 AddIndexOnPersonalAccessTokensUserIdAndExpiresAt: migrating =
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:personal_access_tokens, [:user_id, :expires_at], {:name=>"index_pat_on_user_id_and_expires_at", :using=>:btree, :algorithm=>:concurrently})
   -> 0.0024s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- add_index(:personal_access_tokens, [:user_id, :expires_at], {:name=>"index_pat_on_user_id_and_expires_at", :using=>:btree, :algorithm=>:concurrently})
   -> 0.0142s
-- execute("RESET ALL")
   -> 0.0002s
== 20191112105448 AddIndexOnPersonalAccessTokensUserIdAndExpiresAt: migrated (0.0171s) 

Some numbers related to tokens and users

I pulled some approximated numbers for GitLab.com on how many tokens do we have and on which state they are, we have approximately 88994 tokens that are not revoked with an expires_at in the future and 1894773 with no expires_at.

Query plans

User.with_invalid_expires_at_tokens(expiration_date)

Raw SQL"

SELECT  "users".* FROM "users" WHERE "users"."id" IN 
  (SELECT "personal_access_tokens"."user_id" FROM (
    (SELECT  "personal_access_tokens".* FROM "personal_access_tokens" WHERE "personal_access_tokens"."revoked" = FALSE AND "personal_access_tokens"."expires_at" IS NULL LIMIT 1000)                                                                                                                  
    UNION
    (SELECT  "personal_access_tokens".* FROM "personal_access_tokens" WHERE "personal_access_tokens"."revoked" = FALSE AND (expires_at > '2019-12-14') LIMIT 1000)) 
    personal_access_tokens)

Plan:

Nested Loop  (cost=1306.07..2199.64 rows=2360470 width=1645) (actual time=36.645..2389.259 rows=1653 loops=1)
   Buffers: shared dirtied=157 hit=7185 read=2390
   ->  HashAggregate  (cost=1305.64..1307.64 rows=200 width=4) (actual time=32.199..34.012 rows=1653 loops=1)
         Group Key: personal_access_tokens.user_id
         Buffers: shared dirtied=44 hit=2478 read=444
         ->  HashAggregate  (cost=1260.64..1280.64 rows=2000 width=126) (actual time=30.959..31.596 rows=2000 loops=1)
               Group Key: personal_access_tokens.id, personal_access_tokens.user_id, personal_access_tokens.name, personal_access_tokens.revoked, personal_access_tokens.expires_at, personal_access_tokens.created_at, personal_access_tokens.updated_at, personal_access_tokens.scopes, personal_access_tokens.impersonation, personal_access_tokens.token_digest
               Buffers: shared dirtied=44 hit=2478 read=444
               ->  Append  (cost=0.43..1210.64 rows=2000 width=126) (actual time=0.209..29.230 rows=2000 loops=1)
                     Buffers: shared dirtied=44 hit=2478 read=444
                     ->  Limit  (cost=0.43..80.56 rows=1000 width=106) (actual time=0.209..5.228 rows=1000 loops=1)
                           Buffers: shared dirtied=11 hit=1060 read=7
                           ->  Index Scan using index_personal_access_tokens_on_user_id_and_expires_at on public.personal_access_tokens  (cost=0.43..148375.15 rows=1851560 width=106) (actual time=0.207..5.125 rows=1000 loops=1)
                                 Index Cond: (personal_access_tokens.expires_at IS NULL)
                                 Filter: (NOT personal_access_tokens.revoked)
                                 Rows Removed by Filter: 114
                                 Buffers: shared dirtied=11 hit=1060 read=7
                     ->  Limit  (cost=0.43..1110.07 rows=1000 width=106) (actual time=0.012..23.733 rows=1000 loops=1)
                           Buffers: shared dirtied=33 hit=1418 read=437
                           ->  Index Scan using index_personal_access_tokens_on_user_id_and_expires_at on public.personal_access_tokens personal_access_tokens_1  (cost=0.43..119009.72 rows=107250 width=106) (actual time=0.012..23.612 rows=1000 loops=1)
                                 Index Cond: (personal_access_tokens_1.expires_at > '2019-12-14'::date)
                                 Filter: (NOT personal_access_tokens_1.revoked)
                                 Rows Removed by Filter: 452
                                 Buffers: shared dirtied=33 hit=1418 read=437
   ->  Index Scan using users_pkey on public.users  (cost=0.43..4.45 rows=1 width=1645) (actual time=1.411..1.422 rows=1 loops=1653)
         Index Cond: (users.id = personal_access_tokens.user_id)
         Buffers: shared dirtied=113 hit=4707 read=1946

Does this MR meet the acceptance criteria?

Conformity

Performance 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