Commit 2b27038d authored by Marc Shaw's avatar Marc Shaw

Merge branch 'pedropombeiro/354894/fix-token-prefix-length-in-audit-log' into 'master'

Increase token preview length in runner audit logs

See merge request gitlab-org/gitlab!82523
parents 848fa973 6a880b3c
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module AuditEvents module AuditEvents
class RunnerAuditEventService < ::AuditEventService class RunnerAuditEventService < ::AuditEventService
SAFE_TOKEN_LENGTH = 8
# Logs an audit event related to a runner event # Logs an audit event related to a runner event
# #
# @param [Ci::Runner] runner # @param [Ci::Runner] runner
...@@ -21,14 +23,12 @@ module AuditEvents ...@@ -21,14 +23,12 @@ module AuditEvents
} }
details.merge!(entity_id: @token_scope.id, entity_type: @token_scope.class.name) if @token_scope details.merge!(entity_id: @token_scope.id, entity_type: @token_scope.class.name) if @token_scope
if author.is_a?(String) safe_author = safe_author(author)
author = author[0...8] details[token_field] = safe_author if safe_author.is_a?(String)
details[token_field] = author
end
details[:errors] = @runner.errors.full_messages unless @runner.errors.empty? details[:errors] = @runner.errors.full_messages unless @runner.errors.empty?
super(author, token_scope, details) super(safe_author, token_scope, details)
end end
def track_event def track_event
...@@ -40,6 +40,10 @@ module AuditEvents ...@@ -40,6 +40,10 @@ module AuditEvents
private private
def token_field
raise NotImplementedError, "Please implement #{self.class}##{__method__}"
end
def message def message
raise NotImplementedError, "Please implement #{self.class}##{__method__}" raise NotImplementedError, "Please implement #{self.class}##{__method__}"
end end
...@@ -59,5 +63,14 @@ module AuditEvents ...@@ -59,5 +63,14 @@ module AuditEvents
url_helpers.admin_runner_path(@runner) url_helpers.admin_runner_path(@runner)
end end
end end
def safe_author(author)
return author unless author.is_a?(String)
safe_token_length = SAFE_TOKEN_LENGTH
safe_token_length += ::RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX.length if author.start_with?(::RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX)
author[0...safe_token_length]
end
end end
end end
...@@ -56,7 +56,7 @@ RSpec.describe AuditEvents::RegisterRunnerAuditEventService do ...@@ -56,7 +56,7 @@ RSpec.describe AuditEvents::RegisterRunnerAuditEventService do
entity_path: nil, entity_path: nil,
target_details: target_details, target_details: target_details,
details: { details: {
runner_registration_token: author[0...8], runner_registration_token: author[0...described_class::SAFE_TOKEN_LENGTH],
entity_path: nil, entity_path: nil,
target_details: target_details target_details: target_details
} }
...@@ -90,13 +90,28 @@ RSpec.describe AuditEvents::RegisterRunnerAuditEventService do ...@@ -90,13 +90,28 @@ RSpec.describe AuditEvents::RegisterRunnerAuditEventService do
end end
context 'on persisted runner' do context 'on persisted runner' do
let(:runner) { create(:ci_runner) } let_it_be(:runner) { create(:ci_runner) }
let(:target_details) { ::Gitlab::Routing.url_helpers.admin_runner_path(runner) } let(:target_details) { ::Gitlab::Routing.url_helpers.admin_runner_path(runner) }
let(:extra_attrs) do let(:extra_attrs) do
{ details: { custom_message: 'Registered instance CI runner' } } { details: { custom_message: 'Registered instance CI runner' } }
end end
it_behaves_like 'expected audit log' it_behaves_like 'expected audit log'
context 'with registration token prefixed with RUNNERS_TOKEN_PREFIX' do
let(:author) { "#{::RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}b6bce79c3a" }
let(:extra_attrs) do
{
details: {
custom_message: 'Registered instance CI runner',
runner_registration_token: author[0...::RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX.length + described_class::SAFE_TOKEN_LENGTH]
}
}
end
it_behaves_like 'expected audit log'
end
end end
end end
...@@ -107,14 +122,14 @@ RSpec.describe AuditEvents::RegisterRunnerAuditEventService do ...@@ -107,14 +122,14 @@ RSpec.describe AuditEvents::RegisterRunnerAuditEventService do
let(:target_details) { } let(:target_details) { }
let(:attrs) do let(:attrs) do
common_attrs.deep_merge( common_attrs.deep_merge(
author_name: author[0...8], author_name: author[0...described_class::SAFE_TOKEN_LENGTH],
entity_id: entity.id, entity_id: entity.id,
entity_type: entity.class.name, entity_type: entity.class.name,
entity_path: entity.full_path, entity_path: entity.full_path,
target_details: target_details, target_details: target_details,
details: { details: {
author_name: author[0...8], author_name: author[0...described_class::SAFE_TOKEN_LENGTH],
runner_registration_token: author[0...8], runner_registration_token: author[0...described_class::SAFE_TOKEN_LENGTH],
custom_message: 'Registered group CI runner', custom_message: 'Registered group CI runner',
entity_id: entity.id, entity_id: entity.id,
entity_type: entity.class.name, entity_type: entity.class.name,
...@@ -167,14 +182,14 @@ RSpec.describe AuditEvents::RegisterRunnerAuditEventService do ...@@ -167,14 +182,14 @@ RSpec.describe AuditEvents::RegisterRunnerAuditEventService do
let(:target_details) { } let(:target_details) { }
let(:attrs) do let(:attrs) do
common_attrs.deep_merge( common_attrs.deep_merge(
author_name: author[0...8], author_name: author[0...described_class::SAFE_TOKEN_LENGTH],
entity_id: entity.id, entity_id: entity.id,
entity_type: entity.class.name, entity_type: entity.class.name,
entity_path: entity.full_path, entity_path: entity.full_path,
target_details: target_details, target_details: target_details,
details: { details: {
author_name: author[0...8], author_name: author[0...described_class::SAFE_TOKEN_LENGTH],
runner_registration_token: author[0...8], runner_registration_token: author[0...described_class::SAFE_TOKEN_LENGTH],
entity_id: entity.id, entity_id: entity.id,
entity_type: entity.class.name, entity_type: entity.class.name,
entity_path: entity.full_path, entity_path: entity.full_path,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment