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