fix(providers): Handle Azure AD tenants correctly

[JibbityJobbity]

☕️ Reasoning

Endpoints returned by Azure AD want us to edit the path so that each request gets routed to their proper tenant IDs. The old implementation didn't handle this properly when using the "common" tenant ID, which basically lets us use it normally like any other provider. We would receive the template endpoint paths and use those in error checking and stuff rather than the ones we actually use.

I've just thrown something up and I've done some quick testing. It seems to work normally. I'll test custom tenants at some point and I suspect it won't work until I test and fix it.

🧢 Checklist

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

🎫 Affected issues

Fixes: #9635

📌 Resources

• Security guidelines
• Contributing guidelines
• Code of conduct
• Contributing to Open Source

[vercel[bot]]

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

Name	Status	Preview	Comments	Updated (UTC)
auth-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 9, 2024 5:43pm

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
next-auth-docs	⬜️ Ignored (Inspect)	Visit Preview		Jun 9, 2024 5:43pm

[JibbityJobbity]

Testing needed

• Common tenant group
• organizations and consumers tenant groups
• Any other specific tenant ID
• Other providers still work

[panva]

I think there should be two distinct providers

One where you know the tenant you're working with and you're NOT using any of the "common" issuers (the existing one with the exception that tenant id is required).

And then another one which can be used with one of the "common" issuers

  'https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration'
  'https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration'
  'https://login.microsoftonline.com/consumers/v2.0/.well-known/openid-configuration'

This one has the workarounds in and requires explicit configuration to

• allow login from all tenants
• allow login from specific tenants (by providing a list of tenant ids), the ID Token's tid would be checked to match the expected tenants.

[JibbityJobbity]

While we all know the real solution to this, I think by the time I have a crack at this, we will know what ends up happening. But that's definitely considerable if conform() doesn't end up happening. However in the meantime, I think the conform() method is the first way forward with this to try. It provides a way for us to sort out LinkedIn too, but I'd be careful about getting too comfortable with hacking around providers as I can imagine conform() would make it easier to do exactly that.

[JibbityJobbity]

I've done what I can for now. Completely open to suggestions and feedback.

I know this doesn't quite match the sample, but I wanted the check to work the other way around. Rather than having it try to check against the "fake" URL with {tenantid}, I thought it would be cleaner to perform a real check.

[vercel[bot]]

@JibbityJobbity is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

[panva]

I'm not finding all the individual bits from my original recommendation nor the recommended split from https://github.com/nextauthjs/next-auth/pull/9718#issuecomment-1909893145 so I'm assuming you're going with your own workarounds.

[JibbityJobbity]

I'm not finding all the individual bits from my original recommendation nor the recommended split from #9718 (comment) so I'm assuming you're going with your own workarounds.

It's a mix between what you suggested in the other issue and the conform methods. It still achieves the same thing, which is satisfying those underlying checks in your library while bringing the conformity to the provider level, as suggested by @balazsorban44. If this isn't good enough then I'll change the PR to satisfy the solution you gave earlier. The special serverConform is a bit icky so I see where you're coming from.

Anyway, I just did some testing with the other grouped tenants. The issuer URL comes back with some weird tenant ID instead of getting the user to replace "{tenantid}" or some other value we can expect. Replacing "{tenantid}" only seems to work with "common" and doesn't work with "organizations" or "consumers". This keeps rearing its ugly head, you almost need something to the effect of this to override the check to bypass it:

    const discoveryResponse: Response | undefined = await o.discoveryRequest(issuer)
    let json = await discoveryResponse.json();
    const as = await o.processDiscoveryResponse(new URL(json.issuer), discoveryResponse!)

I'll need another go at this. I'll shoot for tomorrow.

[JibbityJobbity]

This seems to work nicely :) I'm largely happy with the current solution now.

[JibbityJobbity]

Bump, rebased.

[amplicity]

hey! great work here, we could really use this. would love to see this get merged soon.

[MrLoh]

It would be great to see this get merged. I'm sure it blocks a lot of people from adopting v5.

[jaysin586]

Is there an update on this?

[ladparth]

Hello, Thank you for working on this PR. I am also stuck on this stage of my development. Is there any update on this fix or is there any temporary fix i can do while this PR is in process?

[JibbityJobbity]

Hello, Thank you for working on this PR. I am also stuck on this stage of my development. Is there any update on this fix or is there any temporary fix i can do while this PR is in process?

