From 24f28da3f1abbc85fffb980346c9bcf6dd95c24c Mon Sep 17 00:00:00 2001 From: "tembo[bot]" <208362400+tembo[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 23:00:25 +0000 Subject: [PATCH] fix(oauth): include scope param in Microsoft OAuth callback token exchange Co-authored-by: Konsti --- .claude/CLAUDE-KNOWLEDGE.md | 3 ++ .../oauth/callback/[provider_id]/route.tsx | 1 + apps/backend/src/oauth/providers/base.test.ts | 23 ++++++++++++++- apps/backend/src/oauth/providers/base.tsx | 28 ++++++++++++++++++- .../backend/src/oauth/providers/microsoft.tsx | 1 + 5 files changed, 54 insertions(+), 2 deletions(-) diff --git a/.claude/CLAUDE-KNOWLEDGE.md b/.claude/CLAUDE-KNOWLEDGE.md index d88b86b20e..2039914856 100644 --- a/.claude/CLAUDE-KNOWLEDGE.md +++ b/.claude/CLAUDE-KNOWLEDGE.md @@ -562,3 +562,6 @@ A: Project config overrides only support the hosted `sourceOfTruth` shape. Legac ## Q: How should managed email onboarding e2e tests wait for mock verification? A: Do not rely on a fixed `wait(1500)` after setup. The mock onboarding path flips the domain to `verified` asynchronously through `runAsynchronously`, so tests should poll the managed-onboarding check endpoint until the expected status appears. + +## Q: How should Microsoft inner OAuth callback token exchanges include scopes? +A: Microsoft can reject the authorization-code token exchange with `AADSTS70011` if no `scope` parameter is sent. Keep this provider-specific by threading `providerScope` from `OAuthOuterInfo` into `OAuthBaseProvider.getCallback`, and opt Microsoft into adding the merged base and provider scope through `CallbackExtras.exchangeBody.scope`; do not add token-exchange scope globally for every provider. diff --git a/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx b/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx index 3e7613c3b9..c0ccd9c4ba 100644 --- a/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx +++ b/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx @@ -168,6 +168,7 @@ const handler = createSmartRouteHandler({ callbackResult = await providerObj.getCallback({ codeVerifier: innerCodeVerifier, state: innerState, + extraScope: providerScope, callbackParams: { ...query, ...body, diff --git a/apps/backend/src/oauth/providers/base.test.ts b/apps/backend/src/oauth/providers/base.test.ts index f732f6d461..c3cb8a6e52 100644 --- a/apps/backend/src/oauth/providers/base.test.ts +++ b/apps/backend/src/oauth/providers/base.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { getOAuthAccessTokenRefreshError, getOAuthAccessTokenRefreshErrorDisposition, isRetryableOAuthUserInfoError, resolveOAuthAccessTokenExpiredAt } from "./base"; +import { getOAuthAccessTokenRefreshError, getOAuthAccessTokenRefreshErrorDisposition, getOAuthCallbackExtras, isRetryableOAuthUserInfoError, resolveOAuthAccessTokenExpiredAt } from "./base"; describe("isRetryableOAuthUserInfoError", () => { it("returns true for openid-client timeout errors", () => { @@ -102,6 +102,27 @@ describe("getOAuthAccessTokenRefreshError", () => { }); }); +describe("getOAuthCallbackExtras", () => { + it("does not add token exchange scope by default", () => { + expect(getOAuthCallbackExtras({ + baseScope: "User.Read openid", + extraScope: "Mail.Read", + })).toBeUndefined(); + }); + + it("adds the merged scope to callback token exchange extras when enabled", () => { + expect(getOAuthCallbackExtras({ + baseScope: "User.Read openid", + extraScope: "Mail.Read User.Read", + includeScopeInCallbackTokenRequest: true, + })).toEqual({ + exchangeBody: { + scope: "User.Read openid Mail.Read", + }, + }); + }); +}); + describe("resolveOAuthAccessTokenExpiredAt", () => { it("uses finite provider expires_in values", () => { expect(resolveOAuthAccessTokenExpiredAt({ diff --git a/apps/backend/src/oauth/providers/base.tsx b/apps/backend/src/oauth/providers/base.tsx index a75fb67fb8..dffde721f7 100644 --- a/apps/backend/src/oauth/providers/base.tsx +++ b/apps/backend/src/oauth/providers/base.tsx @@ -3,7 +3,8 @@ import { HexclaveAssertionError, StatusError, captureError } from "@stackframe/s import { wait } from "@stackframe/stack-shared/dist/utils/promises"; import { Result } from "@stackframe/stack-shared/dist/utils/results"; import { mergeScopeStrings } from "@stackframe/stack-shared/dist/utils/strings"; -import { CallbackParamsType, Client, Issuer, TokenSet as OIDCTokenSet, custom, generators } from "openid-client"; +import type { CallbackExtras, CallbackParamsType, Client, TokenSet as OIDCTokenSet } from "openid-client"; +import { Issuer, custom, generators } from "openid-client"; import { OAuthUserInfo } from "../utils"; const OAUTH_USERINFO_TOTAL_ATTEMPTS = 3; @@ -289,6 +290,22 @@ function processTokenSet(providerName: string, tokenSet: OIDCTokenSet, defaultAc }; } +export function getOAuthCallbackExtras(options: { + baseScope: string, + extraScope?: string, + includeScopeInCallbackTokenRequest?: boolean, +}): CallbackExtras | undefined { + if (options.includeScopeInCallbackTokenRequest !== true) { + return undefined; + } + + return { + exchangeBody: { + scope: mergeScopeStrings(options.baseScope, options.extraScope || ""), + }, + }; +} + export abstract class OAuthBaseProvider { constructor( public readonly oauthClient: Client, @@ -299,6 +316,7 @@ export abstract class OAuthBaseProvider { public readonly noPKCE?: boolean, public readonly openid?: boolean, public readonly alternativeIssuers?: string[], + public readonly includeScopeInCallbackTokenRequest?: boolean, ) {} protected static async createConstructorArgs(options: @@ -312,6 +330,7 @@ export abstract class OAuthBaseProvider { tokenEndpointAuthMethod?: "client_secret_post" | "client_secret_basic", noPKCE?: boolean, alternativeIssuers?: string[], + includeScopeInCallbackTokenRequest?: boolean, } & ( | ({ @@ -360,6 +379,7 @@ export abstract class OAuthBaseProvider { options.noPKCE, options.openid, options.alternativeIssuers, + options.includeScopeInCallbackTokenRequest, ] as const; } @@ -386,6 +406,7 @@ export abstract class OAuthBaseProvider { callbackParams: CallbackParamsType, codeVerifier: string, state: string, + extraScope?: string, }): Promise<{ userInfo: OAuthUserInfo, tokenSet: TokenSet }> { let tokenSet; const callbackParams = { ...options.callbackParams }; @@ -408,6 +429,11 @@ export abstract class OAuthBaseProvider { code_verifier: this.noPKCE ? undefined : options.codeVerifier, state: options.state, }, + getOAuthCallbackExtras({ + baseScope: this.scope, + extraScope: options.extraScope, + includeScopeInCallbackTokenRequest: this.includeScopeInCallbackTokenRequest, + }), ] as const; try { diff --git a/apps/backend/src/oauth/providers/microsoft.tsx b/apps/backend/src/oauth/providers/microsoft.tsx index 3210e4b654..f3215a982e 100644 --- a/apps/backend/src/oauth/providers/microsoft.tsx +++ b/apps/backend/src/oauth/providers/microsoft.tsx @@ -25,6 +25,7 @@ export class MicrosoftProvider extends OAuthBaseProvider { baseScope: "User.Read openid", openid: true, jwksUri: `https://login.microsoftonline.com/${tenantId}/discovery/v2.0/keys`, + includeScopeInCallbackTokenRequest: true, ...options, })); }