From 82ba14aca2cb7b169b34c33638dac708a9a621e5 Mon Sep 17 00:00:00 2001 From: Guust Goossens Date: Thu, 28 May 2026 15:46:20 +0300 Subject: [PATCH 1/2] Re-run OIDC discovery after token exchange for Microsoft Entra ID @convex-dev/auth rejected every Microsoft Entra ID sign-in whenever the Entra app was registered against a multi-tenant authority (/common, /organizations, /consumers). The callback failed inside processAuthorizationCodeResponse with an "unexpected JWT iss claim value" error because the authorization-server metadata still claimed .../common/v2.0 as the issuer while the id_token was signed with the per-tenant issuer (...//v2.0). Port the post-token-exchange re-discovery that @auth/core's handleOAuth runs for providers carrying the conformInternal symbol (packages/core/src/lib/actions/callback/oauth/callback.ts). After the token exchange, decode the id_token, derive the per-tenant authority from its tid claim, re-fetch the discovery document for that authority and replace the AuthorizationServer metadata before processAuthorizationCodeResponse runs. Because @auth/core's conformInternal symbol is marked @internal and is not listed in the package's exports map, importing it from @auth/core/lib/symbols.js breaks under bundlers that enforce package exports (Vite/vitest). Detect the symbol structurally by description on the provider object instead, mirroring upstream's gate without a fragile deep import. The re-discovery uses an inline customFetch wrapper that rewrites the {tenantid} placeholder in the discovery document's issuer field with the real tid. We deliberately bypass provider[customFetch] here because the upstream Microsoft Entra ID provider's hook captures config.issuer at construction time (/common/v2.0) and would rewrite {tenantid} to "common", causing the same mismatch this PR fixes. Doing the rewrite locally makes the fix work against the @auth/core version currently pinned and independent of any upstream release. Add a test/convex/microsoftEntra.test.ts suite covering the re-discovery happy path and a spoofed-issuer rejection, and register the Microsoft Entra ID provider in test/convex/auth.ts with a no-op profile callback so tests don't hit graph.microsoft.com. --- src/server/oauth/callback.ts | 123 +++++++++- test/convex/auth.ts | 15 ++ test/convex/microsoftEntra.test.ts | 347 +++++++++++++++++++++++++++++ 3 files changed, 484 insertions(+), 1 deletion(-) create mode 100644 test/convex/microsoftEntra.test.ts diff --git a/src/server/oauth/callback.ts b/src/server/oauth/callback.ts index 3cbe42dc..495e5ed0 100644 --- a/src/server/oauth/callback.ts +++ b/src/server/oauth/callback.ts @@ -12,11 +12,37 @@ import { callbackUrl, getAuthorizationSignature, } from "./convexAuth.js"; +import { decodeJwt } from "jose"; function formUrlEncode(token: string) { return encodeURIComponent(token).replace(/%20/g, "+"); } +/** + * ConvexAuth: Detect `@auth/core`'s internal `conformInternal` symbol on a + * provider config without importing it. The symbol is marked `@internal` and + * is not exposed by the package's `exports` map, so any direct import from + * `@auth/core/lib/symbols.js` breaks under bundlers that enforce package + * exports (e.g. Vite during `vitest`). + * + * Providers that opt into core conformance logic (currently + * `microsoft-entra-id`, `azure-ad`, `apple`) set + * `[conformInternal]: true` using a `Symbol("conform-internal")` instance + * from `@auth/core`'s internal module. We identify that same symbol on the + * provider by description and read its value. + */ +function hasConformInternalSymbol(provider: object): boolean { + for (const sym of Object.getOwnPropertySymbols(provider)) { + if ( + sym.description === "conform-internal" && + (provider as Record)[sym] === true + ) { + return true; + } + } + return false; +} + /** * Formats client_id and client_secret as an HTTP Basic Authentication header as per the OAuth 2.0 * specified in RFC6749. @@ -52,7 +78,11 @@ export async function handleOAuth( const { provider } = options; // ConvexAuth: The `token` property is not used here - const { userinfo, as } = provider; + // ConvexAuth: `as` is `let` because some providers (e.g. Microsoft Entra ID + // multi-tenant) require re-running OIDC discovery after the token exchange + // using a claim from the ID token. See the `conformInternal` block below. + const { userinfo } = provider; + let { as } = provider; const client: o.Client = { client_id: provider.clientId, @@ -151,6 +181,97 @@ export async function handleOAuth( let profile: Profile = {}; + // ConvexAuth: Ported from @auth/core's `handleOAuth` + // (packages/core/src/lib/actions/callback/oauth/callback.ts). + // Some providers (currently Microsoft Entra ID / Azure AD) need the + // authorization server metadata to be re-processed after the token exchange + // because the initial discovery document uses a placeholder issuer (e.g. + // `https://login.microsoftonline.com/{tenantid}/v2.0`) and the real tenant id + // is only known once we have the ID token. Without this, `validateIssuer` + // inside `processAuthorizationCodeResponse` rejects the ID token whenever the + // app is configured as multi-tenant (`/common`, `/organizations`, `/consumers`). + // + // `@auth/core` gates this on its internal `conformInternal` symbol, but that + // symbol is not exported from the package's public entry points (it lives in + // `@auth/core/lib/symbols.js`, which is not listed in the package's `exports` + // map). To avoid relying on a deep import that breaks bundlers / Vite, we + // detect the symbol structurally by description on the provider object. + if (hasConformInternalSymbol(provider)) { + switch (provider.id) { + case "microsoft-entra-id": + case "azure-ad": { + /** + * These providers return errors in the response body and + * need the authorization server metadata to be re-processed + * based on the `id_token`'s `tid` claim. + * @see: https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-auth-code-flow#error-response-1 + */ + const responseJson = await codeGrantResponse.clone().json(); + if (responseJson.error) { + const cause = { + providerId: provider.id, + ...responseJson, + }; + logWithLevel("DEBUG", "OAuthCallbackError", cause); + throw new Error( + `OAuth Provider returned an error: ${responseJson.error}`, + { cause }, + ); + } + const { tid } = decodeJwt(responseJson.id_token); + if (typeof tid === "string") { + const tenantRe = /microsoftonline\.com\/(\w+)\/v2\.0/; + const tenantId = as.issuer?.match(tenantRe)?.[1] ?? "common"; + const issuer = new URL(as.issuer.replace(tenantId, tid)); + // ConvexAuth: Rewrite the `{tenantid}` placeholder Microsoft returns + // in the discovery document's `issuer` field using the real `tid` + // from the ID token. The provider's own `customFetch` hook (in + // `@auth/core@0.37.x`) derives the replacement from `config.issuer` + // captured at construction, so it can't see the per-tenant authority + // during this re-discovery. The upstream fix at + // https://github.com/nextauthjs/next-auth/pull/13440 derives the + // tenant from the request URL instead; once an `@auth/core` release + // including it is pinned, this wrapper can defer to + // `provider[customFetch]`. + const discoveryResponse = await o.discoveryRequest(issuer, { + [o.customFetch]: async ( + ...args: Parameters + ) => { + const response = await fetch(...args); + const first = args[0]; + const requestUrl = + typeof first === "string" + ? first + : first instanceof URL + ? first.toString() + : first.url; + if ( + !requestUrl.endsWith(".well-known/openid-configuration") + ) { + return response; + } + const json = await response.clone().json(); + if ( + typeof json?.issuer === "string" && + json.issuer.includes("{tenantid}") + ) { + return Response.json({ + ...json, + issuer: json.issuer.replace("{tenantid}", tid), + }); + } + return response; + }, + }); + as = await o.processDiscoveryResponse(issuer, discoveryResponse); + } + break; + } + default: + break; + } + } + // ConvexAuth: We use the value of the nonce later, aside from feeding it into the // `processAuthorizationCodeResponse` function. const nonce = await checks.nonce.use(cookies, resCookies, options); diff --git a/test/convex/auth.ts b/test/convex/auth.ts index 28bf0199..adb43392 100644 --- a/test/convex/auth.ts +++ b/test/convex/auth.ts @@ -3,6 +3,7 @@ import GitHub from "@auth/core/providers/github"; import Google from "@auth/core/providers/google"; import Resend from "@auth/core/providers/resend"; import Apple from "@auth/core/providers/apple"; +import MicrosoftEntraID from "@auth/core/providers/microsoft-entra-id"; import { Anonymous } from "@convex-dev/auth/providers/Anonymous"; import { Password } from "@convex-dev/auth/providers/Password"; import { ConvexError } from "convex/values"; @@ -45,6 +46,20 @@ export const { auth, signIn, signOut, store, isAuthenticated } = convexAuth({ }, profile: undefined, }), + MicrosoftEntraID({ + clientId: process.env.AUTH_MICROSOFT_ENTRA_ID_ID, + clientSecret: process.env.AUTH_MICROSOFT_ENTRA_ID_SECRET, + // Skip the profile-photo fetch in tests; the default `profile` callback + // calls `https://graph.microsoft.com/...` which our test fetch mocks + // don't expect. + profile(profile) { + return { + id: profile.sub, + name: profile.name, + email: profile.email, + }; + }, + }), Resend({ from: process.env.AUTH_EMAIL ?? "My App ", }), diff --git a/test/convex/microsoftEntra.test.ts b/test/convex/microsoftEntra.test.ts new file mode 100644 index 00000000..1c5a898f --- /dev/null +++ b/test/convex/microsoftEntra.test.ts @@ -0,0 +1,347 @@ +import { convexTest } from "convex-test"; +import { SignJWT, importPKCS8 } from "jose"; +import { describe, expect, test, vi } from "vitest"; +import { api } from "./_generated/api"; +import schema from "./schema"; +import { + CONVEX_SITE_URL, + JWKS, + JWT_PRIVATE_KEY, +} from "./test.helpers"; + +// Microsoft Entra ID's discovery document returns the literal string +// `{tenantid}` as the issuer for multi-tenant authorities (`/common`, +// `/organizations`, `/consumers`). The `@auth/core` Microsoft Entra ID provider +// rewrites that placeholder via a `customFetch` hook so the initial issuer +// validation succeeds, but the real per-tenant issuer is only known after the +// token exchange, when we can read `tid` from the `id_token`. +// +// Before the fix, `@convex-dev/auth`'s callback handler skipped the +// post-token-exchange re-discovery that `@auth/core` performs for these +// providers. As a result, multi-tenant tokens were rejected by +// `processAuthorizationCodeResponse`'s issuer check, since the AS metadata +// still claimed `/common/v2.0` as the issuer while the `id_token` carried the +// per-tenant URL. +// +// These tests exercise the ported re-discovery block. + +const TENANT_ID = "11111111-2222-3333-4444-555555555555"; +const SPOOFED_TENANT_ID = "99999999-9999-9999-9999-999999999999"; +const SHARED_AUTHORITY = "https://login.microsoftonline.com/common/v2.0"; + +type DiscoveryDoc = { + issuer: string; + authorization_endpoint: string; + token_endpoint: string; + userinfo_endpoint: string; + jwks_uri: string; + id_token_signing_alg_values_supported: string[]; + response_types_supported: string[]; + subject_types_supported: string[]; + code_challenge_methods_supported: string[]; +}; + +function discoveryDocFor(authorityUrl: string): DiscoveryDoc { + const baseAuthority = "https://login.microsoftonline.com"; + // Microsoft returns `{tenantid}` as a placeholder for the issuer field on + // the multi-tenant authorities. The provider's `customFetch` hook rewrites + // it to whatever segment is in the request URL. We mimic that response + // shape verbatim so the provider can do its replacement. + return { + issuer: `${baseAuthority}/{tenantid}/v2.0`, + authorization_endpoint: `${authorityUrl}/oauth2/v2.0/authorize`, + token_endpoint: `${authorityUrl}/oauth2/v2.0/token`, + userinfo_endpoint: "https://graph.microsoft.com/oidc/userinfo", + jwks_uri: `${authorityUrl}/discovery/v2.0/keys`, + id_token_signing_alg_values_supported: ["RS256"], + response_types_supported: ["code", "id_token"], + subject_types_supported: ["pairwise"], + code_challenge_methods_supported: ["S256"], + }; +} + +async function signIdToken(args: { + issuer: string; + audience: string; + tid: string; + sub: string; + email: string; + name: string; +}) { + const privateKey = await importPKCS8(JWT_PRIVATE_KEY, "RS256"); + return await new SignJWT({ + tid: args.tid, + email: args.email, + name: args.name, + }) + .setProtectedHeader({ alg: "RS256" }) + .setIssuer(args.issuer) + .setAudience(args.audience) + .setSubject(args.sub) + .setIssuedAt() + .setExpirationTime("1h") + .sign(privateKey); +} + +function getJwks() { + return JSON.parse(JWKS) as { + keys: Array>; + }; +} + +async function driveSignInUpTo(t: ReturnType) { + // Stub fetch for the *initial* OIDC discovery that runs lazily when + // convex-auth materializes the provider config inside `t.fetch`. + vi.stubGlobal( + "fetch", + vi.fn(async (input: string | URL) => { + const url = typeof input === "string" ? input : input.toString(); + if ( + url === + `${SHARED_AUTHORITY}/.well-known/openid-configuration` + ) { + return new Response(JSON.stringify(discoveryDocFor(SHARED_AUTHORITY)), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + throw new Error(`Unexpected fetch (init): ${url}`); + }), + ); + + const { redirect, verifier } = await t.action(api.auth.signIn, { + provider: "microsoft-entra-id", + params: {}, + }); + vi.unstubAllGlobals(); + + expect(redirect).toEqual( + expect.stringContaining( + CONVEX_SITE_URL + "/api/auth/signin/microsoft-entra-id", + ), + ); + + const url = new URL(redirect!); + + vi.stubGlobal( + "fetch", + vi.fn(async (input: string | URL) => { + const u = typeof input === "string" ? input : input.toString(); + if ( + u === + `${SHARED_AUTHORITY}/.well-known/openid-configuration` + ) { + return new Response(JSON.stringify(discoveryDocFor(SHARED_AUTHORITY)), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + throw new Error(`Unexpected fetch (redirect): ${u}`); + }), + ); + const response = await t.fetch(`${url.pathname}${url.search}`); + vi.unstubAllGlobals(); + + expect(response.status).toBe(302); + const redirectedTo = response.headers.get("Location")!; + const cookies = response.headers.get("Set-Cookie")!; + const redirectedToParams = new URL(redirectedTo).searchParams; + + return { verifier, cookies, redirectedToParams }; +} + +async function driveCallback( + t: ReturnType, + ctx: { + verifier: string | undefined; + cookies: string; + redirectedToParams: URLSearchParams; + }, + opts: { + idTokenIssuerTenantId: string; + idTokenTidClaim: string; + rediscoveryReturnsTenant?: string; + }, +) { + const callbackUrl = ctx.redirectedToParams.get("redirect_uri")!; + const state = ctx.redirectedToParams.get("state") ?? undefined; + const codeChallenge = ctx.redirectedToParams.get("code_challenge") ?? undefined; + + const issuedCode = "ms-entra-code"; + const issuedAccessToken = "ms-entra-access"; + const expectedTenantAuthority = `https://login.microsoftonline.com/${opts.rediscoveryReturnsTenant ?? opts.idTokenTidClaim}/v2.0`; + + // The id_token's issuer is constructed from `idTokenIssuerTenantId` so we + // can simulate both legitimate and spoofed tokens. + const idTokenIssuer = `https://login.microsoftonline.com/${opts.idTokenIssuerTenantId}/v2.0`; + const audience = process.env.AUTH_MICROSOFT_ENTRA_ID_ID!; + + const idToken = await signIdToken({ + issuer: idTokenIssuer, + audience, + tid: opts.idTokenTidClaim, + sub: "ms-user-1", + email: "user@contoso.example", + name: "Contoso User", + }); + + const fetchMock = vi.fn(async (input: string | URL | Request) => { + const u = + typeof input === "string" + ? input + : input instanceof URL + ? input.toString() + : input.url; + + // The initial discovery may be re-run here because the callback action + // also lazily materializes the provider. + if (u === `${SHARED_AUTHORITY}/.well-known/openid-configuration`) { + return new Response(JSON.stringify(discoveryDocFor(SHARED_AUTHORITY)), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + + // Token exchange. + if (u === `${SHARED_AUTHORITY}/oauth2/v2.0/token`) { + return new Response( + JSON.stringify({ + access_token: issuedAccessToken, + token_type: "Bearer", + expires_in: 3600, + id_token: idToken, + }), + { + status: 200, + headers: { "Content-Type": "application/json" }, + }, + ); + } + + // Re-discovery (the bit this PR is about). + if ( + u === `${expectedTenantAuthority}/.well-known/openid-configuration` + ) { + return new Response( + JSON.stringify(discoveryDocFor(expectedTenantAuthority)), + { + status: 200, + headers: { "Content-Type": "application/json" }, + }, + ); + } + + // JWKS for either authority. + if ( + u === `${SHARED_AUTHORITY}/discovery/v2.0/keys` || + u === `${expectedTenantAuthority}/discovery/v2.0/keys` + ) { + return new Response(JSON.stringify(getJwks()), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + + throw new Error(`Unexpected fetch (callback): ${u}`); + }); + + vi.stubGlobal("fetch", fetchMock); + + // `Set-Cookie` values returned by `Headers.get` are joined with `, ` but + // commas also appear inside `Expires=...` dates. Split on the cookie + // boundary (`,` followed by a space and a token character) instead of just + // `,` so we don't mangle the nonce cookie. + const cookieHeader = ctx.cookies + .split(/,(?=\s*[A-Za-z0-9_-]+=)/) + .map((cookie) => { + const [name, value] = cookie.split(";")[0].split("="); + return `${name.trim()}=${value};`; + }) + .join(" "); + + const callbackParams = new URLSearchParams({ code: issuedCode }); + if (state) callbackParams.set("state", state); + if (codeChallenge) callbackParams.set("code_challenge", codeChallenge); + + const callbackResponse = await t.fetch( + `${new URL(callbackUrl).pathname}?${callbackParams.toString()}`, + { + headers: { Cookie: cookieHeader }, + }, + ); + + vi.unstubAllGlobals(); + + return { callbackResponse, fetchMock, verifier: ctx.verifier }; +} + +function setupEnv() { + process.env.SITE_URL = "http://localhost:5173"; + process.env.CONVEX_SITE_URL = CONVEX_SITE_URL; + process.env.JWT_PRIVATE_KEY = JWT_PRIVATE_KEY; + process.env.JWKS = JWKS; + process.env.AUTH_MICROSOFT_ENTRA_ID_ID = "ms-entra-client-id"; + process.env.AUTH_MICROSOFT_ENTRA_ID_SECRET = "ms-entra-client-secret"; + process.env.AUTH_LOG_LEVEL = "ERROR"; +} + +describe("microsoft entra id multi-tenant issuer re-discovery", () => { + test("accepts an id_token whose issuer matches the tid claim", async () => { + setupEnv(); + const t = convexTest(schema); + const ctx = await driveSignInUpTo(t); + + const { callbackResponse, fetchMock } = await driveCallback( + t, + ctx, + { + idTokenIssuerTenantId: TENANT_ID, + idTokenTidClaim: TENANT_ID, + }, + ); + + expect(callbackResponse.status).toBe(302); + + // The re-discovery endpoint for the real tenant must have been hit. + const requestedUrls = fetchMock.mock.calls.map((c) => { + const input = c[0]; + return typeof input === "string" + ? input + : input instanceof URL + ? input.toString() + : input.url; + }); + expect(requestedUrls).toContain( + `https://login.microsoftonline.com/${TENANT_ID}/v2.0/.well-known/openid-configuration`, + ); + + const finalRedirectedTo = callbackResponse.headers.get("Location")!; + // The callback redirected the browser back to the app with an auth code. + // Before the fix, this redirect would have included an `error=` query + // string instead (because the issuer mismatch made the token exchange + // throw). Asserting on the presence of a non-null `code` query param is + // sufficient to demonstrate the re-discovery succeeded. + const code = new URL(finalRedirectedTo).searchParams.get("code"); + expect(code).not.toBeNull(); + }); + + test("rejects an id_token whose issuer does not match the tid claim", async () => { + setupEnv(); + const t = convexTest(schema); + const ctx = await driveSignInUpTo(t); + + // The id_token claims `tid=TENANT_ID`, so we re-discover against + // `TENANT_ID/v2.0`, but the token itself was issued by the spoofed tenant. + const { callbackResponse } = await driveCallback(t, ctx, { + idTokenIssuerTenantId: SPOOFED_TENANT_ID, + idTokenTidClaim: TENANT_ID, + }); + + // The callback handler turns the OAuth error into a redirect with an + // `error` query param rather than throwing, but importantly the response + // is NOT a successful 302 to the SITE_URL with a code. + expect(callbackResponse.status).toBe(302); + const location = callbackResponse.headers.get("Location")!; + expect(location).not.toContain("?code="); + }); +}); From c695dbe80a6c53ec33cd1083e3e756cc9f2939d0 Mon Sep 17 00:00:00 2001 From: Guust Goossens Date: Tue, 2 Jun 2026 13:36:03 +0200 Subject: [PATCH 2/2] Tighten tenant-id regex to match GUIDs and anchor URL rewrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous re-discovery used `(\w+)` to extract the tenant segment from `as.issuer`, which doesn't match hyphenated tenant GUIDs like `8a2a7b4d-1234-5678-9abc-def012345678` — so when a caller configured the provider with a single-tenant authority, the match silently fell back to `"common"` and `.replace(tenantId, tid)` was a no-op. Switch to `[^/]+` so any URL path segment matches (authority names and GUIDs alike), and rewrite via the segment-anchored regex instead of a plain substring replace so we can't accidentally clobber an unrelated portion of the issuer URL. Matches the upstream fix in nextauthjs/next-auth#13440. --- src/server/oauth/callback.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/server/oauth/callback.ts b/src/server/oauth/callback.ts index 495e5ed0..72301d1c 100644 --- a/src/server/oauth/callback.ts +++ b/src/server/oauth/callback.ts @@ -220,9 +220,17 @@ export async function handleOAuth( } const { tid } = decodeJwt(responseJson.id_token); if (typeof tid === "string") { - const tenantRe = /microsoftonline\.com\/(\w+)\/v2\.0/; - const tenantId = as.issuer?.match(tenantRe)?.[1] ?? "common"; - const issuer = new URL(as.issuer.replace(tenantId, tid)); + // Match any URL path segment so we cover both authority names like + // `common`/`organizations`/`consumers` and hyphenated tenant GUIDs. + // Rewrite via the segment-anchored regex (not a substring replace) + // so we can't accidentally clobber an unrelated part of the URL. + const tenantSegmentRe = /microsoftonline\.com\/[^/]+\/v2\.0/; + const issuer = new URL( + as.issuer.replace( + tenantSegmentRe, + `microsoftonline.com/${tid}/v2.0`, + ), + ); // ConvexAuth: Rewrite the `{tenantid}` placeholder Microsoft returns // in the discovery document's `issuer` field using the real `tid` // from the ID token. The provider's own `customFetch` hook (in