From 6643b92b8807e2d59f36d676303b89ea01824f22 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Wed, 20 Mar 2019 18:39:18 -0500
Subject: [PATCH] Use parent object when authorizing scalar types
---
.../authorize/authorize_field_service.rb | 31 ++++--
spec/graphql/features/authorization_spec.rb | 53 +++++++++++
.../authorize/authorize_field_service_spec.rb | 95 +++++++++++++------
3 files changed, 138 insertions(+), 41 deletions(-)
diff --git a/lib/gitlab/graphql/authorize/authorize_field_service.rb b/lib/gitlab/graphql/authorize/authorize_field_service.rb
index f3ca82ec6975a..8deff79fc84c3 100644
--- a/lib/gitlab/graphql/authorize/authorize_field_service.rb
+++ b/lib/gitlab/graphql/authorize/authorize_field_service.rb
@@ -14,9 +14,10 @@ def authorizations?
end
def authorized_resolve
- proc do |obj, args, ctx|
- resolved_obj = @old_resolve_proc.call(obj, args, ctx)
- checker = build_checker(ctx[:current_user])
+ proc do |parent_typed_object, args, ctx|
+ resolved_obj = @old_resolve_proc.call(parent_typed_object, args, ctx)
+ authorizing_obj = authorize_against(parent_typed_object)
+ checker = build_checker(ctx[:current_user], authorizing_obj)
if resolved_obj.respond_to?(:then)
resolved_obj.then(&checker)
@@ -51,22 +52,28 @@ def field_authorizations
Array.wrap(@field.metadata[:authorize])
end
- def build_checker(current_user)
- lambda do |value|
+ # If it's a built-in/scalar type, authorize using its parent object.
+ # nil means authorize using the resolved object
+ def authorize_against(parent_typed_object)
+ parent_typed_object.object if built_in_type? && parent_typed_object.respond_to?(:object)
+ end
+
+ def build_checker(current_user, authorizing_obj)
+ lambda do |resolved_obj|
# Load the elements if they were not loaded by BatchLoader yet
- value = value.sync if value.respond_to?(:sync)
+ resolved_obj = resolved_obj.sync if resolved_obj.respond_to?(:sync)
check = lambda do |object|
authorizations.all? do |ability|
- Ability.allowed?(current_user, ability, object)
+ Ability.allowed?(current_user, ability, authorizing_obj || object)
end
end
- case value
+ case resolved_obj
when Array, ActiveRecord::Relation
- value.select(&check)
+ resolved_obj.select(&check)
else
- value if check.call(value)
+ resolved_obj if check.call(resolved_obj)
end
end
end
@@ -88,6 +95,10 @@ def node_type_for_relay_connection(type)
def node_type_for_basic_connection(type)
type.unwrap
end
+
+ def built_in_type?
+ GraphQL::Schema::BUILT_IN_TYPES.has_value?(node_type_for_basic_connection(@field.type))
+ end
end
end
end
diff --git a/spec/graphql/features/authorization_spec.rb b/spec/graphql/features/authorization_spec.rb
index f863c4444b8c0..00e31568a9ebc 100644
--- a/spec/graphql/features/authorization_spec.rb
+++ b/spec/graphql/features/authorization_spec.rb
@@ -75,6 +75,59 @@
end
end
+ describe 'Field authorizations when field is a built in type' do
+ let(:query_type) do
+ query_factory do |query|
+ query.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object }
+ end
+ end
+
+ describe 'with a single permission' do
+ let(:type) do
+ type_factory do |type|
+ type.field :name, GraphQL::STRING_TYPE, null: true, authorize: permission_single
+ end
+ end
+
+ it 'returns the protected field when user has permission' do
+ permit(permission_single)
+
+ expect(subject).to eq('name' => test_object.name)
+ end
+
+ it 'returns nil when user is not authorized' do
+ expect(subject).to eq('name' => nil)
+ end
+ end
+
+ describe 'with a collection of permissions' do
+ let(:type) do
+ permissions = permission_collection
+ type_factory do |type|
+ type.field :name, GraphQL::STRING_TYPE, null: true do
+ authorize permissions
+ end
+ end
+ end
+
+ it 'returns the protected field when user has all permissions' do
+ permit(*permission_collection)
+
+ expect(subject).to eq('name' => test_object.name)
+ end
+
+ it 'returns nil when user only has one of the permissions' do
+ permit(permission_collection.first)
+
+ expect(subject).to eq('name' => nil)
+ end
+
+ it 'returns nil when user only has none of the permissions' do
+ expect(subject).to eq('name' => nil)
+ end
+ end
+ end
+
describe 'Type authorizations' do
let(:query_type) do
query_factory do |query|
diff --git a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb
index ce320a2bdb023..6114aca0616cb 100644
--- a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb
+++ b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb
@@ -9,55 +9,88 @@
let(:current_user) { double(:current_user) }
let(:abilities) { [double(:first_ability), double(:last_ability)] }
- let(:checker) do
- service = described_class.new(double(resolve_proc: proc {}))
- allow(service).to receive(:authorizations).and_return(abilities)
- service.__send__(:build_checker, current_user)
- end
+ context 'when authorizing against the object' do
+ let(:checker) do
+ service = described_class.new(double(resolve_proc: proc {}))
+ allow(service).to receive(:authorizations).and_return(abilities)
+ service.__send__(:build_checker, current_user, nil)
+ end
- it 'returns a checker which checks for a single object' do
- object = double(:object)
+ it 'returns a checker which checks for a single object' do
+ object = double(:object)
- abilities.each do |ability|
- spy_ability_check_for(ability, object, passed: true)
- end
+ abilities.each do |ability|
+ spy_ability_check_for(ability, object, passed: true)
+ end
- expect(checker.call(object)).to eq(object)
- end
+ expect(checker.call(object)).to eq(object)
+ end
- it 'returns a checker which checks for all objects' do
- objects = [double(:first), double(:last)]
+ it 'returns a checker which checks for all objects' do
+ objects = [double(:first), double(:last)]
- abilities.each do |ability|
- objects.each do |object|
- spy_ability_check_for(ability, object, passed: true)
+ abilities.each do |ability|
+ objects.each do |object|
+ spy_ability_check_for(ability, object, passed: true)
+ end
end
+
+ expect(checker.call(objects)).to eq(objects)
end
- expect(checker.call(objects)).to eq(objects)
- end
+ context 'when some objects would not pass the check' do
+ it 'returns nil when it is single object' do
+ disallowed = double(:object)
+
+ spy_ability_check_for(abilities.first, disallowed, passed: false)
- context 'when some objects would not pass the check' do
- it 'returns nil when it is single object' do
- disallowed = double(:object)
+ expect(checker.call(disallowed)).to be_nil
+ end
+
+ it 'returns only objects which passed when there are more than one' do
+ allowed = double(:allowed)
+ disallowed = double(:disallowed)
- spy_ability_check_for(abilities.first, disallowed, passed: false)
+ spy_ability_check_for(abilities.first, disallowed, passed: false)
- expect(checker.call(disallowed)).to be_nil
+ abilities.each do |ability|
+ spy_ability_check_for(ability, allowed, passed: true)
+ end
+
+ expect(checker.call([disallowed, allowed])).to contain_exactly(allowed)
+ end
end
+ end
+
+ context 'when authorizing against another object' do
+ let(:authorizing_obj) { double(:object) }
- it 'returns only objects which passed when there are more than one' do
- allowed = double(:allowed)
- disallowed = double(:disallowed)
+ let(:checker) do
+ service = described_class.new(double(resolve_proc: proc {}))
+ allow(service).to receive(:authorizations).and_return(abilities)
+ service.__send__(:build_checker, current_user, authorizing_obj)
+ end
+
+ it 'returns a checker which checks for a single object' do
+ object = double(:object)
+
+ abilities.each do |ability|
+ spy_ability_check_for(ability, authorizing_obj, passed: true)
+ end
+
+ expect(checker.call(object)).to eq(object)
+ end
- spy_ability_check_for(abilities.first, disallowed, passed: false)
+ it 'returns a checker which checks for all objects' do
+ objects = [double(:first), double(:last)]
abilities.each do |ability|
- spy_ability_check_for(ability, allowed, passed: true)
+ objects.each do |object|
+ spy_ability_check_for(ability, authorizing_obj, passed: true)
+ end
end
- expect(checker.call([disallowed, allowed]))
- .to contain_exactly(allowed)
+ expect(checker.call(objects)).to eq(objects)
end
end
end
--
GitLab