From a59a9bfb072e32cc73611c85ac03d4e61286e630 Mon Sep 17 00:00:00 2001 From: sudie-codes Date: Thu, 9 Apr 2026 20:05:54 -0700 Subject: [PATCH] fix(msteams): accept Bot Framework audience in JWT validation (#58249) (#62674) * fix(msteams): use jsonwebtoken directly for JWT validation with correct audience (#58249) * chore(msteams): regenerate lockfile for jwt deps * fix(msteams): clean up unused serviceUrl parameter in JWT validator * test(msteams): cover STS issuer in JWT validation * fix(msteams): type jwt verify audiences and issuers --------- Co-authored-by: Brad Groux --- extensions/msteams/package.json | 5 +- extensions/msteams/src/sdk.test.ts | 199 ++++++++++++++--------------- extensions/msteams/src/sdk.ts | 153 +++++++++++++++------- pnpm-lock.yaml | 31 +++++ 4 files changed, 236 insertions(+), 152 deletions(-) diff --git a/extensions/msteams/package.json b/extensions/msteams/package.json index b5f7ebff324..89e43a1f0af 100644 --- a/extensions/msteams/package.json +++ b/extensions/msteams/package.json @@ -6,10 +6,13 @@ "dependencies": { "@microsoft/teams.api": "2.0.6", "@microsoft/teams.apps": "2.0.6", - "express": "^5.2.1" + "express": "^5.2.1", + "jsonwebtoken": "^9.0.3", + "jwks-rsa": "^4.0.1" }, "devDependencies": { "@openclaw/plugin-sdk": "workspace:*", + "@types/jsonwebtoken": "^9.0.9", "openclaw": "workspace:*" }, "peerDependencies": { diff --git a/extensions/msteams/src/sdk.test.ts b/extensions/msteams/src/sdk.test.ts index 1ea84925aa8..bfb951b9998 100644 --- a/extensions/msteams/src/sdk.test.ts +++ b/extensions/msteams/src/sdk.test.ts @@ -7,33 +7,43 @@ import { } from "./sdk.js"; import type { MSTeamsCredentials } from "./token.js"; -const jwtValidatorState = vi.hoisted(() => ({ - instances: [] as Array<{ config: Record }>, - behaviorByJwks: new Map(), - calls: [] as Array<{ jwksUri: string; token: string; overrideOptions?: unknown }>, -})); - const clientConstructorState = vi.hoisted(() => ({ calls: [] as Array<{ serviceUrl: string; options: unknown }>, })); -vi.mock("@microsoft/teams.apps/dist/middleware/auth/jwt-validator.js", () => ({ - JwtValidator: class JwtValidator { - private readonly config: Record; +// Track jwt.verify calls to assert audience/issuer/algorithm config. +const jwtState = vi.hoisted(() => ({ + verifyBehavior: "success" as "success" | "throw", + decodedHeader: { kid: "key-1" } as { kid?: string } | null, + decodedPayload: { iss: "https://api.botframework.com" } as { iss?: string } | null, + verifyCalls: [] as Array<{ token: string; options: unknown }>, +})); - constructor(config: Record) { - this.config = config; - jwtValidatorState.instances.push({ config }); +const jwtMockImpl = { + decode: (token: string, opts?: { complete?: boolean }) => { + if (opts?.complete) { + return jwtState.decodedHeader ? { header: jwtState.decodedHeader } : null; } + return jwtState.decodedPayload; + }, + verify: (token: string, _key: string, options: unknown) => { + jwtState.verifyCalls.push({ token, options }); + if (jwtState.verifyBehavior === "throw") { + throw new Error("invalid signature"); + } + return { sub: "ok" }; + }, +}; - async validateAccessToken(token: string, overrideOptions?: unknown): Promise { - const jwksUri = String((this.config.jwksUriOptions as { uri?: string })?.uri ?? ""); - jwtValidatorState.calls.push({ jwksUri, token, overrideOptions }); - const behavior = jwtValidatorState.behaviorByJwks.get(jwksUri) ?? "null"; - if (behavior === "throw") { - throw new Error("validator error"); - } - return behavior === "success" ? { sub: "ok" } : null; +vi.mock("jsonwebtoken", () => ({ + ...jwtMockImpl, + default: jwtMockImpl, +})); + +vi.mock("jwks-rsa", () => ({ + JwksClient: class JwksClient { + async getSigningKey(_kid: string) { + return { getPublicKey: () => "mock-public-key" }; } }, })); @@ -43,9 +53,10 @@ const originalFetch = globalThis.fetch; afterEach(() => { globalThis.fetch = originalFetch; clientConstructorState.calls.length = 0; - jwtValidatorState.instances.length = 0; - jwtValidatorState.calls.length = 0; - jwtValidatorState.behaviorByJwks.clear(); + jwtState.verifyCalls.length = 0; + jwtState.verifyBehavior = "success"; + jwtState.decodedHeader = { kid: "key-1" }; + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; vi.restoreAllMocks(); }); @@ -186,106 +197,90 @@ describe("createBotFrameworkJwtValidator", () => { tenantId: "tenant-id", } satisfies MSTeamsCredentials; - it("validates with legacy Bot Framework JWKS and issuer first", async () => { - jwtValidatorState.behaviorByJwks.set( - "https://login.botframework.com/v1/.well-known/keys", - "success", - ); + it("validates a token with Bot Framework issuer and correct audience list", async () => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-1", "https://service.example.com")).resolves.toBe( - true, - ); + await expect(validator.validate("Bearer token-bf")).resolves.toBe(true); - expect(jwtValidatorState.instances).toHaveLength(2); - expect(jwtValidatorState.calls).toHaveLength(1); - expect(jwtValidatorState.calls[0]).toMatchObject({ - jwksUri: "https://login.botframework.com/v1/.well-known/keys", - token: "token-1", - overrideOptions: { - validateServiceUrl: { expectedServiceUrl: "https://service.example.com" }, - }, - }); + expect(jwtState.verifyCalls).toHaveLength(1); + const opts = jwtState.verifyCalls[0]?.options as Record; + expect(opts.audience).toEqual(["app-id", "api://app-id", "https://api.botframework.com"]); + expect(opts.algorithms).toEqual(["RS256"]); + expect(opts.clockTolerance).toBe(300); }); - it("falls back to Entra JWKS when Bot Framework validation fails", async () => { - jwtValidatorState.behaviorByJwks.set( - "https://login.botframework.com/v1/.well-known/keys", - "null", - ); - jwtValidatorState.behaviorByJwks.set( - "https://login.microsoftonline.com/common/discovery/v2.0/keys", - "success", - ); + it("accepts tokens with aud: https://api.botframework.com (#58249)", async () => { + // This is the critical fix: the old JwtValidator rejected this audience. + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-2")).resolves.toBe(true); + await expect(validator.validate("Bearer botfw-token")).resolves.toBe(true); - expect(jwtValidatorState.calls).toHaveLength(2); - expect(jwtValidatorState.calls[0]?.jwksUri).toBe( - "https://login.botframework.com/v1/.well-known/keys", - ); - expect(jwtValidatorState.calls[1]?.jwksUri).toBe( - "https://login.microsoftonline.com/common/discovery/v2.0/keys", - ); - - const entraConfig = jwtValidatorState.instances - .map((instance) => instance.config) - .find( - (config) => - String((config.jwksUriOptions as { uri?: string })?.uri) === - "https://login.microsoftonline.com/common/discovery/v2.0/keys", - ); - expect(entraConfig).toBeDefined(); - expect(entraConfig?.validateIssuer).toEqual({ allowedTenantIds: ["tenant-id"] }); + const opts = jwtState.verifyCalls[0]?.options as Record; + expect((opts.audience as string[]).includes("https://api.botframework.com")).toBe(true); }); - it("falls back to Entra JWKS when Bot Framework validation throws", async () => { - jwtValidatorState.behaviorByJwks.set( - "https://login.botframework.com/v1/.well-known/keys", - "throw", - ); - jwtValidatorState.behaviorByJwks.set( - "https://login.microsoftonline.com/common/discovery/v2.0/keys", - "success", - ); + it("validates a token with Entra issuer", async () => { + jwtState.decodedPayload = { iss: `https://login.microsoftonline.com/tenant-id/v2.0` }; const validator = await createBotFrameworkJwtValidator(creds); - await expect( - validator.validate("Bearer token-throw", "https://service.example.com"), - ).resolves.toBe(true); + await expect(validator.validate("Bearer token-entra")).resolves.toBe(true); - expect(jwtValidatorState.calls).toHaveLength(2); - expect(jwtValidatorState.calls[0]).toMatchObject({ - jwksUri: "https://login.botframework.com/v1/.well-known/keys", - token: "token-throw", - overrideOptions: { - validateServiceUrl: { expectedServiceUrl: "https://service.example.com" }, - }, - }); - expect(jwtValidatorState.calls[1]).toMatchObject({ - jwksUri: "https://login.microsoftonline.com/common/discovery/v2.0/keys", - token: "token-throw", - overrideOptions: { - validateServiceUrl: { expectedServiceUrl: "https://service.example.com" }, - }, - }); + expect(jwtState.verifyCalls).toHaveLength(1); + const opts = jwtState.verifyCalls[0]?.options as Record; + expect(opts.issuer as string[]).toContain("https://login.microsoftonline.com/tenant-id/v2.0"); }); - it("returns false when all validator paths fail", async () => { - jwtValidatorState.behaviorByJwks.set( - "https://login.botframework.com/v1/.well-known/keys", - "throw", - ); + it("validates a token with STS Windows issuer", async () => { + jwtState.decodedPayload = { + iss: "https://sts.windows.net/d6d49420-f39b-4df7-a1dc-d59a935871db/", + }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-3")).resolves.toBe(false); - expect(jwtValidatorState.calls).toHaveLength(2); + await expect(validator.validate("Bearer token-sts")).resolves.toBe(true); + + expect(jwtState.verifyCalls).toHaveLength(1); + const opts = jwtState.verifyCalls[0]?.options as Record; + expect(opts.issuer as string[]).toContain( + "https://sts.windows.net/d6d49420-f39b-4df7-a1dc-d59a935871db/", + ); + }); + + it("rejects tokens with unknown issuer", async () => { + jwtState.decodedPayload = { iss: "https://evil.example.com" }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer token-evil")).resolves.toBe(false); + expect(jwtState.verifyCalls).toHaveLength(0); + }); + + it("returns false when signature verification fails", async () => { + jwtState.verifyBehavior = "throw"; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer token-bad")).resolves.toBe(false); }); it("returns false for empty bearer token", async () => { const validator = await createBotFrameworkJwtValidator(creds); await expect(validator.validate("Bearer ")).resolves.toBe(false); - expect(jwtValidatorState.calls).toHaveLength(0); + expect(jwtState.verifyCalls).toHaveLength(0); + }); + + it("returns false when token has no kid header", async () => { + jwtState.decodedHeader = { kid: undefined }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer no-kid")).resolves.toBe(false); + expect(jwtState.verifyCalls).toHaveLength(0); + }); + + it("returns false when token has no issuer claim", async () => { + jwtState.decodedPayload = { iss: undefined }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer no-iss")).resolves.toBe(false); + expect(jwtState.verifyCalls).toHaveLength(0); }); }); diff --git a/extensions/msteams/src/sdk.ts b/extensions/msteams/src/sdk.ts index c62a20f9eb5..7504215da70 100644 --- a/extensions/msteams/src/sdk.ts +++ b/extensions/msteams/src/sdk.ts @@ -428,72 +428,127 @@ export async function loadMSTeamsSdkWithAuth(creds: MSTeamsCredentials) { } /** - * Create a Bot Framework JWT validator with strict multi-issuer support. + * Bot Framework issuer → JWKS mapping. + * During Microsoft's transition, inbound service tokens can be signed by either + * the legacy Bot Framework issuer or the Entra issuer. Each gets its own JWKS + * endpoint so we verify signatures with the correct key set. + */ +const BOT_FRAMEWORK_ISSUERS: ReadonlyArray<{ + issuer: string | ((tenantId: string) => string); + jwksUri: string; +}> = [ + { + issuer: "https://api.botframework.com", + jwksUri: "https://login.botframework.com/v1/.well-known/keys", + }, + { + issuer: (tenantId: string) => `https://login.microsoftonline.com/${tenantId}/v2.0`, + jwksUri: "https://login.microsoftonline.com/common/discovery/v2.0/keys", + }, + { + issuer: "https://sts.windows.net/d6d49420-f39b-4df7-a1dc-d59a935871db/", + jwksUri: "https://login.microsoftonline.com/common/discovery/v2.0/keys", + }, +]; + +/** + * Create a Bot Framework JWT validator using jsonwebtoken + jwks-rsa directly. * - * During Microsoft's transition, inbound service tokens can be signed by either: - * - Legacy Bot Framework issuer/JWKS - * - Entra issuer/JWKS + * The @microsoft/teams.apps JwtValidator hardcodes audience to [clientId, api://clientId], + * which rejects valid Bot Framework tokens that carry aud: "https://api.botframework.com". + * This implementation uses jsonwebtoken directly with the correct audience list, matching + * the behavior of the legacy @microsoft/agents-hosting authorizeJWT middleware. * - * Security invariants are preserved for both paths: - * - signature verification (issuer-specific JWKS) - * - audience validation (appId) - * - issuer validation (strict allowlist) - * - expiration validation (Teams SDK defaults) + * Security invariants: + * - signature verification via issuer-specific JWKS endpoints + * - audience validation: appId, api://appId, and https://api.botframework.com + * - issuer validation: strict allowlist (Bot Framework + tenant-scoped Entra) + * - expiration validation with 5-minute clock tolerance */ export async function createBotFrameworkJwtValidator(creds: MSTeamsCredentials): Promise<{ - validate: (authHeader: string, serviceUrl?: string) => Promise; + validate: (authHeader: string) => Promise; }> { - const { JwtValidator } = - await import("@microsoft/teams.apps/dist/middleware/auth/jwt-validator.js"); + const jwt = await import("jsonwebtoken"); + const { JwksClient } = await import("jwks-rsa"); - const botFrameworkValidator = new JwtValidator({ - clientId: creds.appId, - tenantId: creds.tenantId, - validateIssuer: { allowedIssuer: "https://api.botframework.com" }, - jwksUriOptions: { - type: "uri", - uri: "https://login.botframework.com/v1/.well-known/keys", - }, - }); + const allowedAudiences: [string, ...string[]] = [ + creds.appId, + `api://${creds.appId}`, + "https://api.botframework.com", + ]; - const entraValidator = new JwtValidator({ - clientId: creds.appId, - tenantId: creds.tenantId, - validateIssuer: { allowedTenantIds: [creds.tenantId] }, - jwksUriOptions: { - type: "uri", - uri: "https://login.microsoftonline.com/common/discovery/v2.0/keys", - }, - }); + const allowedIssuers = BOT_FRAMEWORK_ISSUERS.map((entry) => + typeof entry.issuer === "function" ? entry.issuer(creds.tenantId) : entry.issuer, + ) as [string, ...string[]]; - async function validateWithFallback( - token: string, - overrides: { validateServiceUrl: { expectedServiceUrl: string } } | undefined, - ): Promise { - for (const validator of [botFrameworkValidator, entraValidator]) { - try { - const result = await validator.validateAccessToken(token, overrides); - if (result != null) { - return true; - } - } catch { - continue; - } + // One JWKS client per distinct endpoint, cached for the validator lifetime. + const jwksClients = new Map>(); + function getJwksClient(uri: string): InstanceType { + let client = jwksClients.get(uri); + if (!client) { + client = new JwksClient({ + jwksUri: uri, + cache: true, + cacheMaxAge: 600_000, + rateLimit: true, + }); + jwksClients.set(uri, client); } - return false; + return client; + } + + /** Decode the token header without verification to determine the kid. */ + function decodeHeader(token: string): { kid?: string } | null { + const decoded = jwt.decode(token, { complete: true }); + return decoded && typeof decoded === "object" ? (decoded.header as { kid?: string }) : null; + } + + /** Resolve the issuer entry for a token's issuer claim (pre-verification). */ + function resolveIssuerEntry(issuerClaim: string | undefined) { + if (!issuerClaim) { + return undefined; + } + return BOT_FRAMEWORK_ISSUERS.find((entry) => { + const expected = + typeof entry.issuer === "function" ? entry.issuer(creds.tenantId) : entry.issuer; + return expected === issuerClaim; + }); } return { - async validate(authHeader: string, serviceUrl?: string): Promise { + async validate(authHeader: string, _serviceUrl?: string): Promise { const token = authHeader.startsWith("Bearer ") ? authHeader.slice(7) : authHeader; if (!token) { return false; } - const overrides = serviceUrl - ? ({ validateServiceUrl: { expectedServiceUrl: serviceUrl } } as const) - : undefined; - return await validateWithFallback(token, overrides); + // Decode without verification to extract issuer and kid for key lookup. + const header = decodeHeader(token); + const unverifiedPayload = jwt.decode(token) as { iss?: string } | null; + if (!header?.kid || !unverifiedPayload?.iss) { + return false; + } + + // Resolve which JWKS endpoint to use based on the issuer claim. + const issuerEntry = resolveIssuerEntry(unverifiedPayload.iss); + if (!issuerEntry) { + return false; + } + + const client = getJwksClient(issuerEntry.jwksUri); + try { + const signingKey = await client.getSigningKey(header.kid); + const publicKey = signingKey.getPublicKey(); + jwt.verify(token, publicKey, { + audience: allowedAudiences, + issuer: allowedIssuers, + algorithms: ["RS256"], + clockTolerance: 300, + }); + return true; + } catch { + return false; + } }, }; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index aaa9030657c..4b36209519c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -830,10 +830,19 @@ importers: express: specifier: ^5.2.1 version: 5.2.1 + jsonwebtoken: + specifier: ^9.0.3 + version: 9.0.3 + jwks-rsa: + specifier: ^4.0.1 + version: 4.0.1 devDependencies: '@openclaw/plugin-sdk': specifier: workspace:* version: link:../../packages/plugin-sdk + '@types/jsonwebtoken': + specifier: ^9.0.9 + version: 9.0.10 openclaw: specifier: workspace:* version: link:../.. @@ -5427,6 +5436,10 @@ packages: resolution: {integrity: sha512-BqTyEDV+lS8F2trk3A+qJnxV5Q9EqKCBJOPti3W97r7qTympCZjb7h2X6f2kc+0K3rsSTY1/6YG2eaXKoj497w==} engines: {node: '>=14'} + jwks-rsa@4.0.1: + resolution: {integrity: sha512-poXwUA8S4cP9P5N8tZS3xnUDJH8WmwSGfKK9gIaRPdjLHyJtd9iX/cngX9CUIe0Caof5JhK2EbN7N5lnnaf9NA==} + engines: {node: ^20.19.0 || ^22.12.0 || >= 23.0.0} + jws@4.0.1: resolution: {integrity: sha512-EKI/M/yqPncGUUh44xz0PxSidXFr/+r0pA70+gIYhjv+et7yxM+s29Y+VGDkovRofQem0fs7Uvf4+YmAdyRduA==} @@ -5623,6 +5636,9 @@ packages: lru-memoizer@2.3.0: resolution: {integrity: sha512-GXn7gyHAMhO13WSKrIiNfztwxodVsP8IoZ3XfrJV4yH2x0/OeTO/FIaAHTY5YekdGgW94njfuKmyyt1E0mR6Ug==} + lru-memoizer@3.0.0: + resolution: {integrity: sha512-m83w/cYXLdUIboKSPxzPAGfYnk+vqeDYXuoSrQRw1q+yVEd8IXhvMufN8Q5TIPe7e2jyX4SRNrDJI2Skw1yznQ==} + lru_map@0.4.1: resolution: {integrity: sha512-I+lBvqMMFfqaV8CJCISjI3wbjmwVu/VyOoU7+qtu9d7ioW5klMgsTTiUOUp+DJvfTTzKXoPbyC6YfgkNcyPSOg==} @@ -12146,6 +12162,16 @@ snapshots: transitivePeerDependencies: - supports-color + jwks-rsa@4.0.1: + dependencies: + '@types/jsonwebtoken': 9.0.10 + debug: 4.4.3 + jose: 6.2.2 + limiter: 1.1.5 + lru-memoizer: 3.0.0 + transitivePeerDependencies: + - supports-color + jws@4.0.1: dependencies: jwa: 2.0.1 @@ -12303,6 +12329,11 @@ snapshots: lodash.clonedeep: 4.5.0 lru-cache: 6.0.0 + lru-memoizer@3.0.0: + dependencies: + lodash.clonedeep: 4.5.0 + lru-cache: 11.2.7 + lru_map@0.4.1: {} magic-string@0.30.21: