Merge branch 'fj-37436-fix-create-personal-snippet-ability' into 'master'

Include create_personal_snippet ability for new/create snippet actions

See merge request gitlab-org/gitlab!20838

app/controllers/snippets_controller.rb

@@ -15,13 +15,9 @@ class SnippetsController < ApplicationController
 
   before_action :snippet, only: [:show, :edit, :destroy, :update, :raw]
 
-  # Allow read snippet
+  before_action :authorize_create_snippet!, only: [:new, :create]
   before_action :authorize_read_snippet!, only: [:show, :raw]
-
-  # Allow modify snippet
   before_action :authorize_update_snippet!, only: [:edit, :update]
-
-  # Allow destroy snippet
   before_action :authorize_admin_snippet!, only: [:destroy]
 
   skip_before_action :authenticate_user!, only: [:index, :show, :raw]
@@ -140,6 +136,10 @@ class SnippetsController < ApplicationController
     return render_404 unless can?(current_user, :admin_personal_snippet, @snippet)
   end
 
+  def authorize_create_snippet!
+    return render_404 unless can?(current_user, :create_personal_snippet)
+  end
+
   def snippet_params
     params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description)
   end

app/policies/global_policy.rb

@@ -75,12 +75,15 @@ class GlobalPolicy < BasePolicy
 
   rule { ~anonymous }.policy do
     enable :read_instance_metadata
+    enable :create_personal_snippet
   end
 
   rule { admin }.policy do
     enable :read_custom_attribute
     enable :update_custom_attribute
   end
+
+  rule { external_user }.prevent :create_personal_snippet
 end
 
 GlobalPolicy.prepend_if_ee('EE::GlobalPolicy')

app/policies/personal_snippet_policy.rb

@@ -17,9 +17,6 @@ class PersonalSnippetPolicy < BasePolicy
     enable :create_note
   end
 
-  rule { ~anonymous }.enable :create_personal_snippet
-  rule { external_user }.prevent :create_personal_snippet
-
   rule { internal_snippet & ~external_user }.policy do
     enable :read_personal_snippet
     enable :create_note

app/views/layouts/header/_new_dropdown.haml

@@ -38,4 +38,5 @@
         %li= link_to _('New project'), new_project_path, class: 'qa-global-new-project-link'
       - if current_user.can_create_group?
         %li= link_to _('New group'), new_group_path
-      %li= link_to _('New snippet'), new_snippet_path, class: 'qa-global-new-snippet-link'
+      - if current_user.can?(:create_personal_snippet)
+        %li= link_to _('New snippet'), new_snippet_path, class: 'qa-global-new-snippet-link'

app/views/snippets/_actions.html.haml

@@ -7,8 +7,9 @@
   - if can?(current_user, :admin_personal_snippet, @snippet)
     = link_to snippet_path(@snippet), method: :delete, data: { confirm: _("Are you sure?") }, class: "btn btn-grouped btn-inverted btn-remove", title: _('Delete Snippet') do
       = _("Delete")
-  = link_to new_snippet_path, class: "btn btn-grouped btn-success btn-inverted", title: _("New snippet") do
-    = _("New snippet")
+  - if can?(current_user, :create_personal_snippet)
+    = link_to new_snippet_path, class: "btn btn-grouped btn-success btn-inverted", title: _("New snippet") do
+      = _("New snippet")
   - if @snippet.submittable_as_spam_by?(current_user)
     = link_to _('Submit as spam'), mark_as_spam_snippet_path(@snippet), method: :post, class: 'btn btn-grouped btn-spam', title: _('Submit as spam')
 .d-block.d-sm-none.dropdown
@@ -17,9 +18,10 @@
     = icon('caret-down')
   .dropdown-menu.dropdown-menu-full-width
     %ul
-      %li
-        = link_to new_snippet_path, title: _("New snippet") do
-          = _("New snippet")
+      - if can?(current_user, :create_personal_snippet)
+        %li
+          = link_to new_snippet_path, title: _("New snippet") do
+            = _("New snippet")
       - if can?(current_user, :admin_personal_snippet, @snippet)
         %li
           = link_to snippet_path(@snippet), method: :delete, data: { confirm: _("Are you sure?") }, title: _('Delete Snippet') do

changelogs/unreleased/fj-37436-fix-create-personal-snippet-ability.yml

+---
+title: Ensure to check create_personal_snippet ability
+merge_request: 20838
+author:
+type: fixed

spec/controllers/snippets_controller_spec.rb

@@ -53,6 +53,16 @@ describe SnippetsController do
 
         expect(response).to have_gitlab_http_status(200)
       end
+
+      context 'when user is not allowed to create a personal snippet' do
+        let(:user) { create(:user, :external) }
+
+        it 'responds with status 404' do
+          get :new
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
     end
 
     context 'when not signed in' do
@@ -215,6 +225,20 @@ describe SnippetsController do
       expect(snippet.description).to eq('Description')
     end
 
+    context 'when user is not allowed to create a personal snippet' do
+      let(:user) { create(:user, :external) }
+
+      it 'responds with status 404' do
+        aggregate_failures do
+          expect do
+            create_snippet(visibility_level: Snippet::PUBLIC)
+          end.not_to change { Snippet.count }
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+    end
+
     context 'when the snippet description contains a file' do
       include FileMoverHelpers
 

spec/features/snippets/show_spec.rb

@@ -158,4 +158,21 @@ describe 'Snippet', :js do
 
     subject { visit snippet_path(snippet) }
   end
+
+  context 'when user cannot create snippets' do
+    let(:user) { create(:user, :external) }
+    let(:snippet) { create(:personal_snippet, :public) }
+
+    before do
+      sign_in(user)
+
+      visit snippet_path(snippet)
+
+      wait_for_requests
+    end
+
+    it 'does not show the "New Snippet" button' do
+      expect(page).not_to have_link('New snippet')
+    end
+  end
 end

spec/policies/global_policy_spec.rb

@@ -306,4 +306,22 @@ describe GlobalPolicy do
       it { is_expected.not_to be_allowed(:use_slash_commands) }
     end
   end
+
+  describe 'create_personal_snippet' do
+    context 'when anonymous' do
+      let(:current_user) { nil }
+
+      it { is_expected.not_to be_allowed(:create_personal_snippet) }
+    end
+
+    context 'regular user' do
+      it { is_expected.to be_allowed(:create_personal_snippet) }
+    end
+
+    context 'when external' do
+      let(:current_user) { build(:user, :external) }
+
+      it { is_expected.not_to be_allowed(:create_personal_snippet) }
+    end
+  end
 end

spec/views/layouts/header/_new_dropdown.haml_spec.rb

@@ -126,6 +126,16 @@ describe 'layouts/header/_new_dropdown' do
 
       expect(rendered).to have_link('New snippet', href: new_snippet_path)
     end
+
+    context 'when the user is not allowed to create snippets' do
+      let(:user) { create(:user, :external)}
+
+      it 'has no "New snippet" link' do
+        render
+
+        expect(rendered).not_to have_link('New snippet', href: new_snippet_path)
+      end
+    end
   end
 
   def stub_current_user(current_user)