Include the scope parameter in certain Microsoft OAuth operations

.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 OAuth callback token exchange include scopes?
+A: Microsoft Entra ID's v2 token endpoint can reject authorization-code exchanges with `AADSTS70011` if the token request omits `scope`. Keep scope emission opt-in at the provider layer (`includeScopeInCallbackTokenExchange`) and pass the same merged base/provider scopes to `openid-client` via the callback `extras.exchangeBody.scope` parameter. The callback route must forward stored `providerScope` from the outer OAuth info so custom Microsoft provider scopes are included in the token exchange.

AGENTS.md

@@ -118,6 +118,7 @@ To see all development ports, refer to the index.html of `apps/dev-launchpad/pub
 - NEVER EVER return a server error with an internal server error that may contain information that the user shouldn't see. For example, never return the error message on a public API from an upstream provider without properly filtering it first. Most of the time, for internal server errors, you should just use StackAssertionError (which won't pass the message to the user), not StatusError (you almost never want to instantiate a StatusError with status 5xx).
 - When adding code to the `private` part of the backend, put the actual implementation into `implementation` (if the submodule is checked out), and implement a simple fallback in `implementation-fallback` for self-hosters. `implementation.generated.ts` will automatically be generated, which you can then import from `index.ts`. (See the existing code as an example.) If the submodule isn't checked out, but you need to add code to the `private` part of the backend, let the user know.
 - Security-sensitive code on the backend that shouldn't be public should be in the `private` part of the backend.
+- When you fix some obscure bug, or otherwise make a small change that is the result of a complex thought, add a concise comment explaining the thought in detail. Your mental model should be that we want to keep track of all the tiny decisions that are not clearly visible in the code, such that when/if we rewrite the code in the future, we don't have to re-learn all the tiny decisions that were made iteratively.
 
 ### Code-related
 - Use ES6 maps instead of records wherever you can.

apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx

@@ -170,6 +170,7 @@ const handler = createSmartRouteHandler({
         callbackResult = await providerObj.getCallback({
           codeVerifier: innerCodeVerifier,
           state: innerState,
+          extraScope: providerScope,
           callbackParams: {
             ...query,
             ...body,

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, getOAuthCallbackExtraParams, isRetryableOAuthUserInfoError, resolveOAuthAccessTokenExpiredAt } from "./base";
 
 describe("isRetryableOAuthUserInfoError", () => {
   it("returns true for openid-client timeout errors", () => {
@@ -102,6 +102,28 @@ describe("getOAuthAccessTokenRefreshError", () => {
   });
 });
 
+describe("getOAuthCallbackExtraParams", () => {
+  it("omits token exchange params unless the provider opts in", () => {
+    expect(getOAuthCallbackExtraParams({
+      includeScopeInTokenExchange: false,
+      baseScope: "openid",
+      extraScope: "User.Read",
+    })).toBeUndefined();
+  });
+
+  it("includes merged scopes for providers that require scope on callback exchange", () => {
+    expect(getOAuthCallbackExtraParams({
+      includeScopeInTokenExchange: true,
+      baseScope: "User.Read openid",
+      extraScope: "Mail.Read",
+    })).toEqual({
+      exchangeBody: {
+        scope: "User.Read openid Mail.Read",
+      },
+    });
+  });
+});
+
 describe("resolveOAuthAccessTokenExpiredAt", () => {
   it("uses finite provider expires_in values", () => {
     expect(resolveOAuthAccessTokenExpiredAt({

apps/backend/src/oauth/providers/base.tsx

@@ -219,6 +219,22 @@ export function getOAuthAccessTokenRefreshError(error: unknown, options: {
 
 type DefaultAccessTokenExpiresInMillis = number | null | ((tokenSet: OIDCTokenSet) => number | null | undefined);
 
+export function getOAuthCallbackExtraParams(options: {
+  includeScopeInTokenExchange: boolean | undefined,
+  baseScope: string,
+  extraScope: string | undefined,
+}) {
+  if (!options.includeScopeInTokenExchange) {
+    return undefined;
+  }
+
+  return {
+    exchangeBody: {
+      scope: mergeScopeStrings(options.baseScope, options.extraScope ?? ""),
+    },
+  };
+}
+
 function getFiniteNumber(value: unknown): number | undefined {
   return typeof value === "number" && Number.isFinite(value) ? value : undefined;
 }
@@ -299,6 +315,7 @@ export abstract class OAuthBaseProvider {
     public readonly noPKCE?: boolean,
     public readonly openid?: boolean,
     public readonly alternativeIssuers?: string[],
+    public readonly includeScopeInCallbackTokenExchange?: boolean,
   ) {}
 
   protected static async createConstructorArgs(options:
@@ -312,6 +329,7 @@ export abstract class OAuthBaseProvider {
       tokenEndpointAuthMethod?: "client_secret_post" | "client_secret_basic",
       noPKCE?: boolean,
       alternativeIssuers?: string[],
+      includeScopeInCallbackTokenExchange?: boolean,
     }
     & (
       | ({
@@ -360,6 +378,7 @@ export abstract class OAuthBaseProvider {
       options.noPKCE,
       options.openid,
       options.alternativeIssuers,
+      options.includeScopeInCallbackTokenExchange,
     ] as const;
   }
 
@@ -386,6 +405,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 +428,11 @@ export abstract class OAuthBaseProvider {
         code_verifier: this.noPKCE ? undefined : options.codeVerifier,
         state: options.state,
       },
+      getOAuthCallbackExtraParams({
+        includeScopeInTokenExchange: this.includeScopeInCallbackTokenExchange,
+        baseScope: this.scope,
+        extraScope: options.extraScope,
+      }),
     ] as const;
 
     try {

apps/backend/src/oauth/providers/microsoft.tsx

@@ -26,6 +26,11 @@ export class MicrosoftProvider extends OAuthBaseProvider {
       baseScope: "User.Read openid",
       openid: true,
       jwksUri: `https://login.microsoftonline.com/${tenantId}/discovery/v2.0/keys`,
+      // Microsoft treats the token-request scope as a resource selector during
+      // authorization-code redemption. Plain sign-in can work without restating
+      // it, but connected-account flows with extra provider_scope values have
+      // returned AADSTS70011 unless we include the originally requested scopes.
+      includeScopeInCallbackTokenExchange: true,
       ...rest,
     }));
   }