From 1118c00867f54af2a8e568a84f7af6f6e12f7644 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 5 Jun 2026 15:59:34 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9C=A8=20Audit=20log=20for=20every=20aut?= =?UTF-8?q?horization=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 2/2] =?UTF-8?q?=E2=9C=A8=20Configurable=20required=20subje?= =?UTF-8?q?ct=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);