From 35f293640d75f25f884f8bc3c2562583fe6d556a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gugler=20Michael=20=28O=C3=96=20Gemdat=29?= Date: Sun, 10 Mar 2024 08:39:41 +0100 Subject: [PATCH 1/2] The most significant changes involve the validation of signing algorithms in the `DiscoveryEndpointResponse` class and the addition of "RS512" as a required algorithm in the `OidcConstants` class. A new test method has also been added to the `discovery_endpoint_response_should` class to test the validation of the `DiscoveryEndpointResponse` when one of the required signing algorithms is provided. 1. The `DiscoveryEndpointResponse` class in `DiscoveryEndpointResponse.cs` now uses the `ValidateOneOfRequiredValues` method instead of `ValidateRequiredValues` for `SigningAlgorithmsSupported`. This allows for the validation of multiple signing algorithms rather than just one. 2. The `OidcConstants` class in `OidcConstants.cs` has been updated to include "RS512" as a required algorithm, in addition to "RS256". This expands the list of required signing algorithms. 3. The `discovery_endpoint_response_should` class in `DiscoveryEndpointResponseTests.cs` has been updated with a new test method `be_valid_when_one_required_id_token_signing_alg_value_is_provided`. This method tests the validation of the `DiscoveryEndpointResponse` when one of the required signing algorithms is provided. It uses the `InlineData` attribute to test with both "RS256" and "RS512". The error message in the `validate` action has also been updated to reflect these changes. --- .../DiscoveryEndpointResponse.cs | 2 +- .../OidcConstants.cs | 2 +- .../DiscoveryEndpointResponseTests.cs | 22 ++++++++++++++++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs b/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs index b34427e943..8d8510958b 100644 --- a/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs +++ b/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs @@ -37,7 +37,7 @@ public void ValidateResponse() // 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); + ValidateOneOfRequiredValues(SigningAlgorithmsSupported, OidcConstants.ALGORITHMS_SUPPORTED, OidcConstants.REQUIRED_ALGORITHMS); } private static void ValidateValue(string value, string metadata) diff --git a/src/HealthChecks.OpenIdConnectServer/OidcConstants.cs b/src/HealthChecks.OpenIdConnectServer/OidcConstants.cs index 8df2c8fdab..cc5dd12f9c 100644 --- a/src/HealthChecks.OpenIdConnectServer/OidcConstants.cs +++ b/src/HealthChecks.OpenIdConnectServer/OidcConstants.cs @@ -20,5 +20,5 @@ internal class OidcConstants internal static string[] REQUIRED_SUBJECT_TYPES => new[] { "pairwise", "public" }; - internal static string[] REQUIRED_ALGORITHMS => new[] { "RS256" }; + internal static string[] REQUIRED_ALGORITHMS => new[] { "RS256", "RS512" }; } diff --git a/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs b/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs index 7752b8b395..59eb01739a 100644 --- a/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs +++ b/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs @@ -129,7 +129,27 @@ public void be_invalid_when_required_id_token_signing_alg_values_supported_is_mi validate .ShouldThrow() - .Message.ShouldBe("Invalid discovery response - 'id_token_signing_alg_values_supported' must contain the following values: RS256!"); + .Message.ShouldBe($"Invalid discovery response - 'id_token_signing_alg_values_supported' must be one of the following values: {string.Join(",", OidcConstants.REQUIRED_ALGORITHMS)}!"); + } + + [Theory] + [InlineData("RS256")] + [InlineData("RS512")] + public void be_valid_when_one_required_id_token_signing_alg_value_is_provided(string supportedSigningAlgorithm) + { + var response = new DiscoveryEndpointResponse + { + Issuer = RandomString, + AuthorizationEndpoint = RandomString, + JwksUri = RandomString, + ResponseTypesSupported = REQUIRED_RESPONSE_TYPES, + SubjectTypesSupported = OidcConstants.REQUIRED_SUBJECT_TYPES, + SigningAlgorithmsSupported = new[] { supportedSigningAlgorithm }, + }; + + Action validate = () => response.ValidateResponse(); + + validate.ShouldNotThrow(); } [Fact] From 64846baa3c3e684c5819aa65ccc9259f8ed52cf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gugler=20Michael=20=28O=C3=96=20Gemdat=29?= Date: Mon, 18 Mar 2024 15:16:35 +0100 Subject: [PATCH 2/2] The required signature algorithms can now be optionally configured. --- .../IdSvrHealthCheckBuilderExtensions.cs | 73 +++++++++++++++++++ .../DiscoveryEndpointResponse.cs | 4 +- .../IdSvrHealthCheck.cs | 14 +++- .../DependencyInjection/RegistrationTests.cs | 32 ++++++++ .../DiscoveryEndpointResponseTests.cs | 42 +++++++++++ 5 files changed, 160 insertions(+), 5 deletions(-) diff --git a/src/HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs b/src/HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs index cad4e41af8..b7e47b8750 100644 --- a/src/HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs +++ b/src/HealthChecks.OpenIdConnectServer/DependencyInjection/IdSvrHealthCheckBuilderExtensions.cs @@ -79,4 +79,77 @@ public static IHealthChecksBuilder AddIdentityServer( tags, timeout)); } + + /// + /// Add a health check for Identity Server. + /// + /// The . + /// The uri of the Identity Server to check. + /// Identity Server discover configuration segment. + /// The signature algorithms that the identity server must support. + /// 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 + /// the default status of will be reported. + /// + /// A list of tags that can be used to filter sets of health checks. Optional. + /// An optional representing the timeout of the check. + /// The specified . + public static IHealthChecksBuilder AddIdentityServer( + this IHealthChecksBuilder builder, + Uri idSvrUri, + string[] requiredSigningAlgorithms, + string discoverConfigurationSegment = IDSVR_DISCOVER_CONFIGURATION_SEGMENT, + string? name = default, + HealthStatus? failureStatus = default, + IEnumerable? tags = default, + TimeSpan? timeout = default) + { + var registrationName = name ?? NAME; + + builder.Services.AddHttpClient(registrationName, client => client.BaseAddress = idSvrUri); + + return builder.Add(new HealthCheckRegistration( + registrationName, + sp => new IdSvrHealthCheck(() => sp.GetRequiredService().CreateClient(registrationName), discoverConfigurationSegment, requiredSigningAlgorithms), + failureStatus, + tags, + timeout)); + } + + /// + /// Add a health check for Identity Server. + /// + /// The . + /// Factory for providing the uri of the Identity Server to check. + /// Identity Server discover configuration segment. + /// The signature algorithms that the identity server must support. + /// 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 + /// the default status of will be reported. + /// A list of tags that can be used to filter sets of health checks. Optional. + /// An optional representing the timeout of the check. + /// The specified . + public static IHealthChecksBuilder AddIdentityServer( + this IHealthChecksBuilder builder, + Func uriProvider, + string[] requiredSigningAlgorithms, + string discoverConfigurationSegment = IDSVR_DISCOVER_CONFIGURATION_SEGMENT, + string? name = null, + HealthStatus? failureStatus = null, + IEnumerable? tags = null, + TimeSpan? timeout = null) + { + var registrationName = name ?? NAME; + + builder.Services.AddHttpClient(registrationName, (sp, client) => client.BaseAddress = uriProvider(sp)); + + return builder.Add(new HealthCheckRegistration( + registrationName, + sp => new IdSvrHealthCheck(() => sp.GetRequiredService().CreateClient(registrationName), discoverConfigurationSegment, requiredSigningAlgorithms), + failureStatus, + tags, + timeout)); + } } diff --git a/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs b/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs index 8d8510958b..063c2667cc 100644 --- a/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs +++ b/src/HealthChecks.OpenIdConnectServer/DiscoveryEndpointResponse.cs @@ -25,7 +25,7 @@ internal class DiscoveryEndpointResponse /// /// Validates Discovery response according to the OpenID specification /// - public void ValidateResponse() + public void ValidateResponse(string[]? requiredAlgorithms = null) { ValidateValue(Issuer, OidcConstants.ISSUER); ValidateValue(AuthorizationEndpoint, OidcConstants.AUTHORIZATION_ENDPOINT); @@ -37,7 +37,7 @@ public void ValidateResponse() // 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); - ValidateOneOfRequiredValues(SigningAlgorithmsSupported, OidcConstants.ALGORITHMS_SUPPORTED, OidcConstants.REQUIRED_ALGORITHMS); + ValidateOneOfRequiredValues(SigningAlgorithmsSupported, OidcConstants.ALGORITHMS_SUPPORTED, requiredAlgorithms ?? OidcConstants.REQUIRED_ALGORITHMS); } private static void ValidateValue(string value, string metadata) diff --git a/src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs b/src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs index 62bafadf77..9141522ee1 100644 --- a/src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs +++ b/src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs @@ -8,16 +8,24 @@ public class IdSvrHealthCheck : IHealthCheck { private readonly Func _httpClientFactory; private readonly string _discoverConfigurationSegment; + private readonly string[]? _requiredAlgorithms; public IdSvrHealthCheck(Func httpClientFactory) - : this(httpClientFactory, IdSvrHealthCheckBuilderExtensions.IDSVR_DISCOVER_CONFIGURATION_SEGMENT) + : this(httpClientFactory, IdSvrHealthCheckBuilderExtensions.IDSVR_DISCOVER_CONFIGURATION_SEGMENT, OidcConstants.REQUIRED_ALGORITHMS) { } - public IdSvrHealthCheck(Func httpClientFactory, string discoverConfigurationSegment) { _httpClientFactory = Guard.ThrowIfNull(httpClientFactory); _discoverConfigurationSegment = discoverConfigurationSegment; + _requiredAlgorithms = null; + } + + public IdSvrHealthCheck(Func httpClientFactory, string discoverConfigurationSegment, string[] requiredAlgorithms) + { + _httpClientFactory = Guard.ThrowIfNull(httpClientFactory); + _discoverConfigurationSegment = discoverConfigurationSegment; + _requiredAlgorithms = requiredAlgorithms; } /// @@ -39,7 +47,7 @@ public async Task CheckHealthAsync(HealthCheckContext context .ConfigureAwait(false) ?? throw new ArgumentException("Could not deserialize to discovery endpoint response!"); - discoveryResponse.ValidateResponse(); + discoveryResponse.ValidateResponse(_requiredAlgorithms); return HealthCheckResult.Healthy(); } diff --git a/test/HealthChecks.OpenIdConnectServer.Tests/DependencyInjection/RegistrationTests.cs b/test/HealthChecks.OpenIdConnectServer.Tests/DependencyInjection/RegistrationTests.cs index a6ab2c74bc..1dc3687f2a 100644 --- a/test/HealthChecks.OpenIdConnectServer.Tests/DependencyInjection/RegistrationTests.cs +++ b/test/HealthChecks.OpenIdConnectServer.Tests/DependencyInjection/RegistrationTests.cs @@ -66,4 +66,36 @@ public void add_named_health_check_when_properly_configured_with_uri_provider() registration.Name.ShouldBe("my-idsvr-group"); check.ShouldBeOfType(); } + [Fact] + public void add_health_check_when_properly_configured_with_requiredSigningAlgorithms() + { + var services = new ServiceCollection(); + services.AddHealthChecks() + .AddIdentityServer(new Uri("http://myidsvr"), requiredSigningAlgorithms: ["RS256"]); + + using var serviceProvider = services.BuildServiceProvider(); + var options = serviceProvider.GetRequiredService>(); + + var registration = options.Value.Registrations.First(); + var check = registration.Factory(serviceProvider); + + registration.Name.ShouldBe("idsvr"); + check.ShouldBeOfType(); + } + [Fact] + public void add_health_check_when_properly_configured_with_uri_provider_and_requiredSigningAlgorithms() + { + var services = new ServiceCollection(); + services.AddHealthChecks() + .AddIdentityServer(sp => new Uri("http://myidsvr"), requiredSigningAlgorithms: ["RS256"]); + + using var serviceProvider = services.BuildServiceProvider(); + var options = serviceProvider.GetRequiredService>(); + + var registration = options.Value.Registrations.First(); + var check = registration.Factory(serviceProvider); + + registration.Name.ShouldBe("idsvr"); + check.ShouldBeOfType(); + } } diff --git a/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs b/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs index 59eb01739a..1ecd2cd765 100644 --- a/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs +++ b/test/HealthChecks.OpenIdConnectServer.Tests/Functional/DiscoveryEndpointResponseTests.cs @@ -132,6 +132,28 @@ public void be_invalid_when_required_id_token_signing_alg_values_supported_is_mi .Message.ShouldBe($"Invalid discovery response - 'id_token_signing_alg_values_supported' must be one of the following values: {string.Join(",", OidcConstants.REQUIRED_ALGORITHMS)}!"); } + [Fact] + public void be_invalid_when_custom_required_id_token_signing_alg_values_supported_is_wrong() + { + string[] requiredSigningAlgorithms = { "ES512" }; + + var response = new DiscoveryEndpointResponse + { + Issuer = RandomString, + AuthorizationEndpoint = RandomString, + JwksUri = RandomString, + ResponseTypesSupported = REQUIRED_RESPONSE_TYPES, + SubjectTypesSupported = OidcConstants.REQUIRED_SUBJECT_TYPES, + SigningAlgorithmsSupported = new[] { "RS256" }, + }; + + Action validate = () => response.ValidateResponse(requiredSigningAlgorithms); + + validate + .ShouldThrow() + .Message.ShouldBe($"Invalid discovery response - 'id_token_signing_alg_values_supported' must be one of the following values: {string.Join(",", requiredSigningAlgorithms)}!"); + } + [Theory] [InlineData("RS256")] [InlineData("RS512")] @@ -152,6 +174,26 @@ public void be_valid_when_one_required_id_token_signing_alg_value_is_provided(st validate.ShouldNotThrow(); } + [Fact] + public void be_valid_when_custom_required_id_token_signing_alg_value_is_provided() + { + string[] requiredSigningAlgorithms = { "ES512" }; + + var response = new DiscoveryEndpointResponse + { + Issuer = RandomString, + AuthorizationEndpoint = RandomString, + JwksUri = RandomString, + ResponseTypesSupported = REQUIRED_RESPONSE_TYPES, + SubjectTypesSupported = OidcConstants.REQUIRED_SUBJECT_TYPES, + SigningAlgorithmsSupported = new[] { "ES512" }, + }; + + Action validate = () => response.ValidateResponse(requiredSigningAlgorithms); + + validate.ShouldNotThrow(); + } + [Fact] public void be_valid_when_all_required_values_are_provided() {