summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2019-04-04 11:38:16 +0000
committerNick Thomas <nick@gitlab.com>2019-04-04 11:38:16 +0000
commit7af1ba122fb425214d6b7c9e51ea621a515d6ac0 (patch)
tree9f37fe6e0e7b68ab3bf36df2606936c51b701c0e
parent60a0ef21d385e1943f9e6a68adc9d7e04e8d69c8 (diff)
parent8cf0d8926a325c8be7707c356ef20f42139d7bf3 (diff)
downloadgitlab-ce-7af1ba122fb425214d6b7c9e51ea621a515d6ac0.tar.gz
Merge branch '54417-graphql-type-authorization' into 'master'
GraphQL Type authorization Closes #54417 See merge request gitlab-org/gitlab-ce!25724
-rw-r--r--app/graphql/types/ci/pipeline_type.rb6
-rw-r--r--app/graphql/types/issue_type.rb12
-rw-r--r--app/graphql/types/merge_request_type.rb8
-rw-r--r--app/graphql/types/milestone_type.rb2
-rw-r--r--app/graphql/types/project_type.rb14
-rw-r--r--app/graphql/types/query_type.rb3
-rw-r--r--app/graphql/types/user_type.rb2
-rw-r--r--changelogs/unreleased/54417-graphql-type-authorization.yml5
-rw-r--r--config/initializers/graphql.rb3
-rw-r--r--doc/development/api_graphql_styleguide.md140
-rw-r--r--lib/gitlab/graphql/authorize/authorize_field_service.rb94
-rw-r--r--lib/gitlab/graphql/authorize/instrumentation.rb44
-rw-r--r--lib/gitlab/graphql/errors.rb1
-rw-r--r--spec/graphql/features/authorization_spec.rb221
-rw-r--r--spec/graphql/types/issue_type_spec.rb2
-rw-r--r--spec/graphql/types/merge_request_type_spec.rb11
-rw-r--r--spec/graphql/types/milestone_type_spec.rb9
-rw-r--r--spec/graphql/types/project_type_spec.rb12
-rw-r--r--spec/graphql/types/query_type_spec.rb4
-rw-r--r--spec/graphql/types/user_type_spec.rb9
-rw-r--r--spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb (renamed from spec/lib/gitlab/graphql/authorize/instrumentation_spec.rb)22
21 files changed, 457 insertions, 167 deletions
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/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md
index 501092ff2aa..8d2bfff3a5d 100644
--- a/doc/development/api_graphql_styleguide.md
+++ b/doc/development/api_graphql_styleguide.md
@@ -9,38 +9,6 @@ can be shared.
It is also possible to add a `private_token` to the querystring, or
add a `HTTP_PRIVATE_TOKEN` header.
-### Authorization
-
-Fields can be authorized using the same abilities used in the Rails
-app. This can be done by supplying the `authorize` option:
-
-```ruby
-module Types
- class QueryType < BaseObject
- graphql_name 'Query'
-
- field :project, Types::ProjectType, null: true, resolver: Resolvers::ProjectResolver, authorize: :read_project
- end
-end
-```
-
-Fields can be authorized against multiple abilities, in which case all
-ability checks must pass. This requires explicitly passing a block to `field`:
-
-```ruby
-field :project, Types::ProjectType, null: true, resolver: Resolvers::ProjectResolver do
- authorize [:read_project, :another_ability]
-end
-```
-
-The object found by the resolve call is used for authorization.
-
-TIP: **Tip:**
-When authorizing collections, try to load only what the currently
-authenticated user is allowed to view with our existing finders first.
-This minimizes database queries and unnecessary authorization checks of
-the loaded records.
-
## Types
When exposing a model through the GraphQL API, we do so by creating a
@@ -197,6 +165,114 @@ end
policies at once. The fields for these will all have be non-nullable
booleans with a default description.
+## Authorization
+
+Authorizations can be applied to both types and fields using the same
+abilities as in the Rails app.
+
+If the:
+
+- Currently authenticated user fails the authorization, the authorized
+resource will be returned as `null`.
+- Resource is part of a collection, the collection will be filtered to
+exclude the objects that the user's authorization checks failed against.
+
+TIP: **Tip:**
+Try to load only what the currently authenticated user is allowed to
+view with our existing finders first, without relying on authorization
+to filter the records. This minimizes database queries and unnecessary
+authorization checks of the loaded records.
+
+### Type authorization
+
+Authorize a type by passing an ability to the `authorize` method. All
+fields with the same type will be authorized by checking that the
+currently authenticated user has the required ability.
+
+For example, the following authorization ensures that the currently
+authenticated user can only see projects that they have the
+`read_project` ability for (so long as the project is returned in a
+field that uses `Types::ProjectType`):
+
+```ruby
+module Types
+ class ProjectType < BaseObject
+ authorize :read_project
+ end
+end
+```
+
+You can also authorize against multiple abilities, in which case all of
+the ability checks must pass.
+
+For example, the following authorization ensures that the currently
+authenticated user must have `read_project` and `another_ability`
+abilities to see a project:
+
+```ruby
+module Types
+ class ProjectType < BaseObject
+ authorize [:read_project, :another_ability]
+ end
+end
+```
+
+### Field authorization
+
+Fields can be authorized with the `authorize` option.
+
+For example, the following authorization ensures that the currently
+authenticated user must have the `owner_access` ability to see the
+project:
+
+```ruby
+module Types
+ class MyType < BaseObject
+ field :project, Types::ProjectType, null: true, resolver: Resolvers::ProjectResolver, authorize: :owner_access
+ end
+end
+```
+
+Fields can also be authorized against multiple abilities, in which case
+all of ability checks must pass. **Note:** This requires explicitly
+passing a block to `field`:
+
+```ruby
+module Types
+ class MyType < BaseObject
+ field :project, Types::ProjectType, null: true, resolver: Resolvers::ProjectResolver do
+ authorize [:owner_access, :another_ability]
+ end
+ end
+end
+```
+
+NOTE: **Note:** If the field's type already [has a particular
+authorization](#type-authorization) then there is no need to add that
+same authorization to the field.
+
+### Type and Field authorizations together
+
+Authorizations are cumulative, so where authorizations are defined on
+a field, and also on the field's type, then the currently authenticated
+user would need to pass all ability checks.
+
+In the following simplified example the currently authenticated user
+would need both `first_permission` and `second_permission` abilities in
+order to see the author of the issue.
+
+```ruby
+class UserType
+ authorize :first_permission
+end
+```
+
+```ruby
+class IssueType
+ field :author, UserType, authorize: :second_permission
+end
+```
+
## Resolvers
To find objects to display in a field, we can add resolvers to
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
+
+ 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