From a1e5796ba162b68d81b8fd80765b7b8b6364c9ab Mon Sep 17 00:00:00 2001 From: Linca <41134017+Lhcfl@users.noreply.github.com> Date: Fri, 27 Sep 2024 20:08:05 +0800 Subject: [PATCH] FEAT: Allow admin delete user's associated accounts (#29018) This commit introduces a feature that allows an admin to delete a user's associated account. After deletion, a log will be recorded in staff actions. ref=t/136675 --- .../addon/controllers/admin-user-index.js | 14 ++++- .../admin/addon/models/admin-user.js | 11 ++++ .../admin/addon/templates/user-index.hbs | 11 ++++ app/controllers/admin/users_controller.rb | 24 +++++++++ app/models/user_history.rb | 2 + app/services/staff_action_logger.rb | 11 ++++ config/locales/client.en.yml | 5 ++ config/routes.rb | 1 + lib/guardian/user_guardian.rb | 4 ++ spec/requests/admin/users_controller_spec.rb | 52 +++++++++++++++++++ 10 files changed, 133 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/addon/controllers/admin-user-index.js b/app/assets/javascripts/admin/addon/controllers/admin-user-index.js index b3133a9f053..d1fac2a80d3 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-user-index.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-user-index.js @@ -78,8 +78,8 @@ export default class AdminUserIndexController extends Controller.extend( @discourseComputed("model.associated_accounts") associatedAccounts(associatedAccounts) { return associatedAccounts - .map((provider) => `${provider.name} (${provider.description})`) - .join(", "); + ?.map((provider) => `${provider.name} (${provider.description})`) + ?.join(", "); } @discourseComputed("model.user_fields.[]") @@ -319,6 +319,16 @@ export default class AdminUserIndexController extends Controller.extend( return this.model.silence(); } + @action + deleteAssociatedAccounts() { + this.dialog.yesNoConfirm({ + message: I18n.t("admin.user.delete_associated_accounts_confirm"), + didConfirm: () => { + this.model.deleteAssociatedAccounts().catch(popupAjaxError); + }, + }); + } + @action anonymize() { const user = this.model; diff --git a/app/assets/javascripts/admin/addon/models/admin-user.js b/app/assets/javascripts/admin/addon/models/admin-user.js index 79489398db6..0449fd5056c 100644 --- a/app/assets/javascripts/admin/addon/models/admin-user.js +++ b/app/assets/javascripts/admin/addon/models/admin-user.js @@ -287,6 +287,17 @@ export default class AdminUser extends User { }); } + deleteAssociatedAccounts() { + return ajax(`/admin/users/${this.id}/delete_associated_accounts`, { + type: "PUT", + data: { + context: window.location.pathname, + }, + }).then(() => { + this.set("associated_accounts", []); + }); + } + destroy(formData) { return ajax(`/admin/users/${this.id}.json`, { type: "DELETE", diff --git a/app/assets/javascripts/admin/addon/templates/user-index.hbs b/app/assets/javascripts/admin/addon/templates/user-index.hbs index a2c69d3864f..6244c0f7022 100644 --- a/app/assets/javascripts/admin/addon/templates/user-index.hbs +++ b/app/assets/javascripts/admin/addon/templates/user-index.hbs @@ -157,6 +157,17 @@ /> {{/if}} + {{#if (and this.currentUser.admin this.associatedAccounts)}} +
+ +
+ {{/if}} {{/if}} diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index f4878dd9138..94594b8f085 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -26,6 +26,7 @@ class Admin::UsersController < Admin::StaffController disable_second_factor delete_posts_batch sso_record + delete_associated_accounts ] def index @@ -514,6 +515,29 @@ class Admin::UsersController < Admin::StaffController render json: success_json end + def delete_associated_accounts + guardian.ensure_can_delete_user_associated_accounts!(@user) + previous_value = + @user + .user_associated_accounts + .select(:provider_name, :provider_uid, :info) + .map do |associated_account| + { + provider: associated_account.provider_name, + uid: associated_account.provider_uid, + info: associated_account.info, + }.to_s + end + .join(",") + StaffActionLogger.new(current_user).log_delete_associated_accounts( + @user, + previous_value:, + context: params[:context], + ) + @user.user_associated_accounts.delete_all + render json: success_json + end + private def fetch_user diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 14b83cc3454..92562313b21 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -155,6 +155,7 @@ class UserHistory < ActiveRecord::Base tag_group_create: 116, tag_group_destroy: 117, tag_group_change: 118, + delete_associated_accounts: 119, ) end @@ -272,6 +273,7 @@ class UserHistory < ActiveRecord::Base tag_group_create tag_group_destroy tag_group_change + delete_associated_accounts ] end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index ee16aa51c9d..29e2f241cb4 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -1079,6 +1079,17 @@ class StaffActionLogger ) end + def log_delete_associated_accounts(user, previous_value:, context:) + UserHistory.create!( + params.merge( + action: UserHistory.actions[:delete_associated_accounts], + target_user_id: user.id, + previous_value:, + context:, + ), + ) + end + private def json_params(previous_value, new_value) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6a6879e175b..9ef3816efb9 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -6346,6 +6346,7 @@ en: tag_group_create: "create tag group" tag_group_destroy: "delete tag group" tag_group_change: "change tag group" + delete_associated_accounts: "delete associated accounts" screened_emails: title: "Screened Emails" description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed." @@ -6575,6 +6576,9 @@ en: check_sso: title: "Reveal SSO payload" text: "Show" + delete_associated_accounts: + title: "Delete all associated accounts for this user" + text: "Delete associated accounts" user: suspend_failed: "Something went wrong suspending this user %{error}" @@ -6692,6 +6696,7 @@ en: post_edits_count: "Post Edits" anonymize: "Anonymize User" anonymize_confirm: "Are you SURE you want to anonymize this account? This will change the username and email, and reset all profile information." + delete_associated_accounts_confirm: "Are you SURE you want to delete associated accounts from this account? They may not be able to log in." anonymize_yes: "Yes, anonymize this account" anonymize_failed: "There was a problem anonymizing the account." delete: "Delete User" diff --git a/config/routes.rb b/config/routes.rb index 88ccaa708ba..89a10b2eff9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -162,6 +162,7 @@ Discourse::Application.routes.draw do put "disable_second_factor" delete "sso_record" get "similar-users.json" => "users#similar_users" + put "delete_associated_accounts" end get "users/:id.json" => "users#show", :defaults => { format: "json" } get "users/:id/:username" => "users#show", diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb index 28da0c247b6..79f2c7b3876 100644 --- a/lib/guardian/user_guardian.rb +++ b/lib/guardian/user_guardian.rb @@ -204,6 +204,10 @@ module UserGuardian SiteSetting.enable_discourse_connect && user && is_admin? end + def can_delete_user_associated_accounts?(user) + user && is_admin? + end + def can_change_tracking_preferences?(user) (SiteSetting.allow_changing_staged_user_tracking || !user.staged) && can_edit_user?(user) end diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 8c7879308c0..6c643fa7171 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -2440,6 +2440,58 @@ RSpec.describe Admin::UsersController do end end + describe "#delete_associated_accounts" do + fab!(:user_associated_accounts) do + UserAssociatedAccount.create!( + provider_name: "github", + provider_uid: "123456789", + user_id: user.id, + last_used: 1.seconds.ago, + ) + end + + context "when logged in as an admin" do + before { sign_in(admin) } + + it "deletes the record and logs the deletion" do + put "/admin/users/#{user.id}/delete_associated_accounts.json" + + expect(response.status).to eq(200) + expect(user.user_associated_accounts).to eq([]) + expect(UserHistory.last).to have_attributes( + acting_user_id: admin.id, + target_user_id: user.id, + action: UserHistory.actions[:delete_associated_accounts], + ) + expect(UserHistory.last.previous_value).to include(':uid=>"123456789"') + end + end + + context "when logged in as a moderator" do + before { sign_in(moderator) } + + it "prevents deletion of associated accounts with a 403 response" do + put "/admin/users/#{user.id}/delete_associated_accounts.json" + + expect(response.status).to eq(403) + expect(response.parsed_body["errors"]).to include(I18n.t("invalid_access")) + expect(user.user_associated_accounts).to be_present + end + end + + context "when logged in as a non-staff user" do + before { sign_in(user) } + + it "prevents deletion of associated accounts with a 404 response" do + put "/admin/users/#{user.id}/delete_associated_accounts.json" + + expect(response.status).to eq(404) + expect(response.parsed_body["errors"]).to include(I18n.t("not_found")) + expect(user.user_associated_accounts).to be_present + end + end + end + describe "#anonymize" do shared_examples "user anonymization possible" do it "will make the user anonymous" do