Merge branch 'security-jej/prevent-web-sign-in-bypass' into 'master'

Prevent password sign in restriction bypass

See merge request gitlab/gitlabhq!2702

3 files changed, 47 insertions, 1 deletions

diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 6fea61cf45d..a841859621e 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -18,6 +18,7 @@ class SessionsController < Devise::SessionsController
   prepend_before_action :store_redirect_uri, only: [:new]
   prepend_before_action :ldap_servers, 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 :load_recaptcha
@@ -138,6 +139,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 6bcff7f975c..9c4ddce5409 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -58,7 +58,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
@@ -153,6 +172,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
       #