From aaeb41c24ccb5bcb54e3dd902a1c807e6653a067 Mon Sep 17 00:00:00 2001 From: Joe Woodward <jwoodward@gitlab.com> Date: Tue, 18 Jul 2023 18:47:54 +0100 Subject: [PATCH] Expose deploy key in push access level graphql type Branch rules requires us to display when a deploy key is assigned to the push access levels. Prior to this change we didn't return deploy key's as a type. In EE we expose `user` and `group` as their own types so clients can differentiate between role access levels. Used as inspiration to expose `deploy_key`s. Adds a new policy rule to `DeployKeyPolicy` to enable `read_deploy_key` if the deploy key is public or the user has access to a project where the key is assigned. Changelog: added --- .../types/access_levels/deploy_key_type.rb | 32 ++++++ .../graphql/types/access_levels/user_type.rb | 2 +- .../push_access_level_type.rb | 5 + app/policies/deploy_key_policy.rb | 10 +- doc/api/graphql/reference/index.md | 16 ++- .../push_access_level_type_spec.rb | 2 +- spec/factories/keys.rb | 10 +- .../access_levels/deploy_key_type_spec.rb | 13 +++ .../types/access_levels/user_type_spec.rb | 2 +- .../push_access_level_type_spec.rb | 4 +- spec/policies/deploy_key_policy_spec.rb | 100 +++++++++++------- 11 files changed, 146 insertions(+), 50 deletions(-) create mode 100644 app/graphql/types/access_levels/deploy_key_type.rb rename {ee/app => app}/graphql/types/access_levels/user_type.rb (94%) create mode 100644 spec/graphql/types/access_levels/deploy_key_type_spec.rb rename {ee/spec => spec}/graphql/types/access_levels/user_type_spec.rb (92%) diff --git a/app/graphql/types/access_levels/deploy_key_type.rb b/app/graphql/types/access_levels/deploy_key_type.rb new file mode 100644 index 0000000000000..e4e9061989146 --- /dev/null +++ b/app/graphql/types/access_levels/deploy_key_type.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Types + module AccessLevels + class DeployKeyType < BaseObject + graphql_name 'AccessLevelDeployKey' + description 'Representation of a GitLab deploy key.' + + authorize :read_deploy_key + + field :id, + type: GraphQL::Types::ID, + null: false, + description: 'ID of the deploy key.' + + field :title, + type: GraphQL::Types::String, + null: false, + description: 'Title of the deploy key.' + + field :expires_at, + type: Types::DateType, + null: true, + description: 'Expiration date of the deploy key.' + + field :user, + type: Types::AccessLevels::UserType, + null: false, + description: 'User assigned to the deploy key.' + end + end +end diff --git a/ee/app/graphql/types/access_levels/user_type.rb b/app/graphql/types/access_levels/user_type.rb similarity index 94% rename from ee/app/graphql/types/access_levels/user_type.rb rename to app/graphql/types/access_levels/user_type.rb index 5c273bdc1c1cb..82aba6732506c 100644 --- a/ee/app/graphql/types/access_levels/user_type.rb +++ b/app/graphql/types/access_levels/user_type.rb @@ -18,7 +18,7 @@ class UserType < BaseObject field :username, type: GraphQL::Types::String, null: false, - description: 'Username of the user. Unique within this instance of GitLab.' + description: 'Username of the user.' field :name, type: GraphQL::Types::String, diff --git a/app/graphql/types/branch_protections/push_access_level_type.rb b/app/graphql/types/branch_protections/push_access_level_type.rb index c5e21fad88db7..2a66f1a4ec59b 100644 --- a/app/graphql/types/branch_protections/push_access_level_type.rb +++ b/app/graphql/types/branch_protections/push_access_level_type.rb @@ -6,6 +6,11 @@ class PushAccessLevelType < BaseAccessLevelType # rubocop:disable Graphql/Author graphql_name 'PushAccessLevel' description 'Defines which user roles, users, or groups can push to a protected branch.' accepts ::ProtectedBranch::PushAccessLevel + + field :deploy_key, + Types::AccessLevels::DeployKeyType, + null: true, + description: 'Deploy key assigned to the access level.' end end end diff --git a/app/policies/deploy_key_policy.rb b/app/policies/deploy_key_policy.rb index b117bb5792198..ccf1bda26bb6b 100644 --- a/app/policies/deploy_key_policy.rb +++ b/app/policies/deploy_key_policy.rb @@ -3,10 +3,14 @@ class DeployKeyPolicy < BasePolicy with_options scope: :subject, score: 0 condition(:private_deploy_key) { @subject.private? } + condition(:public_deploy_key) { @subject.public? } condition(:has_deploy_key) { @user.project_deploy_keys.any? { |pdk| pdk.id.eql?(@subject.id) } } rule { anonymous }.prevent_all - - rule { admin }.enable :update_deploy_key - rule { private_deploy_key & has_deploy_key }.enable :update_deploy_key + rule { public_deploy_key | admin | has_deploy_key }.policy do + enable :read_deploy_key + end + rule { admin | (private_deploy_key & has_deploy_key) }.policy do + enable :update_deploy_key + end end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 941e077ffaa3d..b554525ba63dc 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -12077,6 +12077,19 @@ Represents the access level of a relationship between a User and object that it | <a id="accesslevelintegervalue"></a>`integerValue` | [`Int`](#int) | Integer representation of access level. | | <a id="accesslevelstringvalue"></a>`stringValue` | [`AccessLevelEnum`](#accesslevelenum) | String representation of access level. | +### `AccessLevelDeployKey` + +Representation of a GitLab deploy key. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="accessleveldeploykeyexpiresat"></a>`expiresAt` | [`Date`](#date) | Expiration date of the deploy key. | +| <a id="accessleveldeploykeyid"></a>`id` | [`ID!`](#id) | ID of the deploy key. | +| <a id="accessleveldeploykeytitle"></a>`title` | [`String!`](#string) | Title of the deploy key. | +| <a id="accessleveldeploykeyuser"></a>`user` | [`AccessLevelUser!`](#accessleveluser) | User assigned to the deploy key. | + ### `AccessLevelGroup` Representation of a GitLab group. @@ -12103,7 +12116,7 @@ Representation of a GitLab user. | <a id="accessleveluserid"></a>`id` | [`ID!`](#id) | ID of the user. | | <a id="accesslevelusername"></a>`name` | [`String!`](#string) | Human-readable name of the user. Returns `****` if the user is a project bot and the requester does not have permission to view the project. | | <a id="accessleveluserpublicemail"></a>`publicEmail` | [`String`](#string) | User's public email. | -| <a id="accessleveluserusername"></a>`username` | [`String!`](#string) | Username of the user. Unique within this instance of GitLab. | +| <a id="accessleveluserusername"></a>`username` | [`String!`](#string) | Username of the user. | | <a id="accessleveluserwebpath"></a>`webPath` | [`String!`](#string) | Web path of the user. | | <a id="accessleveluserweburl"></a>`webUrl` | [`String!`](#string) | Web URL of the user. | @@ -21910,6 +21923,7 @@ Defines which user roles, users, or groups can push to a protected branch. | ---- | ---- | ----------- | | <a id="pushaccesslevelaccesslevel"></a>`accessLevel` | [`Int!`](#int) | GitLab::Access level. | | <a id="pushaccesslevelaccessleveldescription"></a>`accessLevelDescription` | [`String!`](#string) | Human readable representation for this access level. | +| <a id="pushaccessleveldeploykey"></a>`deployKey` | [`AccessLevelDeployKey`](#accessleveldeploykey) | Deploy key assigned to the access level. | | <a id="pushaccesslevelgroup"></a>`group` | [`AccessLevelGroup`](#accesslevelgroup) | Group associated with this access level. | | <a id="pushaccessleveluser"></a>`user` | [`AccessLevelUser`](#accessleveluser) | User associated with this access level. | diff --git a/ee/spec/graphql/ee/types/branch_protections/push_access_level_type_spec.rb b/ee/spec/graphql/ee/types/branch_protections/push_access_level_type_spec.rb index 671f1591b6a6a..4c6a2cce71b06 100644 --- a/ee/spec/graphql/ee/types/branch_protections/push_access_level_type_spec.rb +++ b/ee/spec/graphql/ee/types/branch_protections/push_access_level_type_spec.rb @@ -5,7 +5,7 @@ RSpec.describe GitlabSchema.types['PushAccessLevel'] do subject { described_class } - let(:fields) { %i[access_level access_level_description user group] } + let(:fields) { %i[access_level access_level_description user group deploy_key] } specify { is_expected.to have_graphql_fields(fields).only } end diff --git a/spec/factories/keys.rb b/spec/factories/keys.rb index f6f06a994947b..4bd41c1faa1cf 100644 --- a/spec/factories/keys.rb +++ b/spec/factories/keys.rb @@ -30,7 +30,15 @@ key { SSHData::PrivateKey::RSA.generate(3072, unsafe_allow_small_key: true).public_key.openssh } end - factory :deploy_key, class: 'DeployKey' + factory :deploy_key, class: 'DeployKey' do + trait :private do + public { false } + end + + trait :public do + public { true } + end + end factory :group_deploy_key, class: 'GroupDeployKey' do user diff --git a/spec/graphql/types/access_levels/deploy_key_type_spec.rb b/spec/graphql/types/access_levels/deploy_key_type_spec.rb new file mode 100644 index 0000000000000..02f58ec4c1508 --- /dev/null +++ b/spec/graphql/types/access_levels/deploy_key_type_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['AccessLevelDeployKey'], feature_category: :source_code_management do + subject { described_class } + + let(:fields) { %i[id title expires_at user] } + + specify { is_expected.to require_graphql_authorizations(:read_deploy_key) } + + specify { is_expected.to have_graphql_fields(fields).at_least } +end diff --git a/ee/spec/graphql/types/access_levels/user_type_spec.rb b/spec/graphql/types/access_levels/user_type_spec.rb similarity index 92% rename from ee/spec/graphql/types/access_levels/user_type_spec.rb rename to spec/graphql/types/access_levels/user_type_spec.rb index 5ea35074d49e0..7a34f70e166e4 100644 --- a/ee/spec/graphql/types/access_levels/user_type_spec.rb +++ b/spec/graphql/types/access_levels/user_type_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GitlabSchema.types['AccessLevelUser'] do +RSpec.describe GitlabSchema.types['AccessLevelUser'], feature_category: :source_code_management do include GraphqlHelpers describe 'config' do diff --git a/spec/graphql/types/branch_protections/push_access_level_type_spec.rb b/spec/graphql/types/branch_protections/push_access_level_type_spec.rb index c78c0bda74c90..ec5d42ac72038 100644 --- a/spec/graphql/types/branch_protections/push_access_level_type_spec.rb +++ b/spec/graphql/types/branch_protections/push_access_level_type_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe GitlabSchema.types['PushAccessLevel'] do +RSpec.describe GitlabSchema.types['PushAccessLevel'], feature_category: :source_code_management do subject { described_class } - let(:fields) { %i[access_level access_level_description] } + let(:fields) { %i[access_level access_level_description deploy_key] } specify { is_expected.to require_graphql_authorizations(:read_protected_branch) } diff --git a/spec/policies/deploy_key_policy_spec.rb b/spec/policies/deploy_key_policy_spec.rb index d84b80a8738bb..754f36ce3b00d 100644 --- a/spec/policies/deploy_key_policy_spec.rb +++ b/spec/policies/deploy_key_policy_spec.rb @@ -2,69 +2,89 @@ require 'spec_helper' -RSpec.describe DeployKeyPolicy do +RSpec.describe DeployKeyPolicy, feature_category: :groups_and_projects do subject { described_class.new(current_user, deploy_key) } - describe 'updating a deploy_key' do - context 'when a regular user' do - let(:current_user) { create(:user) } + let_it_be(:current_user, refind: true) { create(:user) } + let_it_be(:admin) { create(:user, :admin) } - context 'tries to update private deploy key attached to project' do - let(:deploy_key) { create(:deploy_key, public: false) } - let(:project) { create(:project_empty_repo) } + context 'when deploy key is public' do + let_it_be(:deploy_key) { create(:deploy_key, public: true) } - before do - project.add_maintainer(current_user) - project.deploy_keys << deploy_key - end + context 'and current_user is nil' do + let(:current_user) { nil } - it { is_expected.to be_allowed(:update_deploy_key) } - end + it { is_expected.to be_disallowed(:read_deploy_key) } + + it { is_expected.to be_disallowed(:update_deploy_key) } + end - context 'tries to update private deploy key attached to other project' do - let(:deploy_key) { create(:deploy_key, public: false) } - let(:other_project) { create(:project_empty_repo) } + context 'and current_user is present' do + it { is_expected.to be_allowed(:read_deploy_key) } - before do - other_project.deploy_keys << deploy_key - end + it { is_expected.to be_disallowed(:update_deploy_key) } + end - it { is_expected.to be_disallowed(:update_deploy_key) } + context 'when current_user is admin' do + let(:current_user) { admin } + + context 'when admin mode enabled', :enable_admin_mode do + it { is_expected.to be_allowed(:read_deploy_key) } + + it { is_expected.to be_allowed(:update_deploy_key) } end - context 'tries to update public deploy key' do - let(:deploy_key) { create(:another_deploy_key, public: true) } + context 'when admin mode disabled' do + it { is_expected.to be_allowed(:read_deploy_key) } it { is_expected.to be_disallowed(:update_deploy_key) } end end + end + + context 'when deploy key is private' do + let_it_be(:deploy_key) { create(:deploy_key, :private) } + + context 'and current_user is nil' do + let(:current_user) { nil } - context 'when an admin user' do - let(:current_user) { create(:user, :admin) } + it { is_expected.to be_disallowed(:read_deploy_key) } - context 'tries to update private deploy key' do - let(:deploy_key) { create(:deploy_key, public: false) } + it { is_expected.to be_disallowed(:update_deploy_key) } + end + + context 'when current_user is admin' do + let(:current_user) { admin } - context 'when admin mode enabled', :enable_admin_mode do - it { is_expected.to be_allowed(:update_deploy_key) } - end + context 'when admin mode enabled', :enable_admin_mode do + it { is_expected.to be_allowed(:read_deploy_key) } - context 'when admin mode disabled' do - it { is_expected.to be_disallowed(:update_deploy_key) } - end + it { is_expected.to be_allowed(:update_deploy_key) } end - context 'when an admin user tries to update public deploy key' do - let(:deploy_key) { create(:another_deploy_key, public: true) } + context 'when admin mode disabled' do + it { is_expected.to be_disallowed(:read_deploy_key) } + + it { is_expected.to be_disallowed(:update_deploy_key) } + end + end - context 'when admin mode enabled', :enable_admin_mode do - it { is_expected.to be_allowed(:update_deploy_key) } - end + context 'when assigned to the project' do + let_it_be(:deploy_keys_project) { create(:deploy_keys_project, deploy_key: deploy_key) } - context 'when admin mode disabled' do - it { is_expected.to be_disallowed(:update_deploy_key) } - end + before_all do + deploy_keys_project.project.add_maintainer(current_user) end + + it { is_expected.to be_allowed(:read_deploy_key) } + + it { is_expected.to be_allowed(:update_deploy_key) } + end + + context 'when assigned to another project' do + it { is_expected.to be_disallowed(:read_deploy_key) } + + it { is_expected.to be_disallowed(:update_deploy_key) } end end end -- GitLab