reboot: bind an app_internal route's context to its endpoint

reboot/aio/http.py

@@ -66,6 +66,11 @@ class APIRoute:
         path: str
         kwargs: dict
         endpoint: Callable[..., Any]
+        # Whether the endpoint is handed an *app-internal*
+        # `ExternalContext` (one carrying the application's
+        # `caller_id`) instead of the external one. See the DANGER note
+        # in `HTTP._api_route`.
+        app_internal: bool = False
 
     @dataclass(kw_only=True, frozen=True)
     class Mount:
@@ -88,18 +93,17 @@ class HTTP:
         def __init__(self):
             self._api_routes: list[PythonWebFramework.APIRoute] = []
             self._mounts: list[PythonWebFramework.Mount] = []
-            # Exact request paths whose handlers receive an *app-internal*
-            # context (one that can call app-internal-only servicers)
-            # instead of the usual external one, because they opted in via
-            # `app_internal=True`. See the DANGER note in `_api_route`.
-            self._app_internal_paths: set[str] = set()
 
         def _api_route(self, path: str, **kwargs):
             # `app_internal` is our own kwarg, not one of FastAPI's, so we
             # pop it before storing the rest. When set, this route's
             # handler is given an *app-internal* `ExternalContext` (one
             # carrying the application's `caller_id`, able to call
             # app-internal-only servicers) instead of the external one.
+            # The grant is bound to the endpoint: it reaches exactly the
+            # requests that Starlette dispatches to this handler, however
+            # its path is spelled (a template such as `/things/{id}`
+            # included).
             #
             # DANGER: an app-internal context bypasses authorizers, so a
             # route that gets one can make trusted in-app calls on behalf
@@ -109,8 +113,7 @@ def _api_route(self, path: str, **kwargs):
             # only after the authorization code has been exchanged and
             # validated. Never set it on a route that acts on unvalidated
             # request input.
-            if kwargs.pop("app_internal", False):
-                self._app_internal_paths.add(path)
+            app_internal: bool = kwargs.pop("app_internal", False)
 
             # TODO: add type annotations for `endpoint` so that what
             # we take in is exactly what we return.
@@ -127,6 +130,7 @@ def decorator(endpoint):
                         path=path,
                         endpoint=endpoint,
                         kwargs=kwargs,
+                        app_internal=app_internal,
                     )
                 )
                 return endpoint
@@ -271,26 +275,34 @@ def app_internal_external_context_from_request(
                 caller_id=CallerID(application_id=application_id),
             )
 
+        def app_internal_external_context_dependency(request: Request):
+            # A route-level dependency that hands its endpoint an
+            # app-internal context. FastAPI resolves it after Starlette
+            # has dispatched the request to that endpoint and before the
+            # endpoint runs, so the grant reaches exactly the requests
+            # the endpoint handles.
+            request.state.reboot_external_context = (
+                app_internal_external_context_from_request(request)
+            )
+
         fastapi = FastAPI()
 
         @fastapi.middleware("http")
         async def external_context_middleware(request: Request, call_next):
-            # Most routes get an *external* context (no `caller_id`): an
-            # HTTP handler serves untrusted external traffic, so handing it
-            # a caller that bypasses authorizers would let external
-            # requests escalate to trusted in-app calls. Those routes must
-            # do their own end-user auth. Only routes that opted in via
-            # `app_internal=True` get an *app-internal* context instead —
-            # see the DANGER note on `HTTP._api_route`. We namespace this
-            # on `request.state` so other middleware doesn't clash.
-            if request.url.path in self._http._app_internal_paths:
-                request.state.reboot_external_context = (
-                    app_internal_external_context_from_request(request)
-                )
-            else:
-                request.state.reboot_external_context = (
-                    external_context_from_request(request)
-                )
+            # Every request gets an *external* context (no `caller_id`):
+            # an HTTP handler serves untrusted external traffic, so
+            # handing it a caller that bypasses authorizers would let
+            # external requests escalate to trusted in-app calls. Routes
+            # must do their own end-user auth. Only routes that opted in
+            # via `app_internal=True` get an *app-internal* context
+            # instead, which `app_internal_external_context_dependency`
+            # puts in place once the request has been routed to such an
+            # endpoint — see the DANGER note on `HTTP._api_route`. We
+            # namespace this on `request.state` so other middleware
+            # doesn't clash.
+            request.state.reboot_external_context = (
+                external_context_from_request(request)
+            )
 
             return await call_next(request)
 
@@ -304,10 +316,18 @@ async def external_context_middleware(request: Request, call_next):
             )
 
         for api_route in self._http._api_routes:
