diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 138827331f..be007c2cfe 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -52,7 +52,7 @@ 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 +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}'"); + : $"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); }