diff options
| author | Rémy Coutable <remy@rymai.me> | 2017-02-02 16:29:04 +0000 | 
|---|---|---|
| committer | Rémy Coutable <remy@rymai.me> | 2017-02-02 16:29:04 +0000 | 
| commit | 29a2843c229ab5c8be4884e63f062a134eb04bef (patch) | |
| tree | 773d1f02d1652005e93c457a17dddfe91f3cb32f | |
| parent | ee59a64b2035622b02760d5ab928ea59a35cd45f (diff) | |
| parent | 34918d94c011e8f81bd962d43d67fe8bd9f21e3e (diff) | |
| download | gitlab-ce-29a2843c229ab5c8be4884e63f062a134eb04bef.tar.gz | |
Merge branch 'snippet-spam' into 'master'
Snippet spam
Closes #26276
See merge request !8911
25 files changed, 286 insertions, 26 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/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 3a83ae15dd8..fc93acfe63e 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -93,10 +93,6 @@ module VisibilityLevelHelper      current_application_settings.default_project_visibility    end -  def default_snippet_visibility -    current_application_settings.default_snippet_visibility -  end -    def default_group_visibility      current_application_settings.default_group_visibility    end 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 771a7350556..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 @@ -17,7 +18,7 @@ class Snippet < ActiveRecord::Base      default_content_html_invalidator || file_name_changed?    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 :project @@ -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/projects/snippets/edit.html.haml b/app/views/projects/snippets/edit.html.haml index 216f70f5605..fb39028529d 100644 --- a/app/views/projects/snippets/edit.html.haml +++ b/app/views/projects/snippets/edit.html.haml @@ -3,4 +3,4 @@  %h3.page-title    Edit Snippet  %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) diff --git a/app/views/projects/snippets/new.html.haml b/app/views/projects/snippets/new.html.haml index 772a594269c..cfed3a79bc5 100644 --- a/app/views/projects/snippets/new.html.haml +++ b/app/views/projects/snippets/new.html.haml @@ -3,4 +3,4 @@  %h3.page-title    New Snippet  %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) diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml index 0c788032020..2d22782eb36 100644 --- a/app/views/shared/snippets/_form.html.haml +++ b/app/views/shared/snippets/_form.html.haml @@ -11,7 +11,7 @@        .col-sm-10          = 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        .form-group @@ -34,4 +34,3 @@          = link_to "Cancel", namespace_project_snippets_path(@project.namespace, @project), class: "btn btn-cancel"        - else          = link_to "Cancel", snippets_path(@project), class: "btn btn-cancel" - 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/app/views/snippets/edit.html.haml b/app/views/snippets/edit.html.haml index 82f44a9a5c3..915bf98eb3e 100644 --- a/app/views/snippets/edit.html.haml +++ b/app/views/snippets/edit.html.haml @@ -2,4 +2,4 @@  %h3.page-title    Edit Snippet  %hr -= render 'shared/snippets/form', url: snippet_path(@snippet), visibility_level: @snippet.visibility_level += render 'shared/snippets/form', url: snippet_path(@snippet) diff --git a/app/views/snippets/new.html.haml b/app/views/snippets/new.html.haml index 79e2392490d..ca8afb4bb6a 100644 --- a/app/views/snippets/new.html.haml +++ b/app/views/snippets/new.html.haml @@ -2,4 +2,4 @@  %h3.page-title    New Snippet  %hr -= render "shared/snippets/form", url: snippets_path(@snippet), visibility_level: default_snippet_visibility += render "shared/snippets/form", url: snippets_path(@snippet) 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..19e948d8fb8 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -6,8 +6,8 @@ describe Projects::SnippetsController do    let(:user2)   { create(:user) }    before do -    project.team << [user, :master] -    project.team << [user2, :master] +    project.add_master(user) +    project.add_master(user2)    end    describe 'GET #index' do @@ -69,6 +69,86 @@ describe Projects::SnippetsController do      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|      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..45d5ae267c5 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 @@ -22,7 +23,7 @@ describe API::ProjectSnippets, api: true do      let(:user) { create(:user) }      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)        internal_snippet = create(:project_snippet, :internal, project: project)        private_snippet = create(:project_snippet, :private, project: project) @@ -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.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    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 | 
