Skip to content
Snippets Groups Projects

Add increasing wait time between phone verification code sends

Merged Add increasing wait time between phone verification code sends
el-add-phone-verification-endpoints-delay into master
Merged Eugie Limpin requested to merge el-add-phone-verification-endpoints-delay into master

What does this MR do and why?

Implements https://gitlab.com/gitlab-org/modelops/anti-abuse/team-tasks/-/issues/539+

Add increasing wait time between subsequent phone number verification code SMS sends after the first one.

Why?

See https://gitlab.com/gitlab-org/modelops/anti-abuse/team-tasks/-/issues/539.

Database changes

  • sms_sent_at (smallint) and sms_send_count (datetime_with_timezone) columns are added to users_phone_number_validations table
  • No new or modified queries

Screenshots or screen recordings

SMS send number
1 Screen_Recording_2023-11-28_at_5.37.11_PM
2 Screen_Recording_2023-11-28_at_5.38.43_PM
3 Screen_Recording_2023-11-28_at_5.41.42_PM
4 Screen_Recording_2023-11-28_at_5.46.48_PM
5 (rate limited) Screen_Recording_2023-11-28_at_5.56.53_PM

How to set up and validate locally

  1. Enable the relevant feature flags

    > Feature.enable(:arkose_labs_signup_challenge)
> Feature.enable(:identity_verification_phone_number)
> Feature.enable(:identity_verification)
> Feature.enable(:sms_send_wait_time)

  2. Configure application settings for Identity Verification

    > ApplicationSetting.first.update(email_confirmation_setting: 'hard')
> ApplicationSetting.first.update(arkose_labs_public_api_key: "XXX", arkose_labs_private_api_key: "YYY", require_admin_approval_after_user_signup: false)
> ApplicationSetting.first.update(telesign_customer_xid: 'XXX', telesign_api_key: 'YYY')

    Note: credentials are in 1Password under Telesign API keys (Development) and ArkoseLabs API keys (Development)

  3. Register a new user

  4. Force user to have medium risk

    > User.last.custom_attributes.by_key('arkose_risk_band').first.update!(value: 'Medium')

  5. Verify the user's email

    > User.last.update(confirmed_at: Time.zone.now)

  6. On the phone verification step, send a code to a valid phone number

  7. Verify that resend links and buttons are disabled and displays a 1 minute wait time

  8. Wait for the wait time to expire then send another code

  9. Verify that resend links and buttons are disabled and displays the appropriate wait time (3 minutes, 5 minutes, 10 minutes, rate limited). See the demo videos above.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Eugie Limpin

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Eugie Limpin changed milestone to %16.7

    changed milestone to %16.7

  • Eugie Limpin added featureenhancement groupanti-abuse typefeature labels

    added featureenhancement groupanti-abuse typefeature labels

  • Eugie Limpin assigned to @eugielimpin

    assigned to @eugielimpin

  • 🤖 GitLab Bot 🤖 added devopsgovern sectionsec labels

    added devopsgovern sectionsec labels

  • A deleted user added backend database databasereview pending frontend labels

    added backend database databasereview pending frontend labels

  • Ghost User
    Ghost User @ghost1
    Contributor
    5 Warnings
    :warning: This merge request is quite big (650 lines changed), please consider splitting it into multiple merge requests.
    :warning: 5fcb6392: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.
    :warning:

    featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.

    For more information, see:
    :warning: This merge request contains lines with testid selectors. Please ensure e2e:package-and-test job is run.
    :warning:

    This merge request changed undocumented Vue components in vue_shared/. Please consider creating Stories for these components:

    • app/assets/javascripts/vue_shared/components/gl_countdown.vue

    testid selectors

    The following changed lines in this MR contain testid selectors:

    ee/app/assets/javascripts/users/identity_verification/components/international_phone_input.vue

    +      data-testid="submit-btn"

    ee/app/assets/javascripts/users/identity_verification/components/verify_phone_verification_code.vue

    +          <gl-link data-testid="resend-code-link" @click="resendCode">{{ content }}</gl-link>

    If the e2e:package-and-test job in the qa stage has run automatically, please ensure the tests are passing. If the job has not run, please start the trigger-omnibus-and-follow-up-e2e job in the qa stage and ensure the tests in follow-up-e2e:package-and-test-ee pipeline are passing.

    For the list of known failures please refer to the latest pipeline triage issue.

    If your changes are under a feature flag, please check our Testing with feature flags documentation for instructions.

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    backend @panoskanell profile link current availability (UTC+2, 6 hours behind author) @rymai profile link current availability (UTC+1, 7 hours behind author)
    database @morefice profile link current availability (UTC+1, 7 hours behind author) @jon_jenkins profile link current availability (UTC-6, 14 hours behind author)
    frontend @dzubova profile link current availability (UTC+1, 7 hours behind author) @jannik_lehmann profile link current availability (UTC+1, 7 hours behind author)
    ~"Authentication" Reviewer review is optional for ~"Authentication" @sgarg_gitlab profile link current availability (UTC+5.5, 2.5 hours behind author)

    Please check reviewer's status!

    • available Reviewer is available!
    • unavailable Reviewer is unavailable!

    Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

    Edited by Ghost User
  • A deleted user added Data WarehouseImpact Check label

    added Data WarehouseImpact Check label

  • Ghost User
    Ghost User @ghost1
    Contributor

    Bundle size analysis [beta]

    This compares changes in bundle size for entry points between the commits 72a25905 and fdbea1b6

    :sparkles: Special assets

    Entrypoint / Name Size before Size after Diff Diff in percent
    average 4.12 MB 4.12 MB - 0.0 %
    mainChunk 3.09 MB 3.09 MB - 0.0 %

    Note: We do not have exact data for 72a25905. So we have used data from: 51c5dd0d.
    The intended commit has no webpack pipeline, so we chose the last commit with one before it.

    Please look at the full report for more details

    Read more about how this report works.

    Generated by :no_entry_sign: Danger

    Edited by Ghost User
  • Ghost User
    Ghost User @ghost1
    Contributor

    E2E Test Result Summary

    allure-report-publisher generated test report!

    e2e-test-on-gdk: :white_check_mark: test report for fdbea1b6

    expand test summary 
    +------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Create      | 54     | 0      | 7       | 0     | 61    | ✅     |
| Data Stores | 23     | 0      | 0       | 0     | 23    | ✅     |
| Govern      | 67     | 0      | 0       | 0     | 67    | ✅     |
| Plan        | 54     | 0      | 1       | 0     | 55    | ✅     |
| Verify      | 31     | 0      | 0       | 0     | 31    | ✅     |
| Analytics   | 2      | 0      | 0       | 0     | 2     | ✅     |
| Release     | 5      | 0      | 0       | 0     | 5     | ✅     |
| Package     | 15     | 0      | 1       | 0     | 16    | ✅     |
| Monitor     | 7      | 0      | 0       | 0     | 7     | ✅     |
| Manage      | 0      | 0      | 1       | 0     | 1     | ➖     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 258    | 0      | 10      | 0     | 268   | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

    e2e-package-and-test: :white_check_mark: test report for fdbea1b6

    expand test summary 
    +------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Govern      | 170    | 0      | 12      | 4     | 182   | ✅     |
| Create      | 16     | 0      | 4       | 0     | 20    | ✅     |
| Data Stores | 4      | 0      | 0       | 0     | 4     | ✅     |
| Monitor     | 8      | 0      | 0       | 0     | 8     | ✅     |
| Plan        | 8      | 0      | 0       | 0     | 8     | ✅     |
| Package     | 0      | 0      | 2       | 0     | 2     | ➖     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 206    | 0      | 18      | 4     | 224   | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

    e2e-review-qa: :white_check_mark: test report for fdbea1b6

    expand test summary 
    +------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Package     | 0      | 0      | 1       | 0     | 1     | ➖     |
