fix(providers): Handle Azure AD tenants correctly #9718

JibbityJobbity · 2024-01-23T07:57:53Z

☕️ 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

Documentation
Tests
Ready to be merged

🎫 Affected issues

Fixes: #9635

📌 Resources

vercel · 2024-01-23T07:57:57Z

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 · 2024-01-23T08:22:34Z

Testing needed

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

balazsorban44

Any non-compliance needs to be handled at the provider level. we don't want branching based on specific providers, it's not maintainable in the long term

This will need a different solution. Something similar to the conform method on token.

some useful info at the end of the thread here #8831

panva · 2024-01-25T10:43:54Z

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 · 2024-01-25T21:40:45Z

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.

packages/core/src/providers/azure-ad.ts

packages/core/src/lib/actions/callback/oauth/callback.ts

JibbityJobbity · 2024-01-27T11:27:42Z

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 · 2024-02-03T01:30:55Z

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

A member of the Team first needs to authorize it.

JibbityJobbity · 2024-02-03T01:35:59Z

packages/core/src/providers/azure-ad.ts

+
+        return {
+          ...authServer,
+          issuer: authServer.issuer.replace("common", tenantId),


This isn't right. Need to detect the tenant that's being used. Probably just replace the provider's tenant ID with whatever we expect from the JWT.

panva · 2024-02-03T10:30:48Z

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.

JibbityJobbity · 2024-02-03T11:49:02Z

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 · 2024-02-04T23:47:46Z

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

JibbityJobbity · 2024-03-02T23:44:30Z

Bump, rebased.

amplicity · 2024-03-07T20:16:52Z

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

MrLoh · 2024-03-12T02:50:06Z

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

Original PR on next-auth: nextauthjs#9718 Patched waiting for this to be released upstream

jaysin586 · 2024-04-07T18:34:30Z

Is there an update on this?

ladparth · 2024-04-08T15:05:38Z

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 · 2024-04-08T15:12:10Z

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 · 2024-04-08T15:35:15Z

Thank you @JibbityJobbity

murilobd · 2024-05-09T20:37:38Z

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

Ghernouz · 2024-06-02T22:14:52Z

Any updates guys?

RonB · 2024-06-03T06:58:13Z

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 · 2024-06-03T14:37:15Z

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

The way Microsoft integrates their tenant system will change the underlying endpoints underneath the oauth/openid ones it provides. This works around the non-conformity and makes it functional.

JibbityJobbity · 2024-06-09T17:44:36Z

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 · 2024-06-10T17:29:15Z

@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 · 2024-07-09T14:01:58Z

@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 🤞

JibbityJobbity · 2024-07-09T18:44:41Z

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 · 2024-07-22T07:43:56Z

@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

JibbityJobbity requested a review from ThangHuuVu as a code owner January 23, 2024 07:57

github-actions bot added the core Refers to `@auth/core` label Jan 23, 2024

JibbityJobbity changed the title ~~fix: Handle Azure AD tenants correctly~~ fix(providers): Handle Azure AD tenants correctly Jan 23, 2024

vercel bot had a problem deploying to Preview – auth-docs January 23, 2024 07:59 Failure

balazsorban44 requested changes Jan 25, 2024

View reviewed changes

balazsorban44 mentioned this pull request Jan 25, 2024

AzureADProvider does not work with default (common) endpoint #8374

Closed

panva mentioned this pull request Jan 25, 2024

refactor(providers): rebranded Azure AD to Microsoft Entra #9664

Closed

3 tasks

JibbityJobbity force-pushed the azure-ad-common-tenant branch from 59df0ac to 3626664 Compare January 27, 2024 09:18

github-actions bot added the providers label Jan 27, 2024

vercel bot deployed to Preview – auth-docs January 27, 2024 09:23 View deployment

panva reviewed Jan 27, 2024

View reviewed changes

packages/core/src/providers/azure-ad.ts Outdated Show resolved Hide resolved

panva reviewed Jan 27, 2024

View reviewed changes

packages/core/src/providers/azure-ad.ts Outdated Show resolved Hide resolved

JibbityJobbity force-pushed the azure-ad-common-tenant branch 2 times, most recently from e4ffecd to cfaf8da Compare January 27, 2024 11:21

JibbityJobbity commented Jan 27, 2024

View reviewed changes

packages/core/src/lib/actions/callback/oauth/callback.ts Show resolved Hide resolved

vercel bot deployed to Preview – auth-docs January 27, 2024 11:25 View deployment

JibbityJobbity force-pushed the azure-ad-common-tenant branch 2 times, most recently from b8e41d1 to db8b784 Compare February 3, 2024 01:34

JibbityJobbity commented Feb 3, 2024

View reviewed changes

vercel bot deployed to Preview – auth-docs February 3, 2024 01:39 View deployment

vercel bot deployed to Preview – auth-docs February 4, 2024 23:44 View deployment

JibbityJobbity force-pushed the azure-ad-common-tenant branch from b488878 to 43a1482 Compare February 4, 2024 23:45

vercel bot deployed to Preview – auth-docs February 4, 2024 23:48 View deployment

JibbityJobbity requested a review from balazsorban44 February 5, 2024 00:13

JibbityJobbity force-pushed the azure-ad-common-tenant branch from 360734d to ab0a2a5 Compare March 2, 2024 23:43

vercel bot deployed to Preview – auth-docs March 2, 2024 23:47 View deployment

jo-sm added a commit to clearyai/next-auth that referenced this pull request Mar 13, 2024

Fixes login for Azure users

13944a6

Original PR on next-auth: nextauthjs#9718 Patched waiting for this to be released upstream

MrLoh mentioned this pull request Apr 19, 2024

AzureAD common tenant URLs aren't being checked properly #9635

Closed

fix(providers): Handle Azure AD tenants correctly

f0ee97e

The way Microsoft integrates their tenant system will change the underlying endpoints underneath the oauth/openid ones it provides. This works around the non-conformity and makes it functional.

JibbityJobbity force-pushed the azure-ad-common-tenant branch from ab0a2a5 to f0ee97e Compare June 9, 2024 17:39

vercel bot deployed to Preview – auth-docs June 9, 2024 17:43 View deployment

balazsorban44 mentioned this pull request Aug 8, 2024

Correct an error in the Entra docs regarding tenant configuration #11519

Closed

3 tasks

This was referenced Oct 5, 2024

feat(oauth): add conformResponse option for non-conform providers #11978

Closed

fix(providers): conform Entra ID when responding with the wrong issuer #11980

Merged

balazsorban44 closed this in #11980 Oct 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(providers): Handle Azure AD tenants correctly #9718

fix(providers): Handle Azure AD tenants correctly #9718

JibbityJobbity commented Jan 23, 2024 •

edited

Loading

vercel bot commented Jan 23, 2024 •

edited

Loading

JibbityJobbity commented Jan 23, 2024

balazsorban44 left a comment •

edited

Loading

panva commented Jan 25, 2024 •

edited

Loading

JibbityJobbity commented Jan 25, 2024

JibbityJobbity commented Jan 27, 2024

vercel bot commented Feb 3, 2024

JibbityJobbity Feb 3, 2024

panva commented Feb 3, 2024

JibbityJobbity commented Feb 3, 2024 •

edited

Loading

JibbityJobbity commented Feb 4, 2024

JibbityJobbity commented Mar 2, 2024

amplicity commented Mar 7, 2024

MrLoh commented Mar 12, 2024

jaysin586 commented Apr 7, 2024

ladparth commented Apr 8, 2024 •

edited

Loading

JibbityJobbity commented Apr 8, 2024

ladparth commented Apr 8, 2024

murilobd commented May 9, 2024

Ghernouz commented Jun 2, 2024

RonB commented Jun 3, 2024

MrLoh commented Jun 3, 2024

JibbityJobbity commented Jun 9, 2024

RonB commented Jun 10, 2024 •

edited

Loading

michal-boruczkowski commented Jul 9, 2024 •

edited

Loading

JibbityJobbity commented Jul 9, 2024

hiromijorge commented Jul 22, 2024 •

edited

Loading

fix(providers): Handle Azure AD tenants correctly #9718

fix(providers): Handle Azure AD tenants correctly #9718

Conversation

JibbityJobbity commented Jan 23, 2024 • edited Loading

☕️ Reasoning

🧢 Checklist

🎫 Affected issues

📌 Resources

vercel bot commented Jan 23, 2024 • edited Loading

JibbityJobbity commented Jan 23, 2024

balazsorban44 left a comment • edited Loading

Choose a reason for hiding this comment

panva commented Jan 25, 2024 • edited Loading

JibbityJobbity commented Jan 25, 2024

JibbityJobbity commented Jan 27, 2024

vercel bot commented Feb 3, 2024

JibbityJobbity Feb 3, 2024

Choose a reason for hiding this comment

panva commented Feb 3, 2024

JibbityJobbity commented Feb 3, 2024 • edited Loading

JibbityJobbity commented Feb 4, 2024

JibbityJobbity commented Mar 2, 2024

amplicity commented Mar 7, 2024

MrLoh commented Mar 12, 2024

jaysin586 commented Apr 7, 2024

ladparth commented Apr 8, 2024 • edited Loading

JibbityJobbity commented Apr 8, 2024

ladparth commented Apr 8, 2024

murilobd commented May 9, 2024

Ghernouz commented Jun 2, 2024

RonB commented Jun 3, 2024

MrLoh commented Jun 3, 2024

JibbityJobbity commented Jun 9, 2024

RonB commented Jun 10, 2024 • edited Loading

michal-boruczkowski commented Jul 9, 2024 • edited Loading

JibbityJobbity commented Jul 9, 2024

hiromijorge commented Jul 22, 2024 • edited Loading

JibbityJobbity commented Jan 23, 2024 •

edited

Loading

vercel bot commented Jan 23, 2024 •

edited

Loading

balazsorban44 left a comment •

edited

Loading

panva commented Jan 25, 2024 •

edited

Loading

JibbityJobbity commented Feb 3, 2024 •

edited

Loading

ladparth commented Apr 8, 2024 •

edited

Loading

RonB commented Jun 10, 2024 •

edited

Loading

michal-boruczkowski commented Jul 9, 2024 •

edited

Loading

hiromijorge commented Jul 22, 2024 •

edited

Loading