+            kwargs = dict(api_route.kwargs)
+            if api_route.app_internal:
+                # Ahead of any dependencies the route declared itself,
+                # so those already see the app-internal context.
+                kwargs["dependencies"] = [
+                    Depends(app_internal_external_context_dependency),
+                    *(kwargs.get("dependencies") or []),
+                ]
             fastapi.add_api_route(
                 api_route.path,
                 api_route.endpoint,
-                **api_route.kwargs,
+                **kwargs,
             )
 
         config = uvicorn.Config(

tests/reboot/aio/BUILD.bazel

@@ -1,3 +1,4 @@
+load("@rbt_pypi//:requirements.bzl", "requirement")
 load("@rules_python//python:defs.bzl", "py_test")
 
 py_test(
@@ -61,6 +62,21 @@ py_test(
     ],
 )
 
+py_test(
+    name = "http_app_internal_test_py",
+    timeout = "short",
+    srcs = [":http_app_internal_test.py"],
+    main = "http_app_internal_test.py",
+    deps = [
+        "//reboot/aio:applications_py",
+        "//reboot/aio:external_py",
+        "//reboot/aio:http_py",
+        "//reboot/aio:tests_py",
+        "//tests/reboot:greeter_servicers_py",
+        requirement("httpx"),
+    ],
+)
+
 py_test(
     name = "caller_id_test_py",
     timeout = "short",

tests/reboot/aio/http_app_internal_test.py

@@ -0,0 +1,93 @@
+"""
+Tests which `ExternalContext` a custom HTTP route is handed: a route
+registered with `app_internal=True` gets an app-internal one (carrying
+the application's `caller_id`), whatever the shape of its path, while a
+route registered without it gets an external one even when it lives
+under the same path prefix as an app-internal route.
+"""
+
+import httpx
+import unittest
+from reboot.aio.applications import Application
+from reboot.aio.external import ExternalContext
+from reboot.aio.http import InjectExternalContext
+from reboot.aio.tests import Reboot
+from tests.reboot.greeter_servicers import MyGreeterServicer
+
+# Generous per-request HTTP timeout: each request crosses a full Reboot
+# cluster plus a local Envoy, which can take a while on a loaded CI
+# runner, and Bazel's test timeout remains the backstop against a hang.
+_HTTP_TIMEOUT_SECONDS = 30.0
+
+
+def _describe(context: ExternalContext) -> dict[str, bool]:
+    """The kind of context a handler was given: an app-internal context
+    carries the application's `caller_id`, an external one carries
+    none."""
+    return {"app_internal": context.caller_id is not None}
+
+
+class HTTPAppInternalTest(unittest.IsolatedAsyncioTestCase):
+
+    async def asyncSetUp(self) -> None:
+        self.rbt = Reboot()
+        await self.rbt.start()
+
+        application = Application(servicers=[MyGreeterServicer])
+
+        # Starlette dispatches to the first route whose path matches,
+        # so the static routes are registered ahead of the
+        # parameterized one that would otherwise capture them.
+        @application.http.get("/__/test/static", app_internal=True)
+        def static(context: ExternalContext = InjectExternalContext):
+            return _describe(context)
+
+        @application.http.get("/__/test/plain")
+        def plain(context: ExternalContext = InjectExternalContext):
+            return _describe(context)
+
+        @application.http.get("/__/test/{item}", app_internal=True)
+        def parameterized(
+            item: str,
+            context: ExternalContext = InjectExternalContext,
+        ):
+            return _describe(context)
+
+        await self.rbt.up(application)
+
+    async def asyncTearDown(self) -> None:
+        await self.rbt.stop()
+
+    async def _get(self, path: str) -> dict[str, bool]:
+        async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT_SECONDS) as client:
+            response = await client.get(self.rbt.http_localhost_url(path))
+        self.assertEqual(200, response.status_code, response.text)
+        return response.json()
+
+    async def test_static_app_internal_route(self) -> None:
+        self.assertEqual(
+            {"app_internal": True},
+            await self._get("/__/test/static"),
+        )
+
+    async def test_parameterized_app_internal_route(self) -> None:
+        # The route's path is a template (`/__/test/{item}`); the
+        # request's path is concrete, so an exact path lookup would
+        # never match it.
+        self.assertEqual(
+            {"app_internal": True},
+            await self._get("/__/test/some-item"),
+        )
+
+    async def test_plain_route_under_app_internal_prefix(self) -> None:
+        # `/__/test/plain` also matches the `/__/test/{item}` pattern,
+        # so an app-internal grant keyed on a path pattern rather than
+        # on the dispatched endpoint would leak to this route.
+        self.assertEqual(
+            {"app_internal": False},
+            await self._get("/__/test/plain"),
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()