Merge branch '54417-graphql-type-authorization' into 'master'

GraphQL Type authorization

Closes #54417

See merge request gitlab-org/gitlab-ce!25724

8 files changed, 214 insertions, 76 deletions

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
+
+      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
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
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