Skip to content

Resolve "PAT/SSH Inventory MVC"

Manoj M J requested to merge 36742-pat-ssh-inventory-mvc into master

What does this MR do?

For #36742 (closed)

Introduces a new Credentials page in Admin UI, where admin can see the details of existing Personal Access Tokens and SSH Tokens.

Screenshots

SSH Keys

Screenshot_2019-12-04_at_1.12.43_PM

Personal Access Tokens

Screenshot_2019-12-05_at_12.15.27_PM

Does this MR meet the acceptance criteria?

Conformity

Database

Added a new index, since we are going to be sorting by the last_used_at column on keys table. Without an index, this was performing a sequential scan:

Results from database-lab:

For SSH Keys

explain SELECT * FROM keys WHERE type IN ('LDAPKey', 'Key') OR type IS NULL ORDER BY last_used_at DESC NULLS LAST LIMIT 20 OFFSET 0;

Before:

 Limit  (cost=1012561.57..1012561.62 rows=20 width=596) (actual time=77820.365..77820.377 rows=20 loops=1)
   Buffers: shared hit=460 read=227575 dirtied=2621 written=1690
   I/O Timings: read=75073.945 write=46.859
   ->  Sort  (cost=1012561.57..1018764.38 rows=2481124 width=596) (actual time=77820.362..77820.369 rows=20 loops=1)
         Sort Key: keys.last_used_at DESC NULLS LAST
         Sort Method: top-N heapsort  Memory: 40kB
         Buffers: shared hit=460 read=227575 dirtied=2621 written=1690
         I/O Timings: read=75073.945 write=46.859
         ->  Seq Scan on public.keys  (cost=0.00..946539.75 rows=2481124 width=596) (actual time=2.427..76770.731 rows=2508820 loops=1)
               Filter: (((keys.type)::text = ANY ('{LDAPKey,Key}'::text[])) OR (keys.type IS NULL))
               Rows Removed by Filter: 273310
               Buffers: shared hit=457 read=227575 dirtied=2621 written=1690
               I/O Timings: read=75073.945 write=46.859
Time: 1.140 min
  - planning: 2.393 ms
  - execution: 1.140 min
    - I/O read: 1.091 min
    - I/O write: 145.565 ms

https://explain.depesz.com/s/H1zs

After:

 Limit  (cost=0.43..6.53 rows=20 width=596) (actual time=0.044..0.256 rows=20 loops=1)
   Buffers: shared hit=26
   ->  Index Scan using index_keys_on_last_used_at on public.keys  (cost=0.43..764956.47 rows=2507432 width=596) (actual time=0.043..0.253 rows=20 loops=1)
         Filter: (((keys.type)::text = ANY ('{LDAPKey,Key}'::text[])) OR (keys.type IS NULL))
         Rows Removed by Filter: 3
         Buffers: shared hit=26
Time: 2.837 ms
  - planning: 2.457 ms
  - execution: 0.380 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

https://explain.depesz.com/s/JXfp

For Personal Access Tokens

explain SELECT "personal_access_tokens".* FROM "personal_access_tokens" WHERE "personal_access_tokens"."impersonation" = false AND (revoked = false AND (expires_at >= '20191210' OR expires_at IS NULL)) ORDER BY "personal_access_tokens"."id" DESC LIMIT 20

https://explain.depesz.com/s/upeK

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 Manoj M J

Merge request reports

Loading