Allow RS512 as valid signing algorithm #2176
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…thms 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.
|
@dotnet-policy-service agree |
m-gug
changed the title
|
I have added the possibility that the signature algorithms can be passed as parameters. |
|
Can this PR be merged? We also need for the signature algorithms to be configurable when setting up the Healthchecks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
The current implementation of the OpenIdConnect health check requires that the server supports RS256 as a signing algorithm.
If the server only supports RS512, the health check will fail.
This PR extends the checking of permitted signature algorithms to include RS512 (others can be added to the constant).
Which issue(s) this PR fixes:
#2175
Special notes for your reviewer:
The test be_healthy_if_idsvr_is_available fails in my setup - i guess it needs a running OIDC Server?
Does this PR introduce a user-facing change?:
No
Please make sure you've completed the relevant tasks for this PR, out of the following list: