diff options
23 files changed, 350 insertions, 51 deletions
diff --git a/app/graphql/types/base_object.rb b/app/graphql/types/base_object.rb index e033ef96ce9..754adf4c04d 100644 --- a/app/graphql/types/base_object.rb +++ b/app/graphql/types/base_object.rb @@ -1,6 +1,7 @@ module Types class BaseObject < GraphQL::Schema::Object prepend Gitlab::Graphql::Present + prepend Gitlab::Graphql::ExposePermissions field_class Types::BaseField end diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index d5d24952984..a1f3c0dd8c0 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -1,5 +1,7 @@ module Types class MergeRequestType < BaseObject + expose_permissions Types::PermissionTypes::MergeRequest + present_using MergeRequestPresenter graphql_name 'MergeRequest' diff --git a/app/graphql/types/permission_types/base_permission_type.rb b/app/graphql/types/permission_types/base_permission_type.rb new file mode 100644 index 00000000000..934ed572e56 --- /dev/null +++ b/app/graphql/types/permission_types/base_permission_type.rb @@ -0,0 +1,38 @@ +module Types + module PermissionTypes + class BasePermissionType < BaseObject + extend Gitlab::Allowable + + RESOLVING_KEYWORDS = [:resolver, :method, :hash_key, :function].to_set.freeze + + def self.abilities(*abilities) + abilities.each { |ability| ability_field(ability) } + end + + def self.ability_field(ability, **kword_args) + unless resolving_keywords?(kword_args) + kword_args[:resolve] ||= -> (object, args, context) do + can?(context[:current_user], ability, object, args.to_h) + end + end + + permission_field(ability, **kword_args) + end + + def self.permission_field(name, **kword_args) + kword_args = kword_args.reverse_merge( + name: name, + type: GraphQL::BOOLEAN_TYPE, + description: "Whether or not a user can perform `#{name}` on this resource", + null: false) + + field(**kword_args) + end + + def self.resolving_keywords?(arguments) + RESOLVING_KEYWORDS.intersect?(arguments.keys.to_set) + end + private_class_method :resolving_keywords? + end + end +end diff --git a/app/graphql/types/permission_types/merge_request.rb b/app/graphql/types/permission_types/merge_request.rb new file mode 100644 index 00000000000..5c21f6ee9c6 --- /dev/null +++ b/app/graphql/types/permission_types/merge_request.rb @@ -0,0 +1,17 @@ +module Types + module PermissionTypes + class MergeRequest < BasePermissionType + present_using MergeRequestPresenter + description 'Check permissions for the current user on a merge request' + graphql_name 'MergeRequestPermissions' + + abilities :read_merge_request, :admin_merge_request, + :update_merge_request, :create_note + + permission_field :push_to_source_branch, method: :can_push_to_source_branch? + permission_field :remove_source_branch, method: :can_remove_source_branch? + permission_field :cherry_pick_on_current_merge_request, method: :can_cherry_pick_on_current_merge_request? + permission_field :revert_on_current_merge_request, method: :can_revert_on_current_merge_request? + end + end +end diff --git a/app/graphql/types/permission_types/project.rb b/app/graphql/types/permission_types/project.rb new file mode 100644 index 00000000000..755699a4415 --- /dev/null +++ b/app/graphql/types/permission_types/project.rb @@ -0,0 +1,20 @@ +module Types + module PermissionTypes + class Project < BasePermissionType + graphql_name 'ProjectPermissions' + + abilities :change_namespace, :change_visibility_level, :rename_project, + :remove_project, :archive_project, :remove_fork_project, + :remove_pages, :read_project, :create_merge_request_in, + :read_wiki, :read_project_member, :create_issue, :upload_file, + :read_cycle_analytics, :download_code, :download_wiki_code, + :fork_project, :create_project_snippet, :read_commit_status, + :request_access, :create_pipeline, :create_pipeline_schedule, + :create_merge_request_from, :create_wiki, :push_code, + :create_deployment, :push_to_delete_protected_branch, + :admin_wiki, :admin_project, :update_pages, + :admin_remote_mirror, :create_label, :update_wiki, :destroy_wiki, + :create_pages, :destroy_pages + end + end +end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index d9058ae7431..a832e8b4bde 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -1,5 +1,7 @@ module Types class ProjectType < BaseObject + expose_permissions Types::PermissionTypes::Project + graphql_name 'Project' field :id, GraphQL::ID_TYPE, null: false diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index eb54ab2cda6..f77b3541644 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -168,6 +168,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated .can_push_to_branch?(source_branch) end + def can_remove_source_branch? + source_branch_exists? && merge_request.can_remove_source_branch?(current_user) + end + def mergeable_discussions_state # This avoids calling MergeRequest#mergeable_discussions_state without # considering the state of the MR first. If a MR isn't mergeable, we can diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 0426afc1b4a..5d72ebdd7fd 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -109,7 +109,7 @@ class MergeRequestWidgetEntity < IssuableEntity expose :current_user do expose :can_remove_source_branch do |merge_request| - merge_request.source_branch_exists? && merge_request.can_remove_source_branch?(current_user) + presenter(merge_request).can_remove_source_branch? end expose :can_revert_on_current_merge_request do |merge_request| diff --git a/changelogs/unreleased/bvl-graphql-permissions.yml b/changelogs/unreleased/bvl-graphql-permissions.yml new file mode 100644 index 00000000000..42d5e24bb15 --- /dev/null +++ b/changelogs/unreleased/bvl-graphql-permissions.yml @@ -0,0 +1,5 @@ +--- +title: 'Expose permissions of the current user on resources in GraphQL' +merge_request: 20152 +author: +type: added diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md index 59e27922f64..71922318227 100644 --- a/doc/api/graphql/index.md +++ b/doc/api/graphql/index.md @@ -1,4 +1,4 @@ -# GraphQL API (Beta) +# GraphQL API (Alpha) > [Introduced][ce-19008] in GitLab 11.0. diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index f74e4f0bd7e..33f078b0a63 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -54,6 +54,51 @@ a new presenter specifically for GraphQL. The presenter is initialized using the object resolved by a field, and the context. +### Exposing permissions for a type + +To expose permissions the current user has on a resource, you can call +the `expose_permissions` passing in a separate type representing the +permissions for the resource. + +For example: + +```ruby +module Types + class MergeRequestType < BaseObject + expose_permissions Types::MergeRequestPermissionsType + end +end +``` + +The permission type inherits from `BasePermissionType` which includes +some helper methods, that allow exposing permissions as non-nullable +booleans: + +```ruby +class MergeRequestPermissionsType < BasePermissionType + present_using MergeRequestPresenter + + graphql_name 'MergeRequestPermissions' + + abilities :admin_merge_request, :update_merge_request, :create_note + + ability_field :resolve_note, + description: 'Whether or not the user can resolve disussions on the merge request' + permission_field :push_to_source_branch, method: :can_push_to_source_branch? +end +``` + +- **`permission_field`**: Will act the same as `graphql-ruby`'s + `field` method but setting a default description and type and making + them non-nullable. These options can still be overridden by adding + them as arguments. +- **`ability_field`**: Expose an ability defined in our policies. This + takes behaves the same way as `permission_field` and the same + arguments can be overridden. +- **`abilities`**: Allows exposing several abilities defined in our + policies at once. The fields for these will all have be non-nullable + booleans with a default description. + ## Resolvers To find objects to display in a field, we can add resolvers to diff --git a/lib/gitlab/graphql/expose_permissions.rb b/lib/gitlab/graphql/expose_permissions.rb new file mode 100644 index 00000000000..e3779995406 --- /dev/null +++ b/lib/gitlab/graphql/expose_permissions.rb @@ -0,0 +1,15 @@ +module Gitlab + module Graphql + module ExposePermissions + extend ActiveSupport::Concern + prepended do + def self.expose_permissions(permission_type, description: 'Permissions for the current user on the resource') + field :user_permissions, permission_type, + description: description, + null: false, + resolve: -> (obj, _, _) { obj } + end + end + end + end +end diff --git a/lib/gitlab/graphql/present/instrumentation.rb b/lib/gitlab/graphql/present/instrumentation.rb index 1688262974b..6f2b26c9676 100644 --- a/lib/gitlab/graphql/present/instrumentation.rb +++ b/lib/gitlab/graphql/present/instrumentation.rb @@ -10,9 +10,18 @@ module Gitlab old_resolver = field.resolve_proc resolve_with_presenter = -> (presented_type, args, context) do + # We need to wrap the original presentation type into a type that + # uses the presenter as an object. object = presented_type.object + + if object.is_a?(presented_in.presenter_class) + next old_resolver.call(presented_type, args, context) + end + presenter = presented_in.presenter_class.new(object, **context.to_h) - old_resolver.call(presenter, args, context) + wrapped = presented_type.class.new(presenter, context) + + old_resolver.call(wrapped, args, context) end field.redefine do diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb new file mode 100644 index 00000000000..6e57122867a --- /dev/null +++ b/spec/graphql/types/merge_request_type_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe Types::MergeRequestType do + it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::MergeRequest) } +end diff --git a/spec/graphql/types/permission_types/base_permission_type_spec.rb b/spec/graphql/types/permission_types/base_permission_type_spec.rb new file mode 100644 index 00000000000..a7e51797047 --- /dev/null +++ b/spec/graphql/types/permission_types/base_permission_type_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Types::PermissionTypes::BasePermissionType do + let(:permitable) { double('permittable') } + let(:current_user) { build(:user) } + let(:context) { { current_user: current_user } } + subject(:test_type) do + Class.new(described_class) do + graphql_name 'TestClass' + + permission_field :do_stuff, resolve: -> (_, _, _) { true } + ability_field(:read_issue) + abilities :admin_issue + end + end + + describe '.permission_field' do + it 'adds a field for the required permission' do + is_expected.to have_graphql_field(:do_stuff) + end + end + + describe '.ability_field' do + it 'adds a field for the required permission' do + is_expected.to have_graphql_field(:read_issue) + end + + it 'does not add a resolver block if another resolving param is passed' do + expected_keywords = { + name: :resolve_using_hash, + hash_key: :the_key, + type: GraphQL::BOOLEAN_TYPE, + description: "custom description", + null: false + } + expect(test_type).to receive(:field).with(expected_keywords) + + test_type.ability_field :resolve_using_hash, hash_key: :the_key, description: "custom description" + end + end + + describe '.abilities' do + it 'adds a field for the passed permissions' do + is_expected.to have_graphql_field(:admin_issue) + end + end +end diff --git a/spec/graphql/types/permission_types/merge_request_spec.rb b/spec/graphql/types/permission_types/merge_request_spec.rb new file mode 100644 index 00000000000..e1026b01a74 --- /dev/null +++ b/spec/graphql/types/permission_types/merge_request_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe Types::PermissionTypes::MergeRequest do + it do + expected_permissions = [ + :read_merge_request, :admin_merge_request, :update_merge_request, + :create_note, :push_to_source_branch, :remove_source_branch, + :cherry_pick_on_current_merge_request, :revert_on_current_merge_request + ] + + expect(described_class).to have_graphql_fields(expected_permissions) + end +end diff --git a/spec/graphql/types/permission_types/merge_request_type_spec.rb b/spec/graphql/types/permission_types/merge_request_type_spec.rb new file mode 100644 index 00000000000..6e57122867a --- /dev/null +++ b/spec/graphql/types/permission_types/merge_request_type_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe Types::MergeRequestType do + it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::MergeRequest) } +end diff --git a/spec/graphql/types/permission_types/project_spec.rb b/spec/graphql/types/permission_types/project_spec.rb new file mode 100644 index 00000000000..89eecef096e --- /dev/null +++ b/spec/graphql/types/permission_types/project_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe Types::PermissionTypes::Project do + it do + expected_permissions = [ + :change_namespace, :change_visibility_level, :rename_project, :remove_project, :archive_project, + :remove_fork_project, :remove_pages, :read_project, :create_merge_request_in, + :read_wiki, :read_project_member, :create_issue, :upload_file, :read_cycle_analytics, + :download_code, :download_wiki_code, :fork_project, :create_project_snippet, + :read_commit_status, :request_access, :create_pipeline, :create_pipeline_schedule, + :create_merge_request_from, :create_wiki, :push_code, :create_deployment, :push_to_delete_protected_branch, + :admin_wiki, :admin_project, :update_pages, :admin_remote_mirror, :create_label, + :update_wiki, :destroy_wiki, :create_pages, :destroy_pages + ] + + expect(described_class).to have_graphql_fields(expected_permissions) + end +end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index b4eeca2e3f1..7b5bc335511 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe GitlabSchema.types['Project'] do + it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::Project) } + it { expect(described_class.graphql_name).to eq('Project') } describe 'nested merge request' do diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb new file mode 100644 index 00000000000..ad57c43bc87 --- /dev/null +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe 'getting merge request information nested in a project' do + include GraphqlHelpers + + let(:project) { create(:project, :repository, :public) } + let(:current_user) { create(:user) } + let(:merge_request_graphql_data) { graphql_data['project']['mergeRequest'] } + let!(:merge_request) { create(:merge_request, source_project: project) } + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('mergeRequest', iid: merge_request.iid) + ) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'contains merge request information' do + post_graphql(query, current_user: current_user) + + expect(merge_request_graphql_data).not_to be_nil + end + + # This is a field coming from the `MergeRequestPresenter` + it 'includes a web_url' do + post_graphql(query, current_user: current_user) + + expect(merge_request_graphql_data['webUrl']).to be_present + end + + context 'permissions on the merge request' do + it 'includes the permissions for the current user on a public project' do + expected_permissions = { + 'readMergeRequest' => true, + 'adminMergeRequest' => false, + 'createNote' => true, + 'pushToSourceBranch' => false, + 'removeSourceBranch' => false, + 'cherryPickOnCurrentMergeRequest' => false, + 'revertOnCurrentMergeRequest' => false, + 'updateMergeRequest' => false + } + post_graphql(query, current_user: current_user) + + permission_data = merge_request_graphql_data['userPermissions'] + + expect(permission_data).to be_present + expect(permission_data).to eq(expected_permissions) + end + end + + context 'when the user does not have access to the merge request' do + let(:project) { create(:project, :public, :repository) } + + it 'returns nil' do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + + post_graphql(query) + + expect(merge_request_graphql_data).to be_nil + end + end +end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index 796ffc9d569..a2b3dc5d121 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -26,50 +26,6 @@ describe 'getting project information' do post_graphql(query, current_user: current_user) end end - - context 'when requesting a nested merge request' do - let(:merge_request) { create(:merge_request, source_project: project) } - let(:merge_request_graphql_data) { graphql_data['project']['mergeRequest'] } - - let(:query) do - graphql_query_for( - 'project', - { 'fullPath' => project.full_path }, - query_graphql_field('mergeRequest', iid: merge_request.iid) - ) - end - - it_behaves_like 'a working graphql query' do - before do - post_graphql(query, current_user: current_user) - end - end - - it 'contains merge request information' do - post_graphql(query, current_user: current_user) - - expect(merge_request_graphql_data).not_to be_nil - end - - # This is a field coming from the `MergeRequestPresenter` - it 'includes a web_url' do - post_graphql(query, current_user: current_user) - - expect(merge_request_graphql_data['webUrl']).to be_present - end - - context 'when the user does not have access to the merge request' do - let(:project) { create(:project, :public, :repository) } - - it 'returns nil' do - project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) - - post_graphql(query) - - expect(merge_request_graphql_data).to be_nil - end - end - end end context 'when the user does not have access to the project' do diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb index d23cbaf4beb..be6fa4c71a0 100644 --- a/spec/support/matchers/graphql_matchers.rb +++ b/spec/support/matchers/graphql_matchers.rb @@ -7,9 +7,24 @@ RSpec::Matchers.define :require_graphql_authorizations do |*expected| end RSpec::Matchers.define :have_graphql_fields do |*expected| + def expected_field_names + expected.map { |name| GraphqlHelpers.fieldnamerize(name) } + end + match do |kls| - field_names = expected.map { |name| GraphqlHelpers.fieldnamerize(name) } - expect(kls.fields.keys).to contain_exactly(*field_names) + expect(kls.fields.keys).to contain_exactly(*expected_field_names) + end + + failure_message do |kls| + missing = expected_field_names - kls.fields.keys + extra = kls.fields.keys - expected_field_names + + message = [] + + message << "is missing fields: <#{missing.inspect}>" if missing.any? + message << "contained unexpected fields: <#{extra.inspect}>" if extra.any? + + message.join("\n") end end @@ -44,3 +59,13 @@ RSpec::Matchers.define :have_graphql_resolver do |expected| end end end + +RSpec::Matchers.define :expose_permissions_using do |expected| + match do |type| + permission_field = type.fields['userPermissions'] + + expect(permission_field).not_to be_nil + expect(permission_field.type).to be_non_null + expect(permission_field.type.of_type.graphql_name).to eq(expected.graphql_name) + end +end diff --git a/spec/support/shared_examples/requests/graphql_shared_examples.rb b/spec/support/shared_examples/requests/graphql_shared_examples.rb index 9b2b74593a5..fe7b7bc306f 100644 --- a/spec/support/shared_examples/requests/graphql_shared_examples.rb +++ b/spec/support/shared_examples/requests/graphql_shared_examples.rb @@ -3,8 +3,8 @@ require 'spec_helper' shared_examples 'a working graphql query' do include GraphqlHelpers - it 'is returns a successfull response', :aggregate_failures do - expect(response).to be_success + it 'returns a successful response', :aggregate_failures do + expect(response).to have_gitlab_http_status(:success) expect(graphql_errors['errors']).to be_nil expect(json_response.keys).to include('data') end |