From 1118c00867f54af2a8e568a84f7af6f6e12f7644 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 15:59:34 +0200 Subject: [PATCH 01/10] =?UTF-8?q?=E2=9C=A8=20Audit=20log=20for=20every=20a?= =?UTF-8?q?uthorization=20decision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PermissionVerbHandler now resolves the calling principal's subject id (sub claim), display name (preferred_username, falling back to name then sub), and roles, and emits a structured "allow" or "deny" entry through the new IAuthorizationAuditLog for every verb-level check. Both outcomes are captured — denies alone are insufficient for most compliance use cases — and the reason embeds the matched role(s) for allow and the candidate role(s) for deny. AuthorizationAuditLog writes on the stable category "ServiceControl.Audit" via a source-generated structured log method so any ILogger-compatible sink (Seq, OTLP, file, in-memory test double, …) can collect or filter the trail without coupling to the concrete type name. The audit log is registered alongside the verb handler — only when OIDC is enabled and the handler has decisions to make. Unauthenticated requests are skipped at the top of HandleRequirementAsync so the audit log only records identified principals; the framework challenges with 401 via the policy's RequireAuthenticatedUser anyway. Ported from the keycloak-rbac-poc spike (with the namespace flattened from Infrastructure.Auth.Rbac to Infrastructure.Auth to match the real branch) along with a RecordingLoggerProvider test helper colocated with the unit tests. --- .../Auth/PermissionAuthorizationExtensions.cs | 6 +- .../Auth/PermissionVerbHandler.cs | 47 ++++++++-- .../Auth/AuthorizationAuditLogTests.cs | 94 +++++++++++++++++++ .../Auth/RecordingLoggerProvider.cs | 59 ++++++++++++ .../Auth/AuthorizationAuditLog.cs | 45 +++++++++ .../Auth/IAuthorizationAuditLog.cs | 25 +++++ 6 files changed, 269 insertions(+), 7 deletions(-) create mode 100644 src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs create mode 100644 src/ServiceControl.Infrastructure.Tests/Auth/RecordingLoggerProvider.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs create mode 100644 src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs diff --git a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs index 07229849eb..5ad13b6ed5 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs @@ -5,6 +5,7 @@ namespace ServiceControl.Hosting.Auth; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; +using ServiceControl.Infrastructure.Auth; /// /// Registers the permission-based policy authorization services: a dynamic @@ -34,9 +35,12 @@ public static void AddServiceControlAuthorization(this IHostApplicationBuilder h new PermissionPolicyProvider(sp.GetRequiredService>(), oidcEnabled)); // The role-based handler is only needed when OIDC is enabled — otherwise the provider produces - // no PermissionRequirement for it to evaluate. + // no PermissionRequirement for it to evaluate. The handler emits an audit-log entry for every + // decision through IAuthorizationAuditLog (registered alongside) so the platform can show, after + // the fact, who attempted what and how the system responded. if (oidcEnabled) { + services.AddSingleton(); services.AddSingleton(); } } diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 7155bde555..31c7fb0013 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -1,7 +1,9 @@ #nullable enable namespace ServiceControl.Hosting.Auth; +using System.IdentityModel.Tokens.Jwt; using System.Linq; +using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using ServiceControl.Infrastructure.Auth; @@ -9,14 +11,15 @@ namespace ServiceControl.Hosting.Auth; /// /// Verb-level authorization handler for . It resolves the user's /// roles and checks them against the hardcoded policy: the user must hold -/// a role (e.g. reader / writer) that grants the requested permission. +/// a role (e.g. reader / writer) that grants the requested permission. Every decision is +/// captured through for compliance. /// /// Only registered — and only reached — when OIDC is enabled. When it is disabled, /// returns an allow-all policy that carries no /// , so this handler is not needed. /// /// -public sealed class PermissionVerbHandler : AuthorizationHandler +public sealed class PermissionVerbHandler(IAuthorizationAuditLog auditLog) : AuthorizationHandler { // The per-IdP variability of the source claim is absorbed by RolesClaimsTransformation, which // reads from the path configured in Authentication.RolesClaim and emits canonical "roles" claims. @@ -26,16 +29,48 @@ protected override Task HandleRequirementAsync( AuthorizationHandlerContext context, PermissionRequirement requirement) { - var roles = context.User.FindAll(RoleClaimType).Select(claim => claim.Value); + // Unauthenticated requests have no subject and no roles. The framework will challenge with + // 401 because the policy also includes RequireAuthenticatedUser; skipping here keeps the + // audit log restricted to identified principals. + if (context.User.Identity?.IsAuthenticated != true) + { + return Task.CompletedTask; + } + var subjectId = context.User.FindFirst(JwtRegisteredClaimNames.Sub)?.Value ?? ""; + var subjectName = context.User.FindFirst("preferred_username")?.Value + ?? context.User.FindFirst(ClaimTypes.Name)?.Value + ?? subjectId; + var roles = context.User.FindAll(RoleClaimType).Select(claim => claim.Value).ToArray(); + var permission = requirement.Permission; - // TODO: Although plural, likely roles will only contain a single value unless we want to define a role for each instance but likely customers don't care about instances - if (RolePermissions.IsGranted(roles, requirement.Permission)) + if (RolePermissions.IsGranted(roles, permission)) { + auditLog.Decision( + subjectId, + subjectName, + permission, + resource: null, + allowed: true, + reason: roles.Length == 0 + ? $"User holds '{permission}'" + : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]"); + context.Succeed(requirement); + return Task.CompletedTask; } - // Otherwise leave the requirement unmet → the request is denied (403/401). + auditLog.Decision( + subjectId, + subjectName, + permission, + resource: null, + allowed: false, + reason: roles.Length == 0 + ? $"User has no roles granting '{permission}'" + : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'"); + + // 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 new file mode 100644 index 0000000000..aa6a471373 --- /dev/null +++ b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs @@ -0,0 +1,94 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Tests.Auth; + +using System; +using Microsoft.Extensions.Logging; +using NUnit.Framework; +using ServiceControl.Infrastructure.Auth; + +[TestFixture] +public class AuthorizationAuditLogTests +{ + [Test] + public void Decision_allow_emits_one_entry_on_audit_category() + { + 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", "acme.sales", allowed: true, reason: "role:reader matched"); + + var entries = provider.EntriesFor("ServiceControl.Audit"); + Assert.That(entries, Has.Count.EqualTo(1)); + Assert.That(entries[0].Message, Does.Contain("alice-sub-001")); + Assert.That(entries[0].Message, Does.Contain("Alice Smith")); + Assert.That(entries[0].Message, Does.Contain("error:messages:retry")); + Assert.That(entries[0].Message, Does.Contain("allow")); + Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Information)); + } + + [Test] + public void Decision_deny_emits_one_entry_on_audit_category() + { + 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 matching role"); + + var entries = provider.EntriesFor("ServiceControl.Audit"); + Assert.That(entries, Has.Count.EqualTo(1)); + Assert.That(entries[0].Message, Does.Contain("bob-sub-002")); + Assert.That(entries[0].Message, Does.Contain("Bob Jones")); + Assert.That(entries[0].Message, Does.Contain("error:messages:retry")); + Assert.That(entries[0].Message, Does.Contain("deny")); + Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Information)); + } + + [Test] + public void Decision_does_not_appear_on_other_categories() + { + var provider = new RecordingLoggerProvider(); + var factory = LoggerFactory.Create(b => b.AddProvider(provider)); + var auditLog = new AuthorizationAuditLog(factory); + + auditLog.Decision("carol-sub-003", "Carol White", "error:endpoints:view", null, allowed: true, reason: "role:reader matched"); + + Assert.That(provider.EntriesFor("ServiceControl.SomeOtherCategory"), Is.Empty); + } + + [Test] + public void Multiple_decisions_accumulate_in_order() + { + var provider = new RecordingLoggerProvider(); + var factory = LoggerFactory.Create(b => b.AddProvider(provider)); + var auditLog = new AuthorizationAuditLog(factory); + + auditLog.Decision("alice-sub-001", "alice", "error:messages:view", null, allowed: true, "role matched"); + auditLog.Decision("alice-sub-001", "alice", "error:messages:retry", "acme.finance", allowed: false, "out of scope"); + + var entries = provider.EntriesFor("ServiceControl.Audit"); + Assert.That(entries, Has.Count.EqualTo(2)); + Assert.That(entries[0].Message, Does.Contain("allow")); + Assert.That(entries[1].Message, Does.Contain("deny")); + } + + [TestCase(null, "Alice", "error:messages:retry", "reason")] + [TestCase("", "Alice", "error:messages:retry", "reason")] + [TestCase("alice-sub-001", null, "error:messages:retry", "reason")] + [TestCase("alice-sub-001", "", "error:messages:retry", "reason")] + [TestCase("alice-sub-001", "Alice", null, "reason")] + [TestCase("alice-sub-001", "Alice", "", "reason")] + [TestCase("alice-sub-001", "Alice", "error:messages:retry", null)] + [TestCase("alice-sub-001", "Alice", "error:messages:retry", "")] + public void Decision_throws_when_required_argument_is_null_or_empty(string? subjectId, string? subjectName, string? permission, string? reason) + { + var provider = new RecordingLoggerProvider(); + var factory = LoggerFactory.Create(b => b.AddProvider(provider)); + var auditLog = new AuthorizationAuditLog(factory); + + Assert.That( + () => auditLog.Decision(subjectId!, subjectName!, permission!, resource: null, allowed: true, reason: reason!), + Throws.InstanceOf()); + } +} diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/RecordingLoggerProvider.cs b/src/ServiceControl.Infrastructure.Tests/Auth/RecordingLoggerProvider.cs new file mode 100644 index 0000000000..a753a2b8e4 --- /dev/null +++ b/src/ServiceControl.Infrastructure.Tests/Auth/RecordingLoggerProvider.cs @@ -0,0 +1,59 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Tests.Auth; + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.Logging; + +/// +/// In-memory that captures log entries for test assertions. +/// Thread-safe. Use for all captured entries; +/// to filter by category. +/// +sealed class RecordingLoggerProvider : ILoggerProvider +{ + readonly ConcurrentQueue entries = new(); + + public IReadOnlyList Entries => entries.ToArray(); + + public IReadOnlyList EntriesFor(string category) => + entries.Where(e => e.Category == category).ToArray(); + + public ILogger CreateLogger(string categoryName) => + new RecordingLogger(categoryName, entries); + + public void Dispose() { } +} + +sealed record LogEntry( + string Category, + LogLevel Level, + EventId EventId, + string Message, + Exception? Exception); + +sealed class RecordingLogger(string category, ConcurrentQueue sink) : ILogger +{ + public IDisposable? BeginScope(TState state) where TState : notnull => NullScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => logLevel != LogLevel.None; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + var message = formatter(state, exception); + sink.Enqueue(new LogEntry(category, logLevel, eventId, message, exception)); + } + + sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + public void Dispose() { } + } +} diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs new file mode 100644 index 0000000000..6ac35835a9 --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -0,0 +1,45 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +using System; +using Microsoft.Extensions.Logging; + +/// +/// Default that emits every decision as a structured log entry on +/// the stable category ServiceControl.Audit. Sinks filter on the category, not on the type name. +/// +public sealed partial class AuthorizationAuditLog : IAuthorizationAuditLog +{ + const string AuditCategory = "ServiceControl.Audit"; + + readonly ILogger logger; + + public AuthorizationAuditLog(ILoggerFactory loggerFactory) + { + logger = loggerFactory.CreateLogger(AuditCategory); + } + + public void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason) + { + ArgumentException.ThrowIfNullOrEmpty(subjectId); + ArgumentException.ThrowIfNullOrEmpty(subjectName); + ArgumentException.ThrowIfNullOrEmpty(permission); + ArgumentException.ThrowIfNullOrEmpty(reason); + + LogDecision(logger, subjectId, subjectName, permission, resource, allowed ? "allow" : "deny", reason); + } + + // Source-generated structured log method — zero allocation on the hot path. + [LoggerMessage( + EventId = 1001, + Level = LogLevel.Information, + Message = "Authorization {Outcome}: subjectId={SubjectId} subjectName={SubjectName} permission={Permission} resource={Resource} reason={Reason}")] + static partial void LogDecision( + ILogger logger, + string subjectId, + string subjectName, + string permission, + string? resource, + string outcome, + string reason); +} diff --git a/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs new file mode 100644 index 0000000000..d6449673cc --- /dev/null +++ b/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs @@ -0,0 +1,25 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Auth; + +/// +/// 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 — +/// denies alone are insufficient for most compliance use cases. +/// +/// Implementations write structured log entries on a stable category so sinks (Seq, OTLP, file, +/// in-memory test double, …) can filter on it without coupling to the concrete type name. +/// +/// +public interface IAuthorizationAuditLog +{ + /// + /// Records a single authorization decision. + /// + /// Stable identifier of the principal (e.g. the JWT sub claim). Must not be null or empty. + /// Human-readable display name of the principal (e.g. preferred_username). Must not be null or empty. + /// The permission that was evaluated (e.g. error:messages:retry). + /// 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); +} From 64c5ddcca8ea3a4591736545f3984e8e4d749216 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 16:31:02 +0200 Subject: [PATCH 02/10] =?UTF-8?q?=E2=9C=A8=20Configurable=20required=20sub?= =?UTF-8?q?ject=20claims=20for=20the=20audit=20log?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PermissionVerbHandler now reads the subject id and subject name from configurable claim keys (Authentication.SubjectIdClaim, default "sub"; Authentication.SubjectNameClaim, default "preferred_username") and throws InvalidOperationException when an authenticated principal lacks either claim or carries an empty value — both are required for the audit log to be meaningful and a missing value indicates an IdP misconfiguration the operator needs to fix. The settings are passed through AddServiceControlAuthorization, which now takes the full OpenIdConnectSettings (the existing bool-only overload is removed; the six callers — three RunCommand entry points and three acceptance-test runners — pass the settings object). The MockOidcServer test helper defaults preferred_username to the subject value so existing OIDC acceptance tests don't have to repeat the boilerplate. --- .../OpenIdConnect/MockOidcServer.cs | 4 +++ .../ServiceControlComponentRunner.cs | 2 +- .../ServiceControlComponentRunner.cs | 2 +- ...rovals.PlatformSampleSettings.approved.txt | 2 ++ .../Hosting/Commands/RunCommand.cs | 2 +- .../Auth/PermissionAuthorizationExtensions.cs | 16 ++++++++---- .../Auth/PermissionVerbHandler.cs | 26 ++++++++++++++----- .../OpenIdConnectSettings.cs | 25 ++++++++++++++++-- .../ServiceControlComponentRunner.cs | 2 +- ...sTests.PlatformSampleSettings.approved.txt | 2 ++ .../Hosting/Commands/RunCommand.cs | 2 +- ...rovals.PlatformSampleSettings.approved.txt | 2 ++ .../Hosting/Commands/RunCommand.cs | 2 +- 13 files changed, 70 insertions(+), 19 deletions(-) diff --git a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/MockOidcServer.cs b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/MockOidcServer.cs index 03f5c156eb..81322cf47c 100644 --- a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/MockOidcServer.cs +++ b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/MockOidcServer.cs @@ -184,9 +184,13 @@ public string GenerateToken( { var credentials = new SigningCredentials(securityKey, SecurityAlgorithms.RsaSha256); + // sub + preferred_username are required by PermissionVerbHandler for the audit log; + // defaulting them here keeps callers concise. Callers that need to test the + // missing-claim path pass an explicit additionalClaim with an empty value to override. var claims = new List { new(JwtRegisteredClaimNames.Sub, subject), + new("preferred_username", subject), new(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString()) }; diff --git a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 45c6726718..c3b87d68fd 100644 --- a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -123,7 +123,7 @@ async Task InitializeServiceControl(ScenarioContext context) EnvironmentName = Environments.Development }); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControl(settings, configuration); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlApi(settings.CorsSettings); diff --git a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 6e0d233f12..53a548f038 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -120,7 +120,7 @@ async Task InitializeServiceControl(ScenarioContext context) EnvironmentName = Environments.Development }); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlAudit((criticalErrorContext, cancellationToken) => { var logitem = new ScenarioContext.LogItem diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 82cf9de13f..6df6a5a94d 100644 --- a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -13,6 +13,8 @@ "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, "RolesClaim": "realm_access.roles", + "SubjectIdClaim": "sub", + "SubjectNameClaim": "preferred_username", "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null diff --git a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs index d8229e8774..2bfdb9c065 100644 --- a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs @@ -19,7 +19,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlAudit((_, __) => { diff --git a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs index 5ad13b6ed5..a6cc21e4a4 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs @@ -5,6 +5,7 @@ namespace ServiceControl.Hosting.Auth; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; +using ServiceControl.Infrastructure; using ServiceControl.Infrastructure.Auth; /// @@ -21,7 +22,7 @@ namespace ServiceControl.Hosting.Auth; /// public static class PermissionAuthorizationExtensions { - public static void AddServiceControlAuthorization(this IHostApplicationBuilder hostBuilder, bool oidcEnabled) + public static void AddServiceControlAuthorization(this IHostApplicationBuilder hostBuilder, OpenIdConnectSettings oidcSettings) { var services = hostBuilder.Services; @@ -32,16 +33,21 @@ public static void AddServiceControlAuthorization(this IHostApplicationBuilder h // policy provider registered by AddAuthorization(). When OIDC is disabled it returns allow-all // policies (no requirement); when enabled it emits a PermissionRequirement for the verb handler. services.AddSingleton(sp => - new PermissionPolicyProvider(sp.GetRequiredService>(), oidcEnabled)); + new PermissionPolicyProvider(sp.GetRequiredService>(), oidcSettings.Enabled)); // The role-based handler is only needed when OIDC is enabled — otherwise the provider produces // no PermissionRequirement for it to evaluate. The handler emits an audit-log entry for every // decision through IAuthorizationAuditLog (registered alongside) so the platform can show, after - // the fact, who attempted what and how the system responded. - if (oidcEnabled) + // the fact, who attempted what and how the system responded. The subject-id and subject-name + // claim names flow through from configuration so the handler can read them off the principal. + if (oidcSettings.Enabled) { services.AddSingleton(); - services.AddSingleton(); + services.AddSingleton(sp => + new PermissionVerbHandler( + sp.GetRequiredService(), + oidcSettings.SubjectIdClaim, + oidcSettings.SubjectNameClaim)); } } } diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 31c7fb0013..65cab4842b 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -1,7 +1,7 @@ #nullable enable namespace ServiceControl.Hosting.Auth; -using System.IdentityModel.Tokens.Jwt; +using System; using System.Linq; using System.Security.Claims; using System.Threading.Tasks; @@ -19,7 +19,11 @@ namespace ServiceControl.Hosting.Auth; /// , so this handler is not needed. /// /// -public sealed class PermissionVerbHandler(IAuthorizationAuditLog auditLog) : AuthorizationHandler +public sealed class PermissionVerbHandler( + IAuthorizationAuditLog auditLog, + string subjectIdClaim, + string subjectNameClaim) + : AuthorizationHandler { // The per-IdP variability of the source claim is absorbed by RolesClaimsTransformation, which // reads from the path configured in Authentication.RolesClaim and emits canonical "roles" claims. @@ -37,10 +41,8 @@ protected override Task HandleRequirementAsync( return Task.CompletedTask; } - var subjectId = context.User.FindFirst(JwtRegisteredClaimNames.Sub)?.Value ?? ""; - var subjectName = context.User.FindFirst("preferred_username")?.Value - ?? context.User.FindFirst(ClaimTypes.Name)?.Value - ?? subjectId; + var subjectId = RequireClaim(context.User, subjectIdClaim, "Authentication.SubjectIdClaim"); + var subjectName = RequireClaim(context.User, subjectNameClaim, "Authentication.SubjectNameClaim"); var roles = context.User.FindAll(RoleClaimType).Select(claim => claim.Value).ToArray(); var permission = requirement.Permission; @@ -73,4 +75,16 @@ protected override Task HandleRequirementAsync( // Leave the requirement unmet → the framework forbids (403). return Task.CompletedTask; } + + static string RequireClaim(ClaimsPrincipal user, string claimType, string settingName) + { + var value = user.FindFirst(claimType)?.Value; + if (string.IsNullOrEmpty(value)) + { + throw new InvalidOperationException( + $"Authenticated principal is missing the required '{claimType}' claim configured by {settingName}. " + + "Configure the identity provider to emit this claim, or point the setting at the claim the IdP actually emits."); + } + return value; + } } diff --git a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs index 23c4f58565..1252af8a2f 100644 --- a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs +++ b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs @@ -39,6 +39,13 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC // this path and flattens the values into canonical "roles" claims for the authorization handler. RolesClaim = SettingsReader.Read(rootNamespace, "Authentication.RolesClaim", "realm_access.roles"); + // Claims that identify the principal in the authorization audit log. The handler treats both + // as required — a missing or empty value is a sign that the IdP isn't emitting the expected + // claim and the operator needs to fix the configuration, so the handler will throw rather + // than substitute a placeholder. + SubjectIdClaim = SettingsReader.Read(rootNamespace, "Authentication.SubjectIdClaim", "sub"); + SubjectNameClaim = SettingsReader.Read(rootNamespace, "Authentication.SubjectNameClaim", "preferred_username"); + // ServicePulse settings are only relevant for the primary ServiceControl instance // which serves the OIDC configuration endpoint that ServicePulse uses for login if (requireServicePulseSettings) @@ -112,6 +119,20 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC /// public string RolesClaim { get; } + /// + /// Claim that carries the stable subject identifier (e.g. the JWT sub claim) recorded in + /// the authorization audit log. Required — the handler throws if the configured claim is absent + /// or empty on an authenticated principal. + /// + public string SubjectIdClaim { get; } + + /// + /// Claim that carries the human-readable subject name (e.g. preferred_username) recorded + /// in the authorization audit log. Required — the handler throws if the configured claim is + /// absent or empty on an authenticated principal. + /// + public string SubjectNameClaim { get; } + /// /// Optional override for the authority URL that ServicePulse should use for authentication. /// If not specified, ServicePulse uses the main Authority value. @@ -203,8 +224,8 @@ void LogConfiguration(bool requireServicePulseSettings) var servicePulseAuthorityDisplay = requireServicePulseSettings ? (ServicePulseAuthority ?? "(not configured)") : "(n/a)"; var servicePulseApiScopesDisplay = requireServicePulseSettings ? (ServicePulseApiScopes ?? "(not configured)") : "(n/a)"; - logger.LogInformation("Authentication settings: Enabled={Enabled}, Authority={Authority}, Audience={Audience}, ValidateIssuer={ValidateIssuer}, ValidateAudience={ValidateAudience}, ValidateLifetime={ValidateLifetime}, ValidateIssuerSigningKey={ValidateIssuerSigningKey}, RequireHttpsMetadata={RequireHttpsMetadata}, RolesClaim={RolesClaim}, ServicePulseClientId={ServicePulseClientId}, ServicePulseAuthority={ServicePulseAuthority}, ServicePulseApiScopes={ServicePulseApiScopes}", - Enabled, authorityDisplay, audienceDisplay, ValidateIssuer, ValidateAudience, ValidateLifetime, ValidateIssuerSigningKey, RequireHttpsMetadata, RolesClaim, servicePulseClientIdDisplay, servicePulseAuthorityDisplay, servicePulseApiScopesDisplay); + logger.LogInformation("Authentication settings: Enabled={Enabled}, Authority={Authority}, Audience={Audience}, ValidateIssuer={ValidateIssuer}, ValidateAudience={ValidateAudience}, ValidateLifetime={ValidateLifetime}, ValidateIssuerSigningKey={ValidateIssuerSigningKey}, RequireHttpsMetadata={RequireHttpsMetadata}, RolesClaim={RolesClaim}, SubjectIdClaim={SubjectIdClaim}, SubjectNameClaim={SubjectNameClaim}, ServicePulseClientId={ServicePulseClientId}, ServicePulseAuthority={ServicePulseAuthority}, ServicePulseApiScopes={ServicePulseApiScopes}", + Enabled, authorityDisplay, audienceDisplay, ValidateIssuer, ValidateAudience, ValidateLifetime, ValidateIssuerSigningKey, RequireHttpsMetadata, RolesClaim, SubjectIdClaim, SubjectNameClaim, servicePulseClientIdDisplay, servicePulseAuthorityDisplay, servicePulseApiScopesDisplay); // Warn about potential misconfigurations var hasAuthConfig = !string.IsNullOrWhiteSpace(Authority) || !string.IsNullOrWhiteSpace(Audience); diff --git a/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 319ea95329..aaab866c82 100644 --- a/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -100,7 +100,7 @@ async Task InitializeServiceControl(ScenarioContext context) hostBuilder.Logging.ConfigureLogging(LogLevel.Information); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlMonitoring((criticalErrorContext, cancellationToken) => { var logitem = new ScenarioContext.LogItem diff --git a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt index b29ddc3856..35b5d3bd46 100644 --- a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt @@ -13,6 +13,8 @@ "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, "RolesClaim": "realm_access.roles", + "SubjectIdClaim": "sub", + "SubjectNameClaim": "preferred_username", "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null diff --git a/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs b/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs index da23814397..6e197e463a 100644 --- a/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs @@ -16,7 +16,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlMonitoring((_, __) => Task.CompletedTask, settings, endpointConfiguration); hostBuilder.AddServiceControlMonitoringApi(); diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 5ca8fcdf90..330fa54694 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -13,6 +13,8 @@ "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, "RolesClaim": "realm_access.roles", + "SubjectIdClaim": "sub", + "SubjectNameClaim": "preferred_username", "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null diff --git a/src/ServiceControl/Hosting/Commands/RunCommand.cs b/src/ServiceControl/Hosting/Commands/RunCommand.cs index 1f895ec9d3..c25485bf5c 100644 --- a/src/ServiceControl/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl/Hosting/Commands/RunCommand.cs @@ -25,7 +25,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControl(settings, endpointConfiguration); hostBuilder.AddServiceControlApi(settings.CorsSettings); From 2f5433af46ed6a145081e40c1ca95150e68ca250 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Thu, 11 Jun 2026 09:28:28 +0200 Subject: [PATCH 03/10] Separate allow/deny log templates, log deny as Warning --- .../Auth/AuthorizationAuditLog.cs | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index 6ac35835a9..6d9d2ddce8 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -8,16 +8,11 @@ namespace ServiceControl.Infrastructure.Auth; /// Default that emits every decision as a structured log entry on /// the stable category ServiceControl.Audit. Sinks filter on the category, not on the type name. /// -public sealed partial class AuthorizationAuditLog : IAuthorizationAuditLog +public sealed partial class AuthorizationAuditLog(ILoggerFactory loggerFactory) : IAuthorizationAuditLog { const string AuditCategory = "ServiceControl.Audit"; - readonly ILogger logger; - - public AuthorizationAuditLog(ILoggerFactory loggerFactory) - { - logger = loggerFactory.CreateLogger(AuditCategory); - } + readonly ILogger logger = loggerFactory.CreateLogger(AuditCategory); public void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason) { @@ -26,20 +21,40 @@ public void Decision(string subjectId, string subjectName, string permission, st ArgumentException.ThrowIfNullOrEmpty(permission); ArgumentException.ThrowIfNullOrEmpty(reason); - LogDecision(logger, subjectId, subjectName, permission, resource, allowed ? "allow" : "deny", reason); + if(allowed) + { + LogAllow(logger, subjectId, subjectName, permission, resource, reason); + } + else + { + LogDeny(logger, subjectId, subjectName, permission, resource, reason); + } } // Source-generated structured log method — zero allocation on the hot path. [LoggerMessage( EventId = 1001, Level = LogLevel.Information, - Message = "Authorization {Outcome}: subjectId={SubjectId} subjectName={SubjectName} permission={Permission} resource={Resource} reason={Reason}")] - static partial void LogDecision( + Message = "Allow: subjectId={SubjectId} subjectName={SubjectName} permission={Permission} resource={Resource} reason={Reason}")] + static partial void LogAllow( + ILogger logger, + string subjectId, + string subjectName, + string permission, + string? resource, + string reason + ); + + [LoggerMessage( + EventId = 1002, + Level = LogLevel.Warning, + Message = "Deny: subjectId={SubjectId} subjectName={SubjectName} permission={Permission} resource={Resource} reason={Reason}")] + static partial void LogDeny( ILogger logger, string subjectId, string subjectName, string permission, string? resource, - string outcome, - string reason); + string reason + ); } From 696e5448b75154bf55466f6486e346c5cfec2cf6 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Thu, 11 Jun 2026 09:29:38 +0200 Subject: [PATCH 04/10] Add TODO for instance-specific audit categories in AuthorizationAuditLog --- src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index 6d9d2ddce8..92404d2a61 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -10,7 +10,7 @@ namespace ServiceControl.Infrastructure.Auth; /// public sealed partial class AuthorizationAuditLog(ILoggerFactory loggerFactory) : IAuthorizationAuditLog { - const string AuditCategory = "ServiceControl.Audit"; + const string AuditCategory = "ServiceControl.Audit"; // TODO: Currently this is in infrastructure thus used bv all instances. Would we need to use a different category per instance? readonly ILogger logger = loggerFactory.CreateLogger(AuditCategory); From f29a3187b7093b63a80b07485205a504a2e21786 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Thu, 11 Jun 2026 10:16:45 +0200 Subject: [PATCH 05/10] IDE0055 --- src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index 92404d2a61..cb1944b35a 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -21,7 +21,7 @@ public void Decision(string subjectId, string subjectName, string permission, st ArgumentException.ThrowIfNullOrEmpty(permission); ArgumentException.ThrowIfNullOrEmpty(reason); - if(allowed) + if (allowed) { LogAllow(logger, subjectId, subjectName, permission, resource, reason); } From 1e2d894bbd0f3acb05ac35ff2463809e42bd4f4f Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 12 Jun 2026 09:30:51 +0200 Subject: [PATCH 06/10] Refactor dependencies on settings to inject `OpenIdConnectSettings` --- .../Auth/HostApplicationBuilderExtensions.cs | 11 ++++++++--- .../Auth/PermissionAuthorizationExtensions.cs | 19 +++++++++---------- .../Auth/PermissionVerbHandler.cs | 8 ++++---- .../Auth/RolesClaimsTransformation.cs | 5 +++-- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs index eba714e32d..08c4db3cf3 100644 --- a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; + using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; using Microsoft.IdentityModel.Tokens; using ServiceControl.Infrastructure; @@ -21,6 +22,11 @@ public static void AddServiceControlAuthentication(this IHostApplicationBuilder return; } + // Shared with the authorization services and the claims transformation below; registered + // once so it can be constructor-injected rather than captured. TryAdd keeps it idempotent + // with AddServiceControlAuthorization, which registers the same instance. + hostBuilder.Services.TryAddSingleton(oidcSettings); + _ = hostBuilder.Services.AddAuthentication(options => { options.DefaultScheme = "Bearer"; @@ -104,9 +110,8 @@ public static void AddServiceControlAuthentication(this IHostApplicationBuilder // Normalise per-IdP role claim shapes (Keycloak's nested realm_access.roles, Entra app // roles, Cognito groups) into canonical "roles" claims for the verb handler. The source - // path is configurable via Authentication.RolesClaim. - hostBuilder.Services.AddSingleton( - new RolesClaimsTransformation(oidcSettings.RolesClaim)); + // path is configurable via Authentication.RolesClaim, read off the injected settings. + hostBuilder.Services.AddSingleton(); } static string GetErrorMessage(JwtBearerChallengeContext context) diff --git a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs index dcf84d9637..6e56aa6e89 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs @@ -3,8 +3,8 @@ namespace ServiceControl.Hosting.Auth; using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Options; using ServiceControl.Infrastructure; using ServiceControl.Infrastructure.Auth; @@ -26,6 +26,10 @@ public static void AddServiceControlAuthorization(this IHostApplicationBuilder h { var services = hostBuilder.Services; + // The settings are shared by every auth service below (and the authentication wiring), so they + // are registered once in DI and constructor-injected rather than captured in factory lambdas. + services.TryAddSingleton(oidcSettings); + // Ensure the authorization core services and options are present (idempotent). services.AddAuthorization(); @@ -35,20 +39,15 @@ public static void AddServiceControlAuthorization(this IHostApplicationBuilder h // request to an annotated endpoint. When RBAC is disabled the provider returns allow-all // policies (no requirement), so anonymous-to-the-policy calls pass through and the verb // handler is unnecessary. - services.AddSingleton(sp => - new PermissionPolicyProvider(sp.GetRequiredService>(), oidcSettings)); + services.AddSingleton(); // The provider only emits a PermissionRequirement when RBAC is enabled, so the handler is the // only thing that evaluates one. It is registered alongside the provider (cheap singleton, never // invoked when no requirement is produced). The handler emits an audit-log entry for every // decision through IAuthorizationAuditLog so the platform can show, after the fact, who attempted - // what and how the system responded. The subject-id and subject-name claim names flow through - // from configuration so the handler can read them off the principal. + // what and how the system responded. The subject-id and subject-name claim names are read off the + // injected OpenIdConnectSettings so the handler can match them on the principal. services.AddSingleton(); - services.AddSingleton(sp => - new PermissionVerbHandler( - sp.GetRequiredService(), - oidcSettings.SubjectIdClaim, - oidcSettings.SubjectNameClaim)); + services.AddSingleton(); } } diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 65cab4842b..e564cf4635 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -6,6 +6,7 @@ namespace ServiceControl.Hosting.Auth; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; +using ServiceControl.Infrastructure; using ServiceControl.Infrastructure.Auth; /// @@ -21,8 +22,7 @@ namespace ServiceControl.Hosting.Auth; /// public sealed class PermissionVerbHandler( IAuthorizationAuditLog auditLog, - string subjectIdClaim, - string subjectNameClaim) + OpenIdConnectSettings oidcSettings) : AuthorizationHandler { // The per-IdP variability of the source claim is absorbed by RolesClaimsTransformation, which @@ -41,8 +41,8 @@ protected override Task HandleRequirementAsync( return Task.CompletedTask; } - var subjectId = RequireClaim(context.User, subjectIdClaim, "Authentication.SubjectIdClaim"); - var subjectName = RequireClaim(context.User, subjectNameClaim, "Authentication.SubjectNameClaim"); + var subjectId = RequireClaim(context.User, oidcSettings.SubjectIdClaim, "Authentication.SubjectIdClaim"); + var subjectName = RequireClaim(context.User, oidcSettings.SubjectNameClaim, "Authentication.SubjectNameClaim"); var roles = context.User.FindAll(RoleClaimType).Select(claim => claim.Value).ToArray(); var permission = requirement.Permission; diff --git a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs index 2f59829edc..ad14ef1228 100644 --- a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs +++ b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs @@ -5,6 +5,7 @@ namespace ServiceControl.Hosting.Auth; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; +using ServiceControl.Infrastructure; using ServiceControl.Infrastructure.Auth; /// @@ -18,7 +19,7 @@ namespace ServiceControl.Hosting.Auth; /// claim makes the transformation idempotent and returns the same principal on subsequent calls. /// /// -public sealed class RolesClaimsTransformation(string rolesClaimPath) : IClaimsTransformation +public sealed class RolesClaimsTransformation(OpenIdConnectSettings oidcSettings) : IClaimsTransformation { const string SentinelClaimType = "_roles_transformed"; // The sentinel's value is irrelevant; only the claim's presence matters. A non-empty @@ -34,7 +35,7 @@ public Task TransformAsync(ClaimsPrincipal principal) return Task.FromResult(principal); } - var roles = RolesClaimExtractor.Extract(principal, rolesClaimPath); + var roles = RolesClaimExtractor.Extract(principal, oidcSettings.RolesClaim); var claims = new Claim[roles.Count + 1]; claims[0] = new Claim(SentinelClaimType, SentinelClaimValue); From 96a1af14f7677f633a18fba55688350e2abbc261 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 12 Jun 2026 10:10:06 +0200 Subject: [PATCH 07/10] =?UTF-8?q?=E2=9C=A8=20Route=20ServiceControl.Audit?= =?UTF-8?q?=20category=20to=20a=20structured=20JSON=20log=20target?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a dedicated NLog target and a final logging rule that emit the authorization audit trail as structured JSON on the ServiceControl.Audit category, separate from the plain-text operational log, so it can be shipped to a SIEM without the two streams polluting each other. Audit events are captured from Info upward (allow = Information, deny = Warning) independent of the operational LogLevel, so lowering operational verbosity never drops entries from the audit trail. The audit rule is registered before the catch-all operational rules and marked Final so audit events are not duplicated into the operational log. Extracts BuildConfiguration from ConfigureNLog, registers the targets explicitly so the configuration is fully formed before it is installed, and exposes AuthorizationAuditLog.AuditCategory so the routing is unit-testable against a single source of truth for the category name. Tests assert the routing structure and that a real decision renders as one valid JSON object per line. --- .../LoggingConfiguratorTests.cs | 105 ++++++++++++++++++ .../Auth/AuthorizationAuditLog.cs | 2 +- .../LoggingConfigurator.cs | 70 +++++++++++- 3 files changed, 172 insertions(+), 5 deletions(-) create mode 100644 src/ServiceControl.Infrastructure.Tests/LoggingConfiguratorTests.cs diff --git a/src/ServiceControl.Infrastructure.Tests/LoggingConfiguratorTests.cs b/src/ServiceControl.Infrastructure.Tests/LoggingConfiguratorTests.cs new file mode 100644 index 0000000000..1ad9438775 --- /dev/null +++ b/src/ServiceControl.Infrastructure.Tests/LoggingConfiguratorTests.cs @@ -0,0 +1,105 @@ +#nullable enable +namespace ServiceControl.Infrastructure.Tests; + +using System.IO; +using System.Linq; +using System.Text.Json; +using Microsoft.Extensions.Logging; +using NLog; +using NLog.Config; +using NLog.Extensions.Logging; +using NLog.Layouts; +using NLog.Targets; +using NUnit.Framework; +using ServiceControl.Infrastructure; +using ServiceControl.Infrastructure.Auth; +using LogLevel = NLog.LogLevel; + +[TestFixture] +public class LoggingConfiguratorTests +{ + static readonly string AuditPattern = $"{AuthorizationAuditLog.AuditCategory}*"; + + static LoggingConfiguration BuildConfig() => + LoggingConfigurator.BuildConfiguration("logfile.txt", Path.GetTempPath(), LogLevel.Info); + + [Test] + public void Audit_category_is_routed_to_a_structured_json_target() + { + var config = BuildConfig(); + + var auditRule = config.LoggingRules.Single(r => r.LoggerNamePattern == AuditPattern); + + Assert.That( + auditRule.Targets.OfType().Any(t => t.Layout is JsonLayout), + Is.True, + "the audit category should be routed to a target that uses a JSON layout"); + } + + [Test] + public void Audit_events_do_not_fall_through_to_the_operational_log() + { + var config = BuildConfig(); + + var auditRule = config.LoggingRules.Single(r => r.LoggerNamePattern == AuditPattern); + var operationalConsoleRule = config.LoggingRules.Single(r => r.LoggerNamePattern == "*" && r.Targets.Any(t => t.Name == "console")); + + Assert.That(auditRule.Final, Is.True, "the audit rule must be final so audit JSON is not duplicated into the plain-text operational log"); + Assert.That( + config.LoggingRules.IndexOf(auditRule), + Is.LessThan(config.LoggingRules.IndexOf(operationalConsoleRule)), + "the audit rule must be evaluated before the catch-all console rule for Final to take effect"); + } + + [Test] + public void Audit_decisions_render_as_valid_structured_json() + { + // Use the exact JSON layout the production configuration builds... + var auditLayout = BuildConfig().AllTargets + .OfType() + .Single(t => t.Name == "audit-console") + .Layout; + + // ...and capture what it renders, driven through the real audit logger over an isolated NLog factory. + var captured = new MemoryTarget("audit-capture") { Layout = auditLayout }; + var captureConfig = new LoggingConfiguration(); + captureConfig.AddRule(LogLevel.Info, LogLevel.Fatal, captured, AuditPattern); + var logFactory = new LogFactory { Configuration = captureConfig }; + + using (var loggerFactory = LoggerFactory.Create(b => b.AddNLog(_ => logFactory))) + { + var audit = new AuthorizationAuditLog(loggerFactory); + audit.Decision("alice-sub-001", "Alice Smith", "error:messages:retry", "acme.sales", allowed: true, reason: "role:sc-operator matched"); + audit.Decision("bob-sub-002", "Bob Jones", "error:messages:retry", null, allowed: false, reason: "no matching role"); + } + + logFactory.Flush(); + + Assert.That(captured.Logs, Has.Count.EqualTo(2), "expected one JSON line per decision"); + + foreach (var line in captured.Logs) + { + TestContext.Progress.WriteLine(line); + } + + var allow = JsonDocument.Parse(captured.Logs[0]).RootElement; + Assert.Multiple(() => + { + Assert.That(allow.GetProperty("level").GetString(), Is.EqualTo("INFO")); + Assert.That(allow.GetProperty("category").GetString(), Is.EqualTo(AuthorizationAuditLog.AuditCategory)); + Assert.That(allow.GetProperty("SubjectId").GetString(), Is.EqualTo("alice-sub-001")); + Assert.That(allow.GetProperty("SubjectName").GetString(), Is.EqualTo("Alice Smith")); + Assert.That(allow.GetProperty("Permission").GetString(), Is.EqualTo("error:messages:retry")); + Assert.That(allow.GetProperty("Resource").GetString(), Is.EqualTo("acme.sales")); + Assert.That(allow.TryGetProperty("timestamp", out _), Is.True, "timestamp attribute should be present"); + }); + + var deny = JsonDocument.Parse(captured.Logs[1]).RootElement; + Assert.Multiple(() => + { + Assert.That(deny.GetProperty("level").GetString(), Is.EqualTo("WARN"), "denies must surface at Warning level"); + Assert.That(deny.GetProperty("SubjectId").GetString(), Is.EqualTo("bob-sub-002")); + Assert.That(deny.GetProperty("Permission").GetString(), Is.EqualTo("error:messages:retry")); + }); + } +} diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index cb1944b35a..0a3b16b229 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -10,7 +10,7 @@ namespace ServiceControl.Infrastructure.Auth; /// public sealed partial class AuthorizationAuditLog(ILoggerFactory loggerFactory) : IAuthorizationAuditLog { - const string AuditCategory = "ServiceControl.Audit"; // TODO: Currently this is in infrastructure thus used bv all instances. Would we need to use a different category per instance? + public const string AuditCategory = "ServiceControl.Audit"; // TODO: Currently this is in infrastructure thus used bv all instances. Would we need to use a different category per instance? readonly ILogger logger = loggerFactory.CreateLogger(AuditCategory); diff --git a/src/ServiceControl.Infrastructure/LoggingConfigurator.cs b/src/ServiceControl.Infrastructure/LoggingConfigurator.cs index b94fd8b296..f6e4b1dfa1 100644 --- a/src/ServiceControl.Infrastructure/LoggingConfigurator.cs +++ b/src/ServiceControl.Infrastructure/LoggingConfigurator.cs @@ -7,6 +7,7 @@ namespace ServiceControl.Infrastructure using NLog.Layouts; using NLog.Targets; using ServiceControl.Configuration; + using ServiceControl.Infrastructure.Auth; using LogManager = NServiceBus.Logging.LogManager; using LogLevel = NLog.LogLevel; @@ -28,6 +29,17 @@ public static void ConfigureLogging(LoggingSettings loggingSettings) } public static string ConfigureNLog(string logFileName, string logPath, LogLevel logLevel) + { + var nlogConfig = BuildConfiguration(logFileName, logPath, logLevel); + + NLog.LogManager.Configuration = nlogConfig; + + var logEventInfo = new LogEventInfo { TimeStamp = DateTime.UtcNow }; + var fileTarget = nlogConfig.FindTargetByName("file"); + return AppEnvironment.RunningInContainer ? "console" : fileTarget.FileName.Render(logEventInfo); + } + + public static LoggingConfiguration BuildConfiguration(string logFileName, string logPath, LogLevel logLevel) { //configure NLog var nlogConfig = new LoggingConfiguration(); @@ -65,20 +77,70 @@ public static string ConfigureNLog(string logFileName, string logPath, LogLevel FinalMinLevel = LogLevel.Warn }; + // The authorization audit trail is emitted on a dedicated category as structured JSON so it can be + // shipped to a SIEM without being polluted by — or polluting — the plain-text operational log. + var auditLayout = new JsonLayout + { + IncludeEventProperties = true, // SubjectId, SubjectName, Permission, Resource, Reason, … + Attributes = + { + new JsonAttribute("timestamp", "${longdate:universalTime=true}"), + new JsonAttribute("level", "${level:uppercase=true}"), + new JsonAttribute("category", "${logger}"), + new JsonAttribute("message", "${message}") + } + }; + + var auditConsoleTarget = new ConsoleTarget + { + Name = "audit-console", + Layout = auditLayout + }; + + var auditFileTarget = new FileTarget + { + Name = "audit-file", + ArchiveEvery = FileArchivePeriod.Day, + FileName = Path.Combine(logPath, "audit.json"), + ArchiveSuffixFormat = ".{1:yyyy-MM-dd}.{0:00}", + Layout = auditLayout, + MaxArchiveFiles = 14, + ArchiveAboveSize = 30 * megaByte + }; + + // Audit events are captured from Info upward (allow = Information, deny = Warning) regardless of the + // operational LogLevel — lowering the operational verbosity must never drop entries from the audit trail. + // Final stops audit events from also reaching the catch-all operational rules below, so this rule must + // be registered before them. + var auditRule = new LoggingRule + { + LoggerNamePattern = $"{AuthorizationAuditLog.AuditCategory}*", + Final = true + }; + auditRule.SetLoggingLevels(LogLevel.Info, LogLevel.Fatal); + auditRule.Targets.Add(auditConsoleTarget); + if (!AppEnvironment.RunningInContainer) + { + auditRule.Targets.Add(auditFileTarget); + } + + nlogConfig.AddTarget(consoleTarget); + nlogConfig.AddTarget(auditConsoleTarget); + nlogConfig.LoggingRules.Add(aspNetCoreRule); nlogConfig.LoggingRules.Add(httpClientRule); + nlogConfig.LoggingRules.Add(auditRule); nlogConfig.LoggingRules.Add(new LoggingRule("*", logLevel, consoleTarget)); if (!AppEnvironment.RunningInContainer) { + nlogConfig.AddTarget(fileTarget); + nlogConfig.AddTarget(auditFileTarget); nlogConfig.LoggingRules.Add(new LoggingRule("*", logLevel, fileTarget)); } - NLog.LogManager.Configuration = nlogConfig; - - var logEventInfo = new LogEventInfo { TimeStamp = DateTime.UtcNow }; - return AppEnvironment.RunningInContainer ? "console" : fileTarget.FileName.Render(logEventInfo); + return nlogConfig; } static LogLevel ToNLogLevel(this Microsoft.Extensions.Logging.LogLevel level) => From 084fc6a5fb0873a458ea999e3b1f6dde6b38076d Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Mon, 15 Jun 2026 09:56:29 +0200 Subject: [PATCH 08/10] =?UTF-8?q?=F0=9F=90=9B=20Align=20AuthorizationAudit?= =?UTF-8?q?Log=20tests=20with=20the=20Allow:/Deny:=20templates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit log message templates were changed to a capitalised "Allow:"/"Deny:" prefix (and deny moved to Warning) when the allow/deny templates were split, but these tests still asserted the old lowercase "allow"/"deny" substrings and an Information level for deny — so they failed on CI (Linux-Default, Windows-Default). Update the assertions to match the current output: "Allow:"/"Deny:" and deny at Warning level. --- .../Auth/AuthorizationAuditLogTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs index aa6a471373..3ed842ca96 100644 --- a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs +++ b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs @@ -23,7 +23,7 @@ public void Decision_allow_emits_one_entry_on_audit_category() Assert.That(entries[0].Message, Does.Contain("alice-sub-001")); Assert.That(entries[0].Message, Does.Contain("Alice Smith")); Assert.That(entries[0].Message, Does.Contain("error:messages:retry")); - Assert.That(entries[0].Message, Does.Contain("allow")); + Assert.That(entries[0].Message, Does.Contain("Allow:")); Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Information)); } @@ -41,8 +41,8 @@ public void Decision_deny_emits_one_entry_on_audit_category() Assert.That(entries[0].Message, Does.Contain("bob-sub-002")); Assert.That(entries[0].Message, Does.Contain("Bob Jones")); Assert.That(entries[0].Message, Does.Contain("error:messages:retry")); - Assert.That(entries[0].Message, Does.Contain("deny")); - Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Information)); + Assert.That(entries[0].Message, Does.Contain("Deny:")); + Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Warning)); } [Test] @@ -69,8 +69,8 @@ public void Multiple_decisions_accumulate_in_order() var entries = provider.EntriesFor("ServiceControl.Audit"); Assert.That(entries, Has.Count.EqualTo(2)); - Assert.That(entries[0].Message, Does.Contain("allow")); - Assert.That(entries[1].Message, Does.Contain("deny")); + Assert.That(entries[0].Message, Does.Contain("Allow:")); + Assert.That(entries[1].Message, Does.Contain("Deny:")); } [TestCase(null, "Alice", "error:messages:retry", "reason")] From bcb1082e5aee318acf03522892a0cf822f6079e9 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Mon, 15 Jun 2026 10:20:17 +0200 Subject: [PATCH 09/10] =?UTF-8?q?=E2=9C=A8=20Emit=20the=20authorization=20?= =?UTF-8?q?audit=20event=20as=20an=20Elastic=20Common=20Schema=20(ECS)=20d?= =?UTF-8?q?ocument?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AuthorizationAuditLog now serialises each decision as an ECS-shaped JSON document (@timestamp, event.kind/category/type/action/outcome, user.id/name, and the app-specific servicecontrol.* namespace) so it ingests into Elastic/Kibana — and most SIEMs — with no custom mapping. The schema is owned in the domain class; the NLog audit target now writes the pre-rendered document verbatim (one object per line) instead of assembling JSON in logging configuration. Allow/deny is carried by event.type (["allowed"]/["denied"]) and event.outcome (success/failure); the log level still differs (Information/Warning) so sinks can alert on denies without parsing the payload. Relaxed JSON escaping keeps the output readable for log sinks. Only fields available at the verb-level check are populated today; user.roles, user.email and resource scope follow as that data reaches the decision point. --- .../Auth/AuthorizationAuditLogTests.cs | 24 +++--- .../LoggingConfiguratorTests.cs | 39 ++++----- .../Auth/AuthorizationAuditLog.cs | 79 ++++++++++++------- .../LoggingConfigurator.cs | 18 ++--- 4 files changed, 91 insertions(+), 69 deletions(-) diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs index 3ed842ca96..69213d9887 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.Text.Json; using Microsoft.Extensions.Logging; using NUnit.Framework; using ServiceControl.Infrastructure.Auth; @@ -20,10 +21,12 @@ public void Decision_allow_emits_one_entry_on_audit_category() var entries = provider.EntriesFor("ServiceControl.Audit"); Assert.That(entries, Has.Count.EqualTo(1)); - Assert.That(entries[0].Message, Does.Contain("alice-sub-001")); - Assert.That(entries[0].Message, Does.Contain("Alice Smith")); - Assert.That(entries[0].Message, Does.Contain("error:messages:retry")); - Assert.That(entries[0].Message, Does.Contain("Allow:")); + var ecs = JsonDocument.Parse(entries[0].Message).RootElement; + Assert.That(ecs.GetProperty("event").GetProperty("type")[0].GetString(), Is.EqualTo("allowed")); + Assert.That(ecs.GetProperty("event").GetProperty("outcome").GetString(), Is.EqualTo("success")); + Assert.That(ecs.GetProperty("user").GetProperty("id").GetString(), Is.EqualTo("alice-sub-001")); + Assert.That(ecs.GetProperty("user").GetProperty("name").GetString(), Is.EqualTo("Alice Smith")); + Assert.That(ecs.GetProperty("event").GetProperty("action").GetString(), Is.EqualTo("error:messages:retry")); Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Information)); } @@ -38,10 +41,11 @@ public void Decision_deny_emits_one_entry_on_audit_category() var entries = provider.EntriesFor("ServiceControl.Audit"); Assert.That(entries, Has.Count.EqualTo(1)); - Assert.That(entries[0].Message, Does.Contain("bob-sub-002")); - Assert.That(entries[0].Message, Does.Contain("Bob Jones")); - Assert.That(entries[0].Message, Does.Contain("error:messages:retry")); - Assert.That(entries[0].Message, Does.Contain("Deny:")); + var ecs = JsonDocument.Parse(entries[0].Message).RootElement; + Assert.That(ecs.GetProperty("event").GetProperty("type")[0].GetString(), Is.EqualTo("denied")); + Assert.That(ecs.GetProperty("event").GetProperty("outcome").GetString(), Is.EqualTo("failure")); + Assert.That(ecs.GetProperty("user").GetProperty("id").GetString(), Is.EqualTo("bob-sub-002")); + Assert.That(ecs.GetProperty("servicecontrol").GetProperty("resource").ValueKind, Is.EqualTo(JsonValueKind.Null)); Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Warning)); } @@ -69,8 +73,8 @@ public void Multiple_decisions_accumulate_in_order() var entries = provider.EntriesFor("ServiceControl.Audit"); Assert.That(entries, Has.Count.EqualTo(2)); - Assert.That(entries[0].Message, Does.Contain("Allow:")); - Assert.That(entries[1].Message, Does.Contain("Deny:")); + Assert.That(JsonDocument.Parse(entries[0].Message).RootElement.GetProperty("event").GetProperty("type")[0].GetString(), Is.EqualTo("allowed")); + Assert.That(JsonDocument.Parse(entries[1].Message).RootElement.GetProperty("event").GetProperty("type")[0].GetString(), Is.EqualTo("denied")); } [TestCase(null, "Alice", "error:messages:retry", "reason")] diff --git a/src/ServiceControl.Infrastructure.Tests/LoggingConfiguratorTests.cs b/src/ServiceControl.Infrastructure.Tests/LoggingConfiguratorTests.cs index 1ad9438775..cbc790d9de 100644 --- a/src/ServiceControl.Infrastructure.Tests/LoggingConfiguratorTests.cs +++ b/src/ServiceControl.Infrastructure.Tests/LoggingConfiguratorTests.cs @@ -8,7 +8,6 @@ namespace ServiceControl.Infrastructure.Tests; using NLog; using NLog.Config; using NLog.Extensions.Logging; -using NLog.Layouts; using NLog.Targets; using NUnit.Framework; using ServiceControl.Infrastructure; @@ -24,16 +23,17 @@ static LoggingConfiguration BuildConfig() => LoggingConfigurator.BuildConfiguration("logfile.txt", Path.GetTempPath(), LogLevel.Info); [Test] - public void Audit_category_is_routed_to_a_structured_json_target() + public void Audit_target_emits_the_prerendered_event_verbatim() { - var config = BuildConfig(); + var auditTarget = BuildConfig().LoggingRules + .Single(r => r.LoggerNamePattern == AuditPattern) + .Targets.OfType() + .Single(t => t.Name == "audit-console"); - var auditRule = config.LoggingRules.Single(r => r.LoggerNamePattern == AuditPattern); + var rendered = auditTarget.Layout.Render(new LogEventInfo(LogLevel.Info, AuthorizationAuditLog.AuditCategory, "ECS-PAYLOAD")); - Assert.That( - auditRule.Targets.OfType().Any(t => t.Layout is JsonLayout), - Is.True, - "the audit category should be routed to a target that uses a JSON layout"); + Assert.That(rendered, Is.EqualTo("ECS-PAYLOAD"), + "the audit target must pass the pre-rendered ECS JSON through unwrapped, not double-encode it"); } [Test] @@ -85,21 +85,24 @@ public void Audit_decisions_render_as_valid_structured_json() var allow = JsonDocument.Parse(captured.Logs[0]).RootElement; Assert.Multiple(() => { - Assert.That(allow.GetProperty("level").GetString(), Is.EqualTo("INFO")); - Assert.That(allow.GetProperty("category").GetString(), Is.EqualTo(AuthorizationAuditLog.AuditCategory)); - Assert.That(allow.GetProperty("SubjectId").GetString(), Is.EqualTo("alice-sub-001")); - Assert.That(allow.GetProperty("SubjectName").GetString(), Is.EqualTo("Alice Smith")); - Assert.That(allow.GetProperty("Permission").GetString(), Is.EqualTo("error:messages:retry")); - Assert.That(allow.GetProperty("Resource").GetString(), Is.EqualTo("acme.sales")); - Assert.That(allow.TryGetProperty("timestamp", out _), Is.True, "timestamp attribute should be present"); + Assert.That(allow.GetProperty("@timestamp").GetString(), Is.Not.Empty, "ECS @timestamp should be present"); + Assert.That(allow.GetProperty("event").GetProperty("kind").GetString(), Is.EqualTo("event")); + Assert.That(allow.GetProperty("event").GetProperty("category")[0].GetString(), Is.EqualTo("iam")); + Assert.That(allow.GetProperty("event").GetProperty("type")[0].GetString(), Is.EqualTo("allowed")); + Assert.That(allow.GetProperty("event").GetProperty("action").GetString(), Is.EqualTo("error:messages:retry")); + Assert.That(allow.GetProperty("event").GetProperty("outcome").GetString(), Is.EqualTo("success")); + Assert.That(allow.GetProperty("user").GetProperty("id").GetString(), Is.EqualTo("alice-sub-001")); + Assert.That(allow.GetProperty("user").GetProperty("name").GetString(), Is.EqualTo("Alice Smith")); + Assert.That(allow.GetProperty("servicecontrol").GetProperty("resource").GetString(), Is.EqualTo("acme.sales")); }); var deny = JsonDocument.Parse(captured.Logs[1]).RootElement; Assert.Multiple(() => { - Assert.That(deny.GetProperty("level").GetString(), Is.EqualTo("WARN"), "denies must surface at Warning level"); - Assert.That(deny.GetProperty("SubjectId").GetString(), Is.EqualTo("bob-sub-002")); - Assert.That(deny.GetProperty("Permission").GetString(), Is.EqualTo("error:messages:retry")); + Assert.That(deny.GetProperty("event").GetProperty("type")[0].GetString(), Is.EqualTo("denied")); + Assert.That(deny.GetProperty("event").GetProperty("outcome").GetString(), Is.EqualTo("failure")); + Assert.That(deny.GetProperty("user").GetProperty("id").GetString(), Is.EqualTo("bob-sub-002")); + Assert.That(deny.GetProperty("servicecontrol").GetProperty("resource").ValueKind, Is.EqualTo(JsonValueKind.Null), "absent resource should be JSON null"); }); } } diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index 0a3b16b229..34a3b1a47a 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -2,6 +2,9 @@ namespace ServiceControl.Infrastructure.Auth; using System; +using System.Collections.Generic; +using System.Text.Encodings.Web; +using System.Text.Json; using Microsoft.Extensions.Logging; /// @@ -14,6 +17,10 @@ public sealed partial class AuthorizationAuditLog(ILoggerFactory loggerFactory) readonly ILogger logger = loggerFactory.CreateLogger(AuditCategory); + // Relaxed escaping keeps the JSON readable for log sinks (no \uXXXX for '+', '<', accented names, …); + // the HTML-safe default only matters in a browser context, which an audit log is not. + static readonly JsonSerializerOptions EcsJsonOptions = new() { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping }; + public void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason) { ArgumentException.ThrowIfNullOrEmpty(subjectId); @@ -21,40 +28,56 @@ public void Decision(string subjectId, string subjectName, string permission, st ArgumentException.ThrowIfNullOrEmpty(permission); ArgumentException.ThrowIfNullOrEmpty(reason); + var auditEvent = BuildEcsEvent(subjectId, subjectName, permission, resource, allowed, reason); + if (allowed) { - LogAllow(logger, subjectId, subjectName, permission, resource, reason); + LogAllow(logger, auditEvent); } else { - LogDeny(logger, subjectId, subjectName, permission, resource, reason); + LogDeny(logger, auditEvent); } } - // Source-generated structured log method — zero allocation on the hot path. - [LoggerMessage( - EventId = 1001, - Level = LogLevel.Information, - Message = "Allow: subjectId={SubjectId} subjectName={SubjectName} permission={Permission} resource={Resource} reason={Reason}")] - static partial void LogAllow( - ILogger logger, - string subjectId, - string subjectName, - string permission, - string? resource, - string reason - ); - - [LoggerMessage( - EventId = 1002, - Level = LogLevel.Warning, - Message = "Deny: subjectId={SubjectId} subjectName={SubjectName} permission={Permission} resource={Resource} reason={Reason}")] - static partial void LogDeny( - ILogger logger, - string subjectId, - string subjectName, - string permission, - string? resource, - string reason - ); + // Serialises one authorization decision as an Elastic Common Schema (ECS) document so it ingests into + // 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) + { + var ecs = new Dictionary + { + ["@timestamp"] = DateTimeOffset.UtcNow.ToString("O"), + ["event"] = new + { + kind = "event", + category = new[] { "iam" }, + type = new[] { allowed ? "allowed" : "denied" }, + action = permission, + outcome = allowed ? "success" : "failure" + }, + ["user"] = new + { + id = subjectId, + name = subjectName + }, + ["servicecontrol"] = new + { + permission, + resource, + reason + } + }; + + return JsonSerializer.Serialize(ecs, EcsJsonOptions); + } + + // Source-generated structured log methods — the audit event is the pre-rendered ECS JSON document. Allow + // and deny differ only by level so sinks can alert on denies (Warning) without parsing the payload. + [LoggerMessage(EventId = 1001, Level = LogLevel.Information, Message = "{AuditEvent}")] + static partial void LogAllow(ILogger logger, string auditEvent); + + [LoggerMessage(EventId = 1002, Level = LogLevel.Warning, Message = "{AuditEvent}")] + static partial void LogDeny(ILogger logger, string auditEvent); } diff --git a/src/ServiceControl.Infrastructure/LoggingConfigurator.cs b/src/ServiceControl.Infrastructure/LoggingConfigurator.cs index f6e4b1dfa1..6c0b090dea 100644 --- a/src/ServiceControl.Infrastructure/LoggingConfigurator.cs +++ b/src/ServiceControl.Infrastructure/LoggingConfigurator.cs @@ -77,19 +77,11 @@ public static LoggingConfiguration BuildConfiguration(string logFileName, string FinalMinLevel = LogLevel.Warn }; - // The authorization audit trail is emitted on a dedicated category as structured JSON so it can be - // shipped to a SIEM without being polluted by — or polluting — the plain-text operational log. - var auditLayout = new JsonLayout - { - IncludeEventProperties = true, // SubjectId, SubjectName, Permission, Resource, Reason, … - Attributes = - { - new JsonAttribute("timestamp", "${longdate:universalTime=true}"), - new JsonAttribute("level", "${level:uppercase=true}"), - new JsonAttribute("category", "${logger}"), - new JsonAttribute("message", "${message}") - } - }; + // The authorization audit trail is emitted on a dedicated category, separate from the plain-text + // operational log, so it can be shipped to a SIEM without the two streams polluting each other. + // Each event is already a complete ECS JSON document (built in AuthorizationAuditLog); the target + // writes it verbatim, one object per line. + var auditLayout = new SimpleLayout("${message}"); var auditConsoleTarget = new ConsoleTarget { From 1203afdd09978d092d579cb6cb39f07fe68668ab Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Mon, 15 Jun 2026 12:01:45 +0200 Subject: [PATCH 10/10] Remove outdated TODO in `AuthorizationAuditLog` comment regarding instance-specific categories --- src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index 34a3b1a47a..75a15fb980 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -13,7 +13,7 @@ namespace ServiceControl.Infrastructure.Auth; /// public sealed partial class AuthorizationAuditLog(ILoggerFactory loggerFactory) : IAuthorizationAuditLog { - public const string AuditCategory = "ServiceControl.Audit"; // TODO: Currently this is in infrastructure thus used bv all instances. Would we need to use a different category per instance? + public const string AuditCategory = "ServiceControl.Audit"; // Logger name is used in logging configuration to write audit entries to a separate file. readonly ILogger logger = loggerFactory.CreateLogger(AuditCategory);