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 <bradgroux@users.noreply.github.com>
2026-04-12 09:41:11 +00:00 · 2026-04-09 20:05:54 -07:00
parent 95d467398e
commit a59a9bfb07
4 changed files with 236 additions and 152 deletions
--- 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": {
--- 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<string, unknown> }>,
-  behaviorByJwks: new Map<string, "success" | "null" | "throw">(),
-  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<string, unknown>;
+// 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<string, unknown>) {
-      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<object | null> {
-      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<string, unknown>;
+    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<string, unknown>;
+    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<string, unknown>;
+    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<string, unknown>;
+    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);
  });
 });
--- 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<boolean>;
+  validate: (authHeader: string) => Promise<boolean>;
 }> {
-  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<boolean> {
-    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<string, InstanceType<typeof JwksClient>>();
+  function getJwksClient(uri: string): InstanceType<typeof JwksClient> {
+    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<boolean> {
+    async validate(authHeader: string, _serviceUrl?: string): Promise<boolean> {
      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;
+      }
    },
  };
 }
--- 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: