Allow password authentication to be disabled entirely

17 files changed, 127 insertions, 50 deletions

diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 768c7e99c96..fe95d1ef9cd 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -41,14 +41,13 @@ describe ApplicationController do
       controller.send(:check_password_expiration)
     end
 
-    it 'redirects if the user is over their password expiry and sign-in is disabled' do
-      stub_application_setting(password_authentication_enabled: false)
+    it 'does not redirect if the user is over their password expiry but password authentication is disabled for the web interface' do
+      stub_application_setting(password_authentication_enabled_for_web: false)
+      stub_application_setting(password_authentication_enabled_for_git: 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).to receive(:redirect_to)
-      expect(controller).to receive(:new_profile_password_path)
+      expect(controller).not_to receive(:redirect_to)
 
       controller.send(:check_password_expiration)
     end
diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb
index 8778bff1190..4d31cfedbd2 100644
--- a/spec/controllers/passwords_controller_spec.rb
+++ b/spec/controllers/passwords_controller_spec.rb
@@ -1,18 +1,20 @@
 require 'spec_helper'
 
 describe PasswordsController do
-  describe '#prevent_ldap_reset' do
+  describe '#check_password_authentication_available' do
     before do
       @request.env["devise.mapping"] = Devise.mappings[:user]
     end
 
-    context 'when password authentication is disabled' do
-      it 'allows password reset' do
-        stub_application_setting(password_authentication_enabled: false)
+    context 'when password authentication is disabled for the web interface and Git' do
+      it 'prevents a password reset' do
+        stub_application_setting(password_authentication_enabled_for_web: false)
+        stub_application_setting(password_authentication_enabled_for_git: false)
 
         post :create
 
         expect(response).to have_gitlab_http_status(302)
+        expect(flash[:alert]).to eq 'Password authentication is unavailable.'
       end
     end
 
@@ -22,7 +24,7 @@ describe PasswordsController do
       it 'prevents a password reset' do
         post :create, user: { email: user.email }
 
-        expect(flash[:alert]).to eq('Cannot reset password for LDAP user.')
+        expect(flash[:alert]).to eq 'Password authentication is unavailable.'
       end
     end
   end
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 1d3ddfbd220..346944fd5b0 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -118,7 +118,8 @@ describe RegistrationsController do
 
     context 'user does not require password confirmation' do
       before do
-        stub_application_setting(password_authentication_enabled: false)
+        stub_application_setting(password_authentication_enabled_for_web: false)
+        stub_application_setting(password_authentication_enabled_for_git: false)
       end
 
       it 'fails if username confirmation is not provided' do
diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb
index fb4355074df..4665626f114 100644
--- a/spec/features/profiles/password_spec.rb
+++ b/spec/features/profiles/password_spec.rb
@@ -53,12 +53,13 @@ describe 'Profile > Password' do
     context 'Regular user' do
       let(:user) { create(:user) }
 
-      it 'renders 200 when sign-in is disabled' do
-        stub_application_setting(password_authentication_enabled: false)
+      it 'renders 404 when password authentication is disabled for the web interface and Git' do
+        stub_application_setting(password_authentication_enabled_for_web: false)
+        stub_application_setting(password_authentication_enabled_for_git: false)
 
         visit edit_profile_password_path
 
-        expect(page).to have_gitlab_http_status(200)
+        expect(page).to have_gitlab_http_status(404)
       end
     end
 
diff --git a/spec/features/projects/no_password_spec.rb b/spec/features/projects/no_password_spec.rb
index 6aff079dd39..b3b3212556c 100644
--- a/spec/features/projects/no_password_spec.rb
+++ b/spec/features/projects/no_password_spec.rb
@@ -30,7 +30,7 @@ feature 'No Password Alert' do
     let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml') }
 
     before do
-      stub_application_setting(password_authentication_enabled?: false)
+      stub_application_setting(password_authentication_enabled_for_git?: false)
       stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config])
     end
 
diff --git a/spec/helpers/button_helper_spec.rb b/spec/helpers/button_helper_spec.rb
index 4423560ecaa..e5158761333 100644
--- a/spec/helpers/button_helper_spec.rb
+++ b/spec/helpers/button_helper_spec.rb
@@ -35,7 +35,7 @@ describe ButtonHelper do
 
     context 'with internal auth disabled' do
       before do
-        stub_application_setting(password_authentication_enabled?: false)
+        stub_application_setting(password_authentication_enabled_for_git?: false)
       end
 
       context 'when user has no personal access tokens' do
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 5777b5c4025..ede9d232efd 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -150,17 +150,26 @@ describe ProjectsHelper do
       end
     end
 
-    context 'user requires a password' do
-      let(:user) { create(:user, password_automatically_set: true) }
+    context 'user has hidden the message' do
+      it 'returns false' do
+        allow(helper).to receive(:cookies).and_return(hide_no_password_message: true)
+
+        expect(helper.show_no_password_message?).to be_falsey
+      end
+    end
 
+    context 'user requires a password for Git' do
       it 'returns true' do
+        allow(user).to receive(:require_password_creation_for_git?).and_return(true)
+
         expect(helper.show_no_password_message?).to be_truthy
       end
     end
 
-    context 'user requires a personal access token' do
+    context 'user requires a personal access token for Git' do
       it 'returns true' do
-        stub_application_setting(password_authentication_enabled?: false)
+        allow(user).to receive(:require_password_creation_for_git?).and_return(false)
+        allow(user).to receive(:require_personal_access_token_creation_for_git_auth?).and_return(true)
 
         expect(helper.show_no_password_message?).to be_truthy
       end
@@ -168,23 +177,23 @@ describe ProjectsHelper do
   end
 
   describe '#link_to_set_password' do
+    let(:user) { create(:user, password_automatically_set: true) }
+
     before do
       allow(helper).to receive(:current_user).and_return(user)
     end
 
-    context 'user requires a password' do
-      let(:user) { create(:user, password_automatically_set: true) }
-
+    context 'password authentication is enabled for Git' do
       it 'returns link to set a password' do
+        stub_application_setting(password_authentication_enabled_for_git?: true)
+
         expect(helper.link_to_set_password).to match %r{<a href="#{edit_profile_password_path}">set a password</a>}
       end
     end
 
-    context 'user requires a personal access token' do
-      let(:user) { create(:user) }
-
+    context 'password authentication is disabled for Git' do
       it 'returns link to create a personal access token' do
-        stub_application_setting(password_authentication_enabled?: false)
+        stub_application_setting(password_authentication_enabled_for_git?: false)
 
         expect(helper.link_to_set_password).to match %r{<a href="#{profile_personal_access_tokens_path}">create a personal access token</a>}
       end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 3164d2ebf04..5e822a0026a 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -251,7 +251,7 @@ describe Gitlab::Auth do
     end
 
     it 'throws an error suggesting user create a PAT when internal auth is disabled' do
-      allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false }
+      allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false }
 
       expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::MissingPersonalAccessTokenError)
     end
@@ -324,6 +324,26 @@ describe Gitlab::Auth do
         gl_auth.find_with_user_password('ldap_user', 'password')
       end
     end
