Skip to content

Revert serviceUrl allowlist (ADO 5310460)#415

Merged
corinagum merged 2 commits into
mainfrom
cg/revert-serviceurl-allowlist
Apr 28, 2026
Merged

Revert serviceUrl allowlist (ADO 5310460)#415
corinagum merged 2 commits into
mainfrom
cg/revert-serviceurl-allowlist

Conversation

@corinagum

Copy link
Copy Markdown
Contributor

Summary

Reverting the serviceUrl allowlist defense-in-depth feature before public release. Open design questions on the work item (default sovereign cloud domains, narrowing of *.botframework.com, applying to proactive ConversationReferences, consultation with APX) should be resolved before this becomes part of the public API surface.

This feature has not shipped in any release. Removing the public additional_allowed_domains option is a breaking change for anyone building from main, which is not ideal but acceptable since no release has been cut. Reverting now buys time to discuss without breaking customers later.

Removed

  • is_allowed_service_url helper
  • additional_allowed_domains option on AppOptions/InternalAppOptions, plus plumbing through App -> HttpServer -> TokenValidator (for_service, for_entra, validate_token)
  • CloudEnvironment.allowed_service_urls field and per-cloud entries (PUBLIC/US_GOV/US_GOV_DOD/CHINA)
  • Service URL hostname check in HttpServer.handle_request
  • All allowlist tests in test_token_validator.py and test_http_server.py

