Allow users to reuse unverified phone numbers used by banned users
What does this MR do and why?
Resolves https://gitlab.com/gitlab-org/modelops/anti-abuse/team-tasks/-/issues/530+
Problem
Previously we were banning new users automatically when they use a phone number that has already been used by a banned user even if the banned user didn't validate the phone number (i.e. banned user triggered verification code delivery to the phone number but didn't enter the code in GitLab to prove the phone number was theirs). This resulted to needing to delete phone number validation records associated with banned users just in case they didn't actually own the phone numbers they used.
Solution
This MR updates the logic of the method (Users::PhoneNumberValidation.related_to_banned_user?(international_dial_code, phone_number)
that checks if a phone number was previously used by a banned user. Now related_to_banned_user?
only returns true when there is a Users::PhoneNumberValidation
record that meets all of the following conditions:
- record belongs to a banned user
- (NEW) record
validated_at
is notnil
- meaning the banned user actually proved the phone number was theirs - record
international_dial_code
is equal to giveninternational_dial_code
- record
phone_number
is equal to givenphone_number
With this change we will only be auto-banning new users that we know were already banned before.
Database changes
Users::PhoneNumberValidation.related_to_banned_user?
class method
Before
SELECT
1 AS one
FROM
"user_phone_number_validations"
INNER JOIN "banned_users" ON "banned_users"."user_id" = "user_phone_number_validations"."user_id"
WHERE
"user_phone_number_validations"."international_dial_code" = 380
AND "user_phone_number_validations"."phone_number" = '689818097'
LIMIT 1
Explain:
https://console.postgres.ai/shared/cdc96f59-0747-405e-a1af-c72198026b29
Limit (cost=0.71..6.75 rows=1 width=4) (actual time=0.057..0.059 rows=1 loops=1)
Buffers: shared hit=10
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=0.71..6.75 rows=1 width=4) (actual time=0.056..0.057 rows=1 loops=1)
Buffers: shared hit=10
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_user_phone_validations_on_dial_code_phone_number on public.user_phone_number_validations (cost=0.42..3.44 rows=1 width=8) (actual time=0.046..0.046 rows=1 loops=1)
Index Cond: ((user_phone_number_validations.international_dial_code = 380) AND (user_phone_number_validations.phone_number = '689818097'::text))
Buffers: shared hit=7
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using banned_users_pkey on public.banned_users (cost=0.29..3.31 rows=1 width=8) (actual time=0.007..0.007 rows=1 loops=1)
Index Cond: (banned_users.user_id = user_phone_number_validations.user_id)
Heap Fetches: 0
Buffers: shared hit=3
I/O Timings: read=0.000 write=0.000
After
SELECT
1 AS one
FROM
"user_phone_number_validations"
INNER JOIN "banned_users" ON "banned_users"."user_id" = "user_phone_number_validations"."user_id"
WHERE
"user_phone_number_validations"."international_dial_code" = 380
AND "user_phone_number_validations"."phone_number" = '689818097'
AND "user_phone_number_validations"."validated_at" IS NOT NULL
LIMIT 1
Explain:
https://console.postgres.ai/shared/a16b1a95-3e00-4006-8c6a-7a510c446c59
Limit (cost=0.71..6.75 rows=1 width=4) (actual time=15.211..15.214 rows=1 loops=1)
Buffers: shared hit=5 read=5
I/O Timings: read=15.067 write=0.000
-> Nested Loop (cost=0.71..6.75 rows=1 width=4) (actual time=15.209..15.211 rows=1 loops=1)
Buffers: shared hit=5 read=5
I/O Timings: read=15.067 write=0.000
-> Index Scan using index_user_phone_validations_on_dial_code_phone_number on public.user_phone_number_validations (cost=0.42..3.44 rows=1 width=8) (actual time=12.704..12.704 rows=1 loops=1)
Index Cond: ((user_phone_number_validations.international_dial_code = 380) AND (user_phone_number_validations.phone_number = '689818097'::text))
Filter: (user_phone_number_validations.validated_at IS NOT NULL)
Rows Removed by Filter: 0
Buffers: shared hit=3 read=4
I/O Timings: read=12.609 write=0.000
-> Index Only Scan using banned_users_pkey on public.banned_users (cost=0.29..3.31 rows=1 width=8) (actual time=2.494..2.495 rows=1 loops=1)
Index Cond: (banned_users.user_id = user_phone_number_validations.user_id)
Heap Fetches: 0
Buffers: shared hit=2 read=1
I/O Timings: read=2.458 write=0.000
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.