Merge branch 'security-remove-deploy-access-levels-on-project-group-link-deletion' into 'master'

Remove deploy access level when project/group link is deleted

See merge request gitlab/gitlab-ee!1329

changelogs/unreleased/security-remove-deploy-access-levels-on-project-group-link-deletion.yml

+---
+title: Remove deploy access level when project/group link is deleted
+merge_request:
+author:
+type: security

ee/app/models/ee/project_group_link.rb

@@ -5,14 +5,18 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
-      before_destroy :delete_branch_protection
+      before_destroy :delete_related_access_levels
     end
 
-    def delete_branch_protection
-      if group.present? && project.present?
-        project.protected_branches.merge_access_by_group(group).destroy_all # rubocop: disable DestroyAll
-        project.protected_branches.push_access_by_group(group).destroy_all # rubocop: disable DestroyAll
-      end
+    def delete_related_access_levels
+      return unless group.present? && project.present?
+
+      # For protected branches
+      project.protected_branches.merge_access_by_group(group).destroy_all # rubocop: disable DestroyAll
+      project.protected_branches.push_access_by_group(group).destroy_all # rubocop: disable DestroyAll
+
+      # For protected environments
+      project.protected_environments.deploy_access_levels_by_group(group).delete_all
     end
   end
 end

ee/app/models/protected_environment.rb

@@ -19,6 +19,11 @@ class ProtectedEnvironment < ApplicationRecord
              ' AND protected_environments.project_id = environments.project_id')
   end
 
+  scope :deploy_access_levels_by_group, -> (group) do
+    ProtectedEnvironment::DeployAccessLevel
+      .joins(:protected_environment).where(group: group)
+  end
+
   def accessible_to?(user)
     deploy_access_levels
       .any? { |deploy_access_level| deploy_access_level.check_access(user) }

ee/spec/models/ee/project_group_link_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectGroupLink do
+  describe '#destroy' do
+    let(:project) { create(:project) }
+    let(:group) { create(:group) }
+    let(:user) { create(:user) }
+    let!(:project_group_link) { create(:project_group_link, project: project, group: group) }
+
+    it 'removes related protected environment deploy access levels' do
+      params = attributes_for(:protected_environment,
+                              deploy_access_levels_attributes: [{ group_id: group.id }, { user_id: user.id }])
+
+      protected_environment = ProtectedEnvironments::CreateService.new(project, user, params).execute
+
+      expect { project_group_link.destroy! }.to change(ProtectedEnvironment::DeployAccessLevel, :count).by(-1)
+
+      expect(protected_environment.deploy_access_levels.find_by_group_id(group)).to be_nil
+      expect(protected_environment.deploy_access_levels.find_by_user_id(user)).to be_persisted
+    end
+  end
+end

ee/spec/models/protected_environment_spec.rb

@@ -116,6 +116,21 @@ describe ProtectedEnvironment do
     end
   end
 
+  describe '.deploy_access_levels_by_group' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project) }
+    let(:environment) { create(:environment, project: project, name: 'production') }
+    let(:protected_environment) { create(:protected_environment, project: project, name: 'production') }
+
+    it 'returns matching deploy access levels for the given group' do
+      _deploy_access_level_for_different_group = create_deploy_access_level(group: create(:group))
+      _deploy_access_level_for_user = create_deploy_access_level(user: create(:user))
+      deploy_access_level = create_deploy_access_level(group: group)
+
+      expect(described_class.deploy_access_levels_by_group(group)).to contain_exactly(deploy_access_level)
+    end
+  end
+
   def create_deploy_access_level(**opts)
     protected_environment.deploy_access_levels.create(**opts)
   end