From 187b0bfb430c28faaa30208e6f32d5492e41c2e5 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Thu, 8 Dec 2022 14:42:33 +0200 Subject: [PATCH] FEATURE: Show similar users when penalizing a user (#19334) * FEATURE: Show similar users when penalizing a user Moderators will be notified if other users with the same IP address exist before penalizing a user. * FEATURE: Allow staff to penalize multiple users This allows staff members to suspend or silence multiple users belonging to the same person. --- .../components/admin-penalty-similar-users.js | 29 ++++ .../controllers/modals/admin-silence-user.js | 7 +- .../controllers/modals/admin-suspend-user.js | 8 +- .../admin-penalty-similar-users.hbs | 35 +++++ .../templates/modal/admin-silence-user.hbs | 4 + .../templates/modal/admin-suspend-user.hbs | 5 +- .../stylesheets/common/admin/admin_base.scss | 1 + .../stylesheets/common/admin/penalty.scss | 11 ++ app/controllers/admin/users_controller.rb | 133 +++++++++++------- .../admin_detailed_user_serializer.rb | 26 +++- .../similar_admin_user_serializer.rb | 14 ++ config/locales/client.en.yml | 9 ++ spec/requests/admin/users_controller_spec.rb | 33 +++++ 13 files changed, 259 insertions(+), 56 deletions(-) create mode 100644 app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.js create mode 100644 app/assets/javascripts/admin/addon/templates/components/admin-penalty-similar-users.hbs create mode 100644 app/assets/stylesheets/common/admin/penalty.scss create mode 100644 app/serializers/similar_admin_user_serializer.rb diff --git a/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.js b/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.js new file mode 100644 index 00000000000..0e0158e8684 --- /dev/null +++ b/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.js @@ -0,0 +1,29 @@ +import Component from "@ember/component"; +import { action } from "@ember/object"; +import discourseComputed from "discourse-common/utils/decorators"; + +export default Component.extend({ + tagName: "", + + @discourseComputed("type") + penaltyField(penaltyType) { + if (penaltyType === "suspend") { + return "can_be_suspended"; + } else if (penaltyType === "silence") { + return "can_be_silenced"; + } + }, + + @action + selectUserId(userId, event) { + if (!this.selectedUserIds) { + return; + } + + if (event.target.checked) { + this.selectedUserIds.pushObject(userId); + } else { + this.selectedUserIds.removeObject(userId); + } + }, +}); diff --git a/app/assets/javascripts/admin/addon/controllers/modals/admin-silence-user.js b/app/assets/javascripts/admin/addon/controllers/modals/admin-silence-user.js index 506598625d4..077a0863306 100644 --- a/app/assets/javascripts/admin/addon/controllers/modals/admin-silence-user.js +++ b/app/assets/javascripts/admin/addon/controllers/modals/admin-silence-user.js @@ -9,7 +9,11 @@ export default Controller.extend(PenaltyController, { onShow() { this.resetModal(); - this.setProperties({ silenceUntil: null, silencing: false }); + this.setProperties({ + silenceUntil: null, + silencing: false, + otherUserIds: [], + }); }, finishedSetup() { @@ -36,6 +40,7 @@ export default Controller.extend(PenaltyController, { post_id: this.postId, post_action: this.postAction, post_edit: this.postEdit, + other_user_ids: this.otherUserIds, }); }).finally(() => this.set("silencing", false)); }, diff --git a/app/assets/javascripts/admin/addon/controllers/modals/admin-suspend-user.js b/app/assets/javascripts/admin/addon/controllers/modals/admin-suspend-user.js index c82a5628084..c9f34d89577 100644 --- a/app/assets/javascripts/admin/addon/controllers/modals/admin-suspend-user.js +++ b/app/assets/javascripts/admin/addon/controllers/modals/admin-suspend-user.js @@ -9,7 +9,11 @@ export default Controller.extend(PenaltyController, { onShow() { this.resetModal(); - this.setProperties({ suspendUntil: null, suspending: false }); + this.setProperties({ + suspendUntil: null, + suspending: false, + otherUserIds: [], + }); }, finishedSetup() { @@ -28,7 +32,6 @@ export default Controller.extend(PenaltyController, { } this.set("suspending", true); - this.penalize(() => { return this.user.suspend({ suspend_until: this.suspendUntil, @@ -37,6 +40,7 @@ export default Controller.extend(PenaltyController, { post_id: this.postId, post_action: this.postAction, post_edit: this.postEdit, + other_user_ids: this.otherUserIds, }); }).finally(() => this.set("suspending", false)); }, diff --git a/app/assets/javascripts/admin/addon/templates/components/admin-penalty-similar-users.hbs b/app/assets/javascripts/admin/addon/templates/components/admin-penalty-similar-users.hbs new file mode 100644 index 00000000000..ccb35da4d87 --- /dev/null +++ b/app/assets/javascripts/admin/addon/templates/components/admin-penalty-similar-users.hbs @@ -0,0 +1,35 @@ +
+

+ {{i18n "admin.user.other_matches" (hash count=this.user.similar_users_count username=this.user.username)}} +

+ + + + + + + + + + + + + + + + {{#each this.user.similar_users as |user|}} + + + + + + + + + + {{/each}} + +
{{i18n "username"}}{{i18n "last_seen"}}{{i18n "admin.user.topics_entered"}}{{i18n "admin.user.posts_read_count"}}{{i18n "admin.user.time_read"}}{{i18n "created"}}
+ + {{avatar user imageSize="small"}} {{user.username}}{{format-duration user.last_seen_age}}{{number user.topics_entered}}{{number user.posts_read_count}}{{format-duration user.time_read}}{{format-duration user.created_at_age}}
+
diff --git a/app/assets/javascripts/admin/addon/templates/modal/admin-silence-user.hbs b/app/assets/javascripts/admin/addon/templates/modal/admin-silence-user.hbs index 08f3e0a909b..374cd5a7a85 100644 --- a/app/assets/javascripts/admin/addon/templates/modal/admin-silence-user.hbs +++ b/app/assets/javascripts/admin/addon/templates/modal/admin-silence-user.hbs @@ -18,6 +18,10 @@ {{/if}} + {{#if this.user.similar_users}} + + {{/if}} + diff --git a/app/assets/javascripts/admin/addon/templates/modal/admin-suspend-user.hbs b/app/assets/javascripts/admin/addon/templates/modal/admin-suspend-user.hbs index 71daba820cd..2aa787a65c6 100644 --- a/app/assets/javascripts/admin/addon/templates/modal/admin-suspend-user.hbs +++ b/app/assets/javascripts/admin/addon/templates/modal/admin-suspend-user.hbs @@ -13,12 +13,15 @@ - + {{#if this.postId}} {{/if}} + {{#if this.user.similar_users}} + + {{/if}} {{else}}
{{i18n "admin.user.cant_suspend"}} diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 07177c845eb..4687bda1890 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -1009,6 +1009,7 @@ a.inline-editable-field { @import "common/admin/dashboard"; @import "common/admin/settings"; @import "common/admin/users"; +@import "common/admin/penalty"; @import "common/admin/suspend"; @import "common/admin/badges"; @import "common/admin/emails"; diff --git a/app/assets/stylesheets/common/admin/penalty.scss b/app/assets/stylesheets/common/admin/penalty.scss new file mode 100644 index 00000000000..c54f63d8cf1 --- /dev/null +++ b/app/assets/stylesheets/common/admin/penalty.scss @@ -0,0 +1,11 @@ +.silence-user-modal, +.suspend-user-modal { + .table { + width: 100%; + + th, + td { + padding: 8px 0px; + } + } +} diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 6d2cf1d3fd0..7d7526417de 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class Admin::UsersController < Admin::StaffController + MAX_SIMILAR_USERS = 10 before_action :fetch_user, only: [:suspend, :unsuspend, @@ -40,7 +41,18 @@ class Admin::UsersController < Admin::StaffController def show @user = User.find_by(id: params[:id]) raise Discourse::NotFound unless @user - render_serialized(@user, AdminDetailedUserSerializer, root: false) + + similar_users = User.real + .where.not(id: @user.id) + .where(ip_address: @user.ip_address) + + render_serialized( + @user, + AdminDetailedUserSerializer, + root: false, + similar_users: similar_users.limit(MAX_SIMILAR_USERS), + similar_users_count: similar_users.count, + ) end def delete_posts_batch @@ -104,44 +116,52 @@ class Admin::UsersController < Admin::StaffController params.require([:suspend_until, :reason]) - @user.suspended_till = params[:suspend_until] - @user.suspended_at = DateTime.now - - message = params[:message] + all_users = [@user] + if Array === params[:other_user_ids] + all_users.concat(User.where(id: params[:other_user_ids]).to_a) + all_users.uniq! + end user_history = nil - User.transaction do - @user.save! + all_users.each do |user| + user.suspended_till = params[:suspend_until] + user.suspended_at = DateTime.now - user_history = StaffActionLogger.new(current_user).log_user_suspend( - @user, - params[:reason], + message = params[:message] + + User.transaction do + user.save! + + user_history = StaffActionLogger.new(current_user).log_user_suspend( + user, + params[:reason], + message: message, + post_id: params[:post_id] + ) + end + user.logged_out + + if message.present? + Jobs.enqueue( + :critical_user_email, + type: "account_suspended", + user_id: user.id, + user_history_id: user_history.id + ) + end + + DiscourseEvent.trigger( + :user_suspended, + user: user, + reason: params[:reason], message: message, - post_id: params[:post_id] + user_history: user_history, + post_id: params[:post_id], + suspended_till: params[:suspend_until], + suspended_at: DateTime.now ) end - @user.logged_out - - if message.present? - Jobs.enqueue( - :critical_user_email, - type: "account_suspended", - user_id: @user.id, - user_history_id: user_history.id - ) - end - - DiscourseEvent.trigger( - :user_suspended, - user: @user, - reason: params[:reason], - message: message, - user_history: user_history, - post_id: params[:post_id], - suspended_till: params[:suspend_until], - suspended_at: DateTime.now - ) perform_post_action @@ -341,31 +361,42 @@ class Admin::UsersController < Admin::StaffController return render json: failed_json.merge(message: message), status: 409 end - message = params[:message] - - silencer = UserSilencer.new( - @user, - current_user, - silenced_till: params[:silenced_till], - reason: params[:reason], - message_body: message, - keep_posts: true, - post_id: params[:post_id] - ) - if silencer.silence - Jobs.enqueue( - :critical_user_email, - type: "account_silenced", - user_id: @user.id, - user_history_id: silencer.user_history.id - ) + all_users = [@user] + if Array === params[:other_user_ids] + all_users.concat(User.where(id: params[:other_user_ids]).to_a) + all_users.uniq! end + + user_history = nil + + all_users.each do |user| + silencer = UserSilencer.new( + user, + current_user, + silenced_till: params[:silenced_till], + reason: params[:reason], + message_body: params[:message], + keep_posts: true, + post_id: params[:post_id] + ) + + if silencer.silence + user_history = silencer.user_history + Jobs.enqueue( + :critical_user_email, + type: "account_silenced", + user_id: user.id, + user_history_id: user_history.id + ) + end + end + perform_post_action render_json_dump( silence: { silenced: true, - silence_reason: silencer.user_history.try(:details), + silence_reason: user_history.try(:details), silenced_till: @user.silenced_till, silenced_at: @user.silenced_at, silenced_by: BasicUserSerializer.new(current_user, root: false).as_json diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb index aab939247c5..ec0d682d570 100644 --- a/app/serializers/admin_detailed_user_serializer.rb +++ b/app/serializers/admin_detailed_user_serializer.rb @@ -36,7 +36,9 @@ class AdminDetailedUserSerializer < AdminUserSerializer :can_disable_second_factor, :can_delete_sso_record, :api_key_count, - :external_ids + :external_ids, + :similar_users, + :similar_users_count has_one :approved_by, serializer: BasicUserSerializer, embed: :objects has_one :suspended_by, serializer: BasicUserSerializer, embed: :objects @@ -156,6 +158,28 @@ class AdminDetailedUserSerializer < AdminUserSerializer external_ids end + def similar_users + ActiveModel::ArraySerializer.new( + @options[:similar_users], + each_serializer: AdminUserListSerializer, + each_serializer: SimilarAdminUserSerializer, + scope: scope, + root: false, + ).as_json + end + + def include_similar_users? + @options[:similar_users].present? + end + + def similar_users_count + @options[:similar_users_count] + end + + def include_similar_users_count? + @options[:similar_users].present? + end + def can_delete_sso_record scope.can_delete_sso_record?(object) end diff --git a/app/serializers/similar_admin_user_serializer.rb b/app/serializers/similar_admin_user_serializer.rb new file mode 100644 index 00000000000..3f343ed8416 --- /dev/null +++ b/app/serializers/similar_admin_user_serializer.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class SimilarAdminUserSerializer < AdminUserListSerializer + attributes :can_be_suspended, + :can_be_silenced + + def can_be_suspended + scope.can_suspend?(object) + end + + def can_be_silenced + scope.can_silence_user?(object) + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5902478635f..61f0eb27035 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5581,6 +5581,15 @@ en: silenced_count: "Silenced" suspended_count: "Suspended" last_six_months: "Last 6 months" + other_matches: + one: "There is %{count} other user with the same IP address. Review and select the suspicious ones to suspend along with %{username}." + other: "There are %{count} other users with the same IP address. Review and select the suspicious ones to suspend along with %{username}." + other_matches_list: + username: "Username" + trust_level: "Trust Level" + read_time: "Read Time" + topics_entered: "Topics Entered" + posts: "Posts" tl3_requirements: title: "Requirements for Trust Level 3" table_title: diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 7acd62b9809..9417d449cb6 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -103,6 +103,18 @@ RSpec.describe Admin::UsersController do expect(response.status).to eq(200) expect(response.parsed_body["id"]).to eq(user.id) end + + it 'returns similar users' do + Fabricate(:user, ip_address: '88.88.88.88') + similar_user = Fabricate(:user, ip_address: user.ip_address) + + get "/admin/users/#{user.id}.json" + + expect(response.status).to eq(200) + expect(response.parsed_body["id"]).to eq(user.id) + expect(response.parsed_body["similar_users_count"]).to eq(1) + expect(response.parsed_body["similar_users"].map { |u| u["id"] }).to contain_exactly(similar_user.id) + end end context "when logged in as a non-staff user" do @@ -229,6 +241,7 @@ RSpec.describe Admin::UsersController do describe '#suspend' do fab!(:created_post) { Fabricate(:post) } + fab!(:other_user) { Fabricate(:user) } let(:suspend_params) do { suspend_until: 5.hours.from_now, reason: "because of this post", @@ -421,6 +434,18 @@ RSpec.describe Admin::UsersController do }, headers: { HTTP_API_KEY: api_key.key } expect(response.status).to eq(403) end + + it "can silence multiple users" do + put "/admin/users/#{user.id}/suspend.json", params: { + suspend_until: 10.days.from_now, + reason: "short reason", + message: "long reason", + other_user_ids: [other_user.id], + } + expect(response.status).to eq(200) + expect(user.reload).to be_suspended + expect(other_user.reload).to be_suspended + end end context "when logged in as a moderator" do @@ -1386,6 +1411,7 @@ RSpec.describe Admin::UsersController do describe '#silence' do fab!(:reg_user) { Fabricate(:user) } + fab!(:other_user) { Fabricate(:user) } context "when logged in as an admin" do before { sign_in(admin) } @@ -1471,6 +1497,13 @@ RSpec.describe Admin::UsersController do ) ) end + + it "can silence multiple users" do + put "/admin/users/#{reg_user.id}/silence.json", params: { other_user_ids: [other_user.id] } + expect(response.status).to eq(200) + expect(reg_user.reload).to be_silenced + expect(other_user.reload).to be_silenced + end end context "when logged in as a moderator" do