Merge branch 'protect_sessions_controller' into 'master'

Add blocking sign in for users not allowed to use password auth for web

See merge request gitlab-org/gitlab!60912

app/controllers/sessions_controller.rb

@@ -22,6 +22,7 @@ class SessionsController < Devise::SessionsController
   prepend_before_action :check_captcha, only: [:create]
   prepend_before_action :store_redirect_uri, only: [:new]
   prepend_before_action :require_no_authentication_without_flash, only: [:new, :create]
+  prepend_before_action :check_forbidden_password_based_login, if: -> { action_name == 'create' && password_based_login? }
   prepend_before_action :ensure_password_authentication_enabled!, if: -> { action_name == 'create' && password_based_login? }
 
   before_action :auto_sign_in_with_provider, only: [:new]
@@ -313,6 +314,13 @@ class SessionsController < Devise::SessionsController
   def set_invite_params
     @invite_email = ActionController::Base.helpers.sanitize(params[:invite_email])
   end
+
+  def check_forbidden_password_based_login
+    if find_user&.password_based_login_forbidden?
+      flash[:alert] = _('You are not allowed to log in using password')
+      redirect_to new_user_session_path
+    end
+  end
 end
 
 SessionsController.prepend_mod_with('SessionsController')

app/models/user.rb

@@ -1121,6 +1121,11 @@ class User < ApplicationRecord
     Gitlab::CurrentSettings.password_authentication_enabled_for_git? && !password_based_omniauth_user?
   end
 
+  # method overriden in EE
+  def password_based_login_forbidden?
+    false
+  end
+
   def can_change_username?
     gitlab_config.username_changing_enabled
   end

ee/app/models/ee/user.rb

@@ -328,6 +328,11 @@ module EE
       super
     end
 
+    override :password_based_login_forbidden?
+    def password_based_login_forbidden?
+      user_authorized_by_provisioning_group? || super
+    end
+
     def user_authorized_by_provisioning_group?
       user_detail.provisioned_by_group? && ::Feature.enabled?(:block_password_auth_for_saml_users, user_detail.provisioned_by_group, type: :ops)
     end

ee/spec/controllers/ee/sessions_controller_spec.rb

@@ -128,5 +128,16 @@ RSpec.describe SessionsController, :geo do
         end
       end
     end
+
+    context 'when user is not allowed to log in using password' do
+      let_it_be(:user) { create(:user, provisioned_by_group: build(:group)) }
+
+      it 'does not authenticate the user' do
+        post(:create, params: { user: { login: user.username, password: user.password } })
+
+        expect(@request.env['warden']).not_to be_authenticated
+        expect(flash[:alert]).to include('You are not allowed to log in using password')
+      end
+    end
   end
 end

ee/spec/models/ee/user_spec.rb

@@ -919,6 +919,44 @@ RSpec.describe User do
     end
   end
 
+  describe '#password_based_login_forbidden?' do
+    context 'when user is provisioned by group' do
+      before do
+        user.user_detail.provisioned_by_group = build(:group)
+      end
+
+      it 'is true' do
+        expect(user.password_based_login_forbidden?).to eq true
+      end
+
+      context 'with feature flag switched off' do
+        before do
+          stub_feature_flags(block_password_auth_for_saml_users: false)
+        end
+
+        it 'is false' do
+          expect(user.password_based_login_forbidden?).to eq false
+        end
+      end
+    end
+
+    context 'when user is not provisioned by group' do
+      it 'is false' do
+        expect(user.password_based_login_forbidden?).to eq false
+      end
+
+      context 'with feature flag switched off' do
+        before do
+          stub_feature_flags(block_password_auth_for_saml_users: false)
+        end
+
+        it 'is false' do
+          expect(user.password_based_login_forbidden?).to eq false
+        end
+      end
+    end
+  end
+
   describe '#using_license_seat?' do
     let(:user) { create(:user) }
 

ee/spec/requests/git_http_spec.rb

@@ -153,4 +153,102 @@ RSpec.describe 'Git HTTP requests' do
 
     it_behaves_like 'pulls are allowed'
   end
