Commit b58fc1e0 authored by Etienne Baqué's avatar Etienne Baqué

Added Deploy Keys option in protected branches

Updated AccessDropdown component.
Updated Protected Branches view.
Added backend to add deploy keys to protected branches
Added rspecs.
parent bb9ef246
...@@ -48,11 +48,12 @@ export default class AccessDropdown { ...@@ -48,11 +48,12 @@ export default class AccessDropdown {
clicked: options => { clicked: options => {
const { $el, e } = options; const { $el, e } = options;
const item = options.selectedObj; const item = options.selectedObj;
const fossWithMergeAccess = !this.hasLicense && this.accessLevel === ACCESS_LEVELS.MERGE;
e.preventDefault(); e.preventDefault();
if (!this.hasLicense) { if (fossWithMergeAccess) {
// We're not multiselecting quite yet with FOSS: // We're not multiselecting quite yet in "Merge" access dropdown, on FOSS:
// remove all preselected items before selecting this item // remove all preselected items before selecting this item
// https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37499 // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37499
this.accessLevelsData.forEach(level => { this.accessLevelsData.forEach(level => {
...@@ -62,7 +63,7 @@ export default class AccessDropdown { ...@@ -62,7 +63,7 @@ export default class AccessDropdown {
if ($el.is('.is-active')) { if ($el.is('.is-active')) {
if (this.noOneObj) { if (this.noOneObj) {
if (item.id === this.noOneObj.id && this.hasLicense) { if (item.id === this.noOneObj.id && !fossWithMergeAccess) {
// remove all others selected items // remove all others selected items
this.accessLevelsData.forEach(level => { this.accessLevelsData.forEach(level => {
if (level.id !== item.id) { if (level.id !== item.id) {
......
...@@ -12,6 +12,10 @@ module ProtectedRef ...@@ -12,6 +12,10 @@ module ProtectedRef
delegate :matching, :matches?, :wildcard?, to: :ref_matcher delegate :matching, :matches?, :wildcard?, to: :ref_matcher
scope :for_project, ->(project) { where(project: project) } scope :for_project, ->(project) { where(project: project) }
def allow_multiple?(type)
false
end
end end
def commit def commit
...@@ -29,7 +33,7 @@ module ProtectedRef ...@@ -29,7 +33,7 @@ module ProtectedRef
# to fail. # to fail.
has_many :"#{type}_access_levels", inverse_of: self.model_name.singular has_many :"#{type}_access_levels", inverse_of: self.model_name.singular
validates :"#{type}_access_levels", length: { is: 1, message: "are restricted to a single instance per #{self.model_name.human}." } validates :"#{type}_access_levels", length: { is: 1, message: "are restricted to a single instance per #{self.model_name.human}." }, unless: -> { allow_multiple?(type) }
accepts_nested_attributes_for :"#{type}_access_levels", allow_destroy: true accepts_nested_attributes_for :"#{type}_access_levels", allow_destroy: true
end end
......
...@@ -45,6 +45,7 @@ module ProtectedRefAccess ...@@ -45,6 +45,7 @@ module ProtectedRefAccess
end end
def check_access(user) def check_access(user)
return false unless user
return true if user.admin? return true if user.admin?
user.can?(:push_code, project) && user.can?(:push_code, project) &&
......
...@@ -48,6 +48,10 @@ class ProtectedBranch < ApplicationRecord ...@@ -48,6 +48,10 @@ class ProtectedBranch < ApplicationRecord
where(fuzzy_arel_match(:name, query.downcase)) where(fuzzy_arel_match(:name, query.downcase))
end end
def allow_multiple?(type)
type == :push
end
end end
ProtectedBranch.prepend_if_ee('EE::ProtectedBranch') ProtectedBranch.prepend_if_ee('EE::ProtectedBranch')
...@@ -37,8 +37,7 @@ class ProtectedBranch::PushAccessLevel < ApplicationRecord ...@@ -37,8 +37,7 @@ class ProtectedBranch::PushAccessLevel < ApplicationRecord
end end
def enabled_deploy_key_for_user?(deploy_key, user) def enabled_deploy_key_for_user?(deploy_key, user)
return false unless deploy_key.user_id == user.id DeployKey.with_write_access_for_project(protected_branch.project, deploy_key: deploy_key).any? &&
deploy_key.user_id == user.id
DeployKey.with_write_access_for_project(protected_branch.project, deploy_key: deploy_key).any?
end end
end end
- select_mode_for_dropdown = Feature.enabled?(:deploy_keys_on_protected_branches, protected_branch.project) ? 'js-multiselect' : ''
- merge_access_levels = protected_branch.merge_access_levels.for_role - merge_access_levels = protected_branch.merge_access_levels.for_role
- push_access_levels = protected_branch.push_access_levels.for_role - push_access_levels = protected_branch.push_access_levels.for_role
...@@ -23,7 +25,7 @@ ...@@ -23,7 +25,7 @@
%td.push_access_levels-container %td.push_access_levels-container
= hidden_field_tag "allowed_to_push_#{protected_branch.id}", push_access_levels.first&.access_level = hidden_field_tag "allowed_to_push_#{protected_branch.id}", push_access_levels.first&.access_level
= dropdown_tag( (push_access_levels.first&.humanize || 'Select') , = dropdown_tag( (push_access_levels.first&.humanize || 'Select') ,
options: { toggle_class: 'js-allowed-to-push', dropdown_class: 'dropdown-menu-selectable js-allowed-to-push-container capitalize-header', options: { toggle_class: "js-allowed-to-push #{select_mode_for_dropdown}", dropdown_class: 'dropdown-menu-selectable js-allowed-to-push-container capitalize-header',
data: { field_name: "allowed_to_push_#{protected_branch.id}", preselected_items: access_levels_data(push_access_levels) }}) data: { field_name: "allowed_to_push_#{protected_branch.id}", preselected_items: access_levels_data(push_access_levels) }})
- if user_push_access_levels.any? - if user_push_access_levels.any?
%p.small %p.small
......
...@@ -65,6 +65,7 @@ module EE ...@@ -65,6 +65,7 @@ module EE
override :check_access override :check_access
def check_access(user) def check_access(user)
return false unless user
return true if user.admin? return true if user.admin?
return user.id == self.user_id if self.user.present? return user.id == self.user_id if self.user.present?
return group.users.exists?(user.id) if self.group.present? return group.users.exists?(user.id) if self.group.present?
......
...@@ -14,7 +14,7 @@ module Gitlab ...@@ -14,7 +14,7 @@ module Gitlab
private private
def can_push? def can_push?
user_access.can_do_action?(:push_code) || user_access.can_push_to_branch?(ref) ||
project.branch_allows_collaboration?(user_access.user, branch_name) project.branch_allows_collaboration?(user_access.user, branch_name)
end end
end end
......
...@@ -12,7 +12,7 @@ RSpec.describe Gitlab::Checks::PushCheck do ...@@ -12,7 +12,7 @@ RSpec.describe Gitlab::Checks::PushCheck do
context 'when the user is not allowed to push to the repo' do context 'when the user is not allowed to push to the repo' do
it 'raises an error' do it 'raises an error' do
expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) expect(user_access).to receive(:can_push_to_branch?).and_return(false)
expect(project).to receive(:branch_allows_collaboration?).with(user_access.user, 'master').and_return(false) expect(project).to receive(:branch_allows_collaboration?).with(user_access.user, 'master').and_return(false)
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to push code to this project.') expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to push code to this project.')
......
...@@ -196,6 +196,57 @@ RSpec.describe Gitlab::UserAccess do ...@@ -196,6 +196,57 @@ RSpec.describe Gitlab::UserAccess do
expect(access.can_merge_to_branch?(@branch.name)).to be_falsey expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
end end
end end
context 'push to a protected branch of this project via a deploy key' do
let(:protected_branch) { create(:protected_branch, :no_one_can_push, project: project) }
context 'when a user cannot push to a project and branch does not allow collaboration' do
let(:deploy_key) { create(:deploy_key, user: user) }
before do
project.add_guest(user)
allow(project).to receive(:branch_allows_collaboration?).with(user, protected_branch.name).and_return(false)
create(:deploy_keys_project, :write_access, project: protected_branch.project, deploy_key: deploy_key)
create(:protected_branch_push_access_level, protected_branch: protected_branch, deploy_key: deploy_key)
end
context 'when the project has active deploy key owned by this user' do
it 'returns true' do
expect(access.can_push_to_branch?(protected_branch.name)).to be_truthy
end
end
context 'when the project has active deploy keys, but not by this user' do
let(:deploy_key) { create(:deploy_key, user: create(:user)) }
it 'returns false' do
expect(access.can_push_to_branch?(protected_branch.name)).to be_falsey
end
end
context 'when there is another branch' do
let(:another_branch) { create(:protected_branch, :no_one_can_push, project: project) }
before do
allow(project).to receive(:branch_allows_collaboration?).with(user, another_branch.name).and_return(false)
end
it 'returns false when trying to push to that other branch' do
expect(access.can_push_to_branch?(another_branch.name)).to be_falsey
end
context 'and the deploy key added for the first protected branch is also added for this other branch' do
it 'returns true for both protected branches' do
create(:protected_branch_push_access_level, protected_branch: another_branch, deploy_key: deploy_key)
expect(access.can_push_to_branch?(protected_branch.name)).to be_truthy
expect(access.can_push_to_branch?(another_branch.name)).to be_truthy
end
end
end
end
end
end end
describe '#can_create_tag?' do describe '#can_create_tag?' do
......
...@@ -75,4 +75,18 @@ RSpec.describe ProtectedBranch::PushAccessLevel do ...@@ -75,4 +75,18 @@ RSpec.describe ProtectedBranch::PushAccessLevel do
end end
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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment