alex-oleshkevich / starsessions

Advanced sessions for Starlette and FastAPI frameworks
MIT License
94 stars 11 forks source link

Regression on v2: session dict is lost after a redirect using httpx client #50

Closed euri10 closed 2 years ago

euri10 commented 2 years ago

ok so I'm in the process of upgrading to v2 from 1.2.3 and I have this weird issue where it seems everything works ok manually (login page sets some stuff in session then redirects), but my tests are all broken.

It is as if on a redirect the request.session after it has been set is entirely lost, the dict is empty.

so to demonstrate i took the example fastapi_app and added this little test, the printed answer should not be an empty json.

It was working with 1.2.3 so there might be a bad combo of httpx client with an app and the current v2 but I'm currently unable to pinpoint where it exactly happens, I'll try to dig further today but if you have an idea in the meantime :)

import datetime
import typing

import pytest
from fastapi import FastAPI
from fastapi.requests import Request
from fastapi.responses import JSONResponse, RedirectResponse
from httpx import AsyncClient

from starsessions import CookieStore, SessionAutoloadMiddleware, SessionMiddleware

app = FastAPI()
app.add_middleware(SessionAutoloadMiddleware)
app.add_middleware(SessionMiddleware, store=CookieStore(secret_key="key"))

@app.get("/", response_class=JSONResponse)
async def homepage(request: Request) -> typing.Mapping[str, typing.Any]:
    """Access this view (GET '/') to display session contents."""
    return request.session

@app.get("/set", response_class=RedirectResponse)
async def set_time(request: Request) -> str:
    """Access this view (GET '/set') to set session contents."""
    request.session["date"] = datetime.datetime.now().isoformat()
    return "/"

@app.get("/clean", response_class=RedirectResponse)
async def clean(request: Request) -> str:
    """Access this view (GET '/clean') to remove all session contents."""
    request.session.clear()
    return "/"

@pytest.mark.asyncio
async def test_set():
    async with AsyncClient(app=app, base_url="http://testserver") as client:
        response = await client.get("/")
        assert response.status_code == 200
        assert response.json() == {}
        response = await client.get("/set", follow_redirects=True)
        assert response.status_code == 200
        print(response.json())
        assert response.json() != {}
euri10 commented 2 years ago

ok I tested with another client and got the same issue:

def test_starlette():
    with TestClient(app) as client:
        response = client.get("/")
        assert response.status_code == 200
        assert response.json() == {}
        response = client.get("/set")
        assert response.status_code == 200
        print(response.json())
        assert response.json() != {}

both tests work fine on 1.3.0, will try to see if I can bisect

alex-oleshkevich commented 2 years ago

FYI: client.get("/set") does not return anything. It only sets date and redirects.

alex-oleshkevich commented 2 years ago

The issue here is that in 2.0 I changed default security policy. By default, the cookie uses browser session lifetime and works only for HTTPS connections. You call your endpoint with HTTP and therefore the cookie is not set.

Add cookie_https_only=False to the middleware config to solve the problem.

euri10 commented 2 years ago

The issue here is that in 2.0 I changed default security policy. By default, the cookie uses browser session lifetime and works only for HTTPS connections. You call your endpoint with HTTP and therefore the cookie is not set.

Add cookie_https_only=False to the middleware config to solve the problem.

rooo ok I get it, but instead of settings the flag to False in the middleware I'd rather change the test client to using the https protocol in the base_url, that solves it thanks