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

GraphQL Type authorization

Closes #54417

See merge request gitlab-org/gitlab-ce!25724
This commit is contained in:
Nick Thomas 2019-04-04 11:38:16 +00:00
commit 7af1ba122f
21 changed files with 461 additions and 171 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -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,

View file

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

View file

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

View file

@ -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)

View file

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

View file

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

View file

@ -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)
if service.authorizations?
field.redefine { resolve(service.authorized_resolve) }
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
field
end
end
end

View file

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

View file

@ -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
let(:result) { execute['data'] }
it 'returns nil when user is not authorized' do
expect(subject).to be_nil
end
end
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'] }
it 'should return the protected field when user has permission' do
permit(:foo)
expect(subject['name']).to eq(test_object.name)
end
it 'should return nil when user is not authorized' do
expect(subject).to be_nil
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
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)
include_examples 'authorization with a single permission'
end
it 'should return nil when user only has one of the permissions' do
permit(:foo)
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
it 'should return nil when user only has none of the permissions' do
expect(subject).to be_nil
include_examples 'authorization with a collection of permissions'
end
end
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
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 '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_type)
end
query(query)
end
schema.execute(
query_string,
context: { current_user: user },
variables: {}
)
end
end

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -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,6 +60,9 @@ describe Gitlab::Graphql::Authorize::Instrumentation do
.to contain_exactly(allowed)
end
end
end
private
def spy_ability_check_for(ability, object, passed: true)
expect(Ability)
@ -64,4 +71,3 @@ describe Gitlab::Graphql::Authorize::Instrumentation do
.and_return(passed)
end
end
end