Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New provider property: protection (support pkce, state, none) #1172

Closed
balazsorban44 opened this issue Jan 21, 2021 · 2 comments · Fixed by #1184
Closed

New provider property: protection (support pkce, state, none) #1172

balazsorban44 opened this issue Jan 21, 2021 · 2 comments · Fixed by #1184
Labels

Comments

@balazsorban44
Copy link
Member

balazsorban44 commented Jan 21, 2021

Summary of proposed feature
We should let users define what protective measures should be taken while logging into an OAuth provider, even if it is a custom one. The options are "state" (using the CSRF token), "pkce" (see the PKCE spec, also, introduced in #941) or "none".

So it should look like this:

{
...
  protection: "pkce" // or "state" or "none"
...
}

We should decide on what should be the default value. Currently state is always checked, except if state: false is set in the options, so protection: "state" seems to be the natural choice here. But is PKCE something we would like to encourage/prefer over state?

Potential problems
As state has been removed in canary, strictly speaking, it MIGHT be a breaking change for custom providers. I can easily add the state option back, and show a deprecation warning in favour of protection: "state".

Describe any alternatives you've considered
In the current stable release of next-auth, a provider can pass an option called state: boolean. This has been removed in the canary in #1022 to simplify the provider options. I was under the assumption that only Apple does not support this, so I hardcoded an Apple specific solution:

if (provider.id !== 'apple') {
const expectedState = createHash('sha256').update(csrfToken).digest('hex')
if (state !== expectedState) {
throw new OAuthCallbackError('Invalid state returned from OAuth provider')
}
}

In retrospect, I think it was not a good alternative, but with the new proposed option protection: "none", should fix this. (question. Anyone knows if Apple supports PKCE?)

Interested in what the community thinks!

@github-actions
Copy link

🎉 This issue has been resolved in version 3.2.0-canary.35 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

github-actions bot commented Feb 1, 2021

🎉 This issue has been resolved in version 3.3.0-canary.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant