AzureAD common tenant URLs aren't being checked properly

[JibbityJobbity]

Provider type

Azure Active Directory

Environment

System:
OS: macOS 14.2.1
CPU: (12) arm64 Apple M2 Max
Memory: 65.83 MB / 32.00 GB
Shell: 5.9 - /bin/zsh
Binaries:
Node: 21.2.0 - ~/.nvm/versions/node/v21.2.0/bin/node
npm: 10.2.3 - ~/.nvm/versions/node/v21.2.0/bin/npm
pnpm: 8.12.0 - ~/Library/pnpm/pnpm
bun: 1.0.14 - ~/.bun/bin/bun
Browsers:
Brave Browser: 120.1.61.109
Safari: 17.2.1
npmPackages:
@auth/core: ^0.18.6 => 0.18.6
@auth/sveltekit: ^0.4.0 => 0.4.0

Reproduction URL

https://github.com/JibbityJobbity/next-auth-example/

Describe the issue

Azure's implementation of OAuth requires manipulating/replacing parts of the URL based on your tenant ID which you are using. Next-auth's codebase frequently has the returned authentication/other endpoints checked and raises an error otherwise. This happens on any app with the common tenant ID, meaning just a generic Microsoft login like you would expect with just about any other OAuth provider. But you can have a look at how I'm using it in my project.

There are two checks where I've noticed that it's an issue.

The location the first check fails is in getAuthorizationUrl in @auth/core/lib/actions/signin/authorization-url.js, where we have this check o.processDiscoveryResponse expect the response url to be https://login.microsoftonline.com/common/v2.0 when what we get back in actuality is https://login.microsoftonline.com/{tenantid}/v2.0. Microsoft expects users to replace {tenantid} with common, or whatever other tenant ID that's supplied when the AzureAD provider is set up.
For this, @panva has kindly provided a code snippet:

import * as oauth from 'oauth4webapi'
import * as jose from 'jose'

const as = await oauth
  .discoveryRequest(new URL('https://login.microsoftonline.com/common/v2.0'))
  .then((response) =>
    oauth.processDiscoveryResponse(
      new URL('https://login.microsoftonline.com/{tenantid}/v2.0'),
      response,
    ),
  )

const response = oauth.authorizationCodeGrantRequest(as, client, params, redirectUri, codeVerifier)

let tenantId
if (response.status === 200) {
  const { id_token } = await response.clone().json()
  ;({ tid: tenantId } = jose.decodeJwt(id_token))
}

if (!isThisTenantIdExpected(tenantId)) {
  throw new Error()
}

const result = await oauth.processAuthorizationCodeOpenIDResponse(
  {
    ...as,
    issuer: as.issuer.replace('{tenantid}', tenantId),
  },
  client,
  response,
)

The second one is in handleOauth in where o.processAuthorizationCodeOpenIDResponse gets called. The way it eventually makes its way down will get the user's specific "issuer" tenant in the returned url. We tell the check to expect 'https://login.microsoftonline.com/common/v2.0' when the returned URL in the oauth4web library will get the same URL from Microsoft, but have common replaced with some tenant ID, I assume it's the user's.

I'd make a PR but I have no clue how to fit a solution in that's compatible with the project's architecture.

How to reproduce

Use the AzureAD provider with the "common" tenant ID and log in.

Expected behavior

Login to work with Microsoft's special endpoints with the tenant IDs that it expects. The checks drill themselves all the way down to oauth4web, but we kinda tell it to check for the wrong thing.

[JibbityJobbity]

Clodged something together, see PR

[balazsorban44]

Thanks for the PR! Left a comment on it. Since this is an upstream non-compliance issue, I feel this is not a bug on us, but something we now unfortunately have to find a workaround for.

[MrLoh]

It doesn't matter where this bug originates; previous versions of next-auth were offering this feature, and the whole point of this library is so that developers don't have to deal with exactly these kinds of issues. So if the maintainers see this as out of scope, it seems like all of us who have a requirement to use AzureAD provider with common tenant will have to find an alternative solution.

Is there a way to help ensure that this PR makes it in soon?

[JibbityJobbity]

Is there a way to help ensure that this PR makes it in soon?

Review, feedback or consolidating solutions around the other issues/PRs is appreciated. I feel somewhat out of my comfort zone here :)

That being said, maybe the issue needs to stay open until Microsoft gets their stuff sorted? Then we can get rid of this hack. I've seen LinkedIn having a similar issue and they seem to be in contact with LinkedIn, but AFAIK nobody has bothered Microsoft about this.

[panva]

That being said, maybe the issue needs to stay open until Microsoft gets their stuff sorted?

They will most certainly not ;)

[Cyclodex]

JFYI: I guess I have the same problem, but with the azure-ad-b2c provider. :(

[MrLoh]

@balazsorban44 Why isn't the fix for this being merged? Is this library still being maintained, the fix PR has been up for over 2 months. This is a pretty big whole in v5 and stopping me from upgrading. I know there are workarounds, but if fixes like this don't get attention, I am loosing confidence in this library.

[balazsorban44]

Feel free to lose confidence or find an alternative, no hard feelings. 🙏💚 The project is definitely being maintained, but this is my (our) side project and you or your company isn't paying me/us to prioritize this issue. OSS !== free work.

FWIW, Azure AD is being replaced by Microsoft Entra ID, which we recently added support for. Will be out in the new release.

Also, this is technically an issue at Microsoft, not a bug in Auth.js. I understand the frustration though, just saying that the critique is knocking on the wrong door in the first place.

[MrLoh]

I do appreciate that this is open source and there is no expectation to get free work. It's just confusing when PRs with fixes for a big regression in the new version don't get any attention, since that's the only way to engage with open source projects. Yes the issue has to do with Microsoft's none standard approach, but who cares, the point of this library is to abstract exactly those details away and that is what it did in the previous version, so this issue blocks many from upgrading.

The PR #9718 tried to address the concern you mentioned in your review 2 months ago and it looks pretty minimally invasive, so it's unclear why it can't be merged. Maybe there's another issue with it and some of us could help address it, but we need a maintainer to give another review, otherwise there's no way to contribute to this open source project.

It would be very much appreciated if someone could take a look and see if the PR is good to go or needs further changes.

[Sitzdev]

Just checked with the new Entra Id provider.
The issue seems to be the same there.

[Sitzdev]

@balazsorban44 what would you suggest as a solution to this problem? I see a couple of possibilities:

1. Merge the PR as it is not really invasive
2. Create an issue at microsofts end and hope they fix it (which they propably won`t)
3. Create a PR for an extra provider which implements the suggested changes without touching the original one

I really love the library and the comfort it gives and this is the only hurdle that I`ve encountered (Yes I know this is not an error on your end but rather on Microsofts end).

[jon-snowman]

@balazsorban44 Entra ID is not a replacement for AD, it's just a rename. Don't take my word for it though, this is in your own documentation (https://authjs.dev/getting-started/providers/microsoft-entra-id) :)

The project is definitely being maintained, but this is my (our) side project and you or your company isn't paying me/us to prioritize this issue. OSS !== free work.

But you/your company is also not paying the community to debug/fix issues, right? So are you saying nobody should be contributing to authjs, because OSS !== free work?