Preserved (other security work bundled into PR #370)

  • Scope validation (exact set membership)
  • Issuer validation logging when tenant_id is missing in for_entra

Preserved (sovereign cloud, separate work)

CloudEnvironment presets, with_overrides, from_name, cloud option on App, and the cloud parameter on TokenValidator factories are unchanged.

Test plan

  • uv run ruff check clean (full repo)
  • uv run pyright clean (full repo, 0 errors / 0 warnings)
  • uv run pytest packages/apps/tests/: 246 tests pass
  • Smoke test: examples/echo starts and uvicorn binds to port

Copilot AI review requested due to automatic review settings April 28, 2026 19:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reverts the (unshipped) serviceUrl hostname allowlist defense-in-depth feature from the Teams Apps SDK before public release, removing the related public configuration surface and tests while keeping other token-validation hardening (scope/issuer handling) intact.

Changes:

  • Removes additional_allowed_domains from app options and stops plumbing it through AppHttpServerTokenValidator.
  • Deletes serviceUrl hostname allowlist logic (helper + enforcement in HttpServer and TokenValidator) and the associated cloud preset data.
  • Removes allowlist-focused unit tests in packages/apps/tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/apps/tests/test_token_validator.py Removes serviceUrl allowlist and additional_allowed_domains token validator tests.
packages/apps/tests/test_http_server.py Removes serviceUrl allowlist request-handling tests and initialization plumbing assertion.
packages/apps/src/microsoft_teams/apps/options.py Removes additional_allowed_domains from AppOptions / InternalAppOptions.
packages/apps/src/microsoft_teams/apps/http/http_server.py Removes allowlist configuration + enforcement in request handling and initialization.
packages/apps/src/microsoft_teams/apps/auth/token_validator.py Deletes allowlist helper + domain enforcement; leaves serviceUrl claim equality validation.
packages/apps/src/microsoft_teams/apps/app.py Removes forwarding of additional_allowed_domains into server/token-validator setup.
packages/api/src/microsoft_teams/api/auth/cloud_environment.py Removes allowed_service_urls field and preset values from CloudEnvironment.

Comment thread packages/apps/src/microsoft_teams/apps/auth/token_validator.py
Addresses review feedback. The cloud parameter on the constructor was
added by the allowlist work and became dead after the revert. Sovereign
cloud routing happens entirely inside the factory methods (for_service,
for_entra), which derive valid_issuers, jwks_uri, and login_endpoint
from cloud and bake them into JwtValidationOptions before construction.

Restores the pre-allowlist constructor signature.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@corinagum
corinagum merged commit db7d832 into main Apr 28, 2026
7 checks passed
@corinagum
corinagum deleted the cg/revert-serviceurl-allowlist branch April 28, 2026 20:13
corinagum added a commit that referenced this pull request May 27, 2026
## Summary

Brings `origin/main` into `release` for the **2.0.12** release. **No
carve-outs this time** — Quoted Replies (PR #321) is included.

`version.json`: `2.0.11` → `2.0.12`.

Aligns with teams.ts/teams.net cadence.

## What's in this release (delta from 2.0.11)

**Features**
- Quoted Replies & new quotes features (#321) — previously held back
- SuggestedActionSubmitActivity for `suggestedAction/submit` invoke
(#434)
- Default targeted replies for targeted inbound messages (#439)
- Reactions API marked GA (#427) — already shipped in 2.0.11 actually,
ignore if duplicate
- Prompt Preview support (#397)
- Sample: opt targeted-messages into slash commands (#430)
- Sample: a2a-in-teams (#401)
- Sample: proactive message update (#405)
- Allow custom HTTP client via `AppOptions` (#416)

**Security & fixes**
- Lock JsonWebToken trust-boundary contract (#432)
- Bump starlette 1.0.0 -> 1.1.0 for PYSEC-2026-161 / BadHost (#441)
- warn when bot starts without credentials (#435)
- fix(apps): support AAD v1 issuers in Entra token validation (#422)
- fix(apps): log inbound activities at info, warn on missing
Authorization (#425)
- fix(http-stream): wait for in-flight flush before sending final close
(#419)
- Mark reactions API as GA and fix sample remove flow (#427)
- Revert serviceUrl allowlist (ADO 5310460) (#415)
- Route `app.get_app_graph` + `ctx.*_graph` to sovereign endpoints via
`cloud.graph_scope` (#402)

**Deprecations / package changes**
- Remove ai, openai, mcpplugin, a2a, and devtools packages (#413)
- Restore and deprecate devtools package (#410)
- Deprecate ai, openai, mcpplugin, a2a packages (#406)
- Add agent framework example, remove chatprompt/mcp client samples
(#386)

**Dependency bumps**
- urllib3 2.6.3 → 2.7.0 (#426)
- python-multipart 0.0.26 → 0.0.27 (#424)
- microsoft-kiota-http 1.9.7 → 1.9.9 (#423)
- idna 3.11 → 3.15 (#431)
- starlette 1.0.0 → 1.1.0 (#441, security)
- /examples/tab/Web: qs, postcss, uuid, follow-redirects bumps

## Quoted Replies inclusion notes

- Teams client rendering is in-sync with the wire format as of
2026-05-06.
- APX QR rollout completed: Public 2026-04-10, GCCH/DoD/Gallatin
2026-04-14.
- Builder methods (`prepend_quote()`, `add_quote()`), `ctx.quote()`, and
the quote-aware `ctx.reply()` behavior are all present in this release.
- `examples/quoting` is included.

## Conflict resolution

The 2.0.11 release branch had the QR-removed shape for four files; took
`main`'s (QR-included) version for all of them:
- `examples/targeted-messages/src/main.py`
- `packages/api/src/microsoft_teams/api/activities/message/message.py`
- `packages/apps/src/microsoft_teams/apps/routing/activity_context.py`
- `packages/apps/tests/test_activity_context.py`

`version.json` conflict resolved to `2.0.12`.

## Test plan

- [x] `uv sync` clean
- [x] `poe check` (ruff format + lint) — clean
- [x] `poe test` — 609 passed
- [x] `pyright` — 0 errors, 0 warnings
- [ ] Pipeline build + test stages green on `release` after merge
- [ ] Publish pipeline run with **Public** → ESRP approval → PyPI
- [ ] `pip install microsoft-teams-apps==2.0.12` smoke install

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Aamir Jawaid <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Co-authored-by: Shanmathi Mayuram Krithivasan <[email protected]>
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.

4 participants