diff --git a/app/assets/javascripts/pages/sessions/new/index.js b/app/assets/javascripts/pages/sessions/new/index.js index a8b4dca084597d272e1d48b474dc69bb312134e0..1d5d885753cecb240dc7bf9a56af0d9a24ae8472 100644 --- a/app/assets/javascripts/pages/sessions/new/index.js +++ b/app/assets/javascripts/pages/sessions/new/index.js @@ -3,6 +3,7 @@ import initVueAlerts from '~/vue_alerts'; import NoEmojiValidator from '~/emoji/no_emoji_validator'; import { initLanguageSwitcher } from '~/language_switcher'; import LengthValidator from '~/validators/length_validator'; +import mountEmailVerificationApplication from '~/sessions/new'; import OAuthRememberMe from './oauth_remember_me'; import preserveUrlFragment from './preserve_url_fragment'; import SigninTabsMemoizer from './signin_tabs_memoizer'; @@ -22,3 +23,4 @@ new OAuthRememberMe({ preserveUrlFragment(window.location.hash); initVueAlerts(); initLanguageSwitcher(); +mountEmailVerificationApplication(); diff --git a/app/assets/javascripts/sessions/new/components/email_verification.vue b/app/assets/javascripts/sessions/new/components/email_verification.vue new file mode 100644 index 0000000000000000000000000000000000000000..87385b91c426eddabecd36a31aadec268888ce82 --- /dev/null +++ b/app/assets/javascripts/sessions/new/components/email_verification.vue @@ -0,0 +1,174 @@ +<script> +import { GlSprintf, GlForm, GlFormGroup, GlFormInput, GlButton } from '@gitlab/ui'; +import { visitUrl } from '~/lib/utils/url_utility'; +import { createAlert, VARIANT_SUCCESS } from '~/alert'; +import axios from '~/lib/utils/axios_utils'; +import { + I18N_EXPLANATION, + I18N_INPUT_LABEL, + I18N_EMAIL_EMPTY_CODE, + I18N_EMAIL_INVALID_CODE, + I18N_SUBMIT_BUTTON, + I18N_RESEND_LINK, + I18N_EMAIL_RESEND_SUCCESS, + I18N_GENERIC_ERROR, + VERIFICATION_CODE_REGEX, + SUCCESS_RESPONSE, + FAILURE_RESPONSE, +} from '../constants'; + +export default { + name: 'EmailVerification', + components: { + GlSprintf, + GlForm, + GlFormGroup, + GlFormInput, + GlButton, + }, + props: { + obfuscatedEmail: { + type: String, + required: true, + }, + verifyPath: { + type: String, + required: true, + }, + resendPath: { + type: String, + required: true, + }, + }, + data() { + return { + verificationCode: '', + submitted: false, + verifyError: '', + }; + }, + computed: { + inputValidation() { + return { + state: !(this.submitted && this.invalidFeedback), + message: this.invalidFeedback, + }; + }, + invalidFeedback() { + if (!this.submitted) { + return ''; + } + + if (!this.verificationCode) { + return I18N_EMAIL_EMPTY_CODE; + } + + if (!VERIFICATION_CODE_REGEX.test(this.verificationCode)) { + return I18N_EMAIL_INVALID_CODE; + } + + return this.verifyError; + }, + }, + watch: { + verificationCode() { + this.verifyError = ''; + }, + }, + methods: { + verify() { + this.submitted = true; + + if (!this.inputValidation.state) return; + + axios + .post(this.verifyPath, { user: { verification_token: this.verificationCode } }) + .then(this.handleVerificationResponse) + .catch(this.handleError); + }, + handleVerificationResponse(response) { + if (response.data.status === undefined) { + this.handleError(); + } else if (response.data.status === SUCCESS_RESPONSE) { + visitUrl(response.data.redirect_path); + } else if (response.data.status === FAILURE_RESPONSE) { + this.verifyError = response.data.message; + } + }, + resend() { + axios + .post(this.resendPath) + .then(this.handleResendResponse) + .catch(this.handleError) + .finally(this.resetForm); + }, + handleResendResponse(response) { + if (response.data.status === undefined) { + this.handleError(); + } else if (response.data.status === SUCCESS_RESPONSE) { + createAlert({ + message: I18N_EMAIL_RESEND_SUCCESS, + variant: VARIANT_SUCCESS, + }); + } else if (response.data.status === FAILURE_RESPONSE) { + createAlert({ message: response.data.message }); + } + }, + handleError(error) { + createAlert({ + message: I18N_GENERIC_ERROR, + captureError: true, + error, + }); + }, + resetForm() { + this.verificationCode = ''; + this.submitted = false; + this.$refs.input.$el.focus(); + }, + }, + i18n: { + explanation: I18N_EXPLANATION, + inputLabel: I18N_INPUT_LABEL, + submitButton: I18N_SUBMIT_BUTTON, + resendLink: I18N_RESEND_LINK, + }, +}; +</script> + +<template> + <gl-form @submit.prevent="verify"> + <section class="gl-mb-5"> + <gl-sprintf :message="$options.i18n.explanation"> + <template #email> + <strong>{{ obfuscatedEmail }}</strong> + </template> + </gl-sprintf> + </section> + <gl-form-group + :label="$options.i18n.inputLabel" + label-for="verification-code" + :state="inputValidation.state" + :invalid-feedback="inputValidation.message" + > + <gl-form-input + id="verification-code" + ref="input" + v-model="verificationCode" + autofocus + autocomplete="one-time-code" + inputmode="numeric" + maxlength="6" + :state="inputValidation.state" + /> + </gl-form-group> + <section class="gl-mt-5"> + <gl-button block variant="confirm" type="submit" :disabled="!inputValidation.state">{{ + $options.i18n.submitButton + }}</gl-button> + <gl-button block variant="link" class="gl-mt-3 gl-h-7" @click="resend">{{ + $options.i18n.resendLink + }}</gl-button> + </section> + </gl-form> +</template> diff --git a/app/assets/javascripts/sessions/new/constants.js b/app/assets/javascripts/sessions/new/constants.js new file mode 100644 index 0000000000000000000000000000000000000000..203a8aee1c40ae71991f37588517e4a701754f99 --- /dev/null +++ b/app/assets/javascripts/sessions/new/constants.js @@ -0,0 +1,18 @@ +import { s__ } from '~/locale'; + +export const I18N_EXPLANATION = s__( + "IdentityVerification|For added security, you'll need to verify your identity. We've sent a verification code to %{email}", +); +export const I18N_INPUT_LABEL = s__('IdentityVerification|Verification code'); +export const I18N_EMAIL_EMPTY_CODE = s__('IdentityVerification|Enter a code.'); +export const I18N_EMAIL_INVALID_CODE = s__('IdentityVerification|Please enter a valid code'); +export const I18N_SUBMIT_BUTTON = s__('IdentityVerification|Verify code'); +export const I18N_RESEND_LINK = s__('IdentityVerification|Resend code'); +export const I18N_EMAIL_RESEND_SUCCESS = s__('IdentityVerification|A new code has been sent.'); +export const I18N_GENERIC_ERROR = s__( + 'IdentityVerification|Something went wrong. Please try again.', +); + +export const VERIFICATION_CODE_REGEX = /^\d{6}$/; +export const SUCCESS_RESPONSE = 'success'; +export const FAILURE_RESPONSE = 'failure'; diff --git a/app/assets/javascripts/sessions/new/index.js b/app/assets/javascripts/sessions/new/index.js new file mode 100644 index 0000000000000000000000000000000000000000..51022a281e3d72ed4a73b36108dd18fe39848c7c --- /dev/null +++ b/app/assets/javascripts/sessions/new/index.js @@ -0,0 +1,22 @@ +import Vue from 'vue'; +import EmailVerification from './components/email_verification.vue'; + +export default () => { + const el = document.querySelector('.js-email-verification'); + + if (!el) { + return null; + } + + const { obfuscatedEmail, verifyPath, resendPath } = el.dataset; + + return new Vue({ + el, + name: 'EmailVerificationRoot', + render(createElement) { + return createElement(EmailVerification, { + props: { obfuscatedEmail, verifyPath, resendPath }, + }); + }, + }); +}; diff --git a/app/controllers/concerns/verifies_with_email.rb b/app/controllers/concerns/verifies_with_email.rb index 13378800ea98c3941cb0b5554b6a5c2628072bfd..f52903fe7e960e060729d49ddcf4f4ad31654782 100644 --- a/app/controllers/concerns/verifies_with_email.rb +++ b/app/controllers/concerns/verifies_with_email.rb @@ -12,6 +12,7 @@ module VerifiesWithEmail skip_before_action :required_signup_info, only: :successful_verification end + # rubocop:disable Metrics/PerceivedComplexity def verify_with_email return unless user = find_user || find_verification_user @@ -34,18 +35,27 @@ def verify_with_email # - their account has been locked because of too many failed login attempts, or # - they have logged in before, but never from the current ip address reason = 'sign in from untrusted IP address' unless user.access_locked? - send_verification_instructions(user, reason: reason) + send_verification_instructions(user, reason: reason) unless send_rate_limited?(user) prompt_for_email_verification(user) end end end end + # rubocop:enable Metrics/PerceivedComplexity def resend_verification_code return unless user = find_verification_user - send_verification_instructions(user) - prompt_for_email_verification(user) + if send_rate_limited?(user) + message = format( + s_("IdentityVerification|You've reached the maximum amount of resends. Wait %{interval} and try again."), + interval: rate_limit_interval(:email_verification_code_send) + ) + render json: { status: :failure, message: message } + else + send_verification_instructions(user) + render json: { status: :success } + end end def successful_verification @@ -67,19 +77,7 @@ def find_verification_user User.find_by_id(session[:verification_user_id]) end - # After successful verification and calling sign_in, devise redirects the - # user to this path. Override it to show the successful verified page. - def after_sign_in_path_for(resource) - if action_name == 'create' && session[:verification_user_id] == resource.id - return users_successful_verification_path - end - - super - end - def send_verification_instructions(user, reason: nil) - return if send_rate_limited?(user) - service = Users::EmailVerification::GenerateTokenService.new(attr: :unlock_token, user: user) raw_token, encrypted_token = service.execute user.unlock_token = encrypted_token @@ -101,21 +99,23 @@ def verify_token(user, token) if result[:status] == :success handle_verification_success(user) + render json: { status: :success, redirect_path: users_successful_verification_path } else handle_verification_failure(user, result[:reason], result[:message]) + render json: result end end def render_sign_in_rate_limited message = format( s_('IdentityVerification|Maximum login attempts exceeded. Wait %{interval} and try again.'), - interval: user_sign_in_interval + interval: rate_limit_interval(:user_sign_in) ) redirect_to new_user_session_path, alert: message end - def user_sign_in_interval - interval_in_seconds = Gitlab::ApplicationRateLimiter.rate_limits[:user_sign_in][:interval] + def rate_limit_interval(rate_limit) + interval_in_seconds = Gitlab::ApplicationRateLimiter.rate_limits[rate_limit][:interval] distance_of_time_in_words(interval_in_seconds) end @@ -126,8 +126,6 @@ def send_rate_limited?(user) def handle_verification_failure(user, reason, message) user.errors.add(:base, message) log_verification(user, :failed_attempt, reason) - - prompt_for_email_verification(user) end def handle_verification_success(user) @@ -135,6 +133,10 @@ def handle_verification_success(user) log_verification(user, :successful) sign_in(user) + + log_audit_event(current_user, user, with: authentication_method) + log_user_activity(user) + verify_known_sign_in end def trusted_ip_address?(user) @@ -146,6 +148,7 @@ def trusted_ip_address?(user) def prompt_for_email_verification(user) session[:verification_user_id] = user.id self.resource = user + add_gon_variables # Necessary to set the sprite_icons path, since we skip the ApplicationController before_filters render 'devise/sessions/email_verification' end diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index 9ef347fff1612c9ee6d5b77ca5a99aba8b3b1d44..d5b642994f19d92e7af6450693ff8a69086bde54 100644 --- a/app/helpers/sessions_helper.rb +++ b/app/helpers/sessions_helper.rb @@ -40,10 +40,6 @@ def set_session_time(expiry_s) request.env['rack.session.options'][:expire_after] = expiry_s end - def send_rate_limited?(user) - Gitlab::ApplicationRateLimiter.peek(:email_verification_code_send, scope: user) - end - def obfuscated_email(email) # Moved to Gitlab::Utils::Email in 15.9 Gitlab::Utils::Email.obfuscated_email(email) @@ -52,4 +48,12 @@ def obfuscated_email(email) def remember_me_enabled? Gitlab::CurrentSettings.remember_me_enabled? end + + def verification_data(user) + { + obfuscated_email: obfuscated_email(user.email), + verify_path: session_path(:user), + resend_path: users_resend_verification_code_path + } + end end diff --git a/app/views/devise/sessions/email_verification.haml b/app/views/devise/sessions/email_verification.haml index e0b5a2669613a875ad16ad099523e1dfd422a404..085204fb6bfbb829e91df90a7675a223077d628d 100644 --- a/app/views/devise/sessions/email_verification.haml +++ b/app/views/devise/sessions/email_verification.haml @@ -2,18 +2,7 @@ = render 'devise/shared/tab_single', tab_title: s_('IdentityVerification|Help us protect your account') .login-box.gl-p-5 .login-body - = gitlab_ui_form_for(resource, as: resource_name, url: session_path(resource_name), method: :post, html: { class: 'gl-show-field-errors' }) do |f| - %p - = s_("IdentityVerification|For added security, you'll need to verify your identity. We've sent a verification code to %{email}").html_safe % { email: "<strong>#{sanitize(obfuscated_email(resource.email))}</strong>".html_safe } - %div - = f.label :verification_token, s_('IdentityVerification|Verification code') - = f.text_field :verification_token, class: 'form-control gl-form-input', required: true, autofocus: true, autocomplete: 'off', title: s_('IdentityVerification|Please enter a valid code'), inputmode: 'numeric', maxlength: 6, pattern: '[0-9]{6}' - %p.gl-field-error.gl-mt-2 - = resource.errors.full_messages.to_sentence - .gl-mt-5 - = f.submit s_('IdentityVerification|Verify code'), class: 'gl-w-full', pajamas_button: true - - unless send_rate_limited?(resource) - = link_to s_('IdentityVerification|Resend code'), users_resend_verification_code_path, method: :post, class: 'form-control gl-button btn-link gl-mt-3 gl-mb-0' + .js-email-verification{ data: verification_data(resource) } %p.gl-p-5.gl-text-secondary - support_link_start = '<a href="https://about.gitlab.com/support/" target="_blank" rel="noopener noreferrer">'.html_safe = s_("IdentityVerification|If you've lost access to the email associated to this account or having trouble with the code, %{link_start}here are some other steps you can take.%{link_end}").html_safe % { link_start: support_link_start, link_end: '</a>'.html_safe } diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index a09b3318c25fad714b65cd2b784b631bfb9ec670..ce9703753cf30516a99403146848b1c1c5daf2bc 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -538,6 +538,26 @@ def authenticate_2fa(user_params) expect(AuthenticationEvent.last.provider).to eq("two-factor-via-webauthn-device") end end + + context 'when the user is locked and submits a valid verification token' do + let(:user) { create(:user) } + let(:user_params) { { verification_token: 'token' } } + let(:session_params) { { verification_user_id: user.id } } + let(:post_action) { post(:create, params: { user: user_params }, session: session_params) } + + before do + encrypted_token = Devise.token_generator.digest(User, user.email, 'token') + user.update!(locked_at: Time.current, unlock_token: encrypted_token) + end + + it_behaves_like 'known sign in' + + it 'successfully logs in a user' do + post_action + + expect(subject.current_user).to eq user + end + end end context 'when login fails' do diff --git a/spec/features/users/email_verification_on_login_spec.rb b/spec/features/users/email_verification_on_login_spec.rb index 1854e812b7306efc4c8459147d52288917b4a647..c9b1670be820b827a27c06e2c02855f72f17ee02 100644 --- a/spec/features/users/email_verification_on_login_spec.rb +++ b/spec/features/users/email_verification_on_login_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Email Verification On Login', :clean_gitlab_redis_rate_limiting, feature_category: :system_access do +RSpec.describe 'Email Verification On Login', :clean_gitlab_redis_rate_limiting, :js, feature_category: :system_access do include EmailHelpers let_it_be(:user) { create(:user) } @@ -33,7 +33,7 @@ # Expect to see the verification form on the login page expect(page).to have_current_path(new_user_session_path) - expect(page).to have_content('Help us protect your account') + expect(page).to have_content(s_('IdentityVerification|Help us protect your account')) # Expect an instructions email to be sent with a code code = expect_instructions_email_and_extract_code @@ -41,7 +41,7 @@ # Signing in again prompts for the code and doesn't send a new one gitlab_sign_in(user) expect(page).to have_current_path(new_user_session_path) - expect(page).to have_content('Help us protect your account') + expect(page).to have_content(s_('IdentityVerification|Help us protect your account')) # Verify the code verify_code(code) @@ -54,7 +54,7 @@ # Expect a confirmation page with a meta refresh tag for 3 seconds to the root expect(page).to have_current_path(users_successful_verification_path) - expect(page).to have_content('Verification successful') + expect(page).to have_content(s_('IdentityVerification|Verification successful')) expect(page).to have_selector("meta[http-equiv='refresh'][content='3; url=#{root_path}']", visible: false) end end @@ -69,7 +69,8 @@ code = expect_instructions_email_and_extract_code # Request a new code - click_link 'Resend code' + click_button s_('IdentityVerification|Resend code') + expect(page).to have_content(s_('IdentityVerification|A new code has been sent.')) expect_log_message('Instructions Sent', 2) new_code = expect_instructions_email_and_extract_code @@ -83,22 +84,16 @@ gitlab_sign_in(user) # It shows a resend button - expect(page).to have_link 'Resend code' + expect(page).to have_button s_('IdentityVerification|Resend code') # Resend more than the rate limited amount of times 10.times do - click_link 'Resend code' + click_button s_('IdentityVerification|Resend code') end - # Expect the link to be gone - expect(page).not_to have_link 'Resend code' - - # Wait for 1 hour - travel 1.hour - - # Now it's visible again - gitlab_sign_in(user) - expect(page).to have_link 'Resend code' + # Expect an error alert + expect(page).to have_content format(s_("IdentityVerification|You've reached the maximum amount of resends. "\ + 'Wait %{interval} and try again.'), interval: 'about 1 hour') end end @@ -118,8 +113,9 @@ # Expect an error message expect_log_message('Failed Attempt', reason: 'rate_limited') - expect(page).to have_content("You've reached the maximum amount of tries. "\ - 'Wait 10 minutes or send a new code and try again.') + expect(page).to have_content( + format(s_("IdentityVerification|You've reached the maximum amount of tries. "\ + 'Wait %{interval} or send a new code and try again.'), interval: '10 minutes')) # Wait for 10 minutes travel 10.minutes @@ -139,7 +135,8 @@ # Expect an error message expect_log_message('Failed Attempt', reason: 'invalid') - expect(page).to have_content('The code is incorrect. Enter it again, or send a new code.') + expect(page).to have_content(s_('IdentityVerification|The code is incorrect. '\ + 'Enter it again, or send a new code.')) end it 'verifies expired codes' do @@ -156,7 +153,7 @@ # Expect an error message expect_log_message('Failed Attempt', reason: 'expired') - expect(page).to have_content('The code has expired. Send a new code and try again.') + expect(page).to have_content(s_('IdentityVerification|The code has expired. Send a new code and try again.')) end end end @@ -250,7 +247,8 @@ it 'shows an error message on on the login page' do expect(page).to have_current_path(new_user_session_path) - expect(page).to have_content('Maximum login attempts exceeded. Wait 10 minutes and try again.') + expect(page).to have_content(format(s_('IdentityVerification|Maximum login attempts exceeded. '\ + 'Wait %{interval} and try again.'), interval: '10 minutes')) end end @@ -271,7 +269,7 @@ stub_feature_flags(require_email_verification: false) # Resending and veryfying the code work as expected - click_link 'Resend code' + click_button s_('IdentityVerification|Resend code') new_code = expect_instructions_email_and_extract_code verify_code(code) @@ -283,7 +281,7 @@ verify_code(new_code) expect(page).to have_content(s_('IdentityVerification|The code has expired. Send a new code and try again.')) - click_link 'Resend code' + click_button s_('IdentityVerification|Resend code') another_code = expect_instructions_email_and_extract_code verify_code(another_code) diff --git a/spec/frontend/sessions/new/components/email_verification_spec.js b/spec/frontend/sessions/new/components/email_verification_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..8ff139e8475aa26d3b68c034b962c9f21d663783 --- /dev/null +++ b/spec/frontend/sessions/new/components/email_verification_spec.js @@ -0,0 +1,205 @@ +import { GlForm, GlFormInput } from '@gitlab/ui'; +import axios from 'axios'; +import MockAdapter from 'axios-mock-adapter'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import { s__ } from '~/locale'; +import { createAlert, VARIANT_SUCCESS } from '~/alert'; +import { HTTP_STATUS_NOT_FOUND, HTTP_STATUS_OK } from '~/lib/utils/http_status'; +import EmailVerification from '~/sessions/new/components/email_verification.vue'; +import { visitUrl } from '~/lib/utils/url_utility'; +import { + I18N_EMAIL_EMPTY_CODE, + I18N_EMAIL_INVALID_CODE, + I18N_GENERIC_ERROR, + I18N_RESEND_LINK, + I18N_EMAIL_RESEND_SUCCESS, +} from '~/sessions/new/constants'; + +jest.mock('~/alert'); +jest.mock('~/lib/utils/url_utility', () => ({ + ...jest.requireActual('~/lib/utils/url_utility'), + visitUrl: jest.fn(), +})); + +describe('EmailVerification', () => { + let wrapper; + let axiosMock; + + const defaultPropsData = { + obfuscatedEmail: 'al**@g*****.com', + verifyPath: '/users/sign_in', + resendPath: '/users/resend_verification_code', + }; + + const createComponent = () => { + wrapper = mountExtended(EmailVerification, { + propsData: defaultPropsData, + }); + }; + + const findForm = () => wrapper.findComponent(GlForm); + const findCodeInput = () => wrapper.findComponent(GlFormInput); + const findSubmitButton = () => wrapper.find('[type="submit"]'); + const findResendLink = () => wrapper.findByText(I18N_RESEND_LINK); + const enterCode = (code) => findCodeInput().setValue(code); + const submitForm = () => findForm().trigger('submit'); + + beforeEach(() => { + axiosMock = new MockAdapter(axios); + createComponent(); + }); + + afterEach(() => { + createAlert.mockClear(); + axiosMock.restore(); + }); + + describe('rendering the form', () => { + it('contains the obfuscated email address', () => { + expect(wrapper.text()).toContain(defaultPropsData.obfuscatedEmail); + }); + }); + + describe('verifying the code', () => { + describe('when successfully verifying the code', () => { + const redirectPath = 'root'; + + beforeEach(async () => { + enterCode('123456'); + + axiosMock + .onPost(defaultPropsData.verifyPath) + .reply(HTTP_STATUS_OK, { status: 'success', redirect_path: redirectPath }); + + await submitForm(); + await axios.waitForAll(); + }); + + it('redirects to the returned redirect path', () => { + expect(visitUrl).toHaveBeenCalledWith(redirectPath); + }); + }); + + describe('error messages', () => { + it.each` + scenario | code | submit | codeValid | errorShown | message + ${'shows no error messages before submitting the form'} | ${''} | ${false} | ${false} | ${false} | ${null} + ${'shows no error messages before submitting the form'} | ${'xxx'} | ${false} | ${false} | ${false} | ${null} + ${'shows no error messages before submitting the form'} | ${'123456'} | ${false} | ${true} | ${false} | ${null} + ${'shows empty code error message when submitting the form'} | ${''} | ${true} | ${false} | ${true} | ${I18N_EMAIL_EMPTY_CODE} + ${'shows invalid error message when submitting the form'} | ${'xxx'} | ${true} | ${false} | ${true} | ${I18N_EMAIL_INVALID_CODE} + ${'shows incorrect code error message returned from the server'} | ${'123456'} | ${true} | ${true} | ${true} | ${s__('IdentityVerification|The code is incorrect. Enter it again, or send a new code.')} + `(`$scenario with code $code`, async ({ code, submit, codeValid, errorShown, message }) => { + enterCode(code); + + if (submit && codeValid) { + axiosMock + .onPost(defaultPropsData.verifyPath) + .replyOnce(HTTP_STATUS_OK, { status: 'failure', message }); + } + + if (submit) { + await submitForm(); + await axios.waitForAll(); + } + + expect(findCodeInput().classes('is-invalid')).toBe(errorShown); + expect(findSubmitButton().props('disabled')).toBe(errorShown); + if (message) expect(wrapper.text()).toContain(message); + }); + + it('keeps showing error messages for invalid codes after submitting the form', async () => { + const serverErrorMessage = 'error message'; + + enterCode('123456'); + + axiosMock + .onPost(defaultPropsData.verifyPath) + .replyOnce(HTTP_STATUS_OK, { status: 'failure', message: serverErrorMessage }); + + await submitForm(); + await axios.waitForAll(); + + expect(wrapper.text()).toContain(serverErrorMessage); + + await enterCode(''); + expect(wrapper.text()).toContain(I18N_EMAIL_EMPTY_CODE); + + await enterCode('xxx'); + expect(wrapper.text()).toContain(I18N_EMAIL_INVALID_CODE); + }); + + it('captures the error and shows an alert message when the request failed', async () => { + enterCode('123456'); + + axiosMock.onPost(defaultPropsData.verifyPath).replyOnce(HTTP_STATUS_OK, null); + + await submitForm(); + await axios.waitForAll(); + + expect(createAlert).toHaveBeenCalledWith({ + message: I18N_GENERIC_ERROR, + captureError: true, + error: expect.any(Error), + }); + }); + + it('captures the error and shows an alert message when the request undefined', async () => { + enterCode('123456'); + + axiosMock.onPost(defaultPropsData.verifyPath).reply(HTTP_STATUS_OK, { status: undefined }); + + await submitForm(); + await axios.waitForAll(); + + expect(createAlert).toHaveBeenCalledWith({ + message: I18N_GENERIC_ERROR, + captureError: true, + error: undefined, + }); + }); + }); + }); + + describe('resending the code', () => { + const failedMessage = 'Failure sending the code'; + const successAlertObject = { + message: I18N_EMAIL_RESEND_SUCCESS, + variant: VARIANT_SUCCESS, + }; + const failedAlertObject = { + message: failedMessage, + }; + const undefinedAlertObject = { + captureError: true, + error: undefined, + message: I18N_GENERIC_ERROR, + }; + const genericAlertObject = { + message: I18N_GENERIC_ERROR, + captureError: true, + error: expect.any(Error), + }; + + it.each` + scenario | statusCode | response | alertObject + ${'the code was successfully resend'} | ${HTTP_STATUS_OK} | ${{ status: 'success' }} | ${successAlertObject} + ${'there was a problem resending the code'} | ${HTTP_STATUS_OK} | ${{ status: 'failure', message: failedMessage }} | ${failedAlertObject} + ${'when the request is undefined'} | ${HTTP_STATUS_OK} | ${{ status: undefined }} | ${undefinedAlertObject} + ${'when the request failed'} | ${HTTP_STATUS_NOT_FOUND} | ${null} | ${genericAlertObject} + `(`shows an alert message when $scenario`, async ({ statusCode, response, alertObject }) => { + enterCode('xxx'); + + await submitForm(); + + axiosMock.onPost(defaultPropsData.resendPath).replyOnce(statusCode, response); + + findResendLink().trigger('click'); + + await axios.waitForAll(); + + expect(createAlert).toHaveBeenCalledWith(alertObject); + expect(findCodeInput().element.value).toBe(''); + }); + }); +}); diff --git a/spec/helpers/sessions_helper_spec.rb b/spec/helpers/sessions_helper_spec.rb index 5a46a20ce1a524f130b4c35a9148380099f7d63f..f35b6b28de804dd1e76b57cc8314c9964f421ea1 100644 --- a/spec/helpers/sessions_helper_spec.rb +++ b/spec/helpers/sessions_helper_spec.rb @@ -51,28 +51,15 @@ end end - describe '#send_rate_limited?' do + describe '#verification_data' do let(:user) { build_stubbed(:user) } - subject { helper.send_rate_limited?(user) } - - before do - allow(::Gitlab::ApplicationRateLimiter) - .to receive(:peek) - .with(:email_verification_code_send, scope: user) - .and_return(rate_limited) - end - - context 'when rate limited' do - let(:rate_limited) { true } - - it { is_expected.to eq(true) } - end - - context 'when not rate limited' do - let(:rate_limited) { false } - - it { is_expected.to eq(false) } + it 'returns the expected data' do + expect(helper.verification_data(user)).to eq({ + obfuscated_email: obfuscated_email(user.email), + verify_path: helper.session_path(:user), + resend_path: users_resend_verification_code_path + }) end end diff --git a/spec/requests/verifies_with_email_spec.rb b/spec/requests/verifies_with_email_spec.rb index f3f8e4a1a8300f2bf3e84c4f9cdd5da830ceebb8..1c7e1bc9217f9f8a937df7fb7a8d696a7636089a 100644 --- a/spec/requests/verifies_with_email_spec.rb +++ b/spec/requests/verifies_with_email_spec.rb @@ -147,12 +147,10 @@ post(user_session_path(user: { verification_token: 'token' })) end - it_behaves_like 'prompt for email verification' - it 'adds a verification error message' do - expect(response.body) - .to include("You've reached the maximum amount of tries. "\ - 'Wait 10 minutes or send a new code and try again.') + expect(json_response) + .to include('message' => "You've reached the maximum amount of tries. "\ + 'Wait 10 minutes or send a new code and try again.') end end @@ -161,11 +159,10 @@ post(user_session_path(user: { verification_token: 'invalid_token' })) end - it_behaves_like 'prompt for email verification' - it 'adds a verification error message' do - expect(response.body) - .to include((s_('IdentityVerification|The code is incorrect. Enter it again, or send a new code.'))) + expect(json_response) + .to include('message' => (s_('IdentityVerification|The code is incorrect. '\ + 'Enter it again, or send a new code.'))) end end @@ -175,27 +172,26 @@ post(user_session_path(user: { verification_token: 'token' })) end - it_behaves_like 'prompt for email verification' - it 'adds a verification error message' do - expect(response.body) - .to include((s_('IdentityVerification|The code has expired. Send a new code and try again.'))) + expect(json_response) + .to include('message' => (s_('IdentityVerification|The code has expired. Send a new code and try again.'))) end end context 'when a valid verification_token param exists' do - before do - post(user_session_path(user: { verification_token: 'token' })) + subject(:submit_token) { post(user_session_path(user: { verification_token: 'token' })) } + + it 'unlocks the user, create logs and records the activity', :freeze_time do + expect { submit_token }.to change { user.reload.unlock_token }.to(nil) + .and change { user.locked_at }.to(nil) + .and change { AuditEvent.count }.by(1) + .and change { AuthenticationEvent.count }.by(1) + .and change { user.last_activity_on }.to(Date.today) end - it 'unlocks the user' do - user.reload - expect(user.unlock_token).to be_nil - expect(user.locked_at).to be_nil - end - - it 'redirects to the successful verification path' do - expect(response).to redirect_to(users_successful_verification_path) + it 'returns the success status and a redirect path' do + submit_token + expect(json_response).to eq('status' => 'success', 'redirect_path' => users_successful_verification_path) end end @@ -206,8 +202,8 @@ post user_session_path, params: { user: { login: another_user.username, password: another_user.password } } end - it 'does not redirect to the successful verification path' do - expect(response).not_to redirect_to(users_successful_verification_path) + it 'redirects to the root path' do + expect(response).to redirect_to(root_path) end end end @@ -277,7 +273,6 @@ end it_behaves_like 'send verification instructions' - it_behaves_like 'prompt for email verification' end context 'when exceeding the rate limit' do @@ -301,8 +296,6 @@ mail = find_email_for(user) expect(mail).to be_nil end - - it_behaves_like 'prompt for email verification' end end diff --git a/spec/support/shared_examples/controllers/known_sign_in_shared_examples.rb b/spec/support/shared_examples/controllers/known_sign_in_shared_examples.rb index 3f147f942ba5427cffbffb3fac3a469e669db957..77dd67c77a48210e10e62cb6b765c5bdbc84061e 100644 --- a/spec/support/shared_examples/controllers/known_sign_in_shared_examples.rb +++ b/spec/support/shared_examples/controllers/known_sign_in_shared_examples.rb @@ -9,10 +9,8 @@ def stub_user_ip(ip) user.update!(current_sign_in_ip: ip) end - def stub_cookie(value = user.id) - cookies.encrypted[KnownSignIn::KNOWN_SIGN_IN_COOKIE] = { - value: value, expires: KnownSignIn::KNOWN_SIGN_IN_COOKIE_EXPIRY - } + def stub_cookie(value = user.id, expires = KnownSignIn::KNOWN_SIGN_IN_COOKIE_EXPIRY) + cookies.encrypted[KnownSignIn::KNOWN_SIGN_IN_COOKIE] = { value: value, expires: expires } end context 'when the remote IP and the last sign in IP match' do @@ -57,15 +55,13 @@ def stub_cookie(value = user.id) end it 'notifies the user when the cookie is expired' do - stub_cookie - - travel_to((KnownSignIn::KNOWN_SIGN_IN_COOKIE_EXPIRY + 1.day).from_now) do - expect_next_instance_of(NotificationService) do |instance| - expect(instance).to receive(:unknown_sign_in) - end + stub_cookie(user.id, 1.day.ago) - post_action + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:unknown_sign_in) end + + post_action end context 'when notify_on_unknown_sign_in global setting is false' do