+
+    context "with password authentication disabled for Git" do
+      before do
+        stub_application_setting(password_authentication_enabled_for_git: 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
+
+      context "with ldap enabled" do
+        before do
+          allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
+        end
+
+        it "does not find non-ldap user by valid login/password" do
+          expect(gl_auth.find_with_user_password(username, password)).to be_nil
+        end
+      end
+    end
   end
 
   private
diff --git a/spec/lib/gitlab/fake_application_settings_spec.rb b/spec/lib/gitlab/fake_application_settings_spec.rb
index 34322c2a693..af12e13d36d 100644
--- a/spec/lib/gitlab/fake_application_settings_spec.rb
+++ b/spec/lib/gitlab/fake_application_settings_spec.rb
@@ -1,25 +1,25 @@
 require 'spec_helper'
 
 describe Gitlab::FakeApplicationSettings do
-  let(:defaults) { { password_authentication_enabled: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } }
+  let(:defaults) { { password_authentication_enabled_for_web: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } }
 
   subject { described_class.new(defaults) }
 
   it 'wraps OpenStruct variables properly' do
-    expect(subject.password_authentication_enabled).to be_falsey
+    expect(subject.password_authentication_enabled_for_web).to be_falsey
     expect(subject.signup_enabled).to be_truthy
     expect(subject.foobar).to eq('asdf')
   end
 
   it 'defines predicate methods' do
-    expect(subject.password_authentication_enabled?).to be_falsey
+    expect(subject.password_authentication_enabled_for_web?).to be_falsey
     expect(subject.signup_enabled?).to be_truthy
   end
 
   it 'predicate method changes when value is updated' do
-    subject.password_authentication_enabled = true
+    subject.password_authentication_enabled_for_web = true
 
-    expect(subject.password_authentication_enabled?).to be_truthy
+    expect(subject.password_authentication_enabled_for_web?).to be_truthy
   end
 
   it 'does not define a predicate method' do
diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb
index a4c1113ae37..b5f2a15ada3 100644
--- a/spec/lib/gitlab/usage_data_spec.rb
+++ b/spec/lib/gitlab/usage_data_spec.rb
@@ -103,7 +103,7 @@ describe Gitlab::UsageData do
     subject { described_class.features_usage_data_ce }
 
     it 'gathers feature usage data' do
-      expect(subject[:signup]).to eq(current_application_settings.signup_enabled?)
+      expect(subject[:signup]).to eq(current_application_settings.allow_signup?)
       expect(subject[:ldap]).to eq(Gitlab.config.ldap.enabled)
       expect(subject[:gravatar]).to eq(current_application_settings.gravatar_enabled?)
       expect(subject[:omniauth]).to eq(Gitlab.config.omniauth.enabled)
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 47b7150d36f..51bf4e65e5d 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -564,4 +564,22 @@ describe ApplicationSetting do
       expect(setting.key_restriction_for(:foo)).to eq(described_class::FORBIDDEN_KEY_VALUE)
     end
   end
+
+  describe '#allow_signup?' do
+    it 'returns true' do
+      expect(setting.allow_signup?).to be_truthy
+    end
+
+    it 'returns false if signup is disabled' do
+      allow(setting).to receive(:signup_enabled?).and_return(false)
+
+      expect(setting.allow_signup?).to be_falsey
+    end
+
+    it 'returns false if password authentication is disabled for the web interface' do
+      allow(setting).to receive(:password_authentication_enabled_for_web?).and_return(false)
+
+      expect(setting.allow_signup?).to be_falsey
+    end
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 86647ddf6ce..b27c1b2cd1a 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2143,25 +2143,47 @@ describe User do
     end
   end
 
-  describe '#allow_password_authentication?' do
+  describe '#allow_password_authentication_for_web?' do
     context 'regular user' do
       let(:user) { build(:user) }
 
-      it 'returns true when sign-in is enabled' do
-        expect(user.allow_password_authentication?).to be_truthy
+      it 'returns true when password authentication is enabled for the web interface' do
+        expect(user.allow_password_authentication_for_web?).to be_truthy
       end
 
-      it 'returns false when sign-in is disabled' do
-        stub_application_setting(password_authentication_enabled: false)
+      it 'returns false when password authentication is disabled for the web interface' do
+        stub_application_setting(password_authentication_enabled_for_web: false)
 
-        expect(user.allow_password_authentication?).to be_falsey
+        expect(user.allow_password_authentication_for_web?).to be_falsey
       end
     end
 
     it 'returns false for ldap user' do
       user = create(:omniauth_user, provider: 'ldapmain')
 
-      expect(user.allow_password_authentication?).to be_falsey
+      expect(user.allow_password_authentication_for_web?).to be_falsey
+    end
+  end
+
+  describe '#allow_password_authentication_for_git?' do
+    context 'regular user' do
+      let(:user) { build(:user) }
+
+      it 'returns true when password authentication is enabled for Git' do
+        expect(user.allow_password_authentication_for_git?).to be_truthy
+      end
+
+      it 'returns false when password authentication is disabled Git' do
+        stub_application_setting(password_authentication_enabled_for_git: false)
+
+        expect(user.allow_password_authentication_for_git?).to be_falsey
+      end
+    end
+
+    it 'returns false for ldap user' do
+      user = create(:omniauth_user, provider: 'ldapmain')
+
+      expect(user.allow_password_authentication_for_git?).to be_falsey
     end
   end
 
@@ -2381,7 +2403,8 @@ describe User do
       let(:expected) { !(password_automatically_set || ldap_user || password_authentication_disabled) }
 
       before do
-        stub_application_setting(password_authentication_enabled: !password_authentication_disabled)
+        stub_application_setting(password_authentication_enabled_for_web: !password_authentication_disabled)
+        stub_application_setting(password_authentication_enabled_for_git: !password_authentication_disabled)
       end
 
       it 'returns false unless all inputs are true' do
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 5d3e78dd7c8..63175c40a18 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -10,7 +10,7 @@ describe API::Settings, 'Settings' do
       expect(response).to have_gitlab_http_status(200)
       expect(json_response).to be_an Hash
       expect(json_response['default_projects_limit']).to eq(42)
-      expect(json_response['password_authentication_enabled']).to be_truthy
+      expect(json_response['password_authentication_enabled_for_web']).to be_truthy
       expect(json_response['repository_storages']).to eq(['default'])
       expect(json_response['koding_enabled']).to be_falsey
       expect(json_response['koding_url']).to be_nil
@@ -37,7 +37,7 @@ describe API::Settings, 'Settings' do
       it "updates application settings" do
         put api("/application/settings", admin),
           default_projects_limit: 3,
-          password_authentication_enabled: false,
+          password_authentication_enabled_for_web: false,
           repository_storages: ['custom'],
           koding_enabled: true,
           koding_url: 'http://koding.example.com',
@@ -58,7 +58,7 @@ describe API::Settings, 'Settings' do
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
-        expect(json_response['password_authentication_enabled']).to be_falsey
+        expect(json_response['password_authentication_enabled_for_web']).to be_falsey
         expect(json_response['repository_storages']).to eq(['custom'])
         expect(json_response['koding_enabled']).to be_truthy
         expect(json_response['koding_url']).to eq('http://koding.example.com')
diff --git a/spec/requests/api/v3/settings_spec.rb b/spec/requests/api/v3/settings_spec.rb
index 25fa0a8aabd..985bfbfa09c 100644
--- a/spec/requests/api/v3/settings_spec.rb
+++ b/spec/requests/api/v3/settings_spec.rb
@@ -28,11 +28,11 @@ describe API::V3::Settings, 'Settings' do
 
       it "updates application settings" do
         put v3_api("/application/settings", admin),
-          default_projects_limit: 3, password_authentication_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com',
+          default_projects_limit: 3, password_authentication_enabled_for_web: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com',
           plantuml_enabled: true, plantuml_url: 'http://plantuml.example.com'
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
-        expect(json_response['password_authentication_enabled']).to be_falsey
+        expect(json_response['password_authentication_enabled_for_web']).to be_falsey
         expect(json_response['repository_storage']).to eq('custom')
         expect(json_response['repository_storages']).to eq(['custom'])
         expect(json_response['koding_enabled']).to be_truthy
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index cd52194033a..a16f98bec36 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -463,7 +463,7 @@ describe 'Git HTTP requests' do
 
               context 'when internal auth is disabled' do
                 before do
-                  allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false }
+                  allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false }
                 end
 
                 it 'rejects pulls with personal access token error message' do
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index 94e04ce5608..6f40a02aaa9 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -105,7 +105,7 @@ describe JwtController do
 
       context 'when internal auth is disabled' do
         it 'rejects the authorization attempt with personal access token message' do
-          allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false }
+          allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false }
           get '/jwt/auth', parameters, headers
 
           expect(response).to have_gitlab_http_status(401)
diff --git a/spec/support/matchers/have_gitlab_http_status.rb b/spec/support/matchers/have_gitlab_http_status.rb
index 3198f1b9edd..e7e418cdde4 100644
--- a/spec/support/matchers/have_gitlab_http_status.rb
+++ b/spec/support/matchers/have_gitlab_http_status.rb
@@ -8,7 +8,11 @@ RSpec::Matchers.define :have_gitlab_http_status do |expected|
   end
 
   failure_message do |actual|
+    # actual can be either an ActionDispatch::TestResponse (which uses #response_code)
+    # or a Capybara::Session (which uses #status_code)
+    response_code = actual.try(:response_code) || actual.try(:status_code)
+
     "expected the response to have status code #{expected.inspect}" \
-    " but it was #{actual.response_code}. The response was: #{actual.body}"
+    " but it was #{response_code}. The response was: #{actual.body}"
   end
 end