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

Fix strict OpenIdConnect server response validation #2164

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hschink
Copy link

@hschink hschink commented Feb 15, 2024

What this PR does / why we need it:

This PR introduces a new parameter for the OpenIdConnect health check which allows to control the validation of the OpenId Provider response.

Specifically, with this PR users can enable/disable the validation of the response_types_supported parameter for Dynamic OpenID Providers. The specification states:

REQUIRED. JSON array containing a list of the OAuth 2.0 response_type values that this OP supports. Dynamic OpenID Providers MUST support the code, id_token, and the id_token token Response Type values.

Which issue(s) this PR fixes:

#2152

Special notes for your reviewer:

On my local machine test be_healthy_if_idsvr_is_available fails. I assume this is related to my local setup.

Does this PR introduce a user-facing change?:

Yes, by default the Dynamic OpenID Providers-specific validation is disabled. Current users of the validation need to activate the check explicitly.

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Unit tests passing
  • End-to-end tests passing
  • Extended the documentation
  • Provided sample for the feature

@hschink
Copy link
Author

hschink commented Feb 15, 2024

@dotnet-policy-service agree company="Trapeze"

@hschink hschink force-pushed the fix/Strict_OpenIdConnectServer_response_validation branch from 6d38118 to e453429 Compare February 15, 2024 12:54
@unaizorrilla unaizorrilla self-assigned this Mar 30, 2024
@unaizorrilla unaizorrilla self-requested a review March 30, 2024 13:11
@nickljones
Copy link

@hschink / @unaizorrilla - is there anything I can do to help get this PR merged?

@hschink
Copy link
Author

hschink commented Aug 13, 2024

@hschink / @unaizorrilla - is there anything I can do to help get this PR merged?

@nickljones Waiting for comments myself. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants