Remove the 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!
This commit is contained in:
parent
acb55198b4
commit
c1892f6c90
5 changed files with 45 additions and 38 deletions
|
@ -56,8 +56,9 @@ class UploadsController < ApplicationController
|
||||||
def authorize_create_access!
|
def authorize_create_access!
|
||||||
return unless model
|
return unless model
|
||||||
|
|
||||||
# for now we support only personal snippets comments
|
# for now we support only personal snippets comments. Only personal_snippet
|
||||||
authorized = can?(current_user, :comment_personal_snippet, model)
|
# is allowed as a model to #create through routing.
|
||||||
|
authorized = can?(current_user, :create_note, model)
|
||||||
|
|
||||||
render_unauthorized unless authorized
|
render_unauthorized unless authorized
|
||||||
end
|
end
|
||||||
|
|
|
@ -128,15 +128,9 @@ module NotesHelper
|
||||||
end
|
end
|
||||||
|
|
||||||
def can_create_note?
|
def can_create_note?
|
||||||
issuable = @issue || @merge_request
|
noteable = @issue || @merge_request || @snippet || @project
|
||||||
|
|
||||||
if @snippet.is_a?(PersonalSnippet)
|
can?(current_user, :create_note, noteable)
|
||||||
can?(current_user, :comment_personal_snippet, @snippet)
|
|
||||||
elsif issuable
|
|
||||||
can?(current_user, :create_note, issuable)
|
|
||||||
else
|
|
||||||
can?(current_user, :create_note, @project)
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def initial_notes_data(autocomplete)
|
def initial_notes_data(autocomplete)
|
||||||
|
|
|
@ -7,7 +7,7 @@ class PersonalSnippetPolicy < BasePolicy
|
||||||
|
|
||||||
rule { public_snippet }.policy do
|
rule { public_snippet }.policy do
|
||||||
enable :read_personal_snippet
|
enable :read_personal_snippet
|
||||||
enable :comment_personal_snippet
|
enable :create_note
|
||||||
end
|
end
|
||||||
|
|
||||||
rule { is_author }.policy do
|
rule { is_author }.policy do
|
||||||
|
@ -15,7 +15,7 @@ class PersonalSnippetPolicy < BasePolicy
|
||||||
enable :update_personal_snippet
|
enable :update_personal_snippet
|
||||||
enable :destroy_personal_snippet
|
enable :destroy_personal_snippet
|
||||||
enable :admin_personal_snippet
|
enable :admin_personal_snippet
|
||||||
enable :comment_personal_snippet
|
enable :create_note
|
||||||
end
|
end
|
||||||
|
|
||||||
rule { ~anonymous }.enable :create_personal_snippet
|
rule { ~anonymous }.enable :create_personal_snippet
|
||||||
|
@ -23,15 +23,12 @@ class PersonalSnippetPolicy < BasePolicy
|
||||||
|
|
||||||
rule { internal_snippet & ~external_user }.policy do
|
rule { internal_snippet & ~external_user }.policy do
|
||||||
enable :read_personal_snippet
|
enable :read_personal_snippet
|
||||||
enable :comment_personal_snippet
|
|
||||||
end
|
|
||||||
|
|
||||||
rule { anonymous }.prevent :comment_personal_snippet
|
|
||||||
|
|
||||||
rule { can?(:comment_personal_snippet) }.policy do
|
|
||||||
enable :create_note
|
enable :create_note
|
||||||
enable :award_emoji
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
rule { anonymous }.prevent :create_note
|
||||||
|
|
||||||
|
rule { can?(:create_note) }.enable :award_emoji
|
||||||
|
|
||||||
rule { full_private_access }.enable :read_personal_snippet
|
rule { full_private_access }.enable :read_personal_snippet
|
||||||
end
|
end
|
||||||
|
|
|
@ -14,13 +14,6 @@ describe PersonalSnippetPolicy do
|
||||||
]
|
]
|
||||||
end
|
end
|
||||||
|
|
||||||
let(:comment_permissions) do
|
|
||||||
[
|
|
||||||
:comment_personal_snippet,
|
|
||||||
:create_note
|
|
||||||
]
|
|
||||||
end
|
|
||||||
|
|
||||||
def permissions(user)
|
def permissions(user)
|
||||||
described_class.new(user, snippet)
|
described_class.new(user, snippet)
|
||||||
end
|
end
|
||||||
|
@ -33,7 +26,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_allowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -44,7 +37,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_allowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -55,7 +48,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_allowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_allowed(*author_permissions)
|
is_expected.to be_allowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -70,7 +63,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_disallowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -81,7 +74,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_allowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -92,7 +85,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_disallowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -103,7 +96,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_allowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_allowed(*author_permissions)
|
is_expected.to be_allowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -118,7 +111,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_disallowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -129,7 +122,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_disallowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -140,7 +133,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_allowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -151,7 +144,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_disallowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_disallowed(*author_permissions)
|
is_expected.to be_disallowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
@ -162,7 +155,7 @@ describe PersonalSnippetPolicy do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
is_expected.to be_allowed(:read_personal_snippet)
|
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(:award_emoji)
|
||||||
is_expected.to be_allowed(*author_permissions)
|
is_expected.to be_allowed(*author_permissions)
|
||||||
end
|
end
|
||||||
|
|
22
spec/routing/uploads_routing_spec.rb
Normal file
22
spec/routing/uploads_routing_spec.rb
Normal file
|
@ -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
|
Loading…
Reference in a new issue