Repository navigation
fix(teams): support microsoft-teams-apps 2.1, lift cap to <2.2 (#250) - #262
Conversation
- 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.
|
Warning Review limit reachedNext included review available in 30 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
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. Comment |
…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.
# Conflicts: # CHANGELOG.md
|
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). |
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.1cap from #251 becomes<2.2in all three dependency lists inpyproject.toml([teams],[all]and thedevgroup).microsoft-teams-commonis now declared too (same<2.2cap): the adapter importsClientOptionsfrom it directly, andmicrosoft-teams-apps2.1 leaves it unbounded.What changed
_create_streamer). 2.1.0 removedApp.activity_sender.activity_sender(2.0.x), the adapter callsactivity_sender.create_stream(ref)as before.HttpStream(app.api.from_service_url(ref.service_url), ref). This matches what 2.1's ownActivityContext.streamdoes._point_app_api_atchanges the sharedApp.apiURL for every outbound call, so a stream using that shared client could have its chunks sent to another region mid-stream.edit_message/delete_message: no runtime change was needed. The issue said the new activitiesupdatesignature broke retargeting. It did not:app.api.conversations.activities(id).update/deletestill exists on 2.1. It now goes throughconversations.update_activity(..., service_url=None), which uses the client URL that was just retargeted. Only the test's fakeupdatebroke, because it did not accept the newservice_url=/agentic_identity=keywords.conversations.update_activity/delete_activitymethods are not used, because the 2.0.13.4 floor does not have them.putand asserts the exact request URL. I added the same test for delete.TokenValidator.for_servicewithInboundActivityTokenValidator.tid, and theserviceurlclaim is not checked.iss, so an unsigned token naming any tenant makes it build a per-tidvalidator and fetch that tenant's JWKS (a blocking HTTP call) before rejecting.BridgeHttpAdaptertakes areject_before_authhook, called with the request headers before the SDK route handler. The adapter's_rejects_before_authdecodes the Bearer token without verification and answers 401 unlessiss == app.cloud.token_issuer(sovereign clouds followCLOUD). No Entra JWKS is ever fetched; a passing token still gets the SDK's full validation._dispatch_activityrepeats the issuer check on the SDK-validatedJsonWebTokenand returns 401 before any handler or state write.Bearerheader, and all requests indangerously_allow_unauthenticated_requests/skip_authmode (read fromApp.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.1still depends on@microsoft/teams.*^2.0.14and callsctx.stream/app.api.conversations.activities(...). No upstream tests to port.Tests
tests/test_teams_native_streaming.py::TestCreateStreamer, rewritten:App.apiand stays on the inbound URL after_point_app_api_atretargetsApp.api.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 → SDKHttpServer→ SDK validator →_dispatch_activitypath with RS256 tokens signed by a test RSA key. Only JWKS key resolution is stubbed (PyJWKClient.get_signing_key_from_jwtreturns the test public key and records the JWKS URI asked for), so the SDK's signature, issuer, audience, expiry andserviceurlchecks all run.serviceurlis still refused by the SDK.CLOUD=USGov:https://api.botframework.usis dispatched,https://api.botframework.comgets 401._dispatch_activity, and unauthenticated mode (pre-check stays out of the way), each have their own test.tests/test_teams_adapter.py::TestSdkDependencyDeclarations(new): everymicrosoft_teams.<pkg>imported by the adapter is declared with an upper bound in[teams],[all]anddev(fails withmicrosoft-teams-commonremoved).tests/test_fixture_replay.py: the Teams skip-auth helper now sets the App's own unauthenticated option beforeApp.initializeinstead of patchingHttpServer.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,--strictat chat@4.31.0, pytest) pluspyrefly check:microsoft-teams-apps/api/cards)uv pip install 'microsoft-teams-apps==2.1.*' 'microsoft-teams-api==2.1.*' 'microsoft-teams-cards==2.1.*')On main with 2.1 installed, 3 tests failed: the two
TestCreateStreamertests andtest_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/putstubbed. It drives a realHttpStreamfrom_create_streamer, retargetsApp.apito 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.jsonis unchanged, so it is not committed.Deviations from the issue
updatesignature change breaks retargeting" was a test-double problem, not a runtime one (see above). No production change for edit or delete.<2.2rather than open-ended, following the issue's note to use tighter bounds for fast-moving SDKs.Consumer impact
chat-sdk[teams]installs now resolvemicrosoft-teams-apps2.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-appsresolves apps 2.0.16 with api/cards/common 2.1.0, an unreviewed mix.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
Merge gate
Final HEAD:
09c235f(includesorigin/mainat0cec634).Independent review findings (6)
issstill made the bot fetch an attacker-chosen tenant's JWKS (blocking) before 401reject_before_authhook 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 printsfetched: []on 2.1.0TestInboundTokenIssuerstubbed all ofTokenValidator.validate_token, not just the signaturePyJWKClient.get_signing_key_from_jwtstubbed; added audience/serviceurlstill-enforced testapp.cloud.token_issuerCLOUD=USGovtest (hard-coding the public issuer now fails it)microsoft-teams-apps<2.1" does not keep you on 2.0.x; "full suite on both" overstated CI coveragemicrosoft-teams-commonimported directly but undeclared/unboundedmicrosoft-teams-common>=2.0.13,<2.2in[teams],[all],dev;TestSdkDependencyDeclarations_stream_via_emitawaitsid_capturedwith no timeout; a terminal non-cancel 403 hangs the DM handler_stream_via_emitplaceholder-aware streaming); details and repro posted theregpt-6-astra: 2 rounds, both clean. Round 1 on
b6489f9: no actionable regressions. Round 2 on the final HEAD09c235f(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,--strictat 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)(nofidelity_target.jsonchange).Still owed before release: the live Teams check listed under "Needs the maintainer".
Closes #250
Part of #184