UX: Use regular reset-password flow for expired passwords (#27316)

This makes it more obvious what's happening, and makes it much less likely that users will send repeated reset emails (and thereby hit the rate limit)

Followup to e97ef7e9af60788f5761f6989ea2b70edaa3b79d
This commit is contained in:
David Taylor 2024-06-04 12:47:33 +01:00 committed by GitHub
parent f0539afb02
commit aa37be3323
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 47 additions and 53 deletions

View File

@ -5,6 +5,7 @@
@flash={{this.flash}} @flash={{this.flash}}
@flashType={{this.flashType}} @flashType={{this.flashType}}
{{did-insert this.preloadLogin}} {{did-insert this.preloadLogin}}
{{on "click" this.interceptResetLink}}
> >
<:body> <:body>
<PluginOutlet @name="login-before-modal-body" @connectorTagName="div" /> <PluginOutlet @name="login-before-modal-body" @connectorTagName="div" />

View File

@ -3,10 +3,12 @@ import { tracked } from "@glimmer/tracking";
import { action } from "@ember/object"; import { action } from "@ember/object";
import { schedule } from "@ember/runloop"; import { schedule } from "@ember/runloop";
import { service } from "@ember/service"; import { service } from "@ember/service";
import { htmlSafe } from "@ember/template";
import { isEmpty } from "@ember/utils"; import { isEmpty } from "@ember/utils";
import { ajax } from "discourse/lib/ajax"; import { ajax } from "discourse/lib/ajax";
import { popupAjaxError } from "discourse/lib/ajax-error"; import { popupAjaxError } from "discourse/lib/ajax-error";
import cookie, { removeCookie } from "discourse/lib/cookie"; import cookie, { removeCookie } from "discourse/lib/cookie";
import { wantsNewWindow } from "discourse/lib/intercept-click";
import { areCookiesEnabled } from "discourse/lib/utilities"; import { areCookiesEnabled } from "discourse/lib/utilities";
import { wavingHandURL } from "discourse/lib/waving-hand-url"; import { wavingHandURL } from "discourse/lib/waving-hand-url";
import { import {
@ -18,6 +20,7 @@ import { SECOND_FACTOR_METHODS } from "discourse/models/user";
import escape from "discourse-common/lib/escape"; import escape from "discourse-common/lib/escape";
import getURL from "discourse-common/lib/get-url"; import getURL from "discourse-common/lib/get-url";
import I18n from "discourse-i18n"; import I18n from "discourse-i18n";
import ForgotPassword from "./forgot-password";
export default class Login extends Component { export default class Login extends Component {
@service capabilities; @service capabilities;
@ -25,6 +28,7 @@ export default class Login extends Component {
@service siteSettings; @service siteSettings;
@service site; @service site;
@service login; @service login;
@service modal;
@tracked loggingIn = false; @tracked loggingIn = false;
@tracked loggedIn = false; @tracked loggedIn = false;
@ -248,6 +252,13 @@ export default class Login extends Component {
} else if (result.reason === "suspended") { } else if (result.reason === "suspended") {
this.args.closeModal(); this.args.closeModal();
this.dialog.alert(result.error); this.dialog.alert(result.error);
} else if (result.reason === "expired") {
this.flash = htmlSafe(
I18n.t("login.password_expired", {
reset_url: getURL("/password-reset"),
})
);
this.flashType = "error";
} else { } else {
this.flash = result.error; this.flash = result.error;
this.flashType = "error"; this.flashType = "error";
@ -344,4 +355,21 @@ export default class Login extends Component {
} }
this.args.model.showCreateAccount(createAccountProps); this.args.model.showCreateAccount(createAccountProps);
} }
@action
interceptResetLink(event) {
if (
!wantsNewWindow(event) &&
event.target.href &&
new URL(event.target.href).pathname === getURL("/password-reset")
) {
event.preventDefault();
event.stopPropagation();
this.modal.show(ForgotPassword, {
model: {
emailOrUsername: this.loginName,
},
});
}
}
} }

View File

@ -353,13 +353,7 @@ class SessionController < ApplicationController
# User's password has expired so they need to reset it # User's password has expired so they need to reset it
if user.password_expired?(password) if user.password_expired?(password)
begin render json: { error: "expired", reason: "expired" }
enqueue_password_reset_for_user(user)
rescue RateLimiter::LimitExceeded
# Just noop here as user would have already been sent the forgot password email more than once
end
render json: { error: I18n.t("login.password_expired") }
return return
end end
else else

View File

@ -2291,6 +2291,7 @@ en:
error: "Unknown error" error: "Unknown error"
cookies_error: "Your browser seems to have cookies disabled. You might not be able to log in without enabling them first." cookies_error: "Your browser seems to have cookies disabled. You might not be able to log in without enabling them first."
rate_limit: "Please wait before trying to log in again." rate_limit: "Please wait before trying to log in again."
password_expired: "Password expired. Please <a href='%{reset_url}'>reset your password</a>."
blank_username: "Please enter your email or username." blank_username: "Please enter your email or username."
blank_username_or_password: "Please enter your email or username, and password." blank_username_or_password: "Please enter your email or username, and password."
reset_password: "Reset Password" reset_password: "Reset Password"

View File

@ -2899,7 +2899,6 @@ en:
not_approved: "Your account hasn't been approved yet. You will be notified by email when you are ready to log in." not_approved: "Your account hasn't been approved yet. You will be notified by email when you are ready to log in."
incorrect_username_email_or_password: "Incorrect username, email or password" incorrect_username_email_or_password: "Incorrect username, email or password"
incorrect_password: "Incorrect password" incorrect_password: "Incorrect password"
password_expired: "Password expired. Reset instructions sent to your email."
incorrect_password_or_passkey: "Incorrect password or passkey" incorrect_password_or_passkey: "Incorrect password or passkey"
wait_approval: "Thanks for signing up. We will notify you when your account has been approved." wait_approval: "Thanks for signing up. We will notify you when your account has been approved."
active: "Your account is activated and ready to use." active: "Your account is activated and ready to use."

View File

@ -2045,53 +2045,14 @@ RSpec.describe SessionController do
use_redis_snapshotting use_redis_snapshotting
it "should return an error response code with the right error message and enqueues the password reset email" do it "should return an error response code with the right error message" do
expect_enqueued_with( post "/session.json", params: { login: user.username, password: "myawesomepassword" }
job: :critical_user_email,
args: {
type: "forgot_password",
user_id: user.id,
},
) do
post "/session.json", params: { login: user.username, password: "myawesomepassword" }
end
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(response.parsed_body["error"]).to eq(I18n.t("login.password_expired")) expect(response.parsed_body["error"]).to eq("expired")
expect(response.parsed_body["reason"]).to eq("expired")
expect(session[:current_user_id]).to eq(nil) expect(session[:current_user_id]).to eq(nil)
end end
it "should limit the number of forgot password emails sent a day to the user when logging in with an expired password" do
SiteSetting.max_logins_per_ip_per_minute =
described_class::FORGOT_PASSWORD_EMAIL_LIMIT_PER_DAY + 1
SiteSetting.max_logins_per_ip_per_hour =
described_class::FORGOT_PASSWORD_EMAIL_LIMIT_PER_DAY + 1
described_class::FORGOT_PASSWORD_EMAIL_LIMIT_PER_DAY.times do
expect_enqueued_with(
job: :critical_user_email,
args: {
type: "forgot_password",
user_id: user.id,
},
) do
post "/session.json", params: { login: user.username, password: "myawesomepassword" }
expect(response.status).to eq(200)
end
end
expect_not_enqueued_with(
job: :critical_user_email,
args: {
type: "forgot_password",
user_id: user.id,
},
) do
post "/session.json", params: { login: user.username, password: "myawesomepassword" }
expect(response.status).to eq(200)
end
end
end end
context "when a user has security key-only 2FA login" do context "when a user has security key-only 2FA login" do

View File

@ -41,13 +41,23 @@ shared_examples "login scenarios" do
it "displays the right message when user's email has been marked as expired" do it "displays the right message when user's email has been marked as expired" do
password = "myawesomepassword" password = "myawesomepassword"
user.update!(password:) user.update!(password:)
expired_user_password = Fabricate(:expired_user_password, user:, password:) Fabricate(:expired_user_password, user:, password:)
login_modal.open login_modal.open
login_modal.fill(username: user.username, password:) login_modal.fill(username: user.username, password:)
login_modal.click_login login_modal.click_login
expect(login_modal).to have_content(I18n.t("login.password_expired")) expect(login_modal.find("#modal-alert")).to have_content(
I18n.t("js.login.password_expired", reset_url: "/password-reset").gsub(/<.*?>/, ""),
)
login_modal.find("#modal-alert a").click
find("button.forgot-password-reset").click
wait_for(timeout: 5) { ActionMailer::Base.deliveries.count != 0 }
mail = ActionMailer::Base.deliveries.last
expect(mail.to).to contain_exactly(user.email)
expect(mail.body).to match(%r{/u/password-reset/\S+})
end end
it "can reset password" do it "can reset password" do