diff --git a/changelog.d/322.security.md b/changelog.d/322.security.md new file mode 100644 index 0000000..eab67bc --- /dev/null +++ b/changelog.d/322.security.md @@ -0,0 +1,10 @@ +**`examples/session_login.py` no longer sends a new visitor's login token in a +cacheable response.** When no session was loaded, the example set the cookie +itself, and the middleware adds `Cache-Control: private, no-store` only to +tokens it sends, so a shared cache could store the login and hand it to the +next visitor. That branch now sends `Cache-Control: private, no-store`, sets the +cookie with every configured `cookie_*` attribute (`domain` was missing, so the +cookie cleared at logout did not match it), and keeps the token out of headers +and the body so page scripts cannot read it; API clients get theirs from a +token endpoint, as in `examples/session_jwt.py`. The session guide says the +same for this case. diff --git a/docs/SESSION.md b/docs/SESSION.md index 48f10cd..2072134 100644 --- a/docs/SESSION.md +++ b/docs/SESSION.md @@ -709,14 +709,21 @@ async def login(request: Request, session: OptionalSession): session.user = user await session_manager.update_session(session) else: - # No session yet: create one with the user and deliver its token yourself, - # as in the login example in Basic Usage. + # No session yet: the middleware has no token to send, so create one + # with the user and deliver its token yourself. _, token = await session_manager.create_session(user=user) ... return {"ok": True} ``` -The complete version, including the cookie for a new visitor, is +In that last branch the response is yours to secure, since the middleware adds nothing +to a token it did not send: set the cookie with every `cookie_*` attribute of the config +(`domain` included, or the cookie cleared at logout will not match it) and send +`Cache-Control: private, no-store` so no shared cache stores the credential. Do not copy the +token into a response header or the body of a browser login: page scripts could read it, +which is what the HttpOnly cookie prevents. Give API clients their token from a separate +endpoint that returns it in the body, as +[`examples/session_jwt.py`](https://github.com/allen0099/FastAPI-CacheX/blob/master/examples/session_jwt.py) does. The complete version is [`examples/session_login.py`](https://github.com/allen0099/FastAPI-CacheX/blob/master/examples/session_login.py). A `login()` helper that does all of this is planned ([#293](https://github.com/allen0099/FastAPI-CacheX/issues/293)). diff --git a/examples/session_login.py b/examples/session_login.py index 8036ce5..a893953 100644 --- a/examples/session_login.py +++ b/examples/session_login.py @@ -3,8 +3,11 @@ A visitor gets an anonymous session as soon as something is written to ``request.session`` (here, a shopping cart). Logging in rotates the session ID against session fixation and attaches the user, keeping the cart; logging out -deletes the session. The token travels in a cookie, as with Starlette's -``SessionMiddleware``; header and ``Authorization: Bearer`` tokens work too. +deletes the session. The token travels in an HttpOnly cookie, as with +Starlette's ``SessionMiddleware``; header and ``Authorization: Bearer`` tokens +work too. A login here hands out only the cookie, so page scripts never see the +token; an API client gets its token from an endpoint that returns it in the +body, as ``session_jwt.py`` does. Run it from a checkout (see ``examples/README.md``):: @@ -103,8 +106,9 @@ async def login( await session_manager.update_session(session) return {"user": user.user_id} - # No session yet: create one for the user and set the cookie ourselves, - # since the middleware only sends tokens for sessions it loaded or created. + # No session yet, so the middleware has no token to send: create the + # session with the user and deliver the token ourselves, with the cookie + # attributes and cache headers the middleware would use. _, token = await session_manager.create_session( user=user, ip_address=client_ip, @@ -115,10 +119,13 @@ async def login( token, max_age=config.cookie_max_age, path=config.cookie_path, + domain=config.cookie_domain, + secure=config.cookie_https_only, httponly=True, samesite=config.cookie_same_site, - secure=config.cookie_https_only, ) + # The token is a credential: no shared cache may store this response. + response.headers["Cache-Control"] = "private, no-store" return {"user": user.user_id} diff --git a/i18n/zh-TW/docs/SESSION.md b/i18n/zh-TW/docs/SESSION.md index a92485c..e6a5c5d 100644 --- a/i18n/zh-TW/docs/SESSION.md +++ b/i18n/zh-TW/docs/SESSION.md @@ -564,14 +564,14 @@ async def login(request: Request, session: OptionalSession): session.user = user await session_manager.update_session(session) else: - # 還沒有 Session:建立帶有使用者的 Session,並自行交付其權杖, - # 做法同基本用法中的登入範例。 + # 還沒有 Session:中介軟體沒有權杖可送, + # 因此建立帶有使用者的 Session,並自行交付其權杖。 _, token = await session_manager.create_session(user=user) ... return {"ok": True} ``` -完整版本(包含為新訪客設定 Cookie)請見 [`examples/session_login.py`](https://github.com/allen0099/FastAPI-CacheX/blob/master/examples/session_login.py)。處理上述所有步驟的 `login()` 輔助函式已在規劃中([#293](https://github.com/allen0099/FastAPI-CacheX/issues/293))。 +最後這個分支的回應要由你自己保護,因為中介軟體不會處理不是由它送出的權杖:設定 Cookie 時帶上設定中所有的 `cookie_*` 屬性(包括 `domain`,否則登出時清除的 Cookie 會對不上),並送出 `Cache-Control: private, no-store`,讓共用快取不會存下這個憑證。不要把權杖複製到瀏覽器登入回應的標頭或本文中:頁面上的指令碼會讀得到它,而這正是 HttpOnly Cookie 要防止的。API 用戶端的權杖請由另一個在本文中回傳權杖的端點發給,如 [`examples/session_jwt.py`](https://github.com/allen0099/FastAPI-CacheX/blob/master/examples/session_jwt.py) 所示。完整版本請見 [`examples/session_login.py`](https://github.com/allen0099/FastAPI-CacheX/blob/master/examples/session_login.py)。處理上述所有步驟的 `login()` 輔助函式已在規劃中([#293](https://github.com/allen0099/FastAPI-CacheX/issues/293))。 在中介軟體之外,請以中介軟體會傳入的相同綁定值載入 Session,並自行將回傳的權杖交給用戶端: diff --git a/tests/test_examples.py b/tests/test_examples.py index 9a1e1aa..930bd21 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -221,6 +221,65 @@ def test_session_login_without_a_prior_session() -> None: assert client.get("/me").json() == {"user": "alice", "cart": []} +def _cookie_attributes(set_cookie: str) -> dict[str, str]: + """The attributes of one `Set-Cookie` value, keys lowercased, value dropped.""" + _, *parts = (part.strip() for part in set_cookie.split(";")) + attributes = {} + for part in parts: + key, _, value = part.partition("=") + attributes[key.lower()] = value + return attributes + + +def _session_set_cookie(set_cookies: list[str]) -> str: + """The one `Set-Cookie` value for the session cookie.""" + [value] = [v for v in set_cookies if v.startswith("session=")] + return value + + +@pytest.mark.parametrize("returning_visitor", [False, True]) +def test_session_login_response_is_private(returning_visitor: bool) -> None: + """The login response carries a credential: never cacheable, same cookie flags.""" + example = load_example("session_login") + example.config.cookie_domain = "example.test" + credentials = {"username": "alice", "password": "alice-demo-password"} + with TestClient(example.app, base_url="http://app.example.test") as client: + # A cookie set by the middleware itself: the reference attributes. + started = client.post("/cart/book") + expected = _cookie_attributes( + _session_set_cookie(started.headers.get_list("set-cookie")) + ) + assert expected == { + "path": "/", + "max-age": str(example.config.cookie_max_age), + "httponly": "", + "samesite": "lax", + "domain": "example.test", + } + if not returning_visitor: + client.cookies.clear() + + login = client.post("/login", json=credentials) + assert login.status_code == 200 + assert login.headers["cache-control"] == "private, no-store" + set_cookie = _session_set_cookie(login.headers.get_list("set-cookie")) + assert _cookie_attributes(set_cookie) == expected + # Only the HttpOnly cookie: a copy in a header would be readable by scripts. + assert "x-session-token" not in login.headers + assert client.get("/me").json()["user"] == "alice" + + logout = client.post("/logout") + cleared = _cookie_attributes( + _session_set_cookie(logout.headers.get_list("set-cookie")) + ) + assert logout.headers["cache-control"] == "private, no-store" + assert cleared["domain"] == expected["domain"] + assert cleared["path"] == expected["path"] + assert "expires" in cleared + assert not client.cookies.get("session") + assert client.get("/me").status_code == 401 + + @pytest.mark.skipif( importlib.util.find_spec("jwt") is None, reason="session_jwt needs the jwt extra (PyJWT)",