Merge branch...

Merge branch '225482-allow-reporters-to-run-jobs-that-deploy-to-an-environments-if-they-are-explicitly-listed' into 'master'

Deployer authorisation for protected environments

Closes #225482

See merge request gitlab-org/gitlab!38188

app/policies/ci/build_policy.rb

@@ -20,6 +20,11 @@ module Ci
       end
     end
 
+    # overridden in EE
+    condition(:protected_environment_access) do
+      false
+    end
+
     condition(:owner_of_job) do
       @subject.triggered_by?(@user)
     end
@@ -40,7 +45,7 @@ module Ci
       @subject.pipeline.webide?
     end
 
-    rule { protected_ref | archived }.policy do
+    rule { ~protected_environment_access & (protected_ref | archived) }.policy do
       prevent :update_build
       prevent :update_commit_status
       prevent :erase_build

ee/app/models/protected_environment/deploy_access_level.rb

@@ -3,6 +3,7 @@ class ProtectedEnvironment::DeployAccessLevel < ApplicationRecord
   ALLOWED_ACCESS_LEVELS = [
     Gitlab::Access::MAINTAINER,
     Gitlab::Access::DEVELOPER,
+    Gitlab::Access::REPORTER,
     Gitlab::Access::ADMIN
   ].freeze
 
@@ -15,9 +16,7 @@ class ProtectedEnvironment::DeployAccessLevel < ApplicationRecord
   belongs_to :group
   belongs_to :protected_environment
 
-  validates :access_level, presence: true, if: :role?, inclusion: {
-    in: ALLOWED_ACCESS_LEVELS
-  }
+  validates :access_level, presence: true, inclusion: { in: ALLOWED_ACCESS_LEVELS }
 
   delegate :project, to: :protected_environment
 

ee/app/policies/ee/ci/build_policy.rb

@@ -7,10 +7,28 @@ module EE
       prepended do
         condition(:deployable_by_user) { deployable_by_user? }
 
-        rule { ~deployable_by_user }.policy do
+        condition(:protected_environment_access) do
+          project = @subject.project
+          environment = @subject.environment
+
+          if environment && project.protected_environments_feature_available?
+            protected_environment = project.protected_environment_by_name(environment)
+
+            !!protected_environment&.accessible_to?(user)
+          else
+            false
+          end
+        end
+
+        rule { ~deployable_by_user & ~protected_environment_access}.policy do
           prevent :update_build
         end
 
+        rule { protected_environment_access }.policy do
+          enable :update_commit_status
+          enable :update_build
+        end
+
         private
 
         alias_method :current_user, :user

ee/changelogs/unreleased/225482-allow-reporters-to-run-jobs-that-deploy-to-an-environments-if-t.yml

+---
+title: Deployer authorisation for protected environments
+merge_request: 38188
+author:
+type: added

ee/spec/models/protected_environment/deploy_access_level_spec.rb

@@ -2,6 +2,10 @@
 require 'spec_helper'
 
 RSpec.describe ProtectedEnvironment::DeployAccessLevel do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:protected_environment) { create(:protected_environment, project: project) }
+  let_it_be(:user) { create(:user) }
+
   describe 'associations' do
     it { is_expected.to belong_to(:protected_environment) }
     it { is_expected.to belong_to(:user) }
@@ -10,13 +14,10 @@ RSpec.describe ProtectedEnvironment::DeployAccessLevel do
 
   describe 'validations' do
     it { is_expected.to validate_presence_of(:access_level) }
+    it { is_expected.to validate_inclusion_of(:access_level).in_array([Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER]) }
   end
 
   describe '#check_access' do
-    let(:project) { create(:project) }
-    let(:protected_environment) { create(:protected_environment, project: project) }
-    let(:user) { create(:user) }
-
     subject { deploy_access_level.check_access(user) }
 
     describe 'admin access' do
@@ -74,6 +75,7 @@ RSpec.describe ProtectedEnvironment::DeployAccessLevel do
     end
 
     describe 'access level' do
+      context 'with a permitted access level' do
         let(:developer_access) { Gitlab::Access::DEVELOPER }
         let(:deploy_access_level) { create(:protected_environment_deploy_access_level, protected_environment: protected_environment, access_level: developer_access) }
 
@@ -93,6 +95,19 @@ RSpec.describe ProtectedEnvironment::DeployAccessLevel do
           it { is_expected.to be_falsy }
         end
       end
+
+      context 'when the access level is not permitted' do
+        let(:deploy_access_level) { create(:protected_environment_deploy_access_level, protected_environment: protected_environment, access_level: Gitlab::Access::GUEST) }
+
+        before do
+          project.add_guest(user)
+        end
+
+        it 'does not save the record' do
+          expect { deploy_access_level }.to raise_error ActiveRecord::RecordInvalid
+        end
+      end
+    end
   end
 
   describe '#humanize' do

ee/spec/policies/ci/build_policy_spec.rb

@@ -14,6 +14,6 @@ RSpec.describe Ci::BuildPolicy do
 
     subject { user.can?(:update_build, build) }
 
-    it_behaves_like 'protected environments access'
+    it_behaves_like 'protected environments access', direct_access: true
   end
 end

ee/spec/policies/environment_policy_spec.rb

@@ -31,6 +31,6 @@ RSpec.describe EnvironmentPolicy do
   describe '#create_environment_terminal' do
     subject { user.can?(:create_environment_terminal, environment) }
 
-    it_behaves_like 'protected environments access', false
+    it_behaves_like 'protected environments access', developer_access: false
   end
 end

ee/spec/serializers/ee/job_entity_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe JobEntity do
 
     subject { entity.as_json[:playable] }
 
-    it_behaves_like 'protected environments access'
+    it_behaves_like 'protected environments access', direct_access: true
   end
 
   describe '#retryable?' do
@@ -27,6 +27,6 @@ RSpec.describe JobEntity do
 
     subject { entity.as_json.include?(:retry_path) }
 
-    it_behaves_like 'protected environments access'
+    it_behaves_like 'protected environments access', direct_access: true
   end
 end

ee/spec/serializers/environment_entity_spec.rb

@@ -66,7 +66,7 @@ RSpec.describe EnvironmentEntity do
 
         subject { entity.as_json.include?(:terminal_path) }
 
-        it_behaves_like 'protected environments access', false
+        it_behaves_like 'protected environments access', developer_access: false
       end
     end
   end

ee/spec/support/shared_examples/policies/protected_environments_shared_examples.rb

 # frozen_string_literal: true
 
-RSpec.shared_examples 'protected environments access' do |developer_access = true|
+RSpec.shared_examples 'protected environments access' do |developer_access: true, direct_access: false|
   using RSpec::Parameterized::TableSyntax
 
   include AdminModeHelper
@@ -13,20 +13,19 @@ RSpec.shared_examples 'protected environments access' do |developer_access = tru
   context 'when Protected Environments feature is not available in the project' do
     let(:feature_available) { false }
 
-    where(:access_level, :admin_mode, :result) do
-      :guest      | nil   | false
-      :reporter   | nil   | false
-      :developer  | nil   | developer_access
-      :maintainer | nil   | true
-      :admin      | false | false
-      :admin      | true  | true
+    where(:access_level, :result) do
+      :guest      | false
+      :reporter   | false
+      :developer  | developer_access
+      :maintainer | true
+      :admin      | true
     end
 
     with_them do
       before do
         environment
 
-        update_user_access(access_level, admin_mode, user, project)
+        update_user_access(access_level, user, project)
       end
 
       it { is_expected.to eq(result) }
@@ -40,20 +39,19 @@ RSpec.shared_examples 'protected environments access' do |developer_access = tru
       let(:protected_environment) { create(:protected_environment, name: environment.name, project: project) }
 
       context 'when user does not have access to the environment' do
-        where(:access_level, :admin_mode, :result) do
-          :guest      | nil   | false
-          :reporter   | nil   | false
-          :developer  | nil   | false
-          :maintainer | nil   | false
-          :admin      | false | false
-          :admin      | true  | true
+        where(:access_level, :result) do
+          :guest      | false
+          :reporter   | false
+          :developer  | false
+          :maintainer | false
+          :admin      | true
         end
 
         with_them do
           before do
             protected_environment
 
-            update_user_access(access_level, admin_mode, user, project)
+            update_user_access(access_level, user, project)
           end
 
           it { is_expected.to eq(result) }
@@ -61,40 +59,49 @@ RSpec.shared_examples 'protected environments access' do |developer_access = tru
       end
 
       context 'when user has access to the environment' do
-        where(:access_level, :admin_mode, :result) do
-          :guest      | nil   | false
-          :reporter   | nil   | false
-          :developer  | nil   | developer_access
-          :maintainer | nil   | true
-          :admin      | false | false
-          :admin      | true  | true
+        where(:access_level, :result) do
+          :reporter   | direct_access
+          :developer  | developer_access
+          :maintainer | true
+          :admin      | true
         end
 
         with_them do
           before do
-            protected_environment.deploy_access_levels.create(user: user)
+            protected_environment.deploy_access_levels.create!(user: user, access_level: deploy_access_level(access_level))
 
-            update_user_access(access_level, admin_mode, user, project)
+            update_user_access(access_level, user, project)
           end
-
           it { is_expected.to eq(result) }
         end
       end
+
+      context 'when the user has access via a group' do
+        let(:group) { create(:group) }
+
+        before do
+          project.add_reporter(user)
+          group.add_reporter(user)
+
+          protected_environment.deploy_access_levels.create!(group: group, access_level: Gitlab::Access::REPORTER)
+        end
+
+        it { is_expected.to eq(direct_access) }
+      end
     end
 
     context 'when environment is not protected' do
-      where(:access_level, :admin_mode, :result) do
-        :guest      | nil   | false
-        :reporter   | nil   | false
-        :developer  | nil   | developer_access
-        :maintainer | nil   | true
-        :admin      | false | false
-        :admin      | true  | true
+      where(:access_level, :result) do
+        :guest      | false
+        :reporter   | false
+        :developer  | developer_access
+        :maintainer | true
+        :admin      | true
       end
 
       with_them do
         before do
-          update_user_access(access_level, admin_mode, user, project)
+          update_user_access(access_level, user, project)
         end
 
         it { is_expected.to eq(result) }
@@ -102,12 +109,27 @@ RSpec.shared_examples 'protected environments access' do |developer_access = tru
     end
   end
 
-  def update_user_access(access_level, admin_mode, user, project)
+  def update_user_access(access_level, user, project)
     if access_level == :admin
       user.update_attribute(:admin, true)
-      enable_admin_mode!(user) if admin_mode
+      enable_admin_mode!(user)
     elsif access_level.present?
       project.add_user(user, access_level)
     end
   end
+
+  def deploy_access_level(access_level)
+    case access_level
+    when :guest
+      Gitlab::Access::GUEST
+    when :reporter
+      Gitlab::Access::REPORTER
+    when :developer
+      Gitlab::Access::DEVELOPER
+    when :maintainer
+      Gitlab::Access::MAINTAINER
+    when :admin
+      Gitlab::Access::MAINTAINER
+    end
+  end
 end