diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 8e00ba8bfe082c12d52f2ac9af688663835178a1..7a5ba63f98c2463f01fea1beb9a61b252df88ecd 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -587,6 +587,7 @@ Audit event types belong to the following product categories. | [`authenticated_with_group_saml`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28575) | Successfully signing in with SAML authentication | **{check-circle}** Yes | GitLab [12.10](https://gitlab.com/gitlab-org/gitlab/-/issues/35710) | Group | | [`ban_user`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116103) | A user is banned, unbanned, blocked, or unblocked | **{check-circle}** Yes | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/377620) | User | | [`change_membership_state`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87924) | A user's membership is updated | **{check-circle}** Yes | GitLab [15.1](https://gitlab.com/gitlab-org/gitlab/-/issues/362200) | Group | +| [`inactive_scim_user_cannot_be_added`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173879) | A user cannot be added to a group during SAML authentication when their SCIM identity is inactive | **{check-circle}** Yes | GitLab [17.7](https://gitlab.com/gitlab-org/gitlab/-/issues/422582) | Group | | [`password_reset_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129079) | A password reset fails for a user | **{dotted-circle}** No | GitLab [16.4](https://gitlab.com/gitlab-org/gitlab/-/issues/377762) | User | | [`unban_user`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116221) | A user is unbanned | **{check-circle}** Yes | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/377620) | User | | [`unblock_user`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115727) | A user is banned | **{check-circle}** Yes | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/377620) | User | diff --git a/ee/config/audit_events/types/inactive_scim_user_cannot_be_added.yml b/ee/config/audit_events/types/inactive_scim_user_cannot_be_added.yml new file mode 100644 index 0000000000000000000000000000000000000000..ec12853f5613ed327e9e51adc07bb151dc04a19c --- /dev/null +++ b/ee/config/audit_events/types/inactive_scim_user_cannot_be_added.yml @@ -0,0 +1,10 @@ +--- +name: inactive_scim_user_cannot_be_added +description: A user cannot be added to a group during SAML authentication when their SCIM identity is inactive +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/422582 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173879 +feature_category: user_management +milestone: '17.7' +saved_to_database: true +streamed: true +scope: [Group] diff --git a/ee/lib/gitlab/auth/group_saml/membership_enforcer.rb b/ee/lib/gitlab/auth/group_saml/membership_enforcer.rb index 37b4e34ea4ee6516cde479915e508d56c2aab5f2..bb0ca4f4ba882495ae8e7e3a9b3a9caf700b0d4e 100644 --- a/ee/lib/gitlab/auth/group_saml/membership_enforcer.rb +++ b/ee/lib/gitlab/auth/group_saml/membership_enforcer.rb @@ -12,7 +12,11 @@ def can_add_user?(user) return true unless root_group.saml_provider&.enforced_sso? return true if user.project_bot? || user.security_policy_bot? return true if user.service_account? && user_provisioned_by_group?(user) - return false if inactive_scim_identity_for_group?(user) + + if inactive_scim_identity_for_group?(user) + log_audit_event(user, root_group) + return false + end GroupSamlIdentityFinder.new(user: user).find_linked(group: root_group) end @@ -32,6 +36,18 @@ def inactive_scim_identity_for_group?(user) def user_provisioned_by_group?(user) user.provisioned_by_group_id == root_group.id end + + def log_audit_event(user, root_group) + audit_context = { + name: "inactive_scim_user_cannot_be_added", + author: user, + scope: root_group, + target: user, + target_details: user.username, + message: "User cannot be added to group due to inactive SCIM identity" + } + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/spec/lib/gitlab/auth/group_saml/membership_enforcer_spec.rb b/ee/spec/lib/gitlab/auth/group_saml/membership_enforcer_spec.rb index 9eb1facd58f92b61c2bfbbbe799ee946e8d48e17..f4a8af740e542a18b6837b99f1484b0cee43b0d1 100644 --- a/ee/spec/lib/gitlab/auth/group_saml/membership_enforcer_spec.rb +++ b/ee/spec/lib/gitlab/auth/group_saml/membership_enforcer_spec.rb @@ -20,10 +20,34 @@ expect(described_class.new(group).can_add_user?(non_saml_user)).to be_falsey end - it 'does not allow adding a user with an inactive scim identity for the group' do - create(:scim_identity, group: group, user: user, active: false) + context 'when the user has an inactive scim identity' do + before do + stub_licensed_features(extended_audit_events: true) + create(:scim_identity, group: group, user: user, active: false) + end + + it 'does not allow adding a user with an inactive scim identity for the group' do + expect(described_class.new(group).can_add_user?(user)).to be_falsey + end - expect(described_class.new(group).can_add_user?(user)).to be_falsey + it 'logs an audit event' do + expect { described_class.new(group).can_add_user?(user) }.to change { AuditEvent.count }.by(1) + + expect(AuditEvent.last).to have_attributes({ + attributes: hash_including({ + "entity_id" => group.id, + "entity_type" => "Group", + "author_id" => user.id, + "target_details" => user.username, + "target_id" => user.id + }), + details: hash_including({ + custom_message: "User cannot be added to group due to inactive SCIM identity", + target_type: "User", + target_details: user.username + }) + }) + end end it 'does allow adding a user with an active scim identity for the group' do