Cherry-pick for release 6.18.1 (#5591)

* Add improvements to authorization settings

* Add ecs.version to the audit log ECS documents (#5584)

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

* 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.

* Add improvements to authorization settings (#5588)

---------

Co-authored-by: Ramon Smits <ramon.smits@gmail.com>

Author: WilliamBZA

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)

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]

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;

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;
@@ -27,6 +28,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));
     }
 
@@ -49,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()
     {

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]

src/ServiceControl.Infrastructure/Auth/AuthorizationAuditLog.cs

@@ -16,14 +16,20 @@ 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, …);
     // the HTML-safe default only matters in a browser context, which an audit log is not. Shared with
     // 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<string>? roles = null)
     {
         ArgumentException.ThrowIfNullOrEmpty(subjectId);
         ArgumentException.ThrowIfNullOrEmpty(subjectName);
@@ -36,19 +42,20 @@ 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);
     }
 
     // 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)
+    static string BuildEcsEvent(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason, IReadOnlyCollection<string>? roles)
     {
         var ecs = new Dictionary<string, object?>
         {
             ["@timestamp"] = DateTimeOffset.UtcNow.ToString("O"),
+            ["ecs"] = new { version = EcsVersion },
             ["event"] = new
             {
                 kind = "event",
@@ -60,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
             {

src/ServiceControl.Infrastructure/Auth/IAuthorizationAuditLog.cs

@@ -1,6 +1,8 @@
 #nullable enable
 namespace ServiceControl.Infrastructure.Auth;
 
+using System.Collections.Generic;
+
 /// <summary>
 /// 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
     /// <param name="resource">The specific resource checked, or <see langword="null"/> for verb-level checks.</param>
     /// <param name="allowed"><see langword="true"/> if the decision was allow; <see langword="false"/> for deny.</param>
     /// <param name="reason">A human-readable explanation (e.g. which role granted the permission, or why nothing matched).</param>
-    void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason);
+    /// <param name="roles">The roles the principal held at decision time, emitted as the ECS <c>user.roles</c> array so SIEMs can facet on them. Omitted from the document when null or empty.</param>
+    void Decision(string subjectId, string subjectName, string permission, string? resource, bool allowed, string reason, IReadOnlyCollection<string>? roles = null);
 }

src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs

@@ -66,6 +66,7 @@ static string BuildEcsEvent(AuditUser user, MessageActionKind kind, string permi
         var ecs = new Dictionary<string, object?>
         {
             ["@timestamp"] = DateTimeOffset.UtcNow.ToString("O"),
+            ["ecs"] = new { version = AuthorizationAuditLog.EcsVersion },
             ["event"] = new
             {
                 kind = "event",

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);

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<Exception>(() => 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()
     {