diff --git a/app/assets/javascripts/sessions/new/components/email_verification.vue b/app/assets/javascripts/sessions/new/components/email_verification.vue
index 02f0d3c2f96e3d7d58ac35a0bd100ff0b44bd9df..e958ad723ccb0de02b32724927a49db315a8f37f 100644
--- a/app/assets/javascripts/sessions/new/components/email_verification.vue
+++ b/app/assets/javascripts/sessions/new/components/email_verification.vue
@@ -11,11 +11,13 @@ import {
I18N_EMAIL_INVALID_CODE,
I18N_SUBMIT_BUTTON,
I18N_RESEND_LINK,
+ I18N_SKIP_FOR_NOW_BUTTON,
I18N_EMAIL_RESEND_SUCCESS,
I18N_GENERIC_ERROR,
I18N_HELP_TEXT,
I18N_SEND_TO_SECONDARY_EMAIL_BUTTON_TEXT,
I18N_SEND_TO_SECONDARY_EMAIL_GUIDE,
+ I18N_DIDNT_GET_CODE,
SUPPORT_URL,
VERIFICATION_CODE_REGEX,
SUCCESS_RESPONSE,
@@ -55,6 +57,15 @@ export default {
type: String,
required: true,
},
+ skipPath: {
+ type: String,
+ required: true,
+ },
+ permittedToSkipEmailOtpInGracePeriod: {
+ type: Boolean,
+ required: true,
+ default: false,
+ },
},
data() {
return {
@@ -87,6 +98,9 @@ export default {
return this.verifyError;
},
+ showSkipButton() {
+ return this.permittedToSkipEmailOtpInGracePeriod;
+ },
},
watch: {
verificationCode() {
@@ -124,6 +138,20 @@ export default {
this.verifyToken(email);
this.resend(email);
},
+ skip() {
+ if (!this.skipPath) return;
+
+ axios.post(this.skipPath).then(this.handleSkipResponse).catch(this.handleError);
+ },
+ handleSkipResponse(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) {
+ createAlert({ message: response.data.message });
+ }
+ },
handleResendResponse(response) {
if (response.data.status === undefined) {
this.handleError();
@@ -162,7 +190,9 @@ export default {
helpText: I18N_HELP_TEXT,
sendToSecondaryEmailButtonText: I18N_SEND_TO_SECONDARY_EMAIL_BUTTON_TEXT,
sendToSecondaryEmailGuide: I18N_SEND_TO_SECONDARY_EMAIL_GUIDE,
+ skipForNowButton: I18N_SKIP_FOR_NOW_BUTTON,
supportUrl: SUPPORT_URL,
+ didntGetCode: I18N_DIDNT_GET_CODE,
},
forms: {
verifyTokenForm: VERIFY_TOKEN_FORM,
@@ -194,6 +224,7 @@ export default {
label-for="verification-code"
:state="inputValidation.state"
:invalid-feedback="inputValidation.message"
+ class="gl-mb-1"
>
+
+
+
+ {{ $options.i18n.didntGetCode }}
+ {{
+ $options.i18n.resendLink
+ }}
+
+
{{
$options.i18n.submitButton
}}
- {{
- $options.i18n.resendLink
- }}
+
+
+
+ {{ $options.i18n.skipForNowButton }}
+
+
diff --git a/app/assets/javascripts/sessions/new/constants.js b/app/assets/javascripts/sessions/new/constants.js
index 8e4367b0a02d883f60b46f2cb949f7e7f8cdabf6..8bf731bb88ab9c566aea3a81f1bf7087fa2ca42f 100644
--- a/app/assets/javascripts/sessions/new/constants.js
+++ b/app/assets/javascripts/sessions/new/constants.js
@@ -9,12 +9,15 @@ 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_RESEND_LINK = s__('IdentityVerification|Resend.');
+export const I18N_SKIP_FOR_NOW_BUTTON = s__('IdentityVerification|Skip for now');
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 I18N_DIDNT_GET_CODE = s__("IdentityVerification|Didn't get the code?");
+
export const I18N_EMAIL = __('Email');
export const I18N_SEND_TO_SECONDARY_EMAIL_BUTTON_TEXT = s__(
'IdentityVerification|send a code to another address associated with this account',
diff --git a/app/assets/javascripts/sessions/new/index.js b/app/assets/javascripts/sessions/new/index.js
index f7679b28e040f6c1214ca1f07015f529d5fae503..8945a66e73d1769bcdbde47a6ccbbea19d6ff428 100644
--- a/app/assets/javascripts/sessions/new/index.js
+++ b/app/assets/javascripts/sessions/new/index.js
@@ -8,7 +8,15 @@ export default () => {
return null;
}
- const { username, obfuscatedEmail, verifyPath, resendPath } = el.dataset;
+ const {
+ username,
+ obfuscatedEmail,
+ verifyPath,
+ resendPath,
+ skipPath,
+ permittedToSkipEmailOtpInGracePeriod,
+ emailOtpRequiredAfter,
+ } = el.dataset;
return new Vue({
el,
@@ -20,6 +28,9 @@ export default () => {
obfuscatedEmail,
verifyPath,
resendPath,
+ skipPath,
+ permittedToSkipEmailOtpInGracePeriod,
+ emailOtpRequiredAfter,
},
});
},
diff --git a/app/controllers/concerns/verifies_with_email.rb b/app/controllers/concerns/verifies_with_email.rb
index b97aaa47aba4a5be0b9dca1e99b405c27f0c6cd9..6b442844d980cdaee47ba78d413ec4d92de6e3a3 100644
--- a/app/controllers/concerns/verifies_with_email.rb
+++ b/app/controllers/concerns/verifies_with_email.rb
@@ -87,6 +87,29 @@ def successful_verification
render layout: 'minimal'
end
+ def skip_verification_for_now
+ user = find_verification_user
+ return unless user && permitted_to_skip_email_otp_in_grace_period?(user)
+
+ session.delete(:verification_user_id)
+ sign_in(user)
+ user.update(email_otp: nil) if user.email_otp.present?
+
+ log_verification(user, :skipped, 'user chose to skip verification in grace period')
+ log_user_activity(user)
+
+ render json: { status: :success, redirect_path: users_skip_verification_confirmation_path }
+ end
+
+ def skip_verification_confirmation
+ render 'skip_verification',
+ layout: 'minimal',
+ locals: {
+ redirect_url: after_sign_in_path_for(current_user),
+ email_otp_required_after: current_user.email_otp_required_after
+ }
+ end
+
private
def skip_verify_with_email?
@@ -144,15 +167,6 @@ def requires_verify_email?(user)
treat_as_locked?(user) || !trusted_ip_address?(user) || require_email_based_otp?(user)
end
- def treat_as_locked?(user)
- # A user can have #access_locked? return false, but we still want
- # to treat as locked during sign in if they were sent an unlock
- # token in the past.
- # See https://docs.gitlab.com/security/unlock_user/#gitlabcom-users
- # and https://gitlab.com/gitlab-org/gitlab/-/issues/560080.
- user.access_locked? || user.unlock_token
- end
-
def verify_email(user)
return true unless requires_verify_email?(user)
@@ -209,7 +223,7 @@ def require_email_based_otp?(user)
# Devise::Confirmable
user.last_sign_in_at.present? &&
user.email_otp_required_after.present? &&
- user.email_otp_required_after <= Time.zone.now
+ (user.email_otp_required_after <= Time.zone.now || user.in_email_otp_grace_period?)
end
def verify_token(user, token)
@@ -278,10 +292,6 @@ def handle_verification_success(user)
verify_known_sign_in
end
- def trusted_ip_address?(user)
- AuthenticationEvent.initial_login_or_known_ip_address?(user, request.ip)
- end
-
def prompt_for_email_verification(user)
session[:verification_user_id] = user.id
self.resource = user
diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb
index b1cf095edaae6f55979c39cf1dae1ee1e1c66423..6b31213ec6bbd60feb0740136244428b850f5232 100644
--- a/app/helpers/sessions_helper.rb
+++ b/app/helpers/sessions_helper.rb
@@ -26,7 +26,31 @@ def verification_data(user)
username: user.username,
obfuscated_email: obfuscated_email(user.email),
verify_path: session_path(:user),
- resend_path: users_resend_verification_code_path
+ resend_path: users_resend_verification_code_path,
+ skip_path: users_skip_verification_for_now_path,
+ permitted_to_skip_email_otp_in_grace_period: permitted_to_skip_email_otp_in_grace_period?(user),
+ email_otp_required_after: user.email_otp_required_after
}
end
+
+ # Used by frontend to decide if we should render the "skip for now" button
+ def permitted_to_skip_email_otp_in_grace_period?(user)
+ Feature.enabled?(:email_based_mfa, user) &&
+ trusted_ip_address?(user) &&
+ !treat_as_locked?(user) &&
+ user.in_email_otp_grace_period?
+ end
+
+ def trusted_ip_address?(user)
+ AuthenticationEvent.initial_login_or_known_ip_address?(user, request.ip)
+ end
+
+ def treat_as_locked?(user)
+ # A user can have #access_locked? return false, but we still want
+ # to treat as locked during sign in if they were sent an unlock
+ # token in the past.
+ # See https://docs.gitlab.com/security/unlock_user/#gitlabcom-users
+ # and https://gitlab.com/gitlab-org/gitlab/-/issues/560080.
+ user.access_locked? || user.unlock_token
+ end
end
diff --git a/app/models/user.rb b/app/models/user.rb
index 6ff5caeab76ebc86fd5ef3a172facc9aedea1847..cae51f59f653cfbb11696e2a6bd49710ccb5dfe4 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -756,6 +756,17 @@ def inactive_message
end
end
+ def in_email_otp_grace_period?
+ now = Time.zone.now
+ return false unless email_otp_required_after.present? && email_otp_required_after > now
+
+ # Reminder that `between` is inclusive
+ now.between?(
+ email_otp_required_after - 30.days,
+ email_otp_required_after
+ )
+ end
+
def self.with_visible_profile(user)
return with_public_profile if user.nil?
diff --git a/app/views/devise/sessions/_skip_verification.haml b/app/views/devise/sessions/_skip_verification.haml
new file mode 100644
index 0000000000000000000000000000000000000000..65392f29ffa8d5d2c265d9a58a58eb69818d25fe
--- /dev/null
+++ b/app/views/devise/sessions/_skip_verification.haml
@@ -0,0 +1,10 @@
+= content_for :meta_tags do
+ %meta{ 'http-equiv': 'refresh', content: "5; url=#{redirect_url}" }
+
+.gl-text-center.gl-max-w-62.gl-mx-auto{ data: local_assigns.fetch(:data, {}) }
+ .svg-content.svg-80
+ = image_tag 'illustrations/gitlab_logo.svg'
+ %h2
+ = s_('IdentityVerification|You will be automatically redirected')
+ %p.gl-pt-2
+ = html_escape(s_("IdentityVerification|You can skip email verification for now. Starting on %{date}, email verification will be mandatory.")) % { date: email_otp_required_after.strftime('%B %d, %Y') }
diff --git a/app/views/devise/sessions/skip_verification.haml b/app/views/devise/sessions/skip_verification.haml
new file mode 100644
index 0000000000000000000000000000000000000000..a57389f52792894a5715dfc38c3a5a43b1913fff
--- /dev/null
+++ b/app/views/devise/sessions/skip_verification.haml
@@ -0,0 +1 @@
+= render 'devise/sessions/skip_verification', local_assigns
diff --git a/config/routes/user.rb b/config/routes/user.rb
index 3ad2bb1c84d8fdff8a5c03e4589e34a8ca54f379..42fb3e02ecafe81ae7b672d8a31303c722942d94 100644
--- a/config/routes/user.rb
+++ b/config/routes/user.rb
@@ -55,6 +55,8 @@ def override_omniauth(provider, controller, path_prefix = '/users/auth')
get '/users/almost_there' => 'confirmations#almost_there'
post '/users/resend_verification_code', to: 'sessions#resend_verification_code'
get '/users/successful_verification', to: 'sessions#successful_verification'
+ post '/users/skip_verification_for_now', to: 'sessions#skip_verification_for_now'
+ get '/users/skip_verification_confirmation', to: 'sessions#skip_verification_confirmation'
# Redirect on GitHub authorization request errors. E.g. it could happen when user:
# 1. cancel authorization the GitLab OAuth app via GitHub to import GitHub repos
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 2a3806ef876cb168e86265c60d3e0bcbc299365c..7cde1c5aad8c9e9b5ccdc3c723c30fff9c5044ae 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -33213,6 +33213,9 @@ msgstr ""
msgid "IdentityVerification|Country or region"
msgstr ""
+msgid "IdentityVerification|Didn't get the code?"
+msgstr ""
+
msgid "IdentityVerification|Email Verification"
msgstr ""
@@ -33291,7 +33294,7 @@ msgstr ""
msgid "IdentityVerification|Please enter a valid email address."
msgstr ""
-msgid "IdentityVerification|Resend code"
+msgid "IdentityVerification|Resend."
msgstr ""
msgid "IdentityVerification|Select country or region"
@@ -33303,6 +33306,9 @@ msgstr ""
msgid "IdentityVerification|Send code in %{timer}"
msgstr ""
+msgid "IdentityVerification|Skip for now"
+msgstr ""
+
msgid "IdentityVerification|Something went wrong. Please try again."
msgstr ""
@@ -33378,6 +33384,12 @@ msgstr ""
msgid "IdentityVerification|You are signed in as %{username}. For added security, you'll need to verify your identity. We've sent a verification code to %{email}"
msgstr ""
+msgid "IdentityVerification|You can skip email verification for now. Starting on %{date}, email verification will be mandatory."
+msgstr ""
+
+msgid "IdentityVerification|You will be automatically redirected"
+msgstr ""
+
msgid "IdentityVerification|You will receive a text containing a code. Standard charges may apply."
msgstr ""
diff --git a/spec/features/users/email_verification_on_login_spec.rb b/spec/features/users/email_verification_on_login_spec.rb
index b898de8912c9f0c6da3f78f847f12f453f71a89a..03ae0d0e6e7da5415c7ea4b9711ad3ec601f6be1 100644
--- a/spec/features/users/email_verification_on_login_spec.rb
+++ b/spec/features/users/email_verification_on_login_spec.rb
@@ -132,7 +132,7 @@
it 'rate limits resends' do
gitlab_sign_in(user)
- expect(page).to have_button s_('IdentityVerification|Resend code')
+ expect(page).to have_button s_('IdentityVerification|Resend')
# Resend more than the rate limited amount of times
10.times { click_request_new_code_button }
@@ -465,6 +465,6 @@ def perform_verification_with_code(code)
end
def click_request_new_code_button
- click_button s_('IdentityVerification|Resend code')
+ click_button s_('IdentityVerification|Resend')
end
end
diff --git a/spec/helpers/sessions_helper_spec.rb b/spec/helpers/sessions_helper_spec.rb
index e6ddad0ebaa6171853f21a84f8b49596e5792d00..abd2fb9f353655ba4904b0a7dd2edfae2cf85dc3 100644
--- a/spec/helpers/sessions_helper_spec.rb
+++ b/spec/helpers/sessions_helper_spec.rb
@@ -21,11 +21,14 @@
let(:user) { build_stubbed(:user) }
it 'returns the expected data' do
- expect(helper.verification_data(user)).to eq({
+ expect(helper.verification_data(user)).to match({
username: user.username,
obfuscated_email: obfuscated_email(user.email),
verify_path: helper.session_path(:user),
- resend_path: users_resend_verification_code_path
+ resend_path: users_resend_verification_code_path,
+ skip_path: users_skip_verification_for_now_path,
+ email_otp_required_after: user.email_otp_required_after,
+ permitted_to_skip_email_otp_in_grace_period: permitted_to_skip_email_otp_in_grace_period?(user)
})
end
end
@@ -84,4 +87,103 @@
it { is_expected.to be false }
end
end
+
+ describe '#trusted_ip_address' do
+ let(:trusted) { false }
+
+ before do
+ allow(AuthenticationEvent)
+ .to receive(:initial_login_or_known_ip_address?)
+ .and_return(trusted)
+ end
+
+ context 'when AuthenticationEvent.initial_login_or_known_ip_address returns false' do
+ it { is_expected.to be false }
+ end
+
+ context 'when AuthenticationEvent.initial_login_or_known_ip_address returns true' do
+ it { is_expected.to be true }
+ end
+ end
+
+ describe '#treat_as_locked' do
+ let(:user_access_locked) { false }
+ let(:user_unlock_token) { nil }
+ let(:user) do
+ build_stubbed(:user, access_locked?: user_access_locked, unlock_token: user_unlock_token)
+ end
+
+ context 'when user access is not locked and has no unlock token' do
+ it { is_expected.to be false }
+ end
+
+ context 'when user access is locked' do
+ let(:user_access_locked?) { true }
+
+ it { is_expected.to be true }
+ end
+
+ context 'when user unlock token is present' do
+ let(:user_unlock_token) { 'some token' }
+
+ it { is_expected.to be true }
+ end
+ end
+
+ describe '#permitted_to_skip_email_otp_in_grace_period?' do
+ let(:trusted_ip) { true }
+ let(:user_access_locked) { false }
+ let(:user_unlock_token) { nil }
+ let(:in_grace_period) { true }
+
+ let(:user) do
+ build_stubbed(:user,
+ access_locked?: user_access_locked,
+ unlock_token: user_unlock_token
+ )
+ end
+
+ before do
+ allow(AuthenticationEvent)
+ .to receive(:initial_login_or_known_ip_address?)
+ .and_return(trusted_ip)
+ allow(user).to receive(:in_email_otp_grace_period?).and_return(in_grace_period)
+ end
+
+ context 'when all conditions are met' do
+ it { is_expected.to be true }
+ end
+
+ context 'when email_based_mfa feature is disabled' do
+ before do
+ stub_feature_flags(email_based_mfa: false)
+ end
+
+ it { is_expected.to be false }
+ end
+
+ context 'when IP address is not trusted' do
+ let(:trusted_ip) { false }
+
+ it { is_expected.to be false }
+ end
+
+ context 'when user is treated as locked due to access_locked' do
+ let(:user_access_locked) { true }
+
+ it { is_expected.to be false }
+ end
+
+ context 'when user is treated as locked due to unlock token' do
+ let(:user_unlock_token) { 'some_token' }
+
+ it { is_expected.to be false }
+ end
+
+ context 'when user is not in email OTP grace period' do
+ let(:in_grace_period) { false }
+
+ it { is_expected.to be false }
+ end
+ end
end