fix(msteams): resolve Graph API chat ID for DM file uploads (#49585)
Fixes #35822 — Bot Framework conversation.id format is incompatible with Graph API /chats/{chatId}. Added resolveGraphChatId() to look up the Graph-native chat ID via GET /me/chats, cached in the conversation store. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7c3af3726f
commit
06845a1974
@ -25,6 +25,13 @@ export type StoredConversationReference = {
|
||||
serviceUrl?: string;
|
||||
/** Locale */
|
||||
locale?: string;
|
||||
/**
|
||||
* Cached Graph API chat ID (format: `19:xxx@thread.tacv2` or `19:xxx@unq.gbl.spaces`).
|
||||
* Bot Framework conversation IDs for personal DMs use a different format (`a:1xxx` or
|
||||
* `8:orgid:xxx`) that the Graph API does not accept. This field caches the resolved
|
||||
* Graph-native chat ID so we don't need to re-query the API on every send.
|
||||
*/
|
||||
graphChatId?: string;
|
||||
};
|
||||
|
||||
export type MSTeamsConversationStoreEntry = {
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { withFetchPreconnect } from "../../../test/helpers/extensions/fetch-mock.js";
|
||||
import { uploadToOneDrive, uploadToSharePoint } from "./graph-upload.js";
|
||||
import { resolveGraphChatId, uploadToOneDrive, uploadToSharePoint } from "./graph-upload.js";
|
||||
|
||||
describe("graph upload helpers", () => {
|
||||
const tokenProvider = {
|
||||
@ -100,3 +100,106 @@ describe("graph upload helpers", () => {
|
||||
).rejects.toThrow("SharePoint upload response missing required fields");
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveGraphChatId", () => {
|
||||
const tokenProvider = {
|
||||
getAccessToken: vi.fn(async () => "graph-token"),
|
||||
};
|
||||
|
||||
it("returns the ID directly when it already starts with 19:", async () => {
|
||||
const fetchFn = vi.fn();
|
||||
const result = await resolveGraphChatId({
|
||||
botFrameworkConversationId: "19:abc123@thread.tacv2",
|
||||
tokenProvider,
|
||||
fetchFn,
|
||||
});
|
||||
// Should short-circuit without making any API call
|
||||
expect(fetchFn).not.toHaveBeenCalled();
|
||||
expect(result).toBe("19:abc123@thread.tacv2");
|
||||
});
|
||||
|
||||
it("resolves personal DM chat ID via Graph API using user AAD object ID", async () => {
|
||||
const fetchFn = vi.fn(
|
||||
async () =>
|
||||
new Response(JSON.stringify({ value: [{ id: "19:dm-chat-id@unq.gbl.spaces" }] }), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await resolveGraphChatId({
|
||||
botFrameworkConversationId: "a:1abc_bot_framework_dm_id",
|
||||
userAadObjectId: "user-aad-object-id-123",
|
||||
tokenProvider,
|
||||
fetchFn,
|
||||
});
|
||||
|
||||
expect(fetchFn).toHaveBeenCalledWith(
|
||||
expect.stringContaining("/me/chats"),
|
||||
expect.objectContaining({
|
||||
headers: expect.objectContaining({ Authorization: "Bearer graph-token" }),
|
||||
}),
|
||||
);
|
||||
// Should filter by user AAD object ID
|
||||
const callUrl = (fetchFn.mock.calls[0] as [string, unknown])[0];
|
||||
expect(callUrl).toContain("user-aad-object-id-123");
|
||||
expect(result).toBe("19:dm-chat-id@unq.gbl.spaces");
|
||||
});
|
||||
|
||||
it("resolves personal DM chat ID without user AAD object ID (lists all 1:1 chats)", async () => {
|
||||
const fetchFn = vi.fn(
|
||||
async () =>
|
||||
new Response(JSON.stringify({ value: [{ id: "19:fallback-chat@unq.gbl.spaces" }] }), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await resolveGraphChatId({
|
||||
botFrameworkConversationId: "8:orgid:user-object-id",
|
||||
tokenProvider,
|
||||
fetchFn,
|
||||
});
|
||||
|
||||
expect(fetchFn).toHaveBeenCalledOnce();
|
||||
expect(result).toBe("19:fallback-chat@unq.gbl.spaces");
|
||||
});
|
||||
|
||||
it("returns null when Graph API returns no chats", async () => {
|
||||
const fetchFn = vi.fn(
|
||||
async () =>
|
||||
new Response(JSON.stringify({ value: [] }), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await resolveGraphChatId({
|
||||
botFrameworkConversationId: "a:1unknown_dm",
|
||||
userAadObjectId: "some-user",
|
||||
tokenProvider,
|
||||
fetchFn,
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when Graph API call fails", async () => {
|
||||
const fetchFn = vi.fn(
|
||||
async () =>
|
||||
new Response("Unauthorized", {
|
||||
status: 401,
|
||||
headers: { "content-type": "text/plain" },
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await resolveGraphChatId({
|
||||
botFrameworkConversationId: "a:1some_dm_id",
|
||||
userAadObjectId: "some-user",
|
||||
tokenProvider,
|
||||
fetchFn,
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@ -264,6 +264,82 @@ export async function getDriveItemProperties(params: {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the Graph API-native chat ID from a Bot Framework conversation ID.
|
||||
*
|
||||
* Bot Framework personal DM conversation IDs use formats like `a:1xxx@unq.gbl.spaces`
|
||||
* or `8:orgid:xxx` that the Graph API does not accept. Graph API requires the
|
||||
* `19:xxx@thread.tacv2` or `19:xxx@unq.gbl.spaces` format.
|
||||
*
|
||||
* This function looks up the matching Graph chat by querying the bot's chats filtered
|
||||
* by the target user's AAD object ID.
|
||||
*
|
||||
* Returns the Graph chat ID if found, or null if resolution fails.
|
||||
*/
|
||||
export async function resolveGraphChatId(params: {
|
||||
/** Bot Framework conversation ID (may be in non-Graph format for personal DMs) */
|
||||
botFrameworkConversationId: string;
|
||||
/** AAD object ID of the user in the conversation (used for filtering chats) */
|
||||
userAadObjectId?: string;
|
||||
tokenProvider: MSTeamsAccessTokenProvider;
|
||||
fetchFn?: typeof fetch;
|
||||
}): Promise<string | null> {
|
||||
const { botFrameworkConversationId, userAadObjectId, tokenProvider } = params;
|
||||
const fetchFn = params.fetchFn ?? fetch;
|
||||
|
||||
// If the conversation ID already looks like a valid Graph chat ID, return it directly.
|
||||
// Graph chat IDs start with "19:" — Bot Framework group chat IDs already use this format.
|
||||
if (botFrameworkConversationId.startsWith("19:")) {
|
||||
return botFrameworkConversationId;
|
||||
}
|
||||
|
||||
// For personal DMs with non-Graph conversation IDs (e.g. `a:1xxx` or `8:orgid:xxx`),
|
||||
// query the bot's chats to find the matching one.
|
||||
const token = await tokenProvider.getAccessToken(GRAPH_SCOPE);
|
||||
|
||||
// Build filter: if we have the user's AAD object ID, narrow the search to 1:1 chats
|
||||
// with that member. Otherwise, fall back to listing all 1:1 chats.
|
||||
let path: string;
|
||||
if (userAadObjectId) {
|
||||
const encoded = encodeURIComponent(
|
||||
`chatType eq 'oneOnOne' and members/any(m:m/microsoft.graph.aadUserConversationMember/userId eq '${userAadObjectId}')`,
|
||||
);
|
||||
path = `/me/chats?$filter=${encoded}&$select=id`;
|
||||
} else {
|
||||
// Fallback: list all 1:1 chats when no user ID is available.
|
||||
// Only safe when the bot has exactly one 1:1 chat; returns null otherwise to
|
||||
// avoid sending to the wrong person's chat.
|
||||
path = `/me/chats?$filter=${encodeURIComponent("chatType eq 'oneOnOne'")}&$select=id`;
|
||||
}
|
||||
|
||||
const res = await fetchFn(`${GRAPH_ROOT}${path}`, {
|
||||
headers: { Authorization: `Bearer ${token}` },
|
||||
});
|
||||
|
||||
if (!res.ok) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const data = (await res.json()) as {
|
||||
value?: Array<{ id?: string }>;
|
||||
};
|
||||
|
||||
const chats = data.value ?? [];
|
||||
|
||||
// When filtered by userAadObjectId, any non-empty result is the right 1:1 chat.
|
||||
if (userAadObjectId && chats.length > 0 && chats[0]?.id) {
|
||||
return chats[0].id;
|
||||
}
|
||||
|
||||
// Without a user ID we can only be certain when exactly one chat is returned;
|
||||
// multiple results would be ambiguous and could route to the wrong person.
|
||||
if (!userAadObjectId && chats.length === 1 && chats[0]?.id) {
|
||||
return chats[0].id;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get members of a Teams chat for per-user sharing.
|
||||
* Used to create sharing links scoped to only the chat participants.
|
||||
|
||||
@ -321,8 +321,10 @@ async function buildActivity(
|
||||
|
||||
if (!isPersonal && !isImage && tokenProvider && sharePointSiteId) {
|
||||
// Non-image in group chat/channel with SharePoint site configured:
|
||||
// Upload to SharePoint and use native file card attachment
|
||||
const chatId = conversationRef.conversation?.id;
|
||||
// Upload to SharePoint and use native file card attachment.
|
||||
// Use the cached Graph-native chat ID when available — Bot Framework conversation IDs
|
||||
// for personal DMs use a format (e.g. `a:1xxx`) that Graph API rejects.
|
||||
const chatId = conversationRef.graphChatId ?? conversationRef.conversation?.id;
|
||||
|
||||
// Upload to SharePoint
|
||||
const uploaded = await uploadAndShareSharePoint({
|
||||
|
||||
@ -9,6 +9,7 @@ import type {
|
||||
MSTeamsConversationStore,
|
||||
StoredConversationReference,
|
||||
} from "./conversation-store.js";
|
||||
import { resolveGraphChatId } from "./graph-upload.js";
|
||||
import type { MSTeamsAdapter } from "./messenger.js";
|
||||
import { getMSTeamsRuntime } from "./runtime.js";
|
||||
import { createMSTeamsAdapter, loadMSTeamsSdkWithAuth } from "./sdk.js";
|
||||
@ -30,6 +31,13 @@ export type MSTeamsProactiveContext = {
|
||||
sharePointSiteId?: string;
|
||||
/** Resolved media max bytes from config (default: 100MB) */
|
||||
mediaMaxBytes?: number;
|
||||
/**
|
||||
* Graph API-native chat ID for this conversation.
|
||||
* Bot Framework personal DM IDs (`a:1xxx` / `8:orgid:xxx`) cannot be used directly
|
||||
* with Graph chat endpoints. This field holds the resolved `19:xxx` format ID.
|
||||
* Null if resolution failed or not applicable.
|
||||
*/
|
||||
graphChatId?: string | null;
|
||||
};
|
||||
|
||||
/**
|
||||
@ -150,6 +158,45 @@ export async function resolveMSTeamsSendContext(params: {
|
||||
resolveChannelLimitMb: ({ cfg }) => cfg.channels?.msteams?.mediaMaxMb,
|
||||
});
|
||||
|
||||
// Resolve Graph API-native chat ID if needed for SharePoint per-user sharing.
|
||||
// Bot Framework personal DM conversation IDs (e.g. `a:1xxx` or `8:orgid:xxx`) cannot
|
||||
// be used directly with Graph /chats/{chatId} endpoints — the Graph API requires the
|
||||
// `19:xxx@thread.tacv2` or `19:xxx@unq.gbl.spaces` format.
|
||||
// We check the cached value first, then resolve via Graph API and cache for future sends.
|
||||
let graphChatId: string | null | undefined = ref.graphChatId ?? undefined;
|
||||
if (graphChatId === undefined && sharePointSiteId) {
|
||||
// Only resolve when SharePoint is configured (the only place chatId matters currently)
|
||||
try {
|
||||
const resolved = await resolveGraphChatId({
|
||||
botFrameworkConversationId: conversationId,
|
||||
userAadObjectId: ref.user?.aadObjectId,
|
||||
tokenProvider,
|
||||
});
|
||||
graphChatId = resolved;
|
||||
|
||||
// Cache in the conversation store so subsequent sends skip the Graph lookup.
|
||||
// NOTE: We intentionally do NOT cache null results. Transient Graph API failures
|
||||
// (network, 401, rate limit) should be retried on subsequent sends rather than
|
||||
// permanently blocking file uploads for this conversation.
|
||||
if (resolved) {
|
||||
await store.upsert(conversationId, { ...ref, graphChatId: resolved });
|
||||
} else {
|
||||
log.warn?.("could not resolve Graph chat ID; file uploads may fail for this conversation", {
|
||||
conversationId,
|
||||
});
|
||||
}
|
||||
} catch (err) {
|
||||
log.warn?.(
|
||||
"failed to resolve Graph chat ID; file uploads may fall back to Bot Framework ID",
|
||||
{
|
||||
conversationId,
|
||||
error: String(err),
|
||||
},
|
||||
);
|
||||
graphChatId = null;
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
appId: creds.appId,
|
||||
conversationId,
|
||||
@ -160,5 +207,6 @@ export async function resolveMSTeamsSendContext(params: {
|
||||
tokenProvider,
|
||||
sharePointSiteId,
|
||||
mediaMaxBytes,
|
||||
graphChatId,
|
||||
};
|
||||
}
|
||||
|
||||
@ -9,6 +9,9 @@ const mockState = vi.hoisted(() => ({
|
||||
prepareFileConsentActivity: vi.fn(),
|
||||
extractFilename: vi.fn(async () => "fallback.bin"),
|
||||
sendMSTeamsMessages: vi.fn(),
|
||||
uploadAndShareSharePoint: vi.fn(),
|
||||
getDriveItemProperties: vi.fn(),
|
||||
buildTeamsFileInfoCard: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../runtime-api.js", () => ({
|
||||
@ -45,6 +48,16 @@ vi.mock("./runtime.js", () => ({
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("./graph-upload.js", () => ({
|
||||
uploadAndShareSharePoint: mockState.uploadAndShareSharePoint,
|
||||
getDriveItemProperties: mockState.getDriveItemProperties,
|
||||
uploadAndShareOneDrive: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./graph-chat.js", () => ({
|
||||
buildTeamsFileInfoCard: mockState.buildTeamsFileInfoCard,
|
||||
}));
|
||||
|
||||
describe("sendMessageMSTeams", () => {
|
||||
beforeEach(() => {
|
||||
mockState.loadOutboundMediaFromUrl.mockReset();
|
||||
@ -53,6 +66,9 @@ describe("sendMessageMSTeams", () => {
|
||||
mockState.prepareFileConsentActivity.mockReset();
|
||||
mockState.extractFilename.mockReset();
|
||||
mockState.sendMSTeamsMessages.mockReset();
|
||||
mockState.uploadAndShareSharePoint.mockReset();
|
||||
mockState.getDriveItemProperties.mockReset();
|
||||
mockState.buildTeamsFileInfoCard.mockReset();
|
||||
|
||||
mockState.extractFilename.mockResolvedValue("fallback.bin");
|
||||
mockState.requiresFileConsent.mockReturnValue(false);
|
||||
@ -106,4 +122,139 @@ describe("sendMessageMSTeams", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses graphChatId instead of conversationId when uploading to SharePoint", async () => {
|
||||
// Simulates a group chat where Bot Framework conversationId is valid but we have
|
||||
// a resolved Graph chat ID cached from a prior send.
|
||||
const graphChatId = "19:graph-native-chat-id@thread.tacv2";
|
||||
const botFrameworkConversationId = "19:bot-framework-id@thread.tacv2";
|
||||
|
||||
mockState.resolveMSTeamsSendContext.mockResolvedValue({
|
||||
adapter: {
|
||||
continueConversation: vi.fn(
|
||||
async (
|
||||
_id: string,
|
||||
_ref: unknown,
|
||||
fn: (ctx: { sendActivity: () => { id: "msg-1" } }) => Promise<void>,
|
||||
) => fn({ sendActivity: () => ({ id: "msg-1" }) }),
|
||||
),
|
||||
},
|
||||
appId: "app-id",
|
||||
conversationId: botFrameworkConversationId,
|
||||
graphChatId,
|
||||
ref: {},
|
||||
log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
conversationType: "groupChat",
|
||||
tokenProvider: { getAccessToken: vi.fn(async () => "token") },
|
||||
mediaMaxBytes: 8 * 1024 * 1024,
|
||||
sharePointSiteId: "site-123",
|
||||
});
|
||||
|
||||
const pdfBuffer = Buffer.alloc(100, "pdf");
|
||||
mockState.loadOutboundMediaFromUrl.mockResolvedValueOnce({
|
||||
buffer: pdfBuffer,
|
||||
contentType: "application/pdf",
|
||||
fileName: "doc.pdf",
|
||||
kind: "file",
|
||||
});
|
||||
mockState.requiresFileConsent.mockReturnValue(false);
|
||||
mockState.uploadAndShareSharePoint.mockResolvedValue({
|
||||
itemId: "item-1",
|
||||
webUrl: "https://sp.example.com/doc.pdf",
|
||||
shareUrl: "https://sp.example.com/share/doc.pdf",
|
||||
name: "doc.pdf",
|
||||
});
|
||||
mockState.getDriveItemProperties.mockResolvedValue({
|
||||
eTag: '"{GUID-123},1"',
|
||||
webDavUrl: "https://sp.example.com/dav/doc.pdf",
|
||||
name: "doc.pdf",
|
||||
});
|
||||
mockState.buildTeamsFileInfoCard.mockReturnValue({
|
||||
contentType: "application/vnd.microsoft.teams.card.file.info",
|
||||
contentUrl: "https://sp.example.com/dav/doc.pdf",
|
||||
name: "doc.pdf",
|
||||
content: { uniqueId: "GUID-123", fileType: "pdf" },
|
||||
});
|
||||
|
||||
await sendMessageMSTeams({
|
||||
cfg: {} as OpenClawConfig,
|
||||
to: "conversation:19:bot-framework-id@thread.tacv2",
|
||||
text: "here is a file",
|
||||
mediaUrl: "https://example.com/doc.pdf",
|
||||
});
|
||||
|
||||
// The Graph-native chatId must be passed to SharePoint upload, not the Bot Framework ID
|
||||
expect(mockState.uploadAndShareSharePoint).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
chatId: graphChatId,
|
||||
siteId: "site-123",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to conversationId when graphChatId is not available", async () => {
|
||||
const botFrameworkConversationId = "19:fallback-id@thread.tacv2";
|
||||
|
||||
mockState.resolveMSTeamsSendContext.mockResolvedValue({
|
||||
adapter: {
|
||||
continueConversation: vi.fn(
|
||||
async (
|
||||
_id: string,
|
||||
_ref: unknown,
|
||||
fn: (ctx: { sendActivity: () => { id: "msg-1" } }) => Promise<void>,
|
||||
) => fn({ sendActivity: () => ({ id: "msg-1" }) }),
|
||||
),
|
||||
},
|
||||
appId: "app-id",
|
||||
conversationId: botFrameworkConversationId,
|
||||
graphChatId: null, // resolution failed — must fall back
|
||||
ref: {},
|
||||
log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
conversationType: "groupChat",
|
||||
tokenProvider: { getAccessToken: vi.fn(async () => "token") },
|
||||
mediaMaxBytes: 8 * 1024 * 1024,
|
||||
sharePointSiteId: "site-456",
|
||||
});
|
||||
|
||||
const pdfBuffer = Buffer.alloc(50, "pdf");
|
||||
mockState.loadOutboundMediaFromUrl.mockResolvedValueOnce({
|
||||
buffer: pdfBuffer,
|
||||
contentType: "application/pdf",
|
||||
fileName: "report.pdf",
|
||||
kind: "file",
|
||||
});
|
||||
mockState.requiresFileConsent.mockReturnValue(false);
|
||||
mockState.uploadAndShareSharePoint.mockResolvedValue({
|
||||
itemId: "item-2",
|
||||
webUrl: "https://sp.example.com/report.pdf",
|
||||
shareUrl: "https://sp.example.com/share/report.pdf",
|
||||
name: "report.pdf",
|
||||
});
|
||||
mockState.getDriveItemProperties.mockResolvedValue({
|
||||
eTag: '"{GUID-456},1"',
|
||||
webDavUrl: "https://sp.example.com/dav/report.pdf",
|
||||
name: "report.pdf",
|
||||
});
|
||||
mockState.buildTeamsFileInfoCard.mockReturnValue({
|
||||
contentType: "application/vnd.microsoft.teams.card.file.info",
|
||||
contentUrl: "https://sp.example.com/dav/report.pdf",
|
||||
name: "report.pdf",
|
||||
content: { uniqueId: "GUID-456", fileType: "pdf" },
|
||||
});
|
||||
|
||||
await sendMessageMSTeams({
|
||||
cfg: {} as OpenClawConfig,
|
||||
to: "conversation:19:fallback-id@thread.tacv2",
|
||||
text: "report",
|
||||
mediaUrl: "https://example.com/report.pdf",
|
||||
});
|
||||
|
||||
// Falls back to conversationId when graphChatId is null
|
||||
expect(mockState.uploadAndShareSharePoint).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
chatId: botFrameworkConversationId,
|
||||
siteId: "site-456",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@ -206,7 +206,9 @@ export async function sendMessageMSTeams(
|
||||
contentType: media.contentType,
|
||||
tokenProvider,
|
||||
siteId: sharePointSiteId,
|
||||
chatId: conversationId,
|
||||
// Use the Graph-native chat ID (19:xxx format) — the Bot Framework conversationId
|
||||
// for personal DMs uses a different format that Graph API rejects.
|
||||
chatId: ctx.graphChatId ?? conversationId,
|
||||
usePerUserSharing: conversationType === "groupChat",
|
||||
});
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user