Merge branch '30769-keys-on-protected-branch-be' into 'master'

RUN AS-IF-FOSS Deploy keys for protected branches - BE

See merge request gitlab-org/gitlab!36345

app/assets/javascripts/projects/settings/access_dropdown.js

@@ -48,11 +48,12 @@ export default class AccessDropdown {
       clicked: options => {
         const { $el, e } = options;
         const item = options.selectedObj;
+        const fossWithMergeAccess = !this.hasLicense && this.accessLevel === ACCESS_LEVELS.MERGE;
 
         e.preventDefault();
 
-        if (!this.hasLicense) {
-          // We're not multiselecting quite yet with FOSS:
+        if (fossWithMergeAccess) {
+          // We're not multiselecting quite yet in "Merge" access dropdown, on FOSS:
           // remove all preselected items before selecting this item
           // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37499
           this.accessLevelsData.forEach(level => {
@@ -62,7 +63,7 @@ export default class AccessDropdown {
 
         if ($el.is('.is-active')) {
           if (this.noOneObj) {
-            if (item.id === this.noOneObj.id && this.hasLicense) {
+            if (item.id === this.noOneObj.id && !fossWithMergeAccess) {
               // remove all others selected items
               this.accessLevelsData.forEach(level => {
                 if (level.id !== item.id) {

app/models/concerns/protected_ref.rb

@@ -12,6 +12,10 @@ module ProtectedRef
     delegate :matching, :matches?, :wildcard?, to: :ref_matcher
 
     scope :for_project, ->(project) { where(project: project) }
+
+    def allow_multiple?(type)
+      false
+    end
   end
 
   def commit
@@ -29,7 +33,7 @@ module ProtectedRef
         # to fail.
         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
       end

app/models/concerns/protected_ref_access.rb

@@ -45,6 +45,7 @@ module ProtectedRefAccess
   end
 
   def check_access(user)
+    return false unless user
     return true if user.admin?
 
     user.can?(:push_code, project) &&

app/models/protected_branch.rb

@@ -48,6 +48,10 @@ class ProtectedBranch < ApplicationRecord
 
     where(fuzzy_arel_match(:name, query.downcase))
   end
+
+  def allow_multiple?(type)
+    type == :push
+  end
 end
 
 ProtectedBranch.prepend_if_ee('EE::ProtectedBranch')

app/models/protected_branch/push_access_level.rb

@@ -37,8 +37,6 @@ class ProtectedBranch::PushAccessLevel < ApplicationRecord
   end
 
   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

app/views/shared/projects/protected_branches/_update_protected_branch.html.haml

+- 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
 - push_access_levels = protected_branch.push_access_levels.for_role
 
@@ -23,7 +25,7 @@
 %td.push_access_levels-container
   = hidden_field_tag "allowed_to_push_#{protected_branch.id}", push_access_levels.first&.access_level
   = 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) }})
   - if user_push_access_levels.any?
     %p.small

ee/app/models/concerns/ee/protected_ref_access.rb

@@ -65,6 +65,7 @@ module EE
 
     override :check_access
     def check_access(user)
+      return false unless user
       return true if user.admin?
       return user.id == self.user_id if self.user.present?
       return group.users.exists?(user.id) if self.group.present?

lib/gitlab/checks/push_check.rb

@@ -14,7 +14,7 @@ module Gitlab
       private
 
       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)
       end
     end

lib/gitlab/git_access.rb

@@ -319,11 +319,11 @@ module Gitlab
     end
 
     def check_change_access!
-      # Deploy keys with write access can push anything
-      return if deploy_key?
+      return if deploy_key? && !deploy_keys_on_protected_branches_enabled?
 
       if changes == ANY
-        can_push = user_can_push? ||
+        can_push = (deploy_key? && deploy_keys_on_protected_branches_enabled?) ||
+                   user_can_push? ||
           project&.any_branch_allows_collaboration?(user_access.user)
 
         unless can_push
@@ -449,6 +449,8 @@ module Gitlab
                          CiAccess.new
                        elsif user && request_from_ci_build?
                          BuildAccess.new(user, container: container)
+                       elsif deploy_key? && deploy_keys_on_protected_branches_enabled?
+                         DeployKeyAccess.new(deploy_key, container: container)
                        else
                          UserAccess.new(user, container: container)
                        end
@@ -526,6 +528,10 @@ module Gitlab
     def size_checker
       container.repository_size_checker
     end
+
+    def deploy_keys_on_protected_branches_enabled?
+      Feature.enabled?(:deploy_keys_on_protected_branches, project)
+    end
   end
 end
 

spec/lib/gitlab/checks/push_check_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Gitlab::Checks::PushCheck do
 
     context 'when the user is not allowed to push to the repo' 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 { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You are not allowed to push code to this project.')

spec/models/protected_branch/push_access_level_spec.rb

@@ -75,4 +75,18 @@ RSpec.describe ProtectedBranch::PushAccessLevel do
       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