Sanity check for GraphQL authorized?

Raise an exception if a developer calls any of the GraphQL authorization
methods and a `authorize :permission` is missing from a mutation class.

Previously `authorized?` would return `true` in this situation, which
although technically is accurate is not what a developer is intending.
This commit is contained in:
Luke Duncalfe 2019-06-21 17:09:02 +12:00
parent 87b468c254
commit b2a615c3c6
2 changed files with 51 additions and 0 deletions

View file

@ -48,6 +48,12 @@ module Gitlab
end end
def authorized?(object) def authorized?(object)
# Sanity check. We don't want to accidentally allow a developer to authorize
# without first adding permissions to authorize against
if self.class.required_permissions.empty?
raise Gitlab::Graphql::Errors::ArgumentError, "#{self.class.name} has no authorizations"
end
self.class.required_permissions.all? do |ability| self.class.required_permissions.all? do |ability|
# The actions could be performed across multiple objects. In which # The actions could be performed across multiple objects. In which
# case the current user is common, and we could benefit from the # case the current user is common, and we could benefit from the

View file

@ -101,6 +101,51 @@ describe Gitlab::Graphql::Authorize::AuthorizeResource do
end end
end end
context 'when the class does not define authorize' do
let(:fake_class) do
Class.new do
include Gitlab::Graphql::Authorize::AuthorizeResource
attr_reader :user, :found_object
def initialize(user, found_object)
@user, @found_object = user, found_object
end
def find_object(*_args)
found_object
end
def current_user
user
end
def self.name
'TestClass'
end
end
end
let(:error) { /#{fake_class.name} has no authorizations/ }
describe '#authorized_find' do
it 'raises a comprehensive error message' do
expect { loading_resource.authorized_find }.to raise_error(error)
end
end
describe '#authorized_find!' do
it 'raises a comprehensive error message' do
expect { loading_resource.authorized_find! }.to raise_error(error)
end
end
describe '#authorized?' do
it 'raises a comprehensive error message' do
expect { loading_resource.authorized?(project) }.to raise_error(error)
end
end
end
describe '#authorize' do describe '#authorize' do
it 'adds permissions from subclasses to those of superclasses when used on classes' do it 'adds permissions from subclasses to those of superclasses when used on classes' do
base_class = Class.new do base_class = Class.new do