2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-10-03 17:21:20 +08:00

UX: Show remove password button without suspense (#34635)

We have this slightly awkward UX on the Preferences > Security page, where we offer to remove the password from an account if it has other means of signing in. You need to first click a cryptic Show button which may or may not lead you to the button you want. This button loads the associated accounts so we can evaluate whether to show the Remove password button or not.

This PR loads that data when entering the route, so that we can just show the button up-front. If removing password isn't possible, the button is disabled and shows an explainer underneath.
This commit is contained in:
Ted Johansson 2025-10-02 15:04:13 +08:00 committed by GitHub
parent 064aba3010
commit ebb1a49ea3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 45 additions and 35 deletions

View file

@ -118,15 +118,23 @@ export default class SecurityController extends Controller {
"model.no_password",
"siteSettings",
"model.user_passkeys",
"model.associated_accounts"
"model.associated_accounts",
"model.can_remove_password"
)
canRemovePassword(
isAnonymous,
noPassword,
siteSettings,
userPasskeys,
associatedAccounts
associatedAccounts,
canRemove
) {
// Hint returned from staff-info controller that
// works even if staff hasn't revealed e-mails.
if (canRemove) {
return true;
}

if (
isAnonymous ||
noPassword ||

View file

@ -11,7 +11,6 @@ import UserPasskeys from "discourse/components/user-preferences/user-passkeys";
import icon from "discourse/helpers/d-icon";
import formatDate from "discourse/helpers/format-date";
import lazyHash from "discourse/helpers/lazy-hash";
import routeAction from "discourse/helpers/route-action";
import { i18n } from "discourse-i18n";

export default RouteTemplate(
@ -47,30 +46,24 @@ export default RouteTemplate(
</div>

{{#unless @controller.model.no_password}}
{{#if @controller.associatedAccountsLoaded}}
{{#if @controller.canRemovePassword}}
<div class="controls">
<a
href
{{on "click" @controller.removePassword}}
hidden={{@controller.removePasswordInProgress}}
id="remove-password-link"
>
{{icon "trash-can"}}
{{i18n "user.change_password.remove"}}
</a>
</div>
{{/if}}
{{else}}
<div class="controls">
<DButton
@action={{fn (routeAction "checkEmail") @controller.model}}
@title="admin.users.check_email.title"
@icon="envelope"
@label="admin.users.check_email.text"
/>
<div class="controls">
<button
{{on "click" @controller.removePassword}}
disabled={{not @controller.canRemovePassword}}
hidden={{@controller.removePasswordInProgress}}
class="btn btn-transparent"
id="remove-password-link"
>
{{icon "trash-can"}}
{{i18n "user.change_password.remove"}}
</button>
</div>

{{#unless @controller.canRemovePassword}}
<div class="instructions">
{{i18n "user.change_password.remove_disabled"}}
</div>
{{/if}}
{{/unless}}
{{/unless}}
</div>


View file

@ -234,17 +234,17 @@ acceptance("User Preferences - Security", function (needs) {
await visit("/u/eviltrout/preferences/security");
// eviltrout starts with an entry in associated_accounts, can remove password
assert
.dom("#remove-password-link")
.exists("shows for user with associated account");
.dom("#remove-password-link:not(:disabled)")
.exists("is enabled for user with associated account");

updateCurrentUser({
associated_accounts: null,
});

assert
.dom("#remove-password-link")
.doesNotExist(
"does not show for user with no associated account and no passkeys"
.dom("#remove-password-link:disabled")
.exists(
"is disabled for user with no associated account and no passkeys"
);

updateCurrentUser({
@ -257,7 +257,9 @@ acceptance("User Preferences - Security", function (needs) {
},
],
});
assert.dom("#remove-password-link").exists("shows for user with passkey");
assert
.dom("#remove-password-link:not(:disabled)")
.exists("is enabled for user with passkey");
});

test("Removing User Password", async function (assert) {

View file

@ -1529,7 +1529,8 @@ class UsersController < ApplicationController
number_of_suspensions
warnings_received_count
number_of_rejected_posts
].each { |info| result[info] = @user.public_send(info) }
can_remove_password?
].each { |info| result[info.delete_suffix("?")] = @user.public_send(info) }

render json: result
end

View file

@ -929,8 +929,12 @@ class User < ActiveRecord::Base
@raw_password = pw # still required to maintain compatibility with usage of password-related User interface
end

def can_remove_password?
associated_accounts.present? || passkey_credential_ids.present?
end

def remove_password
raise Discourse::InvalidAccess if associated_accounts.blank? && passkey_credential_ids.blank?
raise Discourse::InvalidAccess if !can_remove_password?

user_password.destroy if user_password
end

View file

@ -1799,6 +1799,7 @@ en:
verify_identity: "To continue, please verify your identity."
title: "Password Reset"
remove: "Remove Password"
remove_disabled: "The account needs at least one other login method (e.g. a passkey or a linked external account) before removing the password."
remove_detail: "Your account will no longer be accessible with a password unless you reset it."

second_factor_backup:

View file

@ -8022,9 +8022,10 @@ RSpec.describe UsersController do
number_of_suspensions
warnings_received_count
number_of_rejected_posts
can_remove_password?
].each do |info|
user_instance.expects(info).returns(user.public_send(info))
result[info.to_s] = user.public_send(info)
result[info.to_s.delete_suffix("?")] = user.public_send(info)
end

sign_in(admin)