Merge branch '8071-add-group-saml-config-changes-to-audit-events' into 'master'

Add group SAML configuration changes to group audit events

See merge request gitlab-org/gitlab!73656

doc/administration/audit_events.md

@@ -73,6 +73,17 @@ From there, you can see the following actions:
 - Group changed visibility.
 - User was added to group and with which [permissions](../user/permissions.md).
 - User sign-in via [Group SAML](../user/group/saml_sso/index.md).
+- [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/8071) in GitLab 14.5, changes to the following
+  [group SAML](../user/group/saml_sso/index.md) configuration:
+  - Enabled status.
+  - Enforcing SSO-only authentication for web activity.
+  - Enforcing SSO-only authentication for Git and Dependency Proxy activity.
+  - Enforcing users to have dedicated group-managed accounts.
+  - Prohibiting outer forks.
+  - Identity provider SSO URL.
+  - Certificate fingerprint.
+  - Default membership role.
+  - SSO-SAML group sync configuration.
 - Permissions changes of a user assigned to a group.
 - Removed user from group.
 - Project repository imported into group.

ee/app/controllers/groups/saml_group_links_controller.rb

@@ -13,6 +13,7 @@ module Groups
 
       if group_link.save
         flash[:notice] = s_('GroupSAML|New SAML group link saved.')
+        create_audit_event('saml_group_links_created', group, "SAML group links created. Group Name - #{group_link.saml_group_name}, Access Level - #{group_link.access_level}")
       else
         flash[:alert] = alert(group_link.errors.full_messages.join(', '))
       end
@@ -21,7 +22,9 @@ module Groups
     end
 
     def destroy
-      group.saml_group_links.find(params[:id]).destroy
+      saml_group_link = group.saml_group_links.find(params[:id])
+      saml_group_link.destroy
+      create_audit_event('saml_group_links_removed', group, "SAML group links removed. Group Name - #{saml_group_link.saml_group_name}")
 
       redirect_to group_saml_group_links_path(@group), status: :found, notice: s_('GroupSAML|SAML group link was successfully removed.')
     end
@@ -39,5 +42,15 @@ module Groups
     def alert(error_message)
       s_('GroupSAML|Could not create SAML group link: %{errors}.') % { errors: error_message }
     end
+
+    def create_audit_event(name, group, message)
+      ::Gitlab::Audit::Auditor.audit(
+        name: name,
+        author: current_user,
+        scope: group,
+        target: group,
+        message: message
+      )
+    end
   end
 end

ee/app/services/group_saml/saml_provider/base_service.rb

@@ -9,6 +9,10 @@ module GroupSaml
 
       delegate :group, to: :saml_provider
 
+      AUDIT_LOG_ALLOWLIST = %w[
+        enabled certificate_fingerprint sso_url enforced_sso enforced_group_managed_accounts prohibited_outer_forks default_membership_role git_check_enforced
+      ].freeze
+
       def initialize(current_user, saml_provider, params:)
         @saml_provider = saml_provider
         @current_user = current_user
@@ -25,6 +29,24 @@ module GroupSaml
             require_linked_saml_to_enable_group_managed!
           end
         end
+
+        saml_provider.previous_changes.each do |attribute, changes|
+          next unless AUDIT_LOG_ALLOWLIST.include?(attribute)
+
+          audit_context = {
+            name: audit_name,
+            author: current_user,
+            scope: saml_provider.group,
+            target: saml_provider.group,
+            message: message(attribute, changes)
+          }
+
+          ::Gitlab::Audit::Auditor.audit(audit_context)
+        end
+      end
+
+      def audit_name
+        "group_saml_provider"
       end
 
       private
@@ -40,6 +62,16 @@ module GroupSaml
 
         raise ActiveRecord::Rollback
       end
+
+      def message(attribute, changes)
+        change_text = if changes[0].nil?
+                        "#{attribute} changed to #{changes[1]}. "
+                      else
+                        "#{attribute} changed from #{changes[0]} to #{changes[1]}. "
+                      end
+
+        "Group SAML SSO configuration changed: #{change_text}"
+      end
     end
   end
 end

ee/app/services/group_saml/saml_provider/create_service.rb

@@ -9,6 +9,10 @@ module GroupSaml
         @group = group
         super(current_user, group.build_saml_provider, params: params)
       end
