diff --git a/app/models/concerns/protected_ref_access.rb b/app/models/concerns/protected_ref_access.rb
index c1c670db5433f63d802792d9480ee89c68f482ec..ce520f946140fb407252f09dcb05f1829f164663 100644
--- a/app/models/concerns/protected_ref_access.rb
+++ b/app/models/concerns/protected_ref_access.rb
@@ -29,14 +29,23 @@ def allowed_access_levels
def humanize(access_level)
human_access_levels[access_level]
end
+
+ def non_role_types
+ []
+ end
end
included do
scope :maintainer, -> { where(access_level: Gitlab::Access::MAINTAINER) }
scope :developer, -> { where(access_level: Gitlab::Access::DEVELOPER) }
- scope :for_role, -> { where(user_id: nil, group_id: nil) }
-
- validates :access_level, presence: true, if: :role?, inclusion: { in: allowed_access_levels }
+ scope :for_role, -> { non_role_types.present? ? where.missing(*non_role_types) : all }
+
+ protected_ref_fk = "#{module_parent.model_name.singular}_id"
+ validates :access_level,
+ presence: true,
+ inclusion: { in: allowed_access_levels },
+ uniqueness: { scope: protected_ref_fk, conditions: -> { for_role } },
+ if: :role?
end
def humanize
diff --git a/app/models/concerns/protected_ref_deploy_key_access.rb b/app/models/concerns/protected_ref_deploy_key_access.rb
new file mode 100644
index 0000000000000000000000000000000000000000..a62cce1368d5ce33bddd4905fe7cbade3576248b
--- /dev/null
+++ b/app/models/concerns/protected_ref_deploy_key_access.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+module ProtectedRefDeployKeyAccess
+ extend ActiveSupport::Concern
+
+ included do
+ belongs_to :deploy_key
+
+ protected_ref_fk = "#{module_parent.model_name.singular}_id"
+ validates :deploy_key_id, uniqueness: { scope: protected_ref_fk, allow_nil: true }
+ validate :validate_deploy_key_membership
+ end
+
+ class_methods do
+ def non_role_types
+ super << :deploy_key
+ end
+ end
+
+ def type
+ return :deploy_key if deploy_key.present?
+
+ super
+ end
+
+ def humanize
+ return "Deploy key" if deploy_key.present?
+
+ super
+ end
+
+ def check_access(current_user)
+ super do
+ break enabled_deploy_key_for_user?(current_user) if deploy_key?
+
+ yield if block_given?
+ end
+ end
+
+ private
+
+ def deploy_key?
+ type == :deploy_key
+ end
+
+ def validate_deploy_key_membership
+ return if deploy_key.nil? || deploy_key_has_write_access_to_project?
+
+ errors.add(:deploy_key, 'is not enabled for this project')
+ end
+
+ def enabled_deploy_key_for_user?(current_user)
+ current_user.can?(:read_project, project) &&
+ deploy_key.user_id == current_user.id &&
+ deploy_key_has_write_access_to_project?
+ end
+
+ def deploy_key_has_write_access_to_project?
+ DeployKey.with_write_access_for_project(project, deploy_key: deploy_key).exists?
+ end
+end
diff --git a/app/models/protected_branch/push_access_level.rb b/app/models/protected_branch/push_access_level.rb
index c86ca5723fa39f989c70d702597d621c800a36a8..53cec0c551101b6815192aa9f0a0dd319cabca0f 100644
--- a/app/models/protected_branch/push_access_level.rb
+++ b/app/models/protected_branch/push_access_level.rb
@@ -3,49 +3,7 @@
class ProtectedBranch::PushAccessLevel < ApplicationRecord
include Importable
include ProtectedBranchAccess
+ include ProtectedRefDeployKeyAccess
# default value for the access_level column
GITLAB_DEFAULT_ACCESS_LEVEL = Gitlab::Access::MAINTAINER
-
- belongs_to :deploy_key
-
- validates :access_level, uniqueness: { scope: :protected_branch_id, if: :role?,
- conditions: -> { where(user_id: nil, group_id: nil, deploy_key_id: nil) } }
- validates :deploy_key_id, uniqueness: { scope: :protected_branch_id, allow_nil: true }
- validate :validate_deploy_key_membership
-
- def type
- if self.deploy_key.present?
- :deploy_key
- else
- super
- end
- end
-
- def humanize
- return "Deploy key" if deploy_key.present?
-
- super
- end
-
- def check_access(user)
- if user && deploy_key.present?
- return user.can?(:read_project, project) && enabled_deploy_key_for_user?(deploy_key, user)
- end
-
- super
- end
-
- private
-
- def validate_deploy_key_membership
- return unless deploy_key
-
- unless project.deploy_keys_projects.where(deploy_key: deploy_key).exists?
- self.errors.add(:deploy_key, 'is not enabled for this project')
- end
- end
-
- def enabled_deploy_key_for_user?(deploy_key, user)
- deploy_key.user_id == user.id && DeployKey.with_write_access_for_project(protected_branch.project, deploy_key: deploy_key).any?
- end
end
diff --git a/app/models/protected_tag/create_access_level.rb b/app/models/protected_tag/create_access_level.rb
index 5837f3a5afb88f270388dbae478c042faa07384d..0eff99241536ab6436d0bcc3e2f8669bf6e4a7e8 100644
--- a/app/models/protected_tag/create_access_level.rb
+++ b/app/models/protected_tag/create_access_level.rb
@@ -3,48 +3,5 @@
class ProtectedTag::CreateAccessLevel < ApplicationRecord
include Importable
include ProtectedTagAccess
-
- belongs_to :deploy_key
-
- validates :access_level, uniqueness: { scope: :protected_tag_id, if: :role?,
- conditions: -> { where(user_id: nil, group_id: nil, deploy_key_id: nil) } }
- validates :deploy_key_id, uniqueness: { scope: :protected_tag_id, allow_nil: true }
- validate :validate_deploy_key_membership
-
- def type
- return :deploy_key if deploy_key.present?
-
- super
- end
-
- def humanize
- return "Deploy key" if deploy_key.present?
-
- super
- end
-
- def check_access(current_user)
- super do
- break enabled_deploy_key_for_user?(current_user) if deploy_key?
- end
- end
-
- private
-
- def deploy_key?
- type == :deploy_key
- end
-
- def validate_deploy_key_membership
- return unless deploy_key
- return if project.deploy_keys_projects.where(deploy_key: deploy_key).exists?
-
- errors.add(:deploy_key, 'is not enabled for this project')
- end
-
- def enabled_deploy_key_for_user?(current_user)
- current_user.can?(:read_project, project) &&
- deploy_key.user_id == current_user.id &&
- DeployKey.with_write_access_for_project(protected_tag.project, deploy_key: deploy_key).any?
- end
+ include ProtectedRefDeployKeyAccess
end
diff --git a/doc/user/project/deploy_keys/index.md b/doc/user/project/deploy_keys/index.md
index 60cad9d6fe97031b2f9bb1e73072c32050672d81..84567c8de416b7db7d2a3adcca3bc75bfe6bb8b5 100644
--- a/doc/user/project/deploy_keys/index.md
+++ b/doc/user/project/deploy_keys/index.md
@@ -127,6 +127,20 @@ To grant a public deploy key access to a project:
1. In the key's row, select **Edit** (**{pencil}**).
1. Select the **Grant write permissions to this key** checkbox.
+### Edit project access permissions of a deploy key
+
+Prerequisites:
+
+- You must have at least the Maintainer role for the project.
+
+To edit the project access permissions of a deploy key:
+
+1. On the left sidebar, at the top, select **Search GitLab** (**{search}**) to find your project.
+1. Select **Settings > Repository**.
+1. Expand **Deploy keys**.
+1. In the key's row, select **Edit** (**{pencil}**).
+1. Select or clear the **Grant write permissions to this key** checkbox.
+
## Revoke project access of a deploy key
To revoke a deploy key's access to a project, you can disable it. Any service that relies on
@@ -158,8 +172,10 @@ What happens to the deploy key when it is disabled depends on the following:
There are a few scenarios where a deploy key fails to push to a
[protected branch](../protected_branches.md).
-- The owner associated to a deploy key does not have access to the protected branch.
- The owner associated to a deploy key does not have [membership](../members/index.md) to the project of the protected branch.
+- The owner associated to a deploy key has [project membership permissions](../../../user/permissions.md#project-members-permissions) lower than required to **View project code**.
+- The deploy key does not have [read-write permissions for the project](#edit-project-access-permissions-of-a-deploy-key).
+- The deploy key has been [revoked](#revoke-project-access-of-a-deploy-key).
- **No one** is selected in [the **Allowed to push and merge** section](../protected_branches.md#add-protection-to-existing-branches) of the protected branch.
All deploy keys are associated to an account. Since the permissions for an account can change, this might lead to scenarios where a deploy key that was working is suddenly unable to push to a protected branch.
diff --git a/ee/app/models/concerns/ee/protected_ref_access.rb b/ee/app/models/concerns/ee/protected_ref_access.rb
index 4a6a406c8679deb3dd3015d8dd73ad6b3efad0ac..9f2dc86dfb7d81d8aafd3cf1ab5a1b187f78bd9b 100644
--- a/ee/app/models/concerns/ee/protected_ref_access.rb
+++ b/ee/app/models/concerns/ee/protected_ref_access.rb
@@ -19,11 +19,10 @@ module Scopes
belongs_to :user
belongs_to :group
- protected_type = module_parent.model_name.singular
- validates :group_id, uniqueness: { scope: protected_type, allow_nil: true }
- validates :user_id, uniqueness: { scope: protected_type, allow_nil: true }
- validates :access_level, uniqueness: { scope: protected_type, if: :role?,
- conditions: -> { where(user_id: nil, group_id: nil) } }
+ with_options uniqueness: { scope: "#{module_parent.model_name.singular}_id", allow_nil: true } do
+ validates :group_id
+ validates :user_id
+ end
validates :group, :user, absence: true,
unless: -> { importing? || protected_refs_for_users_required_and_available }
@@ -37,6 +36,12 @@ module Scopes
end
end
+ class_methods do
+ def non_role_types
+ super.concat(%i[user group])
+ end
+ end
+
override :type
def type
return :user if user.present?
diff --git a/spec/models/protected_branch/push_access_level_spec.rb b/spec/models/protected_branch/push_access_level_spec.rb
index e56ff2241b1715c9dab1d6f4c53fb020f4503047..05e10fd6763d4514f6d2face36d52f1f024be512 100644
--- a/spec/models/protected_branch/push_access_level_spec.rb
+++ b/spec/models/protected_branch/push_access_level_spec.rb
@@ -4,81 +4,6 @@
RSpec.describe ProtectedBranch::PushAccessLevel, feature_category: :source_code_management do
include_examples 'protected branch access'
+ include_examples 'protected ref deploy_key access'
include_examples 'protected ref access allowed_access_levels'
-
- describe 'associations' do
- it { is_expected.to belong_to(:deploy_key) }
- end
-
- describe 'validations' do
- it 'is not valid when a record exists with the same access level' do
- protected_branch = create(:protected_branch)
- create(:protected_branch_push_access_level, protected_branch: protected_branch)
- level = build(:protected_branch_push_access_level, protected_branch: protected_branch)
-
- expect(level).to be_invalid
- end
-
- it 'is not valid when a record exists with the same access level' do
- protected_branch = create(:protected_branch)
- deploy_key = create(:deploy_key, projects: [protected_branch.project])
- create(:protected_branch_push_access_level, protected_branch: protected_branch, deploy_key: deploy_key)
- level = build(:protected_branch_push_access_level, protected_branch: protected_branch, deploy_key: deploy_key)
-
- expect(level).to be_invalid
- end
-
- it 'checks that a deploy key is enabled for the same project as the protected branch\'s' do
- level = build(:protected_branch_push_access_level, deploy_key: create(:deploy_key))
-
- expect { level.save! }.to raise_error(ActiveRecord::RecordInvalid)
- expect(level.errors.full_messages).to contain_exactly('Deploy key is not enabled for this project')
- end
- end
-
- describe '#check_access' do
- let_it_be(:project) { create(:project) }
- let_it_be(:protected_branch) { create(:protected_branch, :no_one_can_push, project: project) }
- let_it_be(:user) { create(:user) }
- let_it_be(:deploy_key) { create(:deploy_key, user: user) }
-
- let!(:deploy_keys_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key, can_push: can_push) }
- let(:can_push) { true }
-
- before_all do
- project.add_maintainer(user)
- end
-
- context 'when this push_access_level is tied to a deploy key' do
- let(:push_access_level) { create(:protected_branch_push_access_level, protected_branch: protected_branch, deploy_key: deploy_key) }
-
- context 'when the deploy key is among the active keys for this project' do
- specify do
- expect(push_access_level.check_access(user)).to be_truthy
- end
- end
-
- context 'when the deploy key is not among the active keys of this project' do
- let(:can_push) { false }
-
- it 'is false' do
- expect(push_access_level.check_access(user)).to be_falsey
- end
- end
- end
- end
-
- describe '#type' do
- let(:push_level_access) { build(:protected_branch_push_access_level) }
-
- it 'returns :deploy_key when a deploy key is tied to the protected branch' do
- push_level_access.deploy_key = create(:deploy_key)
-
- expect(push_level_access.type).to eq(:deploy_key)
- end
-
- it 'returns :role by default' do
- expect(push_level_access.type).to eq(:role)
- end
- end
end
diff --git a/spec/models/protected_tag/create_access_level_spec.rb b/spec/models/protected_tag/create_access_level_spec.rb
index 8eeccdc9b342e97f3acc62699d6eaa62bc5524a9..d795399060e3da2fbceebb9dd69cc134448db58f 100644
--- a/spec/models/protected_tag/create_access_level_spec.rb
+++ b/spec/models/protected_tag/create_access_level_spec.rb
@@ -4,134 +4,6 @@
RSpec.describe ProtectedTag::CreateAccessLevel, feature_category: :source_code_management do
include_examples 'protected tag access'
+ include_examples 'protected ref deploy_key access'
include_examples 'protected ref access allowed_access_levels'
-
- describe 'associations' do
- it { is_expected.to belong_to(:deploy_key) }
- end
-
- describe 'validations', :aggregate_failures do
- let_it_be(:protected_tag) { create(:protected_tag) }
-
- context 'when deploy key enabled for the project' do
- let(:deploy_key) { create(:deploy_key, projects: [protected_tag.project]) }
-
- it 'is valid' do
- level = build(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: deploy_key)
-
- expect(level).to be_valid
- end
- end
-
- context 'when a record exists with the same access level' do
- before do
- create(:protected_tag_create_access_level, protected_tag: protected_tag)
- end
-
- it 'is not valid' do
- level = build(:protected_tag_create_access_level, protected_tag: protected_tag)
-
- expect(level).to be_invalid
- expect(level.errors.full_messages).to include('Access level has already been taken')
- end
- end
-
- context 'when a deploy key already added for this access level' do
- let!(:create_access_level) do
- create(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: deploy_key)
- end
-
- let(:deploy_key) { create(:deploy_key, projects: [protected_tag.project]) }
-
- it 'is not valid' do
- level = build(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: deploy_key)
-
- expect(level).to be_invalid
- expect(level.errors.full_messages).to contain_exactly('Deploy key has already been taken')
- end
- end
-
- context 'when deploy key is not enabled for the project' do
- let(:create_access_level) do
- build(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: create(:deploy_key))
- end
-
- it 'returns an error' do
- expect(create_access_level).to be_invalid
- expect(create_access_level.errors.full_messages).to contain_exactly(
- 'Deploy key is not enabled for this project'
- )
- end
- end
- end
-
- describe '#check_access' do
- let_it_be(:project) { create(:project) }
- let_it_be(:protected_tag) { create(:protected_tag, :no_one_can_create, project: project) }
- let_it_be(:user) { create(:user) }
- let_it_be(:deploy_key) { create(:deploy_key, user: user) }
-
- let!(:deploy_keys_project) do
- create(:deploy_keys_project, project: project, deploy_key: deploy_key, can_push: can_push)
- end
-
- let(:create_access_level) { protected_tag.create_access_levels.first }
- let(:can_push) { true }
-
- before_all do
- project.add_maintainer(user)
- end
-
- it { expect(create_access_level.check_access(user)).to be_falsey }
-
- context 'when this create_access_level is tied to a deploy key' do
- let(:create_access_level) do
- create(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: deploy_key)
- end
-
- context 'when the deploy key is among the active keys for this project' do
- it { expect(create_access_level.check_access(user)).to be_truthy }
- end
-
- context 'when user is missing' do
- it { expect(create_access_level.check_access(nil)).to be_falsey }
- end
-
- context 'when deploy key does not belong to the user' do
- let(:another_user) { create(:user) }
-
- it { expect(create_access_level.check_access(another_user)).to be_falsey }
- end
-
- context 'when user cannot access the project' do
- before do
- allow(user).to receive(:can?).with(:read_project, project).and_return(false)
- end
-
- it { expect(create_access_level.check_access(user)).to be_falsey }
- end
-
- context 'when the deploy key is not among the active keys of this project' do
- let(:can_push) { false }
-
- it { expect(create_access_level.check_access(user)).to be_falsey }
- end
- end
- end
-
- describe '#type' do
- let(:create_access_level) { build(:protected_tag_create_access_level) }
-
- it 'returns :role by default' do
- expect(create_access_level.type).to eq(:role)
- end
-
- context 'when a deploy key is tied to the protected branch' do
- let(:create_access_level) { build(:protected_tag_create_access_level, deploy_key: build(:deploy_key)) }
-
- it 'returns :deploy_key' do
- expect(create_access_level.type).to eq(:deploy_key)
- end
- end
- end
end
diff --git a/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb b/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb
index 4753d7a4556ebcc4afcd4c4ed5ef64bae62a124b..0e9200f1fd9346218b29aca712dbbcc881b2c91c 100644
--- a/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb
+++ b/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb
@@ -6,16 +6,34 @@
let_it_be(:project) { create(:project) }
let_it_be(:protected_ref) { create(association, project: project) } # rubocop:disable Rails/SaveBang
- it { is_expected.to validate_inclusion_of(:access_level).in_array(described_class.allowed_access_levels) }
+ describe 'validations' do
+ subject { build(described_class.model_name.singular) }
- it { is_expected.to validate_presence_of(:access_level) }
+ context 'when role?' do
+ it { is_expected.to validate_inclusion_of(:access_level).in_array(described_class.allowed_access_levels) }
- context 'when not role?' do
- before do
- allow(subject).to receive(:role?).and_return(false)
+ it { is_expected.to validate_presence_of(:access_level) }
+
+ it do
+ is_expected.to validate_uniqueness_of(:access_level)
+ .scoped_to("#{described_class.module_parent.model_name.singular}_id")
+ end
end
- it { is_expected.not_to validate_presence_of(:access_level) }
+ context 'when not role?' do
+ before do
+ allow(subject).to receive(:role?).and_return(false)
+ end
+
+ it { is_expected.not_to validate_presence_of(:access_level) }
+
+ it { is_expected.not_to validate_inclusion_of(:access_level).in_array(described_class.allowed_access_levels) }
+
+ it do
+ is_expected.not_to validate_uniqueness_of(:access_level)
+ .scoped_to("#{described_class.module_parent.model_name.singular}_id")
+ end
+ end
end
describe '::human_access_levels' do
diff --git a/spec/support/shared_examples/models/concerns/protected_ref_deploy_key_access_examples.rb b/spec/support/shared_examples/models/concerns/protected_ref_deploy_key_access_examples.rb
new file mode 100644
index 0000000000000000000000000000000000000000..f2e79dc377b63ad4ce3b9d2f66a79d27ac25fba4
--- /dev/null
+++ b/spec/support/shared_examples/models/concerns/protected_ref_deploy_key_access_examples.rb
@@ -0,0 +1,132 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'protected ref deploy_key access' do
+ let_it_be(:described_instance) { described_class.model_name.singular }
+ let_it_be(:protected_ref_name) { described_class.module_parent.model_name.singular }
+ let_it_be(:project) { create(:project) }
+ let_it_be(:protected_ref) { create(protected_ref_name, project: project) } # rubocop:disable Rails/SaveBang
+
+ describe 'associations' do
+ it { is_expected.to belong_to(:deploy_key) }
+ end
+
+ describe 'validations' do
+ context 'when deploy_key?' do
+ context 'when deploy key enabled for the project' do
+ let(:deploy_key) do
+ create(:deploy_keys_project, :write_access, project: project).deploy_key
+ end
+
+ it 'is valid' do
+ level = build(described_instance, protected_ref_name => protected_ref, deploy_key: deploy_key)
+
+ expect(level).to be_valid
+ end
+ end
+
+ context 'when a deploy key already added for this access level' do
+ let(:deploy_key) { create(:deploy_keys_project, :write_access, project: project).deploy_key }
+
+ before do
+ create(described_instance, protected_ref_name => protected_ref, deploy_key: deploy_key)
+ end
+
+ subject(:access_level) do
+ build(described_instance, protected_ref_name => protected_ref, deploy_key: deploy_key)
+ end
+
+ it 'is not valid', :aggregate_failures do
+ is_expected.to be_invalid
+ expect(access_level.errors.full_messages).to contain_exactly('Deploy key has already been taken')
+ end
+ end
+
+ context 'when deploy key is not enabled for the project' do
+ subject(:access_level) do
+ build(described_instance, protected_ref_name => protected_ref, deploy_key: create(:deploy_key))
+ end
+
+ it 'is not valid', :aggregate_failures do
+ is_expected.to be_invalid
+ expect(access_level.errors.full_messages).to contain_exactly('Deploy key is not enabled for this project')
+ end
+ end
+
+ context 'when deploy key is not active for the project' do
+ subject(:access_level) do
+ deploy_key = create(:deploy_keys_project, :readonly_access, project: project).deploy_key
+ build(described_instance, protected_ref_name => protected_ref, deploy_key: deploy_key)
+ end
+
+ it 'is not valid', :aggregate_failures do
+ is_expected.to be_invalid
+ expect(access_level.errors.full_messages).to contain_exactly('Deploy key is not enabled for this project')
+ end
+ end
+ end
+ end
+
+ describe '#check_access' do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:deploy_key) { create(:deploy_key, user: user) }
+ let_it_be(:deploy_keys_project) do
+ create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key)
+ end
+
+ before_all do
+ project.add_maintainer(user)
+ end
+
+ context "when this #{described_class.model_name.singular} is tied to a deploy key" do
+ let!(:access_level) do
+ create(described_instance, protected_ref_name => protected_ref, deploy_key: deploy_key)
+ end
+
+ context 'when the deploy key is among the active keys for this project' do
+ it { expect(access_level.check_access(user)).to be_truthy }
+ end
+
+ context 'when user is missing' do
+ it { expect(access_level.check_access(nil)).to be_falsey }
+ end
+
+ context 'when deploy key does not belong to the user' do
+ let(:another_user) { create(:user) }
+
+ it { expect(access_level.check_access(another_user)).to be_falsey }
+ end
+
+ context 'when user cannot access the project' do
+ before do
+ allow(user).to receive(:can?).with(:read_project, project).and_return(false)
+ end
+
+ it { expect(access_level.check_access(user)).to be_falsey }
+ end
+
+ context 'when the deploy key is not among the active keys of this project' do
+ before do
+ deploy_keys_project.update!(can_push: false)
+ end
+
+ after do
+ deploy_keys_project.update!(can_push: true)
+ end
+
+ it { expect(access_level.check_access(user)).to be_falsey }
+ end
+ end
+ end
+
+ describe '#type' do
+ let(:access_level) { build(described_instance) }
+
+ context 'when deploy_key?' do
+ let(:access_level) { build(described_instance, deploy_key: build(:deploy_key)) }
+
+ it 'returns :deploy_key' do
+ expect(access_level.type).to eq(:deploy_key)
+ end
+ end
+ end
+end