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..7939bd2 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,14 @@ 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); + + if (command.AllowOfflineAccess) + { + descriptor.Permissions.Add(Permissions.GrantTypes.RefreshToken); + } + 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/ChangePassword/ChangePassword.feature b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature index 77f8bdc..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 | hybrid | 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 @@ -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..dc0ae8e 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/ChangePassword/ChangePassword.feature.cs @@ -175,8 +175,8 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "estateUIClient", "Merchant Client", "Secret1", - "transactionProcessor,openid,email,profile", - "hybrid", + "transactionProcessor,openid,email,profile,offline_access", + "authorization_code", "https://[url]:[port]/signin-oidc", "https://[url]:[port]/signout-oidc", "false", diff --git a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature index c6667f0..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 | hybrid | 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 | @@ -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..154b141 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/ForgotPassword/ForgotPassword.feature.cs @@ -175,8 +175,8 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "estateUIClient", "Merchant Client", "Secret1", - "transactionProcessor,openid,email,profile", - "hybrid", + "transactionProcessor,openid,email,profile,offline_access", + "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..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 | hybrid | 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 d68f256..19ea044 100644 --- a/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs +++ b/SecurityService.OpenIdConnect.IntegrationTests/UserLogin/UserLogin.feature.cs @@ -173,8 +173,8 @@ public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, globa "estateUIClient", "Merchant Client", "Secret1", - "transactionProcessor,openid,email,profile", - "hybrid", + "transactionProcessor,openid,email,profile,offline_access", + "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 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")] diff --git a/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs b/SecurityService.UnitTests/HostedServices/ManagementBootstrapperTests.cs index 4cfda63..01c02ad 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; @@ -42,6 +44,58 @@ public async Task InitializeAsync_WhenEnabled_CreatesBootstrapClientAndIsIdempot (await dbContext.ClientDefinitions.CountAsync(client => client.ClientId == "bootstrap-client")).ShouldBe(1); } + [Fact] + 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 = "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", + AllowedGrantTypesJson = JsonListSerializer.Serialize([OpenIddictConstants.GrantTypes.AuthorizationCode]), + AllowedScopesJson = "[]", + RedirectUrisJson = "[]", + PostLogoutRedirectUrisJson = "[]", + ClientType = OpenIddictConstants.ClientTypes.Confidential + }); + await dbContext.SaveChangesAsync(); + + 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); + permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.ClientCredentials); + permissions.ShouldNotContain(OpenIddictConstants.Permissions.GrantTypes.AuthorizationCode); + } + [Fact] public async Task InitializeAsync_WhenBootstrapApplicationSecretConflicts_FailsClearly() { 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..3c18ae2 100644 --- a/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs +++ b/SecurityService.UnitTests/RequestHandlers/ClientRequestHandlerTests.cs @@ -40,6 +40,69 @@ 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); + permissions.ShouldContain(OpenIddictConstants.Permissions.GrantTypes.RefreshToken); + 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 606189b..5fd50fd 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; @@ -50,6 +53,8 @@ public async Task StartAsync(CancellationToken cancellationToken) } } + await this.SynchronizeApplicationPermissionsAsync(scope.ServiceProvider, dbContext, cancellationToken); + ManagementBootstrapper bootstrapper = scope.ServiceProvider.GetRequiredService(); await bootstrapper.InitializeAsync(cancellationToken); @@ -62,4 +67,39 @@ 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); + + if (client.AllowOfflineAccess) + { + descriptor.Permissions.Add(Permissions.GrantTypes.RefreshToken); + } + + 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..76a0c4a 100644 --- a/SecurityService/Program.cs +++ b/SecurityService/Program.cs @@ -287,21 +287,24 @@ 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(); + } + 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/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"); 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 new file mode 100644 index 0000000..f92574c --- /dev/null +++ b/docs/superpowers/specs/2026-10-06-oauth-grant-restrictions-design.md @@ -0,0 +1,88 @@ +# 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 +- 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 + +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 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. + +### 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 +- `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, 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 + +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. +- 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.