From 667b525892890de37f58c4e6c36ed8c0ed5a8efc Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Wed, 8 Jul 2026 10:08:08 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9C=A8=20Add=20user.roles=20to=20the=20a?= =?UTF-8?q?uthorization=20audit=20log?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Emit the roles the principal held as the ECS user.roles array on each authorization decision, so SIEMs can facet and alert on roles instead of parsing them out of the free-text reason. Omitted when the principal has no roles. Additive output change; first shipped in 6.18.0 without it, so targets the next patch (6.18.1) or minor (6.19.0). --- .../Auth/PermissionVerbHandler.cs | 6 ++-- .../Auth/AuthorizationAuditLogTests.cs | 28 +++++++++++++++++++ .../Auth/AuthorizationAuditLog.cs | 10 ++++--- .../Auth/IAuthorizationAuditLog.cs | 5 +++- 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 138827331f..a5ca48112a 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -52,7 +52,8 @@ protected override Task HandleRequirementAsync( allowed: true, reason: roles.Length == 0 ? $"User holds '{permission}'" - : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]"); + : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]", + roles: roles); context.Succeed(requirement); return Task.CompletedTask; @@ -66,7 +67,8 @@ protected override Task HandleRequirementAsync( allowed: false, reason: roles.Length == 0 ? $"User has no roles granting '{permission}'" - : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'"); + : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'", + roles: roles); // Leave the requirement unmet → the framework forbids (403). return Task.CompletedTask; diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs index 1db40a1b6c..38952384ac 100644 --- a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs +++ b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs @@ -2,6 +2,7 @@ namespace ServiceControl.Infrastructure.Tests.Auth; using System; +using System.Linq; using System.Text.Json; using Microsoft.Extensions.Logging; using NUnit.Framework; @@ -49,6 +50,33 @@ public void Decision_deny_emits_one_entry_on_audit_category() Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Warning)); } + [Test] + public void Decision_includes_user_roles_when_supplied() + { + var provider = new RecordingLoggerProvider(); + var factory = LoggerFactory.Create(b => b.AddProvider(provider)); + var auditLog = new AuthorizationAuditLog(factory); + + auditLog.Decision("alice-sub-001", "Alice Smith", "error:messages:retry", null, allowed: true, reason: "granted", roles: ["writer", "reader"]); + + var user = JsonDocument.Parse(provider.EntriesFor("ServiceControl.Audit")[0].Message).RootElement.GetProperty("user"); + var roles = user.GetProperty("roles").EnumerateArray().Select(r => r.GetString()).ToArray(); + Assert.That(roles, Is.EqualTo(new[] { "writer", "reader" })); + } + + [Test] + public void Decision_omits_user_roles_when_none() + { + var provider = new RecordingLoggerProvider(); + var factory = LoggerFactory.Create(b => b.AddProvider(provider)); + var auditLog = new AuthorizationAuditLog(factory); + + auditLog.Decision("bob-sub-002", "Bob Jones", "error:messages:retry", null, allowed: false, reason: "no roles", roles: []); + + var user = JsonDocument.Parse(provider.EntriesFor("ServiceControl.Audit")[0].Message).RootElement.GetProperty("user"); + Assert.That(user.TryGetProperty("roles", out _), Is.False, "empty roles should be omitted"); + } + [Test] public void Decision_does_not_appear_on_other_categories() { diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index 83e75e0f7c..04bc26f110 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -23,7 +23,7 @@ public sealed class AuthorizationAuditLog(ILoggerFactory loggerFactory) : IAutho // MessageActionAuditLog so both ECS streams keep the same serialization contract. internal static readonly JsonSerializerOptions EcsJsonOptions = new() { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }; - public void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason) + public void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason, IReadOnlyCollection? roles = null) { ArgumentException.ThrowIfNullOrEmpty(subjectId); ArgumentException.ThrowIfNullOrEmpty(subjectName); @@ -36,7 +36,7 @@ public void Decision(string subjectId, string subjectName, string permission, st return; } - var auditEvent = BuildEcsEvent(subjectId, subjectName, permission, resource, allowed, reason); + var auditEvent = BuildEcsEvent(subjectId, subjectName, permission, resource, allowed, reason, roles); logger.Log(level, allowed ? AllowEventId : DenyEventId, auditEvent, null, IdentityFormatter); } @@ -44,7 +44,7 @@ public void Decision(string subjectId, string subjectName, string permission, st // Elastic/Kibana — and most SIEMs — with no custom mapping. The schema is owned here, in the domain, // rather than in logging configuration. event.type/outcome carry the allow/deny; servicecontrol.* is the // app-specific namespace ECS reserves for custom fields. - static string BuildEcsEvent(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason) + static string BuildEcsEvent(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason, IReadOnlyCollection? roles) { var ecs = new Dictionary { @@ -60,7 +60,9 @@ static string BuildEcsEvent(string subjectId, string subjectName, string permiss ["user"] = new { id = subjectId, - name = subjectName + name = subjectName, + // Omitted (WhenWritingNull) when the principal has no roles, e.g. a denied request. + roles = roles is { Count: > 0 } ? roles : null }, ["servicecontrol"] = new { diff --git a/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs index d6449673cc..a53f86ab1b 100644 --- a/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs @@ -1,6 +1,8 @@ #nullable enable namespace ServiceControl.Infrastructure.Auth; +using System.Collections.Generic; + /// /// Records every authorization allow/deny decision so the platform can demonstrate, after the fact, /// who attempted what and how the system responded. Both allow and deny outcomes are captured — @@ -21,5 +23,6 @@ public interface IAuthorizationAuditLog /// The specific resource checked, or for verb-level checks. /// if the decision was allow; for deny. /// A human-readable explanation (e.g. which role granted the permission, or why nothing matched). - void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason); + /// The roles the principal held at decision time, emitted as the ECS user.roles array so SIEMs can facet on them. Omitted from the document when null or empty. + void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason, IReadOnlyCollection? roles = null); } From 3d025fdca68514dcd83adb1eaa4ecbdadc4e06a4 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Thu, 9 Jul 2026 10:43:39 +0200 Subject: [PATCH 2/2] Formatting --- src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index a5ca48112a..be007c2cfe 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -52,8 +52,7 @@ protected override Task HandleRequirementAsync( allowed: true, reason: roles.Length == 0 ? $"User holds '{permission}'" - : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]", - roles: roles); + : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]", roles: roles); context.Succeed(requirement); return Task.CompletedTask; @@ -67,8 +66,7 @@ protected override Task HandleRequirementAsync( allowed: false, reason: roles.Length == 0 ? $"User has no roles granting '{permission}'" - : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'", - roles: roles); + : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'", roles: roles); // Leave the requirement unmet → the framework forbids (403). return Task.CompletedTask;