+
+      def audit_name
+        "#{super}_create"
+      end
     end
   end
 end

ee/app/services/group_saml/saml_provider/update_service.rb

@@ -5,6 +5,9 @@ require_dependency 'group_saml/saml_provider/base_service'
 module GroupSaml
   module SamlProvider
     class UpdateService < BaseService
+      def audit_name
+        "#{super}_update"
+      end
     end
   end
 end

ee/spec/controllers/groups/saml_group_links_controller_spec.rb

@@ -60,13 +60,25 @@ RSpec.describe Groups::SamlGroupLinksController do
       let_it_be(:saml_provider) { create(:saml_provider, group: group, enabled: true) }
 
       context 'with valid parameters' do
-        let_it_be(:params) { route_params.merge(saml_group_link: { access_level: 'Reporter', saml_group_name: generate(:saml_group_name) }) }
+        let_it_be(:saml_group_name) { generate(:saml_group_name) }
+        let_it_be(:params) { route_params.merge(saml_group_link: { access_level: 'Reporter', saml_group_name: saml_group_name }) }
 
         it 'responds with success' do
+          expect(::Gitlab::Audit::Auditor)
+            .to receive(:audit).with(
+              hash_including(
+                { name: "saml_group_links_created",
+                  author: user,
+                  scope: group,
+                  target: group,
+                  message: "SAML group links created. Group Name - #{saml_group_name}, Access Level - Reporter" })
+            ).and_call_original
+
           call_action
 
           expect(response).to have_gitlab_http_status(:found)
           expect(flash[:notice]).to include('New SAML group link saved.')
+          expect(AuditEvent.last.details[:custom_message]).to eq("SAML group links created. Group Name - #{saml_group_name}, Access Level - Reporter")
         end
 
         it 'creates the group link' do
@@ -102,10 +114,21 @@ RSpec.describe Groups::SamlGroupLinksController do
         let_it_be(:params) { route_params }
 
         it 'responds with success' do
+          expect(::Gitlab::Audit::Auditor)
+            .to receive(:audit).with(
+              hash_including(
+                { name: "saml_group_links_removed",
+                  author: user,
+                  scope: group,
+                  target: group,
+                  message: "SAML group links removed. Group Name - #{group_link.saml_group_name}" })
+            ).and_call_original
+
           call_action
 
           expect(response).to have_gitlab_http_status(:found)
           expect(flash[:notice]).to include('SAML group link was successfully removed.')
+          expect(AuditEvent.last.details[:custom_message]).to eq("SAML group links removed. Group Name - #{group_link.saml_group_name}")
         end
 
         it 'removes the group link' do

ee/spec/services/group_saml/saml_provider/create_service_spec.rb

@@ -8,5 +8,7 @@ RSpec.describe GroupSaml::SamlProvider::CreateService do
 
   let(:group) { create :group }
 
+  let(:audit_event_name) { 'group_saml_provider_create' }
+
   include_examples 'base SamlProvider service'
 end

ee/spec/services/group_saml/saml_provider/update_service_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe GroupSaml::SamlProvider::UpdateService do
   end
 
   let(:group) { saml_provider.group }
+  let(:audit_event_name) { 'group_saml_provider_update' }
 
   include_examples 'base SamlProvider service'
   include_examples 'SamlProvider service toggles Group Managed Accounts'

ee/spec/support/shared_examples/services/group_saml/saml_provider/base_service_shared_examples.rb

@@ -17,6 +17,15 @@ RSpec.shared_examples 'base SamlProvider service' do
   end
 
   it 'updates SAML provider with given params' do
+    expect(::Gitlab::Audit::Auditor)
+      .to receive(:audit).with(
+        hash_including(
+          { name: audit_event_name,
+            author: current_user,
+            scope: group,
+            target: group })
+      ).exactly(4).times.and_call_original
+
     expect do
       service.execute
       group.reload
@@ -24,6 +33,7 @@ RSpec.shared_examples 'base SamlProvider service' do
              .and change { group.saml_provider&.certificate_fingerprint }.to(fingerprint)
              .and change { group.saml_provider&.enabled? }.to(true)
              .and change { group.saml_provider&.enforced_sso? }.to(true)
+             .and change { AuditEvent.count }.by(4)
   end
 end