diff --git a/doc/topics/git/commit.md b/doc/topics/git/commit.md index c086fa0d809450982cc52e7019c55f4c69907031..365d945ad1c16a671fe02e77c85d7d2a6156a4f4 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 9123907098a4836cf5592e2c1de7f94bf01fc6aa..5966229953772592ec8e02ee42329a92d4458eb9 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 c6cc9baf6040e12c5a34c7a98f7cb57fcec598ff..05f6317b59afcc5f7e5c5b3db025abce2973f652 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 af61084968264e5f08c2797a6feb9c11ed79ea71..57f8f75a820a6bc9ab9996179369f54a8af3f8a4 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 0000000000000000000000000000000000000000..cb04af897724ff9b45e55d42ea9fdf4f051d78f8 --- /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 d97d885877fb263ecf0bdbaa97e51ba93ec21ad0..2be4266c6f641a9318ed5ac0b80057dc392527d0 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 506875953585ad9949cd596c9e01bd136a35e18e..7b2ead079981a8ece9819815e3f5d80fd22707eb 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 0000000000000000000000000000000000000000..a858bb6c5a8977a5e944d7aaa99acd7d54ad5a40 --- /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 7243f5873408b1635f23eb2524b8d36e64f6114a..9a6589735bce3ce3239d1807ec27d6bb1cf06606 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 77a828e9ad3024e8fa3dea08a293125c27505b54..a9b53035f8fcc71b6184d48f8f402467e302ce43 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 0000000000000000000000000000000000000000..afe8ebf8150c0b200f6e7a87b90ee1fd22468524 --- /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 67e26186e5dd4c9288bffaf6508a1f6d5808e508..ede7b3c53dded19d81fcb884071bc416c4a20f2c 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 51b9d87079770f408a0a817d2bb6527caf0f886f..1cb3a66015eef1341b937fd93c7517fe29cf9eab 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 0000000000000000000000000000000000000000..4d9a8ccb4e8b1f10840c19afa4d45c8b441ed07f --- /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 fd6272242562bb0a1c81c2b6ec2f907857ed1add..ae091c8af4b306b5d33ff6ec631d1813a76fe025 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 7a1802d41e2e372a68eec7d70b13ce84696ed048..83204bdfd86b0f69893ca646d7b90b33586ba86b 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 0000000000000000000000000000000000000000..babdc0ee29fcec4e754e1ba5050a2eaeebd5887f --- /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 fb80ab7ee293fab58270542fed448b844dd087f4..8ce2ff541aa25d240925bf65f471c33048702579 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 c76c39740ea0366b770932ad75924f7f0737f6f1..8e9fb579a855c15b3c80a41cd80e66b02170dc62 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 2da873559588a0ac4d0d0b312ea5584c25c3f404..12c8f64961e7a87a895e375668102a493ffb9112 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