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.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 138827331f..be007c2cfe 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -52,7 +52,7 @@ protected override Task HandleRequirementAsync( allowed: true, reason: roles.Length == 0 ? $"User holds '{permission}'" - : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]"); + : $"User holds '{permission}' via role(s) [{string.Join(", ", roles)}]", roles: roles); context.Succeed(requirement); return Task.CompletedTask; @@ -66,7 +66,7 @@ protected override Task HandleRequirementAsync( allowed: false, reason: roles.Length == 0 ? $"User has no roles granting '{permission}'" - : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'"); + : $"None of the user's role(s) [{string.Join(", ", roles)}] grants '{permission}'", roles: roles); // Leave the requirement unmet → the framework forbids (403). return Task.CompletedTask; diff --git a/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs b/src/ServiceControl.Infrastructure.Tests/Auth/AuthorizationAuditLogTests.cs index 1db40a1b6c..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; @@ -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() { 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..08be56f22b 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, …); @@ -23,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); @@ -36,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); } @@ -44,11 +50,12 @@ 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 { ["@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 { 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); } 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", 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; }