Automatic merge of gitlab-org/gitlab-ce master

lib/gitlab/graphql/authorize/authorize_field_service.rb

@@ -14,9 +14,10 @@ module Gitlab
         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 @@ module Gitlab
           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 @@ module Gitlab
         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

spec/graphql/features/authorization_spec.rb

@@ -75,6 +75,59 @@ describe 'Gitlab::Graphql::Authorization' do
     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|

spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb

@@ -9,55 +9,88 @@ describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
     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