Fix strict OpenIdConnect server response validation

Xabaril · Feb 15, 2024 · 6d38118 · 6d38118
1 parent 73abc7a
commit 6d38118
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 13 deletions.
diff --git a/...HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs b/...HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs
@@ -17,6 +17,7 @@ public static class IdSvrHealthCheckBuilderExtensions
     /// <param name="builder">The <see cref="IHealthChecksBuilder"/>.</param>
     /// <param name="idSvrUri">The uri of the Identity Server to check.</param>
     /// <param name="discoverConfigurationSegment">Identity Server discover configuration segment.</param>
+    /// <param name="isDynamicOpenIdProvider">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.</param>
     /// <param name="name">The health check name. Optional. If <c>null</c> the type name 'idsvr' will be used for the name.</param>
     /// <param name="failureStatus">
     /// The <see cref="HealthStatus"/> that should be reported when the health check fails. Optional. If <c>null</c> 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<string>? tags = default,
@@ -40,7 +42,7 @@ public static IHealthChecksBuilder AddIdentityServer(
 
         return builder.Add(new HealthCheckRegistration(
             registrationName,
-            sp => new IdSvrHealthCheck(() => sp.GetRequiredService<IHttpClientFactory>().CreateClient(registrationName), discoverConfigurationSegment),
+            sp => new IdSvrHealthCheck(() => sp.GetRequiredService<IHttpClientFactory>().CreateClient(registrationName), discoverConfigurationSegment, isDynamicOpenIdProvider),
             failureStatus,
             tags,
             timeout));
@@ -52,6 +54,7 @@ public static IHealthChecksBuilder AddIdentityServer(
     /// <param name="builder">The <see cref="IHealthChecksBuilder"/>.</param>
     /// <param name="uriProvider">Factory for providing the uri of the Identity Server to check.</param>
     /// <param name="discoverConfigurationSegment">Identity Server discover configuration segment.</param>
+    /// <param name="isDynamicOpenIdProvider">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.</param>
     /// <param name="name">The health check name. Optional. If <c>null</c> the type name 'idsvr' will be used for the name.</param>
     /// <param name="failureStatus"></param>
     /// The <see cref="HealthStatus"/> that should be reported when the health check fails. Optional. If <c>null</c> then
@@ -63,6 +66,7 @@ public static IHealthChecksBuilder AddIdentityServer(
         this IHealthChecksBuilder builder,
         Func<IServiceProvider, Uri> uriProvider,
         string discoverConfigurationSegment = IDSVR_DISCOVER_CONFIGURATION_SEGMENT,
+        bool isDynamicOpenIdProvider = false,
         string? name = null,
         HealthStatus? failureStatus = null,
         IEnumerable<string>? tags = null,
@@ -74,7 +78,7 @@ public static IHealthChecksBuilder AddIdentityServer(
 
         return builder.Add(new HealthCheckRegistration(
             registrationName,
-            sp => new IdSvrHealthCheck(() => sp.GetRequiredService<IHttpClientFactory>().CreateClient(registrationName), discoverConfigurationSegment),
+            sp => new IdSvrHealthCheck(() => sp.GetRequiredService<IHttpClientFactory>().CreateClient(registrationName), discoverConfigurationSegment, isDynamicOpenIdProvider),
             failureStatus,
             tags,
             timeout));

diff --git a/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs b/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs
@@ -25,17 +25,21 @@ internal class DiscoveryEndpointResponse
     /// <summary>
     /// Validates Discovery response according to the <see href="https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata">OpenID specification</see>
     /// </summary>
-    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
@@ -8,16 +8,18 @@ public class IdSvrHealthCheck : IHealthCheck
 {
     private readonly Func<HttpClient> _httpClientFactory;
     private readonly string _discoverConfigurationSegment;
+    private readonly bool _isDynamicOpenIdProvider;
 
     public IdSvrHealthCheck(Func<HttpClient> httpClientFactory)
-        : this(httpClientFactory, IdSvrHealthCheckBuilderExtensions.IDSVR_DISCOVER_CONFIGURATION_SEGMENT)
+        : this(httpClientFactory, IdSvrHealthCheckBuilderExtensions.IDSVR_DISCOVER_CONFIGURATION_SEGMENT, false)
     {
     }
 
-    public IdSvrHealthCheck(Func<HttpClient> httpClientFactory, string discoverConfigurationSegment)
+    public IdSvrHealthCheck(Func<HttpClient> httpClientFactory, string discoverConfigurationSegment, bool isDynamicOpenIdProvider)
     {
         _httpClientFactory = Guard.ThrowIfNull(httpClientFactory);
         _discoverConfigurationSegment = discoverConfigurationSegment;
+        _isDynamicOpenIdProvider = isDynamicOpenIdProvider;
     }
 
     /// <inheritdoc />
@@ -39,7 +41,7 @@ public async Task<HealthCheckResult> 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
@@ -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<ArgumentException>()
@@ -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<ArgumentException>()
@@ -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<ArgumentException>()