Addition of fully synchronous token validation pipeline - #3562
Conversation
support sync version
existing ConfigurationManager-specific tests for sync methods
support sync version
existing ConfigurationManager-specific tests for sync methods
| /// <param name="cancellationToken"></param> | ||
| /// <returns>A <see cref="ValidationResult{ValidatedIssuer, IssuerValidationError}"/> that contains either the issuer that was validated or an error.</returns> | ||
| /// <remarks>An EXACT match is required.</remarks> | ||
| internal static ValidationResult<ValidatedIssuer, ValidationError> ValidateIssuerInternalSync( |
There was a problem hiding this comment.
Why do we use the suffix Sync? I see it in many places.
There was a problem hiding this comment.
....Sync is how I named the functions and new interfaces in the sync pipeline to differentiate between old async flow and new flow
|
|
||
| return readResult.Error!.AddCurrentStackFrame(); | ||
| #else | ||
| // Synchronous document retrieval is not supported before .NET 5.0 |
There was a problem hiding this comment.
Should we just not support the sync flow then? It is anyway configurable in VP...
| [BenchmarkCategory("ValidateTokenSync_Success"), Benchmark] | ||
| public bool JsonWebTokenHandler_ValidateTokenWithVP() | ||
| { | ||
| // Because ValidationResult is an internal type, we cannot return it in the benchmark. |
There was a problem hiding this comment.
Why is ValidationResult an internal type? IIRC north star is to stop throwing exceptions?
There was a problem hiding this comment.
I'm mimicking the existing benchmark file ValidateTokenAsyncTests.cs.
| // Because ValidationResult is an internal type, we cannot return it in the benchmark. | ||
| // We return a boolean instead until the type is made public. | ||
| ValidationResult<ValidatedToken, ValidationError> validationResult = _jsonWebTokenHandler.ValidateToken(_jwsExtendedClaims, _validationParameters, _callContext, CancellationToken.None); | ||
| return validationResult.Succeeded; |
There was a problem hiding this comment.
If the benchmark is broken, we will not catch it...
@microsoft-github-policy-service agree company="Microsoft" |
There was a problem hiding this comment.
Pull request overview
This PR adds a fully synchronous ValidateToken entrypoint and supporting synchronous configuration retrieval pipeline across Wilson/Protocols, while keeping the existing async validation flow unchanged. It introduces ...Sync companion interfaces/APIs to avoid breaking changes, plus mirrored sync test coverage and benchmark harnesses.
Changes:
- Add synchronous token validation entrypoints/pipelines (including sync configuration retrieval where supported).
- Introduce new sync interfaces (
IConfigurationManagerSync,IConfigurationRetrieverSync,ISyncDocumentRetriever, sync configuration event handlers) and implement them in core types. - Add mirrored sync tests and new/updated benchmarks to measure throughput and validate behavior.
Reviewed changes
Copilot reviewed 60 out of 60 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.IdentityModel.Tokens.Tests/Validation/IssuerValidationResultTests.cs | Adds sync issuer validation test coverage. |
| test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs | Updates expected properties; wires new issuer validator delegate. |
| test/Microsoft.IdentityModel.Tokens.Tests/AbstractVirtualsTests.cs | Adds sync base configuration manager virtual test. |
| test/Microsoft.IdentityModel.TestUtils/TokenValidationExtensibility/CustomIssuerValidators.cs | Extends custom validators to sync interface. |
| test/Microsoft.IdentityModel.TestUtils/SkipValidationValidators.cs | Adds sync issuer validator implementation for test utilities. |
| test/Microsoft.IdentityModel.TestUtils/MockHttpMessageHandler.cs | Adds NET5+ sync Send override for test HTTP path. |
| test/Microsoft.IdentityModel.TestUtils/MockConfigurationManager.cs | Implements GetBaseConfigurationSync for tests. |
| test/Microsoft.IdentityModel.TestUtils/InMemoryDocumentRetriever.cs | Implements ISyncDocumentRetriever for tests. |
| test/Microsoft.IdentityModel.TestUtils/DelegateHttpMessageHandler.cs | Adds NET5+ sync Send override for tests. |
| test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs | Adds sync retriever + sync ConfigurationManager test. |
| test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/SyncTests/ConfigurationManagerTelemetryTestsSync.cs | Adds sync telemetry tests for GetConfigurationSync. |
| test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/SyncTests/ConfigurationManagerEventHandlerTestsSync.cs | Adds sync tests for configuration event handlers. |
| test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/SyncTests/ConfigurationManagerEventHandlerContextAwareTestsSync.cs | Adds sync tests for context-aware handler paths. |
| test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/SyncTests/ConfigurationManagerBlockingEventHandlerTestsSync.cs | Adds sync tests for blocking refresh mode. |
| test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/MockConfigurationEventHandlerContextAware.cs | Adds sync context-aware handler implementation for tests. |
| test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/MockConfigurationEventHandler.cs | Adds sync handler implementation for tests. |
| test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandler.ValidateTokenSyncTests.cs | Adds sync ValidateToken tests for JWT handler. |
| src/Microsoft.IdentityModel.Validators/InternalAPI.Unshipped.txt | Records new internal sync issuer validator API. |
| src/Microsoft.IdentityModel.Validators/Experimental/AadIssuerValidator.Internal.cs | Adds ValidateIssuerSync implementation for AAD issuer validator. |
| src/Microsoft.IdentityModel.Tokens/Validators.cs | Adds sync issuer validation path for TVP pipeline. |
| src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs | Adds internal IssuerValidatorSync delegate property. |
| src/Microsoft.IdentityModel.Tokens/PublicAPI.Unshipped.txt | Records new public APIs (sync experimental validation/interfaces). |
| src/Microsoft.IdentityModel.Tokens/Properties/AssemblyInfo.cs | Updates InternalsVisibleTo entries for new harnesses. |
| src/Microsoft.IdentityModel.Tokens/Experimental/Validation/Validators.Issuer.cs | Adds sync issuer validation path in experimental validators. |
| src/Microsoft.IdentityModel.Tokens/Experimental/IResultBasedValidationSync.cs | Adds sync result-based validation interface. |
| src/Microsoft.IdentityModel.Tokens/Experimental/Interfaces.cs | Adds IIssuerValidatorSync experimental interface. |
| src/Microsoft.IdentityModel.Tokens/Experimental/DefaultValidators.cs | Extends default issuer validator to support sync. |
| src/Microsoft.IdentityModel.Tokens/Delegates.cs | Adds IssuerValidatorSync delegate type. |
| src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs | Adds GetBaseConfigurationSync virtual base API. |
| src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Experimental/Saml2SecurityTokenHandler.ValidateToken.Internal.cs | Implements IResultBasedValidationSync via sync-over-async. |
| src/Microsoft.IdentityModel.Tokens.Saml/Saml/Experimental/SamlSecurityTokenHandler.ValidateToken.Internal.cs | Implements IResultBasedValidationSync via sync-over-async. |
| src/Microsoft.IdentityModel.Protocols/PublicAPI/net9.0/PublicAPI.Unshipped.txt | Adds HttpDocumentRetriever.GetDocumentSync API (net9.0). |
| src/Microsoft.IdentityModel.Protocols/PublicAPI/net8.0/PublicAPI.Unshipped.txt | Adds HttpDocumentRetriever.GetDocumentSync API (net8.0). |
| src/Microsoft.IdentityModel.Protocols/PublicAPI/net6.0/PublicAPI.Unshipped.txt | Adds HttpDocumentRetriever.GetDocumentSync API (net6.0). |
| src/Microsoft.IdentityModel.Protocols/PublicAPI/net10.0/PublicAPI.Unshipped.txt | Adds HttpDocumentRetriever.GetDocumentSync API (net10.0). |
| src/Microsoft.IdentityModel.Protocols/PublicAPI.Unshipped.txt | Records new sync configuration manager/retriever/event handler APIs. |
| src/Microsoft.IdentityModel.Protocols/Configuration/StaticConfigurationManager.cs | Implements IConfigurationManagerSync + base config sync. |
| src/Microsoft.IdentityModel.Protocols/Configuration/ISyncDocumentRetriever.cs | Adds sync document retriever interface. |
| src/Microsoft.IdentityModel.Protocols/Configuration/IConfigurationRetrieverSync.cs | Adds sync configuration retriever interface. |
| src/Microsoft.IdentityModel.Protocols/Configuration/IConfigurationManagerSync.cs | Adds sync configuration manager interface. |
| src/Microsoft.IdentityModel.Protocols/Configuration/IConfigurationEventHandlerSync.cs | Adds sync configuration event handler interface. |
| src/Microsoft.IdentityModel.Protocols/Configuration/IConfigurationEventHandlerContextAwareSync.cs | Adds sync context-aware configuration handler interface. |
| src/Microsoft.IdentityModel.Protocols/Configuration/HttpDocumentRetriever.cs | Adds NET5+ sync document retrieval implementation. |
| src/Microsoft.IdentityModel.Protocols/Configuration/FileDocumentRetriever.cs | Adds sync file document retrieval implementation. |
| src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs | Adds GetConfigurationSync non-blocking sync path. |
| src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager_Blocking.cs | Adds blocking-mode sync configuration refresh path. |
| src/Microsoft.IdentityModel.Protocols.WsFederation/PublicAPI.Unshipped.txt | Records new WsFed sync retriever APIs. |
| src/Microsoft.IdentityModel.Protocols.WsFederation/Configuration/WsFederationConfigurationRetriever.cs | Implements sync WsFed configuration retrieval. |
| src/Microsoft.IdentityModel.Protocols.OpenIdConnect/PublicAPI.Unshipped.txt | Records new OIDC sync retriever API. |
| src/Microsoft.IdentityModel.Protocols.OpenIdConnect/Configuration/OpenIdConnectConfigurationRetriever.cs | Implements sync OIDC configuration retrieval. |
| src/Microsoft.IdentityModel.LoggingExtensions/Microsoft.IdentityModel.LoggingExtensions.csproj | Adds net8.0 target framework. |
| src/Microsoft.IdentityModel.JsonWebTokens/PublicAPI.Unshipped.txt | Records new public sync ValidateToken overloads. |
| src/Microsoft.IdentityModel.JsonWebTokens/Properties/AssemblyInfo.cs | Adds InternalsVisibleTo for new harnesses. |
| src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.ValidateToken.cs | Adds sync validation pipeline for TVP path. |
| src/Microsoft.IdentityModel.JsonWebTokens/Experimental/JsonWebTokenHandler.ValidateToken.Internal.cs | Implements IResultBasedValidationSync and sync validation path. |
| benchmark/Microsoft.IdentityModel.ValidateTokenThroughput/Program.cs | Adds standalone throughput harness for sync vs async. |
| benchmark/Microsoft.IdentityModel.ValidateTokenThroughput/Microsoft.IdentityModel.ValidateTokenThroughput.csproj | Adds new throughput harness project. |
| benchmark/Microsoft.IdentityModel.Benchmarks/ValidateTokenSyncTests.cs | Adds benchmark coverage for sync validation. |
| T configuration = ((IConfigurationRetrieverSync<T>)_configRetriever).GetConfigurationSync( | ||
| MetadataAddress, | ||
| _docRetriever, |
| // Don't use the individual CT here, this is a shared operation that shouldn't be affected by an individual's cancellation. | ||
| // The transport should have it's own timeouts, etc.. | ||
| var configuration = ((IConfigurationRetrieverSync<T>)_configRetriever).GetConfigurationSync(MetadataAddress, _docRetriever, CancellationToken.None); | ||
|
|
Merging dev added TokenValidationParameters.IgnoreCaseWhenValidatingAudience while this branch added IssuerValidatorSync, bringing the property count to 64. The count-guard tests (Publics/GetSets) hard-code the expected count, so bump it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
This PR is being split into multiple smaller ones for easier reviewability, first one is here |
Addition of fully synchronous token validation pipeline
This PR has various changes to code, tests, and sample scripts that add a fully synchronous
ValidateTokenentrypoint and pipeline into Wilson. This PR does not change the validation flow for the currentValidateTokenAsyncpipeline. All tests pass.Description
There are a couple new interfaces duplicated from the existing interfaces to prevent breaking changes:
This also adds
...Synccounterpart methods to the ConfigurationManagerBecause the synchronous
HttpClient.Sendwas only introduced in .NET 5, theValidateTokenentrypoint is gated using#if NET5_0_OR_GREATER, and otherwise falls back to a sync-over-asyncreturn ValidateTokenAsync(...).GetAwaiter().GetResult();wrapper call.A slew of new tests are added that essentially duplicate existing tests, but now call the new corresponding
...SyncfunctionsPerformance
Through Wilson benchmarks, there is a noticeable performance improvement of around 1-3% for both runtime and allocations.
MISE token validation benchmarks also show this performance improvement of around 1-3% for both runtime and allocations.
There is a more detailed report of performance in this doc