GraphQL Type authorization

Enables authorizations to be defined on GraphQL Types.

    module Types
      class ProjectType < BaseObject
        authorize :read_project
      end
    end

If a field has authorizations defined on it, and the return type of the
field also has authorizations defined on it. then all of the combined
permissions in the authorizations will be checked and must pass.

Connection fields are checked by "digging" to find the type class of the
"node" field in the expected location of edges->node.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/54417

app/graphql/types/ci/pipeline_type.rb

@@ -3,10 +3,12 @@
 module Types
   module Ci
     class PipelineType < BaseObject
-      expose_permissions Types::PermissionTypes::Ci::Pipeline
-
       graphql_name 'Pipeline'
 
+      authorize :read_pipeline
+
+      expose_permissions Types::PermissionTypes::Ci::Pipeline
+
       field :id, GraphQL::ID_TYPE, null: false
       field :iid, GraphQL::ID_TYPE, null: false
 

app/graphql/types/issue_type.rb

@@ -2,10 +2,12 @@
 
 module Types
   class IssueType < BaseObject
-    expose_permissions Types::PermissionTypes::Issue
-
     graphql_name 'Issue'
 
+    authorize :read_issue
+
+    expose_permissions Types::PermissionTypes::Issue
+
     present_using IssuePresenter
 
     field :iid, GraphQL::ID_TYPE, null: false
@@ -15,16 +17,14 @@ module Types
 
     field :author, Types::UserType,
           null: false,
-          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, obj.author_id).find },
-          authorize: :read_user
+          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, obj.author_id).find }
 
     field :assignees, Types::UserType.connection_type, null: true
 
     field :labels, Types::LabelType.connection_type, null: true
     field :milestone, Types::MilestoneType,
           null: true,
-          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Milestone, obj.milestone_id).find },
-          authorize: :read_milestone
+          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Milestone, obj.milestone_id).find }
 
     field :due_date, Types::TimeType, null: true
     field :confidential, GraphQL::BOOLEAN_TYPE, null: false

app/graphql/types/merge_request_type.rb

@@ -2,12 +2,14 @@
 
 module Types
   class MergeRequestType < BaseObject
+    graphql_name 'MergeRequest'
+
+    authorize :read_merge_request
+
     expose_permissions Types::PermissionTypes::MergeRequest
 
     present_using MergeRequestPresenter
 
-    graphql_name 'MergeRequest'
-
     field :id, GraphQL::ID_TYPE, null: false
     field :iid, GraphQL::ID_TYPE, null: false
     field :title, GraphQL::STRING_TYPE, null: false
@@ -48,7 +50,7 @@ module Types
     field :downvotes, GraphQL::INT_TYPE, null: false
     field :subscribed, GraphQL::BOOLEAN_TYPE, method: :subscribed?, null: false
 
-    field :head_pipeline, Types::Ci::PipelineType, null: true, method: :actual_head_pipeline, authorize: :read_pipeline
+    field :head_pipeline, Types::Ci::PipelineType, null: true, method: :actual_head_pipeline
     field :pipelines, Types::Ci::PipelineType.connection_type,
           resolver: Resolvers::MergeRequestPipelinesResolver
   end

app/graphql/types/milestone_type.rb

@@ -4,6 +4,8 @@ module Types
   class MilestoneType < BaseObject
     graphql_name 'Milestone'
 
+    authorize :read_milestone
+
     field :description, GraphQL::STRING_TYPE, null: true
     field :title, GraphQL::STRING_TYPE, null: false
     field :state, GraphQL::STRING_TYPE, null: false

app/graphql/types/project_type.rb

@@ -2,10 +2,12 @@
 
 module Types
   class ProjectType < BaseObject
-    expose_permissions Types::PermissionTypes::Project
-
     graphql_name 'Project'
 
+    authorize :read_project
+
+    expose_permissions Types::PermissionTypes::Project
+
     field :id, GraphQL::ID_TYPE, null: false
 
     field :full_path, GraphQL::ID_TYPE, null: false
@@ -67,14 +69,12 @@ module Types
     field :merge_requests,
           Types::MergeRequestType.connection_type,
           null: true,
-          resolver: Resolvers::MergeRequestsResolver,
-          authorize: :read_merge_request
+          resolver: Resolvers::MergeRequestsResolver
 
     field :merge_request,
           Types::MergeRequestType,
           null: true,
