fix(msteams): secure inbound defaults and ref persistence
Default Microsoft Teams inbound auth validation to enabled, update the README to match, and prevent denied senders from persisting conversation refs before allowlist checks pass. Made-with: Cursor
This commit is contained in:
@@ -910,7 +910,7 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess
|
||||
|
||||
> - `replyInThread: true` replies to the triggering Teams activity when a stored `activity_id` is available.
|
||||
> - `mentionOnlyResponse` controls what Nanobot receives when a user sends only a bot mention (`<at>Nanobot</at>`). Set to `""` to ignore mention-only messages.
|
||||
> - `validateInboundAuth: true` (recommended for production) enables inbound Bot Framework bearer-token validation (signature, issuer, audience, lifetime, `serviceUrl`). **Default is `false`** — set explicitly to `true` for production deployments.
|
||||
> - `validateInboundAuth: true` enables inbound Bot Framework bearer-token validation (signature, issuer, audience, lifetime, `serviceUrl`). This is the safe default for public deployments. Only set it to `false` for local development or tightly controlled testing.
|
||||
|
||||
**4. Run**
|
||||
|
||||
|
||||
@@ -57,7 +57,7 @@ class MSTeamsConfig(Base):
|
||||
allow_from: list[str] = Field(default_factory=list)
|
||||
reply_in_thread: bool = True
|
||||
mention_only_response: str = "Hi — what can I help with?"
|
||||
validate_inbound_auth: bool = False
|
||||
validate_inbound_auth: bool = True
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -116,9 +116,9 @@ class MSTeamsChannel(BaseChannel):
|
||||
|
||||
if not self.config.validate_inbound_auth:
|
||||
logger.warning(
|
||||
"MSTeams inbound auth validation is DISABLED. "
|
||||
"MSTeams inbound auth validation was explicitly DISABLED in config. "
|
||||
"Anyone who knows the webhook URL can send messages as any user. "
|
||||
"Set validateInboundAuth: true in config for production use."
|
||||
"Only disable this for local development or controlled testing."
|
||||
)
|
||||
|
||||
self._loop = asyncio.get_running_loop()
|
||||
@@ -274,6 +274,14 @@ class MSTeamsChannel(BaseChannel):
|
||||
logger.debug("MSTeams ignoring empty message after Teams text sanitization")
|
||||
return
|
||||
|
||||
if not self.is_allowed(sender_id):
|
||||
logger.warning(
|
||||
"Access denied for sender {} on channel {}. "
|
||||
"Add them to allowFrom list in config to grant access.",
|
||||
sender_id, self.name,
|
||||
)
|
||||
return
|
||||
|
||||
self._conversation_refs[conversation_id] = ConversationRef(
|
||||
service_url=service_url,
|
||||
conversation_id=conversation_id,
|
||||
|
||||
@@ -147,6 +147,40 @@ async def test_handle_activity_ignores_group_messages(make_channel):
|
||||
assert ch._conversation_refs == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handle_activity_denied_sender_does_not_store_ref(make_channel, tmp_path):
|
||||
ch = make_channel(allowFrom=["allowed-user"])
|
||||
|
||||
activity = {
|
||||
"type": "message",
|
||||
"id": "activity-denied",
|
||||
"text": "Hello from denied user",
|
||||
"serviceUrl": "https://smba.trafficmanager.net/amer/",
|
||||
"conversation": {
|
||||
"id": "conv-denied",
|
||||
"conversationType": "personal",
|
||||
},
|
||||
"from": {
|
||||
"id": "29:user-id",
|
||||
"aadObjectId": "aad-user-1",
|
||||
"name": "Bob",
|
||||
},
|
||||
"recipient": {
|
||||
"id": "28:bot-id",
|
||||
"name": "nanobot",
|
||||
},
|
||||
"channelData": {
|
||||
"tenant": {"id": "tenant-id"},
|
||||
},
|
||||
}
|
||||
|
||||
await ch._handle_activity(activity)
|
||||
|
||||
assert ch.bus.inbound == []
|
||||
assert ch._conversation_refs == {}
|
||||
assert not (tmp_path / "state" / "msteams_conversations.json").exists()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handle_activity_mention_only_uses_default_response(make_channel):
|
||||
ch = make_channel()
|
||||
@@ -520,6 +554,7 @@ async def test_start_logs_install_hint_when_pyjwt_missing(make_channel, monkeypa
|
||||
def test_msteams_default_config_includes_restart_notify_fields():
|
||||
cfg = MSTeamsChannel.default_config()
|
||||
|
||||
assert cfg["validateInboundAuth"] is True
|
||||
assert "restartNotifyEnabled" not in cfg
|
||||
assert "restartNotifyPreMessage" not in cfg
|
||||
assert "restartNotifyPostMessage" not in cfg
|
||||
|
||||
Reference in New Issue
Block a user