Commit ad6473de authored by Sean McGivern's avatar Sean McGivern Committed by Ruben Davila

Merge branch '28093-snippet-and-issue-spam-check-on-edit' into 'master'

Spam check and reCAPTCHA improvements

Closes #28093

See merge request !9248
Conflicts:
	app/services/issues/create_service.rb
parent 26e77c38
...@@ -17,13 +17,31 @@ module SpammableActions ...@@ -17,13 +17,31 @@ module SpammableActions
private private
def recaptcha_params def recaptcha_check_with_fallback(&fallback)
return {} unless params[:recaptcha_verification] && Gitlab::Recaptcha.load_configurations! && verify_recaptcha if spammable.valid?
redirect_to spammable
elsif render_recaptcha?
if params[:recaptcha_verification]
flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
end
render :verify
else
fallback.call
end
end
def spammable_params
default_params = { request: request }
recaptcha_check = params[:recaptcha_verification] &&
Gitlab::Recaptcha.load_configurations! &&
verify_recaptcha
return default_params unless recaptcha_check
{ { recaptcha_verified: true,
recaptcha_verified: true, spam_log_id: params[:spam_log_id] }.merge(default_params)
spam_log_id: params[:spam_log_id]
}
end end
def spammable def spammable
......
...@@ -91,15 +91,15 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -91,15 +91,15 @@ class Projects::IssuesController < Projects::ApplicationController
end end
def create def create
extra_params = { request: request, create_params = issue_params
merge_request_for_resolving_discussions: merge_request_for_resolving_discussions } .merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions)
extra_params.merge!(recaptcha_params) .merge(spammable_params)
@issue = Issues::CreateService.new(project, current_user, issue_params.merge(extra_params)).execute @issue = Issues::CreateService.new(project, current_user, create_params).execute
respond_to do |format| respond_to do |format|
format.html do format.html do
html_response_create recaptcha_check_with_fallback { render :new }
end end
format.js do format.js do
@link = @issue.attachment.url.to_js @link = @issue.attachment.url.to_js
...@@ -108,7 +108,9 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -108,7 +108,9 @@ class Projects::IssuesController < Projects::ApplicationController
end end
def update def update
@issue = Issues::UpdateService.new(project, current_user, issue_params).execute(issue) update_params = issue_params.merge(spammable_params)
@issue = Issues::UpdateService.new(project, current_user, update_params).execute(issue)
if params[:move_to_project_id].to_i > 0 if params[:move_to_project_id].to_i > 0
new_project = Project.find(params[:move_to_project_id]) new_project = Project.find(params[:move_to_project_id])
...@@ -120,11 +122,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -120,11 +122,7 @@ class Projects::IssuesController < Projects::ApplicationController
respond_to do |format| respond_to do |format|
format.html do format.html do
if @issue.valid? recaptcha_check_with_fallback { render :edit }
redirect_to issue_path(@issue)
else
render :edit
end
end end
format.json do format.json do
...@@ -176,20 +174,6 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -176,20 +174,6 @@ class Projects::IssuesController < Projects::ApplicationController
protected protected
def html_response_create
if @issue.valid?
redirect_to issue_path(@issue)
elsif render_recaptcha?
if params[:recaptcha_verification]
flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
end
render :verify
else
render :new
end
end
def issue def issue
# The Sortable default scope causes performance issues when used with find_by # The Sortable default scope causes performance issues when used with find_by
@noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take || redirect_old @noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take || redirect_old
......
...@@ -37,27 +37,22 @@ class Projects::SnippetsController < Projects::ApplicationController ...@@ -37,27 +37,22 @@ class Projects::SnippetsController < Projects::ApplicationController
end end
def create def create
create_params = snippet_params.merge(request: request) create_params = snippet_params.merge(spammable_params)
@snippet = CreateSnippetService.new(@project, current_user, create_params).execute @snippet = CreateSnippetService.new(@project, current_user, create_params).execute
if @snippet.valid? recaptcha_check_with_fallback { render :new }
respond_with(@snippet,
location: namespace_project_snippet_path(@project.namespace,
@project, @snippet))
else
render :new
end
end end
def edit def edit
end end
def update def update
UpdateSnippetService.new(project, current_user, @snippet, update_params = snippet_params.merge(spammable_params)
snippet_params).execute
respond_with(@snippet, UpdateSnippetService.new(project, current_user, @snippet, update_params).execute
location: namespace_project_snippet_path(@project.namespace,
@project, @snippet)) recaptcha_check_with_fallback { render :edit }
end end
def show def show
......
...@@ -41,19 +41,22 @@ class SnippetsController < ApplicationController ...@@ -41,19 +41,22 @@ class SnippetsController < ApplicationController
end end
def create def create
create_params = snippet_params.merge(request: request) create_params = snippet_params.merge(spammable_params)
@snippet = CreateSnippetService.new(nil, current_user, create_params).execute @snippet = CreateSnippetService.new(nil, current_user, create_params).execute
respond_with @snippet.becomes(Snippet) recaptcha_check_with_fallback { render :new }
end end
def edit def edit
end end
def update def update
UpdateSnippetService.new(nil, current_user, @snippet, update_params = snippet_params.merge(spammable_params)
snippet_params).execute
respond_with @snippet.becomes(Snippet) UpdateSnippetService.new(nil, current_user, @snippet, update_params).execute
recaptcha_check_with_fallback { render :edit }
end end
def show def show
......
...@@ -13,7 +13,7 @@ module Spammable ...@@ -13,7 +13,7 @@ module Spammable
attr_accessor :spam attr_accessor :spam
attr_accessor :spam_log attr_accessor :spam_log
after_validation :check_for_spam, on: :create after_validation :check_for_spam, on: [:create, :update]
cattr_accessor :spammable_attrs, instance_accessor: false do cattr_accessor :spammable_attrs, instance_accessor: false do
[] []
......
...@@ -9,8 +9,4 @@ class ProjectSnippet < Snippet ...@@ -9,8 +9,4 @@ class ProjectSnippet < Snippet
participant :author participant :author
participant :notes_with_associations participant :notes_with_associations
def check_for_spam?
super && project.public?
end
end end
class CreateSnippetService < BaseService class CreateSnippetService < BaseService
include SpamCheckService
def execute def execute
request = params.delete(:request) filter_spam_check_params
api = params.delete(:api)
snippet = if project snippet = if project
project.snippets.build(params) project.snippets.build(params)
...@@ -15,10 +16,11 @@ class CreateSnippetService < BaseService ...@@ -15,10 +16,11 @@ class CreateSnippetService < BaseService
end end
snippet.author = current_user snippet.author = current_user
snippet.spam = SpamService.new(snippet, request).check(api)
spam_check(snippet, current_user)
if snippet.save if snippet.save
UserAgentDetailService.new(snippet, request).create UserAgentDetailService.new(snippet, @request).create
end end
snippet snippet
......
...@@ -191,14 +191,12 @@ class IssuableBaseService < BaseService ...@@ -191,14 +191,12 @@ class IssuableBaseService < BaseService
# To be overridden by subclasses # To be overridden by subclasses
end end
def after_update(issuable) def before_update(issuable)
# To be overridden by subclasses # To be overridden by subclasses
end end
def update_issuable(issuable, attributes) def after_update(issuable)
issuable.with_transaction_returning_status do # To be overridden by subclasses
issuable.update(attributes.merge(updated_by: current_user))
end
end end
def update(issuable) def update(issuable)
...@@ -212,7 +210,12 @@ class IssuableBaseService < BaseService ...@@ -212,7 +210,12 @@ class IssuableBaseService < BaseService
label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids)
params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids)
if params.present? && update_issuable(issuable, params) if params.present?
issuable.assign_attributes(params.merge(updated_by: current_user))
before_update(issuable)
if issuable.with_transaction_returning_status { issuable.save }
# We do not touch as it will affect a update on updated_at field # We do not touch as it will affect a update on updated_at field
ActiveRecord::Base.no_touching do ActiveRecord::Base.no_touching do
handle_common_system_notes(issuable, old_labels: old_labels) handle_common_system_notes(issuable, old_labels: old_labels)
...@@ -223,6 +226,7 @@ class IssuableBaseService < BaseService ...@@ -223,6 +226,7 @@ class IssuableBaseService < BaseService
issuable.create_new_cross_references!(current_user) issuable.create_new_cross_references!(current_user)
execute_hooks(issuable, 'update') execute_hooks(issuable, 'update')
end end
end
issuable issuable
end end
......
module Issues module Issues
class CreateService < Issues::BaseService class CreateService < Issues::BaseService
include SpamCheckService
def execute def execute
@request = params.delete(:request) filter_spam_check_params
@api = params.delete(:api)
@recaptcha_verified = params.delete(:recaptcha_verified)
@spam_log_id = params.delete(:spam_log_id)
issue_attributes = params.merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions) issue_attributes = params.merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions)
@issue = BuildService.new(project, current_user, issue_attributes).execute @issue = BuildService.new(project, current_user, issue_attributes).execute
...@@ -12,14 +11,8 @@ module Issues ...@@ -12,14 +11,8 @@ module Issues
create(@issue) create(@issue)
end end
def before_create(issuable) def before_create(issue)
if @recaptcha_verified spam_check(issue, current_user)
spam_log = current_user.spam_logs.find_by(id: @spam_log_id, title: issuable.title)
spam_log.update!(recaptcha_verified: true) if spam_log
else
issuable.spam = spam_service.check(@api)
issuable.spam_log = spam_service.spam_log
end
end end
def after_create(issuable) def after_create(issuable)
...@@ -42,10 +35,6 @@ module Issues ...@@ -42,10 +35,6 @@ module Issues
private private
def spam_service
@spam_service ||= SpamService.new(@issue, @request)
end
def user_agent_detail_service def user_agent_detail_service
UserAgentDetailService.new(@issue, @request) UserAgentDetailService.new(@issue, @request)
end end
......
module Issues module Issues
class UpdateService < Issues::BaseService class UpdateService < Issues::BaseService
include SpamCheckService
def execute(issue) def execute(issue)
filter_spam_check_params
update(issue) update(issue)
end end
def before_update(issue)
spam_check(issue, current_user)
end
def handle_changes(issue, old_labels: [], old_mentioned_users: []) def handle_changes(issue, old_labels: [], old_mentioned_users: [])
if has_changes?(issue, old_labels: old_labels) if has_changes?(issue, old_labels: old_labels)
todo_service.mark_pending_todos_as_done(issue, current_user) todo_service.mark_pending_todos_as_done(issue, current_user)
......
# SpamCheckService
#
# Provide helper methods for checking if a given spammable object has
# potential spam data.
#
# Dependencies:
# - params with :request
#
module SpamCheckService
def filter_spam_check_params
@request = params.delete(:request)
@api = params.delete(:api)
@recaptcha_verified = params.delete(:recaptcha_verified)
@spam_log_id = params.delete(:spam_log_id)
end
def spam_check(spammable, user)
spam_service = SpamService.new(spammable, @request)
spam_service.when_recaptcha_verified(@recaptcha_verified, @api) do
user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true)
end
end
end
...@@ -17,15 +17,6 @@ class SpamService ...@@ -17,15 +17,6 @@ class SpamService
end end
end end
def check(api = false)
return false unless request && check_for_spam?
return false unless akismet.is_spam?
create_spam_log(api)
true
end
def mark_as_spam! def mark_as_spam!
return false unless spammable.submittable_as_spam? return false unless spammable.submittable_as_spam?
...@@ -36,8 +27,30 @@ class SpamService ...@@ -36,8 +27,30 @@ class SpamService
end end
end end
def when_recaptcha_verified(recaptcha_verified, api = false)
# In case it's a request which is already verified through recaptcha, yield
# block.
if recaptcha_verified
yield
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)
spammable.spam_log = spam_log
end
end
private private
def check(api)
return false unless request && check_for_spam?
return false unless akismet.is_spam?
create_spam_log(api)
true
end
def akismet def akismet
@akismet ||= AkismetService.new( @akismet ||= AkismetService.new(
spammable_owner, spammable_owner,
......
class UpdateSnippetService < BaseService class UpdateSnippetService < BaseService
include SpamCheckService
attr_accessor :snippet attr_accessor :snippet
def initialize(project, user, snippet, params) def initialize(project, user, snippet, params)
...@@ -17,6 +19,10 @@ class UpdateSnippetService < BaseService ...@@ -17,6 +19,10 @@ class UpdateSnippetService < BaseService
end end
end end
snippet.update_attributes(params) filter_spam_check_params
snippet.assign_attributes(params)
spam_check(snippet, current_user)
snippet.save
end end
end end
- humanized_resource_name = spammable.class.model_name.human.downcase
- resource_name = spammable.class.model_name.singular
%h3.page-title
Anti-spam verification
%hr
%p
#{"We detected potential spam in the #{humanized_resource_name}. Please solve the reCAPTCHA to proceed."}
= form_for form do |f|
.recaptcha
- params[resource_name].each do |field, value|
= hidden_field(resource_name, field, value: value)
= hidden_field_tag(:spam_log_id, spammable.spam_log.id)
= hidden_field_tag(:recaptcha_verification, true)
= recaptcha_tags
-# Yields a block with given extra params.
= yield
.row-content-block.footer-block
= f.submit "Submit #{humanized_resource_name}", class: 'btn btn-create'
- page_title "Anti-spam verification" - form = [@project.namespace.becomes(Namespace), @project, @issue]
%h3.page-title = render layout: 'layouts/recaptcha_verification', locals: { spammable: @issue, form: form } do
Anti-spam verification
%hr
%p
We detected potential spam in the issue description. Please verify that you are not a robot to submit the issue.
= form_for [@project.namespace.becomes(Namespace), @project, @issue] do |f|
.recaptcha
- params[:issue].each do |field, value|
= hidden_field(:issue, field, value: value)
= hidden_field_tag(:merge_request_for_resolving_discussions, params[:merge_request_for_resolving_discussions]) = hidden_field_tag(:merge_request_for_resolving_discussions, params[:merge_request_for_resolving_discussions])
= hidden_field_tag(:spam_log_id, @issue.spam_log.id)
= hidden_field_tag(:recaptcha_verification, true)
= recaptcha_tags
.row-content-block.footer-block
= f.submit "Submit #{@issue.class.model_name.human.downcase}", class: 'btn btn-create'
- form = [@project.namespace.becomes(Namespace), @project, @snippet.becomes(Snippet)]
= render 'layouts/recaptcha_verification', spammable: @snippet, form: form
- form = [@snippet.becomes(Snippet)]
= render 'layouts/recaptcha_verification', spammable: @snippet, form: form
---
title: Spam check and reCAPTCHA improvements
merge_request:
author:
...@@ -231,6 +231,10 @@ module API ...@@ -231,6 +231,10 @@ module API
end end
end end
def render_spam_error!
render_api_error!({ error: 'Spam detected' }, 400)
end
def render_api_error!(message, status) def render_api_error!(message, status)
error!({ 'message' => message }, status, header) error!({ 'message' => message }, status, header)
end end
......
...@@ -177,9 +177,13 @@ module API ...@@ -177,9 +177,13 @@ module API
params.delete(:updated_at) params.delete(:updated_at)
end end
update_params = declared_params(include_missing: false).merge(request: request, api: true)
issue = ::Issues::UpdateService.new(user_project, issue = ::Issues::UpdateService.new(user_project,
current_user, current_user,
declared_params(include_missing: false)).execute(issue) update_params).execute(issue)
render_spam_error! if issue.spam?
if issue.valid? if issue.valid?
present issue, with: Entities::Issue, current_user: current_user, project: user_project present issue, with: Entities::Issue, current_user: current_user, project: user_project
......
...@@ -63,6 +63,8 @@ module API ...@@ -63,6 +63,8 @@ module API
snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute
render_spam_error! if snippet.spam?
if snippet.persisted? if snippet.persisted?
present snippet, with: Entities::ProjectSnippet present snippet, with: Entities::ProjectSnippet
else else
...@@ -92,12 +94,16 @@ module API ...@@ -92,12 +94,16 @@ module API
authorize! :update_project_snippet, snippet authorize! :update_project_snippet, snippet
snippet_params = declared_params(include_missing: false) snippet_params = declared_params(include_missing: false)
.merge(request: request, api: true)
snippet_params[:content] = snippet_params.delete(:code) if snippet_params[:code].present? snippet_params[:content] = snippet_params.delete(:code) if snippet_params[:code].present?
UpdateSnippetService.new(user_project, current_user, snippet, UpdateSnippetService.new(user_project, current_user, snippet,
snippet_params).execute snippet_params).execute
if snippet.persisted? render_spam_error! if snippet.spam?
if snippet.valid?
present snippet, with: Entities::ProjectSnippet present snippet, with: Entities::ProjectSnippet
else else
render_validation_error!(snippet) render_validation_error!(snippet)
......
...@@ -67,6 +67,8 @@ module API ...@@ -67,6 +67,8 @@ module API
attrs = declared_params(include_missing: false).merge(request: request, api: true) attrs = declared_params(include_missing: false).merge(request: request, api: true)
snippet = CreateSnippetService.new(nil, current_user, attrs).execute snippet = CreateSnippetService.new(nil, current_user, attrs).execute
render_spam_error! if snippet.spam?
if snippet.persisted? if snippet.persisted?
present snippet, with: Entities::PersonalSnippet present snippet, with: Entities::PersonalSnippet
else else
...@@ -93,9 +95,12 @@ module API ...@@ -93,9 +95,12 @@ module API
return not_found!('Snippet') unless snippet return not_found!('Snippet') unless snippet
authorize! :update_personal_snippet, snippet authorize! :update_personal_snippet, snippet
attrs = declared_params(include_missing: false) attrs = declared_params(include_missing: false).merge(request: request, api: true)
UpdateSnippetService.new(nil, current_user, snippet, attrs).execute UpdateSnippetService.new(nil, current_user, snippet, attrs).execute
render_spam_error! if snippet.spam?
if snippet.persisted? if snippet.persisted?
present snippet, with: Entities::PersonalSnippet present snippet, with: Entities::PersonalSnippet
else else
......
...@@ -148,9 +148,7 @@ module API ...@@ -148,9 +148,7 @@ module API
issue = ::Issues::CreateService.new(user_project, issue = ::Issues::CreateService.new(user_project,
current_user, current_user,
issue_params.merge(request: request, api: true)).execute issue_params.merge(request: request, api: true)).execute
if issue.spam? render_spam_error! if issue.spam?
render_api_error!({ error: 'Spam detected' }, 400)
end
if issue.valid? if issue.valid?
present issue, with: ::API::Entities::Issue, current_user: current_user, project: user_project present issue, with: ::API::Entities::Issue, current_user: current_user, project: user_project
...@@ -181,9 +179,13 @@ module API ...@@ -181,9 +179,13 @@ module API
params.delete(:updated_at) params.delete(:updated_at)
end end
update_params = declared_params(include_missing: false).merge(request: request, api: true)
issue = ::Issues::UpdateService.new(user_project, issue = ::Issues::UpdateService.new(user_project,
current_user, current_user,
declared_params(include_missing: false)).execute(issue) update_params).execute(issue)
render_spam_error! if issue.spam?
if issue.valid? if issue.valid?
present issue, with: ::API::Entities::Issue, current_user: current_user, project: user_project present issue, with: ::API::Entities::Issue, current_user: current_user, project: user_project
......
...@@ -64,6 +64,8 @@ module API ...@@ -64,6 +64,8 @@ module API
snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute
render_spam_error! if snippet.spam?
if snippet.persisted? if snippet.persisted?
present snippet, with: ::API::V3::Entities::ProjectSnippet present snippet, with: ::API::V3::Entities::ProjectSnippet
else else
...@@ -93,12 +95,16 @@ module API ...@@ -93,12 +95,16 @@ module API
authorize! :update_project_snippet, snippet authorize! :update_project_snippet, snippet
snippet_params = declared_params(include_missing: false) snippet_params = declared_params(include_missing: false)
.merge(request: request, api: true)
snippet_params[:content] = snippet_params.delete(:code) if snippet_params[:code].present? snippet_params[:content] = snippet_params.delete(:code) if snippet_params[:code].present?
UpdateSnippetService.new(user_project, current_user, snippet, UpdateSnippetService.new(user_project, current_user, snippet,
snippet_params).execute snippet_params).execute
if snippet.persisted? render_spam_error! if snippet.spam?
if snippet.valid?
present snippet, with: ::API::V3::Entities::ProjectSnippet present snippet, with: ::API::V3::Entities::ProjectSnippet
else else
render_validation_error!(snippet) render_validation_error!(snippet)
......
...@@ -150,6 +150,113 @@ describe Projects::IssuesController do ...@@ -150,6 +150,113 @@ describe Projects::IssuesController do
end end
end end
context 'Akismet is enabled' do
let(:project) { create(:project_empty_repo, :public) }
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(:is_spam?).and_return(false)
end
it 'normally updates the issue' do
expect { update_issue(title: 'Foo') }.to change { issue.reload.title }.to('Foo')
end
end
context 'when an issue is identified as spam' do
before { allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) }
context 'when captcha is not verified' do
def update_spam_issue
update_issue(title: 'Spam Title', description: 'Spam lives here')
end
before { allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) }
it 'rejects an issue recognized as a spam' do
expect { update_spam_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)
expect { update_spam_issue }.not_to change{ issue.reload.title }
end
it 'creates a spam log' do
update_spam_issue
spam_logs = SpamLog.all
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 verify template' do
update_spam_issue
expect(response).to render_template(:verify)
end
end
context 'when captcha is verified' do
let(:spammy_title) { 'Whatever' }
let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) }
def update_verified_issue
update_issue({ title: spammy_title },
{ 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 'redirect to issue page' do
update_verified_issue
expect(response).
to redirect_to(namespace_project_issue_path(project.namespace, project, issue))
end
it 'accepts an issue after recaptcha is verified' do
expect{ update_verified_issue }.to change{ issue.reload.title }.to(spammy_title)
end
it 'marks spam log as recaptcha_verified' do
expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true)
end
it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do
spam_log = create(:spam_log)
expect { update_issue(spam_log_id: spam_log.id, recaptcha_verification: true) }.
not_to change { SpamLog.last.recaptcha_verified }
end
end
end
end
def update_issue(issue_params = {}, additional_params = {})
params = {
namespace_id: project.namespace.to_param,
project_id: project.to_param,
id: issue.iid,
issue: issue_params
}.merge(additional_params)
put :update, params
end
def move_issue def move_issue
put :update, put :update,
namespace_id: project.namespace.to_param, namespace_id: project.namespace.to_param,
...@@ -382,7 +489,7 @@ describe Projects::IssuesController do ...@@ -382,7 +489,7 @@ describe Projects::IssuesController do
allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
end end
context 'when an issue is not identified as a spam' do context 'when an issue is not identified as spam' do
before do before do
allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false) allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false)
...@@ -393,7 +500,7 @@ describe Projects::IssuesController do ...@@ -393,7 +500,7 @@ describe Projects::IssuesController do
end end
end end
context 'when an issue is identified as a spam' do context 'when an issue is identified as spam' do
before { allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) } before { allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) }
context 'when captcha is not verified' do context 'when captcha is not verified' do
......
...@@ -70,7 +70,7 @@ describe Projects::SnippetsController do ...@@ -70,7 +70,7 @@ describe Projects::SnippetsController do
end end
describe 'POST #create' do describe 'POST #create' do
def create_snippet(project, snippet_params = {}) def create_snippet(project, snippet_params = {}, additional_params = {})
sign_in(user) sign_in(user)
project.add_developer(user) project.add_developer(user)
...@@ -79,7 +79,7 @@ describe Projects::SnippetsController do ...@@ -79,7 +79,7 @@ describe Projects::SnippetsController do
namespace_id: project.namespace.to_param, namespace_id: project.namespace.to_param,
project_id: project.to_param, project_id: project.to_param,
project_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params) project_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params)
} }.merge(additional_params)
end end
context 'when the snippet is spam' do context 'when the snippet is spam' do
...@@ -87,18 +87,6 @@ describe Projects::SnippetsController do ...@@ -87,18 +87,6 @@ describe Projects::SnippetsController do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end end
context 'when the project is private' do
let(:private_project) { create(:project_empty_repo, :private) }
context 'when the snippet is public' do
it 'creates the snippet' do
expect { create_snippet(private_project, visibility_level: Snippet::PUBLIC) }.
to change { Snippet.count }.by(1)
end
end
end
context 'when the project is public' do
context 'when the snippet is private' do context 'when the snippet is private' do
it 'creates the snippet' do it 'creates the snippet' do
expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }. expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }.
...@@ -117,6 +105,162 @@ describe Projects::SnippetsController do ...@@ -117,6 +105,162 @@ describe Projects::SnippetsController do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }. expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1) to change { SpamLog.count }.by(1)
end end
it 'renders :new with recaptcha disabled' do
stub_application_setting(recaptcha_enabled: false)
create_snippet(project, visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:new)
end
context 'recaptcha enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify with recaptcha enabled' do
create_snippet(project, visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:verify)
end
it 'renders snippet page when recaptcha verified' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
create_snippet(project,
{ visibility_level: Snippet::PUBLIC },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(Snippet.last)
end
end
end
end
end
describe 'PUT #update' do
let(:project) { create :project, :public }
let(:snippet) { create :project_snippet, author: user, project: project, visibility_level: visibility_level }
def update_snippet(snippet_params = {}, additional_params = {})
sign_in(user)
project.add_developer(user)
put :update, {
namespace_id: project.namespace.to_param,
project_id: project.to_param,
id: snippet.id,
project_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params)
}.merge(additional_params)
snippet.reload
end
context 'when the snippet is spam' do
before do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end
context 'when the snippet is private' do
let(:visibility_level) { Snippet::PRIVATE }
it 'updates the snippet' do
expect { update_snippet(title: 'Foo') }.
to change { snippet.reload.title }.to('Foo')
end
end
context 'when the snippet is public' do
let(:visibility_level) { Snippet::PUBLIC }
it 'rejects the shippet' do
expect { update_snippet(title: 'Foo') }.
not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo') }.
to change { SpamLog.count }.by(1)
end
it 'renders :edit with recaptcha disabled' do
stub_application_setting(recaptcha_enabled: false)
update_snippet(title: 'Foo')
expect(response).to render_template(:edit)
end
context 'recaptcha enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify with recaptcha enabled' do
update_snippet(title: 'Foo')
expect(response).to render_template(:verify)
end
it 'renders snippet page when recaptcha verified' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
snippet = update_snippet({ title: spammy_title },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(snippet)
end
end
end
context 'when the private snippet is made public' do
let(:visibility_level) { Snippet::PRIVATE }
it 'rejects the shippet' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
it 'renders :edit with recaptcha disabled' do
stub_application_setting(recaptcha_enabled: false)
update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:edit)
end
context 'recaptcha enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify with recaptcha enabled' do
update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:verify)
end
it 'renders snippet page when recaptcha verified' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
snippet = update_snippet({ title: spammy_title, visibility_level: Snippet::PUBLIC },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(snippet)
end
end end
end end
end end
......
...@@ -139,12 +139,14 @@ describe SnippetsController do ...@@ -139,12 +139,14 @@ describe SnippetsController do
end end
describe 'POST #create' do describe 'POST #create' do
def create_snippet(snippet_params = {}) def create_snippet(snippet_params = {}, additional_params = {})
sign_in(user) sign_in(user)
post :create, { post :create, {
personal_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params) personal_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params)
} }.merge(additional_params)
Snippet.last
end end
context 'when the snippet is spam' do context 'when the snippet is spam' do
...@@ -163,13 +165,164 @@ describe SnippetsController do ...@@ -163,13 +165,164 @@ describe SnippetsController do
it 'rejects the shippet' do it 'rejects the shippet' do
expect { create_snippet(visibility_level: Snippet::PUBLIC) }. expect { create_snippet(visibility_level: Snippet::PUBLIC) }.
not_to change { Snippet.count } not_to change { Snippet.count }
expect(response).to render_template(:new)
end end
it 'creates a spam log' do it 'creates a spam log' do
expect { create_snippet(visibility_level: Snippet::PUBLIC) }. expect { create_snippet(visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1) to change { SpamLog.count }.by(1)
end end
it 'renders :new with recaptcha disabled' do
stub_application_setting(recaptcha_enabled: false)
create_snippet(visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:new)
end
context 'recaptcha enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify with recaptcha enabled' do
create_snippet(visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:verify)
end
it 'renders snippet page when recaptcha verified' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
snippet = create_snippet({ title: spammy_title },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(snippet_path(snippet))
end
end
end
end
end
describe 'PUT #update' do
let(:project) { create :project }
let(:snippet) { create :personal_snippet, author: user, project: project, visibility_level: visibility_level }
def update_snippet(snippet_params = {}, additional_params = {})
sign_in(user)
put :update, {
id: snippet.id,
personal_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params)
}.merge(additional_params)
snippet.reload
end
context 'when the snippet is spam' do
before do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end
context 'when the snippet is private' do
let(:visibility_level) { Snippet::PRIVATE }
it 'updates the snippet' do
expect { update_snippet(title: 'Foo') }.
to change { snippet.reload.title }.to('Foo')
end
end
context 'when a private snippet is made public' do
let(:visibility_level) { Snippet::PRIVATE }
it 'rejects the snippet' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
it 'renders :edit with recaptcha disabled' do
stub_application_setting(recaptcha_enabled: false)
update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:edit)
end
context 'recaptcha enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify with recaptcha enabled' do
update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC)
expect(response).to render_template(:verify)
end
it 'renders snippet page when recaptcha verified' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
snippet = update_snippet({ title: spammy_title, visibility_level: Snippet::PUBLIC },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(snippet)
end
end
end
context 'when the snippet is public' do
let(:visibility_level) { Snippet::PUBLIC }
it 'rejects the shippet' do
expect { update_snippet(title: 'Foo') }.
not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo') }.
to change { SpamLog.count }.by(1)
end
it 'renders :edit with recaptcha disabled' do
stub_application_setting(recaptcha_enabled: false)
update_snippet(title: 'Foo')
expect(response).to render_template(:edit)
end
context 'recaptcha enabled' do
before do
stub_application_setting(recaptcha_enabled: true)
end
it 'renders :verify with recaptcha enabled' do
update_snippet(title: 'Foo')
expect(response).to render_template(:verify)
end
it 'renders snippet page when recaptcha verified' do
spammy_title = 'Whatever'
spam_logs = create_list(:spam_log, 2, user: user, title: spammy_title)
snippet = update_snippet({ title: spammy_title },
{ spam_log_id: spam_logs.last.id,
recaptcha_verification: true })
expect(response).to redirect_to(snippet_path(snippet))
end
end
end end
end end
end end
......
...@@ -23,6 +23,7 @@ describe Issue, 'Spammable' do ...@@ -23,6 +23,7 @@ describe Issue, 'Spammable' do
describe '#check_for_spam?' do describe '#check_for_spam?' do
it 'returns true for public project' do it 'returns true for public project' do
issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
expect(issue.check_for_spam?).to eq(true) expect(issue.check_for_spam?).to eq(true)
end end
......
...@@ -919,6 +919,33 @@ describe API::Issues, api: true do ...@@ -919,6 +919,33 @@ describe API::Issues, api: true do
end end
end end
describe 'PUT /projects/:id/issues/:issue_id with spam filtering' do
let(:params) do
{
title: 'updated title',
description: 'content here',
labels: 'label, label2'
}
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(is_spam?: true)
put api("/projects/#{project.id}/issues/#{issue.id}", user), params
expect(response).to have_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')
end
end
describe 'PUT /projects/:id/issues/:issue_id to update labels' do describe 'PUT /projects/:id/issues/:issue_id to update labels' do
let!(:label) { create(:label, title: 'dummy', project: project) } let!(:label) { create(:label, title: 'dummy', project: project) }
let!(:label_link) { create(:label_link, label: label, target: issue) } let!(:label_link) { create(:label_link, label: label, target: issue) }
......
...@@ -73,18 +73,6 @@ describe API::ProjectSnippets, api: true do ...@@ -73,18 +73,6 @@ describe API::ProjectSnippets, api: true do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end end
context 'when the project is private' do
let(:private_project) { create(:project_empty_repo, :private) }
context 'when the snippet is public' do
it 'creates the snippet' do
expect { create_snippet(private_project, visibility_level: Snippet::PUBLIC) }.
to change { Snippet.count }.by(1)
end
end
end
context 'when the project is public' do
context 'when the snippet is private' do context 'when the snippet is private' do
it 'creates the snippet' do it 'creates the snippet' do
expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }. expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }.
...@@ -96,7 +84,9 @@ describe API::ProjectSnippets, api: true do ...@@ -96,7 +84,9 @@ describe API::ProjectSnippets, api: true do
it 'rejects the shippet' do it 'rejects the shippet' do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }. expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }.
not_to change { Snippet.count } not_to change { Snippet.count }
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']).to eq({ "error" => "Spam detected" })
end end
it 'creates a spam log' do it 'creates a spam log' do
...@@ -106,10 +96,10 @@ describe API::ProjectSnippets, api: true do ...@@ -106,10 +96,10 @@ describe API::ProjectSnippets, api: true do
end end
end end
end end
end
describe 'PUT /projects/:project_id/snippets/:id/' do describe 'PUT /projects/:project_id/snippets/:id/' do
let(:snippet) { create(:project_snippet, author: admin) } let(:visibility_level) { Snippet::PUBLIC }
let(:snippet) { create(:project_snippet, author: admin, visibility_level: visibility_level) }
it 'updates snippet' do it 'updates snippet' do
new_content = 'New content' new_content = 'New content'
...@@ -133,6 +123,56 @@ describe API::ProjectSnippets, api: true do ...@@ -133,6 +123,56 @@ describe API::ProjectSnippets, api: true do
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
end end
context 'when the snippet is spam' do
def update_snippet(snippet_params = {})
put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}", admin), snippet_params
end
before do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end
context 'when the snippet is private' do
let(:visibility_level) { Snippet::PRIVATE }
it 'creates the snippet' do
expect { update_snippet(title: 'Foo') }.
to change { snippet.reload.title }.to('Foo')
end
end
context 'when the snippet is public' do
let(:visibility_level) { Snippet::PUBLIC }
it 'rejects the snippet' do
expect { update_snippet(title: 'Foo') }.
not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo') }.
to change { SpamLog.count }.by(1)
end
end
context 'when the private snippet is made public' do
let(:visibility_level) { Snippet::PRIVATE }
it 'rejects the snippet' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
not_to change { snippet.reload.title }
expect(response).to have_http_status(400)
expect(json_response['message']).to eq({ "error" => "Spam detected" })
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
end
end
end end
describe 'DELETE /projects/:project_id/snippets/:id/' do describe 'DELETE /projects/:project_id/snippets/:id/' do
......
...@@ -122,7 +122,9 @@ describe API::Snippets, api: true do ...@@ -122,7 +122,9 @@ describe API::Snippets, api: true do
it 'rejects the shippet' do it 'rejects the shippet' do
expect { create_snippet(visibility_level: Snippet::PUBLIC) }. expect { create_snippet(visibility_level: Snippet::PUBLIC) }.
not_to change { Snippet.count } not_to change { Snippet.count }
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']).to eq({ "error" => "Spam detected" })
end end
it 'creates a spam log' do it 'creates a spam log' do
...@@ -134,16 +136,20 @@ describe API::Snippets, api: true do ...@@ -134,16 +136,20 @@ describe API::Snippets, api: true do
end end
describe 'PUT /snippets/:id' do describe 'PUT /snippets/:id' do
let(:visibility_level) { Snippet::PUBLIC }
let(:other_user) { create(:user) } let(:other_user) { create(:user) }
let(:public_snippet) { create(:personal_snippet, :public, author: user) } let(:snippet) do
create(:personal_snippet, author: user, visibility_level: visibility_level)
end
it 'updates snippet' do it 'updates snippet' do
new_content = 'New content' new_content = 'New content'
put api("/snippets/#{public_snippet.id}", user), content: new_content put api("/snippets/#{snippet.id}", user), content: new_content
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
public_snippet.reload snippet.reload
expect(public_snippet.content).to eq(new_content) expect(snippet.content).to eq(new_content)
end end
it 'returns 404 for invalid snippet id' do it 'returns 404 for invalid snippet id' do
...@@ -154,7 +160,7 @@ describe API::Snippets, api: true do ...@@ -154,7 +160,7 @@ describe API::Snippets, api: true do
end end
it "returns 404 for another user's snippet" do it "returns 404 for another user's snippet" do
put api("/snippets/#{public_snippet.id}", other_user), title: 'fubar' put api("/snippets/#{snippet.id}", other_user), title: 'fubar'
expect(response).to have_http_status(404) expect(response).to have_http_status(404)
expect(json_response['message']).to eq('404 Snippet Not Found') expect(json_response['message']).to eq('404 Snippet Not Found')
...@@ -165,6 +171,56 @@ describe API::Snippets, api: true do ...@@ -165,6 +171,56 @@ describe API::Snippets, api: true do
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
end end
context 'when the snippet is spam' do
def update_snippet(snippet_params = {})
put api("/snippets/#{snippet.id}", user), snippet_params
end
before do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end
context 'when the snippet is private' do
let(:visibility_level) { Snippet::PRIVATE }
it 'updates the snippet' do
expect { update_snippet(title: 'Foo') }.
to change { snippet.reload.title }.to('Foo')
end
end
context 'when the snippet is public' do
let(:visibility_level) { Snippet::PUBLIC }
it 'rejects the shippet' do
expect { update_snippet(title: 'Foo') }.
not_to change { snippet.reload.title }
expect(response).to have_http_status(400)
expect(json_response['message']).to eq({ "error" => "Spam detected" })
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo') }.
to change { SpamLog.count }.by(1)
end
end
context 'when a private snippet is made public' do
let(:visibility_level) { Snippet::PRIVATE }
it 'rejects the snippet' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
end
end
end end
describe 'DELETE /snippets/:id' do describe 'DELETE /snippets/:id' do
......
...@@ -986,6 +986,33 @@ describe API::V3::Issues, api: true do ...@@ -986,6 +986,33 @@ describe API::V3::Issues, api: true do
end end
end end
describe 'PUT /projects/:id/issues/:issue_id with spam filtering' do
let(:params) do
{
title: 'updated title',
description: 'content here',
labels: 'label, label2'
}
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(is_spam?: true)
put v3_api("/projects/#{project.id}/issues/#{issue.id}", user), params
expect(response).to have_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')
end
end
describe 'PUT /projects/:id/issues/:issue_id to update labels' do describe 'PUT /projects/:id/issues/:issue_id to update labels' do
let!(:label) { create(:label, title: 'dummy', project: project) } let!(:label) { create(:label, title: 'dummy', project: project) }
let!(:label_link) { create(:label_link, label: label, target: issue) } let!(:label_link) { create(:label_link, label: label, target: issue) }
......
...@@ -85,18 +85,6 @@ describe API::ProjectSnippets, api: true do ...@@ -85,18 +85,6 @@ describe API::ProjectSnippets, api: true do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end end
context 'when the project is private' do
let(:private_project) { create(:project_empty_repo, :private) }
context 'when the snippet is public' do
it 'creates the snippet' do
expect { create_snippet(private_project, visibility_level: Snippet::PUBLIC) }.
to change { Snippet.count }.by(1)
end
end
end
context 'when the project is public' do
context 'when the snippet is private' do context 'when the snippet is private' do
it 'creates the snippet' do it 'creates the snippet' do
expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }. expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }.
...@@ -108,7 +96,9 @@ describe API::ProjectSnippets, api: true do ...@@ -108,7 +96,9 @@ describe API::ProjectSnippets, api: true do
it 'rejects the shippet' do it 'rejects the shippet' do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }. expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }.
not_to change { Snippet.count } not_to change { Snippet.count }
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']).to eq({ "error" => "Spam detected" })
end end
it 'creates a spam log' do it 'creates a spam log' do
...@@ -118,10 +108,10 @@ describe API::ProjectSnippets, api: true do ...@@ -118,10 +108,10 @@ describe API::ProjectSnippets, api: true do
end end
end end
end end
end
describe 'PUT /projects/:project_id/snippets/:id/' do describe 'PUT /projects/:project_id/snippets/:id/' do
let(:snippet) { create(:project_snippet, author: admin) } let(:visibility_level) { Snippet::PUBLIC }
let(:snippet) { create(:project_snippet, author: admin, visibility_level: visibility_level) }
it 'updates snippet' do it 'updates snippet' do
new_content = 'New content' new_content = 'New content'
...@@ -145,6 +135,56 @@ describe API::ProjectSnippets, api: true do ...@@ -145,6 +135,56 @@ describe API::ProjectSnippets, api: true do
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
end end
context 'when the snippet is spam' do
def update_snippet(snippet_params = {})
put v3_api("/projects/#{snippet.project.id}/snippets/#{snippet.id}", admin), snippet_params
end
before do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end
context 'when the snippet is private' do
let(:visibility_level) { Snippet::PRIVATE }
it 'creates the snippet' do
expect { update_snippet(title: 'Foo') }.
to change { snippet.reload.title }.to('Foo')
end
end
context 'when the snippet is public' do
let(:visibility_level) { Snippet::PUBLIC }
it 'rejects the snippet' do
expect { update_snippet(title: 'Foo') }.
not_to change { snippet.reload.title }
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo') }.
to change { SpamLog.count }.by(1)
end
end
context 'when the private snippet is made public' do
let(:visibility_level) { Snippet::PRIVATE }
it 'rejects the snippet' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
not_to change { snippet.reload.title }
expect(response).to have_http_status(400)
expect(json_response['message']).to eq({ "error" => "Spam detected" })
end
it 'creates a spam log' do
expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
end
end
end end
describe 'DELETE /projects/:project_id/snippets/:id/' do describe 'DELETE /projects/:project_id/snippets/:id/' do
......
...@@ -229,16 +229,6 @@ describe Issues::CreateService, services: true do ...@@ -229,16 +229,6 @@ describe Issues::CreateService, services: true do
expect { issue }.not_to change{SpamLog.last.recaptcha_verified} expect { issue }.not_to change{SpamLog.last.recaptcha_verified}
end end
end end
context 'when spam log title does not match the issue title' do
before do
opts[:title] = 'Another issue'
end
it 'does not mark spam_log as recaptcha_verified' do
expect { issue }.not_to change{SpamLog.last.recaptcha_verified}
end
end
end end
context 'when recaptcha was not verified' do context 'when recaptcha was not verified' do
......
require 'spec_helper' require 'spec_helper'
describe SpamService, services: true do describe SpamService, services: true do
describe '#check' do describe '#when_recaptcha_verified' do
def check_spam(issue, request, recaptcha_verified)
described_class.new(issue, request).when_recaptcha_verified(recaptcha_verified) do
'yielded'
end
end
it 'yields block when recaptcha was already verified' do
issue = build_stubbed(:issue)
expect(check_spam(issue, nil, true)).to eql('yielded')
end
context 'when recaptcha was not verified' do
let(:project) { create(:project, :public) } let(:project) { create(:project, :public) }
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:request) { double(:request, env: {}) } let(:request) { double(:request, env: {}) }
def check_spam(issue, request)
described_class.new(issue, request).check
end
context 'when indicated as spam by akismet' do context 'when indicated as spam by akismet' do
before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: true)) } before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: true)) }
it 'returns false when request is missing' do it 'doesnt check as spam when request is missing' do
expect(check_spam(issue, nil)).to be_falsey check_spam(issue, nil, false)
expect(issue.spam).to be_falsey
end end
it 'returns false when issue is not public' do it 'checks as spam' do
issue = create(:issue, project: create(:project, :private)) check_spam(issue, request, false)
expect(check_spam(issue, request)).to be_falsey expect(issue.spam).to be_truthy
end end
it 'returns true' do it 'creates a spam log' do
expect(check_spam(issue, request)).to be_truthy expect { check_spam(issue, request, false) }
.to change { SpamLog.count }.from(0).to(1)
end end
it 'creates a spam log' do it 'doesnt yield block' do
expect { check_spam(issue, request) }.to change { SpamLog.count }.from(0).to(1) expect(check_spam(issue, request, false))
.to eql(SpamLog.last)
end end
end end
...@@ -36,11 +49,13 @@ describe SpamService, services: true do ...@@ -36,11 +49,13 @@ describe SpamService, services: true do
before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: false)) } before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: false)) }
it 'returns false' do it 'returns false' do
expect(check_spam(issue, request)).to be_falsey expect(check_spam(issue, request, false)).to be_falsey
end end
it 'does not create a spam log' do it 'does not create a spam log' do
expect { check_spam(issue, request) }.not_to change { SpamLog.count } expect { check_spam(issue, request, false) }
.not_to change { SpamLog.count }
end
end end
end end
end 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