diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-09 12:10:14 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-09 12:10:14 +0300 |
commit | 8b50d36626f3a71a2d8552a316d700510559b0de (patch) | |
tree | 154f4b0391d0943438f577559a8adad32885bfe1 /spec | |
parent | 7a544c9ef1136ce0b52e269f54ebe74d0f26ad3d (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/import_export/all_models.yml | 2 | ||||
-rw-r--r-- | spec/models/concerns/spammable_spec.rb | 37 | ||||
-rw-r--r-- | spec/models/note_spec.rb | 101 | ||||
-rw-r--r-- | spec/models/sent_notification_spec.rb | 19 | ||||
-rw-r--r-- | spec/services/notes/create_service_spec.rb | 29 | ||||
-rw-r--r-- | spec/services/notes/update_service_spec.rb | 10 | ||||
-rw-r--r-- | spec/services/spam/spam_action_service_spec.rb | 34 |
7 files changed, 222 insertions, 10 deletions
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 13330f56a31..f4f13896837 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -94,6 +94,7 @@ notes: - diff_note_positions - review - note_metadata +- user_agent_detail note_metadata: - note - email_participant @@ -112,6 +113,7 @@ commit_notes: - diff_note_positions - review - note_metadata +- user_agent_detail label_links: - target - label diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 44cf87aa1c1..457c831e27c 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Spammable do +RSpec.describe Spammable, feature_category: :instance_resiliency do let(:issue) { create(:issue, description: 'Test Desc.') } describe 'Associations' do @@ -59,6 +59,18 @@ RSpec.describe Spammable do end end + context 'when the model needs recaptcha but does not support it' do + subject { invalidate_if_spam(needs_recaptcha: true) } + + before do + allow(issue).to receive(:supports_recaptcha?).and_return(false) + end + + it 'has an error that discards the spammable' do + expect(subject.errors.messages[:base]).to match_array /has been discarded/ + end + end + context 'if the model is spam and also needs recaptcha' do subject { invalidate_if_spam(is_spam: true, needs_recaptcha: true) } @@ -112,11 +124,26 @@ RSpec.describe Spammable do end describe '#needs_recaptcha!' do - it 'adds `needs_recaptcha` flag' do - issue.needs_recaptcha! + context 'when recaptcha is supported' do + it 'adds `needs_recaptcha` flag' do + issue.needs_recaptcha! + + expect(issue.spam).to be_falsey + expect(issue.needs_recaptcha).to be_truthy + end + end - expect(issue.spam).to be_falsey - expect(issue.needs_recaptcha).to be_truthy + context 'when recaptcha is not supported' do + before do + allow(issue).to receive(:supports_recaptcha?).and_return(false) + end + + it 'marks the object as spam' do + issue.needs_recaptcha! + + expect(issue.spam).to be_truthy + expect(issue.needs_recaptcha).to be_falsey + end end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 1453ce9709f..f82235916f2 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -933,6 +933,107 @@ RSpec.describe Note, feature_category: :team_planning do end end + describe '#check_for_spam' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:group) { create(:group, :public) } + let(:issue) { create(:issue, project: project) } + let(:note) { create(:note, note: "test", noteable: issue, project: project) } + let(:note_text) { 'content changed' } + + subject do + note.assign_attributes(note: note_text) + note.check_for_spam?(user: note.author) + end + + before do + allow(issue).to receive(:group).and_return(group) + end + + context 'when note is public' do + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when note is public and spammable attributes are not changed' do + let(:note_text) { 'test' } + + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when project does not exist' do + before do + allow(note).to receive(:project).and_return(nil) + end + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when project is not public' do + before do + allow(project).to receive(:public?).and_return(false) + end + + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when group is not public' do + before do + allow(group).to receive(:public?).and_return(false) + end + + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when note is confidential' do + before do + allow(note).to receive(:confidential?).and_return(true) + end + + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when noteable is confidential' do + before do + allow(issue).to receive(:confidential?).and_return(true) + end + + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when noteable is not public' do + before do + allow(issue).to receive(:public?).and_return(false) + end + + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when note is a system note' do + before do + allow(note).to receive(:system?).and_return(true) + end + + it 'returns false' do + is_expected.to be_falsey + end + end + end + describe ".grouped_diff_discussions" do let!(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb index 8194150532d..5b31e8e5e3c 100644 --- a/spec/models/sent_notification_spec.rb +++ b/spec/models/sent_notification_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe SentNotification do +RSpec.describe SentNotification, :request_store do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } @@ -46,21 +46,31 @@ RSpec.describe SentNotification do end end + shared_examples 'a non-sticky write' do + it 'writes without sticking to primary' do + subject + + expect(Gitlab::Database::LoadBalancing::Session.current.use_primary?).to be false + end + end + describe '.record' do - let(:issue) { create(:issue) } + let_it_be(:issue) { create(:issue) } subject { described_class.record(issue, user.id) } it_behaves_like 'a successful sent notification' + it_behaves_like 'a non-sticky write' end describe '.record_note' do subject { described_class.record_note(note, note.author.id) } context 'for a discussion note' do - let(:note) { create(:diff_note_on_merge_request) } + let_it_be(:note) { create(:diff_note_on_merge_request) } it_behaves_like 'a successful sent notification' + it_behaves_like 'a non-sticky write' it 'sets in_reply_to_discussion_id' do expect(subject.in_reply_to_discussion_id).to eq(note.discussion_id) @@ -68,9 +78,10 @@ RSpec.describe SentNotification do end context 'for an individual note' do - let(:note) { create(:note_on_merge_request) } + let_it_be(:note) { create(:note_on_merge_request) } it_behaves_like 'a successful sent notification' + it_behaves_like 'a non-sticky write' it 'sets in_reply_to_discussion_id' do expect(subject.in_reply_to_discussion_id).to eq(note.discussion_id) diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 5a5e8691a76..89114bf3fd0 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -30,6 +30,24 @@ RSpec.describe Notes::CreateService, feature_category: :team_planning do expect(note).to be_persisted end + it 'checks for spam' do + expect_next_instance_of(Note) do |instance| + expect(instance).to receive(:check_for_spam).with(action: :create, user: user) + end + + note + end + + it 'does not persist when spam' do + expect_next_instance_of(Note) do |instance| + expect(instance).to receive(:check_for_spam).with(action: :create, user: user) do + instance.spam! + end + end + + expect(note).not_to be_persisted + end + context 'with internal parameter' do context 'when confidential' do let(:opts) { base_opts.merge(internal: true) } @@ -511,7 +529,7 @@ RSpec.describe Notes::CreateService, feature_category: :team_planning do end end - context 'when note only have commands' do + context 'when note only has commands' do it 'adds commands applied message to note errors' do note_text = %(/close) service = double(:service) @@ -540,6 +558,15 @@ RSpec.describe Notes::CreateService, feature_category: :team_planning do expect(note.errors[:commands_only]).to contain_exactly('Closed this issue. Could not apply reopen command.') end + + it 'does not check for spam' do + expect_next_instance_of(Note) do |instance| + expect(instance).not_to receive(:check_for_spam).with(action: :create, user: user) + end + + note_text = %(/close) + described_class.new(project, user, opts.merge(note: note_text)).execute + end end end diff --git a/spec/services/notes/update_service_spec.rb b/spec/services/notes/update_service_spec.rb index 808ab46ef99..01b61acb476 100644 --- a/spec/services/notes/update_service_spec.rb +++ b/spec/services/notes/update_service_spec.rb @@ -90,6 +90,11 @@ RSpec.describe Notes::UpdateService, feature_category: :team_planning do end end + it 'checks for spam' do + expect(note).to receive(:check_for_spam).with(action: :update, user: user) + edit_note_text + end + context 'when quick action only update' do it "delete note and return commands_only error" do updated_note = described_class.new(project, user, { note: "/close\n" }).execute(note) @@ -118,6 +123,11 @@ RSpec.describe Notes::UpdateService, feature_category: :team_planning do expect { does_not_edit_note_text }.not_to change { note.reload.updated_by } end end + + it 'does not check for spam' do + expect(note).not_to receive(:check_for_spam) + does_not_edit_note_text + end end context 'when the notable is a merge request' do diff --git a/spec/services/spam/spam_action_service_spec.rb b/spec/services/spam/spam_action_service_spec.rb index 7072f5879ea..15cb4977b61 100644 --- a/spec/services/spam/spam_action_service_spec.rb +++ b/spec/services/spam/spam_action_service_spec.rb @@ -55,6 +55,21 @@ RSpec.describe Spam::SpamActionService, feature_category: :instance_resiliency d expect(issue).not_to be_spam end end + + context 'when user is nil' do + let(:spam_params) { true } + let(:user) { nil } + let(:expected_service_user_not_present_message) do + /Skipped spam check because user was not present/ + end + + it "returns success with a messaage" do + response = subject + + expect(response.message).to match(expected_service_user_not_present_message) + expect(issue).not_to be_spam + end + end end shared_examples 'allows user' do @@ -120,6 +135,7 @@ RSpec.describe Spam::SpamActionService, feature_category: :instance_resiliency d before do allow(Captcha::CaptchaVerificationService).to receive(:new).with(spam_params: spam_params) { fake_captcha_verification_service } allow(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args).and_return(fake_verdict_service) + allow(fake_verdict_service).to receive(:execute).and_return({}) end context 'when captcha response verification returns true' do @@ -170,6 +186,24 @@ RSpec.describe Spam::SpamActionService, feature_category: :instance_resiliency d target.description = 'Lovely Spam! Wonderful Spam!' end + context 'when captcha is not supported' do + before do + allow(target).to receive(:supports_recaptcha?).and_return(false) + end + + it "does not execute with captcha support" do + expect(Captcha::CaptchaVerificationService).not_to receive(:new) + + subject + end + + it "executes a spam check" do + expect(fake_verdict_service).to receive(:execute) + + subject + end + end + context 'when user is a gitlab bot' do before do allow(user).to receive(:gitlab_bot?).and_return(true) |