Commit 37383d9a authored by Tiago Botelho's avatar Tiago Botelho

Rollsback changes made to signing_enabled.

parent d546f7d3
...@@ -202,7 +202,7 @@ class ApplicationController < ActionController::Base ...@@ -202,7 +202,7 @@ class ApplicationController < ActionController::Base
end end
def check_password_expiration 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 return redirect_to new_profile_password_path
end end
end end
......
class PasswordsController < Devise::PasswordsController class PasswordsController < Devise::PasswordsController
include Gitlab::CurrentSettings
before_action :resource_from_email, only: [:create] 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] before_action :throttle_reset, only: [:create]
def edit def edit
...@@ -40,11 +38,11 @@ class PasswordsController < Devise::PasswordsController ...@@ -40,11 +38,11 @@ class PasswordsController < Devise::PasswordsController
self.resource = resource_class.find_by_email(email) self.resource = resource_class.find_by_email(email)
end end
def check_password_authentication_available def prevent_ldap_reset
return if current_application_settings.password_authentication_enabled? && (resource.nil? || resource.allow_password_authentication?) return unless resource&.ldap_user?
redirect_to after_sending_reset_password_instructions_path_for(resource_name), redirect_to after_sending_reset_password_instructions_path_for(resource_name),
alert: "Password authentication is unavailable." alert: "Cannot reset password for LDAP user."
end end
def throttle_reset def throttle_reset
......
...@@ -77,7 +77,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController ...@@ -77,7 +77,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
end end
def authorize_change_password! def authorize_change_password!
render_404 unless @user.allow_password_authentication? render_404 if @user.ldap_user?
end end
def user_params def user_params
......
...@@ -601,7 +601,7 @@ class User < ActiveRecord::Base ...@@ -601,7 +601,7 @@ class User < ActiveRecord::Base
end end
def require_personal_access_token_creation_for_git_auth? 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? PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none?
end end
......
...@@ -153,7 +153,7 @@ ...@@ -153,7 +153,7 @@
.checkbox .checkbox
= f.label :password_authentication_enabled do = f.label :password_authentication_enabled do
= f.check_box :password_authentication_enabled = f.check_box :password_authentication_enabled
Password authentication enabled Sign-in enabled
- if omniauth_enabled? && button_based_providers.any? - if omniauth_enabled? && button_based_providers.any?
.form-group .form-group
= f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2' = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2'
......
...@@ -29,7 +29,7 @@ ...@@ -29,7 +29,7 @@
= link_to profile_emails_path, title: 'Emails' do = link_to profile_emails_path, title: 'Emails' do
%span %span
Emails Emails
- if current_user.allow_password_authentication? - unless current_user.ldap_user?
= nav_link(controller: :passwords) do = nav_link(controller: :passwords) do
= link_to edit_profile_password_path, title: 'Password' do = link_to edit_profile_password_path, title: 'Password' do
%span %span
......
---
title: Reverts changes made to signin_enabled.
merge_request: 13956
author:
type: fixed
...@@ -48,10 +48,6 @@ module Gitlab ...@@ -48,10 +48,6 @@ module Gitlab
# Avoid resource intensive login checks if password is not provided # Avoid resource intensive login checks if password is not provided
return unless password.present? 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 Gitlab::Auth::UniqueIpsLimiter.limit_user! do
user = User.by_login(login) user = User.by_login(login)
......
...@@ -8,34 +8,43 @@ describe ApplicationController do ...@@ -8,34 +8,43 @@ describe ApplicationController do
it 'redirects if the user is over their password expiry' do it 'redirects if the user is over their password expiry' do
user.password_expires_at = Time.new(2002) user.password_expires_at = Time.new(2002)
expect(user.ldap_user?).to be_falsey expect(user.ldap_user?).to be_falsey
allow(controller).to receive(:current_user).and_return(user) allow(controller).to receive(:current_user).and_return(user)
expect(controller).to receive(:redirect_to) expect(controller).to receive(:redirect_to)
expect(controller).to receive(:new_profile_password_path) expect(controller).to receive(:new_profile_password_path)
controller.send(:check_password_expiration) controller.send(:check_password_expiration)
end end
it 'does not redirect if the user is under their password expiry' do it 'does not redirect if the user is under their password expiry' do
user.password_expires_at = Time.now + 20010101 user.password_expires_at = Time.now + 20010101
expect(user.ldap_user?).to be_falsey expect(user.ldap_user?).to be_falsey
allow(controller).to receive(:current_user).and_return(user) allow(controller).to receive(:current_user).and_return(user)
expect(controller).not_to receive(:redirect_to) expect(controller).not_to receive(:redirect_to)
controller.send(:check_password_expiration) controller.send(:check_password_expiration)
end end
it 'does not redirect if the user is over their password expiry but they are an ldap user' do 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) user.password_expires_at = Time.new(2002)
allow(user).to receive(:ldap_user?).and_return(true) allow(user).to receive(:ldap_user?).and_return(true)
allow(controller).to receive(:current_user).and_return(user) allow(controller).to receive(:current_user).and_return(user)
expect(controller).not_to receive(:redirect_to) expect(controller).not_to receive(:redirect_to)
controller.send(:check_password_expiration) controller.send(:check_password_expiration)
end 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) stub_application_setting(password_authentication_enabled: false)
user.password_expires_at = Time.new(2002) user.password_expires_at = Time.new(2002)
expect(user.ldap_user?).to be_falsey
allow(controller).to receive(:current_user).and_return(user) 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) controller.send(:check_password_expiration)
end end
......
require 'spec_helper' require 'spec_helper'
describe PasswordsController do describe PasswordsController do
describe '#check_password_authentication_available' do describe '#prevent_ldap_reset' do
before do before do
@request.env["devise.mapping"] = Devise.mappings[:user] @request.env["devise.mapping"] = Devise.mappings[:user]
end end
context 'when password authentication is disabled' do 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) stub_application_setting(password_authentication_enabled: false)
post :create post :create
expect(flash[:alert]).to eq 'Password authentication is unavailable.' expect(response).to have_http_status(302)
end end
end end
...@@ -22,7 +22,7 @@ describe PasswordsController do ...@@ -22,7 +22,7 @@ describe PasswordsController do
it 'prevents a password reset' do it 'prevents a password reset' do
post :create, user: { email: user.email } 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 end
end end
......
...@@ -53,12 +53,12 @@ describe 'Profile > Password' do ...@@ -53,12 +53,12 @@ describe 'Profile > Password' do
context 'Regular user' do context 'Regular user' do
let(:user) { create(:user) } 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) stub_application_setting(password_authentication_enabled: false)
visit edit_profile_password_path visit edit_profile_password_path
expect(page).to have_http_status(404) expect(page).to have_http_status(200)
end end
end end
......
...@@ -279,16 +279,6 @@ describe Gitlab::Auth do ...@@ -279,16 +279,6 @@ describe Gitlab::Auth do
gl_auth.find_with_user_password('ldap_user', 'password') gl_auth.find_with_user_password('ldap_user', 'password')
end end
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 end
private 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