diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb
index 3a3d0e53aae5b013bc76be99e5a114859c6e1d80..af2f75998fce1da222aa5e9ea5f9dc80a8f0d034 100644
--- a/app/services/members/update_service.rb
+++ b/app/services/members/update_service.rb
@@ -8,7 +8,7 @@ def execute(members, permission: :update)
members = Array.wrap(members)
old_access_level_expiry_map = members.to_h do |member|
- [member.id, { human_access: member.human_access, expires_at: member.expires_at }]
+ [member.id, { human_access: member.human_access_labeled, expires_at: member.expires_at }]
end
updated_members = update_members(members, permission)
diff --git a/ee/app/services/ee/members/approve_access_request_service.rb b/ee/app/services/ee/members/approve_access_request_service.rb
index 26c0be0b8441e581c9cb86983ee75f9ae20e200f..235f4527fa911e0f6e48049495aa1f29fcdeff70 100644
--- a/ee/app/services/ee/members/approve_access_request_service.rb
+++ b/ee/app/services/ee/members/approve_access_request_service.rb
@@ -21,7 +21,7 @@ def log_audit_event(member:)
message: 'Membership created',
additional_details: {
add: 'user_access',
- as: ::Gitlab::Access.options_with_owner.key(member.access_level.to_i),
+ as: member.human_access_labeled,
member_id: member.id
}
}
diff --git a/ee/app/services/ee/members/create_service.rb b/ee/app/services/ee/members/create_service.rb
index c8665e0093e2a5d07f7be871f1ddb009e2889164..901d1374823fc82599db5fbf37864a82086572a9 100644
--- a/ee/app/services/ee/members/create_service.rb
+++ b/ee/app/services/ee/members/create_service.rb
@@ -104,7 +104,7 @@ def log_audit_event(member:)
message: 'Membership created',
additional_details: {
add: 'user_access',
- as: ::Gitlab::Access.options_with_owner.key(member.access_level.to_i),
+ as: member.human_access_labeled,
member_id: member.id
}
}
diff --git a/ee/app/services/ee/members/update_service.rb b/ee/app/services/ee/members/update_service.rb
index a0e73ae5050052fde4b5f56e16a8517e18fdde75..80274d95da23b70156c8945ce01d0dba3828077b 100644
--- a/ee/app/services/ee/members/update_service.rb
+++ b/ee/app/services/ee/members/update_service.rb
@@ -53,10 +53,10 @@ def log_audit_event(old_access_level:, old_expiry:, member:)
additional_details: {
change: 'access_level',
from: old_access_level,
- to: member.human_access,
+ to: member.human_access_labeled,
expiry_from: old_expiry,
expiry_to: member.expires_at,
- as: ::Gitlab::Access.options_with_owner.key(member.access_level.to_i),
+ as: member.human_access_labeled,
member_id: member.id
}
}
diff --git a/ee/lib/ee/gitlab/access.rb b/ee/lib/ee/gitlab/access.rb
index 6fde73260457bc66db131293ddef1e03383025d9..87b6dad6f7d36ea47854063a3dac218880948ef9 100644
--- a/ee/lib/ee/gitlab/access.rb
+++ b/ee/lib/ee/gitlab/access.rb
@@ -9,6 +9,7 @@ module EE
module Gitlab
module Access
extend ActiveSupport::Concern
+ extend ::Gitlab::Utils::Override
MINIMAL_ACCESS_HASH = { "Minimal Access" => ::Gitlab::Access::MINIMAL_ACCESS }.freeze
@@ -32,6 +33,13 @@ def human_access(access)
options_with_minimal_access.key(access)
end
end
+
+ override :human_access_labeled
+ def human_access_labeled
+ return super unless member_role
+
+ "#{s_('Custom role')}: #{member_role.name}"
+ end
end
end
end
diff --git a/ee/spec/features/groups/audit_events_spec.rb b/ee/spec/features/groups/audit_events_spec.rb
index 52039ef379cf5841466863023065ca40d1828197..8d88c560cd31bf9c110d548233e24268c24092db 100644
--- a/ee/spec/features/groups/audit_events_spec.rb
+++ b/ee/spec/features/groups/audit_events_spec.rb
@@ -58,7 +58,7 @@
end
page.within('.audit-log-table') do
- expect(page).to have_content 'Changed access level from Developer to Maintainer'
+ expect(page).to have_content 'Changed access level from Default role: Developer to Default role: Maintainer'
expect(page).to have_content(user.name)
expect(page).to have_content('Alex')
end
diff --git a/ee/spec/features/projects/audit_events_spec.rb b/ee/spec/features/projects/audit_events_spec.rb
index 59225f75e681ecb5a94ff37eb2b69319fe8f4d83..8af65205eceaa38841b61312f4ee7df90a39aeef 100644
--- a/ee/spec/features/projects/audit_events_spec.rb
+++ b/ee/spec/features/projects/audit_events_spec.rb
@@ -130,7 +130,7 @@
end
page.within('.audit-log-table') do
- expect(page).to have_content 'Changed access level from Developer to Maintainer'
+ expect(page).to have_content 'Changed access level from Default role: Developer to Default role: Maintainer'
expect(page).to have_content(project.first_owner.name)
expect(page).to have_content('Pete')
end
diff --git a/ee/spec/lib/ee/gitlab/access_spec.rb b/ee/spec/lib/ee/gitlab/access_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c9e57428e93a3f60d420cf08f683ee414b5aefde
--- /dev/null
+++ b/ee/spec/lib/ee/gitlab/access_spec.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Access, feature_category: :permissions do
+ describe '#human_access_labeled' do
+ let_it_be_with_reload(:member) { create(:group_member, :developer) }
+
+ it 'returns correct label for default role' do
+ expect(member.human_access_labeled).to eq('Default role: Developer')
+ end
+
+ it 'returns correct label for custom role' do
+ member_role = create(:member_role, :developer, name: 'IM')
+ member.update!(member_role: member_role)
+
+ expect(member.human_access_labeled).to eq('Custom role: IM')
+ end
+ end
+end
diff --git a/ee/spec/services/ee/members/approve_access_request_service_spec.rb b/ee/spec/services/ee/members/approve_access_request_service_spec.rb
index a0af970bdd96b12b74b4b49812466b6c6e1064de..aaed19ec4c6997d1bc62667c38f242d37f4cf83f 100644
--- a/ee/spec/services/ee/members/approve_access_request_service_spec.rb
+++ b/ee/spec/services/ee/members/approve_access_request_service_spec.rb
@@ -10,12 +10,19 @@
let(:access_requester) { source.requesters.find_by!(user_id: access_requester_user.id) }
let(:opts) { {} }
let(:params) { {} }
- let(:custom_access_level) { Gitlab::Access::MAINTAINER }
+ let(:access_level_label) { 'Default role: Developer' }
+ let(:details) do
+ {
+ add: 'user_access',
+ as: access_level_label,
+ member_id: access_requester.id
+ }
+ end
shared_examples "auditor with context" do
it "creates audit event with name" do
expect(::Gitlab::Audit::Auditor).to receive(:audit).with(
- hash_including(name: "member_created", target_details: "John Wick")
+ hash_including(name: "member_created", target_details: "John Wick", additional_details: details)
).and_call_original
described_class.new(current_user, params).execute(access_requester, **opts)
diff --git a/ee/spec/services/ee/members/update_service_spec.rb b/ee/spec/services/ee/members/update_service_spec.rb
index ef679301320960638c19d0c75272489550144585..80668c7b7d48f81189d2fb6ae4b85a034bfb8b52 100644
--- a/ee/spec/services/ee/members/update_service_spec.rb
+++ b/ee/spec/services/ee/members/update_service_spec.rb
@@ -8,15 +8,33 @@
let_it_be(:current_user) { create(:user) }
let_it_be(:member_user) { create(:user) }
let(:permission) { :update }
+ let(:expiration) { 2.days.from_now }
+ let(:original_access_level) { Gitlab::Access::DEVELOPER }
+ let(:access_level) { Gitlab::Access::MAINTAINER }
let(:member) { source.members_and_requesters.find_by!(user_id: member_user.id) }
let(:params) do
- { access_level: Gitlab::Access::MAINTAINER, expires_at: 2.days.from_now }
+ { access_level: access_level, expires_at: expiration }
+ end
+
+ let(:audit_role_expiration_from) { nil }
+ let(:audit_role_from) { "Default role: #{Gitlab::Access.human_access(original_access_level)}" }
+ let(:audit_role_to) { "Default role: #{Gitlab::Access.human_access(access_level)}" }
+ let(:audit_role_details) do
+ {
+ change: 'access_level',
+ from: audit_role_from,
+ to: audit_role_to,
+ expiry_from: audit_role_expiration_from,
+ expiry_to: expiration.to_date,
+ as: audit_role_to,
+ member_id: member.id
+ }
end
shared_examples_for 'logs an audit event' do
specify do
expect(::Gitlab::Audit::Auditor).to receive(:audit).with(
- hash_including(name: "member_updated")
+ hash_including(name: "member_updated", additional_details: audit_role_details)
).and_call_original
expect do
@@ -66,10 +84,15 @@
end
context 'when access_level remains the same and expires_at changes' do
+ let(:expiration_from) { 24.days.from_now }
+ let(:original_access_level) { Gitlab::Access::MAINTAINER }
+
+ let(:audit_role_expiration_from) { expiration_from.to_date }
+
before do
described_class.new(
current_user,
- params.merge(expires_at: 24.days.from_now)
+ params.merge(expires_at: expiration_from)
).execute(member, permission: permission)
end
@@ -82,10 +105,13 @@
before do
described_class.new(
current_user,
- params.merge(access_level: Gitlab::Access::OWNER)
+ params.merge(access_level: original_access_level)
).execute(member, permission: permission)
end
+ let(:original_access_level) { Gitlab::Access::OWNER }
+ let(:audit_role_expiration_from) { expiration.to_date }
+
it_behaves_like 'logs an audit event' do
let(:source) { group }
end
@@ -97,7 +123,8 @@
let_it_be(:member_role_guest) { create(:member_role, :guest, namespace: group) }
let_it_be(:member_role_reporter) { create(:member_role, :reporter, namespace: group) }
- let(:params) { { member_role_id: target_member_role&.id } }
+ let(:params) { { expires_at: expiration, member_role_id: target_member_role&.id } }
+ let(:original_access_level) { Gitlab::Access::GUEST }
subject(:update_member) { described_class.new(current_user, params).execute(member) }
@@ -120,7 +147,14 @@
let(:initial_member_role) { nil }
let(:target_member_role) { member_role_guest }
+ let(:audit_role_to) { "Custom role: #{member_role_guest.name}" }
+ let(:audit_role_as) { "Custom role: #{member_role_guest.name}" }
+
it_behaves_like 'correct member role assignement'
+
+ it_behaves_like 'logs an audit event' do
+ let(:source) { group }
+ end
end
context 'when the user does not have access to the member role' do
@@ -150,8 +184,16 @@
let(:initial_member_role) { member_role_guest }
let(:target_member_role) { member_role_reporter }
+ let(:audit_role_from) { "Custom role: #{member_role_guest.name}" }
+ let(:audit_role_to) { "Custom role: #{member_role_reporter.name}" }
+ let(:audit_role_as) { "Custom role: #{member_role_reporter.name}" }
+
it_behaves_like 'correct member role assignement'
+ it_behaves_like 'logs an audit event' do
+ let(:source) { group }
+ end
+
it 'changes the access level of the member accordingly' do
update_member
diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb
index f1777e059ed04de6253131d58955af0116acc527..d1fcf347d1766ba7e9bb06aa893827491f6c118a 100644
--- a/lib/gitlab/access.rb
+++ b/lib/gitlab/access.rb
@@ -176,6 +176,10 @@ def human_access_with_none
Gitlab::Access.human_access_with_none(access_field)
end
+ def human_access_labeled
+ "#{s_('Default role')}: #{human_access}"
+ end
+
def owner?
access_field == OWNER
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index e0966cd65e1166293bc90c00d208d55782bfe7f1..1e054e57f96a0b935b68c317d7798169f7e7d37e 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -15197,6 +15197,9 @@ msgstr ""
msgid "Custom range"
msgstr ""
+msgid "Custom role"
+msgstr ""
+
msgid "Customer experience improvement and third-party offers"
msgstr ""
@@ -16380,6 +16383,9 @@ msgstr ""
msgid "Default projects limit"
msgstr ""
+msgid "Default role"
+msgstr ""
+
msgid "Default timeout"
msgstr ""
diff --git a/qa/qa/specs/features/ee/browser_ui/10_govern/group/group_audit_logs_1_spec.rb b/qa/qa/specs/features/ee/browser_ui/10_govern/group/group_audit_logs_1_spec.rb
index 9f34e82c5a28f578c854cb5ca37b5802fb4811af..d161c2ae80431366af549986dece24c3853750b8 100644
--- a/qa/qa/specs/features/ee/browser_ui/10_govern/group/group_audit_logs_1_spec.rb
+++ b/qa/qa/specs/features/ee/browser_ui/10_govern/group/group_audit_logs_1_spec.rb
@@ -93,7 +93,8 @@ module QA
end
end
- it_behaves_like 'audit event', ['Added user access as Guest', 'Changed access level', 'Removed user access']
+ it_behaves_like 'audit event',
+ ['Added user access as Default role: Guest', 'Changed access level', 'Removed user access']
end
context 'for add and remove project access', :reliable,
diff --git a/qa/qa/specs/features/ee/browser_ui/10_govern/project/project_audit_logs_spec.rb b/qa/qa/specs/features/ee/browser_ui/10_govern/project/project_audit_logs_spec.rb
index 5b95afdba19ece0ab24688ca6990a16dd9ffe62d..82b05bc6afb3163cc61b8cadcf5f3ac40b788680 100644
--- a/qa/qa/specs/features/ee/browser_ui/10_govern/project/project_audit_logs_spec.rb
+++ b/qa/qa/specs/features/ee/browser_ui/10_govern/project/project_audit_logs_spec.rb
@@ -49,7 +49,7 @@ module QA
end
end
- it_behaves_like 'audit event', ["Added user access as Guest"]
+ it_behaves_like 'audit event', ["Added user access as Default role: Guest"]
end
context "for add deploy key", testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347907' do
diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb
index b23f58565754ea0b87cb945864211f744044fbb9..0ebe6503a9d572c10ec74e0ab08ee9feea598ac4 100644
--- a/spec/services/members/update_service_spec.rb
+++ b/spec/services/members/update_service_spec.rb
@@ -71,7 +71,7 @@
members.each do |member|
expect(update_service).to receive(:after_execute).with(
action: permission,
- old_access_level: member.human_access,
+ old_access_level: member.human_access_labeled,
old_expiry: member.expires_at,
member: member
)