Skip to content

Ensure identity queries use lower query to match index

Drew Blessing requested to merge dblessing_rapid_action_identities_queries into master

What does this MR do?

Fixes #331465 (closed).

Ensures all identities queries use the with_extern_uid scope, which ensures the use of lower. In addition to the github provider identified in the issue, I found similar issues in the bitbucket and gitlab importers. I added a new scope to user and pointed all cases there.

Database index is on:

lower(extern_uid), provider

Database

Before:

SELECT
    "users".*
FROM
    "users"
    INNER JOIN "identities" ON "identities"."user_id" = "users"."id"
WHERE
  "identities"."provider" = 'github'
  AND "identities"."extern_uid" = 'my_github_id' 

After:

SELECT
    "users"."id"
FROM
    "users"
    INNER JOIN "identities" ON "identities"."user_id" = "users"."id"
WHERE (LOWER("identities"."extern_uid") = LOWER('my_github_id'))
    AND "identities"."provider" = 'github'
ORDER BY
    "users"."id" ASC
LIMIT 1

Explain: https://explain.depesz.com/s/TU43

 Limit  (cost=7.04..7.04 rows=1 width=4) (actual time=4.602..4.605 rows=0 loops=1)
   Buffers: shared hit=5 read=2
   I/O Timings: read=4.501 write=0.000
   ->  Sort  (cost=7.04..7.04 rows=1 width=4) (actual time=4.600..4.602 rows=0 loops=1)
         Sort Key: users.id
         Sort Method: quicksort  Memory: 25kB
         Buffers: shared hit=5 read=2
         I/O Timings: read=4.501 write=0.000
         ->  Nested Loop  (cost=0.99..7.03 rows=1 width=4) (actual time=4.556..4.558 rows=0 loops=1)
               Buffers: shared hit=2 read=2
               I/O Timings: read=4.501 write=0.000
               ->  Index Scan using index_on_identities_lower_extern_uid_and_provider on public.identities  (cost=0.56..3.57 rows=1 width=4) (actual time=4.553..4.554 rows=0 loops=1)
                     Index Cond: ((lower((identities.extern_uid)::text) = 'my_github_id'::text) AND ((identities.provider)::text = 'github'::text))
                     Buffers: shared hit=2 read=2
                     I/O Timings: read=4.501 write=0.000
               ->  Index Only Scan using users_pkey on public.users  (cost=0.43..3.45 rows=1 width=4) (actual time=0.000..0.000 rows=0 loops=0)
                     Index Cond: (users.id = identities.user_id)
                     Heap Fetches: 0
                     I/O Timings: read=0.000 write=0.000

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • 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 Drew Blessing

Merge request reports