wellknown is missing code_challenge_methods_supported

[gbolo]

Describe the bug

when pkce is enabled, i would expect the wellknown to advertise it's supported methods as defined in RFC 8414

   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.

The final sentence states: If omitted, the authorization server does not support PKCE.

Expected behavior

I expect to see the following snippet in the wellknown response:

...
"code_challenge_methods_supported": [
  "plain",
  "S256"
],
...

[aeneasr]

Hey there, yes, https://tools.ietf.org/html/rfc8414#section-2 is actually not supported ATM, we only support OpenID Connect Discovery. Would you be up for some PRs to address this?

[gbolo]

Hi @aeneasr, if this is something you would like to support I certainly don't mind addressing it in a PR. Whats the scope here? I'm assuming its to simply add the supported methods to the wellknown response when pkce is enabled on hydra. Is that correct? I will familiarize myself with the code base

[aeneasr]

@gbolo sorry for the late reply. Yes, that is correct, and also add a little test (probably 3-5 lines). If you need guidance I can point you to the files!

[gbolo]

hi @aeneasr

I looked through the code. It's fairly simple to modify the wellknown response to include the supported methods. However, I could not find any indication in the code that pkce is actually being validated. Adding support for pkce is a bit more than I bargained for ;) Can you verify this?

[aeneasr]

PKCE is supported and somewhat documented.

[gbolo]

hi @aeneasr it looks like pkce is supported from the client perspective, but I cannot find any server side verification being done. Can you confirm this?

[aeneasr]

Confirm what exactly?

[gbolo]

Confirm what exactly?

@aeneasr that the hydra server will validate the code_verifier with the already received code_challenge and the code_challenge_method during the client's access token request (PKCE)

[aeneasr]

That's what I tried to say #2311 (comment)

What was not clear about it / what could be improved?

[gbolo]

Hi @aeneasr sorry if I'm not being clear enough. I already made the required changes to the wellknown response, however i cannot find any evidence of the hydra server keeping track of the code challenge for validation later on when the token request comes in. Can you point me to the code that does this validation? It would be difficult for me to write that validation code, as I would have to get more intimate with the code base. I hope its clear now.

[aeneasr]

Ory Hydra supports PKCE as required by the specification and does the validation and everything. This happens in the library we are using: https://github.com/ory/fosite/

[gbolo]

thanks @aeneasr

I have modified the response and adjusted the testcase for the response. Please review #2547 when you get a chance.