feat(teams): migrate inbound + auth to microsoft-teams-apps SDK (#93 PR 1/4)#143
Merged
Conversation
… 1 foundation) Adds microsoft-teams-apps/-api/-cards >=2.0.13 to the [teams] extra for the Teams adapter migration to the official MS SDK (mirrors upstream adapter-teams@4.30.0). Graph stays hand-rolled (no [graph] extra / msgraph-sdk). Verified the SDK's TokenValidator (apps/auth/token_validator.py) validates RS256 + audience(app_id + api:// variants) + issuer(api.botframework.com) via the Bot Framework JWKS — equivalent-or-stronger than the hand-rolled _verify_bot_framework_token block it will replace.
…PR 1/4)
Rewire the Teams adapter's inbound webhook path and JWT authentication to
delegate to the official microsoft-teams-apps Python SDK, mirroring upstream
adapter-teams@chat@4.30.0. Outbound (post/edit/delete/typing), native
streaming, and the Graph reader stay hand-rolled (PRs 2/3 + a separate
decision).
New: src/chat_sdk/adapters/teams/bridge.py
- BridgeHttpAdapter implements the SDK HttpServerAdapter protocol. It captures
the route handler the App registers during app.initialize(), exposes an async
dispatch(request, options) for handle_webhook to call, and owns a per-activity
WebhookOptions map (keyed by activity id, cleared in finally). Framework
agnostic — body/header extraction duck-types across web frameworks; response
shaping returns the {body, status, headers} dict consumers expect. Port of
upstream bridge-adapter.ts.
adapter.py:
- Construct the SDK App from TeamsAdapterConfig via _to_app_options (port of
config.ts toAppOptions): app_id/app_password/app_tenant_id/federated/app_type
-> client_id/client_secret/tenant_id/managed_identity_client_id, with the same
TEAMS_* env fallbacks. Inject the bridge; stamp client User-Agent
"Vercel.ChatSDK".
- initialize() registers SDK handlers (on_message/on_message_reaction/
on_card_action/on_dialog_open/on_dialog_submit/on_conversation_update/
on_install_add/remove), awaits app.initialize(), and overrides
server.on_request with _dispatch_activity.
- handle_webhook now delegates to bridge.dispatch. Inbound JWT validation is
performed by the SDK's TokenValidator (RS256 + audience app_id/api:// variants
+ Bot Framework issuer/JWKS), enforced by default (skip_auth=False). Deleted
the hand-rolled _verify_bot_framework_token / _jwks_client / openid-config
fetch / BOT_FRAMEWORK_OPENID_CONFIG_URL and the now-unused request/response
helpers (body/header extraction + response shaping live in the bridge).
- _handle_teams_error (port of errors.ts handleTeamsError) now maps both the
plain dicts the hand-rolled Graph/outbound path raises and the SDK's typed
exceptions (status on inner_http_error.status_code / status_code / retry_after)
onto AuthenticationError / AdapterPermissionError / AdapterRateLimitError /
NetworkError.
Divergence (SDK-forced, documented in docs/UPSTREAM_SYNC.md): the SDK's default
on_request runs strict per-activity validation + a live token fetch before any
handler. We keep the SDK as the auth + transport layer but route the
already-authenticated lenient CoreActivity ourselves so minimal serverless
payloads and the existing dict-based handler logic keep working unchanged.
Tests: add tests/test_teams_bridge.py (protocol conformance, dispatch happy/
error paths, WebhookOptions lifecycle, body/header extraction). Rework the
JWT-bypass fixtures to force the SDK's own skip_auth instead of patching the
deleted method; add TestSdkInboundAuth asserting the SDK rejects unauthenticated
/ bad-token / unconfigured requests. Add _to_app_options and SDK-exception
error-mapping tests. Point the shared body-extraction test at
BridgeHttpAdapter._read_body.
Public Adapter contract and create_teams_adapter(TeamsAdapterConfig(...))
unchanged. Gauntlet green: ruff/format/audit clean, pyrefly 0 errors,
4793 passed / 3 skipped.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe Teams adapter is migrated from hand-rolled Bot Framework JWT verification and manual activity parsing to the Microsoft Teams Apps SDK. A new ChangesTeams SDK Integration via BridgeHttpAdapter
Sequence Diagram(s)sequenceDiagram
participant Client as Inbound Webhook
participant TeamsAdapter
participant BridgeHttpAdapter
participant TeamsSDKApp as Teams SDK App
participant TeamsSDKServer as SDK HttpServer
participant ActivityRouter as _dispatch_activity
Client->>TeamsAdapter: handle_webhook(request, options)
TeamsAdapter->>BridgeHttpAdapter: dispatch(request, options)
BridgeHttpAdapter->>BridgeHttpAdapter: _read_body / _read_headers
BridgeHttpAdapter->>BridgeHttpAdapter: JSON parse → store WebhookOptions[activity.id]
BridgeHttpAdapter->>TeamsSDKServer: captured_handler({ body, headers })
TeamsSDKServer->>TeamsSDKServer: JWT validation (HttpServer.initialize)
TeamsSDKServer->>ActivityRouter: on_request(event)
ActivityRouter->>ActivityRouter: _activity_to_dict → route by type
ActivityRouter-->>TeamsSDKServer: processed
TeamsSDKServer-->>BridgeHttpAdapter: { status, body }
BridgeHttpAdapter->>BridgeHttpAdapter: clear WebhookOptions (finally)
BridgeHttpAdapter-->>TeamsAdapter: { body, status, headers }
TeamsAdapter-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| import inspect | ||
| import json | ||
| from collections.abc import Iterable |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request integrates the official Microsoft Teams Apps Python SDK into the Teams adapter, replacing hand-rolled JWT verification and request body extraction with the SDK's App and a custom BridgeHttpAdapter. Feedback on these changes suggests returning an empty JSON object instead of None for unhandled invoke activities to prevent client timeouts, passing the adapter's logger to _to_app_options with explicit is not None checks to ensure configuration warnings are logged, and adding a fallback to .dict() for Pydantic v1 compatibility during activity extraction.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
610
to
+622
| elif activity_type == "invoke": | ||
| # Adaptive card actions | ||
| # Adaptive card actions (Action.Execute → invoke). | ||
| action_data = (activity.get("value") or {}).get("action", {}).get("data", {}) | ||
| if action_data.get("actionId"): | ||
| if isinstance(action_data, dict) and action_data.get("actionId"): | ||
| await self._handle_adaptive_card_action(activity, action_data, options) | ||
| return self._make_json_response( | ||
| json.dumps( | ||
| { | ||
| "statusCode": 200, | ||
| "type": "application/vnd.microsoft.activity.message", | ||
| "value": "", | ||
| } | ||
| ), | ||
| 200, | ||
| ) | ||
| return { | ||
| "status": 200, | ||
| "body": { | ||
| "statusCode": 200, | ||
| "type": "application/vnd.microsoft.activity.message", | ||
| "value": "", | ||
| }, | ||
| } |
Contributor
There was a problem hiding this comment.
In the Bot Framework protocol, invoke activities are synchronous and expect a valid JSON response. If an invoke activity is received but does not match the expected actionId check, returning an empty body (None) can cause errors or timeout warnings in the Teams client. Returning an empty JSON object ({}) is the standard way to safely acknowledge unhandled invoke activities.
Suggested change
| elif activity_type == "invoke": | |
| # Adaptive card actions | |
| # Adaptive card actions (Action.Execute → invoke). | |
| action_data = (activity.get("value") or {}).get("action", {}).get("data", {}) | |
| if action_data.get("actionId"): | |
| if isinstance(action_data, dict) and action_data.get("actionId"): | |
| await self._handle_adaptive_card_action(activity, action_data, options) | |
| return self._make_json_response( | |
| json.dumps( | |
| { | |
| "statusCode": 200, | |
| "type": "application/vnd.microsoft.activity.message", | |
| "value": "", | |
| } | |
| ), | |
| 200, | |
| ) | |
| return { | |
| "status": 200, | |
| "body": { | |
| "statusCode": 200, | |
| "type": "application/vnd.microsoft.activity.message", | |
| "value": "", | |
| }, | |
| } | |
| elif activity_type == "invoke": | |
| # Adaptive card actions (Action.Execute → invoke). | |
| action_data = (activity.get("value") or {}).get("action", {}).get("data", {}) | |
| if isinstance(action_data, dict) and action_data.get("actionId"): | |
| await self._handle_adaptive_card_action(activity, action_data, options) | |
| return { | |
| "status": 200, | |
| "body": { | |
| "statusCode": 200, | |
| "type": "application/vnd.microsoft.activity.message", | |
| "value": "", | |
| }, | |
| } | |
| return {"status": 200, "body": {}} |
| # Bot Framework OpenID configuration URL for JWT verification | ||
| BOT_FRAMEWORK_OPENID_CONFIG_URL = "https://login.botframework.com/v1/.well-known/openid-configuration" | ||
|
|
||
| def _to_app_options(config: TeamsAdapterConfig) -> dict[str, Any]: |
Contributor
There was a problem hiding this comment.
The _to_app_options function is a module-level helper that only has access to config. If the user does not provide a custom logger in TeamsAdapterConfig, config.logger is None, and the warning about client_audience being ignored will never be logged. Passing the adapter's initialized logger (which defaults to ConsoleLogger) as an optional parameter ensures this warning is always visible.
Suggested change
| def _to_app_options(config: TeamsAdapterConfig) -> dict[str, Any]: | |
| def _to_app_options(config: TeamsAdapterConfig, logger: Logger | None = None) -> dict[str, Any]: |
Comment on lines
+202
to
+204
| federated = config.federated | ||
| if federated is not None and federated.get("client_audience") and config.logger is not None: | ||
| config.logger.warn("federated.clientAudience is not supported by the Teams SDK and will be ignored.") |
Contributor
There was a problem hiding this comment.
Use the passed-in logger parameter (falling back to config.logger) to log the warning about client_audience being ignored. Use is not None to check for the presence of client_audience to avoid silently ignoring falsy but valid values (like an empty string).
Suggested change
| federated = config.federated | |
| if federated is not None and federated.get("client_audience") and config.logger is not None: | |
| config.logger.warn("federated.clientAudience is not supported by the Teams SDK and will be ignored.") | |
| federated = config.federated | |
| if federated is not None and federated.get("client_audience") is not None: | |
| log = logger or config.logger | |
| if log is not None: | |
| log.warn("federated.clientAudience is not supported by the Teams SDK and will be ignored.") |
References
- When checking for optional values that can be falsy but valid (e.g., 0, empty string, empty list), use
is not Noneinstead of a truthiness check to avoid silently ignoring them.
| "The Teams adapter requires the 'teams' extra. Install it with: pip install 'chat-sdk[teams]'" | ||
| ) from exc | ||
|
|
||
| options = _to_app_options(config) |
Contributor
There was a problem hiding this comment.
Pass the adapter's initialized self._logger to _to_app_options so that any configuration warnings are properly logged even if no custom logger was provided in TeamsAdapterConfig.
Suggested change
| options = _to_app_options(config) | |
| options = _to_app_options(config, self._logger) |
Comment on lines
+640
to
+644
| model_dump = getattr(source, "model_dump", None) | ||
| if callable(model_dump): | ||
| dumped = model_dump(by_alias=True, exclude_none=True) | ||
| if isinstance(dumped, dict): | ||
| return dumped |
Contributor
There was a problem hiding this comment.
To ensure compatibility with both Pydantic v1 and v2 (since some environments or older SDK versions might still use Pydantic v1), we should fall back to the .dict() method if .model_dump() is not available on the source object.
Suggested change
| model_dump = getattr(source, "model_dump", None) | |
| if callable(model_dump): | |
| dumped = model_dump(by_alias=True, exclude_none=True) | |
| if isinstance(dumped, dict): | |
| return dumped | |
| model_dump = getattr(source, "model_dump", None) | |
| if not callable(model_dump): | |
| model_dump = getattr(source, "dict", None) | |
| if callable(model_dump): | |
| dumped = model_dump(by_alias=True, exclude_none=True) | |
| if isinstance(dumped, dict): | |
| return dumped |
…-list (#143 CI fix) PR-1's Teams SDK deps were only in the [teams] extra, but CI runs 'uv sync --group dev' — so the SDK was absent at test time and test_teams_bridge.py's module-level import aborted collection of the whole 4793-test suite. Adds microsoft-teams-apps/-api/-cards to the dev group and the all extra, plus pyrefly replace-imports-with-any. Verified: full collection 4796 tests (no abort), teams suites 187 passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why (issue #93, PR 1 of 4)
Rewire the Teams adapter's inbound webhook path and JWT authentication to delegate to the official
microsoft-teams-appsPython SDK, mirroring upstreamadapter-teams@chat@4.30.0. Outbound (post/edit/delete/typing), native streaming, and the Graph reader stay hand-rolled — those are PRs 2/3 and a separate decision.The public
Adaptercontract andcreate_teams_adapter(TeamsAdapterConfig(...))are unchanged (chinchill depends on this). All public config field names are untouched.Inbound + auth flow after this PR
Inbound JWT validation is now performed by the SDK's
TokenValidator(RS256, audience =app_id+api://{app_id}+api://botid-{app_id}, Bot Framework issuer + JWKS), enforced by default (skip_auth=False). The hand-rolled_verify_bot_framework_token/_jwks_client/ openid-config fetch /BOT_FRAMEWORK_OPENID_CONFIG_URLare deleted.New:
src/chat_sdk/adapters/teams/bridge.pyBridgeHttpAdapterimplements the SDKHttpServerAdapterprotocol. It captures the route handler the App registers duringapp.initialize(), exposesasync dispatch(request, options), and owns a per-activityWebhookOptionsmap (keyed by activity id, cleared infinally). Framework-agnostic: body/header extraction duck-types across web frameworks; responses come back as the{body, status, headers}dict consumers expect. Port of upstreambridge-adapter.ts.adapter.py changes
_to_app_options(port ofconfig.tstoAppOptions): mapsapp_id/app_password/app_tenant_id/federated/app_type→ SDKclient_id/client_secret/tenant_id/managed_identity_client_id, with the sameTEAMS_*env fallbacks.User-Agent: Vercel.ChatSDK.initialize()registers SDK handlers (on_message/on_message_reaction/on_card_action/on_dialog_open/on_dialog_submit/on_conversation_update/on_install_add/remove), awaitsapp.initialize(), overridesserver.on_request._handle_teams_error(port oferrors.tshandleTeamsError) maps both the hand-rolled Graph path's plain dicts and the SDK's typed exceptions onto our taxonomy (Authentication / Permission / RateLimit / Network).Divergence (SDK-forced — documented in
docs/UPSTREAM_SYNC.md)The Python SDK's default
on_requestruns strict per-activity validation (ActivityTypeAdapter,recipient/idrequired) and a liveusers.token.getnetwork call inside_build_contextbefore any handler fires. Minimal serverless payloads + the adapter's dict-based handlers can't survive that. So we keep the SDK as the auth + transport layer (JWT genuinely validated) but route the already-authenticated lenientCoreActivityourselves, preserving the exact pre-migration handler behavior. Dialog/modal inbound is registered for parity but not yet wired toprocess_modal_submit(matches the pre-migration Python adapter; later wave of #93).Tests
tests/test_teams_bridge.py(27 tests): protocol conformance, dispatch happy/error paths,WebhookOptionslifecycle, body/header extraction.skip_auth(not the deleted method). NewTestSdkInboundAuthasserts the SDK rejects unauthenticated / bad-token / unconfigured requests._to_app_options+ SDK-exception error-mapping tests; shared body-extraction test points atBridgeHttpAdapter._read_body.Gauntlet
ruff✓ ·ruff format --check✓ ·audit_test_quality.py0 hard failures ✓ ·pyrefly0 errors ✓ ·pytest4793 passed, 3 skipped.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Dependencies
Documentation