Feature/nonce check type

[james-bjss]

Reasoning 💡

Adding the ability to generate and validate a nonce value when authenticating against an Open ID provider.

This resolves a "nonce missmatch" error raised when authenticating against an external IDP with Cognito.

Specifically this is to fix the following issue: https://github.com/nextauthjs/next-auth/discussions/3551#

Checklist 🧢

• [x] Documentation
• [ ] Tests
• [x] Ready to be merged

Affected issues 🎟

Fixes: #3551

Notes On Usage

• This has been Tested with Local Cognito users, Cognito + AAD and also in combination with PKCE and State checks.
• By default the check is disabled but can be enabled by setting the appropriate checks value eg.

import NextAuth from 'next-auth'
import CognitoProvider from 'next-auth/providers/cognito'

const handler = NextAuth({
  providers: [
    CognitoProvider({
      clientId: process.env.COGNITO_CLIENT_ID,
      clientSecret: process.env.COGNITO_CLIENT_SECRET,
      issuer: process.env.COGNITO_ISSUER,
      idToken: true,
      checks: 'nonce',
    }),
  ],
})

export default handler

[vercel[bot]]

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/HggmkVHNf7hrWyf2YA3TjN518wT6
✅ Preview: https://next-auth-git-fork-james-bjss-feature-nonce-c-021620-nextauthjs.vercel.app

[james-bjss]

@balazsorban44 - If you require a test-cognito endpoint and credentials I can potentially set this up for you on my personal account, just give me some notice and I can set it up :)

[james-bjss]

Hi @balazsorban44 sorry to bug you, but would it be possible to give an indication if this fix is viable and something you may be willing to accept into the project?

I wouldn't normally chase up on PRs, but we are faced with a difficult decision on our project at the moment. Basically:

1. Continue to use NextAuth with a fork containing this PR in the hope that the issue will be eventually be fixed.
2. Start looking at refactoring to use Amplify libraries (which potentially brings with it other issues)

I realise this is a side-project and you are busy, but just looking for some confidence that a fix will be forthcoming if we chose Option1 even if its not immediate.

[james-bjss]

Hi @balazsorban44 sorry to bug you, but would it be possible to give an indication if this fix is viable and something you may be willing to accept into the project?

I wouldn't normally chase up on PRs, but we are faced with a difficult decision on our project at the moment. Basically:

1. Continue to use NextAuth with a fork containing this PR in the hope that the issue will be eventually be fixed.
2. Start looking at refactoring to use Amplify libraries (which potentially brings with it other issues)

I realise this is a side-project and you are busy, but just looking for some confidence that a fix will be forthcoming if we chose Option1 even if its not immediate.

@ThangHuuVu @ndom91 - Sorry to tag you but would you be able to answer the above?

[balazsorban44]

Hi, I'm sorry, I'm currently taking a small break from the project to work through personal issues. Thanks for tagging the others.

I'll look into all PRs once I feel better. 💚🙏

[james-bjss]

Hi, I'm sorry, I'm currently taking a small break from the project to work through personal issues. Thanks for tagging the others.

I'll look into all PRs once I feel better. 💚🙏

No problem. Thanks for letting us know. Take care of yourself and hope you feel better soon!

[ndom91]

Hi james, so from my side this looks like it makes a lot of sense and the PR looks clean as well. I dont want to speak for Balazs and we're waiting to release any versions on him anyway, but for the time being you could use something like patch-package to get your own copy up and running with this patch in it.

[ndom91]

Thanks James! I'll mark this as "ready" so we can take a look as soon as we continue with cutting releases :+1:

[kd-tony]

@ndom91 @james-bjss We're also facing the same issues with this missing nouce check type. Any updates on this PR and when it will be a candidate for a release ?

[james-bjss]

@ndom91 @james-bjss We're also facing the same issues with this missing nouce check type. Any updates on this PR and when it will be a candidate for a release ?

Hi Sorry, I don't have any news I am also keen to see this merged too. We are currently using the code form this PR with patch-package method suggested above.

[w0otness]

can you share the patch-package you use?

[ndom91]

can you share the patch-package you use?

Probably this one: https://www.npmjs.com/package/patch-package

[lpanjwani]

I have created a patch package on NPM until this PR has been merged.

