Fixes needed when GitLab sign-in is not enabled

When sign-in is disabled:
 - skip password expiration checks
 - prevent password reset requests
 - don’t show Password tab in User Settings
 - don’t allow login with username/password for Git over HTTP requests
 - render 404 on requests to Profiles::PasswordsController

13 files changed, 143 insertions, 43 deletions

diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index a2720c9b81e..1641bddea11 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -30,6 +30,15 @@ describe ApplicationController do
       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
+      stub_application_setting(password_authentication_enabled: false)
+      user.password_expires_at = Time.new(2002)
+      allow(controller).to receive(:current_user).and_return(user)
+      expect(controller).not_to receive(:redirect_to)
+
+      controller.send(:check_password_expiration)
+    end
   end
 
   describe "#authenticate_user_from_token!" do
diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb
new file mode 100644
index 00000000000..2955d01fad0
--- /dev/null
+++ b/spec/controllers/passwords_controller_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe PasswordsController 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 'prevents a password reset' do
+        stub_application_setting(password_authentication_enabled: false)
+
+        post :create
+
+        expect(flash[:alert]).to eq 'Password authentication is unavailable.'
+      end
+    end
+
+    context 'when reset email belongs to an ldap user' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain', email: 'ldapuser@gitlab.com') }
+
+      it 'prevents a password reset' do
+        post :create, user: { email: user.email }
+
+        expect(flash[:alert]).to eq 'Password authentication is unavailable.'
+      end
+    end
+  end
+end
diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb
index 67975a68ee2..26d6d6658aa 100644
--- a/spec/features/profiles/password_spec.rb
+++ b/spec/features/profiles/password_spec.rb
@@ -1,44 +1,74 @@
 require 'spec_helper'
 
 describe 'Profile > Password', feature: true do
-  let(:user) { create(:user, password_automatically_set: true) }
+  context 'Password authentication enabled' do
+    let(:user) { create(:user, password_automatically_set: true) }
 
-  before do
-    sign_in(user)
-    visit edit_profile_password_path
-  end
+    before do
+      sign_in(user)
+      visit edit_profile_password_path
+    end
 
-  def fill_passwords(password, confirmation)
-    fill_in 'New password',          with: password
-    fill_in 'Password confirmation', with: confirmation
+    def fill_passwords(password, confirmation)
+      fill_in 'New password',          with: password
+      fill_in 'Password confirmation', with: confirmation
 
-    click_button 'Save password'
-  end
+      click_button 'Save password'
+    end
+
+    context 'User with password automatically set' do
+      describe 'User puts different passwords in the field and in the confirmation' do
+        it 'shows an error message' do
+          fill_passwords('mypassword', 'mypassword2')
 
-  context 'User with password automatically set' do
-    describe 'User puts different passwords in the field and in the confirmation' do
-      it 'shows an error message' do
-        fill_passwords('mypassword', 'mypassword2')
+          page.within('.alert-danger') do
+            expect(page).to have_content("Password confirmation doesn't match Password")
+          end
+        end
+
+        it 'does not contain the current password field after an error' do
+          fill_passwords('mypassword', 'mypassword2')
 
-        page.within('.alert-danger') do
-          expect(page).to have_content("Password confirmation doesn't match Password")
+          expect(page).to have_no_field('user[current_password]')
         end
       end
 
-      it 'does not contain the current password field after an error' do
-        fill_passwords('mypassword', 'mypassword2')
+      describe 'User puts the same passwords in the field and in the confirmation' do
+        it 'shows a success message' do
+          fill_passwords('mypassword', 'mypassword')
 
-        expect(page).to have_no_field('user[current_password]')
+          page.within('.flash-notice') do
+            expect(page).to have_content('Password was successfully updated. Please login with it')
+          end
+        end
       end
     end
+  end
 
-    describe 'User puts the same passwords in the field and in the confirmation' do
-      it 'shows a success message' do
-        fill_passwords('mypassword', 'mypassword')
+  context 'Password authentication unavailable' do
+    before do
+      gitlab_sign_in(user)
+    end
 
-        page.within('.flash-notice') do
-          expect(page).to have_content('Password was successfully updated. Please login with it')
-        end
+    context 'Regular user' do
+      let(:user) { create(:user) }
+
+      it 'renders 404 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)
+      end
+    end
+
+    context 'LDAP user' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain') }
+
+      it 'renders 404' do
+        visit edit_profile_password_path
+
+        expect(page).to have_http_status(404)
       end
     end
   end
