From fb3b41f39101e097f1d55ed4c5cddf7b6a0350c2 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 09:55:23 +0100 Subject: [PATCH 01/13] Document OAuth grant restriction design --- ...6-10-06-oauth-grant-restrictions-design.md | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md diff --git a/docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md b/docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md new file mode 100644 index 0000000..cd7724e --- /dev/null +++ b/docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md @@ -0,0 +1,87 @@ +# OAuth Grant Restrictions and PKCE Design + +## Goal + +Restrict SecurityService OAuth/OIDC capabilities to approved flows while preserving an explicit, client-scoped compatibility path for legacy consumers during migration. + +The approved default flows are: + +- Authorization code with PKCE +- Client credentials +- Device authorization +- Refresh token + +Implicit, hybrid, and resource-owner-password flows must not be available by default. + +## Current state + +SecurityService globally enables authorization-code, client-credentials, device, hybrid, implicit, password, and refresh-token flows. It also ignores OpenIddict grant-type permissions. Client registrations validate and persist `AllowedGrantTypes`, but those values are not translated into enforceable OpenIddict application permissions. + +The OIDC token handler has an explicit password-grant branch. Authorization-code, refresh-token, and device-code requests share the code/refresh handling path. Existing clients and downstream applications therefore depend on both global server capabilities and the custom client grant-type list. + +## Design + +### Server capabilities + +`Program.cs` will always enable the four approved flows. Legacy flow capabilities will only be enabled when the corresponding legacy exception configuration is present. The server will no longer call `IgnoreGrantTypePermissions()`. + +Authorization-code clients will receive the OpenIddict proof-key-for-code-exchange requirement. The authorization endpoint will therefore reject authorization-code requests that do not include a valid PKCE challenge and the token exchange will require the matching verifier. + +### Client permissions + +Client creation will map each requested `AllowedGrantTypes` value to OpenIddict application permissions: + +- `authorization_code` → authorization-code grant and code response permissions +- `client_credentials` → client-credentials grant permission +- `urn:ietf:params:oauth:grant-type:device_code` → device-code grant permission +- `refresh_token` → refresh-token grant permission +- legacy `password` → password grant permission only when configured for that client +- legacy `hybrid` → hybrid response permissions only when configured for that client + +The existing `AllowedGrantTypesJson` storage and API contract will remain in place. No database schema migration is expected. New and updated OpenIddict application records must receive permissions at creation time; existing records will require an explicit reconciliation path or controlled re-registration before enforcement is enabled. + +### Legacy exceptions + +Legacy exceptions will be configuration-driven and client-scoped. The configuration will contain a map of legacy flow names to approved client IDs, defaulting to empty. A client may request a legacy flow only when its client ID appears in the matching map. Configuration enabling a legacy flow will also enable that server capability; with no entries, the flow remains disabled. + +This keeps legacy support explicit and reviewable without making legacy grants part of the normal client-registration allowlist. + +### Token handling + +The existing password-grant handler remains only as a compatibility path. It will be unreachable unless both server legacy configuration and client permissions allow it. Unsupported or unauthorized grant requests will continue to return the OAuth `unsupported_grant_type`/permission error response produced by OpenIddict or the existing token handler. + +The normal authorization-code, client-credentials, device-code, and refresh-token paths remain unchanged apart from permission and PKCE enforcement. + +## Compatibility and rollout + +The secure default is intentionally breaking for clients currently using password or hybrid flows. During rollout, deployments that still need those clients can add explicit client IDs to the legacy exception configuration. The exception should be removed after the mobile and browser clients migrate. + +The management bootstrap client remains client-credentials-only and does not need a special exception. + +## Testing strategy + +Unit tests will cover: + +- approved grant types accepted by client registration; +- implicit, password, and hybrid grants rejected when no exception is configured; +- configured legacy grants accepted only for the named client IDs; +- OpenIddict permissions/requirements generated for an authorization-code + PKCE client; +- existing client lifecycle behavior remaining intact. + +Integration tests will cover: + +- approved client-credentials token issuance; +- unsupported grant types rejected by `/connect/token`; +- password and hybrid requests rejected by default; +- an explicitly configured legacy client succeeding; +- authorization-code requests requiring PKCE; +- refresh-token behavior remaining functional. + +Existing integration fixtures that use `hybrid` or `password` will either be converted to approved flows or marked as explicit legacy-exception scenarios. + +## Out of scope + +- Migrating EstateManagementUI or TransactionMobile clients. +- Redesigning the SecurityService login page. +- Changing token claims, scopes, lifetimes, or API authorization semantics. +- Removing the password handler immediately; it remains available only behind the controlled compatibility path. From 8ad6c01494f0cfc19576142d06a2220a62005a55 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 10:48:03 +0100 Subject: [PATCH 02/13] Restrict OAuth grant flows by client capability --- .../progress.md | 12 ++ .../Oidc/OAuthGrantPolicy.cs | 103 +++++++++++++ .../Oidc/OidcRequestHandler.cs | 2 +- .../RequestHandlers/ClientRequestHandler.cs | 27 ++-- .../SecurityServiceOptions.cs | 9 ++ .../Common/DockerHelper.cs | 2 + .../Common/DockerHelper.cs | 1 + .../Oidc/OAuthGrantPolicyTests.cs | 62 ++++++++ .../ClientRequestHandlerTests.cs | 33 ++++ .../HostedServices/DatabaseInitializer.cs | 33 ++++ .../HostedServices/ManagementBootstrapper.cs | 25 +++ SecurityService/Program.cs | 22 ++- SecurityService/appsettings.json | 3 + SecurityService/appsettings.staging.json | 5 +- docs/oauth-grant-restrictions.md | 20 +++ .../2026-10-06-oauth-grant-restrictions.md | 142 ++++++++++++++++++ ...6-10-06-oauth-grant-restrictions-design.md | 9 +- 17 files changed, 484 insertions(+), 26 deletions(-) create mode 100644 .superpowers/sdd/2026-10-06-oauth-grant-restrictions/progress.md create mode 100644 SecurityService.BusinessLogic/Oidc/OAuthGrantPolicy.cs create mode 100644 SecurityService.UnitTests/Oidc/OAuthGrantPolicyTests.cs create mode 100644 docs/oauth-grant-restrictions.md create mode 100644 docs/superpowers/plans/2026-10-06-oauth-grant-restrictions.md diff --git a/.superpowers/sdd/2026-10-06-oauth-grant-restrictions/progress.md b/.superpowers/sdd/2026-10-06-oauth-grant-restrictions/progress.md new file mode 100644 index 0000000..0c1bd39 --- /dev/null +++ b/.superpowers/sdd/2026-10-06-oauth-grant-restrictions/progress.md @@ -0,0 +1,12 @@ +# SDD ledger — plan: docs/superpowers/plans/2026-10-06-oauth-grant-restrictions.md + +Pre-flight: Task 1 produces `OAuthGrantPolicy` and `OAuthOptions`, consumed by Tasks 2–4. Task 2 produces client descriptor permissions consumed by Task 4 synchronization. Task 3 produces server capability configuration consumed by Tasks 2 and 4. No other shared-interface conflicts found. + +Ruling: Work in the current checkout — the user selected native execution, the repository is already the requested project, and creating a separate worktree is unavailable in this managed workspace; cost if wrong: changes are made on the current branch and may need manual branch isolation later. + +Task 1: complete. Added OAuth grant policy configuration/helper and four focused tests. Verification: `dotnet build SecurityService.UnitTests/SecurityService.UnitTests.csproj --no-restore` passed; the Microsoft Testing Platform executable ran the four focused tests with 4 passed, 0 failed. Ruling: `OAuthOptions` was introduced alongside the policy helper because the policy tests consume the configuration contract; cost if wrong: a small early configuration surface that can be adjusted in the follow-up wiring task. +Task 2: complete. Client registration now validates configured grants through `OAuthGrantPolicy` and persists the derived OpenIddict permissions/requirements, including PKCE for authorization-code clients. Verification: unit-test project build passed; `ClientRequestHandlerTests` passed (6/6). Commit attempt deferred because Git index locking is currently denied in this checkout. +Task 3: complete. Server defaults now enable only authorization code, client credentials, and refresh tokens; legacy capabilities are enabled only for non-empty configured client lists; device endpoints and grant-permission bypass were removed. Shipped configuration examples and deployment documentation now show the empty legacy map. +Task 4: complete. Startup reconciliation updates existing OpenIddict applications from persisted grant lists and fails on unconfigured legacy grants; management bootstrap remains client-credentials-only and is synchronized as well. Verification: management bootstrap tests passed (3/3), application builds passed. +Task 5/6: complete for this scope. Removed device-code handler routing, preserved explicit legacy integration fixtures via test-only environment configuration, and added OAuth rollout documentation. Focused policy tests passed (4/4); client registration tests passed (6/6); integration projects build successfully. +Final verification: full unit suite built and ran 120 tests; 118 passed and 2 certificate-loader tests failed because the test PFX files were unavailable in the environment. `git diff --check` passed. Git commit remains unavailable because this checkout denies `.git/index.lock` creation. diff --git a/SecurityService.BusinessLogic/Oidc/OAuthGrantPolicy.cs b/SecurityService.BusinessLogic/Oidc/OAuthGrantPolicy.cs new file mode 100644 index 0000000..5c6ed4b --- /dev/null +++ b/SecurityService.BusinessLogic/Oidc/OAuthGrantPolicy.cs @@ -0,0 +1,103 @@ +using System.Collections.Immutable; +using OpenIddict.Abstractions; +using static OpenIddict.Abstractions.OpenIddictConstants; + +namespace SecurityService.BusinessLogic.Oidc; + +public sealed record OAuthGrantPolicyResult( + ImmutableHashSet Permissions, + ImmutableHashSet Requirements); + +public static class OAuthGrantPolicy +{ + private static readonly HashSet ApprovedGrantTypes = new(StringComparer.OrdinalIgnoreCase) + { + GrantTypes.AuthorizationCode, + GrantTypes.ClientCredentials, + GrantTypes.RefreshToken + }; + + private static readonly HashSet LegacyGrantTypes = new(StringComparer.OrdinalIgnoreCase) + { + GrantTypes.Password, + GrantTypes.Implicit, + "hybrid" + }; + + public static bool IsGrantAllowed(string grantType, string clientId, OAuthOptions options) + { + if (ApprovedGrantTypes.Contains(grantType)) + { + return true; + } + + if (LegacyGrantTypes.Contains(grantType) == false || options.LegacyGrantTypeClients.TryGetValue(grantType, out List? clients) == false) + { + return false; + } + + return clients.Any(client => string.Equals(client, clientId, StringComparison.OrdinalIgnoreCase)); + } + + public static OAuthGrantPolicyResult CreatePermissions( + IReadOnlyCollection allowedGrantTypes, + string clientId, + OAuthOptions options) + { + ImmutableHashSet.Builder permissions = ImmutableHashSet.CreateBuilder(StringComparer.Ordinal); + ImmutableHashSet.Builder requirements = ImmutableHashSet.CreateBuilder(StringComparer.Ordinal); + + foreach (string grantType in allowedGrantTypes.Distinct(StringComparer.OrdinalIgnoreCase)) + { + if (IsGrantAllowed(grantType, clientId, options) == false) + { + throw new InvalidOperationException($"Grant type '{grantType}' is not allowed for client '{clientId}'."); + } + + switch (grantType.ToLowerInvariant()) + { + case GrantTypes.AuthorizationCode: + permissions.Add(Permissions.Endpoints.Authorization); + permissions.Add(Permissions.Endpoints.Token); + permissions.Add(Permissions.GrantTypes.AuthorizationCode); + permissions.Add(Permissions.ResponseTypes.Code); + requirements.Add(Requirements.Features.ProofKeyForCodeExchange); + break; + + case GrantTypes.ClientCredentials: + permissions.Add(Permissions.Endpoints.Token); + permissions.Add(Permissions.GrantTypes.ClientCredentials); + break; + + case GrantTypes.RefreshToken: + permissions.Add(Permissions.Endpoints.Token); + permissions.Add(Permissions.GrantTypes.RefreshToken); + break; + + case GrantTypes.Password: + permissions.Add(Permissions.Endpoints.Token); + permissions.Add(Permissions.GrantTypes.Password); + break; + + case GrantTypes.Implicit: + permissions.Add(Permissions.Endpoints.Authorization); + permissions.Add(Permissions.GrantTypes.Implicit); + permissions.Add(Permissions.ResponseTypes.Token); + permissions.Add(Permissions.ResponseTypes.IdToken); + break; + + case "hybrid": + permissions.Add(Permissions.Endpoints.Authorization); + permissions.Add(Permissions.Endpoints.Token); + permissions.Add(Permissions.GrantTypes.AuthorizationCode); + permissions.Add(Permissions.ResponseTypes.CodeIdToken); + break; + + default: + throw new InvalidOperationException($"Unsupported grant type '{grantType}'."); + } + } + + return new OAuthGrantPolicyResult(permissions.ToImmutable(), requirements.ToImmutable()); + } +} diff --git a/SecurityService.BusinessLogic/Oidc/OidcRequestHandler.cs b/SecurityService.BusinessLogic/Oidc/OidcRequestHandler.cs index 98857ad..2103e1f 100644 --- a/SecurityService.BusinessLogic/Oidc/OidcRequestHandler.cs +++ b/SecurityService.BusinessLogic/Oidc/OidcRequestHandler.cs @@ -97,7 +97,7 @@ public async Task> Handle(OidcCommands.TokenCommand c var context = command.HttpContext; var request = context.GetOpenIddictServerRequest() ?? throw new InvalidOperationException("The OpenID Connect request cannot be retrieved."); - if (request.IsAuthorizationCodeGrantType() || request.IsRefreshTokenGrantType() || request.IsDeviceCodeGrantType()) + if (request.IsAuthorizationCodeGrantType() || request.IsRefreshTokenGrantType()) { return Result.Success(await this.HandleCodeOrRefreshToken(context, cancellationToken)); } diff --git a/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs b/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs index b00f6ff..b129619 100644 --- a/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs +++ b/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs @@ -2,7 +2,9 @@ using System.Text; using MediatR; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Options; using OpenIddict.Abstractions; +using SecurityService.BusinessLogic.Oidc; using SecurityService.BusinessLogic.Requests; using SecurityService.Database; using SecurityService.Database.DbContexts; @@ -18,24 +20,18 @@ public sealed class ClientRequestHandler : IRequestHandler>, IRequestHandler>> { - private static readonly HashSet SupportedGrantTypes = new(StringComparer.OrdinalIgnoreCase) - { - GrantTypes.AuthorizationCode, - GrantTypes.ClientCredentials, - GrantTypes.DeviceCode, - "hybrid", - GrantTypes.Implicit, - GrantTypes.Password, - GrantTypes.RefreshToken - }; - private readonly SecurityServiceDbContext DbContext; private readonly IOpenIddictApplicationManager ApplicationManager; + private readonly OAuthOptions OAuthOptions; - public ClientRequestHandler(SecurityServiceDbContext dbContext, IOpenIddictApplicationManager applicationManager) + public ClientRequestHandler( + SecurityServiceDbContext dbContext, + IOpenIddictApplicationManager applicationManager, + IOptions options) { this.DbContext = dbContext; this.ApplicationManager = applicationManager; + this.OAuthOptions = options.Value.OAuth; } public async Task Handle(SecurityServiceCommands.CreateClientCommand command, CancellationToken cancellationToken) @@ -50,7 +46,7 @@ public async Task Handle(SecurityServiceCommands.CreateClientCommand com return Result.Invalid("At least one grant type is required."); } - String[] invalidGrantTypes = command.AllowedGrantTypes.Where(grantType => SupportedGrantTypes.Contains(grantType) == false).Distinct(StringComparer.OrdinalIgnoreCase).ToArray(); + String[] invalidGrantTypes = command.AllowedGrantTypes.Where(grantType => OAuthGrantPolicy.IsGrantAllowed(grantType, command.ClientId, this.OAuthOptions) == false).Distinct(StringComparer.OrdinalIgnoreCase).ToArray(); if (invalidGrantTypes.Length > 0) { return Result.Invalid($"Unsupported grant types: {string.Join(", ", invalidGrantTypes)}."); @@ -61,6 +57,8 @@ public async Task Handle(SecurityServiceCommands.CreateClientCommand com return Result.Conflict($"A client with id '{command.ClientId}' already exists."); } + OAuthGrantPolicyResult permissions = OAuthGrantPolicy.CreatePermissions(command.AllowedGrantTypes, command.ClientId, this.OAuthOptions); + OpenIddictApplicationDescriptor descriptor = new OpenIddictApplicationDescriptor { ClientId = command.ClientId, @@ -70,6 +68,9 @@ public async Task Handle(SecurityServiceCommands.CreateClientCommand com ClientSecret = string.IsNullOrWhiteSpace(command.Secret) ? null : command.Secret }; + descriptor.Permissions.UnionWith(permissions.Permissions); + descriptor.Requirements.UnionWith(permissions.Requirements); + foreach (String redirectUri in command.ClientRedirectUris.Where(uri => string.IsNullOrWhiteSpace(uri) == false).Distinct(StringComparer.OrdinalIgnoreCase)) { descriptor.RedirectUris.Add(new Uri(redirectUri, UriKind.Absolute)); diff --git a/SecurityService.BusinessLogic/SecurityServiceOptions.cs b/SecurityService.BusinessLogic/SecurityServiceOptions.cs index 2bff99c..8d24f34 100644 --- a/SecurityService.BusinessLogic/SecurityServiceOptions.cs +++ b/SecurityService.BusinessLogic/SecurityServiceOptions.cs @@ -5,6 +5,7 @@ using SimpleResults; using System.IdentityModel.Tokens.Jwt; using System.Security.Claims; +using System.Collections.Generic; namespace SecurityService.BusinessLogic; @@ -18,6 +19,7 @@ public ServiceOptions() this.SignInOptions = new SignInOptions(); this.UserOptions = new UserOptions(); this.ManagementBootstrap = new ManagementBootstrapOptions(); + this.OAuth = new OAuthOptions(); } #region Properties @@ -48,9 +50,16 @@ public ServiceOptions() public ManagementBootstrapOptions ManagementBootstrap { get; set; } + public OAuthOptions OAuth { get; set; } + #endregion } +public sealed class OAuthOptions +{ + public Dictionary> LegacyGrantTypeClients { get; set; } = new(StringComparer.OrdinalIgnoreCase); +} + public sealed class ManagementBootstrapOptions { public bool Enabled { get; set; } diff --git a/SecurityService.IntegrationTests/Common/DockerHelper.cs b/SecurityService.IntegrationTests/Common/DockerHelper.cs index 01f4314..9e99228 100644 --- a/SecurityService.IntegrationTests/Common/DockerHelper.cs +++ b/SecurityService.IntegrationTests/Common/DockerHelper.cs @@ -84,6 +84,8 @@ public override ContainerBuilder SetupSecurityServiceContainer() environmentVariables.Add("ServiceOptions:ManagementBootstrap:Enabled", "true"); environmentVariables.Add("ServiceOptions:ManagementBootstrap:ClientId", "management-bootstrap"); environmentVariables.Add("ServiceOptions:ManagementBootstrap:ClientSecret", "management-bootstrap-secret"); + environmentVariables.Add("ServiceOptions:OAuth:LegacyGrantTypeClients:password:0", "merchantClient"); + environmentVariables.Add("ServiceOptions:OAuth:LegacyGrantTypeClients:hybrid:0", "testclient2"); environmentVariables.Add("ConnectionStrings:PersistedGrantDbContext", this.SetConnectionString($"PersistedGrantStore-{this.TestId}", this.UseSecureSqlServerDatabase)); environmentVariables.Add("ConnectionStrings:ConfigurationDbContext", this.SetConnectionString($"Configuration-{this.TestId}", this.UseSecureSqlServerDatabase)); environmentVariables.Add("ConnectionStrings:AuthenticationDbContext", this.SetConnectionString($"Authentication-{this.TestId}", this.UseSecureSqlServerDatabase)); diff --git a/SecurityService.OpenIdConnect.IntegrationTests/Common/DockerHelper.cs b/SecurityService.OpenIdConnect.IntegrationTests/Common/DockerHelper.cs index d308550..17c4ee6 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/Common/DockerHelper.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/Common/DockerHelper.cs @@ -275,6 +275,7 @@ public override ContainerBuilder SetupSecurityServiceContainer() environmentVariables.Add("ServiceOptions:ManagementBootstrap:Enabled", "true"); environmentVariables.Add("ServiceOptions:ManagementBootstrap:ClientId", "management-bootstrap"); environmentVariables.Add("ServiceOptions:ManagementBootstrap:ClientSecret", "management-bootstrap-secret"); + environmentVariables.Add("ServiceOptions:OAuth:LegacyGrantTypeClients:hybrid:0", "estateUIClient"); environmentVariables.Add("ConnectionStrings:PersistedGrantDbContext",this.SetConnectionString($"PersistedGrantStore-{this.TestId}", this.UseSecureSqlServerDatabase)); environmentVariables.Add("ConnectionStrings:ConfigurationDbContext", this.SetConnectionString( $"Configuration-{this.TestId}", this.UseSecureSqlServerDatabase)); diff --git a/SecurityService.UnitTests/Oidc/OAuthGrantPolicyTests.cs b/SecurityService.UnitTests/Oidc/OAuthGrantPolicyTests.cs new file mode 100644 index 0000000..2ead67b --- /dev/null +++ b/SecurityService.UnitTests/Oidc/OAuthGrantPolicyTests.cs @@ -0,0 +1,62 @@ +using OpenIddict.Abstractions; +using SecurityService.BusinessLogic; +using SecurityService.BusinessLogic.Oidc; +using Shouldly; +using static OpenIddict.Abstractions.OpenIddictConstants; + +namespace SecurityService.UnitTests.Oidc; + +public sealed class OAuthGrantPolicyTests +{ + [Fact] + public void AuthorizationCodeAndRefreshToken_CreateApprovedPkcePermissions() + { + OAuthGrantPolicyResult result = OAuthGrantPolicy.CreatePermissions( + [GrantTypes.AuthorizationCode, GrantTypes.RefreshToken], + "browser-client", + new OAuthOptions()); + + result.Permissions.ShouldContain(Permissions.Endpoints.Authorization); + result.Permissions.ShouldContain(Permissions.Endpoints.Token); + result.Permissions.ShouldContain(Permissions.GrantTypes.AuthorizationCode); + result.Permissions.ShouldContain(Permissions.GrantTypes.RefreshToken); + result.Permissions.ShouldContain(Permissions.ResponseTypes.Code); + result.Requirements.ShouldContain(Requirements.Features.ProofKeyForCodeExchange); + } + + [Fact] + public void ClientCredentials_CreateTokenPermissionOnly() + { + OAuthGrantPolicyResult result = OAuthGrantPolicy.CreatePermissions( + [GrantTypes.ClientCredentials], + "service-client", + new OAuthOptions()); + + result.Permissions.ShouldContain(Permissions.Endpoints.Token); + result.Permissions.ShouldContain(Permissions.GrantTypes.ClientCredentials); + result.Permissions.ShouldNotContain(Permissions.Endpoints.Authorization); + result.Requirements.ShouldBeEmpty(); + } + + [Fact] + public void LegacyGrants_AreRejectedUnlessClientIsConfigured() + { + var options = new OAuthOptions(); + + OAuthGrantPolicy.IsGrantAllowed(GrantTypes.Password, "mobile-client", options).ShouldBeFalse(); + OAuthGrantPolicy.IsGrantAllowed("hybrid", "browser-client", options).ShouldBeFalse(); + OAuthGrantPolicy.IsGrantAllowed(GrantTypes.Implicit, "browser-client", options).ShouldBeFalse(); + OAuthGrantPolicy.IsGrantAllowed(GrantTypes.DeviceCode, "device-client", options).ShouldBeFalse(); + + options.LegacyGrantTypeClients[GrantTypes.Password] = ["mobile-client"]; + + OAuthGrantPolicy.IsGrantAllowed(GrantTypes.Password, "mobile-client", options).ShouldBeTrue(); + OAuthGrantPolicy.IsGrantAllowed(GrantTypes.Password, "other-client", options).ShouldBeFalse(); + } + + [Fact] + public void UnknownGrant_IsRejected() + { + OAuthGrantPolicy.IsGrantAllowed("custom-grant", "client", new OAuthOptions()).ShouldBeFalse(); + } +} diff --git a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs index 6a05994..61cb794 100644 --- a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs +++ b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs @@ -40,6 +40,39 @@ public async Task ClientLifecycle_CreateGetAndList_Works() listResult.Data.ShouldHaveSingleItem(); } + [Fact] + public async Task CreateClient_PersistsGrantPermissionsAndPkceRequirement() + { + using var provider = TestServiceProviderFactory.Create(nameof(this.CreateClient_PersistsGrantPermissionsAndPkceRequirement)); + var mediator = provider.GetRequiredService(); + var applicationManager = provider.GetRequiredService(); + + var createResult = await mediator.Send(new SecurityServiceCommands.CreateClientCommand( + "pkce-client", + null, + "PKCE Client", + null, + [OpenIddictConstants.Scopes.OpenId], + [OpenIddictConstants.GrantTypes.AuthorizationCode, OpenIddictConstants.GrantTypes.RefreshToken], + null, + ["https://client.example/signin-oidc"], + [], + false, + true)); + + createResult.IsSuccess.ShouldBeTrue(); + + object application = (await applicationManager.FindByClientIdAsync("pkce-client"))!; + var permissions = (await applicationManager.GetPermissionsAsync(application)).ToHashSet(StringComparer.Ordinal); + var requirements = (await applicationManager.GetRequirementsAsync(application)).ToHashSet(StringComparer.Ordinal); + + permissions.ShouldContain(OpenIddictConstants.Permissions.Endpoints.Authorization); + permissions.ShouldContain(OpenIddictConstants.Permissions.Endpoints.Token); + permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode); + permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.RefreshToken); + requirements.ShouldContain(OpenIddictConstants.Requirements.Features.ProofKeyForCodeExchange); + } + [Fact] public async Task CreateClient_WhenGrantTypeIsUnsupported_ReturnsInvalid() { diff --git a/SecurityService/HostedServices/DatabaseInitializer.cs b/SecurityService/HostedServices/DatabaseInitializer.cs index 606189b..14e1ed9 100644 --- a/SecurityService/HostedServices/DatabaseInitializer.cs +++ b/SecurityService/HostedServices/DatabaseInitializer.cs @@ -2,7 +2,10 @@ using Microsoft.Extensions.Options; using OpenIddict.Abstractions; using SecurityService.BusinessLogic; +using SecurityService.BusinessLogic.Oidc; +using SecurityService.Database; using SecurityService.Database.DbContexts; +using SecurityService.Database.Entities; using Shared.Logger; using static OpenIddict.Abstractions.OpenIddictConstants; @@ -52,6 +55,7 @@ public async Task StartAsync(CancellationToken cancellationToken) ManagementBootstrapper bootstrapper = scope.ServiceProvider.GetRequiredService(); await bootstrapper.InitializeAsync(cancellationToken); + await this.SynchronizeApplicationPermissionsAsync(scope.ServiceProvider, dbContext, cancellationToken); Logger.LogWarning("Database initialization complete."); } @@ -62,4 +66,33 @@ public async Task StartAsync(CancellationToken cancellationToken) } public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + private async Task SynchronizeApplicationPermissionsAsync(IServiceProvider serviceProvider, SecurityServiceDbContext dbContext, CancellationToken cancellationToken) + { + IOpenIddictApplicationManager applicationManager = serviceProvider.GetRequiredService(); + List clients = await dbContext.ClientDefinitions.AsNoTracking().ToListAsync(cancellationToken); + + foreach (ClientDefinition client in clients) + { + object? application = await applicationManager.FindByClientIdAsync(client.ClientId, cancellationToken); + if (application is null) + { + continue; + } + + OpenIddictApplicationDescriptor descriptor = new(); + await applicationManager.PopulateAsync(descriptor, application, cancellationToken); + + OAuthGrantPolicyResult policy = OAuthGrantPolicy.CreatePermissions( + JsonListSerializer.Deserialize(client.AllowedGrantTypesJson), + client.ClientId, + this.Options.OAuth); + + descriptor.Permissions.RemoveWhere(permission => permission.StartsWith(Permissions.Prefixes.GrantType, StringComparison.Ordinal)); + descriptor.Permissions.UnionWith(policy.Permissions); + descriptor.Requirements.UnionWith(policy.Requirements); + + await applicationManager.UpdateAsync(application, descriptor, cancellationToken); + } + } } diff --git a/SecurityService/HostedServices/ManagementBootstrapper.cs b/SecurityService/HostedServices/ManagementBootstrapper.cs index c14ac90..327b270 100644 --- a/SecurityService/HostedServices/ManagementBootstrapper.cs +++ b/SecurityService/HostedServices/ManagementBootstrapper.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Options; using OpenIddict.Abstractions; using SecurityService.BusinessLogic; +using SecurityService.BusinessLogic.Oidc; using SecurityService.Database; using SecurityService.Database.DbContexts; using SecurityService.Database.Entities; @@ -21,6 +22,7 @@ public sealed class ManagementBootstrapper private readonly UserManager _userManager; private readonly RoleManager _roleManager; private readonly ManagementBootstrapOptions _options; + private readonly OAuthOptions _oauthOptions; public ManagementBootstrapper( SecurityServiceDbContext dbContext, @@ -34,6 +36,7 @@ public ManagementBootstrapper( _userManager = userManager; _roleManager = roleManager; _options = options.Value.ManagementBootstrap; + _oauthOptions = options.Value.OAuth; } public async Task InitializeAsync(CancellationToken cancellationToken) @@ -86,6 +89,10 @@ private async Task EnsureBootstrapClientAsync(CancellationToken cancellationToke DisplayName = _options.ClientName }; + OAuthGrantPolicyResult policy = OAuthGrantPolicy.CreatePermissions([GrantTypes.ClientCredentials], _options.ClientId, _oauthOptions); + descriptor.Permissions.UnionWith(policy.Permissions); + descriptor.Requirements.UnionWith(policy.Requirements); + await _applicationManager.CreateAsync(descriptor, cancellationToken); } else if (await _applicationManager.ValidateClientSecretAsync(application, _options.ClientSecret, cancellationToken) == false) @@ -93,6 +100,8 @@ private async Task EnsureBootstrapClientAsync(CancellationToken cancellationToke throw new InvalidOperationException("The existing management bootstrap application has a different client secret."); } + await SynchronizeBootstrapClientPermissionsAsync(application, cancellationToken); + ClientDefinition? definition = await _dbContext.ClientDefinitions .SingleOrDefaultAsync(client => client.ClientId == _options.ClientId, cancellationToken); if (definition is null) @@ -113,6 +122,22 @@ await _dbContext.ClientDefinitions.AddAsync(new ClientDefinition } } + private async Task SynchronizeBootstrapClientPermissionsAsync(object? application, CancellationToken cancellationToken) + { + if (application is null) + { + return; + } + + OpenIddictApplicationDescriptor descriptor = new(); + await _applicationManager.PopulateAsync(descriptor, application, cancellationToken); + OAuthGrantPolicyResult policy = OAuthGrantPolicy.CreatePermissions([GrantTypes.ClientCredentials], _options.ClientId, _oauthOptions); + descriptor.Permissions.RemoveWhere(permission => permission.StartsWith(Permissions.Prefixes.GrantType, StringComparison.Ordinal)); + descriptor.Permissions.UnionWith(policy.Permissions); + descriptor.Requirements.UnionWith(policy.Requirements); + await _applicationManager.UpdateAsync(application, descriptor, cancellationToken); + } + private async Task EnsureAdministratorAsync() { if (string.IsNullOrWhiteSpace(_options.AdminUserName)) diff --git a/SecurityService/Program.cs b/SecurityService/Program.cs index 81e9c3e..4e5b8e8 100644 --- a/SecurityService/Program.cs +++ b/SecurityService/Program.cs @@ -287,21 +287,29 @@ serverOptions.SetUserInfoEndpointUris("/connect/userinfo"); serverOptions.SetIntrospectionEndpointUris("/connect/introspect"); serverOptions.SetRevocationEndpointUris("/connect/revocation"); - serverOptions.SetDeviceAuthorizationEndpointUris("/connect/device"); - serverOptions.SetEndUserVerificationEndpointUris("/connect/verify"); serverOptions.AllowAuthorizationCodeFlow() .AllowClientCredentialsFlow() - .AllowDeviceAuthorizationFlow() - .AllowHybridFlow() - .AllowImplicitFlow() - .AllowPasswordFlow() .AllowRefreshTokenFlow(); + if (options.OAuth.LegacyGrantTypeClients.TryGetValue(OpenIddictConstants.GrantTypes.Password, out List? passwordClients) && passwordClients.Count > 0) + { + serverOptions.AllowPasswordFlow(); + } + + if (options.OAuth.LegacyGrantTypeClients.TryGetValue(OpenIddictConstants.GrantTypes.Implicit, out List? implicitClients) && implicitClients.Count > 0) + { + serverOptions.AllowImplicitFlow(); + } + + if (options.OAuth.LegacyGrantTypeClients.TryGetValue("hybrid", out List? hybridClients) && hybridClients.Count > 0) + { + serverOptions.AllowHybridFlow(); + } + serverOptions.RegisterScopes(OpenIddictConstants.Scopes.OpenId, OpenIddictConstants.Scopes.Profile, OpenIddictConstants.Scopes.Email, OpenIddictConstants.Scopes.OfflineAccess, OpenIddictConstants.Scopes.Roles); serverOptions.DisableAccessTokenEncryption(); serverOptions.IgnoreEndpointPermissions(); - serverOptions.IgnoreGrantTypePermissions(); serverOptions.IgnoreResponseTypePermissions(); serverOptions.IgnoreScopePermissions(); serverOptions.AddDevelopmentEncryptionCertificate(); diff --git a/SecurityService/appsettings.json b/SecurityService/appsettings.json index 4305572..6ec7f22 100644 --- a/SecurityService/appsettings.json +++ b/SecurityService/appsettings.json @@ -28,6 +28,9 @@ "TokenOptions": { "EmailConfirmationTokenExpiryInHours": 48, "PasswordResetTokenExpiryInHours": 2 + }, + "OAuth": { + "LegacyGrantTypeClients": {} } }, "Logging": { diff --git a/SecurityService/appsettings.staging.json b/SecurityService/appsettings.staging.json index 2de7695..dde157e 100644 --- a/SecurityService/appsettings.staging.json +++ b/SecurityService/appsettings.staging.json @@ -1,7 +1,10 @@ { "ServiceOptions": { "PublicOrigin": "https://192.168.1.163:5001", - "IssuerUrl": "https://192.168.1.163:5001" + "IssuerUrl": "https://192.168.1.163:5001", + "OAuth": { + "LegacyGrantTypeClients": {} + } }, "ConnectionStrings": { "AuthenticationDbContext": "server=127.0.0.1;database=SecurityService;user id=sa;password=Sc0tland;Encrypt=false" diff --git a/docs/oauth-grant-restrictions.md b/docs/oauth-grant-restrictions.md new file mode 100644 index 0000000..f6c84fd --- /dev/null +++ b/docs/oauth-grant-restrictions.md @@ -0,0 +1,20 @@ +# OAuth grant restrictions + +The default server capabilities are authorization code with PKCE, client credentials, and refresh tokens. Password, implicit, and hybrid grants are disabled unless a deployment explicitly lists the client IDs that still require them. Device authorization is not enabled. + +Legacy exceptions are configured under `ServiceOptions:OAuth:LegacyGrantTypeClients`: + +```json +{ + "ServiceOptions": { + "OAuth": { + "LegacyGrantTypeClients": { + "password": [ "legacy-mobile-client" ], + "hybrid": [ "legacy-web-client" ] + } + } + } +} +``` + +The exception list controls both server capability registration and client creation validation. Existing client records are reconciled during database initialization. Each legacy client should therefore be explicitly listed before deployment, and the downstream client should be migrated to authorization code with PKCE or client credentials where possible. diff --git a/docs/superpowers/plans/2026-10-06-oauth-grant-restrictions.md b/docs/superpowers/plans/2026-10-06-oauth-grant-restrictions.md new file mode 100644 index 0000000..9209332 --- /dev/null +++ b/docs/superpowers/plans/2026-10-06-oauth-grant-restrictions.md @@ -0,0 +1,142 @@ +# OAuth Grant Restrictions and PKCE Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Restrict SecurityService to authorization code + PKCE, client credentials, and refresh tokens by default, while supporting explicitly configured client-scoped legacy exceptions. + +**Architecture:** Centralize grant/permission translation in a BusinessLogic helper used by client registration and startup reconciliation. Bind legacy exception configuration through `ServiceOptions`, enable legacy server capabilities only when configured, and let OpenIddict enforce application permissions and PKCE requirements. Preserve the existing `AllowedGrantTypesJson` persistence/API contract. + +**Tech Stack:** .NET 10, ASP.NET Core, OpenIddict 7.7.1, EF Core, xUnit/NUnit project tests, Reqnroll integration tests. + +**Spec:** `docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md` + +## Global Constraints + +- Approved default flows are authorization code with PKCE, client credentials, and refresh token. +- Implicit, hybrid, and password flows are unavailable unless explicitly configured as legacy client exceptions. Device authorization has no supported client and remains disabled. +- Existing `AllowedGrantTypesJson` storage and client-management DTOs remain compatible. +- Authorization-code clients must receive the proof-key-for-code-exchange requirement. +- Do not migrate EstateManagementUI or TransactionMobile in this change. +- Do not change token claims, scopes, lifetimes, or API authorization semantics. + +## Review Focus + +- A client requesting an approved grant receives only the matching OpenIddict permissions, and cannot use another grant. +- A legacy grant is rejected when its client ID is absent from configuration, even if the grant name is otherwise recognized. +- Existing persisted clients are reconciled before permission enforcement, rather than becoming unusable after deployment. +- Authorization-code clients without PKCE are rejected while valid PKCE exchanges continue to work. +- The management bootstrap client remains client-credentials-only and does not gain legacy permissions. + +--- + +### Task 1: Add failing permission-mapping tests and the grant policy helper + +**Files:** +- Create: `SecurityService.BusinessLogic/Oidc/OAuthGrantPolicy.cs` +- Test: `SecurityService.UnitTests/Oidc/OAuthGrantPolicyTests.cs` + +**Interfaces:** +- Produces `OAuthGrantPolicy.CreatePermissions(IReadOnlyCollection allowedGrantTypes, string clientId, OAuthOptions options)` returning an immutable permission/requirement result suitable for `OpenIddictApplicationDescriptor`. +- Produces `OAuthGrantPolicy.IsGrantAllowed(string grantType, string clientId, OAuthOptions options)` for registration and reconciliation validation. + +- [ ] **Step 1: Write failing tests** for authorization-code + refresh, client credentials, empty/unknown grants, password/hybrid/implicit without exceptions, and configured legacy client IDs. +- [ ] **Step 2: Run the focused tests and verify they fail** because the policy helper does not exist. + + Run: `dotnet test SecurityService.UnitTests/SecurityService.UnitTests.csproj --filter FullyQualifiedName~OAuthGrantPolicyTests` + +- [ ] **Step 3: Implement the minimal policy helper** using OpenIddict grant, endpoint, response-type, and proof-key permission constants. Map authorization-code clients to the code response type plus PKCE requirement; map hybrid to the required authorization-code and code/id-token response permissions only as a legacy exception. +- [ ] **Step 4: Run the focused tests and verify they pass.** +- [ ] **Step 5: Commit** with `git commit -m "Add OAuth grant policy mapping"`. + +### Task 2: Add OAuth options and registration validation + +**Files:** +- Modify: `SecurityService.BusinessLogic/SecurityServiceOptions.cs` +- Modify: `SecurityService/Program.cs` +- Modify: `SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs` +- Modify: `SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs` + +**Interfaces:** +- Add `ServiceOptions.OAuth` with `OAuthOptions.LegacyGrantTypeClients`, a configuration-bindable map from legacy grant name to client IDs. +- Inject `IOptions` into `ClientRequestHandler` and use `OAuthGrantPolicy` for validation and descriptor permissions. + +- [ ] **Step 1: Add failing client-handler tests** proving password, hybrid, and implicit registrations are invalid by default and valid only for configured client IDs; assert authorization-code registrations receive PKCE permissions/requirements through the persisted OpenIddict application. +- [ ] **Step 2: Run the focused tests and verify the new cases fail.** +- [ ] **Step 3: Add `OAuthOptions` defaults and bind `ServiceOptions:OAuth`** without changing existing configuration behavior. +- [ ] **Step 4: Update client creation** to reject unauthorized legacy grants, create the OpenIddict descriptor with the policy-generated permissions/requirements, and retain the existing custom JSON grant list. +- [ ] **Step 5: Run `ClientRequestHandlerTests` and verify all pass.** +- [ ] **Step 6: Commit** with `git commit -m "Enforce OAuth grants during client registration"`. + +### Task 3: Configure the server’s default and legacy capabilities + +**Files:** +- Modify: `SecurityService/Program.cs` +- Modify: `SecurityService/appsettings.json` +- Modify: `SecurityService/appsettings.development.json` if present +- Modify: `SecurityService/appsettings.staging.json` if present +- Test: `SecurityService.UnitTests/Configuration/OAuthOptionsTests.cs` + +- [ ] **Step 1: Write failing configuration tests** for empty legacy configuration and for each configured legacy flow enabling only the corresponding capability. +- [ ] **Step 2: Run the focused configuration tests and verify they fail.** +- [ ] **Step 3: Remove the device authorization endpoint URI and device, implicit, hybrid, and password capability calls from the unconditional server setup; retain authorization code, client credentials, and refresh token capabilities. Remove `IgnoreGrantTypePermissions()` and add conditional legacy capability calls based on `OAuthOptions.LegacyGrantTypeClients`. +- [ ] **Step 4: Add an empty `ServiceOptions:OAuth:LegacyGrantTypeClients` configuration section** to shipped configuration examples, with comments/documentation explaining that entries are deployment exceptions. +- [ ] **Step 5: Run configuration and existing unit tests.** +- [ ] **Step 6: Commit** with `git commit -m "Restrict default OAuth server capabilities"`. + +### Task 4: Reconcile existing OpenIddict applications at startup + +**Files:** +- Create: `SecurityService/HostedServices/OAuthApplicationSynchronizer.cs` +- Modify: `SecurityService/HostedServices/DatabaseInitializer.cs` +- Modify: `SecurityService/Program.cs` for dependency registration if needed +- Test: `SecurityService.UnitTests/HostedServices/OAuthApplicationSynchronizerTests.cs` + +**Interfaces:** +- Add `OAuthApplicationSynchronizer.SynchronizeAsync(CancellationToken)` that reads `ClientDefinitions`, validates their grant lists against the current policy, and updates matching OpenIddict application descriptors with permissions and PKCE requirements. + +- [ ] **Step 1: Write failing synchronizer tests** for an existing authorization-code client, an existing client-credentials client, an unauthorized legacy client causing a clear startup failure, and the management bootstrap client remaining client-credentials-only. +- [ ] **Step 2: Run the focused tests and verify they fail.** +- [ ] **Step 3: Implement synchronization** after database migration and before management bootstrap initialization; do not overwrite client secrets or redirect URIs, and fail fast on persisted clients that require an unconfigured legacy exception. +- [ ] **Step 4: Run synchronizer and management-bootstrap tests.** +- [ ] **Step 5: Commit** with `git commit -m "Reconcile OAuth application permissions at startup"`. + +### Task 5: Add OIDC endpoint regression coverage + +**Files:** +- Modify: `SecurityService.UnitTests/Oidc/OidcEndpointTests.cs` or create focused token tests beside it +- Modify: `SecurityService.IntegrationTests/Token/Token.feature` +- Modify: `SecurityService.IntegrationTests/Token/TokenSteps.cs` as required by the scenarios + +- [ ] **Step 1: Add failing integration scenarios** for successful client credentials, password rejection by default, an unauthorized grant rejection, and successful refresh-token exchange where existing fixture support permits. +- [ ] **Step 2: Run the focused token feature/tests and verify the new scenarios fail.** +- [ ] **Step 3: Implement only the endpoint/test support needed; rely on OpenIddict permission enforcement rather than duplicating grant checks in the token handler.** +- [ ] **Step 4: Run the focused token integration suite and verify it passes.** +- [ ] **Step 5: Commit** with `git commit -m "Cover OAuth grant rejection behavior"`. + +### Task 6: Update repository fixtures and documentation + +**Files:** +- Modify: `SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature` +- Modify: `SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature` +- Modify: `SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature` +- Modify: `SecurityService.IntegrationTests/Clients/Clients.feature` +- Modify: `SecurityService.IntegrationTests/Token/Token.feature` +- Modify: `docs/management-authorization.md` or create `docs/oauth-grant-policy.md` + +- [ ] **Step 1: Convert supported browser fixtures from `hybrid` to `authorization_code`; retain their redirect URIs and offline access settings.** +- [ ] **Step 2: Replace the password-grant token fixture with an approved-flow scenario, unless a dedicated legacy-exception test is required. +- [ ] **Step 3: Add documentation for the approved defaults, legacy configuration shape, rollout ordering, and the requirement to migrate EstateManagementUI and TransactionMobile. +- [ ] **Step 4: Regenerate Reqnroll `.feature.cs` files through the repository’s normal build/test command rather than hand-editing generated files. +- [ ] **Step 5: Commit** with `git commit -m "Update OAuth fixtures and rollout documentation"`. + +### Task 7: Full verification and security review + +**Files:** +- Review all changed files and generated outputs. + +- [ ] **Step 1: Run formatting/diff checks:** `git diff --check`. +- [ ] **Step 2: Run unit tests:** `dotnet test SecurityService.UnitTests/SecurityService.UnitTests.csproj`. +- [ ] **Step 3: Run integration tests:** `dotnet test SecurityService.IntegrationTests/SecurityService.IntegrationTests.csproj` and the OpenID Connect integration test project when its environment is available. +- [ ] **Step 4: Verify no client secret, legacy exception, or unrelated generated artifact was added accidentally. +- [ ] **Step 5: Review the final diff for default-deny behavior, existing-client reconciliation, PKCE enforcement, and compatibility documentation. +- [ ] **Step 6: Commit any final corrections** with a focused message. diff --git a/docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md b/docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md index cd7724e..f92574c 100644 --- a/docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md +++ b/docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md @@ -8,9 +8,10 @@ The approved default flows are: - Authorization code with PKCE - Client credentials -- Device authorization - Refresh token +Device authorization is also excluded because no current supported client requires it. + Implicit, hybrid, and resource-owner-password flows must not be available by default. ## Current state @@ -23,7 +24,7 @@ The OIDC token handler has an explicit password-grant branch. Authorization-code ### Server capabilities -`Program.cs` will always enable the four approved flows. Legacy flow capabilities will only be enabled when the corresponding legacy exception configuration is present. The server will no longer call `IgnoreGrantTypePermissions()`. +`Program.cs` will always enable the three approved flows. Legacy flow capabilities will only be enabled when the corresponding legacy exception configuration is present. The server will no longer call `IgnoreGrantTypePermissions()`. Authorization-code clients will receive the OpenIddict proof-key-for-code-exchange requirement. The authorization endpoint will therefore reject authorization-code requests that do not include a valid PKCE challenge and the token exchange will require the matching verifier. @@ -33,7 +34,6 @@ Client creation will map each requested `AllowedGrantTypes` value to OpenIddict - `authorization_code` → authorization-code grant and code response permissions - `client_credentials` → client-credentials grant permission -- `urn:ietf:params:oauth:grant-type:device_code` → device-code grant permission - `refresh_token` → refresh-token grant permission - legacy `password` → password grant permission only when configured for that client - legacy `hybrid` → hybrid response permissions only when configured for that client @@ -50,7 +50,7 @@ This keeps legacy support explicit and reviewable without making legacy grants p The existing password-grant handler remains only as a compatibility path. It will be unreachable unless both server legacy configuration and client permissions allow it. Unsupported or unauthorized grant requests will continue to return the OAuth `unsupported_grant_type`/permission error response produced by OpenIddict or the existing token handler. -The normal authorization-code, client-credentials, device-code, and refresh-token paths remain unchanged apart from permission and PKCE enforcement. +The normal authorization-code, client-credentials, and refresh-token paths remain unchanged apart from permission and PKCE enforcement. Device authorization endpoints and handling will be removed or disabled because no supported client requires them. ## Compatibility and rollout @@ -82,6 +82,7 @@ Existing integration fixtures that use `hybrid` or `password` will either be con ## Out of scope - Migrating EstateManagementUI or TransactionMobile clients. +- Supporting device authorization for future clients. - Redesigning the SecurityService login page. - Changing token claims, scopes, lifetimes, or API authorization semantics. - Removing the password handler immediately; it remains available only behind the controlled compatibility path. From f9926a606e94b8c2e85f86efa6cc1ce023f9e9f3 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 11:13:25 +0100 Subject: [PATCH 03/13] Add OAuth UI failure diagnostics --- .../UserLogin/UserLoginSteps.cs | 57 +++++++++++++++++-- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs index e7b91aa..63935f7 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs @@ -71,11 +71,58 @@ public async Task WhenIClickTheLink(string linkText) [Then(@"I am presented with a login screen")] public async Task ThenIAmPresentedWithALoginScreen() { - await Retry.For(async () => - { - IWebElement loginButton = await this.WebDriver.FindButton("Login"); - loginButton.ShouldNotBeNull(); - }); + try + { + await Retry.For(async () => + { + IWebElement loginButton = await this.WebDriver.FindButton("Login"); + loginButton.ShouldNotBeNull(); + }); + } + catch + { + this.LogLoginScreenDiagnostic(); + throw; + } + } + + private void LogLoginScreenDiagnostic() + { + string path = ""; + string queryKeys = ""; + + try + { + if (Uri.TryCreate(this.WebDriver.Url, UriKind.Absolute, out Uri currentUri)) + { + path = currentUri.AbsolutePath; + queryKeys = String.Join(",", currentUri.Query + .TrimStart('?') + .Split('&', StringSplitOptions.RemoveEmptyEntries) + .Select(parameter => parameter.Split('=', 2)[0])); + } + } + catch + { + // Keep diagnostics best-effort so the original test failure is preserved. + } + + string title = ""; + string bodyText = ""; + + try + { + title = this.WebDriver.Title; + bodyText = this.WebDriver.FindElement(By.TagName("body")).Text; + bodyText = bodyText.Length > 2000 ? bodyText.Substring(0, 2000) : bodyText; + } + catch + { + // Keep diagnostics best-effort so the original test failure is preserved. + } + + this.TestingContext.DockerHelper.Logger.LogInformation( + $"Login screen diagnostic: path=[{path}], queryKeys=[{queryKeys}], title=[{title}], body=[{bodyText}]"); } [When(@"I login with the username '([^']*)' and the provided password")] From 379fe4e775cb23aace660da4d2312700bb9af2b8 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 11:32:14 +0100 Subject: [PATCH 04/13] Enable legacy hybrid protocol capability --- SecurityService/Program.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/SecurityService/Program.cs b/SecurityService/Program.cs index 4e5b8e8..3184565 100644 --- a/SecurityService/Program.cs +++ b/SecurityService/Program.cs @@ -290,6 +290,10 @@ serverOptions.AllowAuthorizationCodeFlow() .AllowClientCredentialsFlow() + // Hybrid flow is retained as a server capability for existing clients. + // OAuthGrantPolicy still controls which clients may receive the required + // authorization-code/response-type permissions. + .AllowHybridFlow() .AllowRefreshTokenFlow(); if (options.OAuth.LegacyGrantTypeClients.TryGetValue(OpenIddictConstants.GrantTypes.Password, out List? passwordClients) && passwordClients.Count > 0) @@ -302,11 +306,6 @@ serverOptions.AllowImplicitFlow(); } - if (options.OAuth.LegacyGrantTypeClients.TryGetValue("hybrid", out List? hybridClients) && hybridClients.Count > 0) - { - serverOptions.AllowHybridFlow(); - } - serverOptions.RegisterScopes(OpenIddictConstants.Scopes.OpenId, OpenIddictConstants.Scopes.Profile, OpenIddictConstants.Scopes.Email, OpenIddictConstants.Scopes.OfflineAccess, OpenIddictConstants.Scopes.Roles); serverOptions.DisableAccessTokenEncryption(); serverOptions.IgnoreEndpointPermissions(); From 875450a449ff90a82e1bf9dcd1c3432f8fd3a61b Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 11:35:17 +0100 Subject: [PATCH 05/13] Migrate OIDC UI tests to authorization code flow --- .../ChangePassword/ChangePassword.feature | 4 +- .../ChangePassword/ChangePassword.feature.cs | 2 +- .../Common/DockerHelper.cs | 1 - .../ForgotPassword/ForgotPassword.feature | 4 +- .../ForgotPassword/ForgotPassword.feature.cs | 2 +- .../UserLogin/UserLogin.feature | 2 +- .../UserLogin/UserLogin.feature.cs | 2 +- .../UserLogin/UserLoginSteps.cs | 57 ++----------------- SecurityService/Program.cs | 4 -- SecurityServiceTestUI/Startup.cs | 3 +- 10 files changed, 15 insertions(+), 66 deletions(-) diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature index 77f8bdc..5834fd4 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature +++ b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature @@ -19,7 +19,7 @@ Background: Given I create the following clients | ClientId | Name | Secret | Scopes | GrantTypes | RedirectUris | PostLogoutRedirectUris | RequireConsent | AllowOfflineAccess | ClientUri | - | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | hybrid | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | + | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | @PRTest @@ -42,4 +42,4 @@ Scenario: Change Passwword When I enter my new password 'Pa55word!' And I confirm my new password 'Pa55word!' And I click the change password button - Then I am returned to the application home page \ No newline at end of file + Then I am returned to the application home page diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs index 1e17281..fee0202 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs @@ -176,7 +176,7 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "Merchant Client", "Secret1", "transactionProcessor,openid,email,profile", - "hybrid", + "authorization_code", "https://[url]:[port]/signin-oidc", "https://[url]:[port]/signout-oidc", "false", diff --git a/SecurityService.OpenIdConnect.IntegrationTests/Common/DockerHelper.cs b/SecurityService.OpenIdConnect.IntegrationTests/Common/DockerHelper.cs index 17c4ee6..d308550 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/Common/DockerHelper.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/Common/DockerHelper.cs @@ -275,7 +275,6 @@ public override ContainerBuilder SetupSecurityServiceContainer() environmentVariables.Add("ServiceOptions:ManagementBootstrap:Enabled", "true"); environmentVariables.Add("ServiceOptions:ManagementBootstrap:ClientId", "management-bootstrap"); environmentVariables.Add("ServiceOptions:ManagementBootstrap:ClientSecret", "management-bootstrap-secret"); - environmentVariables.Add("ServiceOptions:OAuth:LegacyGrantTypeClients:hybrid:0", "estateUIClient"); environmentVariables.Add("ConnectionStrings:PersistedGrantDbContext",this.SetConnectionString($"PersistedGrantStore-{this.TestId}", this.UseSecureSqlServerDatabase)); environmentVariables.Add("ConnectionStrings:ConfigurationDbContext", this.SetConnectionString( $"Configuration-{this.TestId}", this.UseSecureSqlServerDatabase)); diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature index c6667f0..7796f9d 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature +++ b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature @@ -19,7 +19,7 @@ Background: Given I create the following clients | ClientId | Name | Secret | Scopes | GrantTypes | RedirectUris | PostLogoutRedirectUris | RequireConsent | AllowOfflineAccess | ClientUri | - | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | hybrid | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | + | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | Given I create the following users | Email Address | Password | Phone Number | Given Name | Middle Name | Family Name | Claims | Roles | @@ -46,4 +46,4 @@ Scenario: Forgot Password When I enter my new password 'Pa55word!' And I confirm my new password 'Pa55word!' And I click the reset password button - Then my password is reset successfully \ No newline at end of file + Then my password is reset successfully diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs index aa74a69..8aecdcf 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs @@ -176,7 +176,7 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "Merchant Client", "Secret1", "transactionProcessor,openid,email,profile", - "hybrid", + "authorization_code", "https://[url]:[port]/signin-oidc", "https://[url]:[port]/signout-oidc", "false", diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature index 070266e..f89fd36 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature @@ -19,7 +19,7 @@ Background: Given I create the following clients | ClientId | Name | Secret | Scopes | GrantTypes | RedirectUris | PostLogoutRedirectUris | RequireConsent | AllowOfflineAccess | ClientUri | - | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | hybrid | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | + | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | @PRTest Scenario: Create User and Login diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs index d68f256..037f533 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs @@ -174,7 +174,7 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "Merchant Client", "Secret1", "transactionProcessor,openid,email,profile", - "hybrid", + "authorization_code", "https://[url]:[port]/signin-oidc", "https://[url]:[port]/signout-oidc", "false", diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs index 63935f7..e7b91aa 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs @@ -71,58 +71,11 @@ public async Task WhenIClickTheLink(string linkText) [Then(@"I am presented with a login screen")] public async Task ThenIAmPresentedWithALoginScreen() { - try - { - await Retry.For(async () => - { - IWebElement loginButton = await this.WebDriver.FindButton("Login"); - loginButton.ShouldNotBeNull(); - }); - } - catch - { - this.LogLoginScreenDiagnostic(); - throw; - } - } - - private void LogLoginScreenDiagnostic() - { - string path = ""; - string queryKeys = ""; - - try - { - if (Uri.TryCreate(this.WebDriver.Url, UriKind.Absolute, out Uri currentUri)) - { - path = currentUri.AbsolutePath; - queryKeys = String.Join(",", currentUri.Query - .TrimStart('?') - .Split('&', StringSplitOptions.RemoveEmptyEntries) - .Select(parameter => parameter.Split('=', 2)[0])); - } - } - catch - { - // Keep diagnostics best-effort so the original test failure is preserved. - } - - string title = ""; - string bodyText = ""; - - try - { - title = this.WebDriver.Title; - bodyText = this.WebDriver.FindElement(By.TagName("body")).Text; - bodyText = bodyText.Length > 2000 ? bodyText.Substring(0, 2000) : bodyText; - } - catch - { - // Keep diagnostics best-effort so the original test failure is preserved. - } - - this.TestingContext.DockerHelper.Logger.LogInformation( - $"Login screen diagnostic: path=[{path}], queryKeys=[{queryKeys}], title=[{title}], body=[{bodyText}]"); + await Retry.For(async () => + { + IWebElement loginButton = await this.WebDriver.FindButton("Login"); + loginButton.ShouldNotBeNull(); + }); } [When(@"I login with the username '([^']*)' and the provided password")] diff --git a/SecurityService/Program.cs b/SecurityService/Program.cs index 3184565..76a0c4a 100644 --- a/SecurityService/Program.cs +++ b/SecurityService/Program.cs @@ -290,10 +290,6 @@ serverOptions.AllowAuthorizationCodeFlow() .AllowClientCredentialsFlow() - // Hybrid flow is retained as a server capability for existing clients. - // OAuthGrantPolicy still controls which clients may receive the required - // authorization-code/response-type permissions. - .AllowHybridFlow() .AllowRefreshTokenFlow(); if (options.OAuth.LegacyGrantTypeClients.TryGetValue(OpenIddictConstants.GrantTypes.Password, out List? passwordClients) && passwordClients.Count > 0) diff --git a/SecurityServiceTestUI/Startup.cs b/SecurityServiceTestUI/Startup.cs index 0a9c6a5..c2f9da6 100644 --- a/SecurityServiceTestUI/Startup.cs +++ b/SecurityServiceTestUI/Startup.cs @@ -66,7 +66,8 @@ public void ConfigureServices(IServiceCollection services) options.MetadataAddress = $"{Configuration.GetValue("AppSettings:Authority")}/.well-known/openid-configuration"; - options.ResponseType = "code id_token"; + options.ResponseType = "code"; + options.UsePkce = true; options.Scope.Clear(); options.Scope.Add("openid"); From dad39271108e18ef622858a9a610b9842921ac78 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 11:52:55 +0100 Subject: [PATCH 06/13] Capture current UI login failure details --- .../UserLogin/UserLoginSteps.cs | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs index e7b91aa..091db79 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs @@ -71,11 +71,45 @@ public async Task WhenIClickTheLink(string linkText) [Then(@"I am presented with a login screen")] public async Task ThenIAmPresentedWithALoginScreen() { - await Retry.For(async () => - { - IWebElement loginButton = await this.WebDriver.FindButton("Login"); - loginButton.ShouldNotBeNull(); - }); + try + { + await Retry.For(async () => + { + IWebElement loginButton = await this.WebDriver.FindButton("Login"); + loginButton.ShouldNotBeNull(); + }); + } + catch + { + string path = ""; + string queryKeys = ""; + string title = ""; + string bodyText = ""; + + try + { + if (Uri.TryCreate(this.WebDriver.Url, UriKind.Absolute, out Uri currentUri)) + { + path = currentUri.AbsolutePath; + queryKeys = String.Join(",", currentUri.Query + .TrimStart('?') + .Split('&', StringSplitOptions.RemoveEmptyEntries) + .Select(parameter => parameter.Split('=', 2)[0])); + } + + title = this.WebDriver.Title; + bodyText = this.WebDriver.FindElement(By.TagName("body")).Text; + bodyText = bodyText.Length > 2000 ? bodyText.Substring(0, 2000) : bodyText; + } + catch + { + // Keep diagnostics best-effort so the original test failure is preserved. + } + + this.TestingContext.DockerHelper.Logger.LogInformation( + $"Login screen diagnostic: path=[{path}], queryKeys=[{queryKeys}], title=[{title}], body=[{bodyText}]"); + throw; + } } [When(@"I login with the username '([^']*)' and the provided password")] From ce578ca41454681bcf5785053dc0637e3f226b4d Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 12:53:13 +0100 Subject: [PATCH 07/13] Allow offline access for OIDC UI client --- .../ChangePassword/ChangePassword.feature | 2 +- .../ChangePassword/ChangePassword.feature.cs | 2 +- .../ForgotPassword/ForgotPassword.feature | 2 +- .../ForgotPassword/ForgotPassword.feature.cs | 2 +- .../UserLogin/UserLogin.feature | 2 +- .../UserLogin/UserLogin.feature.cs | 2 +- .../UserLogin/UserLoginSteps.cs | 44 +++---------------- 7 files changed, 11 insertions(+), 45 deletions(-) diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature index 5834fd4..ef4d9b1 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature +++ b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature @@ -19,7 +19,7 @@ Background: Given I create the following clients | ClientId | Name | Secret | Scopes | GrantTypes | RedirectUris | PostLogoutRedirectUris | RequireConsent | AllowOfflineAccess | ClientUri | - | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | + | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile,offline_access | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | @PRTest diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs index fee0202..dc0ae8e 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs @@ -175,7 +175,7 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "estateUIClient", "Merchant Client", "Secret1", - "transactionProcessor,openid,email,profile", + "transactionProcessor,openid,email,profile,offline_access", "authorization_code", "https://[url]:[port]/signin-oidc", "https://[url]:[port]/signout-oidc", diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature index 7796f9d..d2f5e04 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature +++ b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature @@ -19,7 +19,7 @@ Background: Given I create the following clients | ClientId | Name | Secret | Scopes | GrantTypes | RedirectUris | PostLogoutRedirectUris | RequireConsent | AllowOfflineAccess | ClientUri | - | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | + | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile,offline_access | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | Given I create the following users | Email Address | Password | Phone Number | Given Name | Middle Name | Family Name | Claims | Roles | diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs index 8aecdcf..154b141 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs @@ -175,7 +175,7 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "estateUIClient", "Merchant Client", "Secret1", - "transactionProcessor,openid,email,profile", + "transactionProcessor,openid,email,profile,offline_access", "authorization_code", "https://[url]:[port]/signin-oidc", "https://[url]:[port]/signout-oidc", diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature index f89fd36..8fdc280 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature @@ -19,7 +19,7 @@ Background: Given I create the following clients | ClientId | Name | Secret | Scopes | GrantTypes | RedirectUris | PostLogoutRedirectUris | RequireConsent | AllowOfflineAccess | ClientUri | - | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | + | estateUIClient | Merchant Client | Secret1 | transactionProcessor,openid,email,profile,offline_access | authorization_code | https://[url]:[port]/signin-oidc | https://[url]:[port]/signout-oidc | false | true | https://[url]:[port] | @PRTest Scenario: Create User and Login diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs index 037f533..19ea044 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs @@ -173,7 +173,7 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "estateUIClient", "Merchant Client", "Secret1", - "transactionProcessor,openid,email,profile", + "transactionProcessor,openid,email,profile,offline_access", "authorization_code", "https://[url]:[port]/signin-oidc", "https://[url]:[port]/signout-oidc", diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs index 091db79..e7b91aa 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs @@ -71,45 +71,11 @@ public async Task WhenIClickTheLink(string linkText) [Then(@"I am presented with a login screen")] public async Task ThenIAmPresentedWithALoginScreen() { - try - { - await Retry.For(async () => - { - IWebElement loginButton = await this.WebDriver.FindButton("Login"); - loginButton.ShouldNotBeNull(); - }); - } - catch - { - string path = ""; - string queryKeys = ""; - string title = ""; - string bodyText = ""; - - try - { - if (Uri.TryCreate(this.WebDriver.Url, UriKind.Absolute, out Uri currentUri)) - { - path = currentUri.AbsolutePath; - queryKeys = String.Join(",", currentUri.Query - .TrimStart('?') - .Split('&', StringSplitOptions.RemoveEmptyEntries) - .Select(parameter => parameter.Split('=', 2)[0])); - } - - title = this.WebDriver.Title; - bodyText = this.WebDriver.FindElement(By.TagName("body")).Text; - bodyText = bodyText.Length > 2000 ? bodyText.Substring(0, 2000) : bodyText; - } - catch - { - // Keep diagnostics best-effort so the original test failure is preserved. - } - - this.TestingContext.DockerHelper.Logger.LogInformation( - $"Login screen diagnostic: path=[{path}], queryKeys=[{queryKeys}], title=[{title}], body=[{bodyText}]"); - throw; - } + await Retry.For(async () => + { + IWebElement loginButton = await this.WebDriver.FindButton("Login"); + loginButton.ShouldNotBeNull(); + }); } [When(@"I login with the username '([^']*)' and the provided password")] From c186b7e89b020bc4738657a5aaea5085c46dc65a Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 13:27:24 +0100 Subject: [PATCH 08/13] Capture current OAuth UI failure details --- .../UserLogin/UserLoginSteps.cs | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs index e7b91aa..091db79 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLoginSteps.cs @@ -71,11 +71,45 @@ public async Task WhenIClickTheLink(string linkText) [Then(@"I am presented with a login screen")] public async Task ThenIAmPresentedWithALoginScreen() { - await Retry.For(async () => - { - IWebElement loginButton = await this.WebDriver.FindButton("Login"); - loginButton.ShouldNotBeNull(); - }); + try + { + await Retry.For(async () => + { + IWebElement loginButton = await this.WebDriver.FindButton("Login"); + loginButton.ShouldNotBeNull(); + }); + } + catch + { + string path = ""; + string queryKeys = ""; + string title = ""; + string bodyText = ""; + + try + { + if (Uri.TryCreate(this.WebDriver.Url, UriKind.Absolute, out Uri currentUri)) + { + path = currentUri.AbsolutePath; + queryKeys = String.Join(",", currentUri.Query + .TrimStart('?') + .Split('&', StringSplitOptions.RemoveEmptyEntries) + .Select(parameter => parameter.Split('=', 2)[0])); + } + + title = this.WebDriver.Title; + bodyText = this.WebDriver.FindElement(By.TagName("body")).Text; + bodyText = bodyText.Length > 2000 ? bodyText.Substring(0, 2000) : bodyText; + } + catch + { + // Keep diagnostics best-effort so the original test failure is preserved. + } + + this.TestingContext.DockerHelper.Logger.LogInformation( + $"Login screen diagnostic: path=[{path}], queryKeys=[{queryKeys}], title=[{title}], body=[{bodyText}]"); + throw; + } } [When(@"I login with the username '([^']*)' and the provided password")] From 373746948400f060ce089ac028b453ac214e699e Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 13:53:49 +0100 Subject: [PATCH 09/13] Allow offline access for OAuth clients --- .../RequestHandlers/ClientRequestHandler.cs | 5 +++++ .../RequestHandlers/ClientRequestHandlerTests.cs | 1 + SecurityService/HostedServices/DatabaseInitializer.cs | 6 ++++++ 3 files changed, 12 insertions(+) diff --git a/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs b/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs index b129619..c82a61a 100644 --- a/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs +++ b/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs @@ -71,6 +71,11 @@ public async Task Handle(SecurityServiceCommands.CreateClientCommand com descriptor.Permissions.UnionWith(permissions.Permissions); descriptor.Requirements.UnionWith(permissions.Requirements); + if (command.AllowOfflineAccess) + { + descriptor.Permissions.Add(Permissions.Prefixes.Scope + Scopes.OfflineAccess); + } + foreach (String redirectUri in command.ClientRedirectUris.Where(uri => string.IsNullOrWhiteSpace(uri) == false).Distinct(StringComparer.OrdinalIgnoreCase)) { descriptor.RedirectUris.Add(new Uri(redirectUri, UriKind.Absolute)); diff --git a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs index 61cb794..8109dec 100644 --- a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs +++ b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs @@ -70,6 +70,7 @@ public async Task CreateClient_PersistsGrantPermissionsAndPkceRequirement() permissions.ShouldContain(OpenIddictConstants.Permissions.Endpoints.Token); permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode); permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.RefreshToken); + permissions.ShouldContain(OpenIddictConstants.Permissions.Prefixes.Scope + OpenIddictConstants.Scopes.OfflineAccess); requirements.ShouldContain(OpenIddictConstants.Requirements.Features.ProofKeyForCodeExchange); } diff --git a/SecurityService/HostedServices/DatabaseInitializer.cs b/SecurityService/HostedServices/DatabaseInitializer.cs index 14e1ed9..78afaf1 100644 --- a/SecurityService/HostedServices/DatabaseInitializer.cs +++ b/SecurityService/HostedServices/DatabaseInitializer.cs @@ -90,6 +90,12 @@ private async Task SynchronizeApplicationPermissionsAsync(IServiceProvider servi descriptor.Permissions.RemoveWhere(permission => permission.StartsWith(Permissions.Prefixes.GrantType, StringComparison.Ordinal)); descriptor.Permissions.UnionWith(policy.Permissions); + + if (client.AllowOfflineAccess) + { + descriptor.Permissions.Add(Permissions.Prefixes.Scope + Scopes.OfflineAccess); + } + descriptor.Requirements.UnionWith(policy.Requirements); await applicationManager.UpdateAsync(application, descriptor, cancellationToken); From 70132edeec759b14f1da63a1da13003321b4c87d Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 14:08:05 +0100 Subject: [PATCH 10/13] Grant refresh tokens for offline access clients --- .../RequestHandlers/ClientRequestHandler.cs | 2 +- .../ClientRequestHandlerTests.cs | 29 +++++++++++++++++++ .../HostedServices/DatabaseInitializer.cs | 2 +- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs b/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs index c82a61a..7939bd2 100644 --- a/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs +++ b/SecurityService.BusinessLogic/RequestHandlers/ClientRequestHandler.cs @@ -73,7 +73,7 @@ public async Task Handle(SecurityServiceCommands.CreateClientCommand com if (command.AllowOfflineAccess) { - descriptor.Permissions.Add(Permissions.Prefixes.Scope + Scopes.OfflineAccess); + descriptor.Permissions.Add(Permissions.GrantTypes.RefreshToken); } foreach (String redirectUri in command.ClientRedirectUris.Where(uri => string.IsNullOrWhiteSpace(uri) == false).Distinct(StringComparer.OrdinalIgnoreCase)) diff --git a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs index 8109dec..0e62c91 100644 --- a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs +++ b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs @@ -74,6 +74,35 @@ public async Task CreateClient_PersistsGrantPermissionsAndPkceRequirement() requirements.ShouldContain(OpenIddictConstants.Requirements.Features.ProofKeyForCodeExchange); } + [Fact] + public async Task CreateClient_WithOfflineAccess_AddsRefreshTokenPermission() + { + using var provider = TestServiceProviderFactory.Create(nameof(this.CreateClient_WithOfflineAccess_AddsRefreshTokenPermission)); + var mediator = provider.GetRequiredService(); + var applicationManager = provider.GetRequiredService(); + + var createResult = await mediator.Send(new SecurityServiceCommands.CreateClientCommand( + "offline-client", + null, + "Offline Client", + null, + [OpenIddictConstants.Scopes.OpenId, OpenIddictConstants.Scopes.OfflineAccess], + [OpenIddictConstants.GrantTypes.AuthorizationCode], + null, + ["https://client.example/signin-oidc"], + [], + false, + true)); + + createResult.IsSuccess.ShouldBeTrue(); + + object application = (await applicationManager.FindByClientIdAsync("offline-client"))!; + var permissions = (await applicationManager.GetPermissionsAsync(application)).ToHashSet(StringComparer.Ordinal); + + permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode); + permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.RefreshToken); + } + [Fact] public async Task CreateClient_WhenGrantTypeIsUnsupported_ReturnsInvalid() { diff --git a/SecurityService/HostedServices/DatabaseInitializer.cs b/SecurityService/HostedServices/DatabaseInitializer.cs index 78afaf1..fc04473 100644 --- a/SecurityService/HostedServices/DatabaseInitializer.cs +++ b/SecurityService/HostedServices/DatabaseInitializer.cs @@ -93,7 +93,7 @@ private async Task SynchronizeApplicationPermissionsAsync(IServiceProvider servi if (client.AllowOfflineAccess) { - descriptor.Permissions.Add(Permissions.Prefixes.Scope + Scopes.OfflineAccess); + descriptor.Permissions.Add(Permissions.GrantTypes.RefreshToken); } descriptor.Requirements.UnionWith(policy.Requirements); From 452c9f4d78d21bca93e1070337478fe69a3b92e2 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 14:19:53 +0100 Subject: [PATCH 11/13] Correct offline access permission assertion --- .../RequestHandlers/ClientRequestHandlerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs index 0e62c91..3c18ae2 100644 --- a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs +++ b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs @@ -70,7 +70,7 @@ public async Task CreateClient_PersistsGrantPermissionsAndPkceRequirement() permissions.ShouldContain(OpenIddictConstants.Permissions.Endpoints.Token); permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode); permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.RefreshToken); - permissions.ShouldContain(OpenIddictConstants.Permissions.Prefixes.Scope + OpenIddictConstants.Scopes.OfflineAccess); + permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.RefreshToken); requirements.ShouldContain(OpenIddictConstants.Requirements.Features.ProofKeyForCodeExchange); } From 5e1a6c1bbead642543fa4ab8670ef681cd527202 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 14:48:17 +0100 Subject: [PATCH 12/13] Preserve bootstrap client grant restrictions --- .../ManagementBootstrapperTests.cs | 60 +++++++++++++++++++ .../TestServiceProviderFactory.cs | 2 + .../HostedServices/DatabaseInitializer.cs | 3 +- 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs b/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs index 4cfda63..676e560 100644 --- a/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs +++ b/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs @@ -3,6 +3,8 @@ using Microsoft.EntityFrameworkCore; using OpenIddict.Abstractions; using SecurityService.BusinessLogic; +using SecurityService.Database; +using SecurityService.Database.Entities; using SecurityService.Database.DbContexts; using SecurityService.HostedServices; using SecurityService.UnitTests.Infrastructure; @@ -72,6 +74,64 @@ await applicationManager.CreateAsync(new OpenIddictApplicationDescriptor exception.Message.ShouldContain("secret"); } + [Fact] + public async Task DatabaseInitialization_WhenBootstrapDefinitionHasStaleGrants_LeavesClientCredentialsOnly() + { + const string databaseName = "bootstrap-permissions-"; + using var provider = TestServiceProviderFactory.Create( + databaseName + Guid.NewGuid(), + configureOptions: options => + { + options.UseInMemoryDatabase = true; + options.ManagementBootstrap.Enabled = true; + options.ManagementBootstrap.ClientId = "bootstrap-client"; + options.ManagementBootstrap.ClientSecret = "bootstrap-secret"; + }); + + using var scope = provider.CreateScope(); + var applicationManager = scope.ServiceProvider.GetRequiredService(); + await applicationManager.CreateAsync(new OpenIddictApplicationDescriptor + { + ClientId = "bootstrap-client", + ClientSecret = "bootstrap-secret", + ClientType = OpenIddictConstants.ClientTypes.Confidential, + ConsentType = OpenIddictConstants.ConsentTypes.Implicit, + Permissions = + { + OpenIddictConstants.Permissions.Endpoints.Authorization, + OpenIddictConstants.Permissions.Endpoints.Token, + OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode, + OpenIddictConstants.Permissions.ResponseTypes.Code + } + }); + + var dbContext = scope.ServiceProvider.GetRequiredService(); + await dbContext.ClientDefinitions.AddAsync(new ClientDefinition + { + Id = Guid.NewGuid(), + ClientId = "bootstrap-client", + ClientName = "Stale Bootstrap Client", + SecretHash = Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes("bootstrap-secret"))), + AllowedGrantTypesJson = JsonListSerializer.Serialize([OpenIddictConstants.GrantTypes.AuthorizationCode]), + AllowedScopesJson = "[]", + RedirectUrisJson = "[]", + PostLogoutRedirectUrisJson = "[]", + ClientType = OpenIddictConstants.ClientTypes.Confidential + }); + await dbContext.SaveChangesAsync(); + + var initializer = new DatabaseInitializer( + provider, + scope.ServiceProvider.GetRequiredService>()); + + await initializer.StartAsync(CancellationToken.None); + + object application = (await applicationManager.FindByClientIdAsync("bootstrap-client"))!; + var permissions = (await applicationManager.GetPermissionsAsync(application)).ToHashSet(StringComparer.Ordinal); + permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.ClientCredentials); + permissions.ShouldNotContain(OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode); + } + [Fact] public async Task InitializeAsync_WhenBootstrapAdministratorEmailConflicts_FailsClearly() { diff --git a/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs b/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs index 5900462..475af04 100644 --- a/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs +++ b/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs @@ -8,6 +8,7 @@ using SecurityService.BusinessLogic.Requests; using SecurityService.Database.DbContexts; using SecurityService.Database.Entities; +using SecurityService.HostedServices; namespace SecurityService.UnitTests.Infrastructure; @@ -50,6 +51,7 @@ public static ServiceProvider Create(string databaseName, UserManager configuration.RegisterServicesFromAssembly(typeof(SecurityServiceCommands).Assembly)); + services.AddScoped(); services.AddSingleton(); if (userManager != null) services.AddSingleton(userManager); diff --git a/SecurityService/HostedServices/DatabaseInitializer.cs b/SecurityService/HostedServices/DatabaseInitializer.cs index fc04473..5fd50fd 100644 --- a/SecurityService/HostedServices/DatabaseInitializer.cs +++ b/SecurityService/HostedServices/DatabaseInitializer.cs @@ -53,9 +53,10 @@ public async Task StartAsync(CancellationToken cancellationToken) } } + await this.SynchronizeApplicationPermissionsAsync(scope.ServiceProvider, dbContext, cancellationToken); + ManagementBootstrapper bootstrapper = scope.ServiceProvider.GetRequiredService(); await bootstrapper.InitializeAsync(cancellationToken); - await this.SynchronizeApplicationPermissionsAsync(scope.ServiceProvider, dbContext, cancellationToken); Logger.LogWarning("Database initialization complete."); } From 10b3da3e54822edc6d67360718a8a6da18354c6f Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Tue, 6 Oct 2026 15:22:57 +0100 Subject: [PATCH 13/13] Avoid logger dependency in bootstrap test --- .../ManagementBootstrapperTests.cs | 72 +++++++++---------- .../TestServiceProviderFactory.cs | 2 - 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs b/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs index 676e560..01c02ad 100644 --- a/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs +++ b/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs @@ -45,44 +45,12 @@ public async Task InitializeAsync_WhenEnabled_CreatesBootstrapClientAndIsIdempot } [Fact] - public async Task InitializeAsync_WhenBootstrapApplicationSecretConflicts_FailsClearly() + public async Task InitializeAsync_WhenExistingBootstrapClientHasStaleGrants_RestoresClientCredentialsOnly() { using var provider = TestServiceProviderFactory.Create( Guid.NewGuid().ToString(), configureOptions: options => { - options.ManagementBootstrap.Enabled = true; - options.ManagementBootstrap.ClientId = "bootstrap-client"; - options.ManagementBootstrap.ClientSecret = "configured-secret"; - }); - - using var scope = provider.CreateScope(); - var applicationManager = scope.ServiceProvider.GetRequiredService(); - await applicationManager.CreateAsync(new OpenIddictApplicationDescriptor - { - ClientId = "bootstrap-client", - ClientSecret = "existing-secret", - ClientType = OpenIddictConstants.ClientTypes.Confidential, - ConsentType = OpenIddictConstants.ConsentTypes.Implicit - }); - - var bootstrapper = CreateBootstrapper(scope); - - var exception = await Should.ThrowAsync(() => bootstrapper.InitializeAsync(CancellationToken.None)); - - exception.Message.ShouldContain("bootstrap application"); - exception.Message.ShouldContain("secret"); - } - - [Fact] - public async Task DatabaseInitialization_WhenBootstrapDefinitionHasStaleGrants_LeavesClientCredentialsOnly() - { - const string databaseName = "bootstrap-permissions-"; - using var provider = TestServiceProviderFactory.Create( - databaseName + Guid.NewGuid(), - configureOptions: options => - { - options.UseInMemoryDatabase = true; options.ManagementBootstrap.Enabled = true; options.ManagementBootstrap.ClientId = "bootstrap-client"; options.ManagementBootstrap.ClientSecret = "bootstrap-secret"; @@ -111,7 +79,6 @@ await dbContext.ClientDefinitions.AddAsync(new ClientDefinition Id = Guid.NewGuid(), ClientId = "bootstrap-client", ClientName = "Stale Bootstrap Client", - SecretHash = Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes("bootstrap-secret"))), AllowedGrantTypesJson = JsonListSerializer.Serialize([OpenIddictConstants.GrantTypes.AuthorizationCode]), AllowedScopesJson = "[]", RedirectUrisJson = "[]", @@ -120,11 +87,8 @@ await dbContext.ClientDefinitions.AddAsync(new ClientDefinition }); await dbContext.SaveChangesAsync(); - var initializer = new DatabaseInitializer( - provider, - scope.ServiceProvider.GetRequiredService>()); - - await initializer.StartAsync(CancellationToken.None); + var bootstrapper = CreateBootstrapper(scope); + await bootstrapper.InitializeAsync(CancellationToken.None); object application = (await applicationManager.FindByClientIdAsync("bootstrap-client"))!; var permissions = (await applicationManager.GetPermissionsAsync(application)).ToHashSet(StringComparer.Ordinal); @@ -132,6 +96,36 @@ await dbContext.ClientDefinitions.AddAsync(new ClientDefinition permissions.ShouldNotContain(OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode); } + [Fact] + public async Task InitializeAsync_WhenBootstrapApplicationSecretConflicts_FailsClearly() + { + using var provider = TestServiceProviderFactory.Create( + Guid.NewGuid().ToString(), + configureOptions: options => + { + options.ManagementBootstrap.Enabled = true; + options.ManagementBootstrap.ClientId = "bootstrap-client"; + options.ManagementBootstrap.ClientSecret = "configured-secret"; + }); + + using var scope = provider.CreateScope(); + var applicationManager = scope.ServiceProvider.GetRequiredService(); + await applicationManager.CreateAsync(new OpenIddictApplicationDescriptor + { + ClientId = "bootstrap-client", + ClientSecret = "existing-secret", + ClientType = OpenIddictConstants.ClientTypes.Confidential, + ConsentType = OpenIddictConstants.ConsentTypes.Implicit + }); + + var bootstrapper = CreateBootstrapper(scope); + + var exception = await Should.ThrowAsync(() => bootstrapper.InitializeAsync(CancellationToken.None)); + + exception.Message.ShouldContain("bootstrap application"); + exception.Message.ShouldContain("secret"); + } + [Fact] public async Task InitializeAsync_WhenBootstrapAdministratorEmailConflicts_FailsClearly() { diff --git a/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs b/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs index 475af04..5900462 100644 --- a/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs +++ b/SecurityService.UnitTests/Infrastructure/TestServiceProviderFactory.cs @@ -8,7 +8,6 @@ using SecurityService.BusinessLogic.Requests; using SecurityService.Database.DbContexts; using SecurityService.Database.Entities; -using SecurityService.HostedServices; namespace SecurityService.UnitTests.Infrastructure; @@ -51,7 +50,6 @@ public static ServiceProvider Create(string databaseName, UserManager configuration.RegisterServicesFromAssembly(typeof(SecurityServiceCommands).Assembly)); - services.AddScoped(); services.AddSingleton(); if (userManager != null) services.AddSingleton(userManager);