I'll rebase, but I haven't heard anything from any of the maintainers since I got it working. Do keep in mind they may be busy with their lives, but there are some other hacks to make it work by configuring the provider itself. This PR would just mean that nobody would have to be concerned about this again.

[ladparth]

Thank you @JibbityJobbity

[murilobd]

Thanks @JibbityJobbity for that PR! Hopefully maintainers of this repo will merge it!

[Ghernouz]

Any updates guys?

[RonB]

It looks like there is some status quo here. Azure AD also has been rebranded to Microsoft Entra Id and that has been adressed in the code already. I am afraid the train is moving on and a solution is getting out of site. @JibbityJobbity, thank you for your hard work. A request on behalf of all of us if you could adress the remaining open ends here?

[MrLoh]

Entra ID has the exact same issue that this PR tries to fix.

[JibbityJobbity]

I've rebased and put the fix onto Entra ID. I haven't changed any logic. Seeing as people have shown interest in getting this fixed, I'd like to kindly ask the community to try testing this out for me. I'm going to have to pull the "life stuff" excuse for not getting this done sooner or testing it. Hopefully this gets merged once some people say it works.

[RonB]

@JibbityJobbity Thank you so much for rebasing, I will advocate for this change to be merged but I am having a little trouble getting it to work. I have a mono-repo with 2 apps and use pnpm. How should i use your repo? In my package.json something like:

"dependencies":
    "@auth/core": "https://github.com/JibbityJobbity/next-auth.git",
    "@auth/sveltekit": "^1.1.0"
"pnpm": {
    "overrides": {
      "@auth/core": "https://github.com/JibbityJobbity/next-auth.git"
    }
  },

It errors with:

Error: Failed to resolve entry for package "@auth/core". The package may have incorrect main/module/exports specified in its package.json.

Does the package has to be build first and do i need to make a fork of your repo and do that myself or is there a simpler solution use your repo? I am not an expert on this depency management stuff so i hope you can point me in the right direction.

And do i need to specify a tenant for the EntraId config like:

	MicrosoftEntraIDProvider({
		clientId: MICROSOFT_ENTRA_ID_CLIENT_ID,
		clientSecret: MICROSOFT_ENTRA_ID_CLIENT_SECRET,
		authorization: 'https://login.microsoftonline.com/common/oauth2/v2.0/authorize',
		token: 'https://login.microsoftonline.com/common/oauth2/v2.0/token',
		tenantId: 'common'
	})

Excuse my ignorance :-) Thank you for your response

[michal-boruczkowski]

@RonB I was trying to use it like this:

dependencies: {
     ...
    "next-auth": "git://github.com/JibbityJobbity/next-auth.git#azure-ad-common-tenant",
}

but I'm getting following error when npm i:

npm error git dep preparation failed
npm error npm warn using --force Recommended protections disabled.
npm error npm error code EUNSUPPORTEDPROTOCOL
npm error npm error Unsupported URL Type "workspace:": workspace:*

pnpm seems to handle this but actual fix is not applied in runtime

I'm using patch prepared here: https://github.com/nextauthjs/next-auth/issues/8374#issuecomment-1803387771 ATM

Would be great if this fix is merged 🤞

[JibbityJobbity]

Yeah, the way I got around it for development was to hack stuff in my node_modules folder. Not entirely sure how to deploy it in my own web app actually :/ if I use the sveltekit framework package in my branch then it tries to pull the core package from npm instead of my branch. Everyone I ask about this says "idk publish the package on npm". Sorry I don't have any useful info.

[hiromijorge]

@RonB I was trying to use it like this:

dependencies: {
     ...
    "next-auth": "git://github.com/JibbityJobbity/next-auth.git#azure-ad-common-tenant",
}

but I'm getting following error when npm i:

npm error git dep preparation failed
npm error npm warn using --force Recommended protections disabled.
npm error npm error code EUNSUPPORTEDPROTOCOL
npm error npm error Unsupported URL Type "workspace:": workspace:*

pnpm seems to handle this but actual fix is not applied in runtime

I'm using patch prepared here: #8374 (comment) ATM

Would be great if this fix is merged 🤞

Hi Michael,

I've followed the instructions in this link for applying the patch.

However, I'm still encountering the same errors as before. Could you please explain how you did it?

Errors [auth][error] CallbackRouteError: Read more at https://errors.authjs.dev#callbackrouteerror [auth][cause]: OperationProcessingError: unexpected JWT "iss" (issuer) claim value