diff --git a/doc/administration/audit_event_streaming/audit_event_types.md b/doc/administration/audit_event_streaming/audit_event_types.md index 2f95bac8ab56c7a8c4e8e8ed0eaabde7095a059d..e0caad66fcabcea7447c1541c7da915900e907fd 100644 --- a/doc/administration/audit_event_streaming/audit_event_types.md +++ b/doc/administration/audit_event_streaming/audit_event_types.md @@ -61,6 +61,12 @@ Audit event types belong to the following product categories. | [`update_event_streaming_destination`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74632) | Event triggered when an external audit event destination is updated| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/344664) | | [`update_instance_event_streaming_destination`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125846) | Event triggered when an instance level external audit event destination is updated| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.2](https://gitlab.com/gitlab-org/gitlab/-/issues/404730) | +### Authorization + +| Name | Description | Saved to database | Streamed | Introduced in | +|:-----|:------------|:------------------|:---------|:--------------| +| [`member_role_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/137087) | Event triggered when a custom role is created.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.7](https://gitlab.com/gitlab-org/gitlab/-/issues/388934) | + ### Code review | Name | Description | Saved to database | Streamed | Introduced in | diff --git a/ee/app/services/member_roles/create_service.rb b/ee/app/services/member_roles/create_service.rb index 0db985e2de9493da37b6621d696ce3db819edfc5..c399f0933ac6e8287ed6fe0f8b0f9a2d66a3f8fd 100644 --- a/ee/app/services/member_roles/create_service.rb +++ b/ee/app/services/member_roles/create_service.rb @@ -18,10 +18,25 @@ def create_member_role member_role = MemberRole.new(params.merge(namespace: group)) if member_role.save + log_audit_event(member_role) + ::ServiceResponse.success(payload: { member_role: member_role }) else ::ServiceResponse.error(message: member_role.errors.full_messages.join(', ')) end end + + def log_audit_event(member_role) + audit_context = { + name: 'member_role_created', + author: current_user, + scope: group, + target: member_role, + target_details: member_role.enabled_permissions.join(', '), + message: "Member role #{member_role.name} was created" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end diff --git a/ee/config/audit_events/types/member_role_created.yml b/ee/config/audit_events/types/member_role_created.yml new file mode 100644 index 0000000000000000000000000000000000000000..26305a0fbd742049824405d0edaeaf8a57f72a02 --- /dev/null +++ b/ee/config/audit_events/types/member_role_created.yml @@ -0,0 +1,9 @@ +--- +name: member_role_created +description: Event triggered when a custom role is created. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/388934 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/137087 +feature_category: authorization +milestone: '16.7' +saved_to_database: true +streamed: true diff --git a/ee/lib/api/member_roles.rb b/ee/lib/api/member_roles.rb index a4f445ebb9ce6cc788600d6de9db9ce40e0245f5..f9df01e4d48aa49e6a18a43e80c9d8b73206d840 100644 --- a/ee/lib/api/member_roles.rb +++ b/ee/lib/api/member_roles.rb @@ -54,12 +54,13 @@ class MemberRoles < ::API::Base group = find_group(params[:id]) name = declared_params[:name].presence || "#{Gitlab::Access.human_access(params[:base_access_level])} - custom" - member_role = group.member_roles.new(declared_params.merge(name: name)) + service = ::MemberRoles::CreateService.new(group, current_user, declared_params.merge(name: name)) + response = service.execute - if member_role.save - present member_role, with: EE::API::Entities::MemberRole + if response.success? + present response.payload[:member_role], with: EE::API::Entities::MemberRole else - render_api_error!(member_role.errors.full_messages.first, 400) + render_api_error!(response.message, 400) end end diff --git a/ee/spec/requests/api/member_roles_spec.rb b/ee/spec/requests/api/member_roles_spec.rb index fff5e48f90bfac0e1d8523a24f8124c4fc016751..8a64658011f7d353123a23078c9e758e059844a1 100644 --- a/ee/spec/requests/api/member_roles_spec.rb +++ b/ee/spec/requests/api/member_roles_spec.rb @@ -259,7 +259,7 @@ subject expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to match(/Namespace must be top-level namespace/) + expect(json_response['message']).to match(/Creation of member role is allowed only for root groups/) end end diff --git a/ee/spec/services/member_roles/create_service_spec.rb b/ee/spec/services/member_roles/create_service_spec.rb index 372c82cc2fee4547fe1bd3dc84eb613804f75d84..bebcf53c323a4d4f0d010b8a403a202d5d24bad4 100644 --- a/ee/spec/services/member_roles/create_service_spec.rb +++ b/ee/spec/services/member_roles/create_service_spec.rb @@ -8,10 +8,11 @@ describe '#execute' do let(:params) do - { name: 'new name', read_vulnerability: true, base_access_level: Gitlab::Access::GUEST } + { name: 'new name', read_vulnerability: true, + admin_merge_request: true, base_access_level: Gitlab::Access::GUEST } end - subject { described_class.new(group, user, params).execute } + subject(:create_member_role) { described_class.new(group, user, params).execute } before do stub_licensed_features(custom_roles: true) @@ -19,15 +20,15 @@ shared_examples 'service returns error' do it 'is not succesful' do - expect(subject).to be_error + expect(create_member_role).to be_error end it 'returns the correct error messages' do - expect(subject.message).to include(error_message) + expect(create_member_role.message).to include(error_message) end it 'does not create the member role' do - expect { subject }.not_to change { MemberRole.count } + expect { create_member_role }.not_to change { MemberRole.count } end end @@ -48,20 +49,45 @@ context 'with valid params' do it 'is successful' do - expect(subject).to be_success + expect(create_member_role).to be_success end it 'returns the object with updated attribute' do - expect(subject.payload[:member_role].name).to eq('new name') + expect(create_member_role.payload[:member_role].name).to eq('new name') end it 'creates the member role correctly' do - expect { subject }.to change { MemberRole.count }.by(1) + expect { create_member_role }.to change { MemberRole.count }.by(1) member_role = MemberRole.last expect(member_role.name).to eq('new name') expect(member_role.read_vulnerability).to eq(true) end + + include_examples 'audit event logging' do + let(:licensed_features_to_stub) { { custom_roles: true } } + let_it_be(:event_type) { 'member_role_created' } + let(:operation) { create_member_role.payload[:member_role] } + let(:fail_condition!) do + allow(group).to receive(:custom_roles_enabled?).and_return(false) + end + + let(:attributes) do + { + author_id: user.id, + entity_id: group.id, + entity_type: 'Group', + details: { + author_name: user.name, + target_id: operation.id, + target_type: 'MemberRole', + target_details: 'admin_merge_request, read_vulnerability', + custom_message: "Member role #{operation.name} was created", + author_class: user.class.name + } + } + end + end end context 'with invalid params' do