From 5158f06786a727587b8f1e7c27f19c1b38253958 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Tue, 22 Oct 2024 00:01:02 +0530 Subject: [PATCH 1/5] Added mutation for creating compliance requirements Changelog: added EE: true --- doc/api/graphql/reference/index.md | 45 +++++++ doc/user/compliance/audit_event_types.md | 1 + ee/app/graphql/ee/types/mutation_type.rb | 2 + .../compliance_requirements/create.rb | 60 +++++++++ .../compliance_requirement_input_type.rb | 19 +++ .../compliance_requirement_type.rb | 26 ++++ .../compliance_requirement.rb | 2 + .../compliance_requirements/create_service.rb | 60 +++++++++ .../types/created_compliance_requirement.yml | 10 ++ .../compliance_requirement_input_type_spec.rb | 9 ++ .../compliance_requirement_type_spec.rb | 17 +++ .../compliance_requirements/create_spec.rb | 125 ++++++++++++++++++ .../compliance_requirement_spec.rb | 3 + .../compliance_requirements/create_spec.rb | 79 +++++++++++ .../create_service_spec.rb | 98 ++++++++++++++ locale/gitlab.pot | 3 + 16 files changed, 559 insertions(+) create mode 100644 ee/app/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create.rb create mode 100644 ee/app/graphql/types/compliance_management/compliance_requirement_input_type.rb create mode 100644 ee/app/graphql/types/compliance_management/compliance_requirement_type.rb create mode 100644 ee/app/services/compliance_management/compliance_framework/compliance_requirements/create_service.rb create mode 100644 ee/config/audit_events/types/created_compliance_requirement.yml create mode 100644 ee/spec/graphql/ee/types/compliance_management/compliance_requirement_input_type_spec.rb create mode 100644 ee/spec/graphql/ee/types/compliance_management/compliance_requirement_type_spec.rb create mode 100644 ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb create mode 100644 ee/spec/requests/api/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb create mode 100644 ee/spec/services/compliance_management/compliance_framework/compliance_requirements/create_service_spec.rb diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index b646b5b826f64f..5f096fa2254ae3 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -3502,6 +3502,30 @@ Input type: `CreateComplianceFrameworkInput` | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | `framework` | [`ComplianceFramework`](#complianceframework) | Created compliance framework. | +### `Mutation.createComplianceRequirement` + +DETAILS: +**Introduced** in GitLab 17.6. +**Status**: Experiment. + +Input type: `CreateComplianceRequirementInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `complianceFrameworkId` | [`ComplianceManagementFrameworkID!`](#compliancemanagementframeworkid) | Global ID of the compliance framework of the new requirement. | +| `params` | [`ComplianceRequirementInput!`](#compliancerequirementinput) | Parameters to update the compliance requirement with. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| `requirement` | [`ComplianceRequirement`](#compliancerequirement) | Created compliance requirement. | + ### `Mutation.createContainerRegistryProtectionRule` Creates a protection rule to restrict access to a project's container registry. Available only when feature flag `container_registry_protected_containers` is enabled. @@ -20509,6 +20533,18 @@ Represents a ComplianceFramework associated with a Project. | `scanExecutionPolicies` | [`ScanExecutionPolicyConnection`](#scanexecutionpolicyconnection) | Scan Execution Policies of the compliance framework. (see [Connections](#connections)) | | `scanResultPolicies` | [`ScanResultPolicyConnection`](#scanresultpolicyconnection) | Scan Result Policies of the compliance framework. (see [Connections](#connections)) | +### `ComplianceRequirement` + +Represents a ComplianceRequirement associated with a ComplianceFramework. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `description` | [`String!`](#string) | Description of the compliance requirement. | +| `id` | [`ID!`](#id) | Compliance requirement ID. | +| `name` | [`String!`](#string) | Name of the compliance requirement. | + ### `ComplianceStandardsAdherence` Compliance standards adherence for a project. @@ -42598,6 +42634,15 @@ Attributes for defining a CI/CD variable. | `name` | [`String`](#string) | New name for the compliance framework. | | `pipelineConfigurationFullPath` **{warning-solid}** | [`String`](#string) | **Deprecated:** Use pipeline execution policies instead. Deprecated in GitLab 17.4. | +### `ComplianceRequirementInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `description` | [`String`](#string) | New description for the compliance requirement. | +| `name` | [`String`](#string) | New name for the compliance requirement. | + ### `ComplianceStandardsAdherenceInput` #### Arguments diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 6cf7a4df30b15e..1183e368cb3fc2 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -140,6 +140,7 @@ Audit event types belong to the following product categories. | [`compliance_framework_removed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157893) | Triggered when a compliance framework is removed from a project | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/464160) | Project | | [`create_compliance_framework`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74292) | Triggered on when a compliance framework is successfully created | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/340649) | Group | | [`create_status_check`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84624) | Triggered when an external status check is created | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/355805) | Project | +| [`created_compliance_requirement`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/169485) | Triggered when a requirement is added to a compliance framework | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.6](https://gitlab.com/gitlab-org/gitlab/-/issues/470695) | Group | | [`delete_status_check`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84624) | Triggered when an external status check is deleted | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/355805) | Project | | [`destroy_compliance_framework`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74292) | Triggered when a compliance framework is successfully deleted | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/340649) | Group | | [`email_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114546) | Triggered when an email is created | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/374107) | User | diff --git a/ee/app/graphql/ee/types/mutation_type.rb b/ee/app/graphql/ee/types/mutation_type.rb index 08d70e3ad2b449..6fb7d5a6130cb9 100644 --- a/ee/app/graphql/ee/types/mutation_type.rb +++ b/ee/app/graphql/ee/types/mutation_type.rb @@ -248,6 +248,8 @@ def self.authorization_scopes mount_mutation ::Mutations::Ai::FeatureSettings::Update, alpha: { milestone: '17.4' } mount_mutation ::Mutations::Projects::TargetBranchRules::Create mount_mutation ::Mutations::Projects::TargetBranchRules::Destroy + mount_mutation ::Mutations::ComplianceManagement::ComplianceFramework::ComplianceRequirements::Create, + alpha: { milestone: '17.6' } prepend(Types::DeprecatedMutations) end diff --git a/ee/app/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create.rb b/ee/app/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create.rb new file mode 100644 index 00000000000000..e43272ee22a35b --- /dev/null +++ b/ee/app/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Mutations + module ComplianceManagement + module ComplianceFramework + module ComplianceRequirements + class Create < BaseMutation + graphql_name 'CreateComplianceRequirement' + + field :requirement, + Types::ComplianceManagement::ComplianceRequirementType, + null: true, + description: 'Created compliance requirement.' + + argument :compliance_framework_id, ::Types::GlobalIDType[::ComplianceManagement::Framework], + required: true, + description: 'Global ID of the compliance framework of the new requirement.' + + argument :params, Types::ComplianceManagement::ComplianceRequirementInputType, + required: true, + description: 'Parameters to update the compliance requirement with.' + + def resolve(args) + framework = framework(id: args[:compliance_framework_id]) + + raise_resource_not_available_error! if framework.nil? + + service = ::ComplianceManagement::ComplianceFramework::ComplianceRequirements::CreateService.new( + framework: framework, + params: args[:params].to_h, + current_user: current_user + ).execute + + service.success? ? success(service) : error(service) + end + + private + + def success(service) + { requirement: service.payload[:requirement], errors: [] } + end + + def error(service) + errors = [service.message] + model_errors = service.payload.try(:full_messages).to_a + + { errors: (errors + model_errors).flatten } + end + + def framework(id:) + ::Gitlab::Graphql::Lazy.force(GitlabSchema.object_from_id(id, + expected_type: ::ComplianceManagement::Framework)) + rescue Gitlab::Graphql::Errors::ArgumentError + nil + end + end + end + end + end +end diff --git a/ee/app/graphql/types/compliance_management/compliance_requirement_input_type.rb b/ee/app/graphql/types/compliance_management/compliance_requirement_input_type.rb new file mode 100644 index 00000000000000..cbbd6903ae85af --- /dev/null +++ b/ee/app/graphql/types/compliance_management/compliance_requirement_input_type.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Types + module ComplianceManagement + class ComplianceRequirementInputType < BaseInputObject + graphql_name 'ComplianceRequirementInput' + + argument :name, + GraphQL::Types::String, + required: false, + description: 'New name for the compliance requirement.' + + argument :description, + GraphQL::Types::String, + required: false, + description: 'New description for the compliance requirement.' + end + end +end diff --git a/ee/app/graphql/types/compliance_management/compliance_requirement_type.rb b/ee/app/graphql/types/compliance_management/compliance_requirement_type.rb new file mode 100644 index 00000000000000..2a70cd16e864a1 --- /dev/null +++ b/ee/app/graphql/types/compliance_management/compliance_requirement_type.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# rubocop: disable Graphql/AuthorizeTypes -- because ComplianceRequirementType is, and should only be, accessible via ProjectType similar to ComplianceFrameworkType + +module Types + module ComplianceManagement + class ComplianceRequirementType < Types::BaseObject + graphql_name 'ComplianceRequirement' + description 'Represents a ComplianceRequirement associated with a ComplianceFramework' + + field :id, GraphQL::Types::ID, + null: false, + description: 'Compliance requirement ID.' + + field :name, GraphQL::Types::String, + null: false, + description: 'Name of the compliance requirement.' + + field :description, GraphQL::Types::String, + null: false, + description: 'Description of the compliance requirement.' + end + end +end + +# rubocop: enable Graphql/AuthorizeTypes diff --git a/ee/app/models/compliance_management/compliance_framework/compliance_requirement.rb b/ee/app/models/compliance_management/compliance_framework/compliance_requirement.rb index ecb3118f82b15a..89585b0a9174d4 100644 --- a/ee/app/models/compliance_management/compliance_framework/compliance_requirement.rb +++ b/ee/app/models/compliance_management/compliance_framework/compliance_requirement.rb @@ -13,6 +13,8 @@ class ComplianceRequirement < ApplicationRecord validates :name, uniqueness: { scope: :framework_id } validate :requirements_count_per_framework + validates :name, :description, length: { maximum: 255 } + has_many :security_policy_requirements, class_name: 'ComplianceManagement::ComplianceFramework::SecurityPolicyRequirement' diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements/create_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements/create_service.rb new file mode 100644 index 00000000000000..7435d1264f11c7 --- /dev/null +++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements/create_service.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module ComplianceManagement + module ComplianceFramework + module ComplianceRequirements + class CreateService < BaseService + attr_reader :params, :current_user, :framework, :requirement + + def initialize(framework:, params:, current_user:) + @framework = framework + @params = params + @current_user = current_user + @requirement = ComplianceManagement::ComplianceFramework::ComplianceRequirement.new + end + + def execute + requirement.assign_attributes( + framework: framework, + namespace_id: framework.namespace&.root_ancestor&.id, + name: params[:name], + description: params[:description] + ) + + return ServiceResponse.error(message: 'Not permitted to create requirement') unless permitted? + + return error unless requirement.save + + audit_create + success + end + + private + + def permitted? + can? current_user, :admin_compliance_framework, framework + end + + def success + ServiceResponse.success(payload: { requirement: requirement }) + end + + def audit_create + audit_context = { + name: 'created_compliance_requirement', + author: current_user, + scope: framework.namespace, + target: requirement, + message: "Created compliance requirement #{requirement.name} for framework #{framework.name}" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + + def error + ServiceResponse.error(message: _('Failed to create compliance requirement'), payload: requirement.errors) + end + end + end + end +end diff --git a/ee/config/audit_events/types/created_compliance_requirement.yml b/ee/config/audit_events/types/created_compliance_requirement.yml new file mode 100644 index 00000000000000..bc50c2d86d6ee0 --- /dev/null +++ b/ee/config/audit_events/types/created_compliance_requirement.yml @@ -0,0 +1,10 @@ +--- +name: created_compliance_requirement +description: Triggered when a requirement is added to a compliance framework +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/470695 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/169485 +milestone: '17.6' +feature_category: compliance_management +saved_to_database: true +streamed: true +scope: [Group] diff --git a/ee/spec/graphql/ee/types/compliance_management/compliance_requirement_input_type_spec.rb b/ee/spec/graphql/ee/types/compliance_management/compliance_requirement_input_type_spec.rb new file mode 100644 index 00000000000000..ba7d784275b791 --- /dev/null +++ b/ee/spec/graphql/ee/types/compliance_management/compliance_requirement_input_type_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Types::ComplianceManagement::ComplianceRequirementInputType, feature_category: :compliance_management do + it { expect(described_class.graphql_name).to eq('ComplianceRequirementInput') } + + it { expect(described_class.arguments.keys).to match_array(%w[name description]) } +end diff --git a/ee/spec/graphql/ee/types/compliance_management/compliance_requirement_type_spec.rb b/ee/spec/graphql/ee/types/compliance_management/compliance_requirement_type_spec.rb new file mode 100644 index 00000000000000..6db3f4436a392f --- /dev/null +++ b/ee/spec/graphql/ee/types/compliance_management/compliance_requirement_type_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['ComplianceRequirement'], feature_category: :compliance_management do + subject { described_class } + + fields = %w[ + id + name + description + ] + + it 'has the correct fields' do + is_expected.to have_graphql_fields(fields) + end +end diff --git a/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb b/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb new file mode 100644 index 00000000000000..f2d7d4b52a0a8d --- /dev/null +++ b/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::ComplianceManagement::ComplianceFramework::ComplianceRequirements::Create, + feature_category: :compliance_management do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:namespace) { create(:group) } + let_it_be(:framework) { create(:compliance_framework, namespace: namespace) } + + let(:params) { valid_params } + let(:mutation) { described_class.new(object: nil, context: query_context, field: nil) } + + subject(:mutate) { mutation.resolve(**params) } + + describe '#resolve' do + context 'when feature is unlicensed' do + before do + stub_licensed_features(custom_compliance_frameworks: false) + end + + it 'does not create a new compliance requirement' do + expect { mutate }.not_to change { framework.compliance_requirements.count } + end + + it 'returns useful error messages' do + expect(mutate[:errors]).to include('Not permitted to create requirement') + end + end + + context 'when feature is licensed' do + before do + stub_licensed_features(custom_compliance_frameworks: true) + end + + context 'when current_user is not group namespace owner' do + it 'does not create a new compliance requirement' do + expect { mutate }.not_to change { framework.compliance_requirements.count } + end + + it 'returns useful error messages' do + expect(mutate[:errors]).to include 'Not permitted to create requirement' + end + end + + context 'when current_user is group owner' do + before_all do + namespace.add_owner(current_user) + end + + it 'creates a new compliance requirement' do + expect { mutate }.to change { framework.compliance_requirements.count }.by 1 + end + end + + context 'when current_user is personal namespace owner' do + let_it_be(:namespace) { create(:user_namespace) } + + let(:current_user) { namespace.owner } + + context 'when framework parameters are valid' do + it 'does not create a new compliance requirement' do + expect { mutate }.not_to change { framework.compliance_requirements.count } + end + end + + context 'when framework id is invalid' do + let(:params) { valid_params.merge(compliance_framework_id: 'non_existing_id') } + + it 'raises an error' do + expect { mutate }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + + context 'when framework id does not exist' do + let(:params) do + valid_params.merge( + compliance_framework_id: "gid://gitlab/ComplianceManagement::Framework/#{non_existing_record_id}" + ) + end + + it 'raises an error' do + expect { mutate }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + + context 'when requirement parameters are invalid' do + subject { mutation.resolve(**invalid_name_params) } + + it 'does not create a new compliance requirement' do + expect { mutate }.not_to change { framework.compliance_requirements.count } + end + + it 'returns useful error messages' do + expect(mutate[:errors]).to include 'Not permitted to create requirement' + end + end + end + end + end + + private + + def valid_params + { + compliance_framework_id: framework.to_gid, + params: { + name: 'Custom framework requirement', + description: 'Example Description' + } + } + end + + def invalid_name_params + { + compliance_framework_id: framework.to_gid, + params: { + name: 'A' * 256, + description: 'Example Description' + } + } + end +end diff --git a/ee/spec/models/compliance_management/compliance_framework/compliance_requirement_spec.rb b/ee/spec/models/compliance_management/compliance_framework/compliance_requirement_spec.rb index 04cc1cdd718cf3..aeebf590672c9b 100644 --- a/ee/spec/models/compliance_management/compliance_framework/compliance_requirement_spec.rb +++ b/ee/spec/models/compliance_management/compliance_framework/compliance_requirement_spec.rb @@ -15,6 +15,9 @@ it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:description) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_length_of(:description).is_at_most(255) } + describe '#requirements_count_per_framework' do let_it_be(:compliance_framework_1) { create(:compliance_framework, :sox, namespace: group) } diff --git a/ee/spec/requests/api/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb b/ee/spec/requests/api/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb new file mode 100644 index 00000000000000..08d9850cf03da0 --- /dev/null +++ b/ee/spec/requests/api/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Create a Compliance Requirement', feature_category: :compliance_management do + include GraphqlHelpers + + let_it_be(:namespace) { create(:group) } + let_it_be(:framework) { create(:compliance_framework, namespace: namespace) } + let_it_be(:current_user) { create(:user) } + + let(:mutation) do + graphql_mutation( + :create_compliance_requirement, + compliance_framework_id: framework.to_gid, + params: { + name: 'Custom framework requirement', + description: 'Example Description' + } + ) + end + + subject(:mutate) { post_graphql_mutation(mutation, current_user: current_user) } + + def mutation_response + graphql_mutation_response(:create_compliance_requirement) + end + + shared_examples 'a mutation that creates a compliance requirement' do + it 'creates a new compliance requirement' do + expect { mutate }.to change { framework.compliance_requirements.count }.by 1 + end + + it 'returns the newly created requirement', :aggregate_failures do + mutate + + expect(mutation_response['requirement']['name']).to eq 'Custom framework requirement' + expect(mutation_response['requirement']['description']).to eq 'Example Description' + end + end + + context 'when framework feature is unlicensed' do + before do + stub_licensed_features(custom_compliance_frameworks: false) + post_graphql_mutation(mutation, current_user: current_user) + end + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Not permitted to create requirement'] + end + + context 'when feature is licensed' do + before do + stub_licensed_features(custom_compliance_frameworks: true, evaluate_group_level_compliance_pipeline: true) + end + + context 'when current_user is group owner' do + before_all do + namespace.add_owner(current_user) + end + + it_behaves_like 'a mutation that creates a compliance requirement' + end + + context 'when current_user is not a group owner' do + context 'when current_user is group owner' do + before_all do + namespace.add_maintainer(current_user) + end + + it 'does not create a new compliance requirement' do + expect { mutate }.not_to change { framework.compliance_requirements.count } + end + + it_behaves_like 'a mutation that returns errors in the response', + errors: ['Not permitted to create requirement'] + end + end + end +end diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements/create_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements/create_service_spec.rb new file mode 100644 index 00000000000000..dfefe20a98fab6 --- /dev/null +++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements/create_service_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ComplianceManagement::ComplianceFramework::ComplianceRequirements::CreateService, + feature_category: :compliance_management do + let_it_be_with_refind(:namespace) { create(:group) } + let_it_be(:current_user) { create(:user, owner_of: namespace) } + let_it_be(:framework) { create(:compliance_framework, namespace: namespace) } + + let(:params) do + { + name: 'Custom framework requirement', + description: 'Description about the requirement' + } + end + + context 'when custom_compliance_frameworks is disabled' do + before do + stub_licensed_features(custom_compliance_frameworks: false) + end + + subject(:requirement_creator) do + described_class.new(framework: framework, params: params, current_user: current_user) + end + + it 'does not create a new compliance requirement' do + expect { requirement_creator.execute }.not_to change { framework.compliance_requirements.count } + end + + it 'responds with an error message' do + expect(requirement_creator.execute.message).to eq('Not permitted to create requirement') + end + end + + context 'when custom_compliance_frameworks is enabled' do + before do + stub_licensed_features(custom_compliance_frameworks: true) + end + + context 'when using invalid parameters' do + subject(:requirement_creator) do + described_class.new(framework: framework, params: params.except(:name), current_user: current_user) + end + + let(:response) { requirement_creator.execute } + + it 'responds with an error service response' do + expect(response.success?).to be_falsey + expect(response.payload.messages[:name]).to contain_exactly "can't be blank" + end + end + + context 'when creating a compliance requirement for a namespace that current_user is not the owner of' do + subject(:requirement_creator) do + described_class.new(framework: framework, params: params, current_user: create(:user)) + end + + it 'responds with an error service response' do + expect(requirement_creator.execute.success?).to be false + end + + it 'does not create a new compliance requirement' do + expect { requirement_creator.execute }.not_to change { framework.compliance_requirements.count } + end + end + + context 'when using parameters for a valid compliance requirement' do + subject(:requirement_creator) do + described_class.new(framework: framework, params: params, current_user: current_user) + end + + it 'audits the changes' do + expect { requirement_creator.execute } + .to change { AuditEvent.where("details LIKE ?", "%created_compliance_requirement%").count }.by(1) + end + + it 'creates a new compliance requirement' do + expect { requirement_creator.execute }.to change { framework.compliance_requirements.count }.by(1) + end + + it 'responds with a successful service response' do + expect(requirement_creator.execute.success?).to be true + end + + it 'has the expected attributes' do + requirement = requirement_creator.execute.payload[:requirement] + + expect(requirement.attributes).to include( + "name" => "Custom framework requirement", + "description" => "Description about the requirement", + "framework_id" => framework.id, + "namespace_id" => namespace.id + ) + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 53b35b6d006770..0821b48c1e8b2d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -22905,6 +22905,9 @@ msgstr "" msgid "Failed to create branch target" msgstr "" +msgid "Failed to create compliance requirement" +msgstr "" + msgid "Failed to create framework" msgstr "" -- GitLab From 48f779cd150661edb99837159718747f61cd389b Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Tue, 22 Oct 2024 00:04:09 +0530 Subject: [PATCH 2/5] Changed comment for rubocop --- .../types/compliance_management/compliance_requirement_type.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/graphql/types/compliance_management/compliance_requirement_type.rb b/ee/app/graphql/types/compliance_management/compliance_requirement_type.rb index 2a70cd16e864a1..459d54a80bb0ab 100644 --- a/ee/app/graphql/types/compliance_management/compliance_requirement_type.rb +++ b/ee/app/graphql/types/compliance_management/compliance_requirement_type.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# rubocop: disable Graphql/AuthorizeTypes -- because ComplianceRequirementType is, and should only be, accessible via ProjectType similar to ComplianceFrameworkType +# rubocop: disable Graphql/AuthorizeTypes -- because ComplianceRequirementType is, and should only be, accessible via ComplianceFrameworkType module Types module ComplianceManagement -- GitLab From 371d32a58deb1758fc984816414da6ab0fc61aec Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Tue, 22 Oct 2024 00:07:29 +0530 Subject: [PATCH 3/5] Modified requirement namespace id --- .../compliance_requirements/create_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements/create_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements/create_service.rb index 7435d1264f11c7..11c7c17de89761 100644 --- a/ee/app/services/compliance_management/compliance_framework/compliance_requirements/create_service.rb +++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements/create_service.rb @@ -16,7 +16,7 @@ def initialize(framework:, params:, current_user:) def execute requirement.assign_attributes( framework: framework, - namespace_id: framework.namespace&.root_ancestor&.id, + namespace_id: framework.namespace.id, name: params[:name], description: params[:description] ) -- GitLab From 5c0dd3b2d3a05402b863bc59d9b0dd0188211217 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Tue, 22 Oct 2024 15:28:39 +0530 Subject: [PATCH 4/5] Added authorization and fixed test cases --- .../compliance_requirements/create.rb | 13 +-- .../compliance_requirements/create_spec.rb | 88 +++++-------------- .../compliance_requirements/create_spec.rb | 5 +- 3 files changed, 28 insertions(+), 78 deletions(-) diff --git a/ee/app/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create.rb b/ee/app/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create.rb index e43272ee22a35b..82ba11d81fe807 100644 --- a/ee/app/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create.rb +++ b/ee/app/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create.rb @@ -7,6 +7,8 @@ module ComplianceRequirements class Create < BaseMutation graphql_name 'CreateComplianceRequirement' + authorize :admin_compliance_framework + field :requirement, Types::ComplianceManagement::ComplianceRequirementType, null: true, @@ -21,9 +23,7 @@ class Create < BaseMutation description: 'Parameters to update the compliance requirement with.' def resolve(args) - framework = framework(id: args[:compliance_framework_id]) - - raise_resource_not_available_error! if framework.nil? + framework = authorized_find!(id: args[:compliance_framework_id]) service = ::ComplianceManagement::ComplianceFramework::ComplianceRequirements::CreateService.new( framework: framework, @@ -46,13 +46,6 @@ def error(service) { errors: (errors + model_errors).flatten } end - - def framework(id:) - ::Gitlab::Graphql::Lazy.force(GitlabSchema.object_from_id(id, - expected_type: ::ComplianceManagement::Framework)) - rescue Gitlab::Graphql::Errors::ArgumentError - nil - end end end end diff --git a/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb b/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb index f2d7d4b52a0a8d..1e28ebfc0f4ef7 100644 --- a/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb +++ b/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb @@ -16,87 +16,45 @@ subject(:mutate) { mutation.resolve(**params) } describe '#resolve' do - context 'when feature is unlicensed' do - before do - stub_licensed_features(custom_compliance_frameworks: false) - end - - it 'does not create a new compliance requirement' do - expect { mutate }.not_to change { framework.compliance_requirements.count } - end - - it 'returns useful error messages' do - expect(mutate[:errors]).to include('Not permitted to create requirement') + shared_examples 'resource not available' do + it 'raises error' do + expect { mutate }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) end end - context 'when feature is licensed' do - before do - stub_licensed_features(custom_compliance_frameworks: true) - end + before do + stub_licensed_features(custom_compliance_frameworks: true) + end - context 'when current_user is not group namespace owner' do - it 'does not create a new compliance requirement' do - expect { mutate }.not_to change { framework.compliance_requirements.count } - end + context 'when current_user is not group namespace owner' do + it_behaves_like 'resource not available' + end - it 'returns useful error messages' do - expect(mutate[:errors]).to include 'Not permitted to create requirement' - end + context 'when current_user is group owner' do + before_all do + namespace.add_owner(current_user) end - context 'when current_user is group owner' do - before_all do - namespace.add_owner(current_user) - end - + context 'when all arguments are valid' do it 'creates a new compliance requirement' do expect { mutate }.to change { framework.compliance_requirements.count }.by 1 end end - context 'when current_user is personal namespace owner' do - let_it_be(:namespace) { create(:user_namespace) } + context 'when requirement parameters are invalid' do + subject { mutation.resolve(**invalid_name_params) } - let(:current_user) { namespace.owner } - - context 'when framework parameters are valid' do - it 'does not create a new compliance requirement' do - expect { mutate }.not_to change { framework.compliance_requirements.count } - end - end - - context 'when framework id is invalid' do - let(:params) { valid_params.merge(compliance_framework_id: 'non_existing_id') } - - it 'raises an error' do - expect { mutate }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) - end - end - - context 'when framework id does not exist' do - let(:params) do - valid_params.merge( - compliance_framework_id: "gid://gitlab/ComplianceManagement::Framework/#{non_existing_record_id}" - ) - end - - it 'raises an error' do - expect { mutate }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) - end - end + it_behaves_like 'resource not available' + end + end - context 'when requirement parameters are invalid' do - subject { mutation.resolve(**invalid_name_params) } + context 'when current_user is personal namespace owner' do + let_it_be(:namespace) { create(:user_namespace) } - it 'does not create a new compliance requirement' do - expect { mutate }.not_to change { framework.compliance_requirements.count } - end + let(:current_user) { namespace.owner } - it 'returns useful error messages' do - expect(mutate[:errors]).to include 'Not permitted to create requirement' - end - end + context 'when framework parameters are valid' do + it_behaves_like 'resource not available' end end end diff --git a/ee/spec/requests/api/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb b/ee/spec/requests/api/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb index 08d9850cf03da0..4784f7b5b46be6 100644 --- a/ee/spec/requests/api/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb @@ -45,7 +45,7 @@ def mutation_response post_graphql_mutation(mutation, current_user: current_user) end - it_behaves_like 'a mutation that returns errors in the response', errors: ['Not permitted to create requirement'] + it_behaves_like 'a mutation that returns a top-level access error' end context 'when feature is licensed' do @@ -71,8 +71,7 @@ def mutation_response expect { mutate }.not_to change { framework.compliance_requirements.count } end - it_behaves_like 'a mutation that returns errors in the response', - errors: ['Not permitted to create requirement'] + it_behaves_like 'a mutation that returns a top-level access error' end end end -- GitLab From d7c21af597359b360cac5efc86e1328edda7ec5a Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Wed, 23 Oct 2024 21:41:04 +0530 Subject: [PATCH 5/5] Fixed failing rspec --- .../compliance_framework/compliance_requirement.rb | 2 -- .../compliance_requirements/create_spec.rb | 12 +++++++++--- .../compliance_requirement_spec.rb | 3 --- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ee/app/models/compliance_management/compliance_framework/compliance_requirement.rb b/ee/app/models/compliance_management/compliance_framework/compliance_requirement.rb index 89585b0a9174d4..ecb3118f82b15a 100644 --- a/ee/app/models/compliance_management/compliance_framework/compliance_requirement.rb +++ b/ee/app/models/compliance_management/compliance_framework/compliance_requirement.rb @@ -13,8 +13,6 @@ class ComplianceRequirement < ApplicationRecord validates :name, uniqueness: { scope: :framework_id } validate :requirements_count_per_framework - validates :name, :description, length: { maximum: 255 } - has_many :security_policy_requirements, class_name: 'ComplianceManagement::ComplianceFramework::SecurityPolicyRequirement' diff --git a/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb b/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb index 1e28ebfc0f4ef7..8f67d223b834cf 100644 --- a/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb +++ b/ee/spec/graphql/mutations/compliance_management/compliance_framework/compliance_requirements/create_spec.rb @@ -42,9 +42,15 @@ end context 'when requirement parameters are invalid' do - subject { mutation.resolve(**invalid_name_params) } + subject(:mutate) { mutation.resolve(**invalid_name_params) } - it_behaves_like 'resource not available' + it 'does not create a new compliance framework' do + expect { mutate }.not_to change { framework.compliance_requirements.count } + end + + it 'returns error for name' do + expect(mutate[:errors]).to include "Name can't be blank" + end end end @@ -75,7 +81,7 @@ def invalid_name_params { compliance_framework_id: framework.to_gid, params: { - name: 'A' * 256, + name: '', description: 'Example Description' } } diff --git a/ee/spec/models/compliance_management/compliance_framework/compliance_requirement_spec.rb b/ee/spec/models/compliance_management/compliance_framework/compliance_requirement_spec.rb index aeebf590672c9b..04cc1cdd718cf3 100644 --- a/ee/spec/models/compliance_management/compliance_framework/compliance_requirement_spec.rb +++ b/ee/spec/models/compliance_management/compliance_framework/compliance_requirement_spec.rb @@ -15,9 +15,6 @@ it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:description) } - it { is_expected.to validate_length_of(:name).is_at_most(255) } - it { is_expected.to validate_length_of(:description).is_at_most(255) } - describe '#requirements_count_per_framework' do let_it_be(:compliance_framework_1) { create(:compliance_framework, :sox, namespace: group) } -- GitLab