summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/application_controller.rb2
-rw-r--r--app/controllers/passwords_controller.rb10
-rw-r--r--app/controllers/profiles/passwords_controller.rb2
-rw-r--r--app/models/user.rb2
-rw-r--r--app/views/admin/application_settings/_form.html.haml2
-rw-r--r--app/views/layouts/nav/_profile.html.haml2
-rw-r--r--changelogs/unreleased/37202-revert-changes-to-signing-enabled.yml5
-rw-r--r--lib/gitlab/auth.rb4
-rw-r--r--spec/controllers/application_controller_spec.rb13
-rw-r--r--spec/controllers/passwords_controller_spec.rb8
-rw-r--r--spec/features/profiles/password_spec.rb4
-rw-r--r--spec/lib/gitlab/auth_spec.rb10
12 files changed, 31 insertions, 33 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 1d92ea11bda..97922e39ba8 100644
--- a/app/controllers/application_controller.rb
+++ b/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
diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index aa8cf630032..fda944adecd 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -1,8 +1,6 @@
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
diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb
index c423761ab24..7beb52dd8e8 100644
--- a/app/controllers/profiles/passwords_controller.rb
+++ b/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
diff --git a/app/models/user.rb b/app/models/user.rb
index fbd08bc4d0a..8dab3475d54 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -601,7 +601,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
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 959af5c0d13..8c2a0975a49 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -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'
diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml
index 26d9640e98a..448f6abedf2 100644
--- a/app/views/layouts/nav/_profile.html.haml
+++ b/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
diff --git a/changelogs/unreleased/37202-revert-changes-to-signing-enabled.yml b/changelogs/unreleased/37202-revert-changes-to-signing-enabled.yml
new file mode 100644
index 00000000000..ddbe79cb498
--- /dev/null
+++ b/changelogs/unreleased/37202-revert-changes-to-signing-enabled.yml
@@ -0,0 +1,5 @@
+---
+title: Reverts changes made to signin_enabled.
+merge_request: 13956
+author:
+type: fixed
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 8cb4060cd97..e6e06570f13 100644
--- a/lib/gitlab/auth.rb
+++ b/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)
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 331903a5543..59a6cfbf4f5 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/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
diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb
index 2955d01fad0..cdaa88bbf5d 100644
--- a/spec/controllers/passwords_controller_spec.rb
+++ b/spec/controllers/passwords_controller_spec.rb
@@ -1,18 +1,18 @@
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
diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb
index 2c757f99a27..225d4c16841 100644
--- a/spec/features/profiles/password_spec.rb
+++ b/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
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 4a498e79c87..f685bb83d0d 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/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
View Lorry Controller Status