diff options
author | Douwe Maan <douwe@gitlab.com> | 2019-05-02 16:31:05 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2019-05-02 16:31:05 +0000 |
commit | d753336eb56bcd8485fd2774d54ddcba9161f5e3 (patch) | |
tree | b0673968de36261c3dd69746ad3756ec8e0b3ea2 | |
parent | 6d7baffd5305333be0548f0d7ea4b87c8e99dbc0 (diff) | |
parent | c1892f6c9000cacafae4f6c8992ba6c1128c8c95 (diff) | |
download | gitlab-ce-d753336eb56bcd8485fd2774d54ddcba9161f5e3.tar.gz |
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
-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 |