From 99571c48618ceed91395000e455415e0eaad7e9c Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Mon, 11 Aug 2025 17:24:37 +0200 Subject: [PATCH] Extend policy bypass checker with users, roles and groups This change extends policy bypass checker to include users, roles and groups. The bypass check also checkes the git push option with bypass reason. EE: true Changelog: added --- doc/topics/git/commit.md | 8 + doc/user/compliance/audit_event_types.md | 1 + ee/app/models/ee/group_member.rb | 7 + ee/app/models/ee/project_team.rb | 7 + .../security_policy_user_push_bypass.yml | 11 + .../ee/gitlab/checks/security/policy_check.rb | 3 + .../scan_result_policies/bypass_settings.rb | 10 +- .../policy_bypass_auditor.rb | 97 +++++++ .../policy_bypass_checker.rb | 64 +++-- .../push_bypass_checker.rb | 2 + .../user_bypass_checker.rb | 55 ++++ .../checks/security/policy_check_spec.rb | 20 ++ .../bypass_settings_spec.rb | 24 +- .../policy_bypass_auditor_spec.rb | 242 +++++++++++++++++ .../policy_bypass_checker_spec.rb | 204 +++++++++++++- .../push_bypass_checker_spec.rb | 2 +- .../user_bypass_checker_spec.rb | 251 ++++++++++++++++++ ee/spec/models/ee/group_member_spec.rb | 89 +++++++ ee/spec/models/ee/project_team_spec.rb | 201 ++++++++++++++ ee/spec/models/security/policy_spec.rb | 12 +- 20 files changed, 1260 insertions(+), 50 deletions(-) create mode 100644 ee/config/audit_events/types/security_policy_user_push_bypass.yml create mode 100644 ee/lib/security/scan_result_policies/policy_bypass_auditor.rb create mode 100644 ee/lib/security/scan_result_policies/user_bypass_checker.rb create mode 100644 ee/spec/lib/security/scan_result_policies/policy_bypass_auditor_spec.rb create mode 100644 ee/spec/lib/security/scan_result_policies/user_bypass_checker_spec.rb diff --git a/doc/topics/git/commit.md b/doc/topics/git/commit.md index c086fa0d809450..365d945ad1c16a 100644 --- a/doc/topics/git/commit.md +++ b/doc/topics/git/commit.md @@ -177,6 +177,14 @@ You can use push options to skip [secret push protection](../../user/application |--------------------------------|-------------|---------| | `secret_push_protection.skip_all` | Do not perform secret push protection for any commit in this push. | `git push -o secret_push_protection.skip_all` | +### Push options for security policy + +You can use push options to [bypass security policies](../../user/application_security/policies/merge_request_approval_policies.md#access-token-and-service-account-exceptions). + +| Push option | Description | Example | +|--------------------------------|-------------|---------| +| `security_policy.bypass_reason` | Set the bypass reason for the security policy. | `git push -o security_policy.bypass_reason="Hot fix"` | + ### Push options for GitGuardian integration You can use the same [push option for Secret push protection](#push-options-for-secret-push-protection) to skip GitGuardian secret detection. diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 9123907098a483..59662299537725 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -547,6 +547,7 @@ Audit event types belong to the following product categories. | [`security_policy_pipeline_skipped`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195325) | A security policy pipeline is skipped | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/539232) | Project | | [`security_policy_service_account_push_bypass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249) | Branch push that is blocked by a security policy is bypassed for configured service account | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549644) | Project | | [`security_policy_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/192797) | A security policy is updated | {{< icon name="check-circle" >}} Yes | GitLab [18.1](https://gitlab.com/gitlab-org/gitlab/-/issues/539230) | Project | +| [`security_policy_user_push_bypass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/201001) | Branch push that is blocked by a security policy is bypassed for configured users | {{< icon name="check-circle" >}} Yes | GitLab [18.4](https://gitlab.com/gitlab-org/gitlab/-/issues/549797) | Project | | [`security_policy_violations_detected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violation is detected in the merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/549811) | Project | | [`security_policy_violations_resolved`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violations are resolved in the merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549812) | Project | | [`security_policy_yaml_invalidated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196721) | The policy YAML is invalidated in security policy project | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/550892) | Project | diff --git a/ee/app/models/ee/group_member.rb b/ee/app/models/ee/group_member.rb index c6cc9baf6040e1..05f6317b59afcc 100644 --- a/ee/app/models/ee/group_member.rb +++ b/ee/app/models/ee/group_member.rb @@ -54,6 +54,13 @@ def member_of_group?(group, user) exists?(group: group, user: user) end + def direct_member_of_groups?(group_ids, user) + active_without_invites_and_requests + .non_minimal_access + .where(source_id: group_ids) + .exists?(user_id: user.id) + end + def filter_by_enterprise_users(value) subquery = ::UserDetail.where( diff --git a/ee/app/models/ee/project_team.rb b/ee/app/models/ee/project_team.rb index af61084968264e..57f8f75a820a6b 100644 --- a/ee/app/models/ee/project_team.rb +++ b/ee/app/models/ee/project_team.rb @@ -24,6 +24,13 @@ def members_with_access_level_or_custom_roles(levels: [], member_role_ids: []) users end + def user_exists_with_access_level_or_custom_roles?(user, levels: [], member_role_ids: []) + return false unless levels.any? || member_role_ids.any? + return false unless user + + members_with_access_level_or_custom_roles(levels: levels, member_role_ids: member_role_ids).exists?(id: user.id) + end + override :add_members def add_members( users, diff --git a/ee/config/audit_events/types/security_policy_user_push_bypass.yml b/ee/config/audit_events/types/security_policy_user_push_bypass.yml new file mode 100644 index 00000000000000..cb04af897724ff --- /dev/null +++ b/ee/config/audit_events/types/security_policy_user_push_bypass.yml @@ -0,0 +1,11 @@ +--- +name: security_policy_user_push_bypass +description: Branch push that is blocked by a security policy is bypassed for configured + users +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/549797 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/201001 +feature_category: security_policy_management +milestone: '18.4' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/lib/ee/gitlab/checks/security/policy_check.rb b/ee/lib/ee/gitlab/checks/security/policy_check.rb index d97d885877fb26..2be4266c6f641a 100644 --- a/ee/lib/ee/gitlab/checks/security/policy_check.rb +++ b/ee/lib/ee/gitlab/checks/security/policy_check.rb @@ -7,6 +7,7 @@ module Security module PolicyCheck PUSH_ERROR_MESSAGE = "Push is blocked by settings overridden by a security policy" FORCE_PUSH_ERROR_MESSAGE = "Force push is blocked by settings overridden by a security policy" + BYPASS_REASON_ERROR_MESSAGE = "Bypass reason is required when bypassing security policy restrictions" LOG_MESSAGE = "Checking if scan result policies apply to branch..." def validate! @@ -40,6 +41,8 @@ def policies_bypass_applied? branch_name: branch_name, push_options: change_access.push_options ).check_bypass! + rescue ::Security::ScanResultPolicies::PolicyBypassChecker::BypassReasonRequiredError + raise ::Gitlab::GitAccess::ForbiddenError, BYPASS_REASON_ERROR_MESSAGE unless matching_merge_request? end def force_push? diff --git a/ee/lib/security/scan_result_policies/bypass_settings.rb b/ee/lib/security/scan_result_policies/bypass_settings.rb index 506875953585ad..7b2ead079981a8 100644 --- a/ee/lib/security/scan_result_policies/bypass_settings.rb +++ b/ee/lib/security/scan_result_policies/bypass_settings.rb @@ -10,12 +10,12 @@ def initialize(bypass_settings) end def access_token_ids - bypass_settings[:access_tokens]&.pluck(:id) + bypass_settings[:access_tokens]&.pluck(:id) || [] end strong_memoize_attr :access_token_ids def service_account_ids - bypass_settings[:service_accounts]&.pluck(:id) + bypass_settings[:service_accounts]&.pluck(:id) || [] end strong_memoize_attr :service_account_ids @@ -25,12 +25,12 @@ def branches strong_memoize_attr :branches def user_ids - bypass_settings[:users]&.pluck(:id) + bypass_settings[:users]&.pluck(:id) || [] end strong_memoize_attr :user_ids def group_ids - bypass_settings[:groups]&.pluck(:id) + bypass_settings[:groups]&.pluck(:id) || [] end strong_memoize_attr :group_ids @@ -40,7 +40,7 @@ def default_roles strong_memoize_attr :default_roles def custom_role_ids - bypass_settings[:custom_roles]&.pluck(:id) + bypass_settings[:custom_roles]&.pluck(:id) || [] end strong_memoize_attr :custom_role_ids diff --git a/ee/lib/security/scan_result_policies/policy_bypass_auditor.rb b/ee/lib/security/scan_result_policies/policy_bypass_auditor.rb new file mode 100644 index 00000000000000..a858bb6c5a8977 --- /dev/null +++ b/ee/lib/security/scan_result_policies/policy_bypass_auditor.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class PolicyBypassAuditor + def initialize(security_policy:, project:, user:, branch_name:) + @security_policy = security_policy + @project = project + @user = user + @branch_name = branch_name + end + + def log_access_token_bypass(token_ids) + audit_details = build_access_token_audit_details(token_ids) + log_bypass_event(:access_token, token_ids, audit_details) + end + + def log_service_account_bypass(service_account_id) + audit_details = build_service_account_audit_details(service_account_id) + log_bypass_event(:service_account, service_account_id, audit_details) + end + + def log_user_bypass(user_bypass_scope, reason) + audit_details = build_user_audit_details(user_bypass_scope, reason) + log_bypass_event(:user, user.id, audit_details, reason) + end + + private + + attr_reader :security_policy, :project, :user, :branch_name + + def log_bypass_event(bypass_type, identifier, additional_details, reason = nil) + message = bypass_audit_message(bypass_type, identifier, reason) + + Gitlab::Audit::Auditor.audit( + name: "security_policy_#{bypass_type}_push_bypass", + author: user, + scope: security_policy.security_policy_management_project, + target: security_policy, + message: message, + additional_details: { + project_id: project.id, + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name + }.merge(additional_details) + ) + end + + def build_access_token_audit_details(token_ids) + { + bypass_type: :access_token, + access_token_ids: token_ids + } + end + + def build_service_account_audit_details(service_account_id) + { + bypass_type: :service_account, + service_account_id: service_account_id + } + end + + def build_user_audit_details(user_bypass_scope, reason) + { + user_id: user.id, + reason: reason, + bypass_type: user_bypass_scope + }.merge(bypass_scope_details(user_bypass_scope)) + end + + def bypass_scope_details(user_bypass_scope) + case user_bypass_scope + when :group + { group_ids: security_policy.bypass_settings.group_ids } + when :role + { + default_roles: security_policy.bypass_settings.default_roles, + custom_role_ids: security_policy.bypass_settings.custom_role_ids + } + else + {} + end + end + + def bypass_audit_message(type, identifier, reason = nil) + message = <<~MSG.squish + Branch push restriction on '#{branch_name}' for project '#{project.full_path}' + has been bypassed by #{type} with ID: #{identifier} + MSG + + message += " with reason: #{reason}" if reason.present? + message + end + end + end +end diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb index 7243f5873408b1..9a6589735bce3c 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -5,6 +5,8 @@ module ScanResultPolicies class PolicyBypassChecker include Gitlab::Utils::StrongMemoize + BypassReasonRequiredError = Class.new(StandardError) + def initialize(security_policy:, project:, user_access:, branch_name:, push_options:) @security_policy = security_policy @project = project @@ -16,9 +18,23 @@ def initialize(security_policy:, project:, user_access:, branch_name:, push_opti def bypass_allowed? return false unless user - bypass_with_access_token? || bypass_with_service_account? + bypass_with_access_token? || bypass_with_service_account? || bypass_with_user? end + private + + attr_reader :security_policy, :project, :user, :branch_name, :push_options + + def audit_logger + PolicyBypassAuditor.new( + security_policy: security_policy, + project: project, + user: user, + branch_name: branch_name + ) + end + strong_memoize_attr :audit_logger + def bypass_with_access_token? policy_token_ids = security_policy.bypass_settings.access_token_ids return false if policy_token_ids.blank? @@ -28,7 +44,7 @@ def bypass_with_access_token? user_token_ids = user.personal_access_tokens.active.id_in(policy_token_ids).pluck_primary_key return false if user_token_ids.blank? - log_bypass_audit!(:access_token, user_token_ids) + audit_logger.log_access_token_bypass(user_token_ids) true end @@ -39,34 +55,34 @@ def bypass_with_service_account? return false unless user.service_account? return false unless policy_service_account_ids.include?(user.id) - log_bypass_audit!(:service_account, user.id) + audit_logger.log_service_account_bypass(user.id) true end - private + def bypass_with_user? + return false if Feature.disabled?(:security_policies_bypass_options_group_roles, project) - attr_reader :security_policy, :project, :user, :branch_name, :push_options + user_bypass_scope = UserBypassChecker.new( + security_policy: security_policy, project: project, current_user: user + ).bypass_scope - def log_bypass_audit!(type, id) - message = <<~MSG.squish - Branch push restriction on '#{branch_name}' for project '#{project.full_path}' - has been bypassed by #{type} with ID: #{id} - MSG - - Gitlab::Audit::Auditor.audit( - name: "security_policy_#{type}_push_bypass", - author: user, - scope: security_policy.security_policy_management_project, - target: security_policy, - message: message, - additional_details: { - project_id: project.id, - security_policy_name: security_policy.name, - security_policy_id: security_policy.id, - branch_name: branch_name - } - ) + return false unless user_bypass_scope + + reason = reason_from_push_options + raise BypassReasonRequiredError, "Bypass reason is required for user bypass" if reason.blank? + + audit_logger.log_user_bypass(user_bypass_scope, reason) + true + end + + def reason_from_push_options + return if push_options.nil? + + reason = push_options.get(:security_policy)&.dig(:bypass_reason) + + Sanitize.clean(reason) if reason.present? end + strong_memoize_attr :reason_from_push_options end end end diff --git a/ee/lib/security/scan_result_policies/push_bypass_checker.rb b/ee/lib/security/scan_result_policies/push_bypass_checker.rb index 77a828e9ad3024..a9b53035f8fcc7 100644 --- a/ee/lib/security/scan_result_policies/push_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/push_bypass_checker.rb @@ -33,6 +33,8 @@ def bypass_allowed?(policy) branch_name: branch_name, push_options: push_options ).bypass_allowed? + rescue Security::ScanResultPolicies::PolicyBypassChecker::BypassReasonRequiredError + raise end end end diff --git a/ee/lib/security/scan_result_policies/user_bypass_checker.rb b/ee/lib/security/scan_result_policies/user_bypass_checker.rb new file mode 100644 index 00000000000000..afe8ebf8150c0b --- /dev/null +++ b/ee/lib/security/scan_result_policies/user_bypass_checker.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class UserBypassChecker + def initialize(security_policy:, project:, current_user:) + @security_policy = security_policy + @project = project + @current_user = current_user + end + + def bypass_scope + return unless current_user + + if users_can_bypass? + :user + elsif roles_can_bypass? + :role + elsif groups_can_bypass? + :group + end + end + + private + + attr_reader :security_policy, :project, :current_user + + def users_can_bypass? + return false if current_user.project_bot? || current_user.service_account? + + security_policy.bypass_settings.user_ids.include?(current_user.id) + end + + def groups_can_bypass? + group_ids = security_policy.bypass_settings.group_ids + return false if group_ids.blank? + + GroupMember.direct_member_of_groups?(group_ids, current_user) + end + + def roles_can_bypass? + default_roles = security_policy.bypass_settings.default_roles + custom_role_ids = security_policy.bypass_settings.custom_role_ids + + return false if default_roles.blank? && custom_role_ids.blank? + + levels = default_roles.filter_map { |role| Gitlab::Access.sym_options[role.to_sym] } + + project.team.user_exists_with_access_level_or_custom_roles?( + current_user, levels: levels, member_role_ids: custom_role_ids + ) + end + end + end +end diff --git a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb index 67e26186e5dd4c..ede7b3c53dded1 100644 --- a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb @@ -129,6 +129,26 @@ ) end end + + context 'when bypass is allowed but no bypass_reason is provided' do + let_it_be(:regular_user) { create(:user) } + let_it_be(:user_access) { Gitlab::UserAccess.new(regular_user, container: project) } + + before do + # Create a policy that allows user bypass but requires a reason + create(:security_policy, linked_projects: [project], content: { + bypass_settings: { + users: [{ id: regular_user.id }] + } + }) + end + + it 'raises bypass reason error' do + expect { policy_check! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, described_class::BYPASS_REASON_ERROR_MESSAGE + ) + end + end end end end diff --git a/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb index 51b9d87079770f..1cb3a66015eef1 100644 --- a/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb @@ -26,8 +26,8 @@ context 'when access_tokens is nil' do let(:bypass_settings) { {} } - it 'returns nil' do - expect(bypass_settings_instance.access_token_ids).to be_nil + it 'returns empty array' do + expect(bypass_settings_instance.access_token_ids).to be_empty end end end @@ -40,8 +40,8 @@ context 'when service_accounts is nil' do let(:bypass_settings) { {} } - it 'returns nil' do - expect(bypass_settings_instance.service_account_ids).to be_nil + it 'returns empty array' do + expect(bypass_settings_instance.service_account_ids).to be_empty end end end @@ -64,7 +64,7 @@ context 'when branches are nil or missing' do let(:bypass_settings) { {} } - it 'returns an empty array' do + it 'returns empty array' do expect(bypass_settings_instance.branches).to be_empty end end @@ -82,8 +82,8 @@ context 'when users is nil or missing' do let(:bypass_settings) { {} } - it 'returns nil' do - expect(bypass_settings_instance.user_ids).to be_nil + it 'returns empty array' do + expect(bypass_settings_instance.user_ids).to be_empty end end end @@ -100,8 +100,8 @@ context 'when groups is nil or missing' do let(:bypass_settings) { {} } - it 'returns nil' do - expect(bypass_settings_instance.group_ids).to be_nil + it 'returns empty array' do + expect(bypass_settings_instance.group_ids).to be_empty end end end @@ -118,7 +118,7 @@ context 'when roles is nil or missing' do let(:bypass_settings) { {} } - it 'returns an empty array' do + it 'returns empty array' do expect(bypass_settings_instance.default_roles).to eq([]) end end @@ -136,8 +136,8 @@ context 'when custom_roles is nil or missing' do let(:bypass_settings) { {} } - it 'returns nil' do - expect(bypass_settings_instance.custom_role_ids).to be_nil + it 'returns empty array' do + expect(bypass_settings_instance.custom_role_ids).to be_empty end end end diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_auditor_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_auditor_spec.rb new file mode 100644 index 00000000000000..4d9a8ccb4e8b1f --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_auditor_spec.rb @@ -0,0 +1,242 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::PolicyBypassAuditor, feature_category: :security_policy_management do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:branch_name) { 'main' } + let_it_be(:security_policy) { create(:security_policy, linked_projects: [project]) } + + let(:auditor) do + described_class.new( + security_policy: security_policy, + project: project, + user: user, + branch_name: branch_name + ) + end + + before do + allow(Gitlab::Audit::Auditor).to receive(:audit) + end + + describe '#log_access_token_bypass' do + let(:token_ids) { [123, 456] } + + it 'logs the bypass event with correct details' do + auditor.log_access_token_bypass(token_ids) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + name: 'security_policy_access_token_push_bypass', + author: user, + scope: security_policy.security_policy_management_project, + target: security_policy, + message: "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by access_token with ID: #{token_ids}", + additional_details: { + project_id: project.id, + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name, + bypass_type: :access_token, + access_token_ids: token_ids + } + ) + end + + context 'with single token ID' do + let(:token_ids) { [789] } + + it 'logs the bypass event with single token ID' do + auditor.log_access_token_bypass(token_ids) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + message: "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by access_token with ID: #{token_ids}", + additional_details: hash_including( + access_token_ids: token_ids + ) + ) + ) + end + end + + context 'with empty token IDs' do + let(:token_ids) { [] } + + it 'logs the bypass event with empty token IDs' do + auditor.log_access_token_bypass(token_ids) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + additional_details: hash_including( + access_token_ids: [] + ) + ) + ) + end + end + end + + describe '#log_service_account_bypass' do + let(:service_account_id) { 789 } + + it 'logs the bypass event with correct details' do + auditor.log_service_account_bypass(service_account_id) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + name: 'security_policy_service_account_push_bypass', + author: user, + scope: security_policy.security_policy_management_project, + target: security_policy, + message: "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by service_account with ID: #{service_account_id}", + additional_details: { + project_id: project.id, + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name, + bypass_type: :service_account, + service_account_id: service_account_id + } + ) + end + + context 'with different service account ID' do + let(:service_account_id) { 999 } + + it 'logs the bypass event with different service account ID' do + auditor.log_service_account_bypass(service_account_id) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + message: "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by service_account with ID: #{service_account_id}", + additional_details: hash_including( + service_account_id: service_account_id + ) + ) + ) + end + end + end + + describe '#log_user_bypass' do + let(:reason) { 'Emergency security fix' } + + context 'with group bypass scope' do + let(:user_bypass_scope) { :group } + let(:group_ids) { [1, 2, 3] } + + before do + allow(security_policy.bypass_settings).to receive(:group_ids).and_return(group_ids) + end + + it 'logs the bypass event with group details' do + auditor.log_user_bypass(user_bypass_scope, reason) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + name: 'security_policy_user_push_bypass', + author: user, + scope: security_policy.security_policy_management_project, + target: security_policy, + message: "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by user with ID: #{user.id} with reason: #{reason}", + additional_details: { + project_id: project.id, + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name, + user_id: user.id, + reason: reason, + bypass_type: :group, + group_ids: group_ids + } + ) + end + end + + context 'with role bypass scope' do + let(:user_bypass_scope) { :role } + let(:default_roles) { %w[MAINTAINER DEVELOPER] } + let(:custom_role_ids) { [4, 5, 6] } + + before do + allow(security_policy.bypass_settings).to receive_messages(default_roles: default_roles, + custom_role_ids: custom_role_ids) + end + + it 'logs the bypass event with role details' do + auditor.log_user_bypass(user_bypass_scope, reason) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + name: 'security_policy_user_push_bypass', + author: user, + scope: security_policy.security_policy_management_project, + target: security_policy, + message: "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by user with ID: #{user.id} with reason: #{reason}", + additional_details: { + project_id: project.id, + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name, + user_id: user.id, + reason: reason, + bypass_type: :role, + default_roles: default_roles, + custom_role_ids: custom_role_ids + } + ) + end + end + + context 'without reason' do + let(:user_bypass_scope) { :group } + let(:reason) { nil } + + before do + allow(security_policy.bypass_settings).to receive(:group_ids).and_return([1]) + end + + it 'logs the bypass event without reason in message' do + auditor.log_user_bypass(user_bypass_scope, reason) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + message: "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by user with ID: #{user.id}", + additional_details: hash_including( + reason: nil + ) + ) + ) + end + end + + context 'with empty reason' do + let(:user_bypass_scope) { :group } + let(:reason) { '' } + + before do + allow(security_policy.bypass_settings).to receive(:group_ids).and_return([1]) + end + + it 'logs the bypass event without reason in message' do + auditor.log_user_bypass(user_bypass_scope, reason) + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + message: "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by user with ID: #{user.id}", + additional_details: hash_including( + reason: '' + ) + ) + ) + end + end + end +end diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb index fd6272242562bb..ae091c8af4b306 100644 --- a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -7,13 +7,17 @@ let_it_be(:branch_name) { 'main' } let_it_be(:user) { create(:user, :project_bot) } let_it_be(:service_account) { create(:service_account) } + let_it_be(:normal_user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:custom_role) { create(:member_role, namespace: project.group) } - let_it_be_with_refind(:security_policy) do + let(:security_policy) do create(:security_policy, linked_projects: [project], content: { bypass_settings: {} }) end let_it_be(:service_account_access) { Gitlab::UserAccess.new(service_account, container: project) } let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } + let_it_be(:normal_user_access) { Gitlab::UserAccess.new(normal_user, container: project) } describe '#bypass_allowed?' do subject(:bypass_allowed?) do @@ -22,10 +26,12 @@ project: project, user_access: user_access, branch_name: branch_name, - push_options: {} + push_options: push_options ).bypass_allowed? end + let(:push_options) { Gitlab::PushOptions.new([]) } + before do allow(Gitlab::Audit::Auditor).to receive(:audit).and_call_original end @@ -82,6 +88,8 @@ "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ "has been bypassed by access_token with ID: [#{personal_access_token.id}]", additional_details: hash_including( + bypass_type: :access_token, + access_token_ids: [personal_access_token.id], security_policy_name: security_policy.name, security_policy_id: security_policy.id, branch_name: branch_name @@ -130,6 +138,8 @@ "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ "has been bypassed by service_account with ID: #{service_account.id}", additional_details: hash_including( + bypass_type: :service_account, + service_account_id: service_account.id, security_policy_name: security_policy.name, security_policy_id: security_policy.id, branch_name: branch_name @@ -153,5 +163,195 @@ it_behaves_like 'bypass is not allowed and audit log is not created' end end + + context 'when bypass_settings has users' do + let_it_be(:user_access) { normal_user_access } + + before do + security_policy.update!(content: { bypass_settings: { users: [{ id: normal_user.id }] } }) + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(security_policies_bypass_options_group_roles: false) + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when user is a project bot' do + let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when user is a service account' do + let_it_be(:user_access) { service_account_access } + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when user is not in the allowed users list' do + before do + security_policy.update!(content: { bypass_settings: { users: [{ id: create(:user).id }] } }) + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when user is in the allowed users list but no bypass reason provided' do + it 'raises BypassReasonRequiredError' do + expect { bypass_allowed? }.to raise_error( + Security::ScanResultPolicies::PolicyBypassChecker::BypassReasonRequiredError, + 'Bypass reason is required for user bypass' + ) + end + end + + context 'when user is in the allowed users list and bypass reason is provided' do + let(:push_options) { Gitlab::PushOptions.new(['security_policy.bypass_reason=Emergency fix']) } + + it 'returns true and creates an audit log' do + result = bypass_allowed? + + expect(result).to be true + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(hash_including( + name: 'security_policy_user_push_bypass', + author: normal_user, + scope: security_policy.security_policy_management_project, + target: security_policy, + message: + "Branch push restriction on '#{branch_name}' for project '#{project.full_path}' " \ + "has been bypassed by user with ID: #{normal_user.id} with reason: Emergency fix", + additional_details: hash_including( + bypass_type: :user, + user_id: normal_user.id, + reason: 'Emergency fix', + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name + ) + )) + end + end + end + + context 'when bypass_settings has groups' do + let_it_be(:user_access) { normal_user_access } + + before do + group.add_member(normal_user, Gitlab::Access::DEVELOPER) + security_policy.update!(content: { bypass_settings: { groups: [{ id: group.id }] } }) + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(security_policies_bypass_options_group_roles: false) + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when user is not a member of the allowed group' do + before do + group.members.find_by(user: normal_user).destroy! + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when user is a member of the allowed group but no bypass reason provided' do + it 'raises BypassReasonRequiredError' do + expect { bypass_allowed? }.to raise_error( + Security::ScanResultPolicies::PolicyBypassChecker::BypassReasonRequiredError, + 'Bypass reason is required for user bypass' + ) + end + end + + context 'when user is a member of the allowed group and bypass reason is provided' do + let(:push_options) { Gitlab::PushOptions.new(['security_policy.bypass_reason=Critical security update']) } + + it 'returns true and creates an audit log' do + result = bypass_allowed? + + expect(result).to be true + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + name: 'security_policy_user_push_bypass', + author: normal_user, + additional_details: hash_including( + bypass_type: :group, + group_ids: [group.id], + user_id: normal_user.id, + reason: 'Critical security update', + event_name: 'security_policy_user_push_bypass' + ) + ) + ).at_least(:once) + end + end + end + + context 'when bypass_settings has both default roles and custom roles' do + let_it_be(:user_access) { normal_user_access } + + before do + project.add_member(normal_user, Gitlab::Access::MAINTAINER) + member = project.members.find_by(user: normal_user) + member.update!(member_role: custom_role) + security_policy.update!(content: { + bypass_settings: { + default_roles: ['MAINTAINER'], + custom_roles: [{ id: custom_role.id }] + } + }) + end + + context 'when bypass reason is provided' do + let(:push_options) { Gitlab::PushOptions.new(['security_policy.bypass_reason=Multiple role bypass']) } + + it 'returns true and creates an audit log' do + result = bypass_allowed? + + expect(result).to be true + expect(Gitlab::Audit::Auditor).to have_received(:audit).with( + hash_including( + name: 'security_policy_user_push_bypass', + author: normal_user, + additional_details: hash_including( + bypass_type: :role, + default_roles: [], + custom_role_ids: [custom_role.id], + reason: 'Multiple role bypass', + event_name: 'security_policy_user_push_bypass' + ) + ) + ).at_least(:once) + end + end + end + + context 'when bypass reason contains HTML' do + let_it_be(:user_access) { normal_user_access } + let(:push_options) do + Gitlab::PushOptions.new(['security_policy.bypass_reason=Emergency fix']) + end + + before do + security_policy.update!(content: { bypass_settings: { users: [{ id: normal_user.id }] } }) + end + + it 'sanitizes the bypass reason' do + result = bypass_allowed? + + expect(result).to be true + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(hash_including( + additional_details: hash_including( + reason: 'Emergency fix' + ) + )).at_least(:once) + end + end end end diff --git a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb index 7a1802d41e2e37..83204bdfd86b0f 100644 --- a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb @@ -13,7 +13,7 @@ project: project, user_access: user_access, branch_name: branch_name, - push_options: {} + push_options: Gitlab::PushOptions.new([]) ) end diff --git a/ee/spec/lib/security/scan_result_policies/user_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/user_bypass_checker_spec.rb new file mode 100644 index 00000000000000..babdc0ee29fcec --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/user_bypass_checker_spec.rb @@ -0,0 +1,251 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::UserBypassChecker, feature_category: :security_policy_management do + let_it_be(:project) { create(:project) } + let_it_be(:normal_user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:custom_role) { create(:member_role, namespace: project.group) } + + let(:security_policy) do + create(:security_policy, linked_projects: [project], content: { bypass_settings: {} }) + end + + let(:checker) do + described_class.new( + security_policy: security_policy, + project: project, + current_user: normal_user + ) + end + + describe '#bypass_scope' do + subject(:bypass_scope) { checker.bypass_scope } + + context 'when user is nil' do + let(:normal_user) { nil } + + it 'returns nil' do + expect(bypass_scope).to be_nil + end + end + + context 'when no bypass methods return true' do + it 'returns nil' do + expect(bypass_scope).to be_nil + end + end + + context 'when users_can_bypass? returns true' do + before do + security_policy.update!(content: { bypass_settings: { users: [{ id: normal_user.id }] } }) + end + + it 'returns :user' do + expect(bypass_scope).to eq(:user) + end + end + + context 'when roles_can_bypass? returns true' do + before do + create(:project_member, :maintainer, project: project, user: normal_user) + security_policy.update!(content: { bypass_settings: { roles: ['maintainer'] } }) + end + + it 'returns :role' do + expect(bypass_scope).to eq(:role) + end + end + + context 'when groups_can_bypass? returns true' do + before do + group.add_member(normal_user, Gitlab::Access::DEVELOPER) + security_policy.update!(content: { bypass_settings: { groups: [{ id: group.id }] } }) + end + + it 'returns :group' do + expect(bypass_scope).to eq(:group) + end + end + + context 'when multiple bypass methods return true' do + before do + security_policy.update!(content: { + bypass_settings: { + users: [{ id: normal_user.id }], + groups: [{ id: group.id }] + } + }) + group.add_member(normal_user, Gitlab::Access::DEVELOPER) + end + + it 'prioritizes user bypass' do + expect(bypass_scope).to eq(:user) + end + end + end + + describe '#users_can_bypass?' do + subject(:users_can_bypass?) { checker.send(:users_can_bypass?) } + + context 'when user is a project bot' do + let(:normal_user) { create(:user, :project_bot) } + + before do + security_policy.update!(content: { bypass_settings: { users: [{ id: normal_user.id }] } }) + end + + it 'returns false' do + expect(users_can_bypass?).to be false + end + end + + context 'when user is a service account' do + let(:normal_user) { create(:service_account) } + + before do + security_policy.update!(content: { bypass_settings: { users: [{ id: normal_user.id }] } }) + end + + it 'returns false' do + expect(users_can_bypass?).to be false + end + end + + context 'when user_ids is blank' do + it 'returns false' do + expect(users_can_bypass?).to be false + end + end + + context 'when user is not in the allowed users list' do + before do + security_policy.update!(content: { bypass_settings: { users: [{ id: create(:user).id }] } }) + end + + it 'returns false' do + expect(users_can_bypass?).to be false + end + end + + context 'when user is in the allowed users list' do + before do + security_policy.update!(content: { bypass_settings: { users: [{ id: normal_user.id }] } }) + end + + it 'returns true' do + expect(users_can_bypass?).to be true + end + end + end + + describe '#groups_can_bypass?' do + subject(:groups_can_bypass?) { checker.send(:groups_can_bypass?) } + + context 'when group_ids is blank' do + it 'returns false' do + expect(groups_can_bypass?).to be false + end + end + + context 'when user is not a member of the allowed group' do + before do + security_policy.update!(content: { bypass_settings: { groups: [{ id: group.id }] } }) + end + + it 'returns false' do + expect(groups_can_bypass?).to be false + end + end + + context 'when user is a member of the allowed group' do + before do + group.add_member(normal_user, Gitlab::Access::DEVELOPER) + security_policy.update!(content: { bypass_settings: { groups: [{ id: group.id }] } }) + end + + it 'returns true' do + expect(groups_can_bypass?).to be true + end + end + end + + describe '#roles_can_bypass?' do + subject(:roles_can_bypass?) { checker.send(:roles_can_bypass?) } + + context 'when both default_roles and custom_role_ids are blank' do + it 'returns false' do + expect(roles_can_bypass?).to be false + end + end + + context 'when default_roles is provided' do + before do + security_policy.update!(content: { bypass_settings: { roles: ['maintainer'] } }) + end + + context 'when user has the required role' do + before do + create(:project_member, :maintainer, project: project, user: normal_user) + end + + it 'returns true' do + expect(roles_can_bypass?).to be true + end + end + + context 'when user does not have the required role' do + it 'returns false' do + expect(roles_can_bypass?).to be false + end + end + end + + context 'when custom_role_ids is provided' do + before do + security_policy.update!(content: { bypass_settings: { custom_roles: [{ id: custom_role.id }] } }) + end + + context 'when user has the required custom role' do + before do + create(:project_member, :developer, project: project, user: normal_user) + member = project.members.find_by(user: normal_user) + member.update!(member_role: custom_role) + end + + it 'returns true' do + expect(roles_can_bypass?).to be true + end + end + + context 'when user does not have the required custom role' do + before do + create(:project_member, :developer, project: project, user: normal_user) + end + + it 'returns false' do + expect(roles_can_bypass?).to be false + end + end + end + + context 'when both default_roles and custom_role_ids are provided' do + before do + create(:project_member, :maintainer, project: project, user: normal_user) + member = project.members.find_by(user: normal_user) + member.update!(member_role: custom_role) + security_policy.update!(content: { + bypass_settings: { + roles: ['maintainer'], + custom_roles: [{ id: custom_role.id }] + } + }) + end + + it 'returns true' do + expect(roles_can_bypass?).to be true + end + end + end +end diff --git a/ee/spec/models/ee/group_member_spec.rb b/ee/spec/models/ee/group_member_spec.rb index fb80ab7ee293fa..8ce2ff541aa25d 100644 --- a/ee/spec/models/ee/group_member_spec.rb +++ b/ee/spec/models/ee/group_member_spec.rb @@ -185,6 +185,95 @@ end end + describe '.direct_member_of_groups?' do + let_it_be(:group1) { create(:group) } + let_it_be(:group2) { create(:group) } + let_it_be(:group3) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:other_user) { create(:user) } + + context 'when user is a direct member of the specified groups' do + before do + group1.add_developer(user) + group2.add_maintainer(user) + end + + it 'returns true for single group' do + expect(described_class.direct_member_of_groups?([group1.id], user)).to be true + end + + it 'returns true for multiple groups where user is member of all' do + expect(described_class.direct_member_of_groups?([group1.id, group2.id], user)).to be true + end + + it 'returns true for multiple groups where user is member of some' do + expect(described_class.direct_member_of_groups?([group1.id, group3.id], user)).to be true + end + + it 'returns false for groups where user is not a member' do + expect(described_class.direct_member_of_groups?([group3.id], user)).to be false + end + + it 'returns false for other user' do + expect(described_class.direct_member_of_groups?([group1.id, group2.id], other_user)).to be false + end + end + + context 'when user has minimal access level' do + before do + stub_licensed_features(minimal_access_role: true) + group1.add_member(user, Gitlab::Access::MINIMAL_ACCESS) + end + + it 'returns false as minimal access is excluded' do + expect(described_class.direct_member_of_groups?([group1.id], user)).to be false + end + end + + context 'when user has pending invite' do + before do + create(:group_member, :invited, group: group1, user: nil, invite_email: user.email) + end + + it 'returns false as invites are excluded' do + expect(described_class.direct_member_of_groups?([group1.id], user)).to be false + end + end + + context 'when user has pending request' do + before do + create(:group_member, :awaiting, group: group1, user: user) + end + + it 'returns false as requests are excluded' do + expect(described_class.direct_member_of_groups?([group1.id], user)).to be false + end + end + + context 'when user is blocked' do + before do + user.update!(state: :blocked) + group1.add_developer(user) + end + + it 'returns false as blocked users are excluded' do + expect(described_class.direct_member_of_groups?([group1.id], user)).to be false + end + end + + context 'with empty group_ids' do + it 'returns false' do + expect(described_class.direct_member_of_groups?([], user)).to be false + end + end + + context 'with nil group_ids' do + it 'returns false' do + expect(described_class.direct_member_of_groups?(nil, user)).to be false + end + end + end + describe '.filter_by_enterprise_users' do let_it_be(:group) { create(:group) } let_it_be(:enterprise_user_member_1_of_group) { group.add_developer(create(:user, enterprise_group_id: group.id)) } diff --git a/ee/spec/models/ee/project_team_spec.rb b/ee/spec/models/ee/project_team_spec.rb index c76c39740ea036..8e9fb579a855c1 100644 --- a/ee/spec/models/ee/project_team_spec.rb +++ b/ee/spec/models/ee/project_team_spec.rb @@ -122,4 +122,205 @@ it { is_expected.to be_empty } end end + + describe '#user_exists_with_access_level_or_custom_roles?' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:custom_role) { create(:member_role) } + let_it_be(:another_custom_role) { create(:member_role) } + + let_it_be(:developer) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:non_member) { create(:user) } + + before do + create(:project_member, :developer, project: project, user: developer, member_role: custom_role) + create(:project_member, :maintainer, project: project, user: maintainer) + create(:project_member, :reporter, project: project, user: reporter) + create(:project_member, :guest, project: project, user: guest) + end + + subject(:user_exists) do + project.team.user_exists_with_access_level_or_custom_roles?(user, levels: levels, + member_role_ids: member_role_ids) + end + + context 'when no parameters are provided' do + let(:levels) { [] } + let(:member_role_ids) { [] } + let(:user) { developer } + + it 'returns false' do + expect(user_exists).to be false + end + end + + context 'when filtering by access level' do + let(:levels) { [Gitlab::Access::MAINTAINER] } + let(:member_role_ids) { [] } + + context 'when user has the specified access level' do + let(:user) { maintainer } + + it 'returns true' do + expect(user_exists).to be true + end + end + + context 'when user does not have the specified access level' do + let(:user) { developer } + + it 'returns false' do + expect(user_exists).to be false + end + end + + context 'when user is not a member of the project' do + let(:user) { non_member } + + it 'returns false' do + expect(user_exists).to be false + end + end + + context 'when filtering by multiple access levels' do + let(:levels) { [Gitlab::Access::MAINTAINER, Gitlab::Access::REPORTER] } + let(:member_role_ids) { [] } + + context 'when user has one of the specified access levels' do + let(:user) { maintainer } + + it 'returns true' do + expect(user_exists).to be true + end + end + + context 'when user has another of the specified access levels' do + let(:user) { reporter } + + it 'returns true' do + expect(user_exists).to be true + end + end + + context 'when user does not have any of the specified access levels' do + let(:user) { guest } + + it 'returns false' do + expect(user_exists).to be false + end + end + end + end + + context 'when filtering by custom roles' do + let(:levels) { [] } + let(:member_role_ids) { [custom_role.id] } + + context 'when user has the specified custom role' do + let(:user) { developer } + + it 'returns true' do + expect(user_exists).to be true + end + end + + context 'when user does not have the specified custom role' do + let(:user) { maintainer } + + it 'returns false' do + expect(user_exists).to be false + end + end + + context 'when user is not a member of the project' do + let(:user) { non_member } + + it 'returns false' do + expect(user_exists).to be false + end + end + + context 'when filtering by multiple custom roles' do + let(:member_role_ids) { [custom_role.id, another_custom_role.id] } + + context 'when user has one of the specified custom roles' do + let(:user) { developer } + + it 'returns true' do + expect(user_exists).to be true + end + end + + context 'when user does not have any of the specified custom roles' do + let(:user) { maintainer } + + it 'returns false' do + expect(user_exists).to be false + end + end + end + + context 'when filtering with non-existent custom role' do + let(:member_role_ids) { [non_existing_record_id] } + + context 'when user is a member' do + let(:user) { developer } + + it 'returns false' do + expect(user_exists).to be false + end + end + end + end + + context 'when filtering by both access level and custom roles' do + let(:levels) { [Gitlab::Access::MAINTAINER] } + let(:member_role_ids) { [custom_role.id] } + + context 'when user has the specified access level' do + let(:user) { maintainer } + + it 'returns true' do + expect(user_exists).to be true + end + end + + context 'when user has the specified custom role' do + let(:user) { developer } + + it 'returns true' do + expect(user_exists).to be true + end + end + + context 'when user has neither the access level nor the custom role' do + let(:user) { guest } + + it 'returns false' do + expect(user_exists).to be false + end + end + + context 'when user is not a member of the project' do + let(:user) { non_member } + + it 'returns false' do + expect(user_exists).to be false + end + end + end + + context 'when user is nil' do + let(:levels) { [Gitlab::Access::MAINTAINER] } + let(:member_role_ids) { [] } + let(:user) { nil } + + it 'returns false' do + expect(user_exists).to be false + end + end + end end diff --git a/ee/spec/models/security/policy_spec.rb b/ee/spec/models/security/policy_spec.rb index 2da873559588a0..12c8f64961e7a8 100644 --- a/ee/spec/models/security/policy_spec.rb +++ b/ee/spec/models/security/policy_spec.rb @@ -1338,18 +1338,18 @@ context 'when bypass_settings is nil' do let(:policy) { build(:security_policy, content: {}) } - it 'returns a BypassSettings object with nil ids' do - expect(policy.bypass_settings.access_token_ids).to be_nil - expect(policy.bypass_settings.service_account_ids).to be_nil + it 'returns a BypassSettings object with empty arrays' do + expect(policy.bypass_settings.access_token_ids).to be_empty + expect(policy.bypass_settings.service_account_ids).to be_empty end end context 'when bypass_settings is empty' do let(:policy) { build(:security_policy, content: { bypass_settings: {} }) } - it 'returns a BypassSettings object with nil ids' do - expect(policy.bypass_settings.access_token_ids).to be_nil - expect(policy.bypass_settings.service_account_ids).to be_nil + it 'returns a BypassSettings object with empty arrays' do + expect(policy.bypass_settings.access_token_ids).to be_empty + expect(policy.bypass_settings.service_account_ids).to be_empty end end -- GitLab