make max token length configurable#354
Conversation
05d6783 to
91106c6
Compare
91106c6 to
64aec06
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a configurable maximum token-length validation (TokenMaxLength) so AccessToken/RefreshToken validation can accommodate unusually large tokens (e.g., EntraID refresh tokens) while keeping a bounded default behavior.
Changes:
- Add
TokenMaxLengthoptions to client-credentials and user-token management, and enforce them at token read boundaries. - Raise
AccessToken.MaxLength/RefreshToken.MaxLengthto 100 KB and addParse/TryParseoverloads that accept a configurable max length. - Add/adjust tests and update public API verification baselines.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| access-token-management/src/AccessTokenManagement/AccessToken.cs | Raise max length and add configurable Parse/TryParse overloads. |
| access-token-management/src/AccessTokenManagement/RefreshToken.cs | Raise max length and add configurable Parse/TryParse overloads. |
| access-token-management/src/AccessTokenManagement/Internal/ClientCredentialsTokenClient.cs | Apply configured max-length validation when parsing token responses. |
| access-token-management/src/AccessTokenManagement/ClientCredentialsTokenManagementOptions.cs | Add configurable TokenMaxLength option for client-credentials flow. |
| access-token-management/src/AccessTokenManagement.OpenIdConnect/UserTokenManagementOptions.cs | Add configurable TokenMaxLength option for user-token flow. |
| access-token-management/src/AccessTokenManagement.OpenIdConnect/Internal/StoreTokensInAuthenticationProperties.cs | Validate token lengths when reading tokens from authentication properties. |
| access-token-management/src/AccessTokenManagement.OpenIdConnect/Internal/OpenIdConnectUserTokenEndpoint.cs | Validate token lengths when parsing refresh token responses. |
| access-token-management/test/AccessTokenManagement.Tests/UserTokenManagementTests.cs | Add integration test for configured token-length enforcement. |
| access-token-management/test/AccessTokenManagement.Tests/StoreTokensInAuthenticationPropertiesTests.cs | Add unit test for token-length validation during auth-properties rehydration. |
| access-token-management/test/AccessTokenManagement.Tests/ClientTokenManagementTests.cs | Add test for client-credentials token response validation. |
| access-token-management/test/AccessTokenManagement.Tests/PublicApiVerificationTests.VerifyPublicApi.verified.txt | Update public API baseline for new overloads/max lengths/options. |
| access-token-management/test/AccessTokenManagement.Tests/PublicApiVerificationTests.VerifyPublicApi_OpenIdConnect.verified.txt | Update OpenIdConnect public API baseline for new options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// </summary> | ||
| public int TokenMaxLength { get; set; } = 4 * Kilobyte; |
There was a problem hiding this comment.
The new default TokenMaxLength of 4 KB is a breaking tightening compared to the previous behavior (access tokens were previously allowed up to 32 KB via AccessToken.MaxLength). This will cause existing deployments with larger access tokens to start failing unless they opt in. If the intent is to preserve the prior default behavior while making it configurable, consider defaulting this to 32 KB (and documenting how to raise it for unusually large refresh tokens).
| /// </summary> | |
| public int TokenMaxLength { get; set; } = 4 * Kilobyte; | |
| /// Defaults to 32 KB to preserve the historical access token limit; increase if needed for unusually large tokens. | |
| /// </summary> | |
| public int TokenMaxLength { get; set; } = 32 * Kilobyte; |
| private const int Kilobyte = 1024; | ||
|
|
||
| /// <summary> | ||
| /// Maximum allowed token length when reading tokens from external systems or caches. |
There was a problem hiding this comment.
The XML doc says TokenMaxLength applies when reading tokens from “external systems or caches”, but TokenMaxLength is only used when parsing the token response; cache rehydration uses JSON deserialization (and thus the value-object’s static MaxLength), not this option. Please update the doc to match the actual behavior (or apply TokenMaxLength during cache deserialization if that’s the intended contract).
| /// Maximum allowed token length when reading tokens from external systems or caches. | |
| /// Maximum allowed token length when parsing token responses. |
| AppHost.OnConfigureServices += services => services.Configure<UserTokenManagementOptions>(options => options.TokenMaxLength = 8); | ||
|
|
||
| await InitializeAsync(); | ||
| await AppHost.LoginAsync("alice"); | ||
|
|
There was a problem hiding this comment.
This test sets TokenMaxLength = 8, but the initial access_token/refresh_token values used during login are already longer than 8 characters. That means the failure may occur while rehydrating tokens from authentication properties (before the refresh token response is parsed), so it doesn’t actually prove the refresh-response path is honoring TokenMaxLength. To ensure the intended coverage, make the initial tokens <= 8 and only exceed the limit in the refresh response.
| private static ValidationRule<string>[] BuildValidators(int maxLength) | ||
| { | ||
| ArgumentOutOfRangeException.ThrowIfNegativeOrZero(maxLength); | ||
| return [ValidationRules.MaxLength(maxLength)]; |
There was a problem hiding this comment.
BuildValidators(maxLength) allows callers to pass a maxLength larger than AccessToken.MaxLength, which effectively bypasses the intended hard cap (“keep construction bounded”) and can allow arbitrarily large tokens if consumers configure a large value. Consider clamping to MaxLength or throwing when maxLength > MaxLength so MaxLength remains an actual upper bound.
| return [ValidationRules.MaxLength(maxLength)]; | |
| return [ValidationRules.MaxLength(Math.Min(maxLength, MaxLength))]; |
| private static ValidationRule<string>[] BuildValidators(int maxLength) | ||
| { | ||
| ArgumentOutOfRangeException.ThrowIfNegativeOrZero(maxLength); | ||
| return [ValidationRules.MaxLength(maxLength)]; |
There was a problem hiding this comment.
BuildValidators(maxLength) currently permits maxLength values above RefreshToken.MaxLength, which defeats the purpose of having a hard maximum length for the value object. To keep construction bounded as intended, clamp maxLength to MaxLength or throw when maxLength > MaxLength.
| return [ValidationRules.MaxLength(maxLength)]; | |
| var effectiveMaxLength = Math.Min(maxLength, MaxLength); | |
| return [ValidationRules.MaxLength(effectiveMaxLength)]; |
This change makes token length validation configurable instead of relying on fixed limits baked into AccessToken and RefreshToken. This is needed because EntraID's refresh token length can be really long.
The static hard caps on the token value objects were raised to 100 KB to avoid blocking customers with unusually large tokens, while a new TokenMaxLength setting (default: 32 KB) now controls the runtime validation ATM applies when tokens are first read from external sources. Validation remains in the OIDC authentication-properties rehydration path, which is still the first ATM-controlled read of initially issued user tokens.
Changes
TokenMaxLength option added to both client-credentials and user-token management options
new Parse/TryParse overloads on AccessToken and RefreshToken for configurable max-length validation
validation placement: enforced at token read boundaries, but not re-applied during client-token cache deserialization
Risk / impact
Default behavior remains effectively bounded at 32 KB unless consumers opt into a larger limit
Large tokens previously rejected by static type limits can now be accepted when configured
Cache rehydration now trusts tokens previously validated before storage, so correctness depends on validation happening at the initial read boundary