nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
23.78k stars 3.27k forks source link

AZURE_AD_TENANT_ID=common throw body "issuer" does not match "expectedIssuer" #6138

Closed xmlking closed 1 year ago

xmlking commented 1 year ago

Provider type

Azure Active Directory

Environment

System: OS: macOS 13.2 CPU: (10) arm64 Apple M1 Max Memory: 38.19 GB / 64.00 GB Shell: 5.8.1 - /bin/zsh Binaries: Node: 19.2.0 - /opt/homebrew/bin/node npm: 8.19.3 - /opt/homebrew/bin/npm Browsers: Chrome: 108.0.5359.124 Safari: 16.2

Reproduction URL

N/A

Describe the issue

Unfortunately when TENANT_ID=common Azure well-known URL https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration will contain non-standard issuer: "issuer":"https://login.microsoftonline.com/{tenantid}/v2.0"

This issue has been discussed here: https://github.com/MicrosoftDocs/azure-docs/issues/38427

Due to issuer missmatch, authjs will throw following error:

This validation is happening here: https://github.com/panva/oauth4webapi/blob/main/src/index.ts#L773

image

How to reproduce

  1. In the Azure portal: set signInAudience=AzureADandPersonalMicrosoftAccount for your registered app
  2. Setup Azure AD Provider
    export const authjs = SvelteKitAuth({
    providers: [
        AzureAD({
            clientId: AZURE_AD_CLIENT_ID,
            clientSecret: AZURE_AD_CLIENT_SECRET,
            tenantId: AZURE_AD_TENANT_ID
        }),
    ]
    });
  3. set AZURE_AD_TENANT_ID=common in .env or remove tenantId: AZURE_AD_TENANT_ID above which will fallback to tenantId=common

Expected behavior

I am asking the AuthJS team to provide overwrite option to overwrite the issuer in pulled well-known/openid-configuration content. Or, provide config option to bypass this check for AzureAD

balazsorban44 commented 1 year ago

We are not going to provide more escape hatches for non-compliant providers, unfortunately.

https://github.com/MicrosoftDocs/azure-docs/issues/38427#issuecomment-555855086

looks like they are fully aware and decided not to fix. šŸ¤· Putting pressure on them is my best advice. If we want the standards to be taken seriously, we need to go to the source.

The discovery request is optional though, but you must manually configure the token and userinfo endpoints

https://github.com/nextauthjs/next-auth/blob/2ba5314e351fe778cfc4d61d915e0597b386f34a/packages/core/src/lib/oauth/callback.ts#L41

xmlking commented 1 year ago

