diff options
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 188 | ||||
-rw-r--r-- | spec/controllers/projects/snippets_controller_spec.rb | 6 | ||||
-rw-r--r-- | spec/controllers/snippets_controller_spec.rb | 8 | ||||
-rw-r--r-- | spec/features/issues/spam_issues_spec.rb | 48 | ||||
-rw-r--r-- | spec/features/snippets/spam_snippets_spec.rb | 73 | ||||
-rw-r--r-- | spec/requests/api/issues/post_projects_issues_spec.rb | 60 | ||||
-rw-r--r-- | spec/requests/api/issues/put_projects_issues_spec.rb | 61 | ||||
-rw-r--r-- | spec/requests/api/project_snippets_spec.rb | 6 | ||||
-rw-r--r-- | spec/requests/api/snippets_spec.rb | 7 | ||||
-rw-r--r-- | spec/services/create_snippet_service_spec.rb | 72 | ||||
-rw-r--r-- | spec/services/issues/create_service_spec.rb | 62 | ||||
-rw-r--r-- | spec/services/spam_service_spec.rb | 48 | ||||
-rw-r--r-- | spec/support/matchers/log_spam.rb | 34 |
13 files changed, 515 insertions, 158 deletions
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index ad57c29850b..2edc0aa5536 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -408,6 +408,7 @@ describe Projects::IssuesController do context 'when user has access to update issue' do before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) project.add_developer(user) end @@ -421,14 +422,30 @@ describe Projects::IssuesController do context 'when Akismet is enabled and the issue is identified as spam' do before do stub_application_setting(recaptcha_enabled: true) - allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: true) + end + end + + context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end + + it 'renders json with recaptcha_html' do + subject + + expect(json_response).to have_key('recaptcha_html') + end end - it 'renders json with recaptcha_html' do - subject + context 'when allow_possible_spam feature flag is true' do + it 'updates the issue' do + subject - expect(json_response).to have_key('recaptcha_html') + expect(response).to have_http_status(:ok) + expect(issue.reload.title).to eq('New title') + end end end end @@ -681,13 +698,13 @@ describe Projects::IssuesController do before do project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) stub_application_setting(recaptcha_enabled: true) - allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) end context 'when an issue is not identified as spam' do before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: false) + end end it 'normally updates the issue' do @@ -696,45 +713,64 @@ describe Projects::IssuesController do end context 'when an issue is identified as spam' do - before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) - end - context 'when captcha is not verified' do before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: true) + end end - it 'rejects an issue recognized as a spam' do - expect { update_issue }.not_to change { issue.reload.title } - end + context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end - it 'rejects an issue recognized as a spam when recaptcha disabled' do - stub_application_setting(recaptcha_enabled: false) + it 'rejects an issue recognized as a spam' do + expect { update_issue }.not_to change { issue.reload.title } + end - expect { update_issue }.not_to change { issue.reload.title } - end + it 'rejects an issue recognized as a spam when recaptcha disabled' do + stub_application_setting(recaptcha_enabled: false) - it 'creates a spam log' do - update_issue(issue_params: { title: 'Spam title' }) + expect { update_issue }.not_to change { issue.reload.title } + end - spam_logs = SpamLog.all + it 'creates a spam log' do + expect { update_issue(issue_params: { title: 'Spam title' }) } + .to log_spam(title: 'Spam title', noteable_type: 'Issue') + end - expect(spam_logs.count).to eq(1) - expect(spam_logs.first.title).to eq('Spam title') - expect(spam_logs.first.recaptcha_verified).to be_falsey - end + it 'renders recaptcha_html json response' do + update_issue + + expect(json_response).to have_key('recaptcha_html') + end - it 'renders recaptcha_html json response' do - update_issue + it 'returns 200 status' do + update_issue - expect(json_response).to have_key('recaptcha_html') + expect(response).to have_gitlab_http_status(200) + end end - it 'returns 200 status' do - update_issue + context 'when allow_possible_spam feature flag is true' do + it 'updates the issue recognized as spam' do + expect { update_issue }.to change { issue.reload.title } + end - expect(response).to have_gitlab_http_status(200) + it 'creates a spam log' do + expect { update_issue(issue_params: { title: 'Spam title' }) } + .to log_spam( + title: 'Spam title', description: issue.description, + noteable_type: 'Issue', recaptcha_verified: false + ) + end + + it 'returns 200 status' do + update_issue + + expect(response).to have_gitlab_http_status(200) + end end end @@ -748,11 +784,6 @@ describe Projects::IssuesController do additional_params: { spam_log_id: spam_logs.last.id, recaptcha_verification: true }) end - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha) - .and_return(true) - end - it 'returns 200 status' do expect(response).to have_gitlab_http_status(200) end @@ -917,55 +948,72 @@ describe Projects::IssuesController do context 'Akismet is enabled' do before do stub_application_setting(recaptcha_enabled: true) - allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) end context 'when an issue is not identified as spam' do before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) + stub_feature_flags(allow_possible_spam: false) + + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: false) + end end - it 'does not create an issue' do - expect { post_new_issue(title: '') }.not_to change(Issue, :count) + it 'creates an issue' do + expect { post_new_issue(title: 'Some title') }.to change(Issue, :count) end end context 'when an issue is identified as spam' do - before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) - end - context 'when captcha is not verified' do def post_spam_issue post_new_issue(title: 'Spam Title', description: 'Spam lives here') end before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: true) + end end - it 'rejects an issue recognized as a spam' do - expect { post_spam_issue }.not_to change(Issue, :count) - end + context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end - it 'creates a spam log' do - post_spam_issue - spam_logs = SpamLog.all + it 'rejects an issue recognized as a spam' do + expect { post_spam_issue }.not_to change(Issue, :count) + end - expect(spam_logs.count).to eq(1) - expect(spam_logs.first.title).to eq('Spam Title') - expect(spam_logs.first.recaptcha_verified).to be_falsey - end + it 'creates a spam log' do + expect { post_spam_issue } + .to log_spam(title: 'Spam Title', noteable_type: 'Issue', recaptcha_verified: false) + end - it 'does not create an issue when it is not valid' do - expect { post_new_issue(title: '') }.not_to change(Issue, :count) + it 'does not create an issue when it is not valid' do + expect { post_new_issue(title: '') }.not_to change(Issue, :count) + end + + it 'does not create an issue when recaptcha is not enabled' do + stub_application_setting(recaptcha_enabled: false) + + expect { post_spam_issue }.not_to change(Issue, :count) + end end - it 'does not create an issue when recaptcha is not enabled' do - stub_application_setting(recaptcha_enabled: false) + context 'when allow_possible_spam feature flag is true' do + it 'creates an issue recognized as spam' do + expect { post_spam_issue }.to change(Issue, :count) + end - expect { post_spam_issue }.not_to change(Issue, :count) + it 'creates a spam log' do + expect { post_spam_issue } + .to log_spam(title: 'Spam Title', noteable_type: 'Issue', recaptcha_verified: false) + end + + it 'does not create an issue when it is not valid' do + expect { post_new_issue(title: '') }.not_to change(Issue, :count) + end end end @@ -977,7 +1025,7 @@ describe Projects::IssuesController do end before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(true) + expect(controller).to receive_messages(verify_recaptcha: true) end it 'accepts an issue after recaptcha is verified' do @@ -1030,8 +1078,12 @@ describe Projects::IssuesController do describe 'POST #mark_as_spam' do context 'properly submits to Akismet' do before do - allow_any_instance_of(AkismetService).to receive_messages(submit_spam: true) - allow_any_instance_of(ApplicationSetting).to receive_messages(akismet_enabled: true) + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(submit_spam: true) + end + expect_next_instance_of(ApplicationSetting) do |setting| + expect(setting).to receive_messages(akismet_enabled: true) + end end def post_spam @@ -1266,7 +1318,9 @@ describe Projects::IssuesController do end it "shows error when upload fails" do - allow_any_instance_of(UploadService).to receive(:execute).and_return(nil) + expect_next_instance_of(UploadService) do |upload_service| + expect(upload_service).to receive(:execute).and_return(nil) + end import_csv diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index b13534b9088..042a5542786 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -111,7 +111,7 @@ describe Projects::SnippetsController do it 'creates a spam log' do expect { create_snippet(project, visibility_level: Snippet::PUBLIC) } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Title', user_id: user.id, noteable_type: 'ProjectSnippet') end it 'renders :new with recaptcha disabled' do @@ -192,7 +192,7 @@ describe Projects::SnippetsController do it 'creates a spam log' do expect { update_snippet(title: 'Foo') } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Foo', user_id: user.id, noteable_type: 'ProjectSnippet') end it 'renders :edit with recaptcha disabled' do @@ -237,7 +237,7 @@ describe Projects::SnippetsController do it 'creates a spam log' do expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Foo', user_id: user.id, noteable_type: 'ProjectSnippet') end it 'renders :edit with recaptcha disabled' do diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 1b3a8965342..e892c736c69 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -269,7 +269,7 @@ describe SnippetsController do it 'creates a spam log' do expect { create_snippet(visibility_level: Snippet::PUBLIC) } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Title', user: user, noteable_type: 'PersonalSnippet') end it 'renders :new with recaptcha disabled' do @@ -345,7 +345,7 @@ describe SnippetsController do it 'creates a spam log' do expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Foo', user: user, noteable_type: 'PersonalSnippet') end it 'renders :edit with recaptcha disabled' do @@ -389,8 +389,8 @@ describe SnippetsController do end it 'creates a spam log' do - expect { update_snippet(title: 'Foo') } - .to change { SpamLog.count }.by(1) + expect {update_snippet(title: 'Foo') } + .to log_spam(title: 'Foo', user: user, noteable_type: 'PersonalSnippet') end it 'renders :edit with recaptcha disabled' do diff --git a/spec/features/issues/spam_issues_spec.rb b/spec/features/issues/spam_issues_spec.rb index 0d009f47fff..18245f249fd 100644 --- a/spec/features/issues/spam_issues_spec.rb +++ b/spec/features/issues/spam_issues_spec.rb @@ -30,21 +30,47 @@ describe 'New issue', :js do visit new_project_issue_path(project) end - it 'creates an issue after solving reCaptcha' do - fill_in 'issue_title', with: 'issue title' - fill_in 'issue_description', with: 'issue description' + context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end - click_button 'Submit issue' + it 'creates an issue after solving reCaptcha' do + fill_in 'issue_title', with: 'issue title' + fill_in 'issue_description', with: 'issue description' - # it is impossible to test recaptcha automatically and there is no possibility to fill in recaptcha - # recaptcha verification is skipped in test environment and it always returns true - expect(page).not_to have_content('issue title') - expect(page).to have_css('.recaptcha') + click_button 'Submit issue' - click_button 'Submit issue' + # it is impossible to test recaptcha automatically and there is no possibility to fill in recaptcha + # recaptcha verification is skipped in test environment and it always returns true + expect(page).not_to have_content('issue title') + expect(page).to have_css('.recaptcha') - expect(page.find('.issue-details h2.title')).to have_content('issue title') - expect(page.find('.issue-details .description')).to have_content('issue description') + click_button 'Submit issue' + + expect(page.find('.issue-details h2.title')).to have_content('issue title') + expect(page.find('.issue-details .description')).to have_content('issue description') + end + end + + context 'when allow_possible_spam feature flag is true' do + before do + fill_in 'issue_title', with: 'issue title' + fill_in 'issue_description', with: 'issue description' + end + + it 'creates an issue without a need to solve reCaptcha' do + click_button 'Submit issue' + + expect(page).not_to have_css('.recaptcha') + expect(page.find('.issue-details h2.title')).to have_content('issue title') + expect(page.find('.issue-details .description')).to have_content('issue description') + end + + it 'creates a spam log record' do + expect { click_button 'Submit issue' } + .to log_spam(title: 'issue title', description: 'issue description', user_id: user.id, noteable_type: 'Issue') + end end end diff --git a/spec/features/snippets/spam_snippets_spec.rb b/spec/features/snippets/spam_snippets_spec.rb new file mode 100644 index 00000000000..3e71a4e7879 --- /dev/null +++ b/spec/features/snippets/spam_snippets_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'User creates snippet', :js do + let(:user) { create(:user) } + + before do + stub_feature_flags(allow_possible_spam: false) + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + + Gitlab::CurrentSettings.update!( + akismet_enabled: true, + akismet_api_key: 'testkey', + recaptcha_enabled: true, + recaptcha_site_key: 'test site key', + recaptcha_private_key: 'test private key' + ) + + sign_in(user) + visit new_snippet_path + + fill_in 'personal_snippet_title', with: 'My Snippet Title' + fill_in 'personal_snippet_description', with: 'My Snippet **Description**' + find('#personal_snippet_visibility_level_20').set(true) + page.within('.file-editor') do + find('.ace_text-input', visible: false).send_keys 'Hello World!' + end + end + + shared_examples 'solve recaptcha' do + it 'creates a snippet after solving reCaptcha' do + click_button('Create snippet') + wait_for_requests + + # it is impossible to test recaptcha automatically and there is no possibility to fill in recaptcha + # recaptcha verification is skipped in test environment and it always returns true + expect(page).not_to have_content('My Snippet Title') + expect(page).to have_css('.recaptcha') + click_button('Submit personal snippet') + + expect(page).to have_content('My Snippet Title') + end + end + + context 'when identified as a spam' do + before do + WebMock.stub_request(:any, /.*akismet.com.*/).to_return(body: "true", status: 200) + end + + context 'when allow_possible_spam feature flag is false' do + it_behaves_like 'solve recaptcha' + end + + context 'when allow_possible_spam feature flag is true' do + it_behaves_like 'solve recaptcha' + end + end + + context 'when not identified as a spam' do + before do + WebMock.stub_request(:any, /.*akismet.com.*/).to_return(body: "false", status: 200) + end + + it 'creates a snippet' do + click_button('Create snippet') + wait_for_requests + + expect(page).not_to have_css('.recaptcha') + expect(page).to have_content('My Snippet Title') + end + end +end diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb index b74e8867310..3a55b437ead 100644 --- a/spec/requests/api/issues/post_projects_issues_spec.rb +++ b/spec/requests/api/issues/post_projects_issues_spec.rb @@ -374,9 +374,17 @@ describe API::Issues do end describe 'POST /projects/:id/issues with spam filtering' do + def post_issue + post api("/projects/#{project.id}/issues", user), params: params + end + before do - allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) - allow_any_instance_of(AkismetService).to receive_messages(spam?: true) + expect_next_instance_of(SpamService) do |spam_service| + expect(spam_service).to receive_messages(check_for_spam?: true) + end + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: true) + end end let(:params) do @@ -387,17 +395,43 @@ describe API::Issues do } end - it 'does not create a new project issue' do - expect { post api("/projects/#{project.id}/issues", user), params: params }.not_to change(Issue, :count) - expect(response).to have_gitlab_http_status(400) - expect(json_response['message']).to eq({ 'error' => 'Spam detected' }) - - spam_logs = SpamLog.all - expect(spam_logs.count).to eq(1) - expect(spam_logs[0].title).to eq('new issue') - expect(spam_logs[0].description).to eq('content here') - expect(spam_logs[0].user).to eq(user) - expect(spam_logs[0].noteable_type).to eq('Issue') + context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end + + it 'does not create a new project issue' do + expect { post_issue }.not_to change(Issue, :count) + end + + it 'returns correct status and message' do + post_issue + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq({ 'error' => 'Spam detected' }) + end + + it 'creates a new spam log entry' do + expect { post_issue } + .to log_spam(title: 'new issue', description: 'content here', user_id: user.id, noteable_type: 'Issue') + end + end + + context 'when allow_possible_spam feature flag is true' do + it 'does creates a new project issue' do + expect { post_issue }.to change(Issue, :count).by(1) + end + + it 'returns correct status' do + post_issue + + expect(response).to have_gitlab_http_status(201) + end + + it 'creates a new spam log entry' do + expect { post_issue } + .to log_spam(title: 'new issue', description: 'content here', user_id: user.id, noteable_type: 'Issue') + end end end diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index 267cba93713..43f302ed194 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -181,6 +181,10 @@ describe API::Issues do end describe 'PUT /projects/:id/issues/:issue_iid with spam filtering' do + def update_issue + put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: params + end + let(:params) do { title: 'updated title', @@ -189,21 +193,52 @@ describe API::Issues do } end - it 'does not create a new project issue' do - allow_any_instance_of(SpamService).to receive_messages(check_for_spam?: true) - allow_any_instance_of(AkismetService).to receive_messages(spam?: true) + before do + expect_next_instance_of(SpamService) do |spam_service| + expect(spam_service).to receive_messages(check_for_spam?: true) + end + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: true) + end + end - put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: params + context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end - expect(response).to have_gitlab_http_status(400) - expect(json_response['message']).to eq({ 'error' => 'Spam detected' }) - - spam_logs = SpamLog.all - expect(spam_logs.count).to eq(1) - expect(spam_logs[0].title).to eq('updated title') - expect(spam_logs[0].description).to eq('content here') - expect(spam_logs[0].user).to eq(user) - expect(spam_logs[0].noteable_type).to eq('Issue') + it 'does not update a project issue' do + expect { update_issue }.not_to change { issue.reload.title } + end + + it 'returns correct status and message' do + update_issue + + expect(response).to have_gitlab_http_status(400) + expect(json_response).to include('message' => { 'error' => 'Spam detected' }) + end + + it 'creates a new spam log entry' do + expect { update_issue } + .to log_spam(title: 'updated title', description: 'content here', user_id: user.id, noteable_type: 'Issue') + end + end + + context 'when allow_possible_spam feature flag is true' do + it 'updates a project issue' do + expect { update_issue }.to change { issue.reload.title } + end + + it 'returns correct status and message' do + update_issue + + expect(response).to have_gitlab_http_status(200) + end + + it 'creates a new spam log entry' do + expect { update_issue } + .to log_spam(title: 'updated title', description: 'content here', user_id: user.id, noteable_type: 'Issue') + end end end diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 2e6e13aa927..ef0cabad4b0 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -198,7 +198,7 @@ describe API::ProjectSnippets do it 'creates a spam log' do expect { create_snippet(project, visibility: 'public') } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Test Title', user_id: user.id, noteable_type: 'ProjectSnippet') end end end @@ -289,7 +289,7 @@ describe API::ProjectSnippets do it 'creates a spam log' do expect { update_snippet(title: 'Foo') } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Foo', user_id: admin.id, noteable_type: 'ProjectSnippet') end end @@ -306,7 +306,7 @@ describe API::ProjectSnippets do it 'creates a spam log' do expect { update_snippet(title: 'Foo', visibility: 'public') } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Foo', user_id: admin.id, noteable_type: 'ProjectSnippet') end end end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 515912cb305..e7eaaea2418 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -254,7 +254,7 @@ describe API::Snippets do it 'creates a spam log' do expect { create_snippet(visibility: 'public') } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Test Title', user_id: user.id, noteable_type: 'PersonalSnippet') end end end @@ -344,8 +344,7 @@ describe API::Snippets do end it 'creates a spam log' do - expect { update_snippet(title: 'Foo') } - .to change { SpamLog.count }.by(1) + expect { update_snippet(title: 'Foo') }.to log_spam(title: 'Foo', user_id: user.id, noteable_type: 'PersonalSnippet') end end @@ -359,7 +358,7 @@ describe API::Snippets do it 'creates a spam log' do expect { update_snippet(title: 'Foo', visibility: 'public') } - .to change { SpamLog.count }.by(1) + .to log_spam(title: 'Foo', user_id: user.id, noteable_type: 'PersonalSnippet') end end end diff --git a/spec/services/create_snippet_service_spec.rb b/spec/services/create_snippet_service_spec.rb index 7d2491b3a49..1751029a78c 100644 --- a/spec/services/create_snippet_service_spec.rb +++ b/spec/services/create_snippet_service_spec.rb @@ -3,26 +3,28 @@ require 'spec_helper' describe CreateSnippetService do - before do - @user = create :user - @admin = create :user, admin: true - @opts = { + let(:user) { create(:user) } + let(:admin) { create(:user, :admin) } + let(:opts) { base_opts.merge(extra_opts) } + let(:base_opts) do + { title: 'Test snippet', file_name: 'snippet.rb', content: 'puts "hello world"', visibility_level: Gitlab::VisibilityLevel::PRIVATE } end + let(:extra_opts) { {} } context 'When public visibility is restricted' do + let(:extra_opts) { { visibility_level: Gitlab::VisibilityLevel::PUBLIC } } + before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) - - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) end it 'non-admins are not able to create a public snippet' do - snippet = create_snippet(nil, @user, @opts) + snippet = create_snippet(nil, user, opts) expect(snippet.errors.messages).to have_key(:visibility_level) expect(snippet.errors.messages[:visibility_level].first).to( match('has been restricted') @@ -30,37 +32,81 @@ describe CreateSnippetService do end it 'admins are able to create a public snippet' do - snippet = create_snippet(nil, @admin, @opts) + snippet = create_snippet(nil, admin, opts) expect(snippet.errors.any?).to be_falsey expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) end describe "when visibility level is passed as a string" do + let(:extra_opts) { { visibility: 'internal' } } + before do - @opts[:visibility] = 'internal' - @opts.delete(:visibility_level) + base_opts.delete(:visibility_level) end it "assigns the correct visibility level" do - snippet = create_snippet(nil, @user, @opts) + snippet = create_snippet(nil, user, opts) expect(snippet.errors.any?).to be_falsey expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) end end end + context 'checking spam' do + shared_examples 'marked as spam' do + let(:snippet) { create_snippet(nil, admin, opts) } + + it 'marks a snippet as a spam ' do + expect(snippet).to be_spam + end + + it 'invalidates the snippet' do + expect(snippet).to be_invalid + end + + it 'creates a new spam_log' do + expect { snippet } + .to log_spam(title: snippet.title, noteable_type: 'PersonalSnippet') + end + + it 'assigns a spam_log to an issue' do + expect(snippet.spam_log).to eq(SpamLog.last) + end + end + + let(:extra_opts) do + { visibility_level: Gitlab::VisibilityLevel::PUBLIC, request: double(:request, env: {}) } + end + + before do + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: true) + end + end + + [true, false, nil].each do |allow_possible_spam| + context "when recaptcha_disabled flag is #{allow_possible_spam.inspect}" do + before do + stub_feature_flags(allow_possible_spam: allow_possible_spam) unless allow_possible_spam.nil? + end + + it_behaves_like 'marked as spam' + end + end + end + describe 'usage counter' do let(:counter) { Gitlab::UsageDataCounters::SnippetCounter } it 'increments count' do expect do - create_snippet(nil, @admin, @opts) + create_snippet(nil, admin, opts) end.to change { counter.read(:create) }.by 1 end it 'does not increment count if create fails' do expect do - create_snippet(nil, @admin, {}) + create_snippet(nil, admin, {}) end.not_to change { counter.read(:create) } end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index b7bedc2f97e..5dc6b6176ee 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -344,7 +344,7 @@ describe Issues::CreateService do end before do - allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) + stub_feature_flags(allow_possible_spam: false) end context 'when recaptcha was verified' do @@ -384,31 +384,67 @@ describe Issues::CreateService do end context 'when recaptcha was not verified' do + before do + expect_next_instance_of(SpamService) do |spam_service| + expect(spam_service).to receive_messages(check_for_spam?: true) + end + end + context 'when akismet detects spam' do before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: true) + end end - it 'marks an issue as a spam ' do - expect(issue).to be_spam - end + context 'when issuables_recaptcha_enabled feature flag is true' do + it 'marks an issue as a spam ' do + expect(issue).to be_spam + end - it 'an issue is not valid ' do - expect(issue.valid?).to be_falsey - end + it 'invalidates the issue' do + expect(issue).to be_invalid + end + + it 'creates a new spam_log' do + expect { issue } + .to log_spam(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue') + end - it 'creates a new spam_log' do - expect {issue}.to change {SpamLog.count}.from(0).to(1) + it 'assigns a spam_log to an issue' do + expect(issue.spam_log).to eq(SpamLog.last) + end end - it 'assigns a spam_log to an issue' do - expect(issue.spam_log).to eq(SpamLog.last) + context 'when issuable_recaptcha_enabled feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: true) + end + + it 'does not mark an issue as a spam ' do + expect(issue).not_to be_spam + end + + it 'accepts the ​issue as valid' do + expect(issue).to be_valid + end + + it 'creates a new spam_log' do + expect { issue } + .to log_spam(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue') + end + + it 'assigns a spam_log to an issue' do + expect(issue.spam_log).to eq(SpamLog.last) + end end end context 'when akismet does not detect spam' do before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) + expect_next_instance_of(AkismetService) do |akismet_service| + expect(akismet_service).to receive_messages(spam?: false) + end end it 'does not mark an issue as a spam ' do diff --git a/spec/services/spam_service_spec.rb b/spec/services/spam_service_spec.rb index b9e5e844c1f..76f77583612 100644 --- a/spec/services/spam_service_spec.rb +++ b/spec/services/spam_service_spec.rb @@ -44,30 +44,50 @@ describe SpamService do end context 'when indicated as spam by akismet' do + shared_examples 'akismet spam' do + it 'doesnt check as spam when request is missing' do + check_spam(issue, nil, false) + + expect(issue).not_to be_spam + end + + it 'creates a spam log' do + expect { check_spam(issue, request, false) } + .to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue') + end + + it 'does not yield to the block' do + expect(check_spam(issue, request, false)) + .to eql(SpamLog.last) + end + end + before do allow(AkismetService).to receive(:new).and_return(double(spam?: true)) end - it 'doesnt check as spam when request is missing' do - check_spam(issue, nil, false) + context 'when allow_possible_spam feature flag is false' do + before do + stub_feature_flags(allow_possible_spam: false) + end - expect(issue.spam).to be_falsey - end + it_behaves_like 'akismet spam' - it 'checks as spam' do - check_spam(issue, request, false) + it 'checks as spam' do + check_spam(issue, request, false) - expect(issue.spam).to be_truthy + expect(issue.spam).to be_truthy + end end - it 'creates a spam log' do - expect { check_spam(issue, request, false) } - .to change { SpamLog.count }.from(0).to(1) - end + context 'when allow_possible_spam feature flag is true' do + it_behaves_like 'akismet spam' + + it 'does not check as spam' do + check_spam(issue, request, false) - it 'doesnt yield block' do - expect(check_spam(issue, request, false)) - .to eql(SpamLog.last) + expect(issue.spam).to be_nil + end end end diff --git a/spec/support/matchers/log_spam.rb b/spec/support/matchers/log_spam.rb new file mode 100644 index 00000000000..541cacf558c --- /dev/null +++ b/spec/support/matchers/log_spam.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# This matcher checkes if one spam log with provided attributes was created +# +# Example: +# +# expect { create_issue }.to log_spam +RSpec::Matchers.define :log_spam do |expected| + def spam_logs + SpamLog.all + end + + match do |block| + block.call + + expect(spam_logs).to contain_exactly( + have_attributes(expected) + ) + end + + description do + count = spam_logs.count + + if count == 1 + keys = expected.keys.map(&:to_s) + actual = spam_logs.first.attributes.slice(*keys) + "create a spam log with #{expected} attributes. #{actual} created instead." + else + "create exactly 1 spam log with #{expected} attributes. #{count} spam logs created instead." + end + end + + supports_block_expectations +end |