IdentityModel / IdentityModel.OidcClient

Certified C#/NetStandard OpenID Connect Client Library for native mobile/desktop Applications (RFC 8252)
Apache License 2.0
599 stars 175 forks source link

committing suggested changes for issue #415 #416

Closed RobK410 closed 7 months ago

RobK410 commented 9 months ago

Code changes made to support the use of Discovery Policy Authority as override to existing Options.Authority when setting Disco Request Address. All tests pass after code change.

RobK410 commented 9 months ago

I am running now into an exception "Error loading discovery document: Issuer name does not match authority: https://cognito-idp.us-east-2.amazonaws.com/...' with my proposed changes.

Also, changing Address to include sub-paths from Cognito results in exception error "Error loading discovery document: Endpoint is on a different host than authority: https://{mydomain}.auth.{region}.amazoncognito.com/oauth2/authorize"

I thought this could be an issue, and it does appear there is a security business rule that is blocking this address change for the discovery Uri. Discovery Policy ValidateEndpoints = false is a valid solution.

RobK410 commented 8 months ago

@RobK410, we saw a similar issue in the IdentityModel repo, and I think you might be able to solve your current problem with similar usage: IdentityModel/IdentityModel#553 (comment). Then my question would be, can you use the AdditionalEndpointBaseAddresses in the discovery policy to accomplish what you want to do here in oidc client, or do we need to make a change to support that here? If we do need to make a change, we should go out of our way to make it really obvious (or at least documented in xmldoc) what the semantics are of the two different authorities, when to use one or the other, and what happens if both are specified.

I'm not 100% certain that I understand what cognito is doing. Can you show an example of a cognito discovery endpoint - both the url and the actual content that cognito produces? That would help me a lot, thanks!

I think the additional endpoint based address may eliminate the need for me to disable endpoint validation due to the different target base addresses, so thanks for that reference! I'll give it a try.

However, the original issue would still remain, that the Disco request (on or about Line 401) uses the main Options.Authority base address, whereas it should IMO use the Authority address specified in the Options.Policy.Discovery.Authority property.

Reasoning: a) context; to me, a discovery request would fully utilize the options/properties of the discovery policy. b) Flexibility: Because AWS Cognito (and perhaps other Cloud IdPs) has a separate Discovery Authority address that differs from the IdP Authority, this would allow developers the flexibility to use a different discovery Authority on the discovery request.

e.g., AWS Cognito Options.Authority = https://{my user pool name}.auth.us-east-2.amazoncognito.com All Cognito Discovery = https://cognito-idp.{region}.amazonaws.com/{your user pool ID}/.well-known/openid-configuration

I don't see any other configurable option for this during the disco request to use the different authority for discovery other than the proposed code change.

Cognito Discovery Document Example: https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒/.well-known/openid-configuration

{"authorization_endpoint":"https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/oauth2/authorize","end_session_endpoint":"https://▒▒▒▒▒auth.us-east-2.amazoncognito.com/logout","id_token_signing_alg_values_supported":["RS256"],"issuer":"https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒▒","jwks_uri":"https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒/.well-known/jwks.json","response_types_supported":["code","token"],"revocation_endpoint":"https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/oauth2/revoke","scopes_supported":["openid","email","phone","profile"],"subject_types_supported":["public"],"token_endpoint":"https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/oauth2/token","token_endpoint_auth_methods_supported":["client_secret_basic","client_secret_post"],"userinfo_endpoint":"https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/oauth2/userInfo"}

josephdecock commented 8 months ago

I agree that the discovery policy's authority is a bit confusing since we also have the authority on the main options, but I'm a little confused by some of what else you're saying. What does it mean to have a separate discovery authority? It seems like https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒ just is the authority you should be using, as that is the base address where you can perform discovery and in the discovery response, cognito sensibly includes that as the issuer : "issuer":"https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒▒. The endpoint urls are at a different origin, but that shouldn't matter if you allow that in the policy. I feel like we're talking past each other here a little, sorry. What's the issuer (iss claim) of tokens produced by cognito in this example?

RobK410 commented 8 months ago

I agree that the discovery policy's authority is a bit confusing since we also have the authority on the main options, but I'm a little confused by some of what else you're saying. What does it mean to have a separate discovery authority? It seems like https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒ just is the authority you should be using, as that is the base address where you can perform discovery and in the discovery response, cognito sensibly includes that as the issuer : "issuer":"https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒▒. The endpoint urls are at a different origin, but that shouldn't matter if you allow that in the policy. I feel like we're talking past each other here a little, sorry. What's the issuer (iss claim) of tokens produced by cognito in this example?

AWS Cognito's multi-tenant Discovery URL domain (Authority) differs from that of the domain (Authority) of the Cognito API.

So to authorize I would call https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/oauth2/authorize But to get the disco document, I need to GET https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒/.well-known/openid-configuration

If all I do, is set the default Options.Authority = https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/ This OidcClient library, on (on or about Line 401) will always append /.well-known/openid-configuration to the end of it, and the disco request will call https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/.well-known/openid-configuration, which will fail.

Does that help?

josephdecock commented 7 months ago

I think you want to go the other way around, and set Options.Authority to https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒. Then, discovery should succeed, and the discovery document should contain the url to the endpoints within the https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/ domain. As long as you use policy to make that second domain valid, the library should use the results of discovery to obtain tokens.

RobK410 commented 7 months ago

I think you want to go the other way around, and set Options.Authority to https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒. Then, discovery should succeed, and the discovery document should contain the url to the endpoints within the https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/ domain. As long as you use policy to make that second domain valid, the library should use the results of discovery to obtain tokens.

That makes sense, and I'll give it a try.

RobK410 commented 7 months ago

I think you want to go the other way around, and set Options.Authority to https://cognito-idp.us-east-2.amazonaws.com/us-east-2_▒▒▒▒▒. Then, discovery should succeed, and the discovery document should contain the url to the endpoints within the https://▒▒▒▒▒.auth.us-east-2.amazoncognito.com/ domain. As long as you use policy to make that second domain valid, the library should use the results of discovery to obtain tokens.

That makes sense, and I'll give it a try.

@josephdecock Thanks for helping me work through the logic on this. My brain was telling me to use the authority provided by Cognito for the instance, vs using the discovery domain to source the urls. Your suggestion, including adding the AdditionalEndpointBaseAddresses reference for validation, helped, and I am now seeing the Cognito login.