| Create      | 8      | 0      | 2       | 1     | 10    | ✅     |
| Monitor     | 4      | 0      | 0       | 0     | 4     | ✅     |
| Govern      | 3      | 0      | 0       | 0     | 3     | ✅     |
| Plan        | 3      | 0      | 1       | 0     | 4     | ✅     |
| Data Stores | 2      | 0      | 0       | 0     | 2     | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 20     | 0      | 4       | 1     | 24    | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
    Edited by Ghost User
  • Eugie Limpin added 1 commit

    added 1 commit

    Compare with previous version

  • Eugie Limpin added 276 commits

    added 276 commits

    Compare with previous version

  • A deleted user added feature flag label

    added feature flag label

  • Eugie Limpin mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@84be021d

    mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@84be021d

  • Eugie Limpin changed title from Add increasing delay between phone verification code send attempts to Add increasing wait time between phone verification code sends

    changed title from Add increasing delay between phone verification code send attempts to Add increasing wait time between phone verification code sends

  • Ghost User
    Ghost User @ghost1
    Contributor

    Database migrations (on the main database)

    Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).

    Migration Type Total runtime Result DB size change
    20231124022520 - AddSmsSentAtAndSmsSendCountToPhoneNumberValidations Regular 1.8 s :white_check_mark: +0.00 B
    Runtime Histogram for all migrations
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 5
    0.1 seconds - 1 second 0
    1 second - 5 seconds 0
    5 seconds - 15 seconds 0
    15 seconds - 5 minutes 0
    5 minutes + 0

    Migration: 20231124022520 - AddSmsSentAtAndSmsSendCountToPhoneNumberValidations

    • Type: Regular
    • Duration: 1.8 s
    • Database size change: +0.00 B
    Calls Total Time Max Time Mean Time Rows Query
    1 2.8 ms 2.8 ms 2.8 ms 0 
    ALTER TABLE "user_phone_number_validations" ADD "sms_send_count" smallint DEFAULT 0 NOT NULL
    1 0.7 ms 0.7 ms 0.7 ms 0 
    ALTER TABLE "user_phone_number_validations" ADD "sms_sent_at" timestamptz
    1 0.0 ms 0.0 ms 0.0 ms 1 
    SELECT $1::regtype::oid
    2 0.0 ms 0.0 ms 0.0 ms 2 
    SELECT pg_backend_pid()
    Histogram for AddSmsSentAtAndSmsSendCountToPhoneNumberValidations
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 5
    0.1 seconds - 1 second 0
    1 second - 5 seconds 0
    5 seconds - 15 seconds 0
    15 seconds - 5 minutes 0
    5 minutes + 0

    Other information

    Other migrations pending on GitLab.com
    Migration Type Total runtime Result DB size change
    20231029142649 - AddMakeProfilePrivateApplicationSetting Regular 2.4 s :white_check_mark: +0.00 B
    20231115101002 - AddPromoteUltimateFeaturesAtToOnboardingProgresses Regular 2.6 s :white_check_mark: +0.00 B
    20231117031416 - AddWebIdeOauthApplicationToSettings Regular 1.9 s :white_check_mark: +0.00 B
    20231117031559 - AddFkWebIdeOauthApplication Regular 4.1 s :white_check_mark: +16.00 KiB
    20231120090305 - CreateOrganizationDetails Regular 2.0 s :warning: +24.00 KiB
    20231121133727 - ChangeMarketingEmailsNullConditions Regular 4.8 s :warning: +0.00 B
    20231123165947 - ChangeFkToMemberRoleOnMembersFromCascadeToNullify Regular 3.0 s :warning: +0.00 B
    20231116105945 - RequeueBackfillFindingIdInVulnerabilities2 Post deploy 1.9 s :white_check_mark: +0.00 B
    20231122100006 - RemoveCustomEmailSmtpColumnsFromServiceDeskSettings Post deploy 2.2 s :white_check_mark: +0.00 B
    20231122110442 - DropIndexWebHooksOnProjectId Post deploy 2.4 s :white_check_mark: -41.32 MiB
    20231122111935 - DropIndexWebHookLogsPartOnWebHookId Post deploy 2.4 s :white_check_mark: -20.52 GiB
    20231122123408 - DropIdxJiraConnectSubscriptionsOnInstallationId Post deploy 2.2 s :white_check_mark: -1.05 MiB
    20231122124815 - DropIndexBulkImportBatchTrackersOnTrackerId Post deploy 2.2 s :white_check_mark: -1016.00 KiB
    20231122125550 - DropIndexBulkImportExportBatchesOnExportId Post deploy 2.1 s :white_check_mark: -456.00 KiB
    20231122130721 - DropIndexProjectRelationExportsOnProjectExportJobId Post deploy 2.2 s :white_check_mark: -8.00 KiB
    Clone details
    Clone ID Clone Created At Clone Data Timestamp Expected Removal Time
    database-testing-2575447-12041078-main 2023-11-28T10:24:22Z 2023-11-28T08:10:09Z 2023-11-28 22:30:13 +0000
    database-testing-2575447-12041078-ci 2023-11-28T10:24:22Z 2023-11-28T08:45:46Z 2023-11-28 22:30:13 +0000

    Job artifacts

    Database migrations (on the ci database)

    Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).

    Migration Type Total runtime Result DB size change
    20231124022520 - AddSmsSentAtAndSmsSendCountToPhoneNumberValidations Regular 2.7 s :white_check_mark: +0.00 B
    Runtime Histogram for all migrations
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 5
    0.1 seconds - 1 second 0
    1 second - 5 seconds 0
    5 seconds - 15 seconds 0
    15 seconds - 5 minutes 0
    5 minutes + 0

    Migration: 20231124022520 - AddSmsSentAtAndSmsSendCountToPhoneNumberValidations

    • Type: Regular
    • Duration: 2.7 s
    • Database size change: +0.00 B
    Calls Total Time Max Time Mean Time Rows Query
    1 6.1 ms 6.1 ms 6.1 ms 0 
    ALTER TABLE "user_phone_number_validations" ADD "sms_sent_at" timestamptz
    1 1.8 ms 1.8 ms 1.8 ms 0 
    ALTER TABLE "user_phone_number_validations" ADD "sms_send_count" smallint DEFAULT 0 NOT NULL
    2 0.0 ms 0.0 ms 0.0 ms 2 
    SELECT pg_backend_pid()
    1 0.0 ms 0.0 ms 0.0 ms 1 
    SELECT $1::regtype::oid
    Histogram for AddSmsSentAtAndSmsSendCountToPhoneNumberValidations
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 5
    0.1 seconds - 1 second 0
    1 second - 5 seconds 0
    5 seconds - 15 seconds 0
    15 seconds - 5 minutes 0
    5 minutes + 0

    Other information

    Other migrations pending on GitLab.com
    Migration Type Total runtime Result DB size change
    20231029142649 - AddMakeProfilePrivateApplicationSetting Regular 3.2 s :warning: +0.00 B
    20231115101002 - AddPromoteUltimateFeaturesAtToOnboardingProgresses Regular 2.8 s :white_check_mark: +0.00 B
    20231117031416 - AddWebIdeOauthApplicationToSettings Regular 2.7 s :white_check_mark: +0.00 B
    20231117031559 - AddFkWebIdeOauthApplication Regular 4.3 s :white_check_mark: +8.00 KiB [note]
    20231120090305 - CreateOrganizationDetails Regular 2.8 s :white_check_mark: +16.00 KiB
    20231121133727 - ChangeMarketingEmailsNullConditions Regular 3.8 s :white_check_mark: +0.00 B
    20231123165947 - ChangeFkToMemberRoleOnMembersFromCascadeToNullify Regular 3.7 s :warning: +0.00 B
    20231116105945 - RequeueBackfillFindingIdInVulnerabilities2 Post deploy 2.6 s :white_check_mark: +0.00 B
    20231122100006 - RemoveCustomEmailSmtpColumnsFromServiceDeskSettings Post deploy 3.1 s :white_check_mark: +0.00 B
    20231122110442 - DropIndexWebHooksOnProjectId Post deploy 3.2 s :white_check_mark: -8.00 KiB
    20231122111935 - DropIndexWebHookLogsPartOnWebHookId Post deploy 3.1 s :warning: -80.00 KiB
    20231122123408 - DropIdxJiraConnectSubscriptionsOnInstallationId Post deploy 3.0 s :white_check_mark: -8.00 KiB
    20231122124815 - DropIndexBulkImportBatchTrackersOnTrackerId Post deploy 3.1 s :white_check_mark: -8.00 KiB
    20231122125550 - DropIndexBulkImportExportBatchesOnExportId Post deploy 3.0 s :white_check_mark: -8.00 KiB
    20231122130721 - DropIndexProjectRelationExportsOnProjectExportJobId Post deploy 3.0 s :white_check_mark: -8.00 KiB
    Clone details
    Clone ID Clone Created At Clone Data Timestamp Expected Removal Time
    database-testing-2575447-12041078-main 2023-11-28T10:24:22Z 2023-11-28T08:10:09Z 2023-11-28 22:30:13 +0000
    database-testing-2575447-12041078-ci 2023-11-28T10:24:22Z 2023-11-28T08:45:46Z 2023-11-28 22:30:13 +0000

    Job artifacts

    Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Epic

    Edited by Ghost User
  • Eugie Limpin changed the description

    changed the description

  • Eugie Limpin added 1 commit

    added 1 commit

    • 6706fcc6 - Add waiting time between phone verification code sends

    Compare with previous version

  • A deleted user added database-testing-automation label

    added database-testing-automation label

  • Eugie Limpin changed the description

    changed the description

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply