Merge branch '37202-revert-changes-to-signing-enabled' into 'master'

Rollback changes made to signing_enabled.

Closes #37202

See merge request !13956

app/controllers/application_controller.rb

@@ -202,7 +202,7 @@ class ApplicationController < ActionController::Base
   end
 
   def check_password_expiration
-    if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && current_user.allow_password_authentication?
+    if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && !current_user.ldap_user?
       return redirect_to new_profile_password_path
     end
   end

app/controllers/passwords_controller.rb

 class PasswordsController < Devise::PasswordsController
-  include Gitlab::CurrentSettings
-
   before_action :resource_from_email, only: [:create]
-  before_action :check_password_authentication_available, only: [:create]
+  before_action :prevent_ldap_reset, only: [:create]
   before_action :throttle_reset,      only: [:create]
 
   def edit
@@ -40,11 +38,11 @@ class PasswordsController < Devise::PasswordsController
     self.resource = resource_class.find_by_email(email)
   end
 
-  def check_password_authentication_available
-    return if current_application_settings.password_authentication_enabled? && (resource.nil? || resource.allow_password_authentication?)
+  def prevent_ldap_reset
+    return unless resource&.ldap_user?
 
     redirect_to after_sending_reset_password_instructions_path_for(resource_name),
-      alert: "Password authentication is unavailable."
+      alert: "Cannot reset password for LDAP user."
   end
 
   def throttle_reset

app/controllers/profiles/passwords_controller.rb

@@ -77,7 +77,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
   end
 
   def authorize_change_password!
-    render_404 unless @user.allow_password_authentication?
+    render_404 if @user.ldap_user?
   end
 
   def user_params

app/models/user.rb

@@ -598,7 +598,7 @@ class User < ActiveRecord::Base
   end
 
   def require_personal_access_token_creation_for_git_auth?
-    return false if allow_password_authentication? || ldap_user?
+    return false if current_application_settings.password_authentication_enabled? || ldap_user?
 
     PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none?
   end

app/views/admin/application_settings/_form.html.haml

@@ -147,7 +147,7 @@
         .checkbox
           = f.label :password_authentication_enabled do
             = f.check_box :password_authentication_enabled
-            Password authentication enabled
+            Sign-in enabled
     - if omniauth_enabled? && button_based_providers.any?
       .form-group
         = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2'

app/views/layouts/nav/_profile.html.haml

@@ -29,7 +29,7 @@
       = link_to profile_emails_path, title: 'Emails' do
         %span
           Emails
-    - if current_user.allow_password_authentication?
+    - unless current_user.ldap_user?
       = nav_link(controller: :passwords) do
         = link_to edit_profile_password_path, title: 'Password' do
           %span

changelogs/unreleased/37202-revert-changes-to-signing-enabled.yml

+---
+title: Reverts changes made to signin_enabled.
+merge_request: 13956
+author:
+type: fixed

lib/gitlab/auth.rb

@@ -48,10 +48,6 @@ module Gitlab
         # Avoid resource intensive login checks if password is not provided
         return unless password.present?
 
-        # Nothing to do here if internal auth is disabled and LDAP is
-        # not configured
-        return unless current_application_settings.password_authentication_enabled? || Gitlab::LDAP::Config.enabled?
-
         Gitlab::Auth::UniqueIpsLimiter.limit_user! do
           user = User.by_login(login)
 

spec/controllers/application_controller_spec.rb

@@ -8,34 +8,43 @@ describe ApplicationController do
 
     it 'redirects if the user is over their password expiry' do
       user.password_expires_at = Time.new(2002)
+
       expect(user.ldap_user?).to be_falsey
       allow(controller).to receive(:current_user).and_return(user)
       expect(controller).to receive(:redirect_to)
       expect(controller).to receive(:new_profile_password_path)
+
       controller.send(:check_password_expiration)
     end
 
     it 'does not redirect if the user is under their password expiry' do
       user.password_expires_at = Time.now + 20010101
+
       expect(user.ldap_user?).to be_falsey
       allow(controller).to receive(:current_user).and_return(user)
       expect(controller).not_to receive(:redirect_to)
+
       controller.send(:check_password_expiration)
     end
 
     it 'does not redirect if the user is over their password expiry but they are an ldap user' do
       user.password_expires_at = Time.new(2002)
+
       allow(user).to receive(:ldap_user?).and_return(true)
       allow(controller).to receive(:current_user).and_return(user)
       expect(controller).not_to receive(:redirect_to)
+
       controller.send(:check_password_expiration)
     end
 
-    it 'does not redirect if the user is over their password expiry but sign-in is disabled' do
+    it 'redirects if the user is over their password expiry and sign-in is disabled' do
       stub_application_setting(password_authentication_enabled: false)
       user.password_expires_at = Time.new(2002)
+
+      expect(user.ldap_user?).to be_falsey
       allow(controller).to receive(:current_user).and_return(user)
-      expect(controller).not_to receive(:redirect_to)
+      expect(controller).to receive(:redirect_to)
+      expect(controller).to receive(:new_profile_password_path)
 
       controller.send(:check_password_expiration)
     end

spec/controllers/passwords_controller_spec.rb

 require 'spec_helper'
 
 describe PasswordsController do
-  describe '#check_password_authentication_available' do
+  describe '#prevent_ldap_reset' do
     before do
       @request.env["devise.mapping"] = Devise.mappings[:user]
     end
 
     context 'when password authentication is disabled' do
-      it 'prevents a password reset' do
+      it 'allows password reset' do
         stub_application_setting(password_authentication_enabled: false)
 
         post :create
 
-        expect(flash[:alert]).to eq 'Password authentication is unavailable.'
+        expect(response).to have_http_status(302)
       end
     end
 
@@ -22,7 +22,7 @@ describe PasswordsController do
       it 'prevents a password reset' do
         post :create, user: { email: user.email }
 
-        expect(flash[:alert]).to eq 'Password authentication is unavailable.'
+        expect(flash[:alert]).to eq('Cannot reset password for LDAP user.')
       end
     end
   end

spec/features/profiles/password_spec.rb

@@ -53,12 +53,12 @@ describe 'Profile > Password' do
     context 'Regular user' do
       let(:user) { create(:user) }
 
-      it 'renders 404 when sign-in is disabled' do
+      it 'renders 200 when sign-in is disabled' do
         stub_application_setting(password_authentication_enabled: false)
 
         visit edit_profile_password_path
 
-        expect(page).to have_http_status(404)
+        expect(page).to have_http_status(200)
       end
     end
 

spec/lib/gitlab/auth_spec.rb

@@ -279,16 +279,6 @@ describe Gitlab::Auth do
         gl_auth.find_with_user_password('ldap_user', 'password')
       end
     end
-
-    context "with sign-in disabled" do
-      before do
-        stub_application_setting(password_authentication_enabled: false)
-      end
-
-      it "does not find user by valid login/password" do
-        expect(gl_auth.find_with_user_password(username, password)).to be_nil
-      end
-    end
   end
 
   private