Skip to content

fix(teams): support microsoft-teams-apps 2.1, lift cap to <2.2 (#250) - #262

Merged
patrick-chinchill merged 5 commits into
mainfrom
fix/teams-sdk-2.1
Sep 30, 2026
Merged

patrick-chinchill merged 5 commits into
mainfrom
fix/teams-sdk-2.1

Conversation

@patrick-chinchill

@patrick-chinchill patrick-chinchill commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This makes the Teams adapter work on microsoft-teams-{apps,api,cards,common} 2.1 and keeps it working on 2.0.x. The adapter checks which SDK features are present instead of reading the version number. The <2.1 cap from #251 becomes <2.2 in all three dependency lists in pyproject.toml ([teams], [all] and the dev group). microsoft-teams-common is now declared too (same <2.2 cap): the adapter imports ClientOptions from it directly, and microsoft-teams-apps 2.1 leaves it unbounded.

What changed

  • Native DM streaming (_create_streamer). 2.1.0 removed App.activity_sender.
    • If the App has activity_sender (2.0.x), the adapter calls activity_sender.create_stream(ref) as before.
    • Otherwise (2.1.x) it builds HttpStream(app.api.from_service_url(ref.service_url), ref). This matches what 2.1's own ActivityContext.stream does.
    • On both versions the stream gets its own client, fixed to the inbound activity's service URL. _point_app_api_at changes the shared App.api URL for every outbound call, so a stream using that shared client could have its chunks sent to another region mid-stream.
    • If the SDK has neither entry point, the adapter falls back to buffered posting.
  • edit_message / delete_message: no runtime change was needed. The issue said the new activities update signature broke retargeting. It did not: app.api.conversations.activities(id).update/delete still exists on 2.1. It now goes through conversations.update_activity(..., service_url=None), which uses the client URL that was just retargeted. Only the test's fake update broke, because it did not accept the new service_url= / agentic_identity= keywords.
    • The flattened conversations.update_activity / delete_activity methods are not used, because the 2.0.13.4 floor does not have them.
    • The edit test now patches the real activities client's HTTP put and asserts the exact request URL. I added the same test for delete.
  • Inbound auth (security; not listed in the issue, see Deviations). 2.1 replaced the Bot Framework-only TokenValidator.for_service with InboundActivityTokenValidator.
    • The new validator also accepts Entra ID ("Agent 365 Agent ID") tokens whose audience is our app id. They can come from any tenant: the issuer is taken from the token's tid, and the serviceurl claim is not checked.
    • The adapter does not support Agent ID activities, so lifting the cap without a guard would have widened who can deliver activities to the bot.
    • The validator also picks the Entra branch from the token's unverified iss, so an unsigned token naming any tenant makes it build a per-tid validator and fetch that tenant's JWKS (a blocking HTTP call) before rejecting.
    • Pre-validation check: BridgeHttpAdapter takes a reject_before_auth hook, called with the request headers before the SDK route handler. The adapter's _rejects_before_auth decodes the Bearer token without verification and answers 401 unless iss == app.cloud.token_issuer (sovereign clouds follow CLOUD). No Entra JWKS is ever fetched; a passing token still gets the SDK's full validation.
    • Post-validation check (defence in depth): _dispatch_activity repeats the issuer check on the SDK-validated JsonWebToken and returns 401 before any handler or state write.
    • Requests without a Bearer header, and all requests in dangerously_allow_unauthenticated_requests / skip_auth mode (read from App.options), go to the SDK unchanged. On 2.0.x the SDK already enforces the issuer; the pre-check only spares the Bot Framework JWKS fetch for tokens it would reject.

Upstream commits mapped

None. This is Python-only SDK compatibility. @chat-adapter/teams@4.41.1 still depends on @microsoft/teams.* ^2.0.14 and calls ctx.stream / app.api.conversations.activities(...). No upstream tests to port.

Tests

  • tests/test_teams_native_streaming.py::TestCreateStreamer, rewritten:
    • Both SDK shapes are forced with stubs, so both branches run on either installed SDK.
    • An unstubbed real-SDK test checks that the stream's client is not App.api and stays on the inbound URL after _point_app_api_at retargets App.api.
    • Also covered: the "neither entry point" fallback, and the existing no-serviceUrl / SSRF / raise cases.
  • tests/test_teams_adapter.py::TestOutboundServiceUrlRouting: edit (rewritten) and delete (new) are asserted at the HTTP boundary.
  • tests/test_teams_adapter.py::TestInboundTokenIssuer (new): runs the real bridge → SDK HttpServer → SDK validator → _dispatch_activity path with RS256 tokens signed by a test RSA key. Only JWKS key resolution is stubbed (PyJWKClient.get_signing_key_from_jwt returns the test public key and records the JWKS URI asked for), so the SDK's signature, issuer, audience, expiry and serviceurl checks all run.
    • A correctly signed Entra token gets 401, no dispatch, no JWKS fetched; unsigned tokens naming attacker tenants (v2 and v1 issuer shapes) fetch nothing.
    • A Bot Framework token is dispatched (only the Bot Framework JWKS is asked for), and one with a wrong audience or serviceurl is still refused by the SDK.
    • CLOUD=USGov: https://api.botframework.us is dispatched, https://api.botframework.com gets 401.
    • The post-validation guard in _dispatch_activity, and unauthenticated mode (pre-check stays out of the way), each have their own test.
    • Mutation checks (each fails at least one test): pre-check not wired, bridge ignoring the hook, issuer hard-coded to the public cloud, post guard removed, auth-disabled check removed.
  • tests/test_teams_adapter.py::TestSdkDependencyDeclarations (new): every microsoft_teams.<pkg> imported by the adapter is declared with an upper bound in [teams], [all] and dev (fails with microsoft-teams-common removed).
  • tests/test_fixture_replay.py: the Teams skip-auth helper now sets the App's own unauthenticated option before App.initialize instead of patching HttpServer.initialize, so the adapter sees the same mode the SDK does.

Validation

Full validation (ruff check, ruff format --check, audit_test_quality, verify_test_fidelity --check-docs, --strict at chat@4.31.0, pytest) plus pyrefly check:

SDK (microsoft-teams-apps/api/cards) pytest strict fidelity pyrefly
2.0.16 (local default env) 5567 passed, 13 skipped 733/733 0 errors
2.1.0 (uv pip install 'microsoft-teams-apps==2.1.*' 'microsoft-teams-api==2.1.*' 'microsoft-teams-cards==2.1.*') 5567 passed, 13 skipped 733/733 0 errors
2.0.13.4 (the declared floor) 5567 passed, 13 skipped n/a n/a

On main with 2.1 installed, 3 tests failed: the two TestCreateStreamer tests and test_edit_message_retargets_real_activities_client.

I also ran a local wire probe on both 2.0.16 and 2.1.0, with the SDK Client.post / put stubbed. It drives a real HttpStream from _create_streamer, retargets App.api to another region mid-stream, and checks every streaming, typing and final POST. All of them still went to the inbound service URL.

Fidelity target: Delta vs committed report (HEAD): missing 282 -> 282 (+0). scripts/fidelity_target.json is unchanged, so it is not committed.

Deviations from the issue

  • The issue's "update signature change breaks retargeting" was a test-double problem, not a runtime one (see above). No production change for edit or delete.
  • Added the inbound issuer guard. The issue did not list it, but without it lifting the cap would widen inbound auth on 2.1.
  • The upper bound is <2.2 rather than open-ended, following the issue's note to use tighter bounds for fast-moving SDKs.

Consumer impact

  • Fresh chat-sdk[teams] installs now resolve microsoft-teams-apps 2.1.x. To stay on 2.0.x, pin all four SDK packages together (microsoft-teams-apps<2.1 microsoft-teams-api<2.1 microsoft-teams-cards<2.1 microsoft-teams-common<2.1); pinning only -apps resolves apps 2.0.16 with api/cards/common 2.1.0, an unreviewed mix.
  • On 2.1, inbound activities carrying Entra ID (Agent ID) tokens get 401 before any JWKS fetch. This matches 2.0.x's accepted-token set.
  • CI's uv sync --group dev (no committed lock) will now test 2.1.0, so 2.0.x is only covered by local runs. A CI matrix over both SDK lines would be a follow-up.

Needs the maintainer

  • A live Teams check is not possible from here and is still owed before release: native DM streaming, edit and delete on 2.1, and a normal inbound message (Bot Framework token accepted).

Merge gate

Final HEAD: 09c235f (includes origin/main at 0cec634).

Independent review findings (6)

# Finding Outcome
1 Issuer guard ran after the SDK validator, so on 2.1 an unauthenticated token with an Entra iss still made the bot fetch an attacker-chosen tenant's JWKS (blocking) before 401 Fixed: pre-validation reject_before_auth hook in the bridge (_rejects_before_auth), post-validation guard kept as defence in depth; tests assert no JWKS URI is asked for. Reviewer's probe now prints fetched: [] on 2.1.0
2 TestInboundTokenIssuer stubbed all of TokenValidator.validate_token, not just the signature Fixed: RS256 test key, only PyJWKClient.get_signing_key_from_jwt stubbed; added audience/serviceurl still-enforced test
3 No sovereign-cloud coverage for app.cloud.token_issuer Fixed: CLOUD=USGov test (hard-coding the public issuer now fails it)
4 CHANGELOG "pin microsoft-teams-apps<2.1" does not keep you on 2.0.x; "full suite on both" overstated CI coverage Fixed: CHANGELOG and this body say pin all four packages; 2.0.16 validated locally, CI covers 2.1.x only
5 microsoft-teams-common imported directly but undeclared/unbounded Fixed: microsoft-teams-common>=2.0.13,<2.2 in [teams], [all], dev; TestSdkDependencyDeclarations
6 _stream_via_emit awaits id_captured with no timeout; a terminal non-cancel 403 hangs the DM handler Declined (out of scope): pre-existing on main (same on 2.0.16, which main already installs); not touched by this PR. Belongs to #219 (_stream_via_emit placeholder-aware streaming); details and repro posted there

gpt-6-astra: 2 rounds, both clean. Round 1 on b6489f9: no actionable regressions. Round 2 on the final HEAD 09c235f (after merging main): "No actionable regressions found."

Bots: CodeRabbit was rate-limited (no review comments); no gemini comments; no inline review comments.

CI (final HEAD): Tests 3.12 / 3.13, Lint & Type Check, CodeQL (python, actions): all green.

Local validation (final HEAD, SDK 2.1.0): ruff check/format, audit_test_quality (0 hard failures), --check-docs, --strict at chat@4.31.0 (all TS tests have Python equivalents), pytest 5771 passed / 24 skipped, pyrefly 0 errors. Teams + fixture-replay tests (576) also pass on 2.0.16 and 2.0.13.4. Fidelity target: Delta vs committed report (HEAD): missing 256 -> 256 (+0) (no fidelity_target.json change).

Still owed before release: the live Teams check listed under "Needs the maintainer".

Closes #250
Part of #184

- Native DM streaming: feature-detect App.activity_sender (2.0.x); on 2.1.x
  build HttpStream on app.api.from_service_url(ref.service_url), mirroring
  2.1's ActivityContext.stream. The stream keeps its own client pinned to the
  inbound service URL on both lines.
- Inbound auth: reject (401) SDK-validated tokens not issued by the cloud's
  Bot Framework issuer. 2.1's InboundActivityTokenValidator also accepts
  Entra ID (Agent ID) tokens from any tenant; this keeps 2.0.x semantics.
- edit/delete: no runtime change; tests now assert the wire URL at the HTTP
  boundary so they hold on both SDK lines.
- pyproject: microsoft-teams-{apps,api,cards} >=2.0.13,<2.2 in all three lists.

Validated on 2.0.13.4, 2.0.16 and 2.1.0.
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 30 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 704446cd-7e1a-425e-ac63-d72b1c89618f

📥 Commits

Reviewing files that changed from the base of the PR and between d94c888 and cc561c3.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • docs/UPSTREAM_SYNC.md
  • pyproject.toml
  • src/chat_sdk/adapters/teams/adapter.py
  • src/chat_sdk/adapters/teams/bridge.py
  • src/chat_sdk/adapters/teams/streamer.py
  • tests/test_fixture_replay.py
  • tests/test_teams_adapter.py
  • tests/test_teams_native_streaming.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…eclare microsoft-teams-common (#250)

Address PR #262 review:
- Pre-validation issuer check in the bridge so 2.1's Entra branch (per-tid
  JWKS fetch) is never reached by unauthenticated requests.
- TestInboundTokenIssuer signs RS256 tokens and stubs only JWKS key
  resolution; adds sovereign-cloud, no-JWKS-fetch, audience/serviceurl,
  post-guard and unauthenticated-mode cases.
- Declare microsoft-teams-common>=2.0.13,<2.2 in [teams]/[all]/dev.
- CHANGELOG: pin all four SDK packages to stay on 2.0.x; CI covers 2.1 only.
@patrick-chinchill

Copy link
Copy Markdown
Collaborator Author

Merge gate: CI green on cc561c3 (test 3.12, test 3.13, Lint & Type Check, CodeQL, Analyze python/actions). cc561c3 is 09c235f plus a conflict-free merge of origin/main (#223 gchat, #236 whatsapp), which does not touch any Teams module. Local validation on cc561c3 is green: 5904 passed, strict fidelity complete, pyrefly 0 errors, fidelity target delta +0. Local Codex review (gpt-6-astra, xhigh, --base origin/main) on 09c235f: "No actionable regressions found. All 585 Teams and fixture-replay tests passed with SDK 2.1.0." No fresh review was run because the main merge does not overlap the reviewed code. 2 astra rounds, both clean. Bots: CodeRabbit rate-limited with no review comments, no inline comments. Merging with --admin (Protect Main requires a code-owner approval).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Teams: support microsoft-teams-apps 2.1 (activity_sender removal, activities update signature)

1 participant