fix(apps): allow sign-in handlers to override default routes#436
fix(apps): allow sign-in handlers to override default routes#436RaghunandanKumar wants to merge 2 commits into
Conversation
Sign-in token exchange and verify-state defaults were registered the same way as user handlers, so app-level overrides were appended instead of replacing the built-in routes. Mark the built-in OAuth handlers as system routes, carry route names through generated registrations, and replace matching system routes when a user handler is registered. Add router and app regressions so custom sign-in handlers become the only matching route.
|
@RaghunandanKumar please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces a routing concept that allows developer-registered handlers to replace built-in (system) OAuth routes (notably sign-in token exchange / verify-state), and adds tests to ensure override behavior.
Changes:
- Add route metadata (
route_name,route_type) to the router and implement “user overrides system default” behavior. - Register OAuth sign-in system handlers directly on the router as
route_type="system". - Update generated handler registration to pass
route_name, and add tests validating override semantics.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/apps/src/microsoft_teams/apps/routing/router.py | Adds RouteEntry + route metadata and implements logic to drop system routes when a user handler with the same name is registered. |
| packages/apps/src/microsoft_teams/apps/app.py | Registers the OAuth sign-in defaults as explicit “system” routes (so user handlers can replace them). |
| packages/apps/src/microsoft_teams/apps/routing/generated_handlers.py | Ensures all generated on_* registrations include a route_name for consistent routing/override behavior. |
| packages/apps/scripts/generate_handlers.py | Updates the generator template so future generated handlers pass route_name. |
| packages/apps/tests/test_app_oauth.py | Adds an async test validating that a developer sign-in token exchange handler replaces the system default. |
| packages/apps/tests/test_app.py | Adds a test that registering on_signin_token_exchange replaces the default route; also modifies nearby assertions. |
Comments suppressed due to low confidence (1)
packages/apps/tests/test_app.py:1
- The assertion that validates
activity_events[0].body.type == core_activity.typeappears to have been removed (there’s now a blank line before the next test). This weakens the existing test by no longer verifying that the event preserves the activity type; please restore the assertion.
"""
| route_name="signin.token-exchange", | ||
| route_type="system", | ||
| ) | ||
| self.router.add_handler( | ||
| ACTIVITY_ROUTES["signin.verify-state"].selector, | ||
| oauth_handlers.sign_in_verify_state, | ||
| route_name="signin.verify-state", |
| class RouteEntry: | ||
| selector: RouteSelector | ||
| handler: ActivityHandler | ||
| route_name: str | None = None |
| """Add a handler for a specific activity configuration.""" | ||
| self._routes.append((selector, handler)) | ||
| if route_type == "user" and route_name is not None: | ||
| self._routes = [ | ||
| route for route in self._routes if not (route.route_name == route_name and route.route_type == "system") | ||
| ] |
Fixes #159
Summary
Mirrors the default-route override behavior from microsoft/teams.ts#317 for the Python apps router.
Today
Appregisters the built-insignin.token-exchangeandsignin.verify-statehandlers the same way user handlers are registered. If an app later callson_signin_token_exchange(...)oron_signin_verify_state(...), the custom handler is appended instead of replacing the built-in default, so both routes remain active.This change introduces route metadata in
ActivityRouter, registers the built-in OAuth handlers assystemroutes, and passesroute_namethrough generated handler registrations. When a user handler is registered for the same named route, the matching system default is removed so the app-level override becomes the only handler for that route.Changes
packages/apps/src/microsoft_teams/apps/routing/router.py(selector, handler)tuples.systemroutes when auserroute with the sameroute_nameis registered.packages/apps/src/microsoft_teams/apps/app.pysystemroutes.packages/apps/scripts/generate_handlers.pyconfig.nameinto generatedadd_handler(...)calls.packages/apps/src/microsoft_teams/apps/routing/generated_handlers.pyroute_name=config.namefor decorator-based registrations.packages/apps/tests/test_app_oauth.pysignin.token-exchangehandler replaces the system default in the middleware chain.packages/apps/tests/test_app.pyApp.on_signin_token_exchange(...)leaves only the custom handler selected forsignin/tokenExchange.Test plan
.venv/bin/ruff format packages/apps/src/microsoft_teams/apps/app.py packages/apps/src/microsoft_teams/apps/routing/router.py packages/apps/src/microsoft_teams/apps/routing/generated_handlers.py packages/apps/scripts/generate_handlers.py packages/apps/tests/test_app.py packages/apps/tests/test_app_oauth.py.venv/bin/ruff check packages/apps/src/microsoft_teams/apps/app.py packages/apps/src/microsoft_teams/apps/routing/router.py packages/apps/src/microsoft_teams/apps/routing/generated_handlers.py packages/apps/scripts/generate_handlers.py packages/apps/tests/test_app.py packages/apps/tests/test_app_oauth.pyPYTHONPATH=packages/api/src:packages/apps/src:packages/common/src:packages/cards/src:packages/botbuilder/src:packages/graph/src .venv/bin/python -m pytest packages/apps/tests/test_app.py packages/apps/tests/test_app_oauth.py -qsystemvsuserroute terminology matches the intended public direction for broader route override support.