-          resolver: Resolvers::MergeRequestsResolver.single,
-          authorize: :read_merge_request
+          resolver: Resolvers::MergeRequestsResolver.single
 
     field :issues,
           Types::IssueType.connection_type,
@@ -88,7 +88,7 @@ module Types
 
     field :pipelines,
           Types::Ci::PipelineType.connection_type,
-          null: false,
+          null: true,
           resolver: Resolvers::ProjectPipelinesResolver
   end
 end

app/graphql/types/query_type.rb

@@ -7,8 +7,7 @@ module Types
     field :project, Types::ProjectType,
           null: true,
           resolver: Resolvers::ProjectResolver,
-          description: "Find a project",
-          authorize: :read_project
+          description: "Find a project"
 
     field :metadata, Types::MetadataType,
           null: true,

app/graphql/types/user_type.rb

@@ -4,6 +4,8 @@ module Types
   class UserType < BaseObject
     graphql_name 'User'
 
+    authorize :read_user
+
     present_using UserPresenter
 
     field :name, GraphQL::STRING_TYPE, null: false

changelogs/unreleased/54417-graphql-type-authorization.yml

+---
+title: GraphQL Types can be made to always authorize access to resources of that Type
+merge_request: 25724
+author:
+type: added

config/initializers/graphql.rb

 # frozen_string_literal: true
 
+GraphQL::ObjectType.accepts_definitions(authorize: GraphQL::Define.assign_metadata_key(:authorize))
 GraphQL::Field.accepts_definitions(authorize: GraphQL::Define.assign_metadata_key(:authorize))
+
+GraphQL::Schema::Object.accepts_definition(:authorize)
 GraphQL::Schema::Field.accepts_definition(:authorize)

lib/gitlab/graphql/authorize/authorize_field_service.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module Authorize
+      class AuthorizeFieldService
+        def initialize(field)
+          @field = field
+          @old_resolve_proc = @field.resolve_proc
+        end
+
+        def authorizations?
+          authorizations.present?
+        end
+
+        def authorized_resolve
+          proc do |obj, args, ctx|
+            resolved_obj = @old_resolve_proc.call(obj, args, ctx)
+            checker = build_checker(ctx[:current_user])
+
+            if resolved_obj.respond_to?(:then)
+              resolved_obj.then(&checker)
+            else
+              checker.call(resolved_obj)
+            end
+          end
+        end
+
+        private
+
+        def authorizations
+          @authorizations ||= (type_authorizations + field_authorizations).uniq
+        end
+
+        # Returns any authorize metadata from the return type of @field
+        def type_authorizations
+          type = @field.type
+
+          # When the return type of @field is a collection, find the singular type
+          if type.get_field('edges')
+            type = node_type_for_relay_connection(type)
+          elsif type.list?
+            type = node_type_for_basic_connection(type)
+          end
+
+          Array.wrap(type.metadata[:authorize])
+        end
+
+        # Returns any authorize metadata from @field
+        def field_authorizations
+          Array.wrap(@field.metadata[:authorize])
+        end
+
+        def build_checker(current_user)
+          lambda do |value|
+            # Load the elements if they were not loaded by BatchLoader yet
+            value = value.sync if value.respond_to?(:sync)
+
+            check = lambda do |object|
+              authorizations.all? do |ability|
+                Ability.allowed?(current_user, ability, object)
+              end
+            end
+
+            case value
+            when Array, ActiveRecord::Relation
+              value.select(&check)
+            else
+              value if check.call(value)
+            end
+          end
+        end
+
+        # Returns the singular type for relay connections.
+        # This will be the type class of edges.node
+        def node_type_for_relay_connection(type)
+          type = type.get_field('edges').type.unwrap.get_field('node')&.type
+
+          if type.nil?
+            raise Gitlab::Graphql::Errors::ConnectionDefinitionError,
+              'Connection Type must conform to the Relay Cursor Connections Specification'
+          end
+
+          type
+        end
+
+        # Returns the singular type for basic connections, for example `[Types::ProjectType]`
+        def node_type_for_basic_connection(type)
+          type.unwrap
+        end
+      end
+    end
+  end
+end

lib/gitlab/graphql/authorize/instrumentation.rb

