Skip to content

Commit 355794c

Browse files
sudie-codesclaudeBradGroux
authored
msteams: add reaction support with delegated auth and pagination helper (#51646)
* msteams: add reaction support (inbound handlers + outbound Graph API) * msteams: address PR #51646 review feedback * msteams: remove react from advertised actions (requires Delegated auth) * msteams: address PR #51646 remaining review feedback (dmPolicy, groupPolicy, reactions auth) - Fix 1: DM reaction authorization now uses resolveDmGroupAccessWithLists to enforce dmPolicy modes (open/disabled/allowlist/pairing), matching the message handler. - Fix 2: Group policy in reaction handler already uses resolveDefaultGroupPolicy for global defaults; moved declaration earlier to share with DM path. - Fix 3: Restore read-only "reactions" (list) action with listReactionsMSTeams, which uses GET and works with Application auth. Keep "react" (write) gated behind delegated-auth. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * msteams: add shared Graph pagination helper (fetchAllGraphPages) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * msteams: add OAuth2 delegated auth flow (PKCE + authorization code) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * msteams: integrate delegated auth (config, token storage, react enablement) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * msteams: fix critical bugs found in architect review - Fix fetchGraphJson→postGraphJson for setReaction/unsetReaction (was sending GET instead of POST) - Fix CSRF bypass in OAuth parseCallbackInput (missing state no longer falls back silently) - Remove stale delegated-auth warning logs (delegated auth is now implemented) - Add CSRF test case for parseCallbackInput Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * msteams: fix 6 PR #51646 review blockers (PKCE/state separation, CSRF, imports, routing, delegated auth bootstrap) * msteams: fix channel.runtime.ts duplicate imports + graph.ts test mock compat * msteams: fix lint/boundary blockers revealed by CI after rebase - token.ts/graph.test.ts: add curly braces around single-statement ifs (eslint/curly). - oauth.flow.ts: rename unused parseCallbackInput param to _expectedState. - reaction-handler.test.ts: rename unused buildDeps param to _runtime. - send.reactions.ts: drop unnecessary non-null assertions on tuple entries. - setup-surface.ts: drop empty-object spread fallback flagged by unicorn/no-useless-fallback-in-spread. - graph.ts: move GraphPagedResponse/PaginatedResult type defs below requestGraph so the raw fetch() stays on line 47 to match the existing no-raw-channel-fetch allowlist entry. - oauth.token.ts: route the Azure AD token exchange and refresh calls through fetchWithSsrFGuard (matches the pattern in sdk.ts), removing the unguarded raw fetch() callsites flagged by lint:tmp:no-raw-channel-fetch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(msteams): restore absolute Graph pagination helper * fix(msteams): satisfy reaction handler lint --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Brad Groux <3053586+BradGroux@users.noreply.github.com>
1 parent 7c5b42e commit 355794c

19 files changed

Lines changed: 2200 additions & 18 deletions

‎extensions/msteams/src/channel.runtime.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ import {
2525
sendAdaptiveCardMSTeams as sendAdaptiveCardMSTeamsImpl,
2626
sendMessageMSTeams as sendMessageMSTeamsImpl,
2727
} from "./send.js";
28+
// NOTE: reactMessageMSTeams / listReactionsMSTeams / unreactMessageMSTeams are
29+
// imported from ./graph-messages.js above. The channel dispatcher in channel.ts
30+
// calls those signatures (messageId + reactionType), not the send.reactions.ts
31+
// variants. send.reactions.ts remains as a delegated-auth implementation that
32+
// is currently wired through its own test surface; do not re-import it here
33+
// until channel.ts is migrated to that signature, otherwise identifiers collide.
2834
export const msTeamsChannelRuntime = {
2935
deleteMessageMSTeams: deleteMessageMSTeamsImpl,
3036
editMessageMSTeams: editMessageMSTeamsImpl,

‎extensions/msteams/src/graph-messages.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,12 +381,17 @@ function normalizeReactionType(raw: string): string {
381381

382382
/**
383383
* Add an emoji reaction to a message via Graph API (beta).
384+
*
385+
* Writes (setReaction) require a Delegated token, so we pass
386+
* `preferDelegated: true`. The resolver falls back to the app-only token when
387+
* delegated auth is not configured, preserving today's behavior while letting
388+
* delegated-auth-enabled deployments hit the user-scoped endpoint.
384389
*/
385390
export async function reactMessageMSTeams(
386391
params: ReactMessageMSTeamsParams,
387392
): Promise<{ ok: true }> {
388393
const reactionType = normalizeReactionType(params.reactionType);
389-
const token = await resolveGraphToken(params.cfg);
394+
const token = await resolveGraphToken(params.cfg, { preferDelegated: true });
390395
const conversationId = await resolveGraphConversationId(params.to);
391396
const { basePath } = resolveConversationPath(conversationId);
392397
const path = `${basePath}/messages/${encodeURIComponent(params.messageId)}/setReaction`;
@@ -396,12 +401,15 @@ export async function reactMessageMSTeams(
396401

397402
/**
398403
* Remove an emoji reaction from a message via Graph API (beta).
404+
*
405+
* Writes (unsetReaction) require a Delegated token, so we pass
406+
* `preferDelegated: true`. See `reactMessageMSTeams` for fallback rules.
399407
*/
400408
export async function unreactMessageMSTeams(
401409
params: ReactMessageMSTeamsParams,
402410
): Promise<{ ok: true }> {
403411
const reactionType = normalizeReactionType(params.reactionType);
404-
const token = await resolveGraphToken(params.cfg);
412+
const token = await resolveGraphToken(params.cfg, { preferDelegated: true });
405413
const conversationId = await resolveGraphConversationId(params.to);
406414
const { basePath } = resolveConversationPath(conversationId);
407415
const path = `${basePath}/messages/${encodeURIComponent(params.messageId)}/unsetReaction`;

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

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { searchGraphUsers } from "./graph-users.js";
3131
import {
3232
deleteGraphRequest,
3333
escapeOData,
34+
fetchAllGraphPages,
3435
fetchGraphJson,
3536
listChannelsForTeam,
3637
listTeamsByName,
@@ -349,4 +350,167 @@ describe("msteams graph helpers", () => {
349350
"/users?$search=%22displayName%3Acarol%22&$select=id,displayName,mail,userPrincipalName&$top=10",
350351
);
351352
});
353+
354+
describe("fetchAllGraphPages", () => {
355+
type Item = { id: string; name: string };
356+
357+
/** Build a paged Graph response with optional nextLink. */
358+
function pagedResponse(items: Item[], nextLink?: string) {
359+
const body: Record<string, unknown> = { value: items };
360+
if (nextLink) {
361+
body["@odata.nextLink"] = nextLink;
362+
}
363+
return body;
364+
}
365+
366+
it("single page, no nextLink", async () => {
367+
const items = [{ id: "1", name: "a" }];
368+
mockJsonFetchResponse(pagedResponse(items));
369+
370+
const result = await fetchAllGraphPages<Item>({
371+
token: graphToken,
372+
path: "/items",
373+
});
374+
375+
expect(result).toEqual({ items, truncated: false });
376+
expect(globalThis.fetch).toHaveBeenCalledTimes(1);
377+
});
378+
379+
it("multiple pages with nextLink chain", async () => {
380+
const page1Items = [{ id: "1", name: "a" }];
381+
const page2Items = [{ id: "2", name: "b" }];
382+
const page3Items = [{ id: "3", name: "c" }];
383+
let callCount = 0;
384+
385+
mockFetch(async () => {
386+
callCount++;
387+
if (callCount === 1) {
388+
return jsonResponse(
389+
pagedResponse(page1Items, "https://graph.microsoft.com/v1.0/items?$skiptoken=page2"),
390+
);
391+
}
392+
if (callCount === 2) {
393+
return jsonResponse(
394+
pagedResponse(page2Items, "https://graph.microsoft.com/v1.0/items?$skiptoken=page3"),
395+
);
396+
}
397+
return jsonResponse(pagedResponse(page3Items));
398+
});
399+
400+
const result = await fetchAllGraphPages<Item>({
401+
token: graphToken,
402+
path: "/items",
403+
});
404+
405+
expect(result.items).toEqual([...page1Items, ...page2Items, ...page3Items]);
406+
expect(result.truncated).toBe(false);
407+
expect(globalThis.fetch).toHaveBeenCalledTimes(3);
408+
});
409+
410+
it("truncation at maxPages", async () => {
411+
mockFetch(async () =>
412+
jsonResponse(
413+
pagedResponse(
414+
[{ id: "x", name: "x" }],
415+
"https://graph.microsoft.com/v1.0/items?$skiptoken=more",
416+
),
417+
),
418+
);
419+
420+
const result = await fetchAllGraphPages<Item>({
421+
token: graphToken,
422+
path: "/items",
423+
maxPages: 2,
424+
});
425+
426+
expect(result.items).toHaveLength(2);
427+
expect(result.truncated).toBe(true);
428+
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
429+
});
430+
431+
it("findOne early exit", async () => {
432+
const target = { id: "target", name: "found-it" };
433+
let callCount = 0;
434+
435+
mockFetch(async () => {
436+
callCount++;
437+
if (callCount === 1) {
438+
return jsonResponse(
439+
pagedResponse(
440+
[{ id: "1", name: "a" }],
441+
"https://graph.microsoft.com/v1.0/items?$skiptoken=p2",
442+
),
443+
);
444+
}
445+
// Page 2 contains the target; page 3 should never be fetched
446+
return jsonResponse(
447+
pagedResponse(
448+
[{ id: "2", name: "b" }, target],
449+
"https://graph.microsoft.com/v1.0/items?$skiptoken=p3",
450+
),
451+
);
452+
});
453+
454+
const result = await fetchAllGraphPages<Item>({
455+
token: graphToken,
456+
path: "/items",
457+
findOne: (item) => item.id === "target",
458+
});
459+
460+
expect(result.found).toEqual(target);
461+
expect(result.truncated).toBe(false);
462+
// Page 1 items + page 2 items (where match was found)
463+
expect(result.items).toEqual([{ id: "1", name: "a" }, { id: "2", name: "b" }, target]);
464+
// Only 2 fetches; page 3 was never requested
465+
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
466+
});
467+
468+
it("findOne with no match (exhausted)", async () => {
469+
mockJsonFetchResponse(pagedResponse([{ id: "1", name: "a" }]));
470+
471+
const result = await fetchAllGraphPages<Item>({
472+
token: graphToken,
473+
path: "/items",
474+
findOne: (item) => item.id === "missing",
475+
});
476+
477+
expect(result.found).toBeUndefined();
478+
expect(result.truncated).toBe(false);
479+
expect(result.items).toEqual([{ id: "1", name: "a" }]);
480+
});
481+
482+
it("findOne with no match (truncated)", async () => {
483+
mockFetch(async () =>
484+
jsonResponse(
485+
pagedResponse(
486+
[{ id: "x", name: "x" }],
487+
"https://graph.microsoft.com/v1.0/items?$skiptoken=more",
488+
),
489+
),
490+
);
491+
492+
const result = await fetchAllGraphPages<Item>({
493+
token: graphToken,
494+
path: "/items",
495+
maxPages: 2,
496+
findOne: (item) => item.id === "missing",
497+
});
498+
499+
expect(result.found).toBeUndefined();
500+
expect(result.truncated).toBe(true);
501+
expect(result.items).toHaveLength(2);
502+
});
503+
504+
it("empty first page", async () => {
505+
mockJsonFetchResponse(pagedResponse([]));
506+
507+
const result = await fetchAllGraphPages<Item>({
508+
token: graphToken,
509+
path: "/items",
510+
});
511+
512+
expect(result).toEqual({ items: [], truncated: false });
513+
expect(globalThis.fetch).toHaveBeenCalledTimes(1);
514+
});
515+
});
352516
});

‎extensions/msteams/src/graph.ts‎

Lines changed: 101 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,9 @@ async function requestGraph(params: {
6464
}
6565

6666
async function readOptionalGraphJson<T>(res: Response): Promise<T> {
67-
if (res.status === 204 || res.headers.get("content-length") === "0") {
67+
// Use optional chaining to stay resilient to partial test mocks that do not
68+
// provide a status or Headers instance (they only shim `ok` + `json()`).
69+
if (res.status === 204 || res.headers?.get?.("content-length") === "0") {
6870
return undefined as T;
6971
}
7072
return (await res.json()) as T;
@@ -74,21 +76,24 @@ export async function fetchGraphJson<T>(params: {
7476
token: string;
7577
path: string;
7678
headers?: Record<string, string>;
79+
/** HTTP method; defaults to "GET" */
80+
method?: string;
81+
/** Request body (serialized as JSON). Only used for non-GET methods. */
82+
body?: unknown;
7783
}): Promise<T> {
7884
const res = await requestGraph({
7985
token: params.token,
8086
path: params.path,
87+
method: params.method as "GET" | "POST" | "DELETE" | undefined,
88+
body: params.body,
8189
headers: params.headers,
8290
});
83-
return (await res.json()) as T;
91+
return await readOptionalGraphJson<T>(res);
8492
}
8593

8694
/**
87-
* Fetch JSON from an absolute Graph API URL (e.g. @odata.nextLink pagination URLs).
88-
* Unlike {@link fetchGraphJson}, this does not prepend GRAPH_ROOT.
89-
*
90-
* Routed through `fetchWithSsrFGuard` so absolute-URL pagination follow-ups
91-
* honor the same SSRF policy as other Graph calls.
95+
* Fetch JSON from an absolute Graph API URL (for example @odata.nextLink
96+
* pagination URLs) without prepending GRAPH_ROOT.
9297
*/
9398
export async function fetchGraphAbsoluteUrl<T>(params: {
9499
token: string;
@@ -119,13 +124,94 @@ export async function fetchGraphAbsoluteUrl<T>(params: {
119124
}
120125
}
121126

122-
export async function resolveGraphToken(cfg: unknown): Promise<string> {
123-
const creds = resolveMSTeamsCredentials(
124-
(cfg as { channels?: { msteams?: unknown } })?.channels?.msteams as MSTeamsConfig | undefined,
125-
);
127+
/** Graph collection response with optional pagination link. */
128+
export type GraphPagedResponse<T> = {
129+
value?: T[];
130+
"@odata.nextLink"?: string;
131+
};
132+
133+
/** Result of a paginated Graph API fetch. */
134+
export type PaginatedResult<T> = {
135+
items: T[];
136+
truncated: boolean;
137+
found?: T;
138+
};
139+
140+
/**
141+
* Fetch all pages of a Graph API collection, following @odata.nextLink.
142+
* Optionally stop early when `findOne` matches an item.
143+
*/
144+
export async function fetchAllGraphPages<T>(params: {
145+
token: string;
146+
path: string;
147+
headers?: Record<string, string>;
148+
/** Max pages to fetch before stopping. Default: 50. */
149+
maxPages?: number;
150+
/** Stop pagination early when this predicate returns true. */
151+
findOne?: (item: T) => boolean;
152+
}): Promise<PaginatedResult<T>> {
153+
const maxPages = params.maxPages ?? 50;
154+
const items: T[] = [];
155+
let nextPath: string | undefined = params.path;
156+
157+
for (let page = 0; page < maxPages && nextPath; page++) {
158+
const res: GraphPagedResponse<T> = await fetchGraphJson<GraphPagedResponse<T>>({
159+
token: params.token,
160+
path: nextPath,
161+
headers: params.headers,
162+
});
163+
164+
const pageItems = res.value ?? [];
165+
166+
if (params.findOne) {
167+
const match = pageItems.find(params.findOne);
168+
if (match) {
169+
items.push(...pageItems);
170+
return { items, truncated: false, found: match };
171+
}
172+
}
173+
174+
items.push(...pageItems);
175+
176+
// @odata.nextLink is an absolute URL; strip the Graph root to get a relative path
177+
const rawNext: string | undefined = res["@odata.nextLink"];
178+
if (rawNext) {
179+
nextPath = rawNext
180+
.replace("https://graph.microsoft.com/v1.0", "")
181+
.replace("https://graph.microsoft.com/beta", "");
182+
} else {
183+
nextPath = undefined;
184+
}
185+
}
186+
187+
return { items, truncated: Boolean(nextPath) };
188+
}
189+
190+
export async function resolveGraphToken(
191+
cfg: unknown,
192+
options?: { preferDelegated?: boolean },
193+
): Promise<string> {
194+
const msteamsCfg = (cfg as { channels?: { msteams?: MSTeamsConfig } })?.channels?.msteams;
195+
const creds = resolveMSTeamsCredentials(msteamsCfg);
126196
if (!creds) {
127197
throw new Error("MS Teams credentials missing");
128198
}
199+
200+
// Try delegated token if requested and configured
201+
if (options?.preferDelegated && msteamsCfg?.delegatedAuth?.enabled) {
202+
// Dynamic import to avoid circular dependency (token.ts imports from graph.ts indirectly)
203+
const { resolveDelegatedAccessToken } = await import("./token.js");
204+
const delegated = await resolveDelegatedAccessToken({
205+
tenantId: creds.tenantId,
206+
clientId: creds.appId,
207+
clientSecret: creds.appPassword,
208+
});
209+
if (delegated) {
210+
return delegated;
211+
}
212+
// Fall through to app-only token
213+
}
214+
129215
const { app } = await loadMSTeamsSdkWithAuth(creds);
130216
const tokenProvider = createMSTeamsTokenProvider(app);
131217
const graphTokenValue = await tokenProvider.getAccessToken("https://graph.microsoft.com");
@@ -140,8 +226,8 @@ export async function listTeamsByName(token: string, query: string): Promise<Gra
140226
const escaped = escapeOData(query);
141227
const filter = `resourceProvisioningOptions/Any(x:x eq 'Team') and startsWith(displayName,'${escaped}')`;
142228
const path = `/groups?$filter=${encodeURIComponent(filter)}&$select=id,displayName`;
143-
const res = await fetchGraphJson<GraphResponse<GraphGroup>>({ token, path });
144-
return res.value ?? [];
229+
const { items } = await fetchAllGraphPages<GraphGroup>({ token, path, maxPages: 5 });
230+
return items;
145231
}
146232

147233
export async function postGraphJson<T>(params: {
@@ -186,6 +272,6 @@ export async function deleteGraphRequest(params: { token: string; path: string }
186272

187273
export async function listChannelsForTeam(token: string, teamId: string): Promise<GraphChannel[]> {
188274
const path = `/teams/${encodeURIComponent(teamId)}/channels?$select=id,displayName`;
189-
const res = await fetchGraphJson<GraphResponse<GraphChannel>>({ token, path });
190-
return res.value ?? [];
275+
const { items } = await fetchAllGraphPages<GraphChannel>({ token, path, maxPages: 10 });
276+
return items;
191277
}

0 commit comments

Comments
 (0)