diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-05-28 13:15:53 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-05-28 13:15:53 +0000 |
commit | 701914e1d9f6e5f56533c277fccdfc110d047a77 (patch) | |
tree | 6e9445064bc7a46ba4a37696f03872f3946f9c1b | |
parent | a6f2df2c8a70e154e655011572f59720dc80960c (diff) | |
parent | 4ace8900e9112115c1fb9d562d4f6c7392e34225 (diff) | |
download | gitlab-ce-701914e1d9f6e5f56533c277fccdfc110d047a77.tar.gz |
Merge branch 'security-jej/prevent-web-sign-in-bypass-11-10' into '11-10-stable'
Prevent password sign in restriction bypass
See merge request gitlab/gitlabhq!3120
-rw-r--r-- | app/controllers/sessions_controller.rb | 10 | ||||
-rw-r--r-- | changelogs/unreleased/security-jej-prevent-web-sign-in-bypass.yml | 5 | ||||
-rw-r--r-- | spec/controllers/sessions_controller_spec.rb | 34 |
3 files changed, 48 insertions, 1 deletions
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 4bd7d71e264..bbf734b69f1 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -15,6 +15,8 @@ class SessionsController < Devise::SessionsController prepend_before_action :check_captcha, only: [:create] prepend_before_action :store_redirect_uri, only: [:new] prepend_before_action :ldap_servers, 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 :load_recaptcha @@ -126,6 +128,14 @@ class SessionsController < Devise::SessionsController end # 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 params.require(:user).permit(:login, :password, :remember_me, :otp_attempt, :device_response) end diff --git a/changelogs/unreleased/security-jej-prevent-web-sign-in-bypass.yml b/changelogs/unreleased/security-jej-prevent-web-sign-in-bypass.yml new file mode 100644 index 00000000000..02773fa1d7c --- /dev/null +++ b/changelogs/unreleased/security-jej-prevent-web-sign-in-bypass.yml @@ -0,0 +1,5 @@ +--- +title: Prevent bypass of restriction disabling web password sign in +merge_request: +author: +type: security diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index ea7242c1aa8..661c6d79597 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -56,7 +56,26 @@ describe SessionsController do it 'authenticates user correctly' do 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 it 'creates an audit log record' do @@ -151,6 +170,19 @@ describe SessionsController do 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 # |