diff --git a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs index dc056b05c3..3148b18275 100644 --- a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs +++ b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs @@ -34,6 +34,18 @@ public OpenIdConnectTestConfiguration WithAuthenticationDisabled() return this; } + /// + /// Enables role-based authorization. When on, controllers carrying + /// [Authorize(Policy = Permissions.X)] require the caller's "roles" claim to map to a + /// role that grants the permission via RolePermissions. When off, the policy provider + /// returns allow-all policies and any authenticated request reaches the controller. + /// + public OpenIdConnectTestConfiguration WithRoleBasedAuthorizationEnabled() + { + SetEnvironmentVariable("AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED", "true"); + return this; + } + /// /// Disables settings validation. This allows testing with placeholder/fake OIDC settings. /// Should only be used in test scenarios where a real OIDC provider is not available. @@ -164,6 +176,7 @@ public void ClearConfiguration() ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_CLIENTID"); ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_APISCOPES"); ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_AUTHORITY"); + ClearEnvironmentVariable("AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED"); ClearEnvironmentVariable("VALIDATECONFIG"); } 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 139c840b77..425b8b6e92 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -36,6 +36,7 @@ public void ConfigureAuth() configuration = new OpenIdConnectTestConfiguration(ServiceControlInstanceType.Primary) .WithConfigurationValidationDisabled() .WithAuthenticationEnabled() + .WithRoleBasedAuthorizationEnabled() .WithAuthority(mockOidcServer.Authority) .WithAudience(TestAudience) .WithServicePulseClientId(TestClientId) diff --git a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 45c6726718..c3b87d68fd 100644 --- a/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -123,7 +123,7 @@ async Task InitializeServiceControl(ScenarioContext context) EnvironmentName = Environments.Development }); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControl(settings, configuration); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlApi(settings.CorsSettings); diff --git a/src/ServiceControl.Audit.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs b/src/ServiceControl.Audit.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs index 1d44d2e64f..1891b545d3 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -33,6 +33,7 @@ public void ConfigureAuth() configuration = new OpenIdConnectTestConfiguration(ServiceControlInstanceType.Audit) .WithConfigurationValidationDisabled() .WithAuthenticationEnabled() + .WithRoleBasedAuthorizationEnabled() .WithAuthority(mockOidcServer.Authority) .WithAudience(TestAudience) .WithRequireHttpsMetadata(false); diff --git a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 6e0d233f12..53a548f038 100644 --- a/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Audit.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -120,7 +120,7 @@ async Task InitializeServiceControl(ScenarioContext context) EnvironmentName = Environments.Development }); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlAudit((criticalErrorContext, cancellationToken) => { var logitem = new ScenarioContext.LogItem diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 82cf9de13f..c73fd28a46 100644 --- a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -12,10 +12,11 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, - "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, - "ServicePulseApiScopes": null + "ServicePulseApiScopes": null, + "RolesClaim": "roles", + "RoleBasedAuthorizationEnabled": false }, "ForwardedHeadersSettings": { "Enabled": true, diff --git a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs index d8229e8774..2bfdb9c065 100644 --- a/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl.Audit/Infrastructure/Hosting/Commands/RunCommand.cs @@ -19,7 +19,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlAudit((_, __) => { diff --git a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs index 3aea1f63dd..eba714e32d 100644 --- a/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/HostApplicationBuilderExtensions.cs @@ -37,7 +37,8 @@ public static void AddServiceControlAuthentication(this IHostApplicationBuilder ValidateLifetime = oidcSettings.ValidateLifetime, ValidateIssuerSigningKey = oidcSettings.ValidateIssuerSigningKey, ValidAudience = oidcSettings.Audience, - ClockSkew = TimeSpan.FromMinutes(5) // Allow 5 minutes clock skew + ClockSkew = TimeSpan.FromMinutes(5), // Allow 5 minutes clock skew + RoleClaimType = oidcSettings.RolesClaim }; options.RequireHttpsMetadata = oidcSettings.RequireHttpsMetadata; // Don't map inbound claims to legacy Microsoft claim types diff --git a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs index 07229849eb..008b1f37ea 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionAuthorizationExtensions.cs @@ -5,6 +5,7 @@ namespace ServiceControl.Hosting.Auth; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; +using ServiceControl.Infrastructure; /// /// Registers the permission-based policy authorization services: a dynamic @@ -20,24 +21,25 @@ namespace ServiceControl.Hosting.Auth; /// public static class PermissionAuthorizationExtensions { - public static void AddServiceControlAuthorization(this IHostApplicationBuilder hostBuilder, bool oidcEnabled) + public static void AddServiceControlAuthorization(this IHostApplicationBuilder hostBuilder, OpenIdConnectSettings oidcSettings) { var services = hostBuilder.Services; // Ensure the authorization core services and options are present (idempotent). services.AddAuthorization(); - // Resolve permission policy names dynamically. Registered last so it supersedes the default - // policy provider registered by AddAuthorization(). When OIDC is disabled it returns allow-all - // policies (no requirement); when enabled it emits a PermissionRequirement for the verb handler. + // The policy provider is registered UNCONDITIONALLY: every instance hosts controllers with + // [Authorize(Policy = Permissions.X)] attributes, and without a provider that knows those + // policy names ASP.NET throws "AuthorizationPolicy named '...' was not found" → 500 on every + // request to an annotated endpoint. When RBAC is disabled the provider returns allow-all + // policies (no requirement), so anonymous-to-the-policy calls pass through and the verb + // handler is unnecessary. services.AddSingleton(sp => - new PermissionPolicyProvider(sp.GetRequiredService>(), oidcEnabled)); + new PermissionPolicyProvider(sp.GetRequiredService>(), oidcSettings.RoleBasedAuthorizationEnabled)); - // The role-based handler is only needed when OIDC is enabled — otherwise the provider produces - // no PermissionRequirement for it to evaluate. - if (oidcEnabled) + if (oidcSettings.RoleBasedAuthorizationEnabled) { - services.AddSingleton(); + services.AddSingleton(_ => new PermissionVerbHandler(oidcSettings.RolesClaim)); } } } diff --git a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs index 1a95ec2ab8..efd07b37e5 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionPolicyProvider.cs @@ -52,7 +52,7 @@ static FrozenDictionary BuildPolicies(bool oidcEnab StringComparer.Ordinal); public Task GetPolicyAsync(string policyName) => - Task.FromResult(policies.GetValueOrDefault(policyName)); + Task.FromResult(policies.GetValueOrDefault(policyName)); public Task GetDefaultPolicyAsync() { diff --git a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs index 7155bde555..46167d2484 100644 --- a/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs +++ b/src/ServiceControl.Hosting/Auth/PermissionVerbHandler.cs @@ -18,9 +18,10 @@ namespace ServiceControl.Hosting.Auth; /// public sealed class PermissionVerbHandler : AuthorizationHandler { - // The per-IdP variability of the source claim is absorbed by RolesClaimsTransformation, which - // reads from the path configured in Authentication.RolesClaim and emits canonical "roles" claims. - const string RoleClaimType = "roles"; + public PermissionVerbHandler(string rolesClaimName) + { + RoleClaimType = rolesClaimName; + } protected override Task HandleRequirementAsync( AuthorizationHandlerContext context, @@ -28,7 +29,6 @@ protected override Task HandleRequirementAsync( { var roles = context.User.FindAll(RoleClaimType).Select(claim => claim.Value); - // TODO: Although plural, likely roles will only contain a single value unless we want to define a role for each instance but likely customers don't care about instances if (RolePermissions.IsGranted(roles, requirement.Permission)) { @@ -38,4 +38,6 @@ protected override Task HandleRequirementAsync( // Otherwise leave the requirement unmet → the request is denied (403/401). return Task.CompletedTask; } + + internal string RoleClaimType = "roles"; } diff --git a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs index f0ff2028a1..7375919263 100644 --- a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs @@ -7,7 +7,7 @@ namespace ServiceControl.Infrastructure.Auth; using System.Linq; /// -/// Hardcoded role → permission policy. Two roles for now: +/// Role → permission policy. Two roles: /// /// reader — granted every *:*:view permission (read-only access). /// writer — granted every permission (*:*:*). @@ -17,10 +17,6 @@ namespace ServiceControl.Infrastructure.Auth; /// immutable of granted permissions per role. As a result both /// and are O(1) hash lookups with no /// per-call pattern matching or allocation. -/// -/// TODO: interim hardcoded model — replace with a configurable role/permission mapping (loaded from -/// configuration or the IdP) when more than these two coarse roles are needed. -/// /// public static class RolePermissions { diff --git a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs index 23c4f58565..9c2d74158f 100644 --- a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs +++ b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs @@ -32,12 +32,8 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC ValidateIssuerSigningKey = SettingsReader.Read(rootNamespace, "Authentication.ValidateIssuerSigningKey", true); RequireHttpsMetadata = SettingsReader.Read(rootNamespace, "Authentication.RequireHttpsMetadata", true); - // Path within the JWT to the user's role values. May be a flat claim name (e.g. "roles" — the - // shape produced by Keycloak with a "User Realm Role" mapper, by Microsoft Entra ID, or by - // AWS Cognito as "cognito:groups") or a dotted path into a nested object claim (e.g. the - // Keycloak out-of-box shape "realm_access.roles"). The RolesClaimsTransformation reads from - // this path and flattens the values into canonical "roles" claims for the authorization handler. - RolesClaim = SettingsReader.Read(rootNamespace, "Authentication.RolesClaim", "realm_access.roles"); + RolesClaim = SettingsReader.Read(rootNamespace, "Authentication.RolesClaim", "roles"); + RoleBasedAuthorizationEnabled = SettingsReader.Read(rootNamespace, "Authentication.RoleBasedAuthorizationEnabled", false); // ServicePulse settings are only relevant for the primary ServiceControl instance // which serves the OIDC configuration endpoint that ServicePulse uses for login @@ -103,15 +99,6 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC /// public bool RequireHttpsMetadata { 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. - /// - public string RolesClaim { get; } - /// /// Optional override for the authority URL that ServicePulse should use for authentication. /// If not specified, ServicePulse uses the main Authority value. @@ -130,6 +117,21 @@ 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. + /// + public string RolesClaim { get; } + + /// + /// Is RBAC enabled. When false, all authenticated users have access to all methods. When true, + /// role based authorization rules are applied. + /// + public bool RoleBasedAuthorizationEnabled { get; } + void Validate(bool requireServicePulseSettings) { if (Enabled) diff --git a/src/ServiceControl.Monitoring.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs b/src/ServiceControl.Monitoring.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs index da0940e81c..0f94dba925 100644 --- a/src/ServiceControl.Monitoring.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.Monitoring.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -33,6 +33,7 @@ public void ConfigureAuth() configuration = new OpenIdConnectTestConfiguration(ServiceControlInstanceType.Monitoring) .WithConfigurationValidationDisabled() .WithAuthenticationEnabled() + .WithRoleBasedAuthorizationEnabled() .WithAuthority(mockOidcServer.Authority) .WithAudience(TestAudience) .WithRequireHttpsMetadata(false); diff --git a/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs b/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs index 319ea95329..aaab866c82 100644 --- a/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs +++ b/src/ServiceControl.Monitoring.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs @@ -100,7 +100,7 @@ async Task InitializeServiceControl(ScenarioContext context) hostBuilder.Logging.ConfigureLogging(LogLevel.Information); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlMonitoring((criticalErrorContext, cancellationToken) => { var logitem = new ScenarioContext.LogItem diff --git a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt index b29ddc3856..fed5a2acf4 100644 --- a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt @@ -12,10 +12,11 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, - "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, - "ServicePulseApiScopes": null + "ServicePulseApiScopes": null, + "RolesClaim": "roles", + "RoleBasedAuthorizationEnabled": false }, "ForwardedHeadersSettings": { "Enabled": true, diff --git a/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs b/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs index da23814397..6e197e463a 100644 --- a/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl.Monitoring/Hosting/Commands/RunCommand.cs @@ -16,7 +16,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControlMonitoring((_, __) => Task.CompletedTask, settings, endpointConfiguration); hostBuilder.AddServiceControlMonitoringApi(); diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 5ca8fcdf90..72e33e6946 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -12,10 +12,11 @@ "ValidateLifetime": true, "ValidateIssuerSigningKey": true, "RequireHttpsMetadata": true, - "RolesClaim": "realm_access.roles", "ServicePulseAuthority": null, "ServicePulseClientId": null, - "ServicePulseApiScopes": null + "ServicePulseApiScopes": null, + "RolesClaim": "roles", + "RoleBasedAuthorizationEnabled": false }, "ForwardedHeadersSettings": { "Enabled": true, diff --git a/src/ServiceControl/Hosting/Commands/RunCommand.cs b/src/ServiceControl/Hosting/Commands/RunCommand.cs index 1f895ec9d3..c25485bf5c 100644 --- a/src/ServiceControl/Hosting/Commands/RunCommand.cs +++ b/src/ServiceControl/Hosting/Commands/RunCommand.cs @@ -25,7 +25,7 @@ public override async Task Execute(HostArguments args, Settings settings) var hostBuilder = WebApplication.CreateBuilder(); hostBuilder.AddServiceControlAuthentication(settings.OpenIdConnectSettings); - hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings.Enabled); + hostBuilder.AddServiceControlAuthorization(settings.OpenIdConnectSettings); hostBuilder.AddServiceControlHttps(settings.HttpsSettings); hostBuilder.AddServiceControl(settings, endpointConfiguration); hostBuilder.AddServiceControlApi(settings.CorsSettings);