Merge branch 'remove-comment-personal-snippet-permission' into 'master'

Remove the `comment_personal_snippet` permission

Closes #56688

See merge request gitlab-org/gitlab-ce!27999

app/controllers/uploads_controller.rb

@@ -56,8 +56,9 @@ class UploadsController < ApplicationController
   def authorize_create_access!
     return unless model
 
-    # for now we support only personal snippets comments
-    authorized = can?(current_user, :comment_personal_snippet, model)
+    # for now we support only personal snippets comments. Only personal_snippet
+    # is allowed as a model to #create through routing.
+    authorized = can?(current_user, :create_note, model)
 
     render_unauthorized unless authorized
   end

app/helpers/notes_helper.rb

@@ -128,15 +128,9 @@ module NotesHelper
   end
 
   def can_create_note?
-    issuable = @issue || @merge_request
+    noteable = @issue || @merge_request || @snippet || @project
 
-    if @snippet.is_a?(PersonalSnippet)
-      can?(current_user, :comment_personal_snippet, @snippet)
-    elsif issuable
-      can?(current_user, :create_note, issuable)
-    else
-      can?(current_user, :create_note, @project)
-    end
+    can?(current_user, :create_note, noteable)
   end
 
   def initial_notes_data(autocomplete)

app/policies/personal_snippet_policy.rb

@@ -7,7 +7,7 @@ class PersonalSnippetPolicy < BasePolicy
 
   rule { public_snippet }.policy do
     enable :read_personal_snippet
-    enable :comment_personal_snippet
+    enable :create_note
   end
 
   rule { is_author }.policy do
@@ -15,7 +15,7 @@ class PersonalSnippetPolicy < BasePolicy
     enable :update_personal_snippet
     enable :destroy_personal_snippet
     enable :admin_personal_snippet
-    enable :comment_personal_snippet
+    enable :create_note
   end
 
   rule { ~anonymous }.enable :create_personal_snippet
@@ -23,15 +23,12 @@ class PersonalSnippetPolicy < BasePolicy
 
   rule { internal_snippet & ~external_user }.policy do
     enable :read_personal_snippet
-    enable :comment_personal_snippet
+    enable :create_note
   end
 
-  rule { anonymous }.prevent :comment_personal_snippet
+  rule { anonymous }.prevent :create_note
 
-  rule { can?(:comment_personal_snippet) }.policy do
-    enable :create_note
-    enable :award_emoji
-  end
+  rule { can?(:create_note) }.enable :award_emoji
 
   rule { full_private_access }.enable :read_personal_snippet
 end

spec/policies/personal_snippet_policy_spec.rb

@@ -14,13 +14,6 @@ describe PersonalSnippetPolicy do
     ]
   end
 
-  let(:comment_permissions) do
-    [
-      :comment_personal_snippet,
-      :create_note
-    ]
-  end
-
   def permissions(user)
     described_class.new(user, snippet)
   end
@@ -33,7 +26,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_allowed(:read_personal_snippet)
-        is_expected.to be_disallowed(*comment_permissions)
+        is_expected.to be_disallowed(:create_note)
         is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -44,7 +37,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_allowed(:read_personal_snippet)
-        is_expected.to be_allowed(*comment_permissions)
+        is_expected.to be_allowed(:create_note)
         is_expected.to be_allowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -55,7 +48,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_allowed(:read_personal_snippet)
-        is_expected.to be_allowed(*comment_permissions)
+        is_expected.to be_allowed(:create_note)
         is_expected.to be_allowed(:award_emoji)
         is_expected.to be_allowed(*author_permissions)
       end
@@ -70,7 +63,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
-        is_expected.to be_disallowed(*comment_permissions)
+        is_expected.to be_disallowed(:create_note)
         is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -81,7 +74,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_allowed(:read_personal_snippet)
-        is_expected.to be_allowed(*comment_permissions)
+        is_expected.to be_allowed(:create_note)
         is_expected.to be_allowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -92,7 +85,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
-        is_expected.to be_disallowed(*comment_permissions)
+        is_expected.to be_disallowed(:create_note)
         is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -103,7 +96,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_allowed(:read_personal_snippet)
-        is_expected.to be_allowed(*comment_permissions)
+        is_expected.to be_allowed(:create_note)
         is_expected.to be_allowed(:award_emoji)
         is_expected.to be_allowed(*author_permissions)
       end
@@ -118,7 +111,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
-        is_expected.to be_disallowed(*comment_permissions)
+        is_expected.to be_disallowed(:create_note)
         is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -129,7 +122,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
-        is_expected.to be_disallowed(*comment_permissions)
+        is_expected.to be_disallowed(:create_note)
         is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -140,7 +133,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_allowed(:read_personal_snippet)
-        is_expected.to be_disallowed(:comment_personal_snippet)
+        is_expected.to be_disallowed(:create_note)
         is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -151,7 +144,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
-        is_expected.to be_disallowed(*comment_permissions)
+        is_expected.to be_disallowed(:create_note)
         is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
@@ -162,7 +155,7 @@ describe PersonalSnippetPolicy do
 
       it do
         is_expected.to be_allowed(:read_personal_snippet)
-        is_expected.to be_allowed(*comment_permissions)
+        is_expected.to be_allowed(:create_note)
         is_expected.to be_allowed(:award_emoji)
         is_expected.to be_allowed(*author_permissions)
       end

spec/routing/uploads_routing_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Uploads', 'routing' do
+  it 'allows creating uploads for personal snippets' do
+    expect(post('/uploads/personal_snippet?id=1')).to route_to(
+      controller: 'uploads',
+      action: 'create',
+      model: 'personal_snippet',
+      id: '1'
+    )
+  end
+
+  it 'does not allow creating uploads for other models' do
+    UploadsController::MODEL_CLASSES.keys.compact.each do |model|
+      next if model == 'personal_snippet'
+
+      expect(post("/uploads/#{model}?id=1")).not_to be_routable
+    end
+  end
+end