Commit 52dd7168 authored by Thong Kuah's avatar Thong Kuah

Merge branch '64023-disable-recaptcha' into 'master'

Add feature flag for disabling reCaptcha

See merge request gitlab-org/gitlab!17604
parents cde4a645 ec82bf09
......@@ -80,4 +80,9 @@ module Spammable
def check_for_spam?
true
end
# Override in Spammable if differs
def allow_possible_spam?
Feature.enabled?(:allow_possible_spam, project)
end
end
......@@ -14,6 +14,7 @@ class Snippet < ApplicationRecord
include Editable
include Gitlab::SQL::Pattern
include FromUnion
extend ::Gitlab::Utils::Override
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description
......@@ -191,6 +192,12 @@ class Snippet < ApplicationRecord
(public? && (title_changed? || content_changed?))
end
# snippers are the biggest sources of spam
override :allow_possible_spam?
def allow_possible_spam?
false
end
def spammable_entity_type
'snippet'
end
......
......@@ -37,7 +37,8 @@ class SpamService
else
# Otherwise, it goes to Akismet and check if it's a spam. If that's the
# case, it assigns spammable record as "spam" and create a SpamLog record.
spammable.spam = check(api)
possible_spam = check(api)
spammable.spam = possible_spam unless spammable.allow_possible_spam?
spammable.spam_log = spam_log
end
end
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
# 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
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment