closes #2311 - wellknown is missing pkce related info

[gbolo]

Related issue

#2311 - wellknown is missing pkce related info

Proposed changes

adds additional parameters to wellknown response as defined in RFC 8414

Checklist

• I have read the contributing guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security. vulnerability, I
  confirm that I got green light (please contact
  security@ory.sh) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

[CLAassistant]

All committers have signed the CLA.

[aeneasr]

Thank you! The idea here being that we follow https://datatracker.ietf.org/doc/html/rfc8414 ?

[gbolo]

Thank you! The idea here being that we follow https://datatracker.ietf.org/doc/html/rfc8414 ?

Hi @aeneasr. Just pointing out section-2 of the above RFC that describes the required parameter to inform the oidc client that this authorization server supports PKCE. I'm assuming that PKCE (as described in RFC7636) is already implemented. If this is true then we must advertise the "PKCE Code Challenge Methods".

   code_challenge_methods_supported
      OPTIONAL.  JSON array containing a list of Proof Key for Code
      Exchange (PKCE) [RFC7636] code challenge methods supported by this
      authorization server.  Code challenge method values are used in
      the "code_challenge_method" parameter defined in Section 4.3 of
      [RFC7636].  The valid code challenge method values are those
      registered in the IANA "PKCE Code Challenge Methods" registry
      [IANA.OAuth.Parameters].  If omitted, the authorization server
      does not support PKCE.

[aeneasr]

I see, thank you! As far as I can tell, it appears that the linked RFC would assume providing this information at a different endpoint (not /.well-known/openid-connect but instead /.well-known/oauth-authorization-server).

So I think it would make sense to open a new endpoint to follow this RFC and then add the values there?

[gbolo]

I see, thank you! As far as I can tell, it appears that the linked RFC would assume providing this information at a different endpoint (not /.well-known/openid-connect but instead /.well-known/oauth-authorization-server).

So I think it would make sense to open a new endpoint to follow this RFC and then add the values there?

I believe that we can use the existing URL as many other providers do it (such as google: https://accounts.google.com/.well-known/openid-configuration)

[aeneasr]

I checked the RFC and it looks to me as they strictly require the document to be discoverable at /.well-known/oauth-authorization-server. See also Section 3 of RFC 8414.

However, as you said, this also appears to be exposed at openid-configuration. Do you know any client libraries which would respect this field at openid-configuration?

[gbolo]

I checked the RFC and it looks to me as they strictly require the document to be discoverable at /.well-known/oauth-authorization-server. See also Section 3 of RFC 8414.

However, as you said, this also appears to be exposed at openid-configuration. Do you know any client libraries which would respect this field at openid-configuration?

Hi @aeneasr, I haven't used any OIDC sdks. However, I have written my own for educational purposes ONLY (https://github.com/gbolo/muggle-oidc/blob/main/oidc/discovery.go#L21) which respects this parameter at openid-configuration

I quickly looked around for any popular openid sdks and i found this one for js which seems to do something similar: https://github.com/panva/node-openid-client

If you would prefer to make a separate endpoint, I can try to update the PR to accomplish that

[aeneasr]

As it has come up before for other endpoints (e.g. revokation) I think it would make sense to implement RFC 8414 according to spec so we can address the other things as well then :) I think that would be the most straight forward way & spec compliant. If there is indeed a popular library expecting this at openid-configuration we could still add it there (as i. this PR:) )

[gbolo]

As it has come up before for other endpoints (e.g. revokation) I think it would make sense to implement RFC 8414 according to spec so we can address the other things as well then :) I think that would be the most straight forward way & spec compliant. If there is indeed a popular library expecting this at openid-configuration we could still add it there (as i. this PR:) )

hey @aeneasr after reading the section 3 you pointed out, it seems like we are fine keeping it the same:

   Some OAuth applications will choose to use the well-known URI suffix
   "openid-configuration".  As described in Section 5, despite the
   identifier "/.well-known/openid-configuration", appearing to be
   OpenID specific, its usage in this specification is actually
   referring to a general OAuth 2.0 feature that is not specific to
   OpenID Connect.

[aeneasr]

I finally found the relevant line in the spec (RFC 8414):

Additional authorization server metadata parameters MAY also be used.
Some are defined by other specifications, such as OpenID Connect
Discovery 1.0 [OpenID.Discovery].

And OpenID Provider Metadata says

Additional OpenID Provider Metadata parameters MAY also be used. Some are defined by other specifications, such as OpenID Connect Session Management 1.0 [OpenID.Session].

So adding RFC 8414 to OpenID PM should be allowed :) Thus not requiring a new endpoint!