diff options
| -rw-r--r-- | app/controllers/concerns/spammable_actions.rb | 2 | ||||
| -rw-r--r-- | app/controllers/projects/snippets_controller.rb | 8 | ||||
| -rw-r--r-- | app/controllers/snippets_controller.rb | 6 | ||||
| -rw-r--r-- | app/models/concerns/spammable.rb | 8 | ||||
| -rw-r--r-- | app/models/project_snippet.rb | 4 | ||||
| -rw-r--r-- | app/models/snippet.rb | 12 | ||||
| -rw-r--r-- | app/services/create_snippet_service.rb | 9 | ||||
| -rw-r--r-- | app/views/projects/snippets/_actions.html.haml | 5 | ||||
| -rw-r--r-- | app/views/snippets/_actions.html.haml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/snippet-spam.yml | 4 | ||||
| -rw-r--r-- | config/routes/project.rb | 1 | ||||
| -rw-r--r-- | config/routes/snippets.rb | 1 | ||||
| -rw-r--r-- | lib/api/project_snippets.rb | 2 | ||||
| -rw-r--r-- | lib/api/snippets.rb | 2 | ||||
| -rw-r--r-- | spec/controllers/projects/snippets_controller_spec.rb | 80 | ||||
| -rw-r--r-- | spec/controllers/snippets_controller_spec.rb | 59 | ||||
| -rw-r--r-- | spec/lib/gitlab/import_export/all_models.yml | 1 | ||||
| -rw-r--r-- | spec/requests/api/project_snippets_spec.rb | 48 | ||||
| -rw-r--r-- | spec/requests/api/snippets_spec.rb | 32 |
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 |