From 1270e1c6994df03245e16c1374db3be156e4941c Mon Sep 17 00:00:00 2001 From: Dennis van der Stelt Date: Tue, 7 Jul 2026 09:57:21 +0200 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=93=9D=20Correct=20RolesClaim=20defau?= =?UTF-8?q?lt=20in=20XML=20docs=20to=20match=20behavior?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RolesClaim setting defaults to the flat "roles" claim in code (and in the published docs), but two XML doc comments claimed the default was "realm_access.roles". That is misleading: an operator following the comment for a stock Keycloak instance would expect roles to resolve out-of-box when in fact the flat default yields no roles until RolesClaim is pointed at the nested path. Align the comments with the actual default. --- .../Auth/RolesClaimsTransformation.cs | 7 ++++--- .../OpenIdConnectSettings.cs | 9 ++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs index f2641c80a1..a70f166769 100644 --- a/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs +++ b/src/ServiceControl.Hosting/Auth/RolesClaimsTransformation.cs @@ -11,9 +11,10 @@ namespace ServiceControl.Hosting.Auth; /// /// Normalises per-IdP role claim shapes into a flat set of roles claims that /// can read directly. The source path is configured via -/// Authentication.RolesClaim (default realm_access.roles — the Keycloak out-of-box -/// shape). Flat claim names work too (roles for Keycloak with a "User Realm Role" mapper or -/// Microsoft Entra ID app roles, cognito:groups for AWS Cognito). +/// Authentication.RolesClaim (default roles — a flat top-level claim, as emitted by +/// Microsoft Entra ID app roles or Keycloak with a "User Realm Role" mapper). A dotted path reaches +/// into a nested JSON object claim: realm_access.roles for Keycloak's out-of-box shape, or +/// cognito:groups for AWS Cognito. /// /// ASP.NET may invoke multiple times for the same principal; a sentinel /// claim makes the transformation idempotent and returns the same principal on subsequent calls. diff --git a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs index 33f0e461a0..3f80fa8550 100644 --- a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs +++ b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs @@ -139,11 +139,10 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC public string ServicePulseApiScopes { get; } /// - /// Path within the JWT where the user's role values live. Defaults to realm_access.roles - /// to match Keycloak's out-of-box token shape. A flat claim name like roles is used when - /// the identity provider emits role values as top-level claims (Keycloak with a "User Realm Role" - /// mapper, Microsoft Entra ID app roles, AWS Cognito groups, etc.). The dotted form navigates - /// into a nested JSON object claim. + /// Path within the JWT where the user's role values live. Defaults to the flat roles + /// claim, as emitted by Microsoft Entra ID app roles or Keycloak with a "User Realm Role" mapper. + /// A dotted path navigates into a nested JSON object claim: set it to realm_access.roles + /// for Keycloak's out-of-box token shape, or cognito:groups for AWS Cognito. /// public string RolesClaim { get; } From 7136310d7343846731fba7ba1d46804f5d3e0c42 Mon Sep 17 00:00:00 2001 From: Dennis van der Stelt Date: Tue, 7 Jul 2026 09:59:38 +0200 Subject: [PATCH 2/3] =?UTF-8?q?=E2=9C=85=20Cover=20the=20RBAC=20deny=20pat?= =?UTF-8?q?h=20and=20role-filtered=20manifest=20end=20to=20end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The acceptance suite exercised authentication thoroughly (invalid, expired, wrong audience/issuer, and a reader accepted with 200) but never asserted the core authorization outcome: an authenticated user lacking the required role is forbidden. Add two end-to-end tests against the Primary instance: - a reader (view-only) POSTing to errors/retry/all is forbidden (403), not merely unauthenticated (401) - my/routes projects to exactly what the role permits: the manifest advertises a view route the reader may call and omits a retry route it may not This pins the invariant the design relies on, that the advertised manifest and the enforced policy read the same inputs and cannot drift apart. --- .../When_authentication_is_enabled.cs | 25 +++++++++++++ .../When_my_routes_are_requested.cs | 36 +++++++++++++++++++ 2 files changed, 61 insertions(+) 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 b88df37305..51009e48ea 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -205,6 +205,31 @@ public async Task Should_reject_requests_with_wrong_issuer() OpenIdConnectAssertions.AssertUnauthorized(response); } + [Test] + public async Task Should_forbid_authenticated_user_lacking_required_permission() + { + HttpResponseMessage response = null; + + _ = await Define() + .Done(async ctx => + { + // The "reader" role grants every :view permission but none of the operate ones. + // Retrying messages requires error:messages:retry (writer/admin only), so an + // authenticated reader must be forbidden (403), not merely unauthenticated (401). + var readerToken = mockOidcServer.GenerateToken( + additionalClaims: new[] { new Claim("roles", "reader") }); + response = await OpenIdConnectAssertions.SendRequestWithBearerToken( + HttpClient, + HttpMethod.Post, + "/api/errors/retry/all", + readerToken); + return response != null; + }) + .Run(); + + OpenIdConnectAssertions.AssertForbidden(response); + } + class Context : ScenarioContext; } } diff --git a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_my_routes_are_requested.cs b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_my_routes_are_requested.cs index a6194367b7..d2fcb9ff6e 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_my_routes_are_requested.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_my_routes_are_requested.cs @@ -63,5 +63,41 @@ public async Task Should_reject_requests_without_bearer_token() OpenIdConnectAssertions.AssertUnauthorized(response); } + [Test] + public async Task Should_return_only_the_routes_the_callers_role_permits() + { + HttpResponseMessage response = null; + + _ = await Define() + .Done(async ctx => + { + // A reader holds every :view permission but none of the operate ones. The manifest is + // the projection of exactly what the server enforces, so it must advertise a view route + // the reader may call and omit a retry route it may not. + var readerToken = mockOidcServer.GenerateToken( + additionalClaims: new[] { new Claim("roles", "reader") }); + response = await OpenIdConnectAssertions.SendRequestWithBearerToken( + HttpClient, HttpMethod.Get, "/api/my/routes", readerToken); + return response != null; + }) + .Run(); + + OpenIdConnectAssertions.AssertAuthenticated(response); + + var root = JsonDocument.Parse(await response.Content.ReadAsStringAsync()).RootElement; + + var roles = root.GetProperty("roles").EnumerateArray().Select(role => role.GetString()); + Assert.That(roles, Does.Contain("reader")); + + var routes = root.GetProperty("routes").EnumerateArray() + .Select(entry => $"{entry.GetProperty("method").GetString()} {entry.GetProperty("url_template").GetString()}") + .ToArray(); + + Assert.That(routes, Does.Contain("GET /api/errors"), + "Reader holds error:messages:view, so the errors list route must be advertised."); + Assert.That(routes, Does.Not.Contain("POST /api/errors/retry/all"), + "Reader lacks error:messages:retry, so the retry route must be filtered out."); + } + class Context : ScenarioContext; } From e3be8f63d4dfc00902b1a99804be4cfffafa3141 Mon Sep 17 00:00:00 2001 From: Dennis van der Stelt Date: Tue, 7 Jul 2026 10:49:50 +0200 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=94=A5=20Remove=20redundant=20RoleCla?= =?UTF-8?q?imType=20from=20JwtBearer=20token=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RoleClaimType only influences IsInRole() and [Authorize(Roles = ...)], neither of which this platform uses. Authorization reads FindAll(ClaimTypes.Role), and those claims are produced solely by RolesClaimsTransformation, which extracts roles from the configured RolesClaim path (including nested paths such as realm_access.roles) and normalizes them to ClaimTypes.Role. RoleClaimType does not rename or emit claims, so setting it changed nothing. Drop it and document why it stays unset. --- .../Auth/HostApplicationBuilderExtensions.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs index 08c4db3cf3..7ecae4d600 100644 --- a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs @@ -43,12 +43,15 @@ public static void AddServiceControlAuthentication(this IHostApplicationBuilder ValidateLifetime = oidcSettings.ValidateLifetime, ValidateIssuerSigningKey = oidcSettings.ValidateIssuerSigningKey, ValidAudience = oidcSettings.Audience, - ClockSkew = TimeSpan.FromMinutes(5), // Allow 5 minutes clock skew - RoleClaimType = oidcSettings.RolesClaim + ClockSkew = TimeSpan.FromMinutes(5) // Allow 5 minutes clock skew }; options.RequireHttpsMetadata = oidcSettings.RequireHttpsMetadata; // Don't map inbound claims to legacy Microsoft claim types options.MapInboundClaims = false; + // No RoleClaimType is set: it only influences IsInRole()/[Authorize(Roles = ...)], + // which this platform does not use. Roles are read from RolesClaim (which may be a + // nested path such as realm_access.roles) and normalized to ClaimTypes.Role by + // RolesClaimsTransformation, which is what the authorization handlers actually read. // Custom error response handling for better client experience options.Events = new JwtBearerEvents