summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2017-02-01 18:15:59 +0000
committerSean McGivern <sean@gitlab.com>2017-02-02 10:23:51 +0000
commitc63194ce6f952173649d7de4038aa96348e90565 (patch)
treea9622e1e5ffc86bf35fc9556152cc03cf2841ef5
parentf799585c41d801bc657f992adf3d4b201af927d2 (diff)
downloadgitlab-ce-c63194ce6f952173649d7de4038aa96348e90565.tar.gz
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.
-rw-r--r--app/controllers/concerns/spammable_actions.rb2
-rw-r--r--app/controllers/projects/snippets_controller.rb8
-rw-r--r--app/controllers/snippets_controller.rb6
-rw-r--r--app/models/concerns/spammable.rb8
-rw-r--r--app/models/project_snippet.rb4
-rw-r--r--app/models/snippet.rb12
-rw-r--r--app/services/create_snippet_service.rb9
-rw-r--r--app/views/projects/snippets/_actions.html.haml5
-rw-r--r--app/views/snippets/_actions.html.haml5
-rw-r--r--changelogs/unreleased/snippet-spam.yml4
-rw-r--r--config/routes/project.rb1
-rw-r--r--config/routes/snippets.rb1
-rw-r--r--lib/api/project_snippets.rb2
-rw-r--r--lib/api/snippets.rb2
-rw-r--r--spec/controllers/projects/snippets_controller_spec.rb80
-rw-r--r--spec/controllers/snippets_controller_spec.rb59
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml1
-rw-r--r--spec/requests/api/project_snippets_spec.rb48
-rw-r--r--spec/requests/api/snippets_spec.rb32
19 files changed, 277 insertions, 12 deletions
diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb
index 99acd98ae13..562f92bd83c 100644
--- a/app/controllers/concerns/spammable_actions.rb
+++ b/app/controllers/concerns/spammable_actions.rb
@@ -7,7 +7,7 @@ module SpammableActions
def 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
redirect_to spammable, alert: 'Error with Akismet. Please check the logs for more info.'
end
diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb
index 02a97c1c574..5d193f26a8e 100644
--- a/app/controllers/projects/snippets_controller.rb
+++ b/app/controllers/projects/snippets_controller.rb
@@ -1,8 +1,9 @@
class Projects::SnippetsController < Projects::ApplicationController
include ToggleAwardEmoji
+ include SpammableActions
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
before_action :authorize_read_project_snippet!, except: [:new, :create, :index]
@@ -36,8 +37,8 @@ class Projects::SnippetsController < Projects::ApplicationController
end
def create
- @snippet = CreateSnippetService.new(@project, current_user,
- snippet_params).execute
+ create_params = snippet_params.merge(request: request)
+ @snippet = CreateSnippetService.new(@project, current_user, create_params).execute
if @snippet.valid?
respond_with(@snippet,
@@ -88,6 +89,7 @@ class Projects::SnippetsController < Projects::ApplicationController
@snippet ||= @project.snippets.find(params[:id])
end
alias_method :awardable, :snippet
+ alias_method :spammable, :snippet
def authorize_read_project_snippet!
return render_404 unless can?(current_user, :read_project_snippet, @snippet)
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index dee57e4a388..b169d993688 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -1,5 +1,6 @@
class SnippetsController < ApplicationController
include ToggleAwardEmoji
+ include SpammableActions
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :download]
@@ -40,8 +41,8 @@ class SnippetsController < ApplicationController
end
def create
- @snippet = CreateSnippetService.new(nil, current_user,
- snippet_params).execute
+ create_params = snippet_params.merge(request: request)
+ @snippet = CreateSnippetService.new(nil, current_user, create_params).execute
respond_with @snippet.becomes(Snippet)
end
@@ -96,6 +97,7 @@ class SnippetsController < ApplicationController
end
end
alias_method :awardable, :snippet
+ alias_method :spammable, :snippet
def authorize_read_snippet!
authenticate_user! unless can?(current_user, :read_personal_snippet, @snippet)
diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb
index 1aa97debe42..1acff093aa1 100644
--- a/app/models/concerns/spammable.rb
+++ b/app/models/concerns/spammable.rb
@@ -34,7 +34,13 @@ module Spammable
end
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
def spam_title
diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb
index 25b5d777641..9bb456eee24 100644
--- a/app/models/project_snippet.rb
+++ b/app/models/project_snippet.rb
@@ -9,4 +9,8 @@ class ProjectSnippet < Snippet
participant :author
participant :notes_with_associations
+
+ def check_for_spam?
+ super && project.public?
+ end
end
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 960f1521be9..2665a7249a3 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -7,6 +7,7 @@ class Snippet < ActiveRecord::Base
include Sortable
include Awardable
include Mentionable
+ include Spammable
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :content
@@ -46,6 +47,9 @@ class Snippet < ActiveRecord::Base
participant :author
participant :notes_with_associations
+ attr_spammable :title, spam_title: true
+ attr_spammable :content, spam_description: true
+
def self.reference_prefix
'$'
end
@@ -127,6 +131,14 @@ class Snippet < ActiveRecord::Base
notes.includes(:author)
end
+ def check_for_spam?
+ public?
+ end
+
+ def spammable_entity_type
+ 'snippet'
+ end
+
class << self
# Searches for snippets with a matching title or file name.
#
diff --git a/app/services/create_snippet_service.rb b/app/services/create_snippet_service.rb
index 95cc9baf406..14f5ba064ff 100644
--- a/app/services/create_snippet_service.rb
+++ b/app/services/create_snippet_service.rb
@@ -1,5 +1,8 @@
class CreateSnippetService < BaseService
def execute
+ request = params.delete(:request)
+ api = params.delete(:api)
+
snippet = if project
project.snippets.build(params)
else
@@ -12,8 +15,12 @@ class CreateSnippetService < BaseService
end
snippet.author = current_user
+ snippet.spam = SpamService.new(snippet, request).check(api)
+
+ if snippet.save
+ UserAgentDetailService.new(snippet, request).create
+ end
- snippet.save
snippet
end
end
diff --git a/app/views/projects/snippets/_actions.html.haml b/app/views/projects/snippets/_actions.html.haml
index 068a6610350..e2a5107a883 100644
--- a/app/views/projects/snippets/_actions.html.haml
+++ b/app/views/projects/snippets/_actions.html.haml
@@ -8,6 +8,8 @@
- 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
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)
.visible-xs-block.dropdown
%button.btn.btn-default.btn-block.append-bottom-0.prepend-top-5{ data: { toggle: "dropdown" } }
@@ -27,3 +29,6 @@
%li
= link_to edit_namespace_project_snippet_path(@project.namespace, @project, @snippet) do
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
diff --git a/app/views/snippets/_actions.html.haml b/app/views/snippets/_actions.html.haml
index 95fc7198104..9a9a3ff9220 100644
--- a/app/views/snippets/_actions.html.haml
+++ b/app/views/snippets/_actions.html.haml
@@ -8,6 +8,8 @@
- if current_user
= link_to new_snippet_path, class: "btn btn-grouped btn-inverted btn-create", title: "New snippet" do
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
.visible-xs-block.dropdown
%button.btn.btn-default.btn-block.append-bottom-0.prepend-top-5{ data: { toggle: "dropdown" } }
@@ -26,3 +28,6 @@
%li
= link_to edit_snippet_path(@snippet) do
Edit
+ - if @snippet.submittable_as_spam? && current_user.admin?
+ %li
+ = link_to 'Submit as spam', mark_as_spam_snippet_path(@snippet), method: :post
diff --git a/changelogs/unreleased/snippet-spam.yml b/changelogs/unreleased/snippet-spam.yml
new file mode 100644
index 00000000000..4867f088953
--- /dev/null
+++ b/changelogs/unreleased/snippet-spam.yml
@@ -0,0 +1,4 @@
+---
+title: Check public snippets for spam
+merge_request:
+author:
diff --git a/config/routes/project.rb b/config/routes/project.rb
index f36febc6e04..efe2fbc521d 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -64,6 +64,7 @@ constraints(ProjectUrlConstrainer.new) do
resources :snippets, concerns: :awardable, constraints: { id: /\d+/ } do
member do
get 'raw'
+ post :mark_as_spam
end
end
diff --git a/config/routes/snippets.rb b/config/routes/snippets.rb
index 3ca096f31ba..ce0d1314292 100644
--- a/config/routes/snippets.rb
+++ b/config/routes/snippets.rb
@@ -2,6 +2,7 @@ resources :snippets, concerns: :awardable do
member do
get 'raw'
get 'download'
+ post :mark_as_spam
end
end
diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb
index 9d8c5b63685..dcc0c82ee27 100644
--- a/lib/api/project_snippets.rb
+++ b/lib/api/project_snippets.rb
@@ -58,7 +58,7 @@ module API
end
post ":id/snippets" do
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 = CreateSnippetService.new(user_project, current_user, snippet_params).execute
diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb
index e096e636806..eb9ece49e7f 100644
--- a/lib/api/snippets.rb
+++ b/lib/api/snippets.rb
@@ -64,7 +64,7 @@ module API
desc: 'The visibility level of the snippet'
end
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
if snippet.persisted?
diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb
index 32b0e42c3cd..88e4f81f232 100644
--- a/spec/controllers/projects/snippets_controller_spec.rb
+++ b/spec/controllers/projects/snippets_controller_spec.rb
@@ -69,6 +69,86 @@ describe Projects::SnippetsController do
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|
describe "GET ##{action}" do
context 'when the project snippet is private' do
diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb
index d76fe9f580f..dadcb90cfc2 100644
--- a/spec/controllers/snippets_controller_spec.rb
+++ b/spec/controllers/snippets_controller_spec.rb
@@ -138,6 +138,65 @@ describe SnippetsController do
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|
describe "GET #{action}" do
context 'when the personal snippet is private' do
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 7fb6829f582..20241d4d63e 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -52,6 +52,7 @@ snippets:
- project
- notes
- award_emoji
+- user_agent_detail
releases:
- project
project_members:
diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb
index 01032c0929b..9e25e30bc86 100644
--- a/spec/requests/api/project_snippets_spec.rb
+++ b/spec/requests/api/project_snippets_spec.rb
@@ -4,6 +4,7 @@ describe API::ProjectSnippets, api: true do
include ApiHelpers
let(:project) { create(:empty_project, :public) }
+ let(:user) { create(:user) }
let(:admin) { create(:admin) }
describe 'GET /projects/:project_id/snippets/:id' do
@@ -50,7 +51,7 @@ describe API::ProjectSnippets, api: true do
title: 'Test Title',
file_name: 'test.rb',
code: 'puts "hello world"',
- visibility_level: Gitlab::VisibilityLevel::PUBLIC
+ visibility_level: Snippet::PUBLIC
}
end
@@ -72,6 +73,51 @@ describe API::ProjectSnippets, api: true do
expect(response).to have_http_status(400)
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
describe 'PUT /projects/:project_id/snippets/:id/' do
diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb
index f6fb6ea5506..6b9a739b439 100644
--- a/spec/requests/api/snippets_spec.rb
+++ b/spec/requests/api/snippets_spec.rb
@@ -80,7 +80,7 @@ describe API::Snippets, api: true do
title: 'Test Title',
file_name: 'test.rb',
content: 'puts "hello world"',
- visibility_level: Gitlab::VisibilityLevel::PUBLIC
+ visibility_level: Snippet::PUBLIC
}
end
@@ -101,6 +101,36 @@ describe API::Snippets, api: true do
expect(response).to have_http_status(400)
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
describe 'PUT /snippets/:id' do