Ensure LDAP users cannot reset local password to bypass LDAP

Merge branch 'security_dblessing_ldap_password_reset_fix' into 'master'

See merge request gitlab-org/security/gitlab!3815

Changelog: security

app/controllers/passwords_controller.rb

@@ -60,11 +60,7 @@ def resource_from_email
   end
 
   def check_password_authentication_available
-    if resource
-      return if resource.allow_password_authentication?
-    elsif Gitlab::CurrentSettings.password_authentication_enabled?
-      return
-    end
+    return if Gitlab::CurrentSettings.password_authentication_enabled?
 
     redirect_to after_sending_reset_password_instructions_path_for(resource_name),
       alert: _("Password authentication is unavailable.")

app/models/concerns/recoverable_by_any_email.rb

@@ -14,6 +14,7 @@ def send_reset_password_instructions(attributes = {})
       return super unless email
 
       recoverable = email.user
+      return recoverable.password_auth_unavailable_error! unless recoverable.allow_password_authentication_for_web?
 
       recoverable.send_reset_password_instructions(to: email.email)
       recoverable
@@ -28,6 +29,11 @@ def send_reset_password_instructions(opts = {})
     token
   end
 
+  def password_auth_unavailable_error!
+    errors.add(:password, :unavailable, message: _('Password authentication is unavailable.'))
+    self
+  end
+
   protected
 
   def send_reset_password_instructions_notification(token, opts = {})

spec/controllers/passwords_controller_spec.rb

@@ -21,16 +21,6 @@
         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, params: { user: { email: user.email } }
-
-        expect(flash[:alert]).to eq _('Password authentication is unavailable.')
-      end
-    end
   end
 
   describe '#update' do

spec/models/concerns/recoverable_by_any_email_spec.rb

@@ -67,11 +67,38 @@
       end
     end
 
+    shared_examples "does not send 'Reset password instructions' email when password auth is not allowed" do
+      it 'find the user with error' do
+        expect(send_reset_password_instructions).to be_instance_of User
+        expect(send_reset_password_instructions.errors[:password])
+          .to include(_('Password authentication is unavailable.'))
+      end
+
+      it 'does not send email to anyone' do
+        reset_delivered_emails!
+
+        expect { send_reset_password_instructions }
+          .not_to have_enqueued_mail(DeviseMailer, :reset_password_instructions)
+
+        perform_enqueued_jobs
+
+        should_not_email_anyone
+      end
+    end
+
     context "when email param matches user's confirmed primary email" do
       let(:expected_user) { user }
       let(:email) { user_confirmed_primary_email }
 
       it_behaves_like "sends 'Reset password instructions' email"
+
+      context 'when password authentication is not allowed' do
+        before do
+          allow(Gitlab::CurrentSettings).to receive_messages(password_authentication_enabled_for_web?: false)
+        end
+
+        it_behaves_like "does not send 'Reset password instructions' email when password auth is not allowed"
+      end
     end
 
     context "when email param matches user's unconfirmed primary email" do
@@ -139,5 +166,23 @@
 
       it_behaves_like "does not send 'Reset password instructions' email"
     end
+
+    context 'with an LDAP user' do
+      let_it_be(:ldap_user) { create(:omniauth_user, :ldap) }
+
+      context 'with a confirmed primary email' do
+        let(:email) { ldap_user.email }
+
+        it_behaves_like "does not send 'Reset password instructions' email when password auth is not allowed"
+      end
+
+      context 'with a confirmed secondary email' do
+        let(:email) do
+          create(:email, :confirmed, user: ldap_user, email: 'confirmed-secondary-ldap-email@example.com').email
+        end
+
+        it_behaves_like "does not send 'Reset password instructions' email when password auth is not allowed"
+      end
+    end
   end
 end