@balazsorban44 thank you for guiding me with workaround. I tried the following options to bypass the discovery request (providing the token and userinfo) but I am still getting errors. please advise if the following config is correct. wonder why they are shown as undefined in the logs (I added console.log in nextauthjs's authorization-url.js)

import {
    AZURE_AD_CLIENT_ID,
    AZURE_AD_CLIENT_SECRET,
    AZURE_AD_TENANT_ID,
    GITHUB_ID,
    GITHUB_SECRET,
    GOOGLE_ID,
    GOOGLE_SECRET
} from '$env/static/private';
import AzureAD from '@auth/core/providers/azure-ad';
import GitHub from '@auth/core/providers/github';
import Google from '@auth/core/providers/google';
import SvelteKitAuth from '@auth/sveltekit';
import type { Handle } from '@sveltejs/kit';

export const authjs = SvelteKitAuth({
    providers: [
        Google({
            clientId: GOOGLE_ID,
            clientSecret: GOOGLE_SECRET,
            authorization: { params: { prompt: 'consent' } }
        }),
        AzureAD({
            clientId: AZURE_AD_CLIENT_ID,
            clientSecret: AZURE_AD_CLIENT_SECRET,
            tenantId: AZURE_AD_TENANT_ID,
            // HINT: https://github.com/nextauthjs/next-auth/issues/6138
            token: `https://login.microsoftonline.com/${AZURE_AD_TENANT_ID}/oauth2/v2.0/token`,
            userinfo: 'https://graph.microsoft.com/oidc/userinfo',
            authorization: {
                url: `https://login.microsoftonline.com/${AZURE_AD_TENANT_ID}/oauth2/v2.0/authorize`,
                params: { scope: 'openid profile user.Read email' }
            }
        }),
        GitHub({ clientId: GITHUB_ID, clientSecret: GITHUB_SECRET })
    ],
    callbacks: {
        async jwt({ token, account, profile }) {
            if (account) {
                token.token = account.access_token; // account.id_token
            }
            if (profile) {
                token.email ??= profile.upn;
                token.roles ??= profile.roles;
            }
            // FIXME: for Azure AD picture is base64 and it is too big to fit in cookie.
            // will through `431 Request Header Fields Too Large` unless we remove it.
            if (token.picture?.startsWith('data:')) delete token.picture;
            return token;
        },
        async session({ session, token }) {
            if (token?.token) {
                session.token = token.token;
            }
            if (token?.roles) {
                session.roles = token.roles;
            }
            return session;
        }
    }
}) satisfies Handle;
Screenshot 2022-12-21 at 7 26 50 PM
xmlking commented 1 year ago

Hi @balazsorban44 should I reopen this issue?

amplicity commented 11 months ago

@xmlking did you ever resolve this? I have the same issue.

moverval commented 10 months ago

@xmlking like you can see in the source code, token and userinfo are objects. Here is a small example on how to provide the options:

AzureProvider({
    clientId: AZURE_CLIENT_ID,
    clientSecret: AZURE_CLIENT_SECRET,
    tenantId: "common",
    token: {
        url: `https://login.microsoftonline.com/common/oauth2/v2.0/token`,
    },
    userinfo: {
        url: "https://graph.microsoft.com/oidc/userinfo",
    },
    authorization: {
        url: `https://login.microsoftonline.com/common/oauth2/v2.0/authorize`,
        params: {
            scope: "openid profile email User.Read"
        }
    },
    async profile(profile, tokens) {
      const response = await fetch(
        `https://graph.microsoft.com/v1.0/me`,
        { headers: { Authorization: `Bearer ${tokens.access_token}` } }
      )

      const json = await response.json()

      return {
        id: profile.sub,
        name: profile.name,
        email: json.userPrincipalName,
      }
    },
})
thomasmost commented 1 week ago

Returning to this issue briefly... if I want to enable SSO for multiple orgs (multi-tenant, not single-tenant deployment), should I set the ENTRA_ID_TENANT_ID to "common" or to the tenant ID of the application anyway? It's confusing in the latest docs, where it seems to specify that the application's tenant ID should be provided, but that the issuer will default to "common" anyway... Could this be clarified?

image

For context, when I set the tenant_id to "common" using the new Entra provider, I get the issuer request error from above

thomasmost commented 1 week ago

Ah, no, I see this does not work

image
jinsley8 commented 1 week ago

Returning to this issue briefly... if I want to enable SSO for multiple orgs (multi-tenant, not single-tenant deployment), should I set the ENTRA_ID_TENANT_ID to "common" or to the tenant ID of the application anyway? It's confusing in the latest docs, where it seems to specify that the application's tenant ID should be provided, but that the issuer will default to "common" anyway... Could this be clarified?

Try using consumers. Using the tenant ID will only work for single tenant. See endpoint details here.

Common doesn't actually work at all right now unless you patch oauth4webapi.

jinsley8 commented 1 week ago

According to the Microsoft Entra docs regarding endpoint issuer value these values should work depending on the account type of the registered app:

Supported account types:

  1. Accounts in this organizational directory only (Current Account only - Single tenant) - AUTH_MICROSOFT_ENTRA_ID_TENANT_ID={tenant id}
  2. Accounts in any organizational directory (Any Microsoft Entra ID tenant - Multitenant) - AUTH_MICROSOFT_ENTRA_ID_TENANT_ID=organizations
  3. Accounts in any organizational directory (Any Microsoft Entra ID tenant - Multitenant) and personal Microsoft accounts (e.g. Skype, Xbox) - AUTH_MICROSOFT_ENTRA_ID_TENANT_ID=common
  4. Personal Microsoft accounts only - AUTH_MICROSOFT_ENTRA_ID_TENANT_ID=consumers

However, if not using the tenant ID then you may need to patch oauth4webapi to use any of the other values.