From 9e8b051aa8c8b3ad3f7d597c3262751421de1d56 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Sun, 5 Dec 2021 14:34:43 +0100 Subject: [PATCH 1/4] Share session data across mounted routes --- starlette/middleware/sessions.py | 2 +- starlette/routing.py | 10 ++++++++++ tests/middleware/test_session.py | 21 ++++++++++++++++++++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/starlette/middleware/sessions.py b/starlette/middleware/sessions.py index ad7a6ee89..7a7510d6d 100644 --- a/starlette/middleware/sessions.py +++ b/starlette/middleware/sessions.py @@ -49,7 +49,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: async def send_wrapper(message: Message) -> None: if message["type"] == "http.response.start": - path = scope.get("root_path", "") or "/" + path = scope.get("session_root_path", "") or "/" if scope["session"]: # We have session data to persist. data = b64encode(json.dumps(scope["session"]).encode("utf-8")) diff --git a/starlette/routing.py b/starlette/routing.py index 3c11c1b0c..43d07216d 100644 --- a/starlette/routing.py +++ b/starlette/routing.py @@ -339,6 +339,7 @@ def __init__( self.app: ASGIApp = app else: self.app = Router(routes=routes) + self._routes_given = routes is not None self.name = name self.path_regex, self.path_format, self.param_convertors = compile_path( self.path + "/{path:path}" @@ -365,6 +366,15 @@ def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: "path_params": path_params, "app_root_path": scope.get("app_root_path", root_path), "root_path": root_path + matched_path, + "session_root_path": ( + # NOTE: if mounting a list of routes, rather than a fully + # separate ASGI app, session data should be placed on the + # current app path. + # See: https://github.com/encode/starlette/issues/1261 + root_path + if isinstance(self.app, Router) + else root_path + matched_path + ), "path": remaining_path, "endpoint": self.app, } diff --git a/tests/middleware/test_session.py b/tests/middleware/test_session.py index 42f4447e5..7cf9bec45 100644 --- a/tests/middleware/test_session.py +++ b/tests/middleware/test_session.py @@ -1,8 +1,10 @@ import re from starlette.applications import Starlette +from starlette.middleware import Middleware from starlette.middleware.sessions import SessionMiddleware from starlette.responses import JSONResponse +from starlette.routing import Mount, Route def view_session(request): @@ -102,7 +104,7 @@ def test_secure_session(test_client_factory): assert response.json() == {"session": {}} -def test_session_cookie_subpath(test_client_factory): +def test_session_mounted_app(test_client_factory): app = create_app() second_app = create_app() second_app.add_middleware(SessionMiddleware, secret_key="example") @@ -114,6 +116,23 @@ def test_session_cookie_subpath(test_client_factory): assert cookie_path == "/second_app" +def test_session_mounted_routes(test_client_factory): + auth_routes = [ + Route("/login", update_session, methods=["POST"]), + ] + + app = Starlette( + routes=[Mount("/auth", routes=auth_routes)], + middleware=[Middleware(SessionMiddleware, secret_key="example")], + ) + + client = test_client_factory(app, base_url="http://testserver") + response = client.post("/auth/login", json={"some": "data"}) + cookie = response.headers["set-cookie"] + cookie_path = re.search(r"; path=(\S+);", cookie).groups()[0] + assert cookie_path == "/" + + def test_invalid_session_cookie(test_client_factory): app = create_app() app.add_middleware(SessionMiddleware, secret_key="example") From 8cde508ac2cc07155bbc287787a2c77d458f4754 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Sun, 5 Dec 2021 14:42:20 +0100 Subject: [PATCH 2/4] Refine code comment --- starlette/routing.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/starlette/routing.py b/starlette/routing.py index 43d07216d..79399849d 100644 --- a/starlette/routing.py +++ b/starlette/routing.py @@ -368,8 +368,11 @@ def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: "root_path": root_path + matched_path, "session_root_path": ( # NOTE: if mounting a list of routes, rather than a fully - # separate ASGI app, session data should be placed on the - # current app path. + # separate ASGI app, session data modified by these routes + # should apply to the parent path. + # This allows e.g. mounting auth routes and have their + # session modifications be available to the parent app, + # for use in e.g. permission checks in neighboring routes. # See: https://github.com/encode/starlette/issues/1261 root_path if isinstance(self.app, Router) From 693f697b2e0c29915d3ee1f5ebd12fa86845e9fb Mon Sep 17 00:00:00 2001 From: Florimond Manca Date: Mon, 6 Dec 2021 12:58:21 +0100 Subject: [PATCH 3/4] Drop "routes_given" Co-authored-by: Marcelo Trylesinski --- starlette/routing.py | 1 - 1 file changed, 1 deletion(-) diff --git a/starlette/routing.py b/starlette/routing.py index 79399849d..609621446 100644 --- a/starlette/routing.py +++ b/starlette/routing.py @@ -339,7 +339,6 @@ def __init__( self.app: ASGIApp = app else: self.app = Router(routes=routes) - self._routes_given = routes is not None self.name = name self.path_regex, self.path_format, self.param_convertors = compile_path( self.path + "/{path:path}" From ceb07a5a4cff4ca5f8f8d9436896c9676df669df Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Mon, 6 Dec 2021 21:58:12 +0100 Subject: [PATCH 4/4] Stick strictly to reacting to "routes=..." --- starlette/middleware/sessions.py | 2 +- starlette/routing.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/starlette/middleware/sessions.py b/starlette/middleware/sessions.py index 7a7510d6d..d4bc8d856 100644 --- a/starlette/middleware/sessions.py +++ b/starlette/middleware/sessions.py @@ -49,7 +49,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: async def send_wrapper(message: Message) -> None: if message["type"] == "http.response.start": - path = scope.get("session_root_path", "") or "/" + path = scope.get("session_cookie_path", "") or "/" if scope["session"]: # We have session data to persist. data = b64encode(json.dumps(scope["session"]).encode("utf-8")) diff --git a/starlette/routing.py b/starlette/routing.py index 609621446..1f1f5a3f8 100644 --- a/starlette/routing.py +++ b/starlette/routing.py @@ -339,6 +339,7 @@ def __init__( self.app: ASGIApp = app else: self.app = Router(routes=routes) + self._should_share_cookies = routes is not None self.name = name self.path_regex, self.path_format, self.param_convertors = compile_path( self.path + "/{path:path}" @@ -365,7 +366,7 @@ def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: "path_params": path_params, "app_root_path": scope.get("app_root_path", root_path), "root_path": root_path + matched_path, - "session_root_path": ( + "session_cookie_path": ( # NOTE: if mounting a list of routes, rather than a fully # separate ASGI app, session data modified by these routes # should apply to the parent path. @@ -374,7 +375,7 @@ def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: # for use in e.g. permission checks in neighboring routes. # See: https://github.com/encode/starlette/issues/1261 root_path - if isinstance(self.app, Router) + if self._should_share_cookies else root_path + matched_path ), "path": remaining_path,