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 88bea721c7a193e22fdf26b2eac52c59eb22432f..172e8d9a83002c4a41cdf2752cecb8dc97f8922b 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24969,6 +24969,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 ""