diff options
author | Sean McGivern <sean@gitlab.com> | 2019-05-02 11:13:42 +0100 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-05-02 11:13:42 +0100 |
commit | c1892f6c9000cacafae4f6c8992ba6c1128c8c95 (patch) | |
tree | 237c2955a1514b9be6b8b09c4bdb16323573641a | |
parent | acb55198b4a05a0b4ac2662bf68cfeb3d744ca01 (diff) | |
download | gitlab-ce-c1892f6c9000cacafae4f6c8992ba6c1128c8c95.tar.gz |
Remove the `comment_personal_snippet` permissionremove-comment-personal-snippet-permission
This is now entirely handled by `create_note`:
1. Project snippets prevent `create_note`.
2. Uploads already only support routing for personal snippets.
This simplifies some policies and access checks, too!
-rw-r--r-- | app/controllers/uploads_controller.rb | 5 | ||||
-rw-r--r-- | app/helpers/notes_helper.rb | 10 | ||||
-rw-r--r-- | app/policies/personal_snippet_policy.rb | 13 | ||||
-rw-r--r-- | spec/policies/personal_snippet_policy_spec.rb | 31 | ||||
-rw-r--r-- | spec/routing/uploads_routing_spec.rb | 22 |
5 files changed, 44 insertions, 37 deletions
diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 568c6e2a852..060b09f015c 100644 --- a/app/controllers/uploads_controller.rb +++ b/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 diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index a50137bea3d..2e31a5e2ed4 100644 --- a/app/helpers/notes_helper.rb +++ b/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) diff --git a/app/policies/personal_snippet_policy.rb b/app/policies/personal_snippet_policy.rb index 2b5cca76c20..40dd49b4afd 100644 --- a/app/policies/personal_snippet_policy.rb +++ b/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 diff --git a/spec/policies/personal_snippet_policy_spec.rb b/spec/policies/personal_snippet_policy_spec.rb index a38e0dbd797..097000ceb6a 100644 --- a/spec/policies/personal_snippet_policy_spec.rb +++ b/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 diff --git a/spec/routing/uploads_routing_spec.rb b/spec/routing/uploads_routing_spec.rb new file mode 100644 index 00000000000..6a041ffdd6c --- /dev/null +++ b/spec/routing/uploads_routing_spec.rb @@ -0,0 +1,22 @@ +# 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 |