Skip to content

Commit 2b5b581

Browse files
sudie-codessudie-codes
authored
fix(msteams): include tenantId and aadObjectId on proactive sends (#58774) (#63949)
* fix(msteams): capture and forward tenantId/aadObjectId on proactive sends (#58774) * msteams: preserve tenantId/aadObjectId on sparse merges, thread recipientId on proactive sends
1 parent 19a2e9d commit 2b5b581

8 files changed

Lines changed: 313 additions & 15 deletions

File tree

‎extensions/msteams/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66

77
- Version alignment with core OpenClaw release numbers.
88

9+
### Fixes
10+
11+
- Capture `tenantId` from `channelData.tenant.id` and `aadObjectId` from `activity.from` on inbound Teams activities and forward them on proactive sends, fixing HTTP 403 errors on bot-initiated messages (e.g. cron announces) from the Bot Framework connector (#58774).
12+
913
## 2026.4.8
1014

1115
### Changes

‎extensions/msteams/src/conversation-store-helpers.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,19 @@ export function mergeStoredConversationReference(
3434
nowIso: string,
3535
): StoredConversationReference {
3636
return {
37-
// Preserve fields from previous entry that may not be present on every activity
38-
// (e.g. timezone is only sent when clientInfo entity is available;
39-
// graphChatId is resolved via Graph API and cached for DM media downloads).
37+
// Preserve fields from the previous entry that may not be present on every
38+
// inbound activity. Without this, sparse activities (e.g. conversationUpdate,
39+
// reactions) would clear previously captured values. Some fields are only
40+
// populated opportunistically, such as timezone from clientInfo entities and
41+
// graphChatId from Graph lookups used for DM media downloads.
4042
...(existing?.timezone && !incoming.timezone ? { timezone: existing.timezone } : {}),
4143
...(existing?.graphChatId && !incoming.graphChatId
4244
? { graphChatId: existing.graphChatId }
4345
: {}),
46+
...(existing?.tenantId && !incoming.tenantId ? { tenantId: existing.tenantId } : {}),
47+
...(existing?.aadObjectId && !incoming.aadObjectId
48+
? { aadObjectId: existing.aadObjectId }
49+
: {}),
4450
...incoming,
4551
lastSeenAt: nowIso,
4652
};

‎extensions/msteams/src/conversation-store.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,20 @@ export type StoredConversationReference = {
1919
bot?: { id?: string; name?: string };
2020
/** Conversation details */
2121
conversation?: { id?: string; conversationType?: string; tenantId?: string };
22+
/**
23+
* Tenant ID sourced from `activity.channelData.tenant.id` at inbound time.
24+
* Bot Framework requires this on outbound proactive messages so the connector
25+
* can route them to the correct Azure AD tenant; without it, the connector
26+
* rejects the request with HTTP 403. For channel activities, `conversation.tenantId`
27+
* is often unset, making `channelData.tenant.id` the reliable source.
28+
*/
29+
tenantId?: string;
30+
/**
31+
* Azure AD object ID of the user who sent the last inbound activity,
32+
* mirrored from `activity.from.aadObjectId` so outbound proactive sends
33+
* can include it on the connector request (required for personal DMs).
34+
*/
35+
aadObjectId?: string;
2236
/** Team ID for channel messages (when available). */
2337
teamId?: string;
2438
/** Channel ID (usually "msteams") */
@@ -36,15 +50,6 @@ export type StoredConversationReference = {
3650
graphChatId?: string;
3751
/** IANA timezone from Teams clientInfo entity (e.g. "America/New_York") */
3852
timezone?: string;
39-
/**
40-
* Thread root message ID for channel thread messages.
41-
* When a message arrives inside a Teams channel thread, the Bot Framework
42-
* sets `conversation.id` to `19:xxx@thread.tacv2;messageid=<rootId>` and/or
43-
* `replyToId` to the thread root activity ID. This field caches that root ID
44-
* so outbound replies can target the correct thread instead of landing as
45-
* top-level channel posts.
46-
*/
47-
threadId?: string;
4853
};
4954

5055
export type MSTeamsConversationStoreEntry = {

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ vi.mock("./graph-upload.js", () => {
2020

2121
import {
2222
buildActivity,
23+
buildConversationReference,
2324
renderReplyPayloadsToMessages,
2425
sendMSTeamsMessages,
2526
type MSTeamsAdapter,
@@ -766,4 +767,96 @@ describe("msteams messenger", () => {
766767
expect(channelData.feedbackLoopEnabled).toBe(false);
767768
});
768769
});
770+
771+
// Regression coverage for #58774: proactive Teams sends fail with HTTP 403
772+
// when the Bot Framework connector does not see `tenantId` / `aadObjectId`
773+
// on the outbound conversation reference.
774+
describe("buildConversationReference tenant/aad forwarding (#58774)", () => {
775+
const storedWithChannelDataTenant: StoredConversationReference = {
776+
activityId: "activity-1",
777+
user: { id: "user123", name: "User", aadObjectId: "aad-user-123" },
778+
agent: { id: "bot123", name: "Bot" },
779+
conversation: {
780+
id: "19:abc@thread.tacv2",
781+
conversationType: "channel",
782+
},
783+
// Canonical channelData source captured by message-handler inbound code.
784+
tenantId: "tenant-abc",
785+
aadObjectId: "aad-user-123",
786+
channelId: "msteams",
787+
serviceUrl: "https://smba.trafficmanager.net/amer/",
788+
};
789+
790+
it("forwards top-level tenantId and aadObjectId onto the outbound reference", () => {
791+
const reference = buildConversationReference(storedWithChannelDataTenant);
792+
expect(reference.tenantId).toBe("tenant-abc");
793+
expect(reference.aadObjectId).toBe("aad-user-123");
794+
expect(reference.conversation.tenantId).toBe("tenant-abc");
795+
expect(reference.user?.aadObjectId).toBe("aad-user-123");
796+
});
797+
798+
it("falls back to conversation.tenantId when no top-level tenantId is stored (legacy ref)", () => {
799+
const legacy: StoredConversationReference = {
800+
activityId: "activity-legacy",
801+
user: { id: "user-legacy", name: "Legacy", aadObjectId: "aad-legacy" },
802+
agent: { id: "bot-legacy", name: "Bot" },
803+
conversation: {
804+
id: "a:personal-chat",
805+
conversationType: "personal",
806+
tenantId: "tenant-legacy",
807+
},
808+
channelId: "msteams",
809+
serviceUrl: "https://smba.trafficmanager.net/amer/",
810+
};
811+
const reference = buildConversationReference(legacy);
812+
expect(reference.tenantId).toBe("tenant-legacy");
813+
expect(reference.aadObjectId).toBe("aad-legacy");
814+
});
815+
816+
it("omits tenantId and aadObjectId when neither source is available", () => {
817+
const minimal: StoredConversationReference = {
818+
activityId: "activity-2",
819+
user: { id: "user456", name: "User" },
820+
agent: { id: "bot456", name: "Bot" },
821+
conversation: { id: "19:xyz@thread.tacv2", conversationType: "channel" },
822+
channelId: "msteams",
823+
serviceUrl: "https://smba.trafficmanager.net/amer/",
824+
};
825+
const reference = buildConversationReference(minimal);
826+
expect(reference.tenantId).toBeUndefined();
827+
expect(reference.aadObjectId).toBeUndefined();
828+
expect(reference.conversation.tenantId).toBeUndefined();
829+
});
830+
831+
it("propagates tenantId/aadObjectId through sendMSTeamsMessages proactive path", async () => {
832+
let capturedReference:
833+
| { tenantId?: string; aadObjectId?: string; user?: { aadObjectId?: string } }
834+
| undefined;
835+
const adapter: MSTeamsAdapter = {
836+
continueConversation: async (_appId, reference, logic) => {
837+
capturedReference = reference as typeof capturedReference;
838+
await logic({
839+
sendActivity: async () => ({ id: "ok" }),
840+
updateActivity: noopUpdateActivity,
841+
deleteActivity: noopDeleteActivity,
842+
});
843+
},
844+
process: async () => {},
845+
updateActivity: noopUpdateActivity,
846+
deleteActivity: noopDeleteActivity,
847+
};
848+
849+
await sendMSTeamsMessages({
850+
replyStyle: "top-level",
851+
adapter,
852+
appId: "app123",
853+
conversationRef: storedWithChannelDataTenant,
854+
messages: [{ text: "hello" }],
855+
});
856+
857+
expect(capturedReference?.tenantId).toBe("tenant-abc");
858+
expect(capturedReference?.aadObjectId).toBe("aad-user-123");
859+
expect(capturedReference?.user?.aadObjectId).toBe("aad-user-123");
860+
});
861+
});
769862
});

‎extensions/msteams/src/messenger.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,18 @@ export type MSTeamsConversationReference = {
5151
channelId: string;
5252
serviceUrl?: string;
5353
locale?: string;
54+
/**
55+
* Top-level tenant ID echoed onto the Bot Framework connector request. Included
56+
* alongside `conversation.tenantId` so the connector can route proactive sends
57+
* to the correct Azure AD tenant. Missing it causes HTTP 403 on proactive
58+
* (bot-initiated) messages.
59+
*/
60+
tenantId?: string;
61+
/**
62+
* Azure AD object ID of the target user, forwarded on proactive sends so
63+
* Bot Framework can resolve the personal DM recipient on the connector side.
64+
*/
65+
aadObjectId?: string;
5466
};
5567

5668
export type MSTeamsAdapter = {
@@ -119,18 +131,26 @@ export function buildConversationReference(
119131
if (!user?.id) {
120132
throw new Error("Invalid stored reference: missing user.id");
121133
}
134+
// Bot Framework proactive sends require `tenantId` on the outbound activity
135+
// so the connector routes to the correct Azure AD tenant; otherwise it rejects
136+
// with HTTP 403. Prefer the explicit top-level `ref.tenantId` (captured from
137+
// `channelData.tenant.id` inbound) and fall back to `conversation.tenantId`.
138+
const tenantId = ref.tenantId ?? ref.conversation?.tenantId;
139+
const aadObjectId = ref.aadObjectId ?? user.aadObjectId;
122140
return {
123141
activityId: ref.activityId,
124-
user,
142+
user: aadObjectId ? { ...user, aadObjectId } : user,
125143
agent,
126144
conversation: {
127145
id: normalizeConversationId(conversationId),
128146
conversationType: ref.conversation?.conversationType,
129-
tenantId: ref.conversation?.tenantId,
147+
tenantId,
130148
},
131149
channelId: ref.channelId ?? "msteams",
132150
serviceUrl: ref.serviceUrl,
133151
locale: ref.locale,
152+
...(tenantId ? { tenantId } : {}),
153+
...(aadObjectId ? { aadObjectId } : {}),
134154
};
135155
}
136156

‎extensions/msteams/src/monitor-handler/message-handler.authz.test.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,8 @@ describe("msteams monitor handler authz", () => {
446446
conversationType: "personal",
447447
tenantId: "tenant-1",
448448
},
449+
tenantId: "tenant-1",
450+
aadObjectId: "new-user-aad",
449451
channelId: "msteams",
450452
serviceUrl: "https://smba.trafficmanager.net/amer/",
451453
locale: "en-US",
@@ -455,6 +457,115 @@ describe("msteams monitor handler authz", () => {
455457
expect(runtimeApiMockState.dispatchReplyFromConfigWithSettledDispatcher).not.toHaveBeenCalled();
456458
});
457459

460+
// Regression coverage for #58774: proactive sends fail with HTTP 403 when
461+
// inbound code drops tenantId/aadObjectId. Capture must prefer the canonical
462+
// `channelData.tenant.id` source and expose top-level fields on the stored ref.
463+
it("captures tenantId from channelData.tenant.id and aadObjectId from from (#58774)", async () => {
464+
const { conversationStore, deps } = createDeps({
465+
channels: {
466+
msteams: {
467+
dmPolicy: "allowlist",
468+
allowFrom: ["sender-aad"],
469+
groupPolicy: "allowlist",
470+
groupAllowFrom: ["sender-aad"],
471+
},
472+
},
473+
} as OpenClawConfig);
474+
475+
const handler = createMSTeamsMessageHandler(deps);
476+
await handler({
477+
activity: {
478+
id: "msg-channel",
479+
type: "message",
480+
text: "hello",
481+
from: {
482+
id: "sender-id",
483+
aadObjectId: "sender-aad",
484+
name: "Sender",
485+
},
486+
recipient: {
487+
id: "bot-id",
488+
name: "Bot",
489+
},
490+
conversation: {
491+
id: "19:team-channel@thread.tacv2",
492+
conversationType: "channel",
493+
// Intentionally no tenantId here: channel activities typically
494+
// carry tenantId only in channelData.tenant.id.
495+
},
496+
channelId: "msteams",
497+
serviceUrl: "https://smba.trafficmanager.net/amer/",
498+
channelData: {
499+
tenant: { id: "tenant-from-channel-data" },
500+
team: { id: "team-1" },
501+
channel: { id: "19:team-channel@thread.tacv2" },
502+
},
503+
attachments: [],
504+
},
505+
sendActivity: vi.fn(async () => undefined),
506+
} as unknown as Parameters<typeof handler>[0]);
507+
508+
expect(conversationStore.upsert).toHaveBeenCalledWith(
509+
"19:team-channel@thread.tacv2",
510+
expect.objectContaining({
511+
tenantId: "tenant-from-channel-data",
512+
aadObjectId: "sender-aad",
513+
conversation: expect.objectContaining({
514+
id: "19:team-channel@thread.tacv2",
515+
tenantId: "tenant-from-channel-data",
516+
}),
517+
}),
518+
);
519+
});
520+
521+
it("does not crash when channelData.tenant is missing and stores no tenantId", async () => {
522+
const { conversationStore, deps } = createDeps({
523+
channels: {
524+
msteams: {
525+
dmPolicy: "allowlist",
526+
allowFrom: ["sender-aad"],
527+
groupPolicy: "allowlist",
528+
groupAllowFrom: ["sender-aad"],
529+
},
530+
},
531+
} as OpenClawConfig);
532+
533+
const handler = createMSTeamsMessageHandler(deps);
534+
await handler({
535+
activity: {
536+
id: "msg-no-tenant",
537+
type: "message",
538+
text: "hello",
539+
from: {
540+
id: "sender-id",
541+
aadObjectId: "sender-aad",
542+
name: "Sender",
543+
},
544+
recipient: {
545+
id: "bot-id",
546+
name: "Bot",
547+
},
548+
conversation: {
549+
id: "19:no-tenant@thread.tacv2",
550+
conversationType: "channel",
551+
},
552+
channelId: "msteams",
553+
serviceUrl: "https://smba.trafficmanager.net/amer/",
554+
// No channelData at all: capture must degrade gracefully.
555+
attachments: [],
556+
},
557+
sendActivity: vi.fn(async () => undefined),
558+
} as unknown as Parameters<typeof handler>[0]);
559+
560+
expect(conversationStore.upsert).toHaveBeenCalledTimes(1);
561+
const storedArg = conversationStore.upsert.mock.calls[0]?.[1] as Record<string, unknown>;
562+
expect(storedArg).toBeDefined();
563+
// Top-level tenantId must not be present when no source is available.
564+
expect(storedArg.tenantId).toBeUndefined();
565+
// aadObjectId still captured from `from.aadObjectId` when present.
566+
expect(storedArg.aadObjectId).toBe("sender-aad");
567+
});
568+
458569
it("logs an info drop reason when dmPolicy allowlist rejects a sender", async () => {
459570
const { deps } = createDeps({
460571
channels: {

‎extensions/msteams/src/monitor-handler/message-handler.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,13 @@ function buildStoredConversationReference(params: {
104104
const clientInfo = activity.entities?.find((e) => e.type === "clientInfo") as
105105
| { timezone?: string }
106106
| undefined;
107+
// Bot Framework requires `tenantId` on outbound proactive activities so the
108+
// connector can route them to the correct Azure AD tenant; missing it causes
109+
// HTTP 403. Channel activities often leave `conversation.tenantId` unset, so
110+
// prefer the canonical `channelData.tenant.id` source when available.
111+
const channelDataTenantId = activity.channelData?.tenant?.id;
112+
const tenantId = channelDataTenantId ?? conversation?.tenantId;
113+
const aadObjectId = from?.aadObjectId;
107114
return {
108115
activityId: activity.id,
109116
user: from ? { id: from.id, name: from.name, aadObjectId: from.aadObjectId } : undefined,
@@ -112,8 +119,10 @@ function buildStoredConversationReference(params: {
112119
conversation: {
113120
id: conversationId,
114121
conversationType,
115-
tenantId: conversation?.tenantId,
122+
tenantId,
116123
},
124+
...(tenantId ? { tenantId } : {}),
125+
...(aadObjectId ? { aadObjectId } : {}),
117126
teamId,
118127
channelId: activity.channelId,
119128
serviceUrl: activity.serviceUrl,

0 commit comments

Comments
 (0)