Commit 7d3e888d authored by Douwe Maan's avatar Douwe Maan

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

Rollback changes made to signing_enabled.

Closes #37202

See merge request !13956
parents 538852fe 37383d9a
......@@ -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
......
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
......
......@@ -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
......
......@@ -603,7 +603,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
......
......@@ -153,7 +153,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'
......
......@@ -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
......
---
title: Reverts changes made to signin_enabled.
merge_request: 13956
author:
type: fixed
......@@ -50,10 +50,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)
......
......@@ -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
......
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
......
......@@ -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
......
......@@ -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
......
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