From 8207f7877fea6987cbd8ef26e6f01feca6608bd2 Mon Sep 17 00:00:00 2001
From: Luke Duncalfe <lduncalfe@eml.cc>
Date: Mon, 4 Mar 2019 15:30:32 +1300
Subject: [PATCH] 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         |   6 +-
 app/graphql/types/issue_type.rb               |  12 +-
 app/graphql/types/merge_request_type.rb       |   8 +-
 app/graphql/types/milestone_type.rb           |   2 +
 app/graphql/types/project_type.rb             |  14 +-
 app/graphql/types/query_type.rb               |   3 +-
 app/graphql/types/user_type.rb                |   2 +
 .../54417-graphql-type-authorization.yml      |   5 +
 config/initializers/graphql.rb                |   3 +
 .../authorize/authorize_field_service.rb      |  94 +++++++
 .../graphql/authorize/instrumentation.rb      |  44 +---
 lib/gitlab/graphql/errors.rb                  |   1 +
 spec/graphql/features/authorization_spec.rb   | 229 ++++++++++++++----
 spec/graphql/types/issue_type_spec.rb         |   2 +
 spec/graphql/types/merge_request_type_spec.rb |  11 +-
 spec/graphql/types/milestone_type_spec.rb     |   9 +
 spec/graphql/types/project_type_spec.rb       |  12 +-
 spec/graphql/types/query_type_spec.rb         |   4 -
 spec/graphql/types/user_type_spec.rb          |   9 +
 ...pec.rb => authorize_field_service_spec.rb} |  22 +-
 20 files changed, 353 insertions(+), 139 deletions(-)
 create mode 100644 changelogs/unreleased/54417-graphql-type-authorization.yml
 create mode 100644 lib/gitlab/graphql/authorize/authorize_field_service.rb
 create mode 100644 spec/graphql/types/milestone_type_spec.rb
 create mode 100644 spec/graphql/types/user_type_spec.rb
 rename spec/lib/gitlab/graphql/authorize/{instrumentation_spec.rb => authorize_field_service_spec.rb} (73%)

diff --git a/app/graphql/types/ci/pipeline_type.rb b/app/graphql/types/ci/pipeline_type.rb
index 18696293b97..de7d6570a3e 100644
--- a/app/graphql/types/ci/pipeline_type.rb
+++ b/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
 
diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb
index 5ad3ea52930..adb137dfee3 100644
--- a/app/graphql/types/issue_type.rb
+++ b/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
diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb
index 1ed27a14e33..120ffe0dfde 100644
--- a/app/graphql/types/merge_request_type.rb
+++ b/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
diff --git a/app/graphql/types/milestone_type.rb b/app/graphql/types/milestone_type.rb
index af31b572c9a..2772fbec86f 100644
--- a/app/graphql/types/milestone_type.rb
+++ b/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
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb
index b96c2f3afb2..fbb4eddd13c 100644
--- a/app/graphql/types/project_type.rb
+++ b/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
diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb
index 472fe5d6ec2..0f655ab9d03 100644
--- a/app/graphql/types/query_type.rb
+++ b/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,
diff --git a/app/graphql/types/user_type.rb b/app/graphql/types/user_type.rb
index a13e65207df..6b53554314b 100644
--- a/app/graphql/types/user_type.rb
+++ b/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
diff --git a/changelogs/unreleased/54417-graphql-type-authorization.yml b/changelogs/unreleased/54417-graphql-type-authorization.yml
new file mode 100644
index 00000000000..528b58a858a
--- /dev/null
+++ b/changelogs/unreleased/54417-graphql-type-authorization.yml
@@ -0,0 +1,5 @@
+---
+title: GraphQL Types can be made to always authorize access to resources of that Type
+merge_request: 25724
+author:
+type: added
diff --git a/config/initializers/graphql.rb b/config/initializers/graphql.rb
index e653556231d..f1bc289f1f0 100644
--- a/config/initializers/graphql.rb
+++ b/config/initializers/graphql.rb
@@ -1,4 +1,7 @@
 # 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)
