Allow changing password if it exists

Previously we'd be hiding the "change password" dialog on the
basis of an external authentication method existing. However, that's
not enough, because (at least with user remapping enabled) it's possible
that a user that logged in via password once, gained the ability to login
through SSO afterwards. Such a user then can use both mean to authenticate,
thus they also need to be able to change a potentially compromised password.

Much more work is needed here: Users need to be aware that their password still
works, they need to be able to delete a password if they only want to use SSO and
maybe there's also a use case for deleting an SSO association and going back to
password-based logins. However, all of these things require more UI changes and
some proper product development first.

This change is a first step to improve the situation.

Author: NobodysNightmare

app/models/user.rb

@@ -369,8 +369,8 @@ def check_password?(clear_password, update_legacy: true)
 
   # Does the backend storage allow this user to change their password?
   def change_password_allowed?
-    return false if uses_external_authentication? ||
-      OpenProject::Configuration.disable_password_login?
+    return false if OpenProject::Configuration.disable_password_login?
+    return false if uses_external_authentication? && current_password.nil?
 
     ldap_auth_source_id.blank?
   end

spec/controllers/users_controller_spec.rb

@@ -787,7 +787,7 @@
                      current_user: :user
 
     context "with external authentication" do
-      let(:some_user) { create(:user, identity_url: "some:identity") }
+      let(:some_user) { create(:user, :passwordless, identity_url: "some:identity") }
 
       before do
         as_logged_in_user(admin) do
@@ -801,6 +801,21 @@
       end
     end
 
+    context "with external authentication and an existing password" do
+      let(:some_user) { create(:user, identity_url: "some:identity") }
+
+      before do
+        as_logged_in_user(admin) do
+          put :update, params: { id: some_user.id, user: { force_password_change: "true" } }
+        end
+        some_user.reload
+      end
+
+      it "accepts setting force_password_change" do
+        expect(some_user.force_password_change).to be(true)
+      end
+    end
+
     context "with ldap auth source" do
       let(:ldap_auth_source) { create(:ldap_auth_source) }
 

spec/factories/user_factory.rb

@@ -79,6 +79,11 @@
       end
     end
 
+    trait :passwordless do
+      password { nil }
+      password_confirmation { nil }
+    end
+
     factory :admin, parent: :user, class: "User" do
       firstname { "OpenProject" }
       sequence(:lastname) { |n| "Admin#{n}" }

spec/features/auth/lost_password_spec.rb

@@ -91,9 +91,9 @@
     end
   end
 
-  context "when user has identity_url" do
+  context "when user only authenticates via SSO" do
     let!(:provider) { create(:saml_provider, slug: "saml", display_name: "The SAML provider") }
-    let!(:user) { create(:user, authentication_provider: provider) }
+    let!(:user) { create(:user, :passwordless, authentication_provider: provider) }
 
     it "sends an email with external auth info" do
       visit account_lost_password_path
@@ -111,4 +111,24 @@
       expect(mail.body.parts.first.body.to_s).to include "(The SAML provider)"
     end
   end
+
+  context "when authenticates via password & SSO" do
+    let!(:provider) { create(:saml_provider, slug: "saml", display_name: "The SAML provider") }
+    let!(:user) { create(:user, authentication_provider: provider) }
+
+    it "sends an email with external auth info" do
+      visit account_lost_password_path
+
+      # shows same flash for invalid and existing users
+      fill_in "mail", with: user.mail
+      click_on "Submit"
+
+      expect_flash(message: I18n.t(:notice_account_lost_email_sent))
+
+      perform_enqueued_jobs
+      expect(ActionMailer::Base.deliveries.size).to be 1
+      mail = ActionMailer::Base.deliveries.first
+      expect(mail.subject).to eq I18n.t("mail_subject_lost_password", value: Setting.app_title)
+    end
+  end
 end