From 9cc7653db64389e2be578f3c537e165d520cb79a Mon Sep 17 00:00:00 2001 From: williambza Date: Thu, 9 Jul 2026 09:45:37 +0200 Subject: [PATCH 1/4] Add improvements to authorization settings --- .../OpenIdConnect/OpenIdConnectAssertions.cs | 8 +++++++- .../OpenIdConnect/When_authentication_is_enabled.cs | 3 ++- .../OpenIdConnectSettings.cs | 7 +++++++ .../Settings/OpenIdConnectSettingsTests.cs | 12 ++++++++++++ .../Authentication/AuthenticationController.cs | 2 ++ 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs index a1007fcc5b..5525b9271f 100644 --- a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs +++ b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs @@ -105,7 +105,8 @@ public static async Task AssertAuthConfigurationResponse( string expectedClientId = null, string expectedAuthority = null, string expectedAudience = null, - string expectedApiScopes = null) + string expectedApiScopes = null, + bool expectedRoleBasedAuthorizationEnabled = false) { Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.OK), "Authentication configuration endpoint should return 200 OK"); @@ -120,6 +121,11 @@ public static async Task AssertAuthConfigurationResponse( "Response should contain 'enabled' property"); Assert.That(enabledProperty.GetBoolean(), Is.EqualTo(expectedEnabled), $"'enabled' should be {expectedEnabled}"); + + Assert.That(root.TryGetProperty("role_based_authorization_enabled", out var roleBasedAuthorizationEnabledProperty), Is.True, + "Response should contain 'role_based_authorization_enabled' property"); + Assert.That(roleBasedAuthorizationEnabledProperty.GetBoolean(), Is.EqualTo(expectedRoleBasedAuthorizationEnabled), + $"'role_based_authorization_enabled' should be {expectedRoleBasedAuthorizationEnabled}"); } // Note: API uses snake_case JSON serialization (client_id, api_scopes) diff --git a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs index 51009e48ea..93342685f1 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -74,7 +74,8 @@ await OpenIdConnectAssertions.AssertAuthConfigurationResponse( expectedEnabled: true, expectedClientId: TestClientId, expectedAudience: TestAudience, - expectedApiScopes: TestApiScopes); + expectedApiScopes: TestApiScopes, + expectedRoleBasedAuthorizationEnabled: true); } [Test] diff --git a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs index 3f80fa8550..058a946168 100644 --- a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs +++ b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs @@ -154,6 +154,13 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC void Validate(bool requireServicePulseSettings) { + if (!Enabled && RoleBasedAuthorizationEnabled) + { + var message = "Authentication.RoleBasedAuthorizationEnabled cannot be true when Authentication.Enabled is false. Role-based authorization requires authentication to be enabled."; + logger.LogCritical(message); + throw new Exception(message); + } + if (Enabled) { ValidateRequiredSettings(requireServicePulseSettings); diff --git a/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs b/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs index 2e3fd20641..d1a588a517 100644 --- a/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs +++ b/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs @@ -29,6 +29,7 @@ public void TearDown() Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_VALIDATELIFETIME", null); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_VALIDATEISSUERSIGNINGKEY", null); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_REQUIREHTTPSMETADATA", null); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED", null); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID", null); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", null); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_AUTHORITY", null); @@ -49,6 +50,7 @@ public void Should_have_correct_defaults() Assert.That(settings.ValidateLifetime, Is.True); Assert.That(settings.ValidateIssuerSigningKey, Is.True); Assert.That(settings.RequireHttpsMetadata, Is.True); + Assert.That(settings.RoleBasedAuthorizationEnabled, Is.False); Assert.That(settings.ServicePulseClientId, Is.Null); Assert.That(settings.ServicePulseApiScopes, Is.Null); Assert.That(settings.ServicePulseAuthority, Is.Null); @@ -265,6 +267,16 @@ public void Should_succeed_without_service_pulse_settings_when_not_required() } } + [Test] + public void Should_throw_when_role_based_authorization_enabled_without_authentication_enabled() + { + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED", "true"); + + var ex = Assert.Throws(() => new OpenIdConnectSettings(TestNamespace, validateConfiguration: true, requireServicePulseSettings: false)); + + Assert.That(ex.Message, Does.Contain("RoleBasedAuthorizationEnabled cannot be true when Authentication.Enabled is false")); + } + [Test] public void Should_not_validate_when_disabled() { diff --git a/src/ServiceControl/Authentication/AuthenticationController.cs b/src/ServiceControl/Authentication/AuthenticationController.cs index 467a7494fa..9a878f568a 100644 --- a/src/ServiceControl/Authentication/AuthenticationController.cs +++ b/src/ServiceControl/Authentication/AuthenticationController.cs @@ -17,6 +17,7 @@ public ActionResult Configuration() var info = new AuthConfig { Enabled = settings.OpenIdConnectSettings.Enabled, + RoleBasedAuthorizationEnabled = settings.OpenIdConnectSettings.RoleBasedAuthorizationEnabled, ClientId = settings.OpenIdConnectSettings.ServicePulseClientId, Authority = settings.OpenIdConnectSettings.ServicePulseAuthority, Audience = settings.OpenIdConnectSettings.Audience, @@ -34,6 +35,7 @@ public ActionResult Configuration() public class AuthConfig { public bool Enabled { get; set; } + public bool RoleBasedAuthorizationEnabled { get; set; } public string ClientId { get; set; } public string Authority { get; set; } public string Audience { get; set; } From 0430f76dd45d37d960cdee20db93e1b81c477a36 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Wed, 8 Jul 2026 16:07:23 +0200 Subject: [PATCH 2/4] Add ecs.version to the audit log ECS documents (#5584) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The authorization and message-action audit documents omitted the ecs.version field. ECS-consuming pipelines (Elasticsearch's built-in logs-*-* index template, Elastic Security, other SIEMs) use ecs.version to select the matching field mappings, so a conformant document should declare it. Both streams now emit "ecs": { "version": "8.11.0" } — the latest ECS schema release, whose event.category/type/outcome and user.* fields are what these documents already use. Additive, non-breaking output change. First shipped in 6.18.0 without the field, so this targets the next patch (6.18.1) or minor (6.19.0). ECS version field: https://www.elastic.co/guide/en/ecs/current/ecs-ecs.html event.type allowed values (allowed/denied/change/deletion): https://www.elastic.co/guide/en/ecs/current/ecs-allowed-values-event-type.html ECS releases: https://github.com/elastic/ecs/releases --- .../Auth/AuthorizationAuditLogTests.cs | 1 + .../Auth/MessageActionAuditLogTests.cs | 1 + .../Auth/AuthorizationAuditLog.cs | 7 +++++++ .../Auth/MessageActionAuditLog.cs | 1 + 4 files changed, 10 insertions(+) diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs index 1db40a1b6c..c6b65e5565 100644 --- a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs +++ b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs @@ -27,6 +27,7 @@ public void Decision_allow_emits_one_entry_on_audit_category() 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(ecs.GetProperty("ecs").GetProperty("version").GetString(), Is.EqualTo("8.11.0")); Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Information)); } diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/MessageActionAuditLogTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/MessageActionAuditLogTests.cs index f2092f5022..31ebebc0fc 100644 --- a/src/ServiceControl.Infrastructure.Tests/Auth/MessageActionAuditLogTests.cs +++ b/src/ServiceControl.Infrastructure.Tests/Auth/MessageActionAuditLogTests.cs @@ -41,6 +41,7 @@ public void Operation_emits_one_entry_on_operation_category() Assert.That(ecs.GetProperty("servicecontrol").GetProperty("resource").GetString(), Is.EqualTo("group-1")); Assert.That(ecs.GetProperty("servicecontrol").GetProperty("count").GetInt32(), Is.EqualTo(42)); Assert.That(ecs.GetProperty("servicecontrol").GetProperty("operation").GetProperty("id").GetString(), Is.EqualTo("op-1")); + Assert.That(ecs.GetProperty("ecs").GetProperty("version").GetString(), Is.EqualTo("8.11.0")); } [Test] diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index 83e75e0f7c..8c4ea1b54b 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -16,6 +16,12 @@ public sealed class AuthorizationAuditLog(ILoggerFactory loggerFactory) : IAutho { public const string AuditCategory = "ServiceControl.Audit"; // Logger name is used in logging configuration to write audit entries to a separate file. + // The ECS version the emitted documents conform to, surfaced as the ecs.version field so downstream + // pipelines can pick the matching mappings. 8.11.0 is the latest ECS schema release; the fields used + // here (event.category/type/outcome, user.*) are stable across the 8.x line. Shared with + // MessageActionAuditLog so both streams declare the same version. + internal const string EcsVersion = "8.11.0"; + readonly ILogger logger = loggerFactory.CreateLogger(AuditCategory); // Relaxed escaping keeps the JSON readable for log sinks (no \uXXXX for '+', '<', accented names, …); @@ -49,6 +55,7 @@ static string BuildEcsEvent(string subjectId, string subjectName, string permiss var ecs = new Dictionary { ["@timestamp"] = DateTimeOffset.UtcNow.ToString("O"), + ["ecs"] = new { version = EcsVersion }, ["event"] = new { kind = "event", diff --git a/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs index abe63658b4..958724d309 100644 --- a/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs @@ -66,6 +66,7 @@ static string BuildEcsEvent(AuditUser user, MessageActionKind kind, string permi var ecs = new Dictionary { ["@timestamp"] = DateTimeOffset.UtcNow.ToString("O"), + ["ecs"] = new { version = AuthorizationAuditLog.EcsVersion }, ["event"] = new { kind = "event", From 761c2ba90c77762ae37d1fddd0ccb2189be5ef20 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Thu, 9 Jul 2026 10:58:50 +0200 Subject: [PATCH 3/4] Add user.roles to the authorization audit log (#5583) Add user.roles to the authorization audit log Emit the roles the principal held as the ECS user.roles array on each authorization decision, so SIEMs can facet and alert on roles instead of parsing them out of the free-text reason. Omitted when the principal has no roles. --- .../Auth/PermissionVerbHandler.cs | 4 +-- .../Auth/AuthorizationAuditLogTests.cs | 28 +++++++++++++++++++ .../Auth/AuthorizationAuditLog.cs | 10 ++++--- .../Auth/IAuthorizationAuditLog.cs | 5 +++- 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 138827331f..be007c2cfe 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -52,7 +52,7 @@ protected override Task HandleRequirementAsync( allowed: true, reason: roles.Length == 0 ? $"User holds '{permission}'" - : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]"); + : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]", roles: roles); context.Succeed(requirement); return Task.CompletedTask; @@ -66,7 +66,7 @@ protected override Task HandleRequirementAsync( allowed: false, reason: roles.Length == 0 ? $"User has no roles granting '{permission}'" - : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'"); + : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'", roles: roles); // Leave the requirement unmet → the framework forbids (403). return Task.CompletedTask; diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs index c6b65e5565..af1184551a 100644 --- a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs +++ b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs @@ -2,6 +2,7 @@ namespace ServiceControl.Infrastructure.Tests.Auth; using System; +using System.Linq; using System.Text.Json; using Microsoft.Extensions.Logging; using NUnit.Framework; @@ -50,6 +51,33 @@ public void Decision_deny_emits_one_entry_on_audit_category() Assert.That(entries[0].Level, Is.EqualTo(LogLevel.Warning)); } + [Test] + public void Decision_includes_user_roles_when_supplied() + { + var provider = new RecordingLoggerProvider(); + var factory = LoggerFactory.Create(b => b.AddProvider(provider)); + var auditLog = new AuthorizationAuditLog(factory); + + auditLog.Decision("alice-sub-001", "Alice Smith", "error:messages:retry", null, allowed: true, reason: "granted", roles: ["writer", "reader"]); + + var user = JsonDocument.Parse(provider.EntriesFor("ServiceControl.Audit")[0].Message).RootElement.GetProperty("user"); + var roles = user.GetProperty("roles").EnumerateArray().Select(r => r.GetString()).ToArray(); + Assert.That(roles, Is.EqualTo(new[] { "writer", "reader" })); + } + + [Test] + public void Decision_omits_user_roles_when_none() + { + var provider = new RecordingLoggerProvider(); + var factory = LoggerFactory.Create(b => b.AddProvider(provider)); + var auditLog = new AuthorizationAuditLog(factory); + + auditLog.Decision("bob-sub-002", "Bob Jones", "error:messages:retry", null, allowed: false, reason: "no roles", roles: []); + + var user = JsonDocument.Parse(provider.EntriesFor("ServiceControl.Audit")[0].Message).RootElement.GetProperty("user"); + Assert.That(user.TryGetProperty("roles", out _), Is.False, "empty roles should be omitted"); + } + [Test] public void Decision_does_not_appear_on_other_categories() { diff --git a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs index 8c4ea1b54b..08be56f22b 100644 --- a/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs @@ -29,7 +29,7 @@ public sealed class AuthorizationAuditLog(ILoggerFactory loggerFactory) : IAutho // MessageActionAuditLog so both ECS streams keep the same serialization contract. internal static readonly JsonSerializerOptions EcsJsonOptions = new() { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }; - public void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason) + public void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason, IReadOnlyCollection? roles = null) { ArgumentException.ThrowIfNullOrEmpty(subjectId); ArgumentException.ThrowIfNullOrEmpty(subjectName); @@ -42,7 +42,7 @@ public void Decision(string subjectId, string subjectName, string permission, st return; } - var auditEvent = BuildEcsEvent(subjectId, subjectName, permission, resource, allowed, reason); + var auditEvent = BuildEcsEvent(subjectId, subjectName, permission, resource, allowed, reason, roles); logger.Log(level, allowed ? AllowEventId : DenyEventId, auditEvent, null, IdentityFormatter); } @@ -50,7 +50,7 @@ public void Decision(string subjectId, string subjectName, string permission, st // Elastic/Kibana — and most SIEMs — with no custom mapping. The schema is owned here, in the domain, // rather than in logging configuration. event.type/outcome carry the allow/deny; servicecontrol.* is the // app-specific namespace ECS reserves for custom fields. - static string BuildEcsEvent(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason) + static string BuildEcsEvent(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason, IReadOnlyCollection? roles) { var ecs = new Dictionary { @@ -67,7 +67,9 @@ static string BuildEcsEvent(string subjectId, string subjectName, string permiss ["user"] = new { id = subjectId, - name = subjectName + name = subjectName, + // Omitted (WhenWritingNull) when the principal has no roles, e.g. a denied request. + roles = roles is { Count: > 0 } ? roles : null }, ["servicecontrol"] = new { diff --git a/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs index d6449673cc..a53f86ab1b 100644 --- a/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs @@ -1,6 +1,8 @@ #nullable enable namespace ServiceControl.Infrastructure.Auth; +using System.Collections.Generic; + /// /// Records every authorization allow/deny decision so the platform can demonstrate, after the fact, /// who attempted what and how the system responded. Both allow and deny outcomes are captured — @@ -21,5 +23,6 @@ public interface IAuthorizationAuditLog /// The specific resource checked, or for verb-level checks. /// if the decision was allow; for deny. /// A human-readable explanation (e.g. which role granted the permission, or why nothing matched). - void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason); + /// The roles the principal held at decision time, emitted as the ECS user.roles array so SIEMs can facet on them. Omitted from the document when null or empty. + void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason, IReadOnlyCollection? roles = null); } From fd87da0ef94fc21e1ec1dc854788f4fd166ce9f7 Mon Sep 17 00:00:00 2001 From: WilliamBZA Date: Thu, 9 Jul 2026 10:52:59 +0200 Subject: [PATCH 4/4] Add improvements to authorization settings (#5588)