diff --git a/ee/app/assets/javascripts/users/identity_verification/components/phone_verification.vue b/ee/app/assets/javascripts/users/identity_verification/components/phone_verification.vue index aaeb098a57e2687c8d591cc7e49003189c759fd7..48a9038a9310d5231e4dda0a4224dae0275b11f7 100644 --- a/ee/app/assets/javascripts/users/identity_verification/components/phone_verification.vue +++ b/ee/app/assets/javascripts/users/identity_verification/components/phone_verification.vue @@ -1,8 +1,13 @@ <script> import { GlButton } from '@gitlab/ui'; +import { createAlert, VARIANT_INFO } from '~/alert'; +import Poll from '~/lib/utils/poll'; +import axios from '~/lib/utils/axios_utils'; import { s__ } from '~/locale'; import { isValidDateString } from '~/lib/utils/datetime_range'; import { calculateRemainingMilliseconds } from '~/lib/utils/datetime_utility'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import { I18N_PHONE_NUMBER_VERIFICATION_UNAVAILABLE } from '../constants'; import InternationalPhoneInput from './international_phone_input.vue'; import VerifyPhoneVerificationCode from './verify_phone_verification_code.vue'; import Captcha from './identity_verification_captcha.vue'; @@ -15,7 +20,8 @@ export default { VerifyPhoneVerificationCode, Captcha, }, - inject: ['phoneNumber', 'offerPhoneNumberExemption'], + mixins: [glFeatureFlagsMixin()], + inject: ['verificationStatePath', 'phoneNumber', 'offerPhoneNumberExemption'], data() { return { stepIndex: 1, @@ -24,6 +30,7 @@ export default { verificationAttempts: 0, disableSubmitButton: false, captchaData: {}, + poll: null, }; }, computed: { @@ -36,11 +43,18 @@ export default { mounted() { this.setSendAllowedOn(this.phoneNumber?.sendAllowedAfter); }, + beforeDestroy() { + this.stopPolling(); + }, methods: { + fetchVerificationState() { + return axios.get(this.verificationStatePath); + }, goToStepTwo({ sendAllowedAfter, ...phoneNumber }) { this.stepIndex = 2; this.latestPhoneNumber = phoneNumber; this.setSendAllowedOn(sendAllowedAfter); + this.startPolling(); }, goToStepOne() { this.stepIndex = 1; @@ -68,6 +82,42 @@ export default { resetTimer() { this.setSendAllowedOn(null); }, + startPolling() { + if (!this.glFeatures.autoRequestPhoneNumberVerificationExemption) return; + + // Poll for possible change in required verification methods for the user. + // Specifically, if the user is from a country blocked by Telesign, we + // auto-request a phone number verification exemption for them. When this + // happens 'phone' is removed from the `verification_methods` array. + this.poll = new Poll({ + resource: { + fetchData: () => this.fetchVerificationState(), + }, + method: 'fetchData', + successCallback: ({ data }) => { + if (!data.verification_methods.includes('phone')) { + createAlert({ + message: I18N_PHONE_NUMBER_VERIFICATION_UNAVAILABLE, + variant: VARIANT_INFO, + }); + + this.stopPolling(); + this.$emit('set-verification-state', data); + } + }, + errorCallback: () => { + this.stopPolling(); + }, + }); + + // Wait five seconds before first poll request to take into account the + // delay of receiving SMS delivery notification callback request. + this.poll.makeDelayedRequest(5000); + }, + stopPolling() { + this.poll?.stop(); + this.poll = null; + }, }, i18n: { verifyWithCreditCard: s__('IdentityVerification|Verify with a credit card instead?'), diff --git a/ee/app/assets/javascripts/users/identity_verification/components/wizard.vue b/ee/app/assets/javascripts/users/identity_verification/components/wizard.vue index 98eaabed3c8659a76f5b7057505cecc45353bfbd..9b4a9e5e69bf1d4d597aa083b8f8617a7401837d 100644 --- a/ee/app/assets/javascripts/users/identity_verification/components/wizard.vue +++ b/ee/app/assets/javascripts/users/identity_verification/components/wizard.vue @@ -1,6 +1,7 @@ <script> import { GlLoadingIcon, GlButton } from '@gitlab/ui'; import { kebabCase } from 'lodash'; +import { mergeUrlParams } from '~/lib/utils/url_utility'; import { __, s__, sprintf } from '~/locale'; import { convertArrayToCamelCase, convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import axios from '~/lib/utils/axios_utils'; @@ -50,7 +51,12 @@ export default { async fetchVerificationState() { this.loading = true; try { - const { data } = await axios.get(this.verificationStatePath); + // Always fetch a fresh copy of the the user's identity verification + // state. This avoids stale data, for example, when the user completes + // the process, gets redirected to the success page and then uses the + // browser's back button. + const url = mergeUrlParams({ no_cache: 1 }, this.verificationStatePath); + const { data } = await axios.get(url); this.setVerificationState(data); } catch (error) { createAlert({ @@ -129,6 +135,7 @@ export default { :is="methodComponent(step)" @completed="onStepCompleted(step)" @exemptionRequested="exemptionRequested" + @set-verification-state="setVerificationState" /> </verification-step> </template> diff --git a/ee/app/assets/javascripts/users/identity_verification/constants.js b/ee/app/assets/javascripts/users/identity_verification/constants.js index 3ddb64e86882dc72ab616e9f56688c8bad5259ef..d97cbff079acc88d6de09d142c91113e96883468 100644 --- a/ee/app/assets/javascripts/users/identity_verification/constants.js +++ b/ee/app/assets/javascripts/users/identity_verification/constants.js @@ -4,6 +4,9 @@ import { s__, sprintf } from '~/locale'; export const MAX_PHONE_NUMBER_LENGTH = 12; export const DEFAULT_COUNTRY = 'US'; +export const I18N_PHONE_NUMBER_VERIFICATION_UNAVAILABLE = s__( + 'IdentityVerification|Phone number verification is unavailable at this time. Please verify with a credit card instead.', +); export const I18N_PHONE_NUMBER_BLANK_ERROR = s__('IdentityVerification|Phone number is required.'); export const I18N_PHONE_NUMBER_NAN_ERROR = s__( 'IdentityVerification|Phone number must contain only digits.', diff --git a/ee/app/controllers/phone_verification/telesign_callbacks_controller.rb b/ee/app/controllers/phone_verification/telesign_callbacks_controller.rb index 5f6766b384fd05e8ab82f63958b2ce0fbe1f32a4..5a55a32c1f61970654438d26c61a2e7ccda156c0 100644 --- a/ee/app/controllers/phone_verification/telesign_callbacks_controller.rb +++ b/ee/app/controllers/phone_verification/telesign_callbacks_controller.rb @@ -12,13 +12,30 @@ class TelesignCallbacksController < ApplicationController urgency :low def notify - callback = ::Telesign::TransactionCallback.new(request, params) - return not_found unless callback.valid? callback.log + exempt_user_from_phone_number_verification if callback.payload.country_blocked? + render json: {} end + + private + + def callback + @callback ||= ::Telesign::TransactionCallback.new(request, params) + end + + def exempt_user_from_phone_number_verification + return unless ::Feature.enabled?(:auto_request_phone_number_verification_exemption, type: :gitlab_com_derisk) + + user = callback.user + + return unless user&.offer_phone_number_exemption? + + user.create_phone_number_exemption! + Gitlab::EtagCaching::Store.new.touch(verification_state_identity_verification_path) + end end end diff --git a/ee/app/controllers/users/identity_verification_controller.rb b/ee/app/controllers/users/identity_verification_controller.rb index 717e2acb72fa316514ad3207b78d1a907ca0f2a9..26e1640cebafb5188003b92de2e0852059c578f8 100644 --- a/ee/app/controllers/users/identity_verification_controller.rb +++ b/ee/app/controllers/users/identity_verification_controller.rb @@ -31,6 +31,8 @@ class IdentityVerificationController < ApplicationController layout 'minimal' def show + push_frontend_feature_flag(:auto_request_phone_number_verification_exemption, type: :gitlab_com_derisk) + # We to perform cookie migration for tracking from logged out to log in # calling this before tracking gives us access to request where the # signed cookie exist with the info we need for migration. @@ -39,8 +41,10 @@ def show end def verification_state + Gitlab::PollingInterval.set_header(response, interval: 10_000) + # if the back button is pressed, don't cache the user's identity verification state - no_cache_headers + no_cache_headers if params['no_cache'] render json: { verification_methods: @user.required_identity_verification_methods, diff --git a/ee/app/models/concerns/identity_verifiable.rb b/ee/app/models/concerns/identity_verifiable.rb index b771d2735270b25971a554b9d115ebabacc81528..3990110e7c672b89bf3055cb3e29491c4b9f1f42 100644 --- a/ee/app/models/concerns/identity_verifiable.rb +++ b/ee/app/models/concerns/identity_verifiable.rb @@ -71,15 +71,24 @@ def credit_card_verified? end def create_phone_number_exemption! + return if phone_verified? + return if exempt_from_phone_number_verification? + custom_attributes.create!( key: UserCustomAttribute::IDENTITY_VERIFICATION_PHONE_EXEMPT, value: true.to_s, user_id: id ) + clear_memoization(:phone_number_exemption_attribute) + clear_memoization(:identity_verification_state) end def destroy_phone_number_exemption - !!phone_number_exemption_attribute && phone_number_exemption_attribute.destroy + return unless phone_number_exemption_attribute + + phone_number_exemption_attribute.destroy + clear_memoization(:phone_number_exemption_attribute) + clear_memoization(:identity_verification_state) end def exempt_from_phone_number_verification? @@ -89,8 +98,6 @@ def exempt_from_phone_number_verification? def toggle_phone_number_verification exempt_from_phone_number_verification? ? destroy_phone_number_exemption : create_phone_number_exemption! - clear_memoization(:phone_number_exemption_attribute) - clear_memoization(:identity_verification_state) end def create_identity_verification_exemption diff --git a/ee/config/feature_flags/gitlab_com_derisk/auto_request_phone_number_verification_exemption.yml b/ee/config/feature_flags/gitlab_com_derisk/auto_request_phone_number_verification_exemption.yml new file mode 100644 index 0000000000000000000000000000000000000000..5ab1b9cb83aee5a7806de6f2f3e32e180265a964 --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/auto_request_phone_number_verification_exemption.yml @@ -0,0 +1,9 @@ +--- +name: auto_request_phone_number_verification_exemption +feature_issue_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142788 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17477 +milestone: '16.9' +group: group::anti-abuse +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/lib/ee/gitlab/etag_caching/router/rails.rb b/ee/lib/ee/gitlab/etag_caching/router/rails.rb index 9d11f1a6664c72772f4649e1b82b039ecd2f5ad0..e3965f28182d4b27434097f28aca9072107000b4 100644 --- a/ee/lib/ee/gitlab/etag_caching/router/rails.rb +++ b/ee/lib/ee/gitlab/etag_caching/router/rails.rb @@ -13,6 +13,12 @@ module Rails 'epic_realtime_changes', ::Groups::EpicsController, :realtime_changes + ], + [ + %r{^/users/identity_verification/verification_state\z}, + 'user_identity_verification_state', + ::Users::IdentityVerificationController, + :verification_state ] ].freeze diff --git a/ee/lib/telesign/transaction_callback.rb b/ee/lib/telesign/transaction_callback.rb index 619f477b1692944659206f373497e673fdde67ea..679ef54a237700a326ff24d5f760ca6efbde068d 100644 --- a/ee/lib/telesign/transaction_callback.rb +++ b/ee/lib/telesign/transaction_callback.rb @@ -41,6 +41,12 @@ def log track_status_update_event end + def user + return unless valid? + + phone_number_validation_record&.user + end + private def phone_number_validation_record @@ -48,10 +54,6 @@ def phone_number_validation_record end strong_memoize_attr :phone_number_validation_record - def user - phone_number_validation_record&.user - end - def track_status_update_event return unless phone_number_validation_record diff --git a/ee/lib/telesign/transaction_callback_payload.rb b/ee/lib/telesign/transaction_callback_payload.rb index 6404932be197dc187806621d59778d01f71b623b..895b2fa661de071b4842793ee7ab97a2e100b00d 100644 --- a/ee/lib/telesign/transaction_callback_payload.rb +++ b/ee/lib/telesign/transaction_callback_payload.rb @@ -16,9 +16,8 @@ def reference_id end def status - code = payload.dig('status', 'code') description = payload.dig('status', 'description') - [code, description].compact.join(' - ') + [status_code, description].compact.join(' - ') end def status_updated_on @@ -34,8 +33,19 @@ def errors end def failed_delivery? - status_code = payload.dig('status', 'code') status_code != 200 end + + def country_blocked? + # https://developer.telesign.com/enterprise/docs/all-status-and-error-codes#status-codes + # 237 - Message blocked in requested country + status_code == 237 + end + + private + + def status_code + @status_code ||= payload.dig('status', 'code') + end end end diff --git a/ee/spec/frontend/users/identity_verification/components/phone_verification_spec.js b/ee/spec/frontend/users/identity_verification/components/phone_verification_spec.js index 9c2faf4fbc6164badf2d957efa0b45960dc3a9bf..e2ec999fc697e4930f8c6591345458d1a051a5fa 100644 --- a/ee/spec/frontend/users/identity_verification/components/phone_verification_spec.js +++ b/ee/spec/frontend/users/identity_verification/components/phone_verification_spec.js @@ -1,3 +1,5 @@ +import axios from 'axios'; +import MockAdapter from 'axios-mock-adapter'; import { nextTick } from 'vue'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import { s__ } from '~/locale'; @@ -6,13 +8,18 @@ import InternationalPhoneInput from 'ee/users/identity_verification/components/i import VerifyPhoneVerificationCode from 'ee/users/identity_verification/components/verify_phone_verification_code.vue'; import Captcha from 'ee/users/identity_verification/components/identity_verification_captcha.vue'; import { calculateRemainingMilliseconds } from '~/lib/utils/datetime_utility'; +import { HTTP_STATUS_OK, HTTP_STATUS_INTERNAL_SERVER_ERROR } from '~/lib/utils/http_status'; +import Poll from '~/lib/utils/poll'; +import { createAlert, VARIANT_INFO } from '~/alert'; jest.mock('~/lib/utils/datetime_utility', () => ({ calculateRemainingMilliseconds: jest.fn(), })); +jest.mock('~/alert'); describe('Phone Verification component', () => { let wrapper; + let axiosMock; const PHONE_NUMBER = { country: 'US', @@ -20,6 +27,17 @@ describe('Phone Verification component', () => { number: '555', }; + const DEFAULT_PROVIDE = { + verificationStatePath: '/users/identity_verification/verification_state', + offerPhoneNumberExemption: true, + phoneNumber: { + enableArkoseChallenge: true, + showArkoseChallenge: true, + showRecaptchaChallenge: true, + }, + glFeatures: { autoRequestPhoneNumberVerificationExemption: true }, + }; + const findInternationalPhoneInput = () => wrapper.findComponent(InternationalPhoneInput); const findVerifyCodeInput = () => wrapper.findComponent(VerifyPhoneVerificationCode); const findPhoneExemptionLink = () => @@ -30,12 +48,7 @@ describe('Phone Verification component', () => { const createComponent = (provide = {}, props = {}) => { wrapper = shallowMountExtended(PhoneVerification, { provide: { - offerPhoneNumberExemption: true, - phoneNumber: { - enableArkoseChallenge: true, - showArkoseChallenge: true, - showRecaptchaChallenge: true, - }, + ...DEFAULT_PROVIDE, ...provide, }, propsData: props, @@ -43,6 +56,7 @@ describe('Phone Verification component', () => { }; beforeEach(() => { + axiosMock = new MockAdapter(axios); calculateRemainingMilliseconds.mockReturnValue(1000); createComponent(); @@ -287,4 +301,108 @@ describe('Phone Verification component', () => { }); }); }); + + describe('Verification state polling', () => { + const pollInterval = 10; + + let pollRequest; + let pollStop; + + const mockVerificationState = (mockState) => { + const data = { + verification_methods: Object.keys(mockState), + verification_state: mockState, + }; + axiosMock + .onGet(DEFAULT_PROVIDE.verificationStatePath) + .reply(HTTP_STATUS_OK, data, { 'poll-interval': pollInterval }); + + return data; + }; + + const setupComponent = async (provide = {}) => { + createComponent(provide); + + await findInternationalPhoneInput().vm.$emit('next', { + ...PHONE_NUMBER, + sendAllowedAfter: '2000-01-01T01:02:03Z', + }); + }; + + beforeEach(() => { + pollRequest = jest.spyOn(Poll.prototype, 'makeRequest'); + pollStop = jest.spyOn(Poll.prototype, 'stop'); + }); + + afterEach(() => { + pollRequest.mockRestore(); + pollStop.mockRestore(); + axiosMock.restore(); + createAlert.mockClear(); + }); + + describe('when request succeeds', () => { + it('emits set-verification-state with response data then stops', async () => { + mockVerificationState({ phone: false }); + + setupComponent(); + + jest.advanceTimersByTime(5000); + await axios.waitForAll(); + + expect(pollRequest).toHaveBeenCalledTimes(1); + expect(pollStop).not.toHaveBeenCalled(); + expect(wrapper.emitted('set-verification-state')).toBeUndefined(); + + const responseData = mockVerificationState({}); + mockVerificationState({}); + + jest.advanceTimersByTime(pollInterval); + await axios.waitForAll(); + + expect(pollRequest).toHaveBeenCalledTimes(2); + expect(pollStop).toHaveBeenCalledTimes(1); + expect(wrapper.emitted('set-verification-state')).toStrictEqual([[responseData]]); + expect(createAlert).toHaveBeenCalledWith({ + message: + 'Phone number verification is unavailable at this time. Please verify with a credit card instead.', + variant: VARIANT_INFO, + }); + + jest.advanceTimersByTime(pollInterval); + await axios.waitForAll(); + + expect(pollRequest).toHaveBeenCalledTimes(2); + expect(pollStop).toHaveBeenCalledTimes(1); + expect(wrapper.emitted('set-verification-state')).toStrictEqual([[responseData]]); + }); + }); + + describe('when request fails', () => { + it('stops', async () => { + axiosMock + .onGet(DEFAULT_PROVIDE.verificationStatePath) + .reply(HTTP_STATUS_INTERNAL_SERVER_ERROR); + + setupComponent(); + + jest.advanceTimersByTime(5000); + await axios.waitForAll(); + + expect(pollRequest).toHaveBeenCalledTimes(1); + expect(pollStop).toHaveBeenCalledTimes(1); + expect(wrapper.emitted('set-verification-state')).toBeUndefined(); + }); + }); + + describe('when autoRequestPhoneNumberVerificationExemption feature flag is disabled', () => { + it('does not start', () => { + setupComponent({ glFeatures: { autoRequestPhoneNumberVerificationExemption: false } }); + + jest.advanceTimersByTime(5000); + + expect(pollRequest).not.toHaveBeenCalled(); + }); + }); + }); }); diff --git a/ee/spec/frontend/users/identity_verification/components/wizard_spec.js b/ee/spec/frontend/users/identity_verification/components/wizard_spec.js index 0f6096fe03e7713c510427bd85299c4b610fb272..ee9b1cc2917e930e71752d4901586ecabbc41bcf 100644 --- a/ee/spec/frontend/users/identity_verification/components/wizard_spec.js +++ b/ee/spec/frontend/users/identity_verification/components/wizard_spec.js @@ -15,9 +15,14 @@ import EmailVerification from 'ee/users/identity_verification/components/email_v import { I18N_GENERIC_ERROR } from 'ee/users/identity_verification/constants'; jest.mock('~/alert'); -jest.mock('~/lib/utils/url_utility', () => ({ - visitUrl: jest.fn().mockName('visitUrlMock'), -})); +jest.mock('~/lib/utils/url_utility', () => { + const originalModule = jest.requireActual('~/lib/utils/url_utility'); + + return { + ...originalModule, + visitUrl: jest.fn().mockName('visitUrlMock'), + }; +}); describe('IdentityVerificationWizard', () => { let wrapper; @@ -40,11 +45,15 @@ describe('IdentityVerificationWizard', () => { const findDescription = () => wrapper.find('p'); const findNextButton = () => wrapper.findComponent(GlButton); + const buildVerificationStateResponse = (mockState) => ({ + verification_methods: Object.keys(mockState), + verification_state: mockState, + }); + const mockVerificationState = (mockState) => { - axiosMock.onGet(DEFAULT_PROVIDE.verificationStatePath).replyOnce(HTTP_STATUS_OK, { - verification_methods: Object.keys(mockState), - verification_state: mockState, - }); + const url = `${DEFAULT_PROVIDE.verificationStatePath}?no_cache=1`; + + axiosMock.onGet(url).replyOnce(HTTP_STATUS_OK, buildVerificationStateResponse(mockState)); }; beforeEach(() => { @@ -217,4 +226,25 @@ describe('IdentityVerificationWizard', () => { }); }); }); + + describe('A verification component emits set-verification-state', () => { + beforeEach(async () => { + mockVerificationState({ email: false }); + createComponent(); + await waitForPromises(); + }); + + it('executes setVerificationState', async () => { + expect(findSteps().at(0).props('completed')).toBe(false); + + findSteps() + .at(0) + .findComponent(EmailVerification) + .vm.$emit('set-verification-state', buildVerificationStateResponse({ email: true })); + + await nextTick(); + + expect(findSteps().at(0).props('completed')).toBe(true); + }); + }); }); diff --git a/ee/spec/lib/telesign/transaction_callback_payload_spec.rb b/ee/spec/lib/telesign/transaction_callback_payload_spec.rb index e2c68ca6ea89f22524caefe2bb94c05e5c24d73a..b4cc0b113b80a46b7b0143c0bbcabf61a4632471 100644 --- a/ee/spec/lib/telesign/transaction_callback_payload_spec.rb +++ b/ee/spec/lib/telesign/transaction_callback_payload_spec.rb @@ -92,4 +92,16 @@ it { is_expected.to eq true } end end + + describe '#country_blocked?' do + subject { response.country_blocked? } + + it { is_expected.to eq false } + + context 'when status code is 237' do + let(:json) { { status: { code: 237 } }.deep_stringify_keys } + + it { is_expected.to eq true } + end + end end diff --git a/ee/spec/lib/telesign/transaction_callback_spec.rb b/ee/spec/lib/telesign/transaction_callback_spec.rb index d7a85163134ef617823b5c7b58ddf204f87d63c2..815ff37abf19d25259aa2e675f42aa651ecc0a04 100644 --- a/ee/spec/lib/telesign/transaction_callback_spec.rb +++ b/ee/spec/lib/telesign/transaction_callback_spec.rb @@ -179,4 +179,43 @@ end end end + + describe '#user' do + let(:callback_valid) { true } + + subject(:user) { described_class.new(instance_double(ActionDispatch::Request), {}).user } + + before do + allow_next_instance_of(described_class) do |callback| + allow(callback).to receive(:valid?).and_return(callback_valid) + end + end + + context 'when callback is not valid' do + let(:callback_valid) { false } + + it { is_expected.to be_nil } + end + + context 'when no matching phone number validation record is found' do + it 'returns nil' do + expect_next_instance_of(Telesign::TransactionCallbackPayload, {}) do |response| + expect(response).to receive(:reference_id).and_return('fake-ref-id') + end + + expect(user).to be_nil + end + end + + it 'returns user associated with the matching phone number validation record' do + ref_id = 'abc123' + record = create(:phone_number_validation, telesign_reference_xid: ref_id) + + expect_next_instance_of(Telesign::TransactionCallbackPayload, {}) do |response| + expect(response).to receive(:reference_id).and_return(ref_id) + end + + expect(user).to eq record.user + end + end end diff --git a/ee/spec/models/concerns/identity_verifiable_spec.rb b/ee/spec/models/concerns/identity_verifiable_spec.rb index 63f0043f9d4f3935a80dc995bde746e4813147ae..5de0b6ce3766d67632efcbf1f6fb9ecd3cd08208 100644 --- a/ee/spec/models/concerns/identity_verifiable_spec.rb +++ b/ee/spec/models/concerns/identity_verifiable_spec.rb @@ -470,11 +470,40 @@ def add_identity_verification_exemption let(:user) { create(:user) } - it 'creates an exemption' do + it 'creates an exemption', :aggregate_failures do + expect(user).to receive(:clear_memoization).with(:phone_number_exemption_attribute).and_call_original + expect(user).to receive(:clear_memoization).with(:identity_verification_state).and_call_original + expect { subject }.to change { user.custom_attributes.by_key(UserCustomAttribute::IDENTITY_VERIFICATION_PHONE_EXEMPT).count }.from(0).to(1) end + + shared_examples 'it does not create an exemption' do + it 'does not create an exemption', :aggregate_failures do + expect(user).not_to receive(:clear_memoization) + + expect { subject }.not_to change { + user.custom_attributes.by_key(UserCustomAttribute::IDENTITY_VERIFICATION_PHONE_EXEMPT).count + } + end + end + + context 'when user has already verified a phone number' do + before do + create(:phone_number_validation, :validated, user: user) + end + + it_behaves_like 'it does not create an exemption' + end + + context 'when user is already exempt' do + before do + add_phone_exemption + end + + it_behaves_like 'it does not create an exemption' + end end describe '#destroy_phone_number_exemption' do @@ -483,11 +512,12 @@ def add_identity_verification_exemption let(:user) { create(:user) } context 'when a user has a phone number exemption' do - before do + it 'destroys the exemption', :aggregate_failures do add_phone_exemption - end - it 'destroys the exemption' do + expect(user).to receive(:clear_memoization).with(:phone_number_exemption_attribute).and_call_original + expect(user).to receive(:clear_memoization).with(:identity_verification_state).and_call_original + subject expect(user.custom_attributes.by_key(UserCustomAttribute::IDENTITY_VERIFICATION_PHONE_EXEMPT)).to be_empty @@ -495,7 +525,7 @@ def add_identity_verification_exemption end context 'when a user does not have a phone number exemption' do - it { is_expected.to be false } + it { is_expected.to be_nil } end end @@ -636,6 +666,11 @@ def add_identity_verification_exemption end describe '#toggle_phone_number_verification' do + before do + allow(user).to receive(:clear_memoization).with(:phone_number_exemption_attribute).and_call_original + allow(user).to receive(:clear_memoization).with(:identity_verification_state).and_call_original + end + subject(:toggle_phone_number_verification) { user.toggle_phone_number_verification } context 'when not exempt from phone number verification' do @@ -647,20 +682,17 @@ def add_identity_verification_exemption end context 'when exempt from phone number verification' do - before do + it 'destroys the exemption' do user.create_phone_number_exemption! - end - it 'destroys the exemption' do expect(user).to receive(:destroy_phone_number_exemption) toggle_phone_number_verification end end - it 'clears memoization of phone_number_exemption_attribute and identity_verification_state', :aggregate_failures do - expect(user).to receive(:clear_memoization).with(:phone_number_exemption_attribute).and_call_original - expect(user).to receive(:clear_memoization).with(:identity_verification_state).and_call_original + it 'clears memoization of identity_verification_state' do + expect(user).to receive(:clear_memoization).with(:identity_verification_state) toggle_phone_number_verification end diff --git a/ee/spec/requests/phone_verification/telesign_callbacks_controller_spec.rb b/ee/spec/requests/phone_verification/telesign_callbacks_controller_spec.rb index 380e11a6a2e0bc06df07ac45eeb96ddafa39405f..fd22db8b84ef2933cb9556e394e022769b6f8d7a 100644 --- a/ee/spec/requests/phone_verification/telesign_callbacks_controller_spec.rb +++ b/ee/spec/requests/phone_verification/telesign_callbacks_controller_spec.rb @@ -7,13 +7,14 @@ subject(:do_request) { post phone_verification_telesign_callback_path } context 'when callback request is not valid (authentication failed)' do - it 'returns not found status', :aggregate_failures do + it 'does not log and returns not found status', :aggregate_failures do expect_next_instance_of( Telesign::TransactionCallback, an_instance_of(ActionDispatch::Request), an_instance_of(ActionController::Parameters) ) do |callback| - expect(callback).to receive(:valid?).and_return(false) + allow(callback).to receive(:valid?).and_return(false) + expect(callback).not_to receive(:log) end do_request @@ -29,7 +30,7 @@ an_instance_of(ActionDispatch::Request), an_instance_of(ActionController::Parameters) ) do |callback| - expect(callback).to receive(:valid?).and_return(true) + allow(callback).to receive(:valid?).and_return(true) expect(callback).to receive(:log) end @@ -38,5 +39,80 @@ expect(response).to have_gitlab_http_status(:ok) end end + + context 'when origin country of user is blocked in Telesign' do + shared_examples 'does not invalidate verification_state_identity_verification_path cache' do + it 'does not invalidate verification_state_identity_verification_path cache' do + expect(Gitlab::EtagCaching).not_to receive(:new) + + do_request + end + end + + shared_examples 'does not exempt the user' do + it 'does not exempt the user' do + expect { do_request }.not_to change { user.exempt_from_phone_number_verification? } + end + end + + before do + allow_next_instance_of( + Telesign::TransactionCallback, + an_instance_of(ActionDispatch::Request), + an_instance_of(ActionController::Parameters) + ) do |callback| + allow(callback).to receive(:valid?).and_return(true) + allow(callback).to receive(:user).and_return(user) + allow(callback).to receive(:log) + + payload = instance_double(Telesign::TransactionCallbackPayload, { country_blocked?: true }) + allow(callback).to receive(:payload).and_return(payload) + end + end + + context 'when no user is associated with the callback' do + let(:user) { nil } + + it_behaves_like 'does not invalidate verification_state_identity_verification_path cache' + end + + context 'when a user is associated with the callback' do + let(:user) { create(:user) } + + before do + allow(user).to receive(:offer_phone_number_exemption?).and_return(true) + end + + it 'exempts the user' do + expect { do_request }.to change { user.exempt_from_phone_number_verification? }.from(false).to(true) + end + + it 'invalidates verification_state_identity_verification_path cache' do + expect_next_instance_of(Gitlab::EtagCaching::Store) do |store| + expect(store).to receive(:touch).with(verification_state_identity_verification_path) + end + + do_request + end + + context 'when user is not qualified for phone number exemption offer' do + before do + allow(user).to receive(:offer_phone_number_exemption?).and_return(false) + end + + it_behaves_like 'does not exempt the user' + it_behaves_like 'does not invalidate verification_state_identity_verification_path cache' + end + + context 'when auto_request_phone_number_verification_exemption feature flag is disabled' do + before do + stub_feature_flags(auto_request_phone_number_verification_exemption: false) + end + + it_behaves_like 'does not exempt the user' + it_behaves_like 'does not invalidate verification_state_identity_verification_path cache' + end + end + end end end diff --git a/ee/spec/requests/users/identity_verification_controller_spec.rb b/ee/spec/requests/users/identity_verification_controller_spec.rb index f8485eb75029c0405c58c2acddfc814510c8bb3a..ea5bcb0232d786dbd15104b1b5d7cec82ca9c0f1 100644 --- a/ee/spec/requests/users/identity_verification_controller_spec.rb +++ b/ee/spec/requests/users/identity_verification_controller_spec.rb @@ -511,6 +511,14 @@ 'verification_state' => { "email" => false } }) end + + describe 'poll interval header' do + it 'is added' do + do_request + + expect(response.headers.to_h).to include(Gitlab::PollingInterval::HEADER_NAME => '10000') + end + end end context 'with a verified user' do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9a17bfbc199873ff18f39ff3d2e78a56806c42a8..020c54ca136ecde15779fd0204beeccba567825b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25029,6 +25029,9 @@ msgstr "" msgid "IdentityVerification|Phone number must contain only digits." msgstr "" +msgid "IdentityVerification|Phone number verification is unavailable at this time. Please verify with a credit card instead." +msgstr "" + msgid "IdentityVerification|Please enter a valid code" msgstr ""