Allow changing password if it exists

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

@@ -52,10 +52,14 @@
 
     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)
 
     # mimic the user clicking on the link in the mail
     token = Token::Recovery.first
-    visit account_lost_password_path(token: token.value)
+    mail_body = mail.body.parts.find { |p| p.mime_type == "text/html" }.body.to_s
+    mail_document = Capybara::Node::Simple.new(mail_body)
+    visit mail_document.find("a")["href"]
 
     fill_in "New password", with: new_password
     fill_in "Confirmation", with: new_password
@@ -91,9 +95,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 +115,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