+
+  describe 'when user cannot use password-based login' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, :repository, :private, group: group) }
+    let_it_be(:user) { create(:user, provisioned_by_group: group) }
+
+    let(:env) { { user: user.username, password: user.password } }
+    let(:path) { "#{project.full_path}.git" }
+
+    before do
+      project.add_developer(user)
+    end
+
+    context 'with feature flag switched off' do
+      before do
+        stub_feature_flags(block_password_auth_for_saml_users: false)
+      end
+
+      it_behaves_like 'pulls are allowed'
+      it_behaves_like 'pushes are allowed'
+    end
+
+    context 'with feature flag switched on' do
+      it 'responds with status 401 Unauthorized for pull action' do
+        download(path, **env) do |response|
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
+      end
+
+      it 'responds with status 401 Unauthorized for push action' do
+        upload(path, **env) do |response|
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
+      end
+
+      context 'when username and personal access token are provided' do
+        let(:user) { create(:user, provisioned_by_group: group) }
+        let(:access_token) { create(:personal_access_token, user: user) }
+        let(:env) { { user: user.username, password: access_token.token } }
+
+        it_behaves_like 'pulls are allowed'
+        it_behaves_like 'pushes are allowed'
+      end
+
+      context 'when user has 2FA enabled' do
+        let(:user) { create(:user, :two_factor, provisioned_by_group: group) }
+        let(:access_token) { create(:personal_access_token, user: user) }
+
+        context 'when username and personal access token are provided' do
+          let(:env) { { user: user.username, password: access_token.token } }
+
+          it_behaves_like 'pulls are allowed'
+          it_behaves_like 'pushes are allowed'
+
+          it 'rejects the push attempt for read_repository scope' do
+            read_access_token = create(:personal_access_token, user: user, scopes: [:read_repository])
+
+            upload(path, user: user.username, password: read_access_token.token) do |response|
+              expect(response).to have_gitlab_http_status(:forbidden)
+              expect(response.body).to include('You are not allowed to upload code')
+            end
+          end
+
+          it 'accepts the push attempt for write_repository scope' do
+            write_access_token = create(:personal_access_token, user: user, scopes: [:write_repository])
+
+            upload(path, user: user.username, password: write_access_token.token) do |response|
+              expect(response).to have_gitlab_http_status(:ok)
+            end
+          end
+
+          it 'accepts the pull attempt for read_repository scope' do
+            read_access_token = create(:personal_access_token, user: user, scopes: [:read_repository])
+
+            download(path, user: user.username, password: read_access_token.token) do |response|
+              expect(response).to have_gitlab_http_status(:ok)
+            end
+          end
+
+          it 'accepts the pull attempt for api scope' do
+            read_access_token = create(:personal_access_token, user: user, scopes: [:api])
+
+            download(path, user: user.username, password: read_access_token.token) do |response|
+              expect(response).to have_gitlab_http_status(:ok)
+            end
+          end
+
+          it 'accepts the push attempt for api scope' do
+            write_access_token = create(:personal_access_token, user: user, scopes: [:api])
+
+            upload(path, user: user.username, password: write_access_token.token) do |response|
+              expect(response).to have_gitlab_http_status(:ok)
+            end
+          end
+        end
+      end
+    end
+  end
 end

lib/gitlab/auth/database/authentication.rb

@@ -9,6 +9,7 @@ module Gitlab
       class Authentication < Gitlab::Auth::OAuth::Authentication
         def login(login, password)
           return false unless Gitlab::CurrentSettings.password_authentication_enabled_for_git?
+          return false if user.password_based_login_forbidden?
 
           return user if user&.valid_password?(password)
         end

locale/gitlab.pot

@@ -36988,6 +36988,9 @@ msgstr ""
 msgid "You are not allowed to approve a user"
 msgstr ""
 
+msgid "You are not allowed to log in using password"
+msgstr ""
+
 msgid "You are not allowed to push into this branch. Create another branch or open a merge request."
 msgstr ""