ory / kratos

Next-gen identity server replacing your Auth0, Okta, Firebase with hardened security and PassKeys, SMS, OIDC, Social Sign In, MFA, FIDO, TOTP and OTP, WebAuthn, passwordless and much more. Golang, headless, API-first. Available as a worry-free SaaS with the fairest pricing on the market!
https://www.ory.sh/kratos/?utm_source=github&utm_medium=banner&utm_campaign=kratos
Apache License 2.0
11.03k stars 951 forks source link

Support for PKCE in OIDC social providers #4009

Open OskarsPakers opened 1 month ago

OskarsPakers commented 1 month ago

Preflight checklist

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 https://github.com/ory/kratos/blob/ff90216f03e7b9c8112f07474fa34d960eb9ceb9/selfservice/strategy/oidc/strategy.go#L526

Version

2.2.0

Additional Context

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

IchordeDionysos commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 3 weeks ago

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 (https://github.com/coreos/go-oidc/issues/401) with fallback to non-PKCE flows in case of failures for backwards-compatiblity.

OskarsPakers commented 3 weeks ago

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 commented 3 weeks ago

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.