feat: add PKCE support

[balazsorban44]

What:

Adding support for Proof Key for Code Exchange by OAuth Public Clients

Why:

Many popular Identity Providers require us to support this flow: https://auth0.com/docs/flows/authorization-code-flow-with-proof-key-for-code-exchange-pkce

How:

A new option to providers:

  protection: "pkce"

If this value is found, next-auth will create a code_challenge - code_verifier pair, which then will respectively be passed to the OAuth2 /authorize and /token endpoints. The code_verifier is saved in a cookie before redirecting to /authorize, and then read and removed before calling the /token endpoint.

Checklist:

• Documentation
• Tests
• Ready to be merged
  

NOTE: This is highly experimental, and not sure yet if creating a single challenge-verifier pair for the entire app is good/secure enough. I just wanted to have this some place, so I can start experimenting.

Closes #685, closes #502 (potentially others as well, please link to this issue, if you stumble upon other open, related issues.)

Useful sources:
YouTube: https://www.youtube.com/watch?v=Gtbm5Fut-j8
Spec: https://tools.ietf.org/html/rfc7636
Auth0 article: https://auth0.com/docs/flows/authorization-code-flow-with-proof-key-for-code-exchange-pkce

Maybe needed, if we need to create the code in the browser(?):
https://developer.mozilla.org/en-US/docs/Web/API/Crypto/subtle

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/rhd2arnzn
✅ Preview: https://next-auth-git-feature-pkce.nextauthjs.vercel.app

[balazsorban44]

Length of 64 was arbitrarily chosen here. The requirement by the spec is 43-128 characters.

Also, this is created for the entire app, meaning this will be the same for all the users. This makes the whole feature much less complicated, but I am not sure if this brings any security issues. If I remember our chat earlier @iaincollins, you said that in theory, we wouldn't even need to care about PKCE. Am I right, or we should find a way to create this per user?

The problem is that in that case, the values must be saved somewhere, so the challenge and verifier can be passed to both /authorize and /token respectively

[mikecabana]

In terms of saving the challenge and verifier, it could be saved in local storage (maybe session) to be cleaned up after a successful log in.

[balazsorban44]

I am not sure we would like to store anything sensitive this way, cause it is perceptible with potentially malicious code on the client. If I understand it correctly, sessionStorage would be a better option in this cause, although I am still not sure it is good enough. Going to discuss it with @iaincollins hopefully this weekend, I hope he has some insights as well.

Maybe the way you describe it IS the way we have to go, we will see. Thanks for weighing in!

[mikecabana]

No problem! I inspired myself from oidc-client-js which I'm currently using in a production spa. They use localStorage for the code_verifier and sessionStorage for the tokens.

Via the spec every new authorization flow should use a new code_verifier so this could be why they've opted to go this route. I'm not really sure either what kind of security concerns this method raises.

In any case I've set up a simple sample to generate the code_verifier and code_challenge in the browser. I don't think it's perfect but it can help to get you started. https://gist.github.com/mikecabana/9cae8b447afd2a36da5d38b94bfc2565

[balazsorban44]

In theory we wouldn't actually need PKCE to be secure I think, since our secrets are all server-side and never accessed by the client. So this is why I went with my implementation. Although there might be something important I leave out. We will have a talk about this soon.

[iaincollins]

@balazsorban44 FWIW for the record this is exactly my current understanding as well.

[balazsorban44]

This can probably be removed and we can create our own, added to be able to get started a bit faster

[balazsorban44]

openid-client has this built-in, we can utilize it when/if we finish #1105

[balazsorban44]

https://stackoverflow.com/a/56873098/5364135 it might just be OK to consider our server app as one single client, and we don't need to generate separate challenges for each user.

(or I am misunderstanding something).

[balazsorban44]

Added some changes. The code_verifier is now saved in a cookie, to be able to create a verifier/challenge pair per user.

Also, the previous method wouldn't have worked anyway, because each invocation would have created a new verifier/challenge pair, and the signin flow would have failed.

This still needs some re-thinking, just sharing my implementation in hope that someone may come with a good solution.

The way I see it, we have 4 alternatives:

1. generate a deterministic code_challenge/code_verifier at the start from the secret provided by the user, and save it in req.options
2. generate a code_challenge/code_verifier from the secret provided by the user at signin and save it in a cookie (encrypted)
3. generate a code_challenge/code_verifier from the secret provided by the user at signin and save it in state (encrypted)
4. generate a code_challenge/code_verifier client-side, store the code_verifier in a cookie, and provide it to /api/auth/callback when needed

[thulstrup]

@balazsorban44 The beta version of nextjs-auth0 supports PKCE. Maybe you can use that to validate your implementation?
https://github.com/auth0/nextjs-auth0/tree/beta

[balazsorban44]

Thanks! I'll definitely check!

I also checked the docs of openid-client which we might turn to later on (#1105)

and there they say this:

store the code_verifier in your framework's session mechanism, if it is a cookie based solution // it should be httpOnly (not readable by javascript) and encrypted.

So option 2 might be the best alternative.

[balazsorban44]

@thulstrup thanks again, your info was very useful! I went with option 2 (generate a code_challenge/code_verifier from the secret provided by the user at signin and save it in a cookie (encrypted)), as someone at nextjs-auth0 confirmed they also saved the code_verifier in an encrypted cookie.

What I am starting to settle on, that we should introduce a new Provider option called protection (maybe a better name?), that can take either pkce or state as a value. Current providers that support PKCE can use the features added in this PR, while providers only supporting state can use the already present features. If a Provider does not support either pkce or state, then no extra checks will be added in the login flow. (As far as I know, currently only the Apple provider is opted-out of thestate check)

I added Auth0 as an example to the dev application (start with npm run dev after adding the right env variables in .env.local), please test it out!

[github-actions]

🎉 This PR is included in version 3.2.0-canary.29 🎉

The release is available on:

• GitHub release
• npm package (@canary dist-tag)

Your semantic-release bot 📦🚀

[thulstrup]

@balazsorban44 That is awesome! I will test later today.

[thulstrup]

@balazsorban44 I had to fix one tiny error to get it working:
import logger from 'src/lib/logger' should be import logger from '../../lib/logger'

After that change I was able to log in using PKCE 🎉

[balazsorban44]

I see! Will fix immediately. Fixed, should be OK in the canary 30 release. Thanks!

[github-actions]

🎉 This PR is included in version 3.3.0-canary.1 🎉

The release is available on:

• GitHub release
• npm package (@canary dist-tag)

Your semantic-release bot 📦🚀

[kierancrown]

I might be a little dumb here. But where do I add this within the config? I can't see anything in the type files.

[aakash14goplani]

Can I use code-verifier from cookie as is or it needs some decryption?