From 6d38118c4f4624f06614e88feac342df1853b766 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Thu, 15 Feb 2024 10:43:09 +0100 Subject: [PATCH] Fix strict OpenIdConnect server response validation --- .../IdSvrHealthCheckBuilderExtensions.cs | 8 ++++++-- .../DiscoveryEndpointResponse.cs | 14 +++++++++----- .../IdSvrHealthCheck.cs | 8 +++++--- .../Functional/DiscoveryEndpointResponseTests.cs | 6 +++--- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs b/src/HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs index cad4e41af8..9c6f68edc5 100644 --- a/src/HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs +++ b/src/HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs @@ -17,6 +17,7 @@ public static class IdSvrHealthCheckBuilderExtensions /// The . /// The uri of the Identity Server to check. /// Identity Server discover configuration segment. + /// Set to true if the health check shall validate the existence of code, id_token, and the id_token token in the reponse type values. /// The health check name. Optional. If null the type name 'idsvr' will be used for the name. /// /// The that should be reported when the health check fails. Optional. If null then @@ -29,6 +30,7 @@ public static IHealthChecksBuilder AddIdentityServer( this IHealthChecksBuilder builder, Uri idSvrUri, string discoverConfigurationSegment = IDSVR_DISCOVER_CONFIGURATION_SEGMENT, + bool isDynamicOpenIdProvider = false, string? name = default, HealthStatus? failureStatus = default, IEnumerable? tags = default, @@ -40,7 +42,7 @@ public static IHealthChecksBuilder AddIdentityServer( return builder.Add(new HealthCheckRegistration( registrationName, - sp => new IdSvrHealthCheck(() => sp.GetRequiredService().CreateClient(registrationName), discoverConfigurationSegment), + sp => new IdSvrHealthCheck(() => sp.GetRequiredService().CreateClient(registrationName), discoverConfigurationSegment, isDynamicOpenIdProvider), failureStatus, tags, timeout)); @@ -52,6 +54,7 @@ public static IHealthChecksBuilder AddIdentityServer( /// The . /// Factory for providing the uri of the Identity Server to check. /// Identity Server discover configuration segment. + /// Set to true if the health check shall validate the existence of code, id_token, and the id_token token in the reponse type values. /// The health check name. Optional. If null the type name 'idsvr' will be used for the name. /// /// The that should be reported when the health check fails. Optional. If null then @@ -63,6 +66,7 @@ public static IHealthChecksBuilder AddIdentityServer( this IHealthChecksBuilder builder, Func uriProvider, string discoverConfigurationSegment = IDSVR_DISCOVER_CONFIGURATION_SEGMENT, + bool isDynamicOpenIdProvider = false, string? name = null, HealthStatus? failureStatus = null, IEnumerable? tags = null, @@ -74,7 +78,7 @@ public static IHealthChecksBuilder AddIdentityServer( return builder.Add(new HealthCheckRegistration( registrationName, - sp => new IdSvrHealthCheck(() => sp.GetRequiredService().CreateClient(registrationName), discoverConfigurationSegment), + sp => new IdSvrHealthCheck(() => sp.GetRequiredService().CreateClient(registrationName), discoverConfigurationSegment, isDynamicOpenIdProvider), failureStatus, tags, timeout)); diff --git a/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs b/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs index b34427e943..22976d6af7 100644 --- a/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs +++ b/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs @@ -25,17 +25,21 @@ internal class DiscoveryEndpointResponse /// /// Validates Discovery response according to the OpenID specification /// - public void ValidateResponse() + public void ValidateResponse(bool isDynamicOpenIdProvider = false) { ValidateValue(Issuer, OidcConstants.ISSUER); ValidateValue(AuthorizationEndpoint, OidcConstants.AUTHORIZATION_ENDPOINT); ValidateValue(JwksUri, OidcConstants.JWKS_URI); - ValidateRequiredValues(ResponseTypesSupported, OidcConstants.RESPONSE_TYPES_SUPPORTED, OidcConstants.REQUIRED_RESPONSE_TYPES); + if (isDynamicOpenIdProvider) + { + ValidateRequiredValues(ResponseTypesSupported, OidcConstants.RESPONSE_TYPES_SUPPORTED, OidcConstants.REQUIRED_RESPONSE_TYPES); + + // Specification describes 'token id_token' response type, + // but some identity providers (f.e. Identity Server and Azure AD) return 'id_token token' + ValidateOneOfRequiredValues(ResponseTypesSupported, OidcConstants.RESPONSE_TYPES_SUPPORTED, OidcConstants.REQUIRED_COMBINED_RESPONSE_TYPES); + } - // Specification describes 'token id_token' response type, - // but some identity providers (f.e. Identity Server and Azure AD) return 'id_token token' - ValidateOneOfRequiredValues(ResponseTypesSupported, OidcConstants.RESPONSE_TYPES_SUPPORTED, OidcConstants.REQUIRED_COMBINED_RESPONSE_TYPES); ValidateOneOfRequiredValues(SubjectTypesSupported, OidcConstants.SUBJECT_TYPES_SUPPORTED, OidcConstants.REQUIRED_SUBJECT_TYPES); ValidateRequiredValues(SigningAlgorithmsSupported, OidcConstants.ALGORITHMS_SUPPORTED, OidcConstants.REQUIRED_ALGORITHMS); } diff --git a/src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs b/src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs index 62bafadf77..3cf47ceb6d 100644 --- a/src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs +++ b/src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs @@ -8,16 +8,18 @@ public class IdSvrHealthCheck : IHealthCheck { private readonly Func _httpClientFactory; private readonly string _discoverConfigurationSegment; + private readonly bool _isDynamicOpenIdProvider; public IdSvrHealthCheck(Func httpClientFactory) - : this(httpClientFactory, IdSvrHealthCheckBuilderExtensions.IDSVR_DISCOVER_CONFIGURATION_SEGMENT) + : this(httpClientFactory, IdSvrHealthCheckBuilderExtensions.IDSVR_DISCOVER_CONFIGURATION_SEGMENT, false) { } - public IdSvrHealthCheck(Func httpClientFactory, string discoverConfigurationSegment) + public IdSvrHealthCheck(Func httpClientFactory, string discoverConfigurationSegment, bool isDynamicOpenIdProvider) { _httpClientFactory = Guard.ThrowIfNull(httpClientFactory); _discoverConfigurationSegment = discoverConfigurationSegment; + _isDynamicOpenIdProvider = isDynamicOpenIdProvider; } /// @@ -39,7 +41,7 @@ public async Task CheckHealthAsync(HealthCheckContext context .ConfigureAwait(false) ?? throw new ArgumentException("Could not deserialize to discovery endpoint response!"); - discoveryResponse.ValidateResponse(); + discoveryResponse.ValidateResponse(_isDynamicOpenIdProvider); return HealthCheckResult.Healthy(); } diff --git a/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs b/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs index 7752b8b395..2df7099544 100644 --- a/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs +++ b/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs @@ -66,7 +66,7 @@ public void be_invalid_when_required_response_types_supported_are_missing(params ResponseTypesSupported = responseTypesSupported, }; - Action validate = () => response.ValidateResponse(); + Action validate = () => response.ValidateResponse(true); validate .ShouldThrow() @@ -84,7 +84,7 @@ public void be_invalid_when_combined_response_types_supported_are_missing() ResponseTypesSupported = new[] { "id_token", "code" }, }; - Action validate = () => response.ValidateResponse(); + Action validate = () => response.ValidateResponse(true); validate .ShouldThrow() @@ -105,7 +105,7 @@ public void be_invalid_when_required_subject_types_supported_are_missing(string SubjectTypesSupported = new[] { subjectTypesSupported }, }; - Action validate = () => response.ValidateResponse(); + Action validate = () => response.ValidateResponse(true); validate .ShouldThrow()