@@ -7,46 +7,12 @@ module Gitlab
         # Replace the resolver for the field with one that will only return the
         # resolved object if the permissions check is successful.
         def instrument(_type, field)
-          required_permissions = Array.wrap(field.metadata[:authorize])
-          return field if required_permissions.empty?
+          service = AuthorizeFieldService.new(field)
 
-          old_resolver = field.resolve_proc
-
-          new_resolver = -> (obj, args, ctx) do
-            resolved_obj = old_resolver.call(obj, args, ctx)
-            checker = build_checker(ctx[:current_user], required_permissions)
-
-            if resolved_obj.respond_to?(:then)
-              resolved_obj.then(&checker)
-            else
-              checker.call(resolved_obj)
-            end
-          end
-
-          field.redefine do
-            resolve(new_resolver)
-          end
-        end
-
-        private
-
-        def build_checker(current_user, abilities)
-          lambda do |value|
-            # Load the elements if they weren't loaded by BatchLoader yet
-            value = value.sync if value.respond_to?(:sync)
-
-            check = lambda do |object|
-              abilities.all? do |ability|
-                Ability.allowed?(current_user, ability, object)
-              end
-            end
-
-            case value
-            when Array
-              value.select(&check)
-            else
-              value if check.call(value)
-            end
+          if service.authorizations?
+            field.redefine { resolve(service.authorized_resolve) }
+          else
+            field
           end
         end
       end

lib/gitlab/graphql/errors.rb

@@ -6,6 +6,7 @@ module Gitlab
       BaseError = Class.new(GraphQL::ExecutionError)
       ArgumentError = Class.new(BaseError)
       ResourceNotAvailable = Class.new(BaseError)
+      ConnectionDefinitionError = Class.new(BaseError)
     end
   end
 end

spec/graphql/features/authorization_spec.rb

@@ -5,61 +5,192 @@ require 'spec_helper'
 describe 'Gitlab::Graphql::Authorization' do
   set(:user) { create(:user) }
 
+  let(:permission_single) { :foo }
+  let(:permission_collection) { [:foo, :bar] }
   let(:test_object) { double(name: 'My name') }
-  let(:object_type) { object_type_class }
-  let(:query_type) { query_type_class(object_type, test_object) }
-  let(:schema) { schema_class(query_type) }
+  let(:query_string) { '{ object() { name } }' }
+  let(:result) { execute_query(query_type)['data'] }
 
-  let(:execute) do
-    schema.execute(
-      query_string,
-      context: { current_user: user },
-      variables: {}
-    )
+  subject { result['object'] }
+
+  shared_examples 'authorization with a single permission' do
+    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 be_nil
+    end
   end
 
-  let(:result) { execute['data'] }
+  shared_examples 'authorization with a collection of permissions' do
+    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 be_nil
+    end
+
+    it 'returns nil when user only has none of the permissions' do
+      expect(subject).to be_nil
+    end
+  end
 
   before do
     # By default, disallow all permissions.
     allow(Ability).to receive(:allowed?).and_return(false)
   end
 
-  describe 'authorizing with a single permission' do
-    let(:query_string) { '{ singlePermission() { name } }' }
+  describe 'Field authorizations' do
+    let(:type) { type_factory }
 
-    subject { result['singlePermission'] }
+    describe 'with a single permission' do
+      let(:query_type) do
+        query_factory do |query|
+          query.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object }, authorize: permission_single
+        end
+      end
+
+      include_examples 'authorization with a single permission'
+    end
+
+    describe 'with a collection of permissions' do
+      let(:query_type) do
+        permissions = permission_collection
+        query_factory do |qt|
+          qt.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object } do
+            authorize permissions
+          end
+        end
+      end
 
-    it 'should return the protected field when user has permission' do
-      permit(:foo)
+      include_examples 'authorization with a collection of permissions'
+    end
+  end
 
-      expect(subject['name']).to eq(test_object.name)
+  describe 'Type authorizations' do
+    let(:query_type) do
+      query_factory do |query|
+        query.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object }
+      end
     end
 
