Skip to content

[4.41/T3] Teams inbound: author email hydration, protected inline attachments, secure downloads #218

Description

@patrick-chinchill

Summary

Brings Teams inbound handling up to upstream 4.41:

  1. Author email (Author.email from [4.41/C2a] Core mentions & message model: tri-state is_mention, mention regex, Author.email/is_system, Message.reply_to #192), hydrated from the Bot Framework conversation-members API (no Graph User.Read.All needed), with UPN fallback and a state cache.
  2. Protected inline images. Pasted images live on the connector and fail anonymous GETs today. They get a bot-authenticated path that sends the token only to the activity's own connector origin.
  3. Anonymous downloads go through the [4.41/SH1] Shared guarded downloader (redirect policy, byte cap, timeout, credential host binding) #204 guarded downloader (size cap, timeout, per-hop redirect checks).

Upstream changes

  • 46681f50 fix(teams): hydrate incoming author email (#711) — chat@4.35.0 — sets message.author.email from a Graph user lookup, cached under teams:userInfo:{aadObjectId} (1h) with a 5-min "unresolvable" negative sentinel.
  • 3895ab3f fix(teams): fall back to user principal name for email (#708) — chat@4.35.0 — email = mail ?? userPrincipalName.
  • 63997aca fix(teams): hydrate incoming users without Graph (#860) — chat@4.39.0 — messages with from.aadObjectId use api.conversations.getMemberById(conversation.id, from.id); email = member.email ?? member.userPrincipalName, cached 1h. Failure only logs and does not write the negative sentinel (Graph getUser() still does).
  • 3c37cfbc fix(teams): authenticate protected inline attachments (#749) — chat@4.36.0 — new attachments.ts:
    • Non-file-card attachments on the activity serviceUrl origin (https, or http on loopback for the Emulator) record fetchMetadata {url, auth:"bot", connectorOrigin} and are fetched with the bot token, no redirects, origin re-checked at fetch time. A rehydrated auth:"bot" entry without connectorOrigin is rejected.
    • File cards (file.download.info) use content.downloadUrl anonymously; MIME is inferred from fileType or the extension.
  • bb926884 fix(teams): secure attachment downloads (#850) — chat@4.39.0 — anonymous downloads go through the shared downloadAttachment: connection-bound DNS/private-IP rejection, per-redirect revalidation, a 25 MB streaming cap and a request timeout.

Current Python behavior

  • src/chat_sdk/types.py:238-248 Author has no email field; [4.41/C2a] Core mentions & message model: tri-state is_mention, mention regex, Author.email/is_system, Message.reply_to #192 adds it.
  • teams/adapter.py:730-918 _handle_message_activity does no user lookup.
  • get_user (:439-506) is a hand-rolled Graph call: email=graph_user.get("mail") (:505), no UPN fallback, cache or sentinel.
  • grep -rn -E "teams:userInfo|get_by_id|user_principal_name|connector_origin|connectorOrigin" src/ finds nothing.
  • :1232-1266 _create_attachment(att) has no service-URL input. It prefers content.downloadUrl for file.download.info and uses the raw contentType as the MIME type.
  • :1310-1333 _build_teams_fetch_data always does an anonymous session.get(url) after the host allowlist _is_trusted_teams_download_url (:1268-1308). The request follows redirects (the aiohttp default), reads the whole body, and has no size cap and no explicit timeout (the bare aiohttp.ClientSession() at :2396 has only aiohttp's 5-min default).
  • rehydrate_attachment (:1335-1361) rebuilds the same closure.

Scope

  • Add _get_incoming_user(activity, user_id, aad_object_id) mirroring upstream getIncomingUser: read teams:userInfo:{aad} (ignore the sentinel); on a miss call api.conversations.members(conversation_id).get(user_id) on the activity's service URL; email = member.email, else user_principal_name; cache 1h on a pinned task; on failure warn and return None.
  • _handle_message_activity (group and DM paths, before dispatch): with from.aadObjectId, await _get_incoming_user; otherwise await get_user. Set message.author.email when the lookup returns one.
  • get_user: same cache key; a cached sentinel returns None; mail → userPrincipalName fallback; cache successes 1h and failures as "unresolvable" for 5 min.
  • New teams/attachments.py (delegated to by _create_attachment / rehydrate_attachment): create_teams_attachment(att, service_url, fetchers), rehydrate_teams_attachment(attachment, fetchers), _connector_origin(url) (https, or http on loopback localhost / 127.x.x.x / [::1]), and the file MIME map. _parse_teams_message passes activity["serviceUrl"].
  • _fetch_authenticated_attachment(url):
    • Re-check _connector_origin(url) == connector_origin.
    • Send the bot token with no redirects, as upstream does (maxRedirects: 0). Add a 25 MB cap as Python-only hardening, since upstream's authenticated path has none, and record it as a divergence row.
    • Rejected or failed fetches raise NetworkError("teams", ...). The rejection message is "Refusing to send a bot token to an untrusted attachment URL".
  • Anonymous fetch goes through the [4.41/SH1] Shared guarded downloader (redirect policy, byte cap, timeout, credential host binding) #204 downloader, with _is_trusted_teams_download_url kept in front of it as defense in depth.
  • docs/UPSTREAM_SYNC.md: :667 becomes parity (upstream now also uses content.downloadUrl for file cards); :666 notes that upstream now validates too.

Out of scope

Porting notes

  • a ?? b → explicit is not None. Upstream ?? keeps member.email == ""; recommended default: treat "" as missing and document it.
  • Upstream's state write is fire-and-forget (.catch(() => {})): use a pinned asyncio.Task with an error-swallowing done-callback (_pin_task, google_chat/adapter.py:93). No bare un-awaited coroutines.
  • Cache values are JSON in the camelCase UserInfo wire shape (userId, userName, fullName, isBot, email, avatarUrl); fetch_metadata keys stay camelCase (auth, connectorOrigin). Both are for cross-SDK state compatibility.
  • The miss path adds one awaited call before dispatch: bound it (recommended 5s) and never raise into dispatch.
  • Bot-token GET: recommended transport is the SDK's self._app.api.http (httpx, token injected, redirects off by default). Enforce the byte cap on Content-Length and on the streamed body.
  • Compare origins with lowercased scheme/host and explicit ports kept. urlparse(...).hostname strips IPv6 brackets, so match loopback on the parsed hostname.

Tests

Adapter tests are not fidelity-mapped.

Port from packages/adapter-teams/src/index.test.ts:

  • › "incoming sender email": "hydrates email from the conversation member without Graph", "falls back to the conversation member user principal name", "replaces a failed Graph lookup cached for the sender", "falls back to the cached AAD object ID", "dispatches the message when conversation member lookup fails", "caches the conversation member lookup across messages", "does not let a failed conversation lookup suppress retries", "hydrates email on the DM path and completes processing"
  • › "getUser": "should fall back to userPrincipalName when mail is missing"
  • › "parseMessage": "authenticates trusted inline attachment downloads", "rejects internal file download URLs from activities", "preserves anonymous fetch overrides during rehydration"

From attachments.test.ts › "Teams attachments":

  • "downloads file cards anonymously and infers their MIME type"
  • it.each "infers the MIME type for %s file cards"
  • "keeps cross-origin inline attachments anonymous"
  • "rejects plain-HTTP inline attachment downloads by default"
  • "authenticates emulator attachments on a loopback HTTP connector"
  • "rehydrates both retrieval modes and revalidates bot destinations"
  • it.each "rejects internal anonymous URL %s after rehydration"

Python-specific: a tampered connectorOrigin in rehydrated metadata sends no token; a connector redirect errors and is not followed; an oversized body is rejected. Use AsyncMock for the members client and state, and a fake transport, not the network.

Acceptance criteria

  • Full validation command from CLAUDE.md passes.
  • docs/UPSTREAM_SYNC.md rows :666 and :667 updated, plus any new divergence (e.g. the "" email handling or the retained host allowlist).
  • CHANGELOG entry under "Unreleased (4.41 wave)".
  • Consumer-visible changes called out: message.author.email populated on Teams; a cache miss adds one network call before dispatch; inline pasted images become downloadable; downloads over 25 MB now fail.

Dependencies

Blocked by #192, #204.

Verify first

  • uv.lock pins microsoft-teams-apps 2.0.13.4; CI resolved 2.0.16 (test(teams): make the skip-auth fixture survive the SDK's flag rename #180). In 2.0.13.4, api.conversations.members(conv).get(id) → get_by_id returns a TeamsChannelAccount with email / user_principal_name (api/clients/conversation/client.py:66, member.py:73). Confirm on 2.0.16, plus that ApiClient.http still injects the token and does not follow redirects.

Metadata

  • Effort: L
  • Consumer impact: high for Teams: new author data, cache-miss latency, attachment behaviour changes (inline images work; large files capped).
  • Suggested branch: sync/4.41-t3

Part of #184.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions