Commit f29af196 authored by Max Woolf's avatar Max Woolf

Merge branch...

Merge branch '347325-add-graphql-type-to-allow-retrieval-of-all-compliance-violations-within-a-group' into 'master'

Add GraphQL type to allow retrieval of all compliance violations within a group

See merge request gitlab-org/gitlab!80995
parents f886c642 7e69ac7f
......@@ -5967,6 +5967,29 @@ The edge type for [`ComplianceFramework`](#complianceframework).
| <a id="complianceframeworkedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. |
| <a id="complianceframeworkedgenode"></a>`node` | [`ComplianceFramework`](#complianceframework) | The item at the end of the edge. |
#### `ComplianceViolationConnection`
The connection type for [`ComplianceViolation`](#complianceviolation).
##### Fields
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="complianceviolationconnectionedges"></a>`edges` | [`[ComplianceViolationEdge]`](#complianceviolationedge) | A list of edges. |
| <a id="complianceviolationconnectionnodes"></a>`nodes` | [`[ComplianceViolation]`](#complianceviolation) | A list of nodes. |
| <a id="complianceviolationconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. |
#### `ComplianceViolationEdge`
The edge type for [`ComplianceViolation`](#complianceviolation).
##### Fields
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="complianceviolationedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. |
| <a id="complianceviolationedgenode"></a>`node` | [`ComplianceViolation`](#complianceviolation) | The item at the end of the edge. |
#### `ConnectedAgentConnection`
The connection type for [`ConnectedAgent`](#connectedagent).
......@@ -9499,6 +9522,20 @@ Represents a ComplianceFramework associated with a Project.
| <a id="complianceframeworkname"></a>`name` | [`String!`](#string) | Name of the compliance framework. |
| <a id="complianceframeworkpipelineconfigurationfullpath"></a>`pipelineConfigurationFullPath` | [`String`](#string) | Full path of the compliance pipeline configuration stored in a project repository, such as `.gitlab/.compliance-gitlab-ci.yml@compliance/hipaa` **(ULTIMATE)**. |
### `ComplianceViolation`
Compliance violation associated with a merged merge request. Available only when feature flag `compliance_violations_graphql_type` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice.
#### Fields
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="complianceviolationid"></a>`id` | [`ID!`](#id) | Compliance violation ID. |
| <a id="complianceviolationmergerequest"></a>`mergeRequest` | [`MergeRequest!`](#mergerequest) | Merge request the compliance violation occurred in. |
| <a id="complianceviolationreason"></a>`reason` | [`ComplianceViolationReason!`](#complianceviolationreason) | Reason the compliance violation occurred. |
| <a id="complianceviolationseveritylevel"></a>`severityLevel` | [`ComplianceViolationSeverity!`](#complianceviolationseverity) | Severity of the compliance violation. |
| <a id="complianceviolationviolatinguser"></a>`violatingUser` | [`UserCore!`](#usercore) | User suspected of causing the compliance violation. |
### `ComposerMetadata`
Composer metadata.
......@@ -11054,6 +11091,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| <a id="groupistemporarystorageincreaseenabled"></a>`isTemporaryStorageIncreaseEnabled` | [`Boolean!`](#boolean) | Status of the temporary storage increase. |
| <a id="grouplfsenabled"></a>`lfsEnabled` | [`Boolean`](#boolean) | Indicates if Large File Storage (LFS) is enabled for namespace. |
| <a id="groupmentionsdisabled"></a>`mentionsDisabled` | [`Boolean`](#boolean) | Indicates if a group is disabled from getting mentioned. |
| <a id="groupmergerequestviolations"></a>`mergeRequestViolations` | [`ComplianceViolationConnection`](#complianceviolationconnection) | Compliance violations reported on merge requests merged within the group. Available only when feature flag `compliance_violations_graphql_type` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice. (see [Connections](#connections)) |
| <a id="groupname"></a>`name` | [`String!`](#string) | Name of the namespace. |
| <a id="grouporganizations"></a>`organizations` | [`CustomerRelationsOrganizationConnection`](#customerrelationsorganizationconnection) | Find organizations of this group. (see [Connections](#connections)) |
| <a id="grouppackagesettings"></a>`packageSettings` | [`PackageSettings`](#packagesettings) | Package settings for the namespace. |
......@@ -17077,6 +17115,28 @@ Mode of a commit action.
| <a id="commitencodingbase64"></a>`BASE64` | Base64 encoding. |
| <a id="commitencodingtext"></a>`TEXT` | Text encoding. |
### `ComplianceViolationReason`
Reason for the compliance violation.
| Value | Description |
| ----- | ----------- |
| <a id="complianceviolationreasonapproved_by_committer"></a>`APPROVED_BY_COMMITTER` | Approved by committer. |
| <a id="complianceviolationreasonapproved_by_insufficient_users"></a>`APPROVED_BY_INSUFFICIENT_USERS` | Approved by insufficient users. |
| <a id="complianceviolationreasonapproved_by_merge_request_author"></a>`APPROVED_BY_MERGE_REQUEST_AUTHOR` | Approved by merge request author. |
### `ComplianceViolationSeverity`
Severity of the compliance violation.
| Value | Description |
| ----- | ----------- |
| <a id="complianceviolationseveritycritical"></a>`CRITICAL` | Critical severity. |
| <a id="complianceviolationseverityhigh"></a>`HIGH` | High severity. |
| <a id="complianceviolationseverityinfo"></a>`INFO` | Info severity. |
| <a id="complianceviolationseveritylow"></a>`LOW` | Low severity. |
| <a id="complianceviolationseveritymedium"></a>`MEDIUM` | Medium severity. |
### `ConanMetadatumFileTypeEnum`
Conan file types.
# frozen_string_literal: true
# ComplianceManagement::MergeRequests::ComplianceViolationsFinder
#
# Used by the API to filter Compliance Violation records created from merge requests merged within a group
#
# Arguments:
# current_user: the current user to validate they have the right permissions to access the compliance violations data
# group_id: the ID of the group to search within
module ComplianceManagement
module MergeRequests
class ComplianceViolationsFinder
include FinderMethods
include MergedAtFilter
def initialize(current_user:, group:)
@current_user = current_user
@group = group
end
def execute
return ::MergeRequests::ComplianceViolation.none unless allowed?
::MergeRequests::ComplianceViolation.by_group(group).order_by_severity_level(:desc)
end
private
attr_reader :current_user, :group
def allowed?
::Feature.enabled?(:compliance_violations_graphql_type, group, default_enabled: :yaml) &&
Ability.allowed?(current_user, :read_group_compliance_dashboard, group)
end
end
end
end
......@@ -98,6 +98,13 @@ module EE
null: true,
description: 'External locations that receive audit events belonging to the group.',
authorize: :admin_external_audit_events
field :merge_request_violations,
::Types::ComplianceManagement::MergeRequests::ComplianceViolationType.connection_type,
null: true,
description: 'Compliance violations reported on merge requests merged within the group.' \
' Available only when feature flag `compliance_violations_graphql_type` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice.',
resolver: ::Resolvers::ComplianceManagement::MergeRequests::ComplianceViolationResolver
end
end
end
......
# frozen_string_literal: true
module Resolvers
module ComplianceManagement
module MergeRequests
class ComplianceViolationResolver < BaseResolver
alias_method :group, :object
type ::Types::ComplianceManagement::MergeRequests::ComplianceViolationType.connection_type, null: true
description 'Compliance violations reported on a merged merge request.'
def resolve
::ComplianceManagement::MergeRequests::ComplianceViolationsFinder.new(current_user: current_user, group: group).execute
end
end
end
end
end
# frozen_string_literal: true
module Types
module ComplianceManagement
module MergeRequests
class ComplianceViolationReasonEnum < BaseEnum
graphql_name 'ComplianceViolationReason'
description 'Reason for the compliance violation.'
::Enums::MergeRequests::ComplianceViolation.reasons.keys.each do |reason|
value reason.to_s.upcase, value: reason.to_s, description: "#{reason.to_s.humanize}"
end
end
end
end
end
# frozen_string_literal: true
module Types
module ComplianceManagement
module MergeRequests
class ComplianceViolationSeverityEnum < BaseEnum
graphql_name 'ComplianceViolationSeverity'
description 'Severity of the compliance violation.'
::Enums::MergeRequests::ComplianceViolation.severity_levels.keys.each do |severity|
value severity.to_s.upcase, value: severity.to_s, description: "#{severity.to_s.humanize} severity"
end
end
end
end
end
# frozen_string_literal: true
module Types
module ComplianceManagement
module MergeRequests
class ComplianceViolationType < ::Types::BaseObject
graphql_name 'ComplianceViolation'
description 'Compliance violation associated with a merged merge request.' \
' Available only when feature flag `compliance_violations_graphql_type` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice.'
authorize :read_group_compliance_dashboard
field :id, GraphQL::Types::ID, null: false,
description: 'Compliance violation ID.'
field :severity_level, ComplianceViolationSeverityEnum, null: false,
description: 'Severity of the compliance violation.'
field :reason, ComplianceViolationReasonEnum, null: false,
description: 'Reason the compliance violation occurred.'
field :violating_user, ::Types::UserType, null: false,
description: 'User suspected of causing the compliance violation.'
field :merge_request, ::Types::MergeRequestType, null: false,
description: 'Merge request the compliance violation occurred in.'
end
end
end
end
......@@ -9,7 +9,13 @@ module MergeRequests
enum reason: ::Enums::MergeRequests::ComplianceViolation.reasons
enum severity_level: ::Enums::MergeRequests::ComplianceViolation.severity_levels
scope :approved_by_committer, -> { where(reason: ::Gitlab::ComplianceManagement::Violations::ApprovedByCommitter::REASON) }
scope :with_merge_requests, -> { preload(merge_request: [{ target_project: :namespace }, :metrics]) }
scope :join_projects, -> { with_merge_requests.joins(merge_request: { target_project: :namespace }) }
scope :by_approved_by_committer, -> { where(reason: ::Gitlab::ComplianceManagement::Violations::ApprovedByCommitter::REASON) }
scope :by_group, -> (group) { join_projects.where(merge_requests: { projects: { namespace_id: group.self_and_descendants } }) }
scope :order_by_severity_level, -> (direction) { order(severity_level: direction, id: direction) }
belongs_to :violating_user, class_name: 'User'
belongs_to :merge_request
......
# frozen_string_literal: true
module MergeRequests
class ComplianceViolationPolicy < BasePolicy
delegate { @subject.merge_request.target_project.namespace }
end
end
......@@ -43,7 +43,7 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord
def existing_violating_user_ids
@merge_request.compliance_violations.approved_by_committer.pluck(:violating_user_id)
@merge_request.compliance_violations.by_approved_by_committer.pluck(:violating_user_id)
end
# rubocop: enable CodeReuse/ActiveRecord
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ComplianceManagement::MergeRequests::ComplianceViolationsFinder do
let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, :repository, group: group) }
let_it_be(:project2) { create(:project, :repository, group: group) }
let_it_be(:project_outside_group) { create(:project, :repository, group: create(:group)) }
let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, state: :merged) }
let_it_be(:merge_request2) { create(:merge_request, source_project: project2, target_project: project2, state: :merged) }
let_it_be(:merge_request_outside_group) { create(:merge_request, source_project: project_outside_group, target_project: project_outside_group, state: :merged) }
let_it_be(:compliance_violation) { create(:compliance_violation, :approved_by_committer, severity_level: :low, merge_request: merge_request) }
let_it_be(:compliance_violation2) { create(:compliance_violation, :approved_by_merge_request_author, severity_level: :high, merge_request: merge_request2) }
let_it_be(:compliance_violation_outside_group) { create(:compliance_violation, :approved_by_committer, merge_request: merge_request_outside_group) }
subject(:finder) { described_class.new(current_user: current_user, group: group) }
describe '#execute' do
subject { finder.execute }
context 'when feature is disabled' do
before do
stub_feature_flags(compliance_violations_graphql_type: false)
end
it 'returns no compliance violations' do
expect(subject).to eq(::MergeRequests::ComplianceViolation.none)
end
end
context 'when feature is enabled' do
before do
stub_feature_flags(compliance_violations_graphql_type: true)
end
context 'when the user is unauthorized' do
it 'returns no compliance violations' do
expect(subject).to eq(::MergeRequests::ComplianceViolation.none)
end
end
context 'when the user is authorized' do
before do
group.add_owner(current_user)
end
context 'without any filters or sorting' do
it 'finds all the compliance violations' do
expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
end
end
end
end
end
end
......@@ -21,6 +21,7 @@ RSpec.describe GitlabSchema.types['Group'] do
it { expect(described_class).to have_graphql_field(:stats) }
it { expect(described_class).to have_graphql_field(:billable_members_count) }
it { expect(described_class).to have_graphql_field(:external_audit_event_destinations) }
it { expect(described_class).to have_graphql_field(:merge_request_violations) }
describe 'vulnerabilities' do
let_it_be(:group) { create(:group) }
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Resolvers::ComplianceManagement::MergeRequests::ComplianceViolationResolver do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, :repository, group: group) }
let_it_be(:project2) { create(:project, :repository, group: group) }
let_it_be(:project_outside_group) { create(:project, :repository, group: create(:group)) }
let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, state: :merged) }
let_it_be(:merge_request2) { create(:merge_request, source_project: project2, target_project: project2, state: :merged) }
let_it_be(:merge_request_outside_group) { create(:merge_request, source_project: project_outside_group, target_project: project_outside_group, state: :merged) }
let_it_be(:compliance_violation) { create(:compliance_violation, :approved_by_committer, severity_level: :low, merge_request: merge_request) }
let_it_be(:compliance_violation2) { create(:compliance_violation, :approved_by_merge_request_author, severity_level: :high, merge_request: merge_request2) }
let_it_be(:compliance_violation_outside_group) { create(:compliance_violation, :approved_by_committer, merge_request: merge_request_outside_group) }
describe '#resolve' do
subject(:resolve_compliance_violations) { resolve(described_class, obj: group, ctx: { current_user: current_user }) }
context 'feature flag is disabled' do
before do
stub_feature_flags(compliance_violations_graphql_type: false)
end
it 'returns an empty collection' do
expect(subject).to be_empty
end
end
context 'feature flag is enabled' do
context 'user is unauthorized' do
it 'returns an empty collection' do
expect(subject).to be_empty
end
end
context 'user is authorized' do
before do
group.add_owner(current_user)
end
context 'without any filters or sorting' do
it 'finds all the compliance violations' do
expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
end
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['ComplianceViolationReason'] do
let_it_be(:fields) do
::Enums::MergeRequests::ComplianceViolation.reasons.keys.map { |r| r.to_s.upcase }
end
specify { expect(described_class.graphql_name).to eq('ComplianceViolationReason') }
specify { expect(described_class.values.keys).to match_array(fields) }
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['ComplianceViolationSeverity'] do
let_it_be(:fields) do
::Enums::MergeRequests::ComplianceViolation.severity_levels.keys.map { |s| s.to_s.upcase }
end
specify { expect(described_class.graphql_name).to eq('ComplianceViolationSeverity') }
specify { expect(described_class.values.keys).to match_array(fields) }
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['ComplianceViolation'] do
let(:fields) do
%i[id severity_level reason violating_user merge_request]
end
specify { expect(described_class.graphql_name).to eq('ComplianceViolation') }
specify { expect(described_class).to have_graphql_fields(fields) }
specify { expect(described_class).to require_graphql_authorizations(:read_group_compliance_dashboard) }
end
......@@ -3,7 +3,10 @@
require 'spec_helper'
RSpec.describe MergeRequests::ComplianceViolation, type: :model do
using RSpec::Parameterized::TableSyntax
let_it_be(:merge_request) { create(:merge_request, state: :merged) }
let_it_be(:user) { create(:user) }
describe "Associations" do
it { is_expected.to belong_to(:violating_user) }
......@@ -17,8 +20,6 @@ RSpec.describe MergeRequests::ComplianceViolation, type: :model do
it { is_expected.to validate_presence_of(:severity_level) }
context "when a violation exists with the same reason and user for a merge request" do
let_it_be(:user) { create(:user) }
before do
allow(merge_request).to receive(:committers).and_return([user])
merge_request.approver_users << user
......@@ -41,21 +42,61 @@ RSpec.describe MergeRequests::ComplianceViolation, type: :model do
it { is_expected.to define_enum_for(:severity_level).with_values(::Enums::MergeRequests::ComplianceViolation.severity_levels) }
end
describe '#approved_by_committer' do
describe '#by_approved_by_committer' do
let_it_be(:violations) do
[
create(:compliance_violation, :approved_by_committer, merge_request: merge_request, violating_user: create(:user)),
create(:compliance_violation, :approved_by_committer, merge_request: merge_request, violating_user: user),
create(:compliance_violation, :approved_by_committer, merge_request: merge_request, violating_user: create(:user))
]
end
before do
# Add a violation using a different reason to make sure it doesn't pick it up
create(:compliance_violation, :approved_by_insufficient_users, merge_request: merge_request, violating_user: create(:user))
create(:compliance_violation, :approved_by_insufficient_users, merge_request: merge_request, violating_user: user)
end
it 'returns the correct collection of violations' do
expect(merge_request.compliance_violations.by_approved_by_committer).to eq violations
end
end
describe '#by_group' do
let_it_be(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:project) { create(:project, :repository, group: group) }
let_it_be(:subgroup_project) { create(:project, :repository, group: subgroup) }
let_it_be(:alt_merge_request) { create(:merge_request, source_project: project, target_project: project, state: :merged) }
let_it_be(:subgroup_merge_request) { create(:merge_request, source_project: subgroup_project, target_project: project, state: :merged) }
let_it_be(:violations) do
[
create(:compliance_violation, :approved_by_committer, merge_request: alt_merge_request, violating_user: user),
create(:compliance_violation, :approved_by_committer, merge_request: subgroup_merge_request, violating_user: user),
create(:compliance_violation, :approved_by_committer, merge_request: merge_request, violating_user: user)
]
end
it 'returns the correct collection of violations' do
expect(merge_request.compliance_violations.approved_by_committer).to eq violations
expect(described_class.by_group(group)).to contain_exactly(violations[0], violations[1])
end
end
describe '#order_by_severity_level' do
let_it_be(:violations) do
[
create(:compliance_violation, :approved_by_committer, severity_level: :low, merge_request: merge_request, violating_user: create(:user)),
create(:compliance_violation, :approved_by_committer, severity_level: :high, merge_request: merge_request, violating_user: create(:user))
]
end
where(:direction, :result) do
'ASC' | lazy { [violations[0], violations[1]] }
'DESC' | lazy { [violations[1], violations[0]] }
end
with_them do
it 'returns the correct collection of violations' do
expect(described_class.order_by_severity_level(direction)).to match_array(result)
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'getting the compliance violations for a group' do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, :repository, group: group) }
let_it_be(:project2) { create(:project, :repository, group: group) }
let_it_be(:project_outside_group) { create(:project, :repository, group: create(:group)) }
let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, state: :merged) }
let_it_be(:merge_request2) { create(:merge_request, source_project: project2, target_project: project2, state: :merged) }
let_it_be(:merge_request_outside_group) { create(:merge_request, source_project: project_outside_group, target_project: project_outside_group, state: :merged) }
let_it_be(:compliance_violation) { create(:compliance_violation, :approved_by_committer, severity_level: :low, merge_request: merge_request) }
let_it_be(:compliance_violation2) { create(:compliance_violation, :approved_by_merge_request_author, severity_level: :high, merge_request: merge_request2) }
let_it_be(:compliance_violation_outside_group) { create(:compliance_violation, :approved_by_committer, merge_request: merge_request_outside_group) }
let(:fields) do
<<~GRAPHQL
nodes {
id
severityLevel
reason
violatingUser {
id
}
mergeRequest {
id
}
}
GRAPHQL
end
def get_violation_values(violation)
{
'id' => violation.to_global_id.to_s,
'severityLevel' => ::Types::ComplianceManagement::MergeRequests::ComplianceViolationSeverityEnum.values[violation.severity_level.upcase].graphql_name,
'reason' => ::Types::ComplianceManagement::MergeRequests::ComplianceViolationReasonEnum.values[violation.reason.underscore.upcase].graphql_name,
'violatingUser' => {
'id' => violation.violating_user.to_global_id.to_s
},
'mergeRequest' => {
'id' => violation.merge_request.to_global_id.to_s
}
}
end
let(:violation_output) do
get_violation_values(compliance_violation)
end
let(:violation2_output) do
get_violation_values(compliance_violation2)
end
def query(params = {})
graphql_query_for(
:group, { full_path: group.full_path }, query_graphql_field("mergeRequestViolations", params, fields)
)
end
let(:compliance_violations) { graphql_data_at(:group, :merge_request_violations, :nodes) }
context 'when feature is disabled' do
before do
stub_feature_flags(compliance_violations_graphql_type: false)
end
it 'returns empty' do
post_graphql(query, current_user: current_user)
expect(compliance_violations).to be_empty
end
end
context 'when feature is enabled' do
before do
stub_feature_flags(compliance_violations_graphql_type: true)
end
context 'when the user is unauthorized' do
it 'returns empty' do
post_graphql(query, current_user: current_user)
expect(compliance_violations).to be_empty
end
end
context 'when the user is authorized' do
before do
group.add_owner(current_user)
end
context 'without any filters or sorting' do
it 'finds all the compliance violations' do
post_graphql(query, current_user: current_user)
expect(compliance_violations).to contain_exactly(violation_output, violation2_output)
end
end
end
end
end
......@@ -20,6 +20,8 @@ RSpec.describe 'getting group information' do
fields = all_graphql_fields_for('Group')
# TODO: Set required timelogs args elsewhere https://gitlab.com/gitlab-org/gitlab/-/issues/325499
fields.selection['timelogs(startDate: "2021-03-01" endDate: "2021-03-30")'] = fields.selection.delete('timelogs')
# TODO: Remove when `compliance_violations_graphql_type` feature flag is removed in https://gitlab.com/gitlab-org/gitlab/-/issues/350249
fields.selection.delete('mergeRequestViolations')
graphql_query_for(
'group',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment