diff --git a/app/models/user.rb b/app/models/user.rb index 49f30176126b..b4df83784711 100644 --- a/app/models/user.rb +++ b/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 diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 2876829199f7..5ac803a00b88 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/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) } diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 70f1405ddf02..594f04c6103c 100644 --- a/spec/factories/user_factory.rb +++ b/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}" } diff --git a/spec/features/auth/lost_password_spec.rb b/spec/features/auth/lost_password_spec.rb index a863d612c951..91b8e354f328 100644 --- a/spec/features/auth/lost_password_spec.rb +++ b/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