From 2c3d7f5badbecbe353004f9a115a5a86fee2f2fc Mon Sep 17 00:00:00 2001 From: Agustin Rivera <31522568+eleqtrizit@users.noreply.github.com> Date: Thu, 28 May 2026 07:31:46 -0700 Subject: [PATCH] fix(msteams): bind bot framework service urls (#87160) * fix(msteams): bind bot framework service urls * fix(msteams): harden service url validation --- .../src/attachments/bot-framework.test.ts | 60 +++++++ .../msteams/src/attachments/bot-framework.ts | 28 ++- .../msteams/src/attachments/shared.test.ts | 27 ++- extensions/msteams/src/attachments/shared.ts | 10 ++ .../msteams/src/monitor.lifecycle.test.ts | 54 +++++- extensions/msteams/src/monitor.ts | 39 +++-- extensions/msteams/src/sdk.test.ts | 159 ++++++++++++++++-- extensions/msteams/src/sdk.ts | 58 ++++++- 8 files changed, 388 insertions(+), 47 deletions(-) diff --git a/extensions/msteams/src/attachments/bot-framework.test.ts b/extensions/msteams/src/attachments/bot-framework.test.ts index 185fbfcdcdc..ce6ca780b2e 100644 --- a/extensions/msteams/src/attachments/bot-framework.test.ts +++ b/extensions/msteams/src/attachments/bot-framework.test.ts @@ -205,6 +205,66 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => { expect(runtime.saveCalls).toHaveLength(0); }); + it("does not send Bot Framework service tokens to non-auth-allowlisted media hosts", async () => { + const seenAuth: Array = []; + const fetchFn: typeof fetch = (async (_input: RequestInfo | URL, init?: RequestInit) => { + seenAuth.push(new Headers(init?.headers).get("authorization")); + return new Response("unauthorized", { status: 401 }); + }) as typeof fetch; + + const media = await downloadMSTeamsBotFrameworkAttachment({ + serviceUrl: "https://attacker.trafficmanager.net", + attachmentId: "att-1", + tokenProvider: buildTokenProvider(), + maxBytes: 10_000_000, + fetchFn, + resolveFn: resolvePublicHost, + }); + + expect(media).toBeUndefined(); + expect(seenAuth).toEqual([null]); + expect(runtime.saveCalls).toHaveLength(0); + }); + + it("sends Bot Framework service tokens to auth-allowlisted service hosts", async () => { + const seenAuth: Array = []; + const fileBytes = Buffer.from("BFBYTES", "utf-8"); + const fetchFn: typeof fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = + typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + seenAuth.push(new Headers(init?.headers).get("authorization")); + if (url.endsWith("/v3/attachments/att-1")) { + return new Response( + JSON.stringify({ + name: "doc.pdf", + type: "application/pdf", + views: [{ viewId: "original", size: fileBytes.byteLength }], + }), + { status: 200, headers: { "content-type": "application/json" } }, + ); + } + if (url.endsWith("/v3/attachments/att-1/views/original")) { + return new Response(fileBytes, { + status: 200, + headers: { "content-length": String(fileBytes.byteLength) }, + }); + } + return new Response("not found", { status: 404 }); + }) as typeof fetch; + + const media = await downloadMSTeamsBotFrameworkAttachment({ + serviceUrl: "https://smba.trafficmanager.net/amer", + attachmentId: "att-1", + tokenProvider: buildTokenProvider(), + maxBytes: 10_000_000, + fetchFn, + resolveFn: resolvePublicHost, + }); + + expect(media?.path).toBe(runtime.savePath); + expect(seenAuth).toEqual(["Bearer bf-token", "Bearer bf-token"]); + }); + it("skips when attachment view size exceeds maxBytes", async () => { const info = { name: "huge.bin", diff --git a/extensions/msteams/src/attachments/bot-framework.ts b/extensions/msteams/src/attachments/bot-framework.ts index a1ebe475d27..d48d0d9c5ff 100644 --- a/extensions/msteams/src/attachments/bot-framework.ts +++ b/extensions/msteams/src/attachments/bot-framework.ts @@ -1,6 +1,7 @@ import { getMSTeamsRuntime } from "../runtime.js"; import { ensureUserAgentHeader } from "../user-agent.js"; import { + applyAuthorizationHeaderForUrl, inferPlaceholder, isUrlAllowed, type MSTeamsAttachmentDownloadLogger, @@ -53,6 +54,21 @@ function normalizeServiceUrl(serviceUrl: string): string { return serviceUrl.replace(/\/+$/, ""); } +function buildBotFrameworkAttachmentHeaders(params: { + url: string; + accessToken: string; + policy: MSTeamsAttachmentFetchPolicy; +}): Headers { + const headers = ensureUserAgentHeader(); + applyAuthorizationHeaderForUrl({ + headers, + url: params.url, + authAllowHosts: params.policy.authAllowHosts, + bearerToken: params.accessToken, + }); + return headers; +} + async function fetchBotFrameworkAttachmentInfo(params: { serviceUrl: string; attachmentId: string; @@ -78,7 +94,11 @@ async function fetchBotFrameworkAttachmentInfo(params: { fetchFn: params.fetchFn, resolveFn: params.resolveFn, requestInit: { - headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }), + headers: buildBotFrameworkAttachmentHeaders({ + url, + accessToken: params.accessToken, + policy: params.policy, + }), }, }); } catch (err) { @@ -128,7 +148,11 @@ async function saveBotFrameworkAttachmentView(params: { fetchFn: params.fetchFn, resolveFn: params.resolveFn, requestInit: { - headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }), + headers: buildBotFrameworkAttachmentHeaders({ + url, + accessToken: params.accessToken, + policy: params.policy, + }), }, }); } catch (err) { diff --git a/extensions/msteams/src/attachments/shared.test.ts b/extensions/msteams/src/attachments/shared.test.ts index cba4aae347d..c9ed6671624 100644 --- a/extensions/msteams/src/attachments/shared.test.ts +++ b/extensions/msteams/src/attachments/shared.test.ts @@ -356,7 +356,7 @@ describe("safeFetch", () => { const fetchMock = vi.fn(async (url: string, init?: RequestInit) => { const auth = new Headers(init?.headers).get("authorization") ?? ""; seenAuth.push(`${url}|${auth}`); - if (url === "https://teams.sharepoint.com/file.pdf") { + if (url === "https://graph.microsoft.com/v1.0/me/photo") { return new Response(null, { status: 302, headers: { location: "https://cdn.sharepoint.com/storage/file.pdf" }, @@ -367,8 +367,8 @@ describe("safeFetch", () => { const headers = new Headers({ Authorization: "Bearer secret" }); const res = await safeFetch({ - url: "https://teams.sharepoint.com/file.pdf", - allowHosts: ["sharepoint.com"], + url: "https://graph.microsoft.com/v1.0/me/photo", + allowHosts: ["graph.microsoft.com", "sharepoint.com"], authorizationAllowHosts: ["graph.microsoft.com"], fetchFn: fetchMock as unknown as typeof fetch, requestInit: { headers }, @@ -378,6 +378,27 @@ describe("safeFetch", () => { expect(seenAuth[0]).toContain("Bearer secret"); expect(seenAuth[1]).toMatch(/\|$/); }); + + it("strips authorization from the initial fetch outside auth allowlist", async () => { + const seenAuth: string[] = []; + const fetchMock = vi.fn(async (url: string, init?: RequestInit) => { + seenAuth.push(new Headers(init?.headers).get("authorization") ?? ""); + expect(url).toBe("https://attacker.trafficmanager.net/v3/attachments/att-1"); + return new Response("ok", { status: 200 }); + }); + + const res = await safeFetch({ + url: "https://attacker.trafficmanager.net/v3/attachments/att-1", + allowHosts: ["trafficmanager.net"], + authorizationAllowHosts: ["smba.trafficmanager.net"], + fetchFn: fetchMock as unknown as typeof fetch, + requestInit: { headers: { Authorization: "Bearer secret" } }, + resolveFn: publicResolve, + }); + + expect(res.status).toBe(200); + expect(seenAuth).toEqual([""]); + }); }); describe("attachment fetch auth helpers", () => { diff --git a/extensions/msteams/src/attachments/shared.ts b/extensions/msteams/src/attachments/shared.ts index 9b6d100fe9a..068688cb6bf 100644 --- a/extensions/msteams/src/attachments/shared.ts +++ b/extensions/msteams/src/attachments/shared.ts @@ -571,6 +571,16 @@ export async function safeFetch(params: { throw new Error(`Initial download URL blocked: ${currentUrl}`); } + // Authorization is only allowed on explicitly auth-allowlisted hosts, including + // the first hop. Redirect hops apply the same rule below before following. + if ( + currentHeaders.has("authorization") && + params.authorizationAllowHosts && + !isUrlAllowed(currentUrl, params.authorizationAllowHosts) + ) { + currentHeaders.delete("authorization"); + } + if (resolveFn) { try { const initialHost = new URL(currentUrl).hostname; diff --git a/extensions/msteams/src/monitor.lifecycle.test.ts b/extensions/msteams/src/monitor.lifecycle.test.ts index 9b9457b5dde..80b211739d8 100644 --- a/extensions/msteams/src/monitor.lifecycle.test.ts +++ b/extensions/msteams/src/monitor.lifecycle.test.ts @@ -304,7 +304,7 @@ describe("monitorMSTeamsProvider lifecycle", () => { ).rejects.toThrow(/EADDRINUSE/); }); - it("runs JWT validation before JSON body parsing", async () => { + it("parses bounded JSON after the Bearer gate and binds serviceUrl during JWT validation", async () => { const abort = new AbortController(); const task = monitorMSTeamsProvider({ cfg: createConfig(0), @@ -322,23 +322,41 @@ describe("monitorMSTeamsProvider lifecycle", () => { if (!app) { throw new Error("expected Express app to be created"); } + // This test intentionally locks auth middleware ordering: the cheap Bearer + // gate must run before bounded JSON parsing, and JWT validation must run + // after parsing so it can bind the token to Activity.serviceUrl. expect(app.use).toHaveBeenCalledTimes(4); const jsonMiddleware = vi.mocked((await import("express")).json).mock.results[0]?.value; if (typeof jsonMiddleware !== "function") { throw new Error("expected Express JSON middleware"); } - expect(readMockCallArg(app.use, 1, 0)).not.toBe(jsonMiddleware); - expect(readMockCallArg(app.use, 2, 0)).toBe(jsonMiddleware); + expect(readMockCallArg(app.use, 1, 0)).toBe(jsonMiddleware); - const jwtMiddleware = readMockCallArg(app.use, 1, 0) as ( + const authGate = readMockCallArg(app.use, 0, 0) as ( + req: Request, + res: Response, + next: (err?: unknown) => void, + ) => void; + const authNext = vi.fn(); + const unauthorizedResponse = { + status: vi.fn().mockReturnThis(), + json: vi.fn(), + } as unknown as Response; + authGate({ headers: {} } as Request, unauthorizedResponse, authNext); + expect(authNext).not.toHaveBeenCalled(); + + const jwtMiddleware = readMockCallArg(app.use, 3, 0) as ( req: Request, res: Response, next: (err?: unknown) => void, ) => void; const next = vi.fn(); jwtMiddleware( - { headers: { authorization: "Bearer token" } } as Request, + { + headers: { authorization: "Bearer token" }, + body: { serviceUrl: "https://smba.trafficmanager.net/amer/" }, + } as Request, { status: vi.fn().mockReturnThis(), json: vi.fn(), @@ -347,10 +365,34 @@ describe("monitorMSTeamsProvider lifecycle", () => { ); await vi.waitFor(() => { - expect(jwtValidate).toHaveBeenCalledWith("Bearer token"); + expect(jwtValidate).toHaveBeenCalledWith( + "Bearer token", + "https://smba.trafficmanager.net/amer/", + ); expect(next).toHaveBeenCalledTimes(1); }); + jwtValidate.mockReset().mockResolvedValueOnce(false); + const missingServiceUrlNext = vi.fn(); + const missingServiceUrlResponse = { + status: vi.fn().mockReturnThis(), + json: vi.fn(), + } as unknown as Response; + jwtMiddleware( + { + headers: { authorization: "Bearer token-no-service-url" }, + body: { type: "message" }, + } as Request, + missingServiceUrlResponse, + missingServiceUrlNext, + ); + + await vi.waitFor(() => { + expect(jwtValidate).toHaveBeenCalledWith("Bearer token-no-service-url", undefined); + expect(missingServiceUrlResponse.status).toHaveBeenCalledWith(401); + expect(missingServiceUrlNext).not.toHaveBeenCalled(); + }); + abort.abort(); await task; }); diff --git a/extensions/msteams/src/monitor.ts b/extensions/msteams/src/monitor.ts index a2ecb2aa665..0d9ac952535 100644 --- a/extensions/msteams/src/monitor.ts +++ b/extensions/msteams/src/monitor.ts @@ -44,6 +44,15 @@ type MonitorMSTeamsResult = { }; const MSTEAMS_WEBHOOK_MAX_BODY_BYTES = DEFAULT_WEBHOOK_MAX_BODY_BYTES; + +function getActivityServiceUrl(body: unknown): string | undefined { + if (!body || typeof body !== "object" || Array.isArray(body)) { + return undefined; + } + const serviceUrl = (body as { serviceUrl?: unknown }).serviceUrl; + return typeof serviceUrl === "string" ? serviceUrl : undefined; +} + export async function monitorMSTeamsProvider( opts: MonitorMSTeamsOpts, ): Promise { @@ -299,16 +308,27 @@ export async function monitorMSTeamsProvider( next(); }); - // JWT validation — verify Bot Framework tokens using the Teams SDK's - // JwtValidator (validates signature via JWKS, audience, issuer, expiration). + // Microsoft requires the JWT serviceurl claim to match the Activity body. + // Keep the cheap Bearer gate above, then parse the bounded JSON payload + // before full JWT validation so the service URL is authenticated. + expressApp.use(express.json({ limit: MSTEAMS_WEBHOOK_MAX_BODY_BYTES })); + expressApp.use((err: unknown, _req: Request, res: Response, next: (err?: unknown) => void) => { + if (err && typeof err === "object" && "status" in err && err.status === 413) { + res.status(413).json({ error: "Payload too large" }); + return; + } + next(err); + }); + + // JWT validation — verify Bot Framework tokens using jsonwebtoken + JWKS, + // including the Microsoft serviceUrl claim binding. const jwtValidator = await createBotFrameworkJwtValidator(creds); expressApp.use((req: Request, res: Response, next: (err?: unknown) => void) => { // Authorization header is guaranteed by the pre-parse auth gate above. - // `serviceUrl` is optional, so authenticate from headers alone before body - // I/O to avoid spending memory and CPU on unauthenticated requests. const authHeader = req.headers.authorization!; + const activityServiceUrl = getActivityServiceUrl(req.body); jwtValidator - .validate(authHeader) + .validate(authHeader, activityServiceUrl) .then((valid) => { if (!valid) { log.debug?.("JWT validation failed"); @@ -339,15 +359,6 @@ export async function monitorMSTeamsProvider( }); }); - expressApp.use(express.json({ limit: MSTEAMS_WEBHOOK_MAX_BODY_BYTES })); - expressApp.use((err: unknown, _req: Request, res: Response, next: (err?: unknown) => void) => { - if (err && typeof err === "object" && "status" in err && err.status === 413) { - res.status(413).json({ error: "Payload too large" }); - return; - } - next(err); - }); - // Set up the messages endpoint - use configured path and /api/messages as fallback const configuredPath = msteamsCfg.webhook?.path ?? "/api/messages"; const messageHandler = (req: Request, res: Response) => { diff --git a/extensions/msteams/src/sdk.test.ts b/extensions/msteams/src/sdk.test.ts index 88c3bf85258..789ea5d2a8c 100644 --- a/extensions/msteams/src/sdk.test.ts +++ b/extensions/msteams/src/sdk.test.ts @@ -58,7 +58,7 @@ 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 } | string | null, - verifyResult: { sub: "ok" } as unknown, + verifyResult: { sub: "ok", serviceurl: "https://smba.trafficmanager.net/amer/" } as unknown, verifyCalls: [] as Array<{ token: string; options: unknown }>, })); @@ -133,7 +133,7 @@ afterEach(() => { jwtState.verifyBehavior = "success"; jwtState.decodedHeader = { kid: "key-1" }; jwtState.decodedPayload = { iss: "https://api.botframework.com" }; - jwtState.verifyResult = { sub: "ok" }; + jwtState.verifyResult = { sub: "ok", serviceurl: "https://smba.trafficmanager.net/amer/" }; vi.restoreAllMocks(); }); @@ -328,6 +328,7 @@ describe("createMSTeamsAdapter", () => { }); describe("createBotFrameworkJwtValidator", () => { + const activityServiceUrl = "https://smba.trafficmanager.net/amer"; const creds = { appId: "app-id", type: "secret", @@ -339,7 +340,7 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.decodedPayload = { iss: "https://api.botframework.com" }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-bf")).resolves.toBe(true); + await expect(validator.validate("Bearer token-bf", activityServiceUrl)).resolves.toBe(true); expect(jwtState.verifyCalls).toHaveLength(1); const opts = jwtState.verifyCalls[0]?.options as Record; @@ -354,24 +355,38 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.verifyResult = { aud: ["https://api.botframework.com"], appid: creds.appId, + serviceurl: activityServiceUrl, }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer botfw-token")).resolves.toBe(true); + await expect(validator.validate("Bearer botfw-token", activityServiceUrl)).resolves.toBe(true); const opts = jwtState.verifyCalls[0]?.options as Record; expect(opts.audience).toContain("https://api.botframework.com"); }); + it("accepts tokens with documented serviceUrl claim casing", async () => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceUrl: activityServiceUrl, + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", activityServiceUrl)).resolves.toBe(true); + }); + it("accepts global audience tokens when azp matches the configured app id", async () => { jwtState.decodedPayload = { iss: "https://api.botframework.com" }; jwtState.verifyResult = { aud: ["https://api.botframework.com"], azp: "APP-ID", + serviceurl: activityServiceUrl, }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer botfw-token-azp")).resolves.toBe(true); + await expect(validator.validate("Bearer botfw-token-azp", activityServiceUrl)).resolves.toBe( + true, + ); }); it("rejects global audience tokens when app binding does not match the configured app id", async () => { @@ -379,10 +394,112 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.verifyResult = { aud: ["https://api.botframework.com"], azp: "other-app-id", + serviceurl: activityServiceUrl, }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer botfw-token-wrong-app")).resolves.toBe(false); + await expect( + validator.validate("Bearer botfw-token-wrong-app", activityServiceUrl), + ).resolves.toBe(false); + }); + + it("rejects tokens when the serviceurl claim does not match the activity serviceUrl", async () => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceurl: "https://attacker.trafficmanager.net/amer", + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", activityServiceUrl)).resolves.toBe(false); + }); + + it("rejects schemeless activity serviceUrls even when the host matches the token claim", async () => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceurl: activityServiceUrl, + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect( + validator.validate("Bearer botfw-token", "smba.trafficmanager.net/amer/"), + ).resolves.toBe(false); + }); + + it("rejects tokens when the serviceurl claim is missing", async () => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + sub: "ok", + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", activityServiceUrl)).resolves.toBe(false); + }); + + it("rejects tokens when the activity serviceUrl is missing", async () => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceurl: activityServiceUrl, + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", undefined)).resolves.toBe(false); + }); + + it("rejects tokens when the activity serviceUrl is malformed", async () => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceurl: activityServiceUrl, + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", "not a url")).resolves.toBe(false); + }); + + it.each([ + "http://smba.trafficmanager.net/amer", + "HTTP://smba.trafficmanager.net/amer", + "wss://smba.trafficmanager.net/amer", + "ftp://smba.trafficmanager.net/amer", + ])("rejects non-HTTPS activity serviceUrl %s", async (serviceUrl) => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceurl: serviceUrl, + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", serviceUrl)).resolves.toBe(false); + }); + + it("rejects serviceUrl values with query strings", async () => { + const queriedServiceUrl = `${activityServiceUrl}?target=attacker`; + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceurl: queriedServiceUrl, + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", queriedServiceUrl)).resolves.toBe(false); + }); + + it("rejects serviceUrl values with fragments", async () => { + const fragmentServiceUrl = `${activityServiceUrl}#fragment`; + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceurl: fragmentServiceUrl, + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", fragmentServiceUrl)).resolves.toBe(false); + }); + + it("rejects tokens when the serviceurl claim is not a string", async () => { + jwtState.decodedPayload = { iss: "https://api.botframework.com" }; + jwtState.verifyResult = { + serviceurl: 123, + }; + + const validator = await createBotFrameworkJwtValidator(creds); + await expect(validator.validate("Bearer botfw-token", activityServiceUrl)).resolves.toBe(false); }); it("rejects non-object verified payloads", async () => { @@ -390,14 +507,16 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.verifyResult = "verified-string-payload"; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer botfw-token-string")).resolves.toBe(false); + await expect(validator.validate("Bearer botfw-token-string", activityServiceUrl)).resolves.toBe( + false, + ); }); 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-entra")).resolves.toBe(true); + await expect(validator.validate("Bearer token-entra", activityServiceUrl)).resolves.toBe(true); expect(jwtState.verifyCalls).toHaveLength(1); const opts = jwtState.verifyCalls[0]?.options as Record; @@ -413,7 +532,7 @@ describe("createBotFrameworkJwtValidator", () => { }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-sts")).resolves.toBe(true); + await expect(validator.validate("Bearer token-sts", activityServiceUrl)).resolves.toBe(true); expect(jwtState.verifyCalls).toHaveLength(1); const opts = jwtState.verifyCalls[0]?.options as Record; @@ -429,7 +548,9 @@ describe("createBotFrameworkJwtValidator", () => { }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-sts-other-tenant")).resolves.toBe(false); + await expect( + validator.validate("Bearer token-sts-other-tenant", activityServiceUrl), + ).resolves.toBe(false); expect(jwtState.verifyCalls).toHaveLength(0); }); @@ -437,7 +558,7 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.decodedPayload = { iss: "https://evil.example.com" }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-evil")).resolves.toBe(false); + await expect(validator.validate("Bearer token-evil", activityServiceUrl)).resolves.toBe(false); expect(jwtState.verifyCalls).toHaveLength(0); }); @@ -445,12 +566,12 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.verifyBehavior = "throw"; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-bad")).resolves.toBe(false); + await expect(validator.validate("Bearer token-bad", activityServiceUrl)).resolves.toBe(false); }); it("returns false for empty bearer token", async () => { const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer ")).resolves.toBe(false); + await expect(validator.validate("Bearer ", activityServiceUrl)).resolves.toBe(false); expect(jwtState.verifyCalls).toHaveLength(0); }); @@ -458,7 +579,7 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.decodedHeader = { kid: undefined }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer no-kid")).resolves.toBe(false); + await expect(validator.validate("Bearer no-kid", activityServiceUrl)).resolves.toBe(false); expect(jwtState.verifyCalls).toHaveLength(0); }); @@ -466,7 +587,7 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.decodedPayload = { iss: undefined }; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer no-iss")).resolves.toBe(false); + await expect(validator.validate("Bearer no-iss", activityServiceUrl)).resolves.toBe(false); expect(jwtState.verifyCalls).toHaveLength(0); }); @@ -484,7 +605,9 @@ describe("createBotFrameworkJwtValidator", () => { const validator = await createBotFrameworkJwtValidator(creds); // Network errors must bubble out — callers can then log them at warn/error // level rather than silently returning 401 that looks like a bad credential. - await expect(validator.validate("Bearer token-firewall")).rejects.toThrow("ECONNREFUSED"); + await expect(validator.validate("Bearer token-firewall", activityServiceUrl)).rejects.toThrow( + "ECONNREFUSED", + ); }); it("returns false (not throws) for non-network JWKS errors like bad signature (#77674)", async () => { @@ -492,7 +615,9 @@ describe("createBotFrameworkJwtValidator", () => { jwtState.decodedPayload = { iss: "https://api.botframework.com" }; jwtState.verifyBehavior = "throw"; const validator = await createBotFrameworkJwtValidator(creds); - await expect(validator.validate("Bearer token-bad-sig")).resolves.toBe(false); + await expect(validator.validate("Bearer token-bad-sig", activityServiceUrl)).resolves.toBe( + false, + ); }); }); diff --git a/extensions/msteams/src/sdk.ts b/extensions/msteams/src/sdk.ts index 98170e78740..9a424e729b4 100644 --- a/extensions/msteams/src/sdk.ts +++ b/extensions/msteams/src/sdk.ts @@ -682,9 +682,16 @@ type JwksClientCtor = BotFrameworkJwtDeps["JwksClient"]; const BOT_FRAMEWORK_GLOBAL_AUDIENCE = "https://api.botframework.com"; -function isJwtPayloadObject( - value: unknown, -): value is { iss?: unknown; aud?: unknown; appid?: unknown; azp?: unknown } { +type BotFrameworkJwtPayload = { + iss?: unknown; + aud?: unknown; + appid?: unknown; + azp?: unknown; + serviceurl?: unknown; + serviceUrl?: unknown; +}; + +function isJwtPayloadObject(value: unknown): value is BotFrameworkJwtPayload { return !!value && typeof value === "object" && !Array.isArray(value); } @@ -727,6 +734,43 @@ function hasExpectedBotIdentity(payload: unknown, expectedAppId: string): boolea ); } +function validateAndNormalizeBotFrameworkServiceUrl(value: unknown): string | null { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + if (!trimmed) { + return null; + } + try { + const url = new URL(trimmed); + // Match the signed endpoint, not a loosely equivalent URL: the URL parser + // normalizes host/default port, while path casing and encoding stay intact. + // Query/fragment values are not valid Bot Framework service endpoints. + if (url.protocol !== "https:" || url.search || url.hash) { + return null; + } + return url.toString().replace(/\/+$/, ""); + } catch { + return null; + } +} + +function hasMatchingServiceUrlClaim( + payload: BotFrameworkJwtPayload, + activityServiceUrl: string | undefined, +): boolean { + const expectedServiceUrl = validateAndNormalizeBotFrameworkServiceUrl(activityServiceUrl); + if (!expectedServiceUrl) { + return false; + } + // Bot Framework tokens commonly use lowercase `serviceurl`; keep the + // documented camelCase spelling as a narrow fallback for SDK/source variants. + const claimValue = payload.serviceurl ?? payload.serviceUrl; + const claimServiceUrl = validateAndNormalizeBotFrameworkServiceUrl(claimValue); + return claimServiceUrl === expectedServiceUrl; +} + let botFrameworkJwtDepsPromise: Promise | null = null; function hasDefaultExport(value: unknown): value is { default?: unknown } { @@ -794,10 +838,11 @@ async function loadBotFrameworkJwtDeps(): Promise { * - 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) + * - service URL binding: JWT serviceurl claim must match a usable Activity.serviceUrl * - expiration validation with 5-minute clock tolerance */ export async function createBotFrameworkJwtValidator(creds: MSTeamsCredentials): Promise<{ - validate: (authHeader: string) => Promise; + validate: (authHeader: string, activityServiceUrl?: string) => Promise; }> { const { jwt, JwksClient } = await loadBotFrameworkJwtDeps(); @@ -846,7 +891,7 @@ export async function createBotFrameworkJwtValidator(creds: MSTeamsCredentials): } return { - async validate(authHeader: string, _serviceUrl?: string): Promise { + async validate(authHeader: string, activityServiceUrl: string | undefined): Promise { const token = authHeader.startsWith("Bearer ") ? authHeader.slice(7) : authHeader; if (!token) { return false; @@ -882,6 +927,9 @@ export async function createBotFrameworkJwtValidator(creds: MSTeamsCredentials): if (!isJwtPayloadObject(verifiedPayload)) { return false; } + if (!hasMatchingServiceUrlClaim(verifiedPayload, activityServiceUrl)) { + return false; + } const audiences = getAudienceClaims(verifiedPayload); if ( audiences.includes(BOT_FRAMEWORK_GLOBAL_AUDIENCE) &&