From ebb1a49ea3216e5e5549f90f9f20202a72e03cc2 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Thu, 2 Oct 2025 15:04:13 +0800 Subject: [PATCH] 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. --- .../app/controllers/preferences/security.js | 12 +++++- .../app/templates/preferences/security.gjs | 41 ++++++++----------- .../user-preferences-security-test.js | 14 ++++--- app/controllers/users_controller.rb | 3 +- app/models/user.rb | 6 ++- config/locales/client.en.yml | 1 + spec/requests/users_controller_spec.rb | 3 +- 7 files changed, 45 insertions(+), 35 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/security.js b/app/assets/javascripts/discourse/app/controllers/preferences/security.js index c2096c2dc3e..60e50de52a1 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/security.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/security.js @@ -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 || diff --git a/app/assets/javascripts/discourse/app/templates/preferences/security.gjs b/app/assets/javascripts/discourse/app/templates/preferences/security.gjs index 474f193cbcf..d6100ff661b 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/security.gjs +++ b/app/assets/javascripts/discourse/app/templates/preferences/security.gjs @@ -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( {{#unless @controller.model.no_password}} - {{#if @controller.associatedAccountsLoaded}} - {{#if @controller.canRemovePassword}} -
- -
- {{/if}} - {{else}} -
- +
+ +
+ + {{#unless @controller.canRemovePassword}} +
+ {{i18n "user.change_password.remove_disabled"}}
- {{/if}} + {{/unless}} {{/unless}}
diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-security-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-security-test.js index ef16a0368a9..b9bbdbc363a 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-security-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-security-test.js @@ -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) { diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 470e948c498..d19b7a37774 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index 03fb6ffbfc8..d2997f3c9ed 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 396c8fc9027..fa0da90369c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -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: diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index f4a19f2e6e6..632293c4fa9 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -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)