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
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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) <noreply@anthropic.com>
@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
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