Commit 29a2843c authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'snippet-spam' into 'master'

Snippet spam

Closes #26276

See merge request !8911
parents ee59a64b 34918d94
...@@ -7,7 +7,7 @@ module SpammableActions ...@@ -7,7 +7,7 @@ module SpammableActions
def mark_as_spam def mark_as_spam
if SpamService.new(spammable).mark_as_spam! if SpamService.new(spammable).mark_as_spam!
redirect_to spammable, notice: "#{spammable.class} was submitted to Akismet successfully." redirect_to spammable, notice: "#{spammable.spammable_entity_type.titlecase} was submitted to Akismet successfully."
else else
redirect_to spammable, alert: 'Error with Akismet. Please check the logs for more info.' redirect_to spammable, alert: 'Error with Akismet. Please check the logs for more info.'
end end
......
class Projects::SnippetsController < Projects::ApplicationController class Projects::SnippetsController < Projects::ApplicationController
include ToggleAwardEmoji include ToggleAwardEmoji
include SpammableActions
before_action :module_enabled before_action :module_enabled
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji] before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam]
# Allow read any snippet # Allow read any snippet
before_action :authorize_read_project_snippet!, except: [:new, :create, :index] before_action :authorize_read_project_snippet!, except: [:new, :create, :index]
...@@ -36,8 +37,8 @@ class Projects::SnippetsController < Projects::ApplicationController ...@@ -36,8 +37,8 @@ class Projects::SnippetsController < Projects::ApplicationController
end end
def create def create
@snippet = CreateSnippetService.new(@project, current_user, create_params = snippet_params.merge(request: request)
snippet_params).execute @snippet = CreateSnippetService.new(@project, current_user, create_params).execute
if @snippet.valid? if @snippet.valid?
respond_with(@snippet, respond_with(@snippet,
...@@ -88,6 +89,7 @@ class Projects::SnippetsController < Projects::ApplicationController ...@@ -88,6 +89,7 @@ class Projects::SnippetsController < Projects::ApplicationController
@snippet ||= @project.snippets.find(params[:id]) @snippet ||= @project.snippets.find(params[:id])
end end
alias_method :awardable, :snippet alias_method :awardable, :snippet
alias_method :spammable, :snippet
def authorize_read_project_snippet! def authorize_read_project_snippet!
return render_404 unless can?(current_user, :read_project_snippet, @snippet) return render_404 unless can?(current_user, :read_project_snippet, @snippet)
......
class SnippetsController < ApplicationController class SnippetsController < ApplicationController
include ToggleAwardEmoji include ToggleAwardEmoji
include SpammableActions
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :download] before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :download]
...@@ -40,8 +41,8 @@ class SnippetsController < ApplicationController ...@@ -40,8 +41,8 @@ class SnippetsController < ApplicationController
end end
def create def create
@snippet = CreateSnippetService.new(nil, current_user, create_params = snippet_params.merge(request: request)
snippet_params).execute @snippet = CreateSnippetService.new(nil, current_user, create_params).execute
respond_with @snippet.becomes(Snippet) respond_with @snippet.becomes(Snippet)
end end
...@@ -96,6 +97,7 @@ class SnippetsController < ApplicationController ...@@ -96,6 +97,7 @@ class SnippetsController < ApplicationController
end end
end end
alias_method :awardable, :snippet alias_method :awardable, :snippet
alias_method :spammable, :snippet
def authorize_read_snippet! def authorize_read_snippet!
authenticate_user! unless can?(current_user, :read_personal_snippet, @snippet) authenticate_user! unless can?(current_user, :read_personal_snippet, @snippet)
......
...@@ -93,10 +93,6 @@ module VisibilityLevelHelper ...@@ -93,10 +93,6 @@ module VisibilityLevelHelper
current_application_settings.default_project_visibility current_application_settings.default_project_visibility
end end
def default_snippet_visibility
current_application_settings.default_snippet_visibility
end
def default_group_visibility def default_group_visibility
current_application_settings.default_group_visibility current_application_settings.default_group_visibility
end end
......
...@@ -34,7 +34,13 @@ module Spammable ...@@ -34,7 +34,13 @@ module Spammable
end end
def check_for_spam def check_for_spam
self.errors.add(:base, "Your #{self.class.name.underscore} has been recognized as spam and has been discarded.") if spam? if spam?
self.errors.add(:base, "Your #{spammable_entity_type} has been recognized as spam and has been discarded.")
end
end
def spammable_entity_type
self.class.name.underscore
end end
def spam_title def spam_title
......
...@@ -9,4 +9,8 @@ class ProjectSnippet < Snippet ...@@ -9,4 +9,8 @@ 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
...@@ -7,6 +7,7 @@ class Snippet < ActiveRecord::Base ...@@ -7,6 +7,7 @@ class Snippet < ActiveRecord::Base
include Sortable include Sortable
include Awardable include Awardable
include Mentionable include Mentionable
include Spammable
cache_markdown_field :title, pipeline: :single_line cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :content cache_markdown_field :content
...@@ -17,7 +18,7 @@ class Snippet < ActiveRecord::Base ...@@ -17,7 +18,7 @@ class Snippet < ActiveRecord::Base
default_content_html_invalidator || file_name_changed? default_content_html_invalidator || file_name_changed?
end end
default_value_for :visibility_level, Snippet::PRIVATE default_value_for(:visibility_level) { current_application_settings.default_snippet_visibility }
belongs_to :author, class_name: 'User' belongs_to :author, class_name: 'User'
belongs_to :project belongs_to :project
...@@ -46,6 +47,9 @@ class Snippet < ActiveRecord::Base ...@@ -46,6 +47,9 @@ class Snippet < ActiveRecord::Base
participant :author participant :author
participant :notes_with_associations participant :notes_with_associations
attr_spammable :title, spam_title: true
attr_spammable :content, spam_description: true
def self.reference_prefix def self.reference_prefix
'$' '$'
end end
...@@ -127,6 +131,14 @@ class Snippet < ActiveRecord::Base ...@@ -127,6 +131,14 @@ class Snippet < ActiveRecord::Base
notes.includes(:author) notes.includes(:author)
end end
def check_for_spam?
public?
end
def spammable_entity_type
'snippet'
end
class << self class << self
# Searches for snippets with a matching title or file name. # Searches for snippets with a matching title or file name.
# #
......
class CreateSnippetService < BaseService class CreateSnippetService < BaseService
def execute def execute
request = params.delete(:request)
api = params.delete(:api)
snippet = if project snippet = if project
project.snippets.build(params) project.snippets.build(params)
else else
...@@ -12,8 +15,12 @@ class CreateSnippetService < BaseService ...@@ -12,8 +15,12 @@ class CreateSnippetService < BaseService
end end
snippet.author = current_user snippet.author = current_user
snippet.spam = SpamService.new(snippet, request).check(api)
if snippet.save
UserAgentDetailService.new(snippet, request).create
end
snippet.save
snippet snippet
end end
end end
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
- if can?(current_user, :create_project_snippet, @project) - if can?(current_user, :create_project_snippet, @project)
= link_to new_namespace_project_snippet_path(@project.namespace, @project), class: 'btn btn-grouped btn-inverted btn-create', title: "New snippet" do = link_to new_namespace_project_snippet_path(@project.namespace, @project), class: 'btn btn-grouped btn-inverted btn-create', title: "New snippet" do
New snippet New snippet
- if @snippet.submittable_as_spam? && current_user.admin?
= link_to 'Submit as spam', mark_as_spam_namespace_project_snippet_path(@project.namespace, @project, @snippet), method: :post, class: 'btn btn-grouped btn-spam', title: 'Submit as spam'
- if can?(current_user, :create_project_snippet, @project) || can?(current_user, :update_project_snippet, @snippet) - if can?(current_user, :create_project_snippet, @project) || can?(current_user, :update_project_snippet, @snippet)
.visible-xs-block.dropdown .visible-xs-block.dropdown
%button.btn.btn-default.btn-block.append-bottom-0.prepend-top-5{ data: { toggle: "dropdown" } } %button.btn.btn-default.btn-block.append-bottom-0.prepend-top-5{ data: { toggle: "dropdown" } }
...@@ -27,3 +29,6 @@ ...@@ -27,3 +29,6 @@
%li %li
= link_to edit_namespace_project_snippet_path(@project.namespace, @project, @snippet) do = link_to edit_namespace_project_snippet_path(@project.namespace, @project, @snippet) do
Edit Edit
- if @snippet.submittable_as_spam? && current_user.admin?
%li
= link_to 'Submit as spam', mark_as_spam_namespace_project_snippet_path(@project.namespace, @project, @snippet), method: :post
...@@ -3,4 +3,4 @@ ...@@ -3,4 +3,4 @@
%h3.page-title %h3.page-title
Edit Snippet Edit Snippet
%hr %hr
= render "shared/snippets/form", url: namespace_project_snippet_path(@project.namespace, @project, @snippet), visibility_level: @snippet.visibility_level = render "shared/snippets/form", url: namespace_project_snippet_path(@project.namespace, @project, @snippet)
...@@ -3,4 +3,4 @@ ...@@ -3,4 +3,4 @@
%h3.page-title %h3.page-title
New Snippet New Snippet
%hr %hr
= render "shared/snippets/form", url: namespace_project_snippets_path(@project.namespace, @project, @snippet), visibility_level: default_snippet_visibility = render "shared/snippets/form", url: namespace_project_snippets_path(@project.namespace, @project, @snippet)
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
.col-sm-10 .col-sm-10
= f.text_field :title, class: 'form-control', required: true, autofocus: true = f.text_field :title, class: 'form-control', required: true, autofocus: true
= render 'shared/visibility_level', f: f, visibility_level: visibility_level, can_change_visibility_level: true, form_model: @snippet = render 'shared/visibility_level', f: f, visibility_level: @snippet.visibility_level, can_change_visibility_level: true, form_model: @snippet
.file-editor .file-editor
.form-group .form-group
...@@ -34,4 +34,3 @@ ...@@ -34,4 +34,3 @@
= link_to "Cancel", namespace_project_snippets_path(@project.namespace, @project), class: "btn btn-cancel" = link_to "Cancel", namespace_project_snippets_path(@project.namespace, @project), class: "btn btn-cancel"
- else - else
= link_to "Cancel", snippets_path(@project), class: "btn btn-cancel" = link_to "Cancel", snippets_path(@project), class: "btn btn-cancel"
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
- if current_user - if current_user
= link_to new_snippet_path, class: "btn btn-grouped btn-inverted btn-create", title: "New snippet" do = link_to new_snippet_path, class: "btn btn-grouped btn-inverted btn-create", title: "New snippet" do
New snippet New snippet
- if @snippet.submittable_as_spam? && current_user.admin?
= link_to 'Submit as spam', mark_as_spam_snippet_path(@snippet), method: :post, class: 'btn btn-grouped btn-spam', title: 'Submit as spam'
- if current_user - if current_user
.visible-xs-block.dropdown .visible-xs-block.dropdown
%button.btn.btn-default.btn-block.append-bottom-0.prepend-top-5{ data: { toggle: "dropdown" } } %button.btn.btn-default.btn-block.append-bottom-0.prepend-top-5{ data: { toggle: "dropdown" } }
...@@ -26,3 +28,6 @@ ...@@ -26,3 +28,6 @@
%li %li
= link_to edit_snippet_path(@snippet) do = link_to edit_snippet_path(@snippet) do
Edit Edit
- if @snippet.submittable_as_spam? && current_user.admin?
%li
= link_to 'Submit as spam', mark_as_spam_snippet_path(@snippet), method: :post
...@@ -2,4 +2,4 @@ ...@@ -2,4 +2,4 @@
%h3.page-title %h3.page-title
Edit Snippet Edit Snippet
%hr %hr
= render 'shared/snippets/form', url: snippet_path(@snippet), visibility_level: @snippet.visibility_level = render 'shared/snippets/form', url: snippet_path(@snippet)
...@@ -2,4 +2,4 @@ ...@@ -2,4 +2,4 @@
%h3.page-title %h3.page-title
New Snippet New Snippet
%hr %hr
= render "shared/snippets/form", url: snippets_path(@snippet), visibility_level: default_snippet_visibility = render "shared/snippets/form", url: snippets_path(@snippet)
---
title: Check public snippets for spam
merge_request:
author:
...@@ -64,6 +64,7 @@ constraints(ProjectUrlConstrainer.new) do ...@@ -64,6 +64,7 @@ constraints(ProjectUrlConstrainer.new) do
resources :snippets, concerns: :awardable, constraints: { id: /\d+/ } do resources :snippets, concerns: :awardable, constraints: { id: /\d+/ } do
member do member do
get 'raw' get 'raw'
post :mark_as_spam
end end
end end
......
...@@ -2,6 +2,7 @@ resources :snippets, concerns: :awardable do ...@@ -2,6 +2,7 @@ resources :snippets, concerns: :awardable do
member do member do
get 'raw' get 'raw'
get 'download' get 'download'
post :mark_as_spam
end end
end end
......
...@@ -58,7 +58,7 @@ module API ...@@ -58,7 +58,7 @@ module API
end end
post ":id/snippets" do post ":id/snippets" do
authorize! :create_project_snippet, user_project authorize! :create_project_snippet, user_project
snippet_params = declared_params snippet_params = declared_params.merge(request: request, api: true)
snippet_params[:content] = snippet_params.delete(:code) snippet_params[:content] = snippet_params.delete(:code)
snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute
......
...@@ -64,7 +64,7 @@ module API ...@@ -64,7 +64,7 @@ module API
desc: 'The visibility level of the snippet' desc: 'The visibility level of the snippet'
end end
post do post do
attrs = declared_params(include_missing: false) 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
if snippet.persisted? if snippet.persisted?
......
...@@ -6,8 +6,8 @@ describe Projects::SnippetsController do ...@@ -6,8 +6,8 @@ describe Projects::SnippetsController do
let(:user2) { create(:user) } let(:user2) { create(:user) }
before do before do
project.team << [user, :master] project.add_master(user)
project.team << [user2, :master] project.add_master(user2)
end end
describe 'GET #index' do describe 'GET #index' do
...@@ -69,6 +69,86 @@ describe Projects::SnippetsController do ...@@ -69,6 +69,86 @@ describe Projects::SnippetsController do
end end
end end
describe 'POST #create' do
def create_snippet(project, snippet_params = {})
sign_in(user)
project.add_developer(user)
post :create, {
namespace_id: project.namespace.to_param,
project_id: project.to_param,
project_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params)
}
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 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
it 'creates the snippet' do
expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }.
to change { Snippet.count }.by(1)
end
end
context 'when the snippet is public' do
it 'rejects the shippet' do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }.
not_to change { Snippet.count }
expect(response).to render_template(:new)
end
it 'creates a spam log' do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
end
end
end
end
describe 'POST #mark_as_spam' do
let(:snippet) { create(:project_snippet, :private, project: project, author: user) }
before do
allow_any_instance_of(AkismetService).to receive_messages(submit_spam: true)
stub_application_setting(akismet_enabled: true)
end
def mark_as_spam
admin = create(:admin)
create(:user_agent_detail, subject: snippet)
project.add_master(admin)
sign_in(admin)
post :mark_as_spam,
namespace_id: project.namespace.path,
project_id: project.path,
id: snippet.id
end
it 'updates the snippet' do
mark_as_spam
expect(snippet.reload).not_to be_submittable_as_spam
end
end
%w[show raw].each do |action| %w[show raw].each do |action|
describe "GET ##{action}" do describe "GET ##{action}" do
context 'when the project snippet is private' do context 'when the project snippet is private' do
......
...@@ -138,6 +138,65 @@ describe SnippetsController do ...@@ -138,6 +138,65 @@ describe SnippetsController do
end end
end end
describe 'POST #create' do
def create_snippet(snippet_params = {})
sign_in(user)
post :create, {
personal_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params)
}
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
it 'creates the snippet' do
expect { create_snippet(visibility_level: Snippet::PRIVATE) }.
to change { Snippet.count }.by(1)
end
end
context 'when the snippet is public' do
it 'rejects the shippet' do
expect { create_snippet(visibility_level: Snippet::PUBLIC) }.
not_to change { Snippet.count }
expect(response).to render_template(:new)
end
it 'creates a spam log' do
expect { create_snippet(visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
end
end
end
describe 'POST #mark_as_spam' do
let(:snippet) { create(:personal_snippet, :public, author: user) }
before do
allow_any_instance_of(AkismetService).to receive_messages(submit_spam: true)
stub_application_setting(akismet_enabled: true)
end
def mark_as_spam
admin = create(:admin)
create(:user_agent_detail, subject: snippet)
sign_in(admin)
post :mark_as_spam, id: snippet.id
end
it 'updates the snippet' do
mark_as_spam
expect(snippet.reload).not_to be_submittable_as_spam
end
end
%w(raw download).each do |action| %w(raw download).each do |action|
describe "GET #{action}" do describe "GET #{action}" do
context 'when the personal snippet is private' do context 'when the personal snippet is private' do
......
...@@ -52,6 +52,7 @@ snippets: ...@@ -52,6 +52,7 @@ snippets:
- project - project
- notes - notes
- award_emoji - award_emoji
- user_agent_detail
releases: releases:
- project - project
project_members: project_members:
......
...@@ -4,6 +4,7 @@ describe API::ProjectSnippets, api: true do ...@@ -4,6 +4,7 @@ describe API::ProjectSnippets, api: true do
include ApiHelpers include ApiHelpers
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :public) }
let(:user) { create(:user) }
let(:admin) { create(:admin) } let(:admin) { create(:admin) }
describe 'GET /projects/:project_id/snippets/:id' do describe 'GET /projects/:project_id/snippets/:id' do
...@@ -22,7 +23,7 @@ describe API::ProjectSnippets, api: true do ...@@ -22,7 +23,7 @@ describe API::ProjectSnippets, api: true do
let(:user) { create(:user) } let(:user) { create(:user) }
it 'returns all snippets available to team member' do it 'returns all snippets available to team member' do
project.team << [user, :developer] project.add_developer(user)
public_snippet = create(:project_snippet, :public, project: project) public_snippet = create(:project_snippet, :public, project: project)
internal_snippet = create(:project_snippet, :internal, project: project) internal_snippet = create(:project_snippet, :internal, project: project)
private_snippet = create(:project_snippet, :private, project: project) private_snippet = create(:project_snippet, :private, project: project)
...@@ -50,7 +51,7 @@ describe API::ProjectSnippets, api: true do ...@@ -50,7 +51,7 @@ describe API::ProjectSnippets, api: true do
title: 'Test Title', title: 'Test Title',
file_name: 'test.rb', file_name: 'test.rb',
code: 'puts "hello world"', code: 'puts "hello world"',
visibility_level: Gitlab::VisibilityLevel::PUBLIC visibility_level: Snippet::PUBLIC
} }
end end
...@@ -72,6 +73,51 @@ describe API::ProjectSnippets, api: true do ...@@ -72,6 +73,51 @@ 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 create_snippet(project, snippet_params = {})
project.add_developer(user)
post api("/projects/#{project.id}/snippets", user), params.merge(snippet_params)
end
before do
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
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
it 'creates the snippet' do
expect { create_snippet(project, visibility_level: Snippet::PRIVATE) }.
to change { Snippet.count }.by(1)
end
end
context 'when the snippet is public' do
it 'rejects the shippet' do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }.
not_to change { Snippet.count }
expect(response).to have_http_status(400)
end
it 'creates a spam log' do
expect { create_snippet(project, visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
end
end
end
end end
describe 'PUT /projects/:project_id/snippets/:id/' do describe 'PUT /projects/:project_id/snippets/:id/' do
......
...@@ -80,7 +80,7 @@ describe API::Snippets, api: true do ...@@ -80,7 +80,7 @@ describe API::Snippets, api: true do
title: 'Test Title', title: 'Test Title',
file_name: 'test.rb', file_name: 'test.rb',
content: 'puts "hello world"', content: 'puts "hello world"',
visibility_level: Gitlab::VisibilityLevel::PUBLIC visibility_level: Snippet::PUBLIC
} }
end end
...@@ -101,6 +101,36 @@ describe API::Snippets, api: true do ...@@ -101,6 +101,36 @@ 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 create_snippet(snippet_params = {})
post api('/snippets', user), params.merge(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
it 'creates the snippet' do
expect { create_snippet(visibility_level: Snippet::PRIVATE) }.
to change { Snippet.count }.by(1)
end
end
context 'when the snippet is public' do
it 'rejects the shippet' do
expect { create_snippet(visibility_level: Snippet::PUBLIC) }.
not_to change { Snippet.count }
expect(response).to have_http_status(400)
end
it 'creates a spam log' do
expect { create_snippet(visibility_level: Snippet::PUBLIC) }.
to change { SpamLog.count }.by(1)
end
end
end
end end
describe 'PUT /snippets/:id' do describe 'PUT /snippets/:id' do
......
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