Ensure attempt is allowed before CreditCardValidation record is created
Fixes https://gitlab.com/gitlab-org/modelops/anti-abuse/team-tasks/-/issues/707
Context
Identity Verification prevents users from doing (or re-doing) verification methods that is not currently active.
For example, if the user is currently doing phone number verification step after doing email verification as shown in the following screenshot
The user will not be allowed to
- hit email verification endpoints (because they already completed that step)
- hit credit card verification endpoints (because they haven't completed phone number verification step yet)
How credit card verification works
- User enters credit card details
- User (optionally) solves Arkose challenge
- GitLab sends POST request to
verify_credit_card_captcha
to ensure user solved Arkose challenge if it was shown - User clicks on
Verify payment method
- GitLab sends request to Customers to add create payment method for the user -> Customers sends API request to GitLab to create a
CreditCardValidation
record for the user (this marks the user as "credit card verified") - GitLab sends GET request to
verify_credit_card
to ensure user hasCreditCardValidation
record - GitLab checks if user is allowed to access
verify_credit_card
endpoint. Proceeds if so. - GitLab redirects user to Identity Verification success page
Problem
The bug this MR tries to fix happens in step 8 above. ensure_verification_method_attempt_allowed!
before action hook returns 400 error because at this point the user is already marked as having already completed credit card verification (CreditCardValidation
record already exists) so it doesn't allow access to the endpoint.
What does this MR do?
This MR moves the call to ensure_verification_method_attempt_allowed!
before step 5 (CreditCardValidation
record is created).
The before action hooks is applied to verify_credit_card_captcha
instead of verify_credit_card
action.
Where is the regression test?
This error is not caught by existing feature specs because the actual credit card verification process in specs is "bypassed" so the specs doesn't actually hit the verify_credit_card
endpoint.
it 'verifies the user' do
expect_to_see_identity_verification_page
verify_email unless skip_email_validation
request_phone_exemption
solve_arkose_verify_challenge
verify_credit_card
# verify_credit_card creates a credit_card verification record &
# refreshes the page. This causes an automatic redirect to the welcome
# page, skipping the verification successful badge, and preventing us
# from calling expect_verification_completed
expect_to_see_dashboard_page
end
This is not ideal, of course, but because of the complexity of how the credit card verification works (to simplify, it loads Zuora script -> render iframe that contains credit card form -> API calls to the Customers -> Customers communicates with Zuora -> Customers communicates with GitLab) this compromise was necessary.
Because of this, we also cannot add a regression test for this bug until we have the ideal spec for the credit card verification process where the whole process or a simulation of it happens.
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
Screen_Recording_2024-05-29_at_11.33.32_AM | Screen_Recording_2024-05-29_at_11.36.05_AM |
How to set up and validate locally
-
Ensure that you have a working local CustomersDot installation with Zuora integration and
zuora_cc_registration_validation_payment_page_id
set to an active hosted payment page -
Start GDK simulating SaaS
$ export GITLAB_SIMULATE_SAAS=1 $ gdk start
-
Enable FFs
$ rails c > Feature.enable(:identity_verification) > Feature.enable(:identity_verification_phone_number) > Feature.enable(:identity_verification_credit_card)
-
Create a new user
-
Set the new user's Arkose risk score to
'Medium'
and set the user'sconfirmed_at
$ rails c > User.last.custom_attributes.find_by_key('arkose_risk_band').update(value: 'Medium') > User.last.update(confirmed_at: Time.now)
-
Go back to your browser and validate that email is already confirmed and phone number is now required
-
Click on
Verify with a credit card instead
-
Fill in the form and click on
Verify payment method
Cardholder name: any Card number: 4242 4242 4242 4242 Expiry Date: Any future date CVV: Any 3-digit number
-
Verify that no error is shown and you are redirected to the success page