Commit fab6a50f authored by James Edwards-Jones's avatar James Edwards-Jones

Prevent password sign in restriction bypass

parent 5396b2a8
...@@ -18,6 +18,7 @@ class SessionsController < Devise::SessionsController ...@@ -18,6 +18,7 @@ class SessionsController < Devise::SessionsController
prepend_before_action :store_redirect_uri, only: [:new] prepend_before_action :store_redirect_uri, only: [:new]
prepend_before_action :ldap_servers, only: [:new, :create] prepend_before_action :ldap_servers, only: [:new, :create]
prepend_before_action :require_no_authentication_without_flash, only: [:new, :create] prepend_before_action :require_no_authentication_without_flash, only: [:new, :create]
prepend_before_action :ensure_password_authentication_enabled!, if: :password_based_login?, only: [:create]
before_action :auto_sign_in_with_provider, only: [:new] before_action :auto_sign_in_with_provider, only: [:new]
before_action :load_recaptcha before_action :load_recaptcha
...@@ -138,6 +139,14 @@ class SessionsController < Devise::SessionsController ...@@ -138,6 +139,14 @@ class SessionsController < Devise::SessionsController
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def ensure_password_authentication_enabled!
render_403 unless Gitlab::CurrentSettings.password_authentication_enabled_for_web?
end
def password_based_login?
user_params[:login].present? || user_params[:password].present?
end
def user_params def user_params
params.require(:user).permit(:login, :password, :remember_me, :otp_attempt, :device_response) params.require(:user).permit(:login, :password, :remember_me, :otp_attempt, :device_response)
end end
......
---
title: Prevent bypass of restriction disabling web password sign in
merge_request:
author:
type: security
...@@ -58,7 +58,26 @@ describe SessionsController do ...@@ -58,7 +58,26 @@ describe SessionsController do
it 'authenticates user correctly' do it 'authenticates user correctly' do
post(:create, params: { user: user_params }) post(:create, params: { user: user_params })
expect(subject.current_user). to eq user expect(subject.current_user).to eq user
end
context 'with password authentication disabled' do
before do
stub_application_setting(password_authentication_enabled_for_web: false)
end
it 'does not sign in the user' do
post(:create, params: { user: user_params })
expect(@request.env['warden']).not_to be_authenticated
expect(subject.current_user).to be_nil
end
it 'returns status 403' do
post(:create, params: { user: user_params })
expect(response.status).to eq 403
end
end end
it 'creates an audit log record' do it 'creates an audit log record' do
...@@ -153,6 +172,19 @@ describe SessionsController do ...@@ -153,6 +172,19 @@ describe SessionsController do
end end
end end
context 'with password authentication disabled' do
before do
stub_application_setting(password_authentication_enabled_for_web: false)
end
it 'allows 2FA stage of non-password login' do
authenticate_2fa(otp_attempt: user.current_otp)
expect(@request.env['warden']).to be_authenticated
expect(subject.current_user).to eq user
end
end
## ##
# See #14900 issue # See #14900 issue
# #
......
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