Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow RS512 as valid signing algorithm #2176

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,77 @@ public static IHealthChecksBuilder AddIdentityServer(
tags,
timeout));
}

/// <summary>
/// Add a health check for Identity Server.
/// </summary>
/// <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="requiredSigningAlgorithms">The signature algorithms that the identity server must support.</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
/// the default status of <see cref="HealthStatus.Unhealthy"/> will be reported.
/// </param>
/// <param name="tags">A list of tags that can be used to filter sets of health checks. Optional.</param>
/// <param name="timeout">An optional <see cref="TimeSpan"/> representing the timeout of the check.</param>
/// <returns>The specified <paramref name="builder"/>.</returns>
public static IHealthChecksBuilder AddIdentityServer(
this IHealthChecksBuilder builder,
Uri idSvrUri,
string[] requiredSigningAlgorithms,
string discoverConfigurationSegment = IDSVR_DISCOVER_CONFIGURATION_SEGMENT,
string? name = default,
HealthStatus? failureStatus = default,
IEnumerable<string>? 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<IHttpClientFactory>().CreateClient(registrationName), discoverConfigurationSegment, requiredSigningAlgorithms),
failureStatus,
tags,
timeout));
}

/// <summary>
/// Add a health check for Identity Server.
/// </summary>
/// <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="requiredSigningAlgorithms">The signature algorithms that the identity server must support.</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
/// the default status of <see cref="HealthStatus.Unhealthy"/> will be reported.
/// <param name="tags">A list of tags that can be used to filter sets of health checks. Optional.</param>
/// <param name="timeout">An optional <see cref="TimeSpan"/> representing the timeout of the check.</param>
/// <returns>The specified <paramref name="builder"/>.</returns>
public static IHealthChecksBuilder AddIdentityServer(
this IHealthChecksBuilder builder,
Func<IServiceProvider, Uri> uriProvider,
string[] requiredSigningAlgorithms,
string discoverConfigurationSegment = IDSVR_DISCOVER_CONFIGURATION_SEGMENT,
string? name = null,
HealthStatus? failureStatus = null,
IEnumerable<string>? 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<IHttpClientFactory>().CreateClient(registrationName), discoverConfigurationSegment, requiredSigningAlgorithms),
failureStatus,
tags,
timeout));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ 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(string[]? requiredAlgorithms = null)
{
ValidateValue(Issuer, OidcConstants.ISSUER);
ValidateValue(AuthorizationEndpoint, OidcConstants.AUTHORIZATION_ENDPOINT);
Expand All @@ -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, requiredAlgorithms ?? OidcConstants.REQUIRED_ALGORITHMS);
}

private static void ValidateValue(string value, string metadata)
Expand Down
14 changes: 11 additions & 3 deletions src/HealthChecks.OpenIdConnectServer/IdSvrHealthCheck.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,24 @@ public class IdSvrHealthCheck : IHealthCheck
{
private readonly Func<HttpClient> _httpClientFactory;
private readonly string _discoverConfigurationSegment;
private readonly string[]? _requiredAlgorithms;

public IdSvrHealthCheck(Func<HttpClient> httpClientFactory)
: this(httpClientFactory, IdSvrHealthCheckBuilderExtensions.IDSVR_DISCOVER_CONFIGURATION_SEGMENT)
: this(httpClientFactory, IdSvrHealthCheckBuilderExtensions.IDSVR_DISCOVER_CONFIGURATION_SEGMENT, OidcConstants.REQUIRED_ALGORITHMS)
{
}

public IdSvrHealthCheck(Func<HttpClient> httpClientFactory, string discoverConfigurationSegment)
{
_httpClientFactory = Guard.ThrowIfNull(httpClientFactory);
_discoverConfigurationSegment = discoverConfigurationSegment;
_requiredAlgorithms = null;
}

public IdSvrHealthCheck(Func<HttpClient> httpClientFactory, string discoverConfigurationSegment, string[] requiredAlgorithms)
{
_httpClientFactory = Guard.ThrowIfNull(httpClientFactory);
_discoverConfigurationSegment = discoverConfigurationSegment;
_requiredAlgorithms = requiredAlgorithms;
}

/// <inheritdoc />
Expand All @@ -39,7 +47,7 @@ public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context
.ConfigureAwait(false)
?? throw new ArgumentException("Could not deserialize to discovery endpoint response!");

discoveryResponse.ValidateResponse();
discoveryResponse.ValidateResponse(_requiredAlgorithms);

return HealthCheckResult.Healthy();
}
Expand Down
2 changes: 1 addition & 1 deletion src/HealthChecks.OpenIdConnectServer/OidcConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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" };
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,36 @@ public void add_named_health_check_when_properly_configured_with_uri_provider()
registration.Name.ShouldBe("my-idsvr-group");
check.ShouldBeOfType<IdSvrHealthCheck>();
}
[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<IOptions<HealthCheckServiceOptions>>();

var registration = options.Value.Registrations.First();
var check = registration.Factory(serviceProvider);

registration.Name.ShouldBe("idsvr");
check.ShouldBeOfType<IdSvrHealthCheck>();
}
[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<IOptions<HealthCheckServiceOptions>>();

var registration = options.Value.Registrations.First();
var check = registration.Factory(serviceProvider);

registration.Name.ShouldBe("idsvr");
check.ShouldBeOfType<IdSvrHealthCheck>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,69 @@ public void be_invalid_when_required_id_token_signing_alg_values_supported_is_mi

validate
.ShouldThrow<ArgumentException>()
.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)}!");
}

[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<ArgumentException>()
.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")]
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]
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]
Expand Down