Commit f2883f40 authored by Stan Hu's avatar Stan Hu

Fix 500 error when unconfirmed OAuth2 user with 2FA logs in

When a user with two-factor auth enabled attempts to use an OAuth2
provider to sign-in, the user would see a 500 error without explanation
why. This occurred because the failure case in
OmniauthCallbacksController was attempting to render the partial of the
new session, but the CAPTCHA helpers are only defined for
SessionsController, not for this one.

To fix this problem, redirect the page with the alert to the sign-in
page and display a flash alert with a notice about an unconfirmed
e-mail. The redirection also cleans up the URL so that the page doesn't
look like it starts from an Omniauth callback.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/232611
parent 65b896d3
......@@ -33,9 +33,7 @@ module AuthenticatesWithTwoFactor
end
def locked_user_redirect(user)
flash.now[:alert] = locked_user_redirect_alert(user)
render 'devise/sessions/new'
redirect_to new_user_session_path, alert: locked_user_redirect_alert(user)
end
def authenticate_with_two_factor
......@@ -54,7 +52,13 @@ module AuthenticatesWithTwoFactor
private
def locked_user_redirect_alert(user)
user.access_locked? ? _('Your account is locked.') : _('Invalid Login or password')
if user.access_locked?
_('Your account is locked.')
elsif !user.confirmed?
I18n.t('devise.failure.unconfirmed')
else
_('Invalid Login or password')
end
end
def clear_two_factor_attempt!
......
---
title: Fix 500 error when unconfirmed OAuth2 user with 2FA logs in
merge_request: 38104
author:
type: fixed
......@@ -181,6 +181,23 @@ RSpec.describe OmniauthCallbacksController, type: :controller do
end
end
context 'when user with 2FA is unconfirmed' do
render_views
let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: provider) }
before do
user.update_column(:confirmed_at, nil)
end
it 'redirects to login page' do
post provider
expect(response).to redirect_to(new_user_session_path)
expect(flash[:alert]).to match(/You have to confirm your email address before continuing./)
end
end
context 'sign up' do
include_context 'sign_up'
......
......@@ -399,7 +399,7 @@ RSpec.describe SessionsController do
end
it 'warns about invalid login' do
expect(response).to set_flash.now[:alert].to /Your account is locked./
expect(flash[:alert]).to eq('Your account is locked.')
end
it 'locks the user' do
......@@ -409,7 +409,7 @@ RSpec.describe SessionsController do
it 'keeps the user locked on future login attempts' do
post(:create, params: { user: { login: user.username, password: user.password } })
expect(response).to set_flash.now[:alert].to /Your account is locked./
expect(flash[:alert]).to eq('Your account is locked.')
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment