Merge branch...

Merge branch '347069-update-graphql-mutation-to-unassign-security-policy-project-to-group' into 'master'

Update mutation to unassign security policy from a group

See merge request gitlab-org/gitlab!83190

doc/api/graphql/reference/index.md

@@ -4380,7 +4380,7 @@ Input type: `SecurityPolicyProjectCreateInput`
 
 ### `Mutation.securityPolicyProjectUnassign`
 
-Unassigns the security policy project for the given project(`project_path`).
+Unassigns the security policy project for the given project (`full_path`).
 
 Input type: `SecurityPolicyProjectUnassignInput`
 
@@ -4389,7 +4389,8 @@ Input type: `SecurityPolicyProjectUnassignInput`
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | <a id="mutationsecuritypolicyprojectunassignclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationsecuritypolicyprojectunassignprojectpath"></a>`projectPath` | [`ID!`](#id) | Full path of the project. |
+| <a id="mutationsecuritypolicyprojectunassignfullpath"></a>`fullPath` | [`String`](#string) | Full path of the project. |
+| <a id="mutationsecuritypolicyprojectunassignprojectpath"></a>`projectPath` **{warning-solid}** | [`ID`](#id) | **Deprecated:** Use `fullPath`. Deprecated in 14.10. |
 
 #### Fields
 

ee/app/graphql/mutations/security_policy/unassign_security_policy_project.rb

@@ -4,20 +4,25 @@ module Mutations
   module SecurityPolicy
     class UnassignSecurityPolicyProject < BaseMutation
       graphql_name 'SecurityPolicyProjectUnassign'
-      description 'Unassigns the security policy project for the given project(`project_path`).'
+      description 'Unassigns the security policy project for the given project (`full_path`).'
 
-      include FindsProject
+      include FindsProjectOrGroupForSecurityPolicies
 
       authorize :update_security_orchestration_policy_project
 
+      argument :full_path, GraphQL::Types::String,
+               required: false,
+               description: 'Full path of the project.'
+
       argument :project_path, GraphQL::Types::ID,
-               required: true,
+               required: false,
+               deprecated: { reason: 'Use `fullPath`', milestone: '14.10' },
                description: 'Full path of the project.'
 
       def resolve(args)
-        project = authorized_find!(args[:project_path])
+        project_or_group = authorized_find!(**args)
 
-        result = unassign_project(project)
+        result = unassign(project_or_group)
         {
           errors: result.success? ? [] : [result.message]
         }
@@ -25,9 +30,9 @@ module Mutations
 
       private
 
-      def unassign_project(project)
+      def unassign(project_or_group)
         ::Security::Orchestration::UnassignService
-          .new(project, current_user)
+          .new(container: project_or_group, current_user: current_user)
           .execute
       end
     end

ee/app/services/security/orchestration/unassign_service.rb

@@ -2,19 +2,19 @@
 
 module Security
   module Orchestration
-    class UnassignService < ::BaseService
+    class UnassignService < ::BaseContainerService
       def execute
         return error(_('Policy project doesn\'t exist')) unless security_orchestration_policy_configuration
 
         result = security_orchestration_policy_configuration.delete
         return success if result
 
-        error(project.security_orchestration_policy_configuration.errors.full_messages.to_sentence)
+        error(container.security_orchestration_policy_configuration.errors.full_messages.to_sentence)
       end
 
       private
 
-      delegate :security_orchestration_policy_configuration, to: :project
+      delegate :security_orchestration_policy_configuration, to: :container
 
       def success
         ServiceResponse.success

ee/spec/factories/namespaces.rb

@@ -41,6 +41,10 @@ FactoryBot.modify do
         create(:ci_namespace_monthly_usage, namespace: namespace, amount_used: 1000)
       end
     end
+
+    trait :with_security_orchestration_policy_configuration do
+      association :security_orchestration_policy_configuration, factory: [:security_orchestration_policy_configuration, :namespace]
+    end
   end
 end
 

ee/spec/graphql/mutations/security_policy/unassign_security_policy_project_spec.rb

@@ -8,45 +8,59 @@ RSpec.describe Mutations::SecurityPolicy::UnassignSecurityPolicyProject do
   describe '#resolve' do
     let_it_be(:owner) { create(:user) }
     let_it_be(:user) { create(:user) }
+    let_it_be(:namespace) { create(:group, :with_security_orchestration_policy_configuration) }
+    let_it_be(:namespace_without_policy_project) { create(:group) }
     let_it_be(:project) { create(:project, :with_security_orchestration_policy_configuration, namespace: owner.namespace) }
     let_it_be(:project_without_policy_project) { create(:project, namespace: owner.namespace) }
 
-    let(:project_full_path) { project.full_path }
+    let(:container_full_path) { container.full_path }
     let(:current_user) { owner }
 
-    subject { mutation.resolve(project_path: project_full_path) }
+    subject { mutation.resolve(full_path: container_full_path) }
 
-    context 'when permission is set for user' do
-      before do
-        stub_licensed_features(security_orchestration_policies: true)
-      end
+    shared_examples 'unassigns security policy project' do
+      context 'when permission is set for user' do
+        before do
+          stub_licensed_features(security_orchestration_policies: true)
+        end
+
+        context 'when user is an owner of the project' do
+          context 'when policy project is assigned to a container' do
+            it 'unassigns the security policy project' do
+              result = subject
+
+              expect(result[:errors]).to be_empty
+              expect(container.reload.security_orchestration_policy_configuration).to be_blank
+            end
+          end
+
+          context 'when policy project is not assigned to a container' do
+            let(:container_full_path) { container_without_policy_project.full_path }
 
-      context 'when user is an owner of the project' do
-        context 'when policy project is assigned to a project' do
-          it 'assigns the security policy project' do
-            result = subject
+            it 'respond with an error' do
+              result = subject
 
-            expect(result[:errors]).to be_empty
-            expect(project.reload.security_orchestration_policy_configuration).to be_blank
+              expect(result[:errors]).to match_array(["Policy project doesn't exist"])
+            end
           end
         end
 
-        context 'when policy project is not assigned to a project' do
-          let(:project_full_path) { project_without_policy_project.full_path }
+        context 'when user is not an owner' do
+          let(:current_user) { user }
 
-          it 'respond with an error' do
-            result = subject
+          before do
+            container.add_maintainer(user)
+          end
 
-            expect(result[:errors]).to match_array(["Policy project doesn't exist"])
+          it 'raises exception' do
+            expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
           end
         end
       end
 
-      context 'when user is not an owner' do
-        let(:current_user) { user }
-
+      context 'when feature is not licensed' do
         before do
-          project.add_maintainer(user)
+          stub_licensed_features(security_orchestration_policies: false)
         end
 
         it 'raises exception' do
@@ -55,13 +69,51 @@ RSpec.describe Mutations::SecurityPolicy::UnassignSecurityPolicyProject do
       end
     end
 
-    context 'when feature is not licensed' do
+    context 'when both fullPath and projectPath are not provided' do
+      subject { mutation.resolve({}) }
+
       before do
-        stub_licensed_features(security_orchestration_policies: false)
+        stub_licensed_features(security_orchestration_policies: true)
       end
 
       it 'raises exception' do
-        expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
+        expect { subject }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
+      end
+    end
+
+    context 'for project' do
+      let(:container) { project }
+      let(:container_without_policy_project) { project_without_policy_project }
+
+      it_behaves_like 'unassigns security policy project'
+    end
+
+    context 'for namespace' do
+      let(:container) { namespace }
+      let(:container_without_policy_project) { namespace_without_policy_project }
+
+      before do
+        namespace.add_owner(owner)
+        namespace_without_policy_project.add_owner(owner)
+      end
+
+      context 'when feature is enabled' do
+        before do
+          stub_feature_flags(group_level_security_policies: [namespace, namespace_without_policy_project])
+        end
+
+        it_behaves_like 'unassigns security policy project'
+      end
+
+      context 'when feature is disabled' do
+        before do
+          stub_licensed_features(security_orchestration_policies: true)
+          stub_feature_flags(group_level_security_policies: false)
+        end
+
+        it 'raises exception' do
+          expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
+        end
       end
     end
   end

ee/spec/requests/api/graphql/mutations/security_policy/unassign_security_policy_project_spec.rb

@@ -2,19 +2,23 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Unassigns scan execution policy project from a project' do
+RSpec.describe 'Unassigns scan execution policy project from a project/namespace' do
   include GraphqlHelpers
 
   let_it_be_with_refind(:owner) { create(:user) }
   let_it_be_with_refind(:user) { create(:user) }
-  let_it_be_with_refind(:project) { create(:project, namespace: owner.namespace) }
+  let_it_be_with_refind(:project) { create(:project, :with_security_orchestration_policy_configuration, namespace: owner.namespace) }
+  let_it_be_with_refind(:project_without_policy_project) { create(:project, namespace: owner.namespace) }
+  let_it_be_with_refind(:namespace) { create(:group, :with_security_orchestration_policy_configuration) }
+  let_it_be_with_refind(:namespace_without_policy_project) { create(:group) }
 
+  let(:container_full_path) { container.full_path }
   let(:current_user) { owner }
 
   subject { post_graphql_mutation(mutation, current_user: current_user) }
 
   def mutation
-    variables = { project_path: project.full_path }
+    variables = { full_path: container_full_path }
 
     graphql_mutation(:security_policy_project_unassign, variables) do
       <<-QL.strip_heredoc
@@ -27,52 +31,88 @@ RSpec.describe 'Unassigns scan execution policy project from a project' do
     graphql_mutation_response(:security_policy_project_unassign)
   end
 
-  context 'when licensed feature is available' do
-    before do
-      stub_licensed_features(security_orchestration_policies: true)
-    end
+  shared_examples 'unassigns security policy project' do
+    context 'when licensed feature is available' do
+      before do
+        stub_licensed_features(security_orchestration_policies: true)
+      end
 
-    context 'when user is an owner of the project' do
-      context 'when there is no security policy project assigned to the project' do
-        it 'unassigns the security policy project', :aggregate_failures do
-          expect { subject }.not_to change { ::Security::OrchestrationPolicyConfiguration.count }
+      context 'when user is an owner of the container' do
+        context 'when there is no security policy project assigned to the container' do
+          let(:container_full_path) { container_without_policy_project.full_path }
 
-          expect(response).to have_gitlab_http_status(:success)
-          expect(mutation_response['errors']).to eq(["Policy project doesn't exist"])
+          it 'does not unassign the security policy project', :aggregate_failures do
+            expect { subject }.not_to change { ::Security::OrchestrationPolicyConfiguration.count }
+
+            expect(response).to have_gitlab_http_status(:success)
+            expect(mutation_response['errors']).to eq(["Policy project doesn't exist"])
+          end
         end
-      end
 
-      context 'when security policy project is assigned to the project' do
-        let!(:security_policy_management_project) { create(:project, :repository, namespace: current_user.namespace) }
-        let!(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project, security_policy_management_project: security_policy_management_project) }
+        context 'when security policy project is assigned to the container' do
+          it 'unassigns the security policy project', :aggregate_failures do
+            expect { subject }.to change { ::Security::OrchestrationPolicyConfiguration.count }.by(-1)
 
-        it 'unassigns the security policy project', :aggregate_failures do
-          expect { subject }.to change { ::Security::OrchestrationPolicyConfiguration.count }.by(-1)
+            expect(response).to have_gitlab_http_status(:success)
+            expect(mutation_response['errors']).to be_empty
+          end
+        end
+      end
+
+      context 'when user is not an owner' do
+        let(:current_user) { user }
 
-          expect(response).to have_gitlab_http_status(:success)
-          expect(mutation_response['errors']).to be_empty
+        before do
+          project.add_maintainer(user)
         end
+
+        it_behaves_like 'a mutation that returns top-level errors',
+                  errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
       end
     end
 
-    context 'when user is not an owner' do
-      let(:current_user) { user }
-
+    context 'when feature is not licensed' do
       before do
-        project.add_maintainer(user)
+        stub_licensed_features(security_orchestration_policies: false)
       end
 
       it_behaves_like 'a mutation that returns top-level errors',
-                errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
+                  errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
     end
   end
 
-  context 'when feature is not licensed' do
+  context 'for project' do
+    let(:container) { project }
+    let(:container_without_policy_project) { project_without_policy_project }
+
+    it_behaves_like 'unassigns security policy project'
+  end
+
+  context 'for namespace' do
+    let(:container) { namespace }
+    let(:container_without_policy_project) { namespace_without_policy_project }
+
     before do
-      stub_licensed_features(security_orchestration_policies: false)
+      namespace.add_owner(owner)
+      namespace_without_policy_project.add_owner(owner)
+    end
+
+    context 'when feature is enabled' do
+      before do
+        stub_feature_flags(group_level_security_policies: [namespace, namespace_without_policy_project])
+      end
+
+      it_behaves_like 'unassigns security policy project'
     end
 
-    it_behaves_like 'a mutation that returns top-level errors',
-                errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
+    context 'when feature is disabled' do
+      before do
+        stub_licensed_features(security_orchestration_policies: true)
+        stub_feature_flags(group_level_security_policies: false)
+      end
+
+      it_behaves_like 'a mutation that returns top-level errors',
+                  errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
+    end
   end
 end

ee/spec/services/security/orchestration/unassign_service_spec.rb

@@ -3,30 +3,49 @@
 require 'spec_helper'
 
 RSpec.describe Security::Orchestration::UnassignService do
-  let_it_be(:project, reload: true) { create(:project, :with_security_orchestration_policy_configuration) }
-  let_it_be(:project_without_policy_project, reload: true) { create(:project) }
-
-  let(:service) { described_class.new(project, nil) }
-
   describe '#execute' do
     subject(:result) { service.execute }
 
-    context 'when policy project is assigned to a project' do
-      let(:service) { described_class.new(project, nil) }
+    shared_examples 'unassigns policy project' do
+      context 'when policy project is assigned to a project or namespace' do
+        let(:service) { described_class.new(container: container, current_user: nil) }
+
+        it 'unassigns policy project from the project', :aggregate_failures do
+          expect(result).to be_success
+          expect(container.security_orchestration_policy_configuration).to be_destroyed
+        end
 
-      it 'unassigns policy project from the project', :aggregate_failures do
-        expect(result).to be_success
-        expect(project.security_orchestration_policy_configuration).to be_destroyed
+        context 'when destroy fails' do
+          before do
+            allow(container.security_orchestration_policy_configuration).to receive(:delete).and_return(false)
+          end
+
+          it { is_expected.not_to be_success }
+        end
       end
-    end
 
-    context 'when policy project is not assigned to a project' do
-      let(:service) { described_class.new(project_without_policy_project, nil) }
+      context 'when policy project is not assigned to a project or namespace' do
+        let(:service) { described_class.new(container: container_without_policy_project, current_user: nil) }
 
-      it 'respond with an error', :aggregate_failures do
-        expect(result).not_to be_success
-        expect(result.message).to eq("Policy project doesn't exist")
+        it 'respond with an error', :aggregate_failures do
+          expect(result).not_to be_success
+          expect(result.message).to eq("Policy project doesn't exist")
+        end
       end
     end
+
+    context 'for project' do
+      let_it_be(:container, reload: true) { create(:project, :with_security_orchestration_policy_configuration) }
+      let_it_be(:container_without_policy_project, reload: true) { create(:project) }
+
+      it_behaves_like 'unassigns policy project'
+    end
+
+    context 'for namespace' do
+      let_it_be(:container, reload: true) { create(:namespace, :with_security_orchestration_policy_configuration) }
+      let_it_be(:container_without_policy_project, reload: true) { create(:namespace) }
+
+      it_behaves_like 'unassigns policy project'
+    end
   end
 end