Commit 8df474ea authored by Sean McGivern's avatar Sean McGivern

Check public snippets for spam

Apply the same spam checks to public snippets (either personal snippets
that are public, or public snippets on public projects) as to issues on
public projects.
parent b87e39d9
...@@ -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)
......
...@@ -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
......
...@@ -14,4 +14,8 @@ class ProjectSnippet < Snippet ...@@ -14,4 +14,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
...@@ -8,6 +8,7 @@ class Snippet < ActiveRecord::Base ...@@ -8,6 +8,7 @@ class Snippet < ActiveRecord::Base
include Elastic::SnippetsSearch include Elastic::SnippetsSearch
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
...@@ -47,6 +48,9 @@ class Snippet < ActiveRecord::Base ...@@ -47,6 +48,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
...@@ -128,6 +132,14 @@ class Snippet < ActiveRecord::Base ...@@ -128,6 +132,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
...@@ -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
---
title: Check public snippets for spam
merge_request:
author:
...@@ -70,6 +70,7 @@ constraints(ProjectUrlConstrainer.new) do ...@@ -70,6 +70,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?
......
...@@ -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.team << [user, :developer]
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.team << [admin, :master]
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
...@@ -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.team << [user, :developer]
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