Support for PKCE in OIDC social providers

[OskarsPakers]

Preflight checklist

• I could not find a solution in the existing issues, docs, nor discussions.
• I agree to follow this project's Code of Conduct.
• I have read and am following this repository's Contribution Guidelines.
• I have joined the Ory Community Slack.
• I am signed up to the Ory Security Patch Newsletter.

Ory Network Project

No response

Describe your problem

Some identity providers require Proof Key for Code Exchange (PKCE) code_challenge query parameter in authorization endpoint and code_verifier in token exchange endpoint

Describe your ideal solution

Generic provider had configuration to enable and generate code_challenge upon redirect to identity provider and sends verifier value when echanging code for token.
oauth2 go package already supports generating verifier and the challenge https://pkg.go.dev/golang.org/x/oauth2#S256ChallengeFromVerifier

Workarounds or alternatives

Implementing custom provider might be an option, but it seems that oidc strategy must be change too to pass AuthCodeOption to token exchange

kratos/selfservice/strategy/oidc/strategy.go

Line 526 in ff90216

token, err = te.Exchange(ctx, code)

Version

2.2.0

Additional Context

I`d like to submit pull request if this feature makes sense. Any hints appreciated

[IchordeDionysos]

Would be amazing to have, we've also ran into this twice now (in our case we could just disable PKCE).
Had this with some customers identity provider and recently I've experienced it while enabling support for Salesforce.

[aeneasr]

Yeah, that makes sense. There’s probably a library for Go that does that and that can easily be added to e.g. Salesforce

[IchordeDionysos]

Note: In Salesforce, you can disable PKCE :) See the documentation here ory/docs#1797

It would be nice to have that possibility in generic providers, though.

[OskarsPakers]

We plan to submit pull request for this in upcoming days.
The plan is to extend provider_config.go Configuration object with PKCSMethod attribute with possible value "S256" and if it is set, generate code_verifier, send code_challenge parameter to authentication request and code_verifier value to /token endpoint when exchanging the code. Does it sound ok?
What is the appropriate place to persist generated code_verifier between requests in the flow?
Extend State struct or maybe s.d.LoginFlowPersister() or s.d.SessionManager()?
Thank you

[alnr]

Hey. We will probably make some adjustments to how OIDC login works (internally) in the near future, so I would recommend maybe holding off on PRs for the moment (sorry I haven't had time yet to look at yours).

Also, we'd probably need auto-discovery of PKCE support (coreos/go-oidc#401) with fallback to non-PKCE flows in case of failures for backwards-compatiblity.

[OskarsPakers]

Well we already did the pull request. Should we then continue improving it with discovery and make it so that challenge is always sent if provider supports it and drop the pkcs_method from the config?

[alnr]

I'm quite confident we don't need to make PKCE configurable. The only scenario where that would make sense is when the provider advertises PKCE support but doesn't actually support it. Seems far-fetched. Then again, you never know.

The more difficult question is where to store the verifier. There will be some changes regarding that part of the code in the near future, so I would recommend holding off on that part of the implementation for now.

[aeneasr]

Thank you @OskarsPakers for your original work on this! It has made it‘s way in another PR to master now :)

[OskarsPakers]

Awesome! Thank you for finishing this!