Skip to content

Commit bb797ba

Browse files
hugenshenhugenshen
committed
fix(msteams): bound token fetch preflight with guard timeoutMs
1 parent f07a1fb commit bb797ba

3 files changed

Lines changed: 109 additions & 18 deletions

File tree

‎extensions/msteams/src/oauth.test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const fetchWithSsrFGuardMock = vi.hoisted(() =>
77
async (params: {
88
url: string;
99
init?: RequestInit;
10+
timeoutMs?: number;
1011
fetchImpl?: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
1112
}) => {
1213
const fetchImpl = params.fetchImpl ?? globalThis.fetch;
@@ -32,6 +33,7 @@ import {
3233
} from "./oauth.flow.js";
3334
import {
3435
MSTEAMS_DEFAULT_DELEGATED_SCOPES,
36+
MSTEAMS_DEFAULT_TOKEN_FETCH_TIMEOUT_MS,
3537
MSTEAMS_OAUTH_REDIRECT_URI,
3638
buildMSTeamsAuthEndpoint,
3739
buildMSTeamsTokenEndpoint,
@@ -215,7 +217,10 @@ describe("exchangeMSTeamsCodeForTokens", () => {
215217
expect(body.get("code")).toBe("auth-code");
216218
expect(body.get("code_verifier")).toBe("pkce-verifier");
217219
expect(body.get("redirect_uri")).toBe(MSTEAMS_OAUTH_REDIRECT_URI);
218-
expect(fetchWithSsrFGuardMock.mock.calls[0]?.[0]).not.toHaveProperty("fetchImpl");
220+
const guardCall = fetchWithSsrFGuardMock.mock.calls[0]?.[0];
221+
expect(guardCall).not.toHaveProperty("fetchImpl");
222+
expect(guardCall?.timeoutMs).toBe(MSTEAMS_DEFAULT_TOKEN_FETCH_TIMEOUT_MS);
223+
expect(guardCall?.init?.signal).toBeUndefined();
219224
});
220225

221226
it("throws on a 400 error response", async () => {
@@ -318,6 +323,9 @@ describe("refreshMSTeamsDelegatedTokens", () => {
318323
expect(body.get("grant_type")).toBe("refresh_token");
319324
expect(body.get("refresh_token")).toBe("original-rt");
320325
expect(body.get("client_secret")).toBe("secret-1");
326+
const guardCall = fetchWithSsrFGuardMock.mock.calls.at(-1)?.[0];
327+
expect(guardCall?.timeoutMs).toBe(MSTEAMS_DEFAULT_TOKEN_FETCH_TIMEOUT_MS);
328+
expect(guardCall?.init?.signal).toBeUndefined();
321329
});
322330

323331
it("uses new refresh token when Azure returns one", async () => {

‎extensions/msteams/src/oauth.token.preflight-timeout.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Real guarded-fetch path: no fetchWithSsrFGuard mock.
2+
// Locks guard-owned timeoutMs at the refreshMSTeamsDelegatedTokens entry point.
3+
// Shared SSRF suites cover stalled DNS; this asserts Teams token refresh still
4+
// forwards timeoutMs into that owner so preflight abort happens before HTTP
5+
// dispatch. Do not rewrite this to AbortSignal.timeout() / init.signal — that
6+
// would regress the guard-owned contract (#105549). Sibling graph.ts already
7+
// passes top-level timeoutMs.
8+
import type { LookupFn } from "openclaw/plugin-sdk/ssrf-runtime";
9+
import { describe, expect, it, vi } from "vitest";
10+
import { refreshMSTeamsDelegatedTokens } from "./oauth.token.js";
11+
12+
describe("refreshMSTeamsDelegatedTokens preflight timeout", () => {
13+
it("times out when preflight lookup stalls before HTTP dispatch", async () => {
14+
const stalledLookup: LookupFn = (() => new Promise<never>(() => {})) as LookupFn;
15+
const fetchSpy = vi.fn(async () => new Response("should not run"));
16+
17+
const started = Date.now();
18+
const outcome = await refreshMSTeamsDelegatedTokens({
19+
tenantId: "tenant-1",
20+
clientId: "client-1",
21+
clientSecret: "secret-1", // pragma: allowlist secret
22+
refreshToken: "original-rt",
23+
fetchImpl: fetchSpy,
24+
lookupFn: stalledLookup,
25+
timeoutMs: 80,
26+
}).then(
27+
(value) => ({ ok: true as const, value }),
28+
(error: unknown) => ({ ok: false as const, error }),
29+
);
30+
const elapsedMs = Date.now() - started;
31+
32+
expect(outcome.ok).toBe(false);
33+
if (!outcome.ok) {
34+
expect(outcome.error).toMatchObject({
35+
name: "TimeoutError",
36+
message: "request timed out",
37+
});
38+
}
39+
expect(elapsedMs).toBeGreaterThanOrEqual(60);
40+
expect(elapsedMs).toBeLessThan(2_000);
41+
expect(fetchSpy).not.toHaveBeenCalled();
42+
console.log(
43+
`[msteams token refresh preflight stall proof] timed_out=${!outcome.ok} name=${
44+
outcome.ok ? "n/a" : (outcome.error as Error).name
45+
} message=${
46+
outcome.ok ? "n/a" : (outcome.error as Error).message
47+
} elapsed_ms=${elapsedMs} fetch_called=${fetchSpy.mock.calls.length}`,
48+
);
49+
});
50+
});

‎extensions/msteams/src/oauth.token.ts‎

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Msteams plugin module implements oauth.token behavior.
22
import { resolveExpiresAtMsFromDurationSeconds } from "openclaw/plugin-sdk/number-runtime";
33
import { readProviderJsonResponse } from "openclaw/plugin-sdk/provider-http";
4-
import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime";
4+
import { fetchWithSsrFGuard, type LookupFn } from "openclaw/plugin-sdk/ssrf-runtime";
55
import { createMSTeamsHttpError } from "./http-error.js";
66
import {
77
MSTEAMS_DEFAULT_DELEGATED_SCOPES,
@@ -21,6 +21,15 @@ type MSTeamsTokenResponse = {
2121
scope?: string;
2222
};
2323

24+
type MSTeamsTokenFetch = (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
25+
26+
/** Optional test hooks so token fetch can exercise the real guarded-fetch owner. */
27+
type MSTeamsTokenFetchDeps = {
28+
fetchImpl?: MSTeamsTokenFetch;
29+
lookupFn?: LookupFn;
30+
timeoutMs?: number;
31+
};
32+
2433
function createMSTeamsTokenBody(params: {
2534
clientId: string;
2635
clientSecret: string;
@@ -74,7 +83,13 @@ async function fetchMSTeamsTokens(params: {
7483
body: URLSearchParams;
7584
auditContext: string;
7685
failureLabel: string;
86+
fetchImpl?: MSTeamsTokenFetch;
87+
lookupFn?: LookupFn;
88+
timeoutMs?: number;
7789
}): Promise<MSTeamsTokenResponse> {
90+
// Guard-owned timeoutMs covers DNS/proxy preflight; init.signal alone does not.
91+
// Sibling graph.ts already passes top-level timeoutMs into fetchWithSsrFGuard.
92+
const timeoutMs = params.timeoutMs ?? MSTEAMS_DEFAULT_TOKEN_FETCH_TIMEOUT_MS;
7893
const { response, release } = await fetchWithSsrFGuard({
7994
url: params.tokenUrl,
8095
init: {
@@ -84,9 +99,11 @@ async function fetchMSTeamsTokens(params: {
8499
Accept: "application/json",
85100
},
86101
body: params.body,
87-
signal: AbortSignal.timeout(MSTEAMS_DEFAULT_TOKEN_FETCH_TIMEOUT_MS),
88102
},
89103
auditContext: params.auditContext,
104+
timeoutMs,
105+
...(params.fetchImpl ? { fetchImpl: params.fetchImpl } : {}),
106+
...(params.lookupFn ? { lookupFn: params.lookupFn } : {}),
90107
});
91108

92109
try {
@@ -113,6 +130,9 @@ async function requestMSTeamsDelegatedTokens(params: {
113130
auditContext: string;
114131
failureLabel: string;
115132
resolveRefreshToken: (data: MSTeamsTokenResponse) => string;
133+
fetchImpl?: MSTeamsTokenFetch;
134+
lookupFn?: LookupFn;
135+
timeoutMs?: number;
116136
}): Promise<MSTeamsDelegatedTokens> {
117137
const scopes = params.scopes ?? MSTEAMS_DEFAULT_DELEGATED_SCOPES;
118138
const body = createMSTeamsTokenBody({
@@ -127,6 +147,9 @@ async function requestMSTeamsDelegatedTokens(params: {
127147
body,
128148
auditContext: params.auditContext,
129149
failureLabel: params.failureLabel,
150+
fetchImpl: params.fetchImpl,
151+
lookupFn: params.lookupFn,
152+
timeoutMs: params.timeoutMs,
130153
});
131154

132155
return {
@@ -137,14 +160,16 @@ async function requestMSTeamsDelegatedTokens(params: {
137160
};
138161
}
139162

140-
export async function exchangeMSTeamsCodeForTokens(params: {
141-
tenantId: string;
142-
clientId: string;
143-
clientSecret: string;
144-
code: string;
145-
verifier: string;
146-
scopes?: readonly string[];
147-
}): Promise<MSTeamsDelegatedTokens> {
163+
export async function exchangeMSTeamsCodeForTokens(
164+
params: {
165+
tenantId: string;
166+
clientId: string;
167+
clientSecret: string;
168+
code: string;
169+
verifier: string;
170+
scopes?: readonly string[];
171+
} & MSTeamsTokenFetchDeps,
172+
): Promise<MSTeamsDelegatedTokens> {
148173
return await requestMSTeamsDelegatedTokens({
149174
tenantId: params.tenantId,
150175
clientId: params.clientId,
@@ -164,16 +189,21 @@ export async function exchangeMSTeamsCodeForTokens(params: {
164189
}
165190
return data.refresh_token;
166191
},
192+
fetchImpl: params.fetchImpl,
193+
lookupFn: params.lookupFn,
194+
timeoutMs: params.timeoutMs,
167195
});
168196
}
169197

170-
export async function refreshMSTeamsDelegatedTokens(params: {
171-
tenantId: string;
172-
clientId: string;
173-
clientSecret: string;
174-
refreshToken: string;
175-
scopes?: readonly string[];
176-
}): Promise<MSTeamsDelegatedTokens> {
198+
export async function refreshMSTeamsDelegatedTokens(
199+
params: {
200+
tenantId: string;
201+
clientId: string;
202+
clientSecret: string;
203+
refreshToken: string;
204+
scopes?: readonly string[];
205+
} & MSTeamsTokenFetchDeps,
206+
): Promise<MSTeamsDelegatedTokens> {
177207
return await requestMSTeamsDelegatedTokens({
178208
tenantId: params.tenantId,
179209
clientId: params.clientId,
@@ -186,5 +216,8 @@ export async function refreshMSTeamsDelegatedTokens(params: {
186216
auditContext: "msteams-oauth-token-refresh",
187217
failureLabel: "token refresh",
188218
resolveRefreshToken: (data) => data.refresh_token ?? params.refreshToken,
219+
fetchImpl: params.fetchImpl,
220+
lookupFn: params.lookupFn,
221+
timeoutMs: params.timeoutMs,
189222
});
190223
}

0 commit comments

Comments
 (0)