-    it 'should return nil when user is not authorized' do
-      expect(subject).to be_nil
+    describe 'with a single permission' do
+      let(:type) do
+        type_factory do |type|
+          type.authorize permission_single
+        end
+      end
+
+      include_examples 'authorization with a single permission'
+    end
+
+    describe 'with a collection of permissions' do
+      let(:type) do
+        type_factory do |type|
+          type.authorize permission_collection
+        end
+      end
+
+      include_examples 'authorization with a collection of permissions'
     end
   end
 
-  describe 'authorizing with an Array of permissions' do
-    let(:query_string) { '{ permissionCollection() { name } }' }
+  describe 'type and field authorizations together' do
+    let(:permission_1) { permission_collection.first }
+    let(:permission_2) { permission_collection.last }
 
-    subject { result['permissionCollection'] }
+    let(:type) do
+      type_factory do |type|
+        type.authorize permission_1
+      end
+    end
 
-    it 'should return the protected field when user has all permissions' do
-      permit(:foo, :bar)
+    let(:query_type) do
+      query_factory do |query|
+        query.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object }, authorize: permission_2
+      end
+    end
 
-      expect(subject['name']).to eq(test_object.name)
+    include_examples 'authorization with a collection of permissions'
+  end
+
+  describe 'type authorizations when applied to a relay connection' do
+    let(:query_string) { '{ object() { edges { node { name } } } }' }
+
+    let(:type) do
+      type_factory do |type|
+        type.authorize permission_single
+      end
+    end
+
+    let(:query_type) do
+      query_factory do |query|
+        query.field :object, type.connection_type, null: true, resolve: ->(obj, args, ctx) { [test_object] }
+      end
     end
 
-    it 'should return nil when user only has one of the permissions' do
-      permit(:foo)
+    subject { result.dig('object', 'edges') }
 
-      expect(subject).to be_nil
+    it 'returns the protected field when user has permission' do
+      permit(permission_single)
+
+      expect(subject).not_to be_empty
+      expect(subject.first['node']).to eq('name' => test_object.name)
     end
 
-    it 'should return nil when user only has none of the permissions' do
-      expect(subject).to be_nil
+    it 'returns nil when user is not authorized' do
+      expect(subject).to be_empty
+    end
+  end
+
+  describe 'type authorizations when applied to a basic connection' do
+    let(:type) do
+      type_factory do |type|
+        type.authorize permission_single
+      end
+    end
+
+    let(:query_type) do
+      query_factory do |query|
+        query.field :object, [type], null: true, resolve: ->(obj, args, ctx) { [test_object] }
+      end
+    end
+
+    subject { result['object'].first }
+
+    include_examples 'authorization with a single permission'
+  end
+
+  describe 'when connections do not follow the correct specification' do
+    let(:query_string) { '{ object() { edges { node { name }} } }' }
+
+    let(:type) do
+      bad_node = type_factory do |type|
+        type.graphql_name 'BadNode'
+        type.field :bad_node, GraphQL::STRING_TYPE, null: true
+      end
+
+      type_factory do |type|
+        type.field :edges, [bad_node], null: true
+      end
+    end
+
+    let(:query_type) do
+      query_factory do |query|
+        query.field :object, type, null: true
+      end
+    end
+
+    it 'throws an error' do
+      expect { result }.to raise_error(Gitlab::Graphql::Errors::ConnectionDefinitionError)
     end
   end
 
@@ -71,36 +202,34 @@ describe 'Gitlab::Graphql::Authorization' do
     end
   end
 
-  def object_type_class
+  def type_factory
     Class.new(Types::BaseObject) do
-      graphql_name 'TestObject'
+      graphql_name 'TestType'
 
       field :name, GraphQL::STRING_TYPE, null: true
+
+      yield(self) if block_given?
     end
   end
 
-  def query_type_class(type, object)
+  def query_factory
     Class.new(Types::BaseObject) do
       graphql_name 'TestQuery'
 
-      field :single_permission, type,
-        null: true,
-        authorize: :foo,
-        resolve: ->(obj, args, ctx) { object }
-
-      field :permission_collection, type,
-        null: true,
-        resolve: ->(obj, args, ctx) { object } do
-        authorize [:foo, :bar]
-      end
+      yield(self) if block_given?
     end
   end
 
-  def schema_class(query)
-    Class.new(GraphQL::Schema) do
+  def execute_query(query_type)
+    schema = Class.new(GraphQL::Schema) do
       use Gitlab::Graphql::Authorize
-
-      query(query)
+      query(query_type)
     end
+
+    schema.execute(
+      query_string,
+      context: { current_user: user },
+      variables: {}
+    )
   end
 end

spec/graphql/types/issue_type_spec.rb

@@ -4,4 +4,6 @@ describe GitlabSchema.types['Issue'] do
   it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::Issue) }
 
   it { expect(described_class.graphql_name).to eq('Issue') }
+
+  it { expect(described_class).to require_graphql_authorizations(:read_issue) }
 end

spec/graphql/types/merge_request_type_spec.rb

@@ -3,14 +3,9 @@ require 'spec_helper'
 describe GitlabSchema.types['MergeRequest'] do
   it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::MergeRequest) }
 
-  describe 'head pipeline' do
-    it 'has a head pipeline field' do
-      expect(described_class).to have_graphql_field(:head_pipeline)
-    end
+  it { expect(described_class).to require_graphql_authorizations(:read_merge_request) }
 
-    it 'authorizes the field' do
-      expect(described_class.fields['headPipeline'])
-        .to require_graphql_authorizations(:read_pipeline)
-    end
+  describe 'nested head pipeline' do
+    it { expect(described_class).to have_graphql_field(:head_pipeline) }
   end
 end

spec/graphql/types/milestone_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GitlabSchema.types['Milestone'] do
+  it { expect(described_class.graphql_name).to eq('Milestone') }
+
+  it { expect(described_class).to require_graphql_authorizations(:read_milestone) }
+end

spec/graphql/types/project_type_spec.rb

@@ -5,19 +5,11 @@ describe GitlabSchema.types['Project'] do
 
   it { expect(described_class.graphql_name).to eq('Project') }
 
+  it { expect(described_class).to require_graphql_authorizations(:read_project) }
+
   describe 'nested merge request' do
     it { expect(described_class).to have_graphql_field(:merge_requests) }
     it { expect(described_class).to have_graphql_field(:merge_request) }
-
-    it 'authorizes the merge request' do
-      expect(described_class.fields['mergeRequest'])
-        .to require_graphql_authorizations(:read_merge_request)
-    end
-
-    it 'authorizes the merge requests' do
-      expect(described_class.fields['mergeRequests'])
-        .to require_graphql_authorizations(:read_merge_request)
-    end
   end
 
   describe 'nested issues' do

spec/graphql/types/query_type_spec.rb

@@ -15,10 +15,6 @@ describe GitlabSchema.types['Query'] do
       is_expected.to have_graphql_type(Types::ProjectType)
       is_expected.to have_graphql_resolver(Resolvers::ProjectResolver)
     end
-
-    it 'authorizes with read_project' do
-      is_expected.to require_graphql_authorizations(:read_project)
-    end
   end
 
   describe 'metadata field' do

spec/graphql/types/user_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GitlabSchema.types['User'] do
+  it { expect(described_class.graphql_name).to eq('User') }
+
+  it { expect(described_class).to require_graphql_authorizations(:read_user) }
+end

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

@@ -2,13 +2,17 @@
 
 require 'spec_helper'
 
-describe Gitlab::Graphql::Authorize::Instrumentation do
+# Also see spec/graphql/features/authorization_spec.rb for
+# integration tests of AuthorizeFieldService
+describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
   describe '#build_checker' do
     let(:current_user) { double(:current_user) }
     let(:abilities) { [double(:first_ability), double(:last_ability)] }
 
     let(:checker) do
-      described_class.new.__send__(:build_checker, current_user, abilities)
+      service = described_class.new(double(resolve_proc: proc {}))
+      allow(service).to receive(:authorizations).and_return(abilities)
+      service.__send__(:build_checker, current_user)
     end
 
     it 'returns a checker which checks for a single object' do
@@ -56,12 +60,14 @@ describe Gitlab::Graphql::Authorize::Instrumentation do
           .to contain_exactly(allowed)
       end
     end
+  end
 
-    def spy_ability_check_for(ability, object, passed: true)
-      expect(Ability)
-        .to receive(:allowed?)
-        .with(current_user, ability, object)
-        .and_return(passed)
-    end
+  private
+
+  def spy_ability_check_for(ability, object, passed: true)
+    expect(Ability)
+      .to receive(:allowed?)
+      .with(current_user, ability, object)
+      .and_return(passed)
   end
 end