diff --git a/app/assets/javascripts/admin/abuse_report/components/report_actions.vue b/app/assets/javascripts/admin/abuse_report/components/report_actions.vue index 560d733c10cebc73161068291b2f42929b63aecf..e005e183c9fc82913c3b2c087f37f745934a0ba6 100644 --- a/app/assets/javascripts/admin/abuse_report/components/report_actions.vue +++ b/app/assets/javascripts/admin/abuse_report/components/report_actions.vue @@ -14,8 +14,10 @@ import { DRAWER_Z_INDEX } from '~/lib/utils/constants'; import { ACTIONS_I18N, NO_ACTION, + TRUST_ACTION, USER_ACTION_OPTIONS, REASON_OPTIONS, + TRUST_REASON, STATUS_OPEN, SUCCESS_ALERT, FAILED_ALERT, @@ -77,6 +79,16 @@ export default { userActionOptions() { return this.isNotCurrentUser ? USER_ACTION_OPTIONS : [NO_ACTION]; }, + reasonOptions() { + if (!this.isNotCurrentUser) { + return []; + } + + if (this.form.user_action === TRUST_ACTION.value) { + return [TRUST_REASON]; + } + return REASON_OPTIONS; + }, }, methods: { toggleActionsDrawer() { @@ -120,7 +132,6 @@ export default { }, }, i18n: ACTIONS_I18N, - reasonOptions: REASON_OPTIONS, DRAWER_Z_INDEX, }; @@ -173,7 +184,7 @@ export default { id="reason" v-model="form.reason" data-testid="reason-select" - :options="$options.reasonOptions" + :options="reasonOptions" :state="validationState.reason" @change="validateReason" /> diff --git a/app/assets/javascripts/admin/abuse_report/constants.js b/app/assets/javascripts/admin/abuse_report/constants.js index 1ecef44ab8f7ba7d7979dc44337de9db23db1903..94ef911e8532a175ce4a405e48fc64c9732d133f 100644 --- a/app/assets/javascripts/admin/abuse_report/constants.js +++ b/app/assets/javascripts/admin/abuse_report/constants.js @@ -25,11 +25,14 @@ export const ACTIONS_I18N = { }; export const NO_ACTION = { value: '', text: s__('AbuseReport|No action') }; +export const TRUST_REASON = { value: 'trusted', text: s__(`AbuseReport|Confirmed trusted user`) }; +export const TRUST_ACTION = { value: 'trust_user', text: s__('AbuseReport|Trust user') }; export const USER_ACTION_OPTIONS = [ NO_ACTION, { value: 'block_user', text: s__('AbuseReport|Block user') }, { value: 'ban_user', text: s__('AbuseReport|Ban user') }, + TRUST_ACTION, { value: 'delete_user', text: s__('AbuseReport|Delete user') }, ]; diff --git a/app/helpers/resource_events/abuse_report_events_helper.rb b/app/helpers/resource_events/abuse_report_events_helper.rb index 8adbc8911840020232594565ac826874a640a88c..207ec73454b2ad7a400bd6fc7933dcf16594633f 100644 --- a/app/helpers/resource_events/abuse_report_events_helper.rb +++ b/app/helpers/resource_events/abuse_report_events_helper.rb @@ -10,6 +10,8 @@ def success_message_for_action(action) s_('AbuseReportEvent|Successfully blocked the user') when 'delete_user' s_('AbuseReportEvent|Successfully scheduled the user for deletion') + when 'trust_user' + s_('AbuseReportEvent|Successfully trusted the user') when 'close_report' s_('AbuseReportEvent|Successfully closed the report') when 'ban_user_and_close_report' @@ -18,6 +20,8 @@ def success_message_for_action(action) s_('AbuseReportEvent|Successfully blocked the user and closed the report') when 'delete_user_and_close_report' s_('AbuseReportEvent|Successfully scheduled the user for deletion and closed the report') + when 'trust_user_and_close_report' + s_('AbuseReportEvent|Successfully trusted the user and closed the report') end end end diff --git a/app/models/resource_events/abuse_report_event.rb b/app/models/resource_events/abuse_report_event.rb index 59f88a63998eec8a55449305955a76b790028f8a..5881f87241d8a837f7869c195b054ae8e3822455 100644 --- a/app/models/resource_events/abuse_report_event.rb +++ b/app/models/resource_events/abuse_report_event.rb @@ -16,7 +16,9 @@ class AbuseReportEvent < ApplicationRecord close_report: 4, ban_user_and_close_report: 5, block_user_and_close_report: 6, - delete_user_and_close_report: 7 + delete_user_and_close_report: 7, + trust_user: 8, + trust_user_and_close_report: 9 } enum reason: { @@ -28,7 +30,8 @@ class AbuseReportEvent < ApplicationRecord copyright: 6, malware: 7, other: 8, - unconfirmed: 9 + unconfirmed: 9, + trusted: 10 } def success_message diff --git a/app/models/user.rb b/app/models/user.rb index e17803af1353b0d9157a5e827698788b0a3cd1a7..b76d19240f8206452279a8bccda76eac171737f9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -271,6 +271,7 @@ def update_tracked_fields!(request) has_many :bulk_imports has_many :custom_attributes, class_name: 'UserCustomAttribute' + has_one :trusted_with_spam_attribute, -> { UserCustomAttribute.trusted_with_spam }, class_name: 'UserCustomAttribute' has_many :callouts, class_name: 'Users::Callout' has_many :group_callouts, class_name: 'Users::GroupCallout' has_many :project_callouts, class_name: 'Users::ProjectCallout' @@ -2223,8 +2224,8 @@ def abuse_metadata } end - def allow_possible_spam? - custom_attributes.by_key(UserCustomAttribute::ALLOW_POSSIBLE_SPAM).exists? + def trusted? + trusted_with_spam_attribute.present? end def namespace_commit_email_for_namespace(namespace) diff --git a/app/models/user_custom_attribute.rb b/app/models/user_custom_attribute.rb index 15d50071bf6d8a0bef857ae665c2c0a2379e58cd..b2674cb4e88f3c6e3a622b4a59906df97f68b501 100644 --- a/app/models/user_custom_attribute.rb +++ b/app/models/user_custom_attribute.rb @@ -10,13 +10,14 @@ class UserCustomAttribute < ApplicationRecord scope :by_user_id, ->(user_id) { where(user_id: user_id) } scope :by_updated_at, ->(updated_at) { where(updated_at: updated_at) } scope :arkose_sessions, -> { by_key('arkose_session') } + scope :trusted_with_spam, -> { by_key(TRUSTED_BY) } BLOCKED_BY = 'blocked_by' UNBLOCKED_BY = 'unblocked_by' ARKOSE_RISK_BAND = 'arkose_risk_band' AUTO_BANNED_BY_ABUSE_REPORT_ID = 'auto_banned_by_abuse_report_id' AUTO_BANNED_BY_SPAM_LOG_ID = 'auto_banned_by_spam_log_id' - ALLOW_POSSIBLE_SPAM = 'allow_possible_spam' + TRUSTED_BY = 'trusted_by' IDENTITY_VERIFICATION_PHONE_EXEMPT = 'identity_verification_phone_exempt' class << self @@ -50,6 +51,17 @@ def set_banned_by_spam_log(spam_log) return unless spam_log custom_attribute = { user_id: spam_log.user_id, key: AUTO_BANNED_BY_SPAM_LOG_ID, value: spam_log.id } + upsert_custom_attributes([custom_attribute]) + end + + def set_trusted_by(user:, trusted_by:) + return unless user && trusted_by + + custom_attribute = { + user_id: user.id, + key: UserCustomAttribute::TRUSTED_BY, + value: "#{trusted_by.username}/#{trusted_by.id}+#{Time.current}" + } upsert_custom_attributes([custom_attribute]) end diff --git a/app/services/admin/abuse_reports/moderate_user_service.rb b/app/services/admin/abuse_reports/moderate_user_service.rb index 823568d9db87104e253a1ff305db99c1f69a002d..1e14806c694998a19b7abd7212cc4b826b1f96e2 100644 --- a/app/services/admin/abuse_reports/moderate_user_service.rb +++ b/app/services/admin/abuse_reports/moderate_user_service.rb @@ -42,6 +42,7 @@ def perform_action when :block_user then block_user when :delete_user then delete_user when :close_report then close_report + when :trust_user then trust_user end end @@ -66,6 +67,10 @@ def close_report success end + def trust_user + Users::TrustService.new(current_user).execute(abuse_report.user) + end + def close_similar_open_reports # admins see the abuse report and other open reports for the same user in one page # hence, if the request is to close the report, close other open reports for the same user too diff --git a/app/services/spam/spam_verdict_service.rb b/app/services/spam/spam_verdict_service.rb index 9efe51b43b815ee3c03b7f5351602dd37471d14f..2d4bebc8b2bb6065253ca1cbc35b26ad7fc49449 100644 --- a/app/services/spam/spam_verdict_service.rb +++ b/app/services/spam/spam_verdict_service.rb @@ -90,7 +90,7 @@ def override_via_allow_possible_spam?(verdict:) end def allow_possible_spam? - target.allow_possible_spam?(user) || user.allow_possible_spam? + target.allow_possible_spam?(user) || user.trusted? end def spamcheck_client diff --git a/app/services/users/allow_possible_spam_service.rb b/app/services/users/allow_possible_spam_service.rb deleted file mode 100644 index d9273fe0fc1cf462e7fd116870de64ff1a24507a..0000000000000000000000000000000000000000 --- a/app/services/users/allow_possible_spam_service.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -module Users - class AllowPossibleSpamService < BaseService - def initialize(current_user) - @current_user = current_user - end - - def execute(user) - custom_attribute = { - user_id: user.id, - key: UserCustomAttribute::ALLOW_POSSIBLE_SPAM, - value: "#{current_user.username}/#{current_user.id}+#{Time.current}" - } - UserCustomAttribute.upsert_custom_attributes([custom_attribute]) - end - end -end diff --git a/app/services/users/disallow_possible_spam_service.rb b/app/services/users/trust_service.rb similarity index 53% rename from app/services/users/disallow_possible_spam_service.rb rename to app/services/users/trust_service.rb index e31ba7ddff02a481ef0d0cd1b11beedb2e2336bd..faf0b9c40eabc0a6f4741b2fd5e2ba1048f0d929 100644 --- a/app/services/users/disallow_possible_spam_service.rb +++ b/app/services/users/trust_service.rb @@ -1,13 +1,14 @@ # frozen_string_literal: true module Users - class DisallowPossibleSpamService < BaseService + class TrustService < BaseService def initialize(current_user) @current_user = current_user end def execute(user) - user.custom_attributes.by_key(UserCustomAttribute::ALLOW_POSSIBLE_SPAM).delete_all + UserCustomAttribute.set_trusted_by(user: user, trusted_by: @current_user) + success end end end diff --git a/app/services/users/untrust_service.rb b/app/services/users/untrust_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..aa5de71b97f8817a00c530713cef6fbff6ddce38 --- /dev/null +++ b/app/services/users/untrust_service.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Users + class UntrustService < BaseService + def initialize(current_user) + @current_user = current_user + end + + def execute(user) + user.trusted_with_spam_attribute.delete + success + end + end +end diff --git a/doc/administration/review_abuse_reports.md b/doc/administration/review_abuse_reports.md index 4ff53a4e1b0a2d2342123668722b78c92b640133..84bb7ab219f4be7ed387ed5193aee88c9e089b7d 100644 --- a/doc/administration/review_abuse_reports.md +++ b/doc/administration/review_abuse_reports.md @@ -32,13 +32,15 @@ To find out more about reporting abuse, see ## Resolving abuse reports +> **Trust user** [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131102) in GitLab 16.4. + To access abuse reports: 1. On the left sidebar, select **Search or go to**. 1. Select **Admin Area**. 1. Select **Abuse Reports**. -There are 3 ways to resolve an abuse report, with a button for each method: +There are four ways to resolve an abuse report, with a button for each method: - Remove user & report. This: - [Deletes the reported user](../user/profile/account/delete_account.md) from the @@ -48,6 +50,9 @@ There are 3 ways to resolve an abuse report, with a button for each method: - Remove report. This: - Removes the abuse report from the list. - Removes access restrictions for the reported user. +- Trust user. This: + - Allows the user to create issues, notes, snippets, and merge requests without being blocked for spam. + - Prevents abuse reports from being created for this user. The following is an example of the **Abuse Reports** page: diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d4e5fe8d58a6b4f3dde8e00618506b68c7b11afe..ff23f14ecb398e488a8431a8488bcc77bb11858f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2239,6 +2239,12 @@ msgstr "" msgid "AbuseReportEvent|Successfully scheduled the user for deletion and closed the report" msgstr "" +msgid "AbuseReportEvent|Successfully trusted the user" +msgstr "" + +msgid "AbuseReportEvent|Successfully trusted the user and closed the report" +msgstr "" + msgid "AbuseReports|%{reportedUser} reported for %{category} by %{count} users" msgstr "" @@ -2308,6 +2314,9 @@ msgstr "" msgid "AbuseReport|Confirmed spam" msgstr "" +msgid "AbuseReport|Confirmed trusted user" +msgstr "" + msgid "AbuseReport|Confirmed violation of a copyright or a trademark" msgstr "" @@ -2428,6 +2437,9 @@ msgstr "" msgid "AbuseReport|Tier" msgstr "" +msgid "AbuseReport|Trust user" +msgstr "" + msgid "AbuseReport|Verification" msgstr "" diff --git a/spec/factories/users.rb b/spec/factories/users.rb index d61d5cc2d78f9bb9c60b7f75fdbfbcb209ebf065..de2b5159fe710743273ef3b2dd2628a2765e3fe0 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -48,6 +48,15 @@ after(:build) { |user, _| user.ban! } end + trait :trusted do + after(:create) do |user, _| + user.custom_attributes.create!( + key: UserCustomAttribute::TRUSTED_BY, + value: "placeholder" + ) + end + end + trait :ldap_blocked do after(:build) { |user, _| user.ldap_block! } end diff --git a/spec/frontend/admin/abuse_report/components/report_actions_spec.js b/spec/frontend/admin/abuse_report/components/report_actions_spec.js index 0e20630db1435af9a635728be02dff1d03d4302f..3c366980c14ba5ccdc3cc5dffbf20e8e7b644616 100644 --- a/spec/frontend/admin/abuse_report/components/report_actions_spec.js +++ b/spec/frontend/admin/abuse_report/components/report_actions_spec.js @@ -17,6 +17,9 @@ import { ERROR_MESSAGE, NO_ACTION, USER_ACTION_OPTIONS, + TRUST_ACTION, + TRUST_REASON, + REASON_OPTIONS, } from '~/admin/abuse_report/constants'; import { mockAbuseReport } from '../mock_data'; @@ -40,10 +43,11 @@ describe('ReportActions', () => { const setCloseReport = (close) => wrapper.findByTestId('close').find('input').setChecked(close); const setSelectOption = (id, value) => wrapper.findByTestId(`${id}-select`).find(`option[value=${value}]`).setSelected(); - const selectAction = (action) => setSelectOption('action', action); + const selectAction = (chosenAction) => setSelectOption('action', chosenAction); const selectReason = (reason) => setSelectOption('reason', reason); const setComment = (comment) => wrapper.findByTestId('comment').find('input').setValue(comment); const submitForm = () => wrapper.findByTestId('submit-button').vm.$emit('click'); + const findReasonOptions = () => wrapper.findByTestId('reason-select'); const createComponent = (props = {}) => { wrapper = mountExtended(ReportActions, { @@ -79,8 +83,8 @@ describe('ReportActions', () => { expect(options).toHaveLength(USER_ACTION_OPTIONS.length); - USER_ACTION_OPTIONS.forEach((action, index) => { - expect(options.at(index).text()).toBe(action.text); + USER_ACTION_OPTIONS.forEach((userAction, index) => { + expect(options.at(index).text()).toBe(userAction.text); }); }); }); @@ -100,6 +104,51 @@ describe('ReportActions', () => { }); }); + describe('reasons', () => { + beforeEach(() => { + clickActionsButton(); + }); + + it('shows all non-trust reasons by default', () => { + const reasons = findReasonOptions().findAll('option'); + expect(reasons).toHaveLength(REASON_OPTIONS.length); + + REASON_OPTIONS.forEach((reason, index) => { + expect(reasons.at(index).text()).toBe(reason.text); + }); + }); + + describe('when user selects any non-trust action', () => { + it('shows non-trust reasons', () => { + const reasonLength = REASON_OPTIONS.length; + let reasons; + + USER_ACTION_OPTIONS.forEach((userAction) => { + if (userAction !== TRUST_ACTION && userAction !== NO_ACTION) { + selectAction(userAction.value); + + reasons = findReasonOptions().findAll('option'); + expect(reasons).toHaveLength(reasonLength); + } + }); + }); + }); + + describe('when user selects "Trust user"', () => { + beforeEach(() => { + selectAction(TRUST_ACTION.value); + }); + + it('only shows "Confirmed trusted user" reason', () => { + const reasons = findReasonOptions().findAll('option'); + + expect(reasons).toHaveLength(1); + + expect(reasons.at(0).text()).toBe(TRUST_REASON.text); + }); + }); + }); + describe('when clicking the actions button', () => { beforeEach(() => { clickActionsButton(); diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c611c3c26e312f019d86d6752c9139f4f1d05b91..d35449ca8c8331b6131c5b0375cbef54c0d1b5c7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -6120,25 +6120,23 @@ def add_user(access) end end - describe '#allow_possible_spam?' do + describe '#trusted?' do context 'when no custom attribute is set' do - it 'is false' do - expect(user.allow_possible_spam?).to be_falsey + it 'is falsey' do + expect(user.trusted?).to be_falsey end end context 'when the custom attribute is set' do before do - user.custom_attributes.upsert_custom_attributes( - [{ - user_id: user.id, - key: UserCustomAttribute::ALLOW_POSSIBLE_SPAM, - value: "test" - }]) + user.custom_attributes.create!( + key: UserCustomAttribute::TRUSTED_BY, + value: "test" + ) end - it '#allow_possible_spam? is true' do - expect(user.allow_possible_spam?).to be_truthy + it 'is truthy' do + expect(user.trusted?).to be_truthy end end end diff --git a/spec/services/admin/abuse_reports/moderate_user_service_spec.rb b/spec/services/admin/abuse_reports/moderate_user_service_spec.rb index 7e08db2b6123ab938df7b787c5da42a566689d2a..3b80d3276a20939a3af3045c5ae9b61094d86f3c 100644 --- a/spec/services/admin/abuse_reports/moderate_user_service_spec.rb +++ b/spec/services/admin/abuse_reports/moderate_user_service_spec.rb @@ -210,6 +210,43 @@ end end + describe 'when trusting the user' do + let(:action) { 'trust_user' } + + it 'calls the Users::TrustService method' do + expect_next_instance_of(Users::TrustService, admin) do |service| + expect(service).to receive(:execute).with(abuse_report.user).and_return(status: :success) + end + + subject + end + + context 'when not closing the report' do + let(:close) { false } + + it_behaves_like 'does not close the report' + it_behaves_like 'records an event', action: 'trust_user' + end + + context 'when closing the report' do + it_behaves_like 'closes the report' + it_behaves_like 'records an event', action: 'trust_user_and_close_report' + end + + context 'when trusting the user fails' do + before do + allow_next_instance_of(Users::TrustService) do |service| + allow(service).to receive(:execute).with(abuse_report.user) + .and_return(status: :error, message: 'Trusting the user failed') + end + end + + it_behaves_like 'returns an error response', 'Trusting the user failed' + it_behaves_like 'does not close the report' + it_behaves_like 'does not record an event' + end + end + describe 'when only closing the report' do let(:action) { '' } diff --git a/spec/services/spam/spam_verdict_service_spec.rb b/spec/services/spam/spam_verdict_service_spec.rb index 70f43d82ead04b5d063b12296a55b735fa08a1b1..361742699b0cbe25d6f4df4c4e13b25dabdf3edb 100644 --- a/spec/services/spam/spam_verdict_service_spec.rb +++ b/spec/services/spam/spam_verdict_service_spec.rb @@ -31,7 +31,7 @@ end let(:check_for_spam) { true } - let_it_be(:user) { create(:user) } + let_it_be_with_reload(:user) { create(:user) } let_it_be(:issue) { create(:issue, author: user) } let_it_be(:snippet) { create(:personal_snippet, :public, author: user) } @@ -136,15 +136,9 @@ end end - context 'if allow_possible_spam user custom attribute is set' do + context 'if user is trusted to create possible spam' do before do - UserCustomAttribute.upsert_custom_attributes( - [{ - user_id: user.id, - key: 'allow_possible_spam', - value: 'does not matter' - }] - ) + user.custom_attributes.create!(key: 'trusted_by', value: 'does not matter') end context 'and a service returns a verdict that should be overridden' do diff --git a/spec/services/users/allow_possible_spam_service_spec.rb b/spec/services/users/trust_service_spec.rb similarity index 80% rename from spec/services/users/allow_possible_spam_service_spec.rb rename to spec/services/users/trust_service_spec.rb index 53618f0c8e961463b0e82b1fe5644e4b35758abe..1f71992ce9b38e78c8de948301fdd44294a578de 100644 --- a/spec/services/users/allow_possible_spam_service_spec.rb +++ b/spec/services/users/trust_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Users::AllowPossibleSpamService, feature_category: :user_management do +RSpec.describe Users::TrustService, feature_category: :user_management do let_it_be(:current_user) { create(:admin) } subject(:service) { described_class.new(current_user) } @@ -18,7 +18,7 @@ operation user.reload - expect(user.custom_attributes.by_key(UserCustomAttribute::ALLOW_POSSIBLE_SPAM)).to be_present + expect(user.custom_attributes.by_key(UserCustomAttribute::TRUSTED_BY)).to be_present end end end diff --git a/spec/services/users/disallow_possible_spam_service_spec.rb b/spec/services/users/untrust_service_spec.rb similarity index 66% rename from spec/services/users/disallow_possible_spam_service_spec.rb rename to spec/services/users/untrust_service_spec.rb index 32a47e05525a7b5f66659aa3711503d2f17dd5fa..054cb9b82dc9bce3e703a01424238f1143eebe7a 100644 --- a/spec/services/users/disallow_possible_spam_service_spec.rb +++ b/spec/services/users/untrust_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Users::DisallowPossibleSpamService, feature_category: :user_management do +RSpec.describe Users::UntrustService, feature_category: :user_management do let_it_be(:current_user) { create(:admin) } subject(:service) { described_class.new(current_user) } @@ -16,19 +16,19 @@ UserCustomAttribute.upsert_custom_attributes( [{ user_id: user.id, - key: :allow_possible_spam, + key: UserCustomAttribute::TRUSTED_BY, value: 'not important' }] ) end it 'updates the custom attributes', :aggregate_failures do - expect(user.custom_attributes.by_key(UserCustomAttribute::ALLOW_POSSIBLE_SPAM)).to be_present + expect(user.trusted_with_spam_attribute).to be_present operation user.reload - expect(user.custom_attributes).to be_empty + expect(user.trusted_with_spam_attribute).to be nil end end end