https://www.npmjs.com/package/next-auth-patch-feature-nonce-check

[w0otness]

I currently use this patches/openid-client+5.1.6.patch which disables the nonce check and bypasses the nonce error, and the sign in process is seamless.

I'll try replacing the entire package with the patched one (though a gist containing the .patch would be preferable) to see if I get the extra protection of the nonce.

[hamidbjss]

I currently use this patches/openid-client+5.1.6.patch which disables the nonce check and bypasses the nonce error, and the sign in process is seamless.

I'll try replacing the entire package with the patched one (though a gist containing the .patch would be preferable) to see if I get the extra protection of the nonce.

@w0otness I created a gist of the patch we use: https://gist.github.com/hamidbjss/b6408e48080f247edd22ec04a3b983e3

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated
next-auth	⬜️ Ignored (Inspect)		Aug 16, 2022 at 8:20AM (UTC)

[iamursky]

Any updates?

[hamidbjss]

@ThangHuuVu @balazsorban44 @ndom91 Hey, it's been some time since we last heard anything regarding this PR, wondering if you can provide an update on this please? :)

[james-bjss]

So I think this should be ready to be merged, thanks again @james-bjss! @balazsorban44 @ndom91 what do you think?

Thanks. Credit to @hamidbjss also, as we paired on this fix.

[alaamub]

@ThangHuuVu Thanks for the fix! how long does it usually take before releasing a new version of the library ?

[mikejackowski]

any chance on some update on when will this be live?

[iamursky]

any chance on some update on when will this be live?

Probably not, @ndom91 is in /dev/null 😁

[hamidbjss]

Created a patch for version 4.10.3 if it's of help to anyone next-auth+4.10.3.patch.

@balazsorban44 @ndom91 @ThangHuuVu @lluia really would be good if we could get this merged in please, it's a pain having to constantly create new patches.

[revmischa]

This error is blocking me from using a third-party IDP with Cognito nonce mismatch, expected undefined

[james-bjss]

This error is blocking me from using a third-party IDP with Cognito nonce mismatch, expected undefined

You can use the patch provided by @hamidbjss above until this is merged.

This PR seems to be in limbo though. Posts above do suggest that it's being considered as a candidate for a new release but it doesn't look like there is a roadmap for future releases and the only changes that are being merged currently are critical fixes and doc changes.

I understand the pressures of maintaining a popular OS project and am immensely grateful for this great library, but it's quite dissappointing when asked to raise a PR for a fix and it just languishes for 6months. We chose to stick with nextauth as we thought this would likely be fixed in the future.

@ndom91 would it be possible to get an update on why it has stalled? It's clear it is affecting a number of people who have chosen to use this lib (have seen this pop up on stack overflow too). Even if we could get a beta/branch build of the fix it would be nice. Or if it's concerns over the change, issues testing or that next release is not yet scheduled till x date etc.. Just some response to keep us updated.

Sorry for the nagging!

Also, for what its worth we have been using the patched version for many months now without issue.

[james-bjss]

Folks, we understand your frustration, and are sorry for the delayed action.

@james-bjss I just tested this PR locally with Google using nonce with and without state and pkce, everything works as expected. 🎉 I also added some comments, could you check and let me know what you think? Thanks again for the PR! Let's work together to get this one merged soon 🙌

Great. Thanks! Just going ahead and making these changes and we will test with Cognito + IDP (Azure AD) and will update on status.

[james-bjss]

Folks, we understand your frustration, and are sorry for the delayed action. @james-bjss I just tested this PR locally with Google using nonce with and without state and pkce, everything works as expected. 🎉 I also added some comments, could you check and let me know what you think? Thanks again for the PR! Let's work together to get this one merged soon 🙌

Great. Thanks! Just going ahead and making these changes and we will test with Cognito + IDP (Azure AD) and will update on status.

@ThangHuuVu - @hamidbjss and I have tested this against our Cognito setup and it works as-expected with those changes.

[james-bjss]

Thanks @ThangHuuVu and all. Sorry for all the nagging! This will be a huge help to our team. :star2: Have donated you all some beer money :)

[mocon]

Hey team, thanks for all your hard work on this! Will these changes be released in a new version?

[kesoji]

I'm all excited about it to be released 😊