diff --git a/lib/gitlab/graphql/authorize/authorize_field_service.rb b/lib/gitlab/graphql/authorize/authorize_field_service.rb
new file mode 100644
index 00000000000..f3ca82ec697
--- /dev/null
+++ b/lib/gitlab/graphql/authorize/authorize_field_service.rb
@@ -0,0 +1,94 @@
+# 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
diff --git a/lib/gitlab/graphql/authorize/instrumentation.rb b/lib/gitlab/graphql/authorize/instrumentation.rb
index 593da8471dd..15ecc3b04f0 100644
--- a/lib/gitlab/graphql/authorize/instrumentation.rb
+++ b/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
diff --git a/lib/gitlab/graphql/errors.rb b/lib/gitlab/graphql/errors.rb
index fe74549e322..bcbba72e017 100644
--- a/lib/gitlab/graphql/errors.rb
+++ b/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
diff --git a/spec/graphql/features/authorization_spec.rb b/spec/graphql/features/authorization_spec.rb
index a229d29afa6..f863c4444b8 100644
--- a/spec/graphql/features/authorization_spec.rb
+++ b/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
 
-    it 'should return the protected field when user has permission' do
-      permit(:foo)
-
-      expect(subject['name']).to eq(test_object.name)
+      include_examples 'authorization with a single permission'
     end
 
-    it 'should return nil when user is not authorized' do
-      expect(subject).to be_nil
+    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
+
+      include_examples 'authorization with a collection of permissions'
     end
   end
 
-  describe 'authorizing with an Array of permissions' do
-    let(:query_string) { '{ permissionCollection() { name } }' }
-
-    subject { result['permissionCollection'] }
-
-    it 'should return the protected field when user has all permissions' do
-      permit(:foo, :bar)
-
-      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 only has one of the permissions' do
-      permit(:foo)
+    describe 'with a single permission' do
+      let(:type) do
+        type_factory do |type|
+          type.authorize permission_single
+        end
+      end
 
-      expect(subject).to be_nil
+      include_examples 'authorization with a single permission'
     end
 
-    it 'should return nil when user only has none of the permissions' do
-      expect(subject).to be_nil
+    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 'type and field authorizations together' do
+    let(:permission_1) { permission_collection.first }
+    let(:permission_2) { permission_collection.last }
+
+    let(:type) do
+      type_factory do |type|
+        type.authorize permission_1
+      end
+    end
+
+    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
+
+    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
+
+    subject { result.dig('object', 'edges') }
+
+    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 '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
diff --git a/spec/graphql/types/issue_type_spec.rb b/spec/graphql/types/issue_type_spec.rb
index 63a07647a60..dc37b15001f 100644
--- a/spec/graphql/types/issue_type_spec.rb
+++ b/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
diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb
index c369953e3ea..89c12879074 100644
--- a/spec/graphql/types/merge_request_type_spec.rb
+++ b/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
diff --git a/spec/graphql/types/milestone_type_spec.rb b/spec/graphql/types/milestone_type_spec.rb
new file mode 100644
index 00000000000..f7ee79eae9f
--- /dev/null
+++ b/spec/graphql/types/milestone_type_spec.rb
@@ -0,0 +1,9 @@
+# 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
diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb
index e8f1c84f8d6..e0ad09bdf22 100644
--- a/spec/graphql/types/project_type_spec.rb
+++ b/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
diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb
index 07c61ea7647..69e3ea8a4a9 100644
--- a/spec/graphql/types/query_type_spec.rb
+++ b/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
diff --git a/spec/graphql/types/user_type_spec.rb b/spec/graphql/types/user_type_spec.rb
new file mode 100644
index 00000000000..8134cc13eb4
--- /dev/null
+++ b/spec/graphql/types/user_type_spec.rb
@@ -0,0 +1,9 @@
+# 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
diff --git a/spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb
similarity index 73%
rename from spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb
rename to spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb
index cf3a8bcc8b4..ce320a2bdb0 100644
--- a/spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb
+++ b/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