diff --git a/spec/features/projects/no_password_spec.rb b/spec/features/projects/no_password_spec.rb
index 53ac18fa7cc..d22a6daac08 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(signin_enabled?: false)
+      stub_application_setting(password_authentication_enabled?: 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 661327d4432..7ecb75da8ce 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(signin_enabled?: false)
+        stub_application_setting(password_authentication_enabled?: 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 487d9800707..22fe2a9cabf 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -160,7 +160,7 @@ describe ProjectsHelper do
 
     context 'user requires a personal access token' do
       it 'returns true' do
-        stub_application_setting(signin_enabled?: false)
+        stub_application_setting(password_authentication_enabled?: false)
 
         expect(helper.show_no_password_message?).to be_truthy
       end
@@ -184,7 +184,7 @@ describe ProjectsHelper do
       let(:user) { create(:user) }
 
       it 'returns link to create a personal access token' do
-        stub_application_setting(signin_enabled?: false)
+        stub_application_setting(password_authentication_enabled?: 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 d09da951869..55780518230 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -206,7 +206,7 @@ describe Gitlab::Auth, lib: true do
     end
 
     it 'throws an error suggesting user create a PAT when internal auth is disabled' do
-      allow_any_instance_of(ApplicationSetting).to receive(:signin_enabled?) { false }
+      allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false }
 
       expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::MissingPersonalTokenError)
     end
@@ -279,6 +279,16 @@ describe Gitlab::Auth, lib: true 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
diff --git a/spec/lib/gitlab/fake_application_settings_spec.rb b/spec/lib/gitlab/fake_application_settings_spec.rb
index b793176d84a..34322c2a693 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) { { signin_enabled: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } }
+  let(:defaults) { { password_authentication_enabled: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } }
 
   subject { described_class.new(defaults) }
 
   it 'wraps OpenStruct variables properly' do
-    expect(subject.signin_enabled).to be_falsey
+    expect(subject.password_authentication_enabled).to be_falsey
     expect(subject.signup_enabled).to be_truthy
     expect(subject.foobar).to eq('asdf')
   end
 
   it 'defines predicate methods' do
-    expect(subject.signin_enabled?).to be_falsey
+    expect(subject.password_authentication_enabled?).to be_falsey
     expect(subject.signup_enabled?).to be_truthy
   end
 
   it 'predicate method changes when value is updated' do
-    subject.signin_enabled = true
+    subject.password_authentication_enabled = true
 
-    expect(subject.signin_enabled?).to be_truthy
+    expect(subject.password_authentication_enabled?).to be_truthy
   end
 
   it 'does not define a predicate method' do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 448555d2190..927a6d301da 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1920,4 +1920,26 @@ describe User, models: true do
       user.invalidate_merge_request_cache_counts
     end
   end
+
+  describe '#allow_password_authentication?' 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
+      end
+
+      it 'returns false when sign-in is disabled' do
+        stub_application_setting(password_authentication_enabled: false)
+
+        expect(user.allow_password_authentication?).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
+    end
+  end
 end
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index ede48b1c888..b71ac6c30b5 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_http_status(200)
       expect(json_response).to be_an Hash
       expect(json_response['default_projects_limit']).to eq(42)
-      expect(json_response['signin_enabled']).to be_truthy
+      expect(json_response['password_authentication_enabled']).to be_truthy
       expect(json_response['repository_storage']).to eq('default')
       expect(json_response['koding_enabled']).to be_falsey
       expect(json_response['koding_url']).to be_nil
@@ -32,7 +32,7 @@ describe API::Settings, 'Settings' do
       it "updates application settings" do
         put api("/application/settings", admin),
           default_projects_limit: 3,
-          signin_enabled: false,
+          password_authentication_enabled: false,
           repository_storage: 'custom',
           koding_enabled: true,
           koding_url: 'http://koding.example.com',
@@ -46,7 +46,7 @@ describe API::Settings, 'Settings' do
           help_page_support_url: 'http://example.com/help'
         expect(response).to have_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
-        expect(json_response['signin_enabled']).to be_falsey
+        expect(json_response['password_authentication_enabled']).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/api/v3/settings_spec.rb b/spec/requests/api/v3/settings_spec.rb
index 41d039b7da0..291f6dcc2aa 100644
--- a/spec/requests/api/v3/settings_spec.rb
+++ b/spec/requests/api/v3/settings_spec.rb
@@ -10,7 +10,7 @@ describe API::V3::Settings, 'Settings' do
       expect(response).to have_http_status(200)
       expect(json_response).to be_an Hash
       expect(json_response['default_projects_limit']).to eq(42)
-      expect(json_response['signin_enabled']).to be_truthy
+      expect(json_response['password_authentication_enabled']).to be_truthy
       expect(json_response['repository_storage']).to eq('default')
       expect(json_response['koding_enabled']).to be_falsey
       expect(json_response['koding_url']).to be_nil
@@ -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, signin_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com',
+          default_projects_limit: 3, password_authentication_enabled: 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_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
-        expect(json_response['signin_enabled']).to be_falsey
+        expect(json_response['password_authentication_enabled']).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 185679e1a0f..95c8fabb4ce 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -463,7 +463,7 @@ describe 'Git HTTP requests', lib: true do
 
               context 'when internal auth is disabled' do
                 before do
-                  allow_any_instance_of(ApplicationSetting).to receive(:signin_enabled?) { false }
+                  allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { 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 5e4cf05748e..8d79ea3dd40 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -101,7 +101,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(:signin_enabled?) { false }
+          allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled?) { false }
           get '/jwt/auth', parameters, headers
 
           expect(response).to have_http_status(401)