fix(teams): match the bot's 28: wire id, keep extras on the instance, trim to two invariant tests
Follow-up to the salvaged #113591 gate: - Teams writes the bot's conversation identity as `28:<app id>` (activity.recipient.id, mention entities) while App.id is the bare app id. The mention match and the own-message filter now accept both spellings, so a real tenant mention no longer fails silently. - A payload that mentions only other people is no longer treated as a bot mention; the `<at>` text fallback applies only when the activity carries no mention entities at all. - `self._extra` holds `platforms.teams.extra` on the instance so keys read after construction (require_mention today) are not dropped with a constructor local (#114366). - Reply exemption uses one bounded deque instead of deque + mirror set. - Tests trimmed to two invariants: gate table (drop before attachment download / keep mention, reply-to-bot, personal) and env-over-YAML precedence. TEAMS_REQUIRE_MENTION documented in plugin.yaml and the Teams docs page. Co-authored-by: Kevin Rajan <7121943+kvnloo@users.noreply.github.com> Co-authored-by: finn763 <165816600+finn763@users.noreply.github.com> Co-authored-by: Fernando Muñoz Suazo <fernandrewm@gmail.com>
This commit is contained in:
@@ -346,14 +346,15 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
|
||||
def __init__(self, config: PlatformConfig):
|
||||
super().__init__(config, Platform("teams"))
|
||||
extra = config.extra or {}
|
||||
# Kept on the instance: ``platforms.teams.extra.*`` keys are read after construction too.
|
||||
self._extra: Dict[str, Any] = config.extra or {}
|
||||
self._client_id, self._client_secret, self._tenant_id = _credentials(config)
|
||||
# (token, expiry monotonic ts) for connector attachment auth; refreshed under
|
||||
# _bf_token_lock so concurrent attachments can't stampede the STS.
|
||||
self._bf_token_cache: Optional[tuple] = None
|
||||
self._bf_token_lock: Optional[asyncio.Lock] = None
|
||||
self._port = coerce_port(extra.get("port") or _get_scoped_secret("TEAMS_PORT", str(_DEFAULT_PORT)), _DEFAULT_PORT)
|
||||
_raw_host = extra.get("host") or _get_scoped_secret("TEAMS_HOST", "") or _DEFAULT_HOST # falsy → dual-stack None
|
||||
self._port = coerce_port(self._extra.get("port") or _get_scoped_secret("TEAMS_PORT", str(_DEFAULT_PORT)), _DEFAULT_PORT)
|
||||
_raw_host = self._extra.get("host") or _get_scoped_secret("TEAMS_HOST", "") or _DEFAULT_HOST # falsy → dual-stack None
|
||||
self._host: Optional[str] = str(_raw_host) if _raw_host else None
|
||||
self._app: Optional["App"] = None
|
||||
self._runner: Optional["web.AppRunner"] = None
|
||||
@@ -363,7 +364,6 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
self._require_mention: bool = self._parse_require_mention(config)
|
||||
# Outbound activity ids (bounded) so require_mention can exempt replies to our own messages.
|
||||
self._sent_ids: deque = deque(maxlen=500)
|
||||
self._sent_id_set: set = set()
|
||||
|
||||
@staticmethod
|
||||
def _parse_require_mention(config) -> bool:
|
||||
@@ -371,15 +371,10 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
default as TELEGRAM_REQUIRE_MENTION). Without RSC Teams only delivers mention activities to a
|
||||
group bot anyway, so the gate changes nothing until the app gains ChannelMessage.Read.Group /
|
||||
ChatMessage.Read.Chat and starts receiving every conversation message."""
|
||||
configured = _extra_or_secret(
|
||||
config.extra,
|
||||
"require_mention",
|
||||
"TEAMS_REQUIRE_MENTION",
|
||||
False,
|
||||
)
|
||||
configured = _extra_or_secret(config.extra, "require_mention", "TEAMS_REQUIRE_MENTION", False)
|
||||
if isinstance(configured, bool):
|
||||
return configured
|
||||
return str(configured).lower() not in {"false", "0", "no", "off"}
|
||||
return str(configured).strip().lower() not in {"false", "0", "no", "off"}
|
||||
|
||||
async def connect(self, *, is_reconnect: bool = False) -> bool:
|
||||
# Reconnect paths reach here without create_adapter()'s installer — re-run to bind SDK globals.
|
||||
@@ -488,8 +483,12 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
|
||||
async def _on_message(self, ctx: ActivityContext[MessageActivity]) -> None:
|
||||
activity = ctx.activity
|
||||
bot_id = self._app.id if self._app else None
|
||||
if bot_id and getattr(activity.from_, "id", None) == bot_id:
|
||||
# Teams writes the bot's conversation identity as ``28:<app id>`` (activity.recipient) while
|
||||
# App.id is the bare app id — accept both when deciding "is this us".
|
||||
recipient_id = getattr(getattr(activity, "recipient", None), "id", None)
|
||||
bot_ids = {i for i in (self._app.id if self._app else None, recipient_id) if isinstance(i, str) and i}
|
||||
bot_ids |= {f"28:{i}" for i in tuple(bot_ids) if not i.startswith("28:")}
|
||||
if getattr(activity.from_, "id", None) in bot_ids:
|
||||
return
|
||||
msg_id = getattr(activity, "id", None)
|
||||
if msg_id and self._dedup.is_duplicate(msg_id):
|
||||
@@ -499,17 +498,12 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
if conv_id: # cache the conversation reference for proactive sends (approval cards, etc.)
|
||||
self._conv_refs[conv_id] = ctx.conversation_ref
|
||||
text = activity.text if hasattr(activity, "text") and activity.text else ""
|
||||
mentioned_bot = self._activity_mentions_bot(activity, bot_id, text)
|
||||
non_personal = getattr(conv, "conversation_type", None) != "personal"
|
||||
if self._require_mention and non_personal:
|
||||
# RSC-delivered history: every conversation message arrives. Keep the ones that
|
||||
# @mention the bot or reply to one of its own messages, drop the rest before
|
||||
# attachment downloads make a gated message cost anything.
|
||||
reply_to_bot = getattr(activity, "reply_to_id", None) in self._sent_id_set
|
||||
if not mentioned_bot and not reply_to_bot:
|
||||
logger.debug(
|
||||
"[teams] Dropping non-personal message without a bot mention (chat=%s, msg=%s)",
|
||||
conv_id, msg_id)
|
||||
if self._require_mention and getattr(conv, "conversation_type", None) != "personal":
|
||||
# RSC-delivered history: every channel/groupChat message arrives. Keep the ones that
|
||||
# @mention the bot or reply to one of its own messages; drop the rest BEFORE the
|
||||
# attachment loop so a gated post never downloads anything onto the host.
|
||||
if not self._activity_mentions_bot(activity, bot_ids, text) and getattr(activity, "reply_to_id", None) not in self._sent_ids:
|
||||
logger.debug("[teams] Dropping non-personal message without a bot mention (chat=%s, msg=%s)", conv_id, msg_id)
|
||||
return
|
||||
if "<at>" in text: # strip the <at>BotName</at> tags Teams prepends for @mentions
|
||||
text = re.sub(r"<at>[^<]*</at>\s*", "", text).strip()
|
||||
@@ -530,19 +524,15 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
text=text, source=source, message_type=msg_type, message_id=msg_id,
|
||||
media_urls=[path for path, _, _ in media], media_types=[mt for _, mt, _ in media]))
|
||||
|
||||
def _activity_mentions_bot(
|
||||
self, activity: Any, bot_id: Optional[str], text: str
|
||||
) -> bool:
|
||||
"""True when the activity carries a mention entity pointing at the bot, or — for payloads
|
||||
where the entity list is absent — an ``<at>`` tag in the text (Teams' rendered mention form)."""
|
||||
bot_id = bot_id or self._client_id
|
||||
for entity in getattr(activity, "entities", None) or []:
|
||||
if getattr(entity, "type", None) != "mention":
|
||||
continue
|
||||
mentioned = getattr(entity, "mentioned", None)
|
||||
if mentioned and str(getattr(mentioned, "id", "")) == str(bot_id):
|
||||
return True
|
||||
return "<at>" in text
|
||||
@staticmethod
|
||||
def _activity_mentions_bot(activity: Any, bot_ids: set, text: str) -> bool:
|
||||
"""True when a ``mention`` entity points at the bot (``mentioned.id`` is ``28:<app id>`` on the
|
||||
wire; ``bot_ids`` carries both spellings). A payload with no mention entities at all falls back
|
||||
to the rendered ``<at>`` tag; one that mentions only other people does not."""
|
||||
mentions = [e for e in getattr(activity, "entities", None) or [] if getattr(e, "type", None) == "mention"]
|
||||
if not mentions:
|
||||
return "<at>" in text
|
||||
return any(str(getattr(getattr(e, "mentioned", None), "id", "")) in bot_ids for e in mentions)
|
||||
|
||||
async def _cache_attachment(self, att: Any) -> Optional[tuple]:
|
||||
"""Download + cache one inbound attachment → ``(path, media_type, kind)`` or ``None``."""
|
||||
@@ -618,14 +608,10 @@ class TeamsAdapter(BasePlatformAdapter):
|
||||
return result
|
||||
|
||||
def _remember_sent(self, result: Any) -> None:
|
||||
"""Track an outbound activity id (bounded) for the require_mention reply exemption."""
|
||||
"""Track an outbound activity id (bounded deque) for the require_mention reply exemption."""
|
||||
sent_id = getattr(result, "id", None)
|
||||
if not sent_id:
|
||||
return
|
||||
if len(self._sent_ids) == self._sent_ids.maxlen:
|
||||
self._sent_id_set.discard(self._sent_ids[0])
|
||||
self._sent_ids.append(sent_id)
|
||||
self._sent_id_set.add(sent_id)
|
||||
if isinstance(sent_id, str) and sent_id:
|
||||
self._sent_ids.append(sent_id)
|
||||
|
||||
@staticmethod
|
||||
def _invoke_message(text: str) -> "InvokeResponse[AdaptiveCardActionMessageResponse]":
|
||||
|
||||
@@ -42,6 +42,10 @@ optional_env:
|
||||
description: "Allow any Teams user to trigger the bot (dev only)"
|
||||
prompt: "Allow all users? (true/false)"
|
||||
password: false
|
||||
- name: TEAMS_REQUIRE_MENTION
|
||||
description: "Only answer channel/group-chat messages that @mention the bot or reply to it (default false; needed once the app has RSC message-read consent)"
|
||||
prompt: "Require @mention in channels and group chats? (true/false)"
|
||||
password: false
|
||||
- name: TEAMS_HOME_CHANNEL
|
||||
description: "Default chat/channel ID for cron / notification delivery"
|
||||
prompt: "Home channel (or empty)"
|
||||
|
||||
@@ -1116,135 +1116,79 @@ class TestTeamsMediaAttachments:
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestTeamsRequireMention:
|
||||
"""With resource-specific consent the adapter receives every conversation
|
||||
message, not just mentions — ``require_mention`` must gate non-personal
|
||||
chats while personal chats and replies-to-the-bot stay ungated."""
|
||||
"""With resource-specific consent Teams delivers every channel/groupChat message, not just
|
||||
mentions. ``require_mention`` must drop unaddressed non-personal posts BEFORE the attachment
|
||||
loop, keep @mentions (wire id ``28:<app id>``) / replies to the bot / personal chats, and be
|
||||
read env-over-YAML like every other adapter."""
|
||||
|
||||
def _make_adapter(self, require_mention=None, **extra):
|
||||
if require_mention is not None:
|
||||
extra["require_mention"] = require_mention
|
||||
APP_ID = "bot-id"
|
||||
|
||||
def _make_adapter(self, monkeypatch=None, **extra):
|
||||
adapter = TeamsAdapter(_make_config(
|
||||
client_id="bot-id", client_secret="secret", tenant_id="tenant", **extra,
|
||||
))
|
||||
client_id=self.APP_ID, client_secret="secret", tenant_id="tenant", **extra))
|
||||
adapter._app = MagicMock()
|
||||
adapter._app.id = "bot-id"
|
||||
adapter._app.id = self.APP_ID
|
||||
adapter.handle_message = AsyncMock()
|
||||
adapter._fetch_attachment_bytes = AsyncMock(return_value=b"\x89PNG" + b"\0" * 32)
|
||||
return adapter
|
||||
|
||||
def _make_activity(
|
||||
self,
|
||||
*,
|
||||
text="Hello",
|
||||
conversation_type="channel",
|
||||
entities=None,
|
||||
reply_to_id=None,
|
||||
activity_id="activity-rm-001",
|
||||
):
|
||||
def _activity(self, conversation_type, *, text="hello", mentioned_id=None, reply_to_id=None):
|
||||
activity = MagicMock()
|
||||
activity.text = text
|
||||
activity.id = activity_id
|
||||
activity.from_ = MagicMock()
|
||||
activity.from_.id = "user-123"
|
||||
activity.from_.aad_object_id = "aad-456"
|
||||
activity.from_.name = "Test User"
|
||||
activity.conversation = MagicMock()
|
||||
activity.conversation.id = "19:channel@thread.v2"
|
||||
activity.conversation.conversation_type = conversation_type
|
||||
activity.conversation.name = "Channel"
|
||||
activity.conversation.tenant_id = "tenant-789"
|
||||
activity.attachments = []
|
||||
activity.entities = entities or []
|
||||
activity.id = f"act-{conversation_type}-{mentioned_id}-{reply_to_id}"
|
||||
activity.from_ = MagicMock(aad_object_id="aad-456", name="Test User")
|
||||
activity.from_.id = "29:user-123"
|
||||
activity.recipient = MagicMock()
|
||||
activity.recipient.id = f"28:{self.APP_ID}"
|
||||
activity.conversation = MagicMock(conversation_type=conversation_type, tenant_id="t")
|
||||
activity.conversation.id = "19:conv@thread.v2"
|
||||
activity.conversation.name = "Conv"
|
||||
att = MagicMock(content_type="image/png")
|
||||
att.name = "a.png"
|
||||
att.content_url = "https://smba.trafficmanager.net/emea/v3/attachments/1/views/original"
|
||||
activity.attachments = [att]
|
||||
activity.reply_to_id = reply_to_id
|
||||
activity.entities = []
|
||||
if mentioned_id:
|
||||
entity = MagicMock(type="mention")
|
||||
entity.mentioned = MagicMock()
|
||||
entity.mentioned.id = mentioned_id
|
||||
activity.entities = [entity]
|
||||
return activity
|
||||
|
||||
def _mention_entity(self, mentioned_id="bot-id"):
|
||||
entity = MagicMock()
|
||||
entity.type = "mention"
|
||||
entity.mentioned = MagicMock()
|
||||
entity.mentioned.id = mentioned_id
|
||||
return entity
|
||||
|
||||
def _make_ctx(self, activity):
|
||||
@pytest.mark.anyio
|
||||
@pytest.mark.parametrize("conversation_type, kwargs, dispatched", [
|
||||
("channel", {}, False),
|
||||
("groupChat", {}, False),
|
||||
("channel", {"text": "<at>Alice</at> hi", "mentioned_id": "29:alice"}, False), # someone else
|
||||
("channel", {"text": "<at>Hermes</at> hi", "mentioned_id": "28:bot-id"}, True), # wire form of the bot id
|
||||
("groupChat", {"text": "<at>Hermes</at> hi", "mentioned_id": "bot-id"}, True),
|
||||
("channel", {"reply_to_id": "bot-msg-1"}, True),
|
||||
("personal", {}, True),
|
||||
])
|
||||
async def test_gate_drops_unaddressed_non_personal_before_attachment_download(
|
||||
self, conversation_type, kwargs, dispatched,
|
||||
):
|
||||
adapter = self._make_adapter(require_mention=True)
|
||||
adapter._sent_ids.append("bot-msg-1")
|
||||
ctx = MagicMock()
|
||||
ctx.activity = activity
|
||||
return ctx
|
||||
ctx.activity = self._activity(conversation_type, **kwargs)
|
||||
await adapter._on_message(ctx)
|
||||
assert adapter.handle_message.await_count == (1 if dispatched else 0)
|
||||
assert adapter._fetch_attachment_bytes.await_count == (1 if dispatched else 0)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_channel_message_without_mention_is_dropped_when_enabled(self):
|
||||
adapter = self._make_adapter(require_mention=True)
|
||||
await adapter._on_message(self._make_ctx(self._make_activity()))
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_channel_message_with_mention_entity_passes_when_enabled(self):
|
||||
adapter = self._make_adapter(require_mention=True)
|
||||
activity = self._make_activity(
|
||||
text="<at>Hermes</at> run the report", entities=[self._mention_entity()]
|
||||
)
|
||||
await adapter._on_message(self._make_ctx(activity))
|
||||
adapter.handle_message.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_group_message_with_at_tag_only_passes_when_enabled(self):
|
||||
# Payloads without an entity list still carry the rendered mention form.
|
||||
adapter = self._make_adapter(require_mention=True)
|
||||
activity = self._make_activity(
|
||||
text="<at>Hermes</at> status?", conversation_type="groupChat")
|
||||
await adapter._on_message(self._make_ctx(activity))
|
||||
adapter.handle_message.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mention_of_another_user_is_dropped_when_enabled(self):
|
||||
adapter = self._make_adapter(require_mention=True)
|
||||
activity = self._make_activity(
|
||||
entities=[self._mention_entity(mentioned_id="other-user")]
|
||||
)
|
||||
await adapter._on_message(self._make_ctx(activity))
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reply_to_bot_message_passes_when_enabled(self):
|
||||
adapter = self._make_adapter(require_mention=True)
|
||||
adapter._remember_sent(MagicMock(id="bot-msg-7"))
|
||||
activity = self._make_activity(reply_to_id="bot-msg-7")
|
||||
await adapter._on_message(self._make_ctx(activity))
|
||||
adapter.handle_message.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reply_to_foreign_message_is_dropped_when_enabled(self):
|
||||
adapter = self._make_adapter(require_mention=True)
|
||||
adapter._remember_sent(MagicMock(id="bot-msg-7"))
|
||||
activity = self._make_activity(reply_to_id="someone-else-msg")
|
||||
await adapter._on_message(self._make_ctx(activity))
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_personal_chat_stays_ungated_when_enabled(self):
|
||||
adapter = self._make_adapter(require_mention=True)
|
||||
activity = self._make_activity(conversation_type="personal")
|
||||
await adapter._on_message(self._make_ctx(activity))
|
||||
adapter.handle_message.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gate_inactive_by_default(self):
|
||||
# Opt-in default (same as TELEGRAM_REQUIRE_MENTION): without RSC Teams only
|
||||
# delivers mention activities, so an ungated adapter keeps today's behaviour.
|
||||
adapter = self._make_adapter()
|
||||
await adapter._on_message(self._make_ctx(self._make_activity()))
|
||||
adapter.handle_message.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_env_override_enables_gate(self, monkeypatch):
|
||||
monkeypatch.setenv("TEAMS_REQUIRE_MENTION", "true")
|
||||
adapter = self._make_adapter()
|
||||
assert adapter._require_mention is True
|
||||
await adapter._on_message(self._make_ctx(self._make_activity()))
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
def test_sent_id_tracking_is_bounded(self):
|
||||
adapter = self._make_adapter()
|
||||
for i in range(600):
|
||||
adapter._remember_sent(MagicMock(id=f"sent-{i}"))
|
||||
assert len(adapter._sent_ids) == 500
|
||||
assert "sent-0" not in adapter._sent_id_set
|
||||
assert "sent-599" in adapter._sent_id_set
|
||||
@pytest.mark.parametrize("yaml_value, env_value, expected", [
|
||||
(None, None, False), # opt-in: absent key leaves every conversation ungated
|
||||
(True, None, True),
|
||||
("false", None, False),
|
||||
(True, "false", False), # explicit env beats YAML, like MATRIX_/MATTERMOST_REQUIRE_MENTION
|
||||
(False, "true", True),
|
||||
])
|
||||
def test_require_mention_read_env_over_yaml(self, monkeypatch, yaml_value, env_value, expected):
|
||||
monkeypatch.delenv("TEAMS_REQUIRE_MENTION", raising=False)
|
||||
if env_value is not None:
|
||||
monkeypatch.setenv("TEAMS_REQUIRE_MENTION", env_value)
|
||||
extra = {} if yaml_value is None else {"require_mention": yaml_value}
|
||||
adapter = self._make_adapter(**extra)
|
||||
assert adapter._require_mention is expected
|
||||
assert adapter._extra.get("require_mention") == yaml_value # extras stay readable on the instance
|
||||
|
||||
@@ -22,6 +22,8 @@ Need meeting summaries from Microsoft Graph events rather than normal bot conver
|
||||
|
||||
Teams delivers @mentions as regular messages with `<at>BotName</at>` tags, which Hermes strips automatically before processing.
|
||||
|
||||
Without resource-specific consent (RSC) Teams only delivers messages that @mention the bot, so no filtering is needed. Once the app manifest grants `ChannelMessage.Read.Group` or `ChatMessage.Read.Chat`, Teams delivers **every** message in the conversation — set `require_mention: true` (or `TEAMS_REQUIRE_MENTION=true`) so the bot only answers channel/group-chat messages that @mention it or reply to one of its own messages. Personal chats are never gated, and a gated message is dropped before its attachments are downloaded.
|
||||
|
||||
---
|
||||
|
||||
For source or local installs, include the Teams extra so the bundled adapter can
|
||||
@@ -168,6 +170,7 @@ Open the printed link in your browser — it opens directly in the Teams client.
|
||||
| `TEAMS_HOME_CHANNEL` | Conversation ID for cron/proactive message delivery |
|
||||
| `TEAMS_HOME_CHANNEL_NAME` | Display name for the home channel |
|
||||
| `TEAMS_PORT` | Webhook port (default: `3978`) |
|
||||
| `TEAMS_REQUIRE_MENTION` | Set `true` to answer only @mentions / replies to the bot in channels and group chats (default: `false`; for apps with RSC message-read consent) |
|
||||
|
||||
### config.yaml
|
||||
|
||||
@@ -182,6 +185,7 @@ platforms:
|
||||
client_secret: "your-secret"
|
||||
tenant_id: "your-tenant-id"
|
||||
port: 3978
|
||||
require_mention: false # true once the app has RSC message-read consent
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
Reference in New Issue
Block a user