volfpeter / fasthx

FastAPI server-side rendering with built-in HTMX support.
https://volfpeter.github.io/fasthx/
MIT License
458 stars 9 forks source link

Unable to access Response in the Route #8

Closed shawnsw closed 9 months ago

shawnsw commented 10 months ago

Problem statement: Unable to access Response in the route (e.g. setting cookies) as it's being handled by fasthx.

Currently using a workaround - hx-target to a hidden div for the returned JWT tokens, and use JS to detect the response code and redirect the browser.

Would be nice if we could access Response from within the route.

volfpeter commented 10 months ago

Thanks for reporting the issue and for submitting a PR!

volfpeter commented 10 months ago

Could you please provide example code for your use-case?

shawnsw commented 10 months ago

I wanted to achieve the following:

  1. The /sign_in POST route can return both JSON and HTML for API and HTMX.
  2. Able to set JWT tokens in cookies in the Response.
  3. Redirect to / after successful login.

Below is the route:

@router.post("/sign_in")
@htmx("auth/sign_in_payload.html")
async def sign_in(user_auth: UserAuth,  response: Response) -> RefreshToken:
    """Authenticate and returns the user's JWT."""
    user = await User.by_email(user_auth.email)
    logger.info(f"User sign_in {user_auth.email}")
    ic(user_auth)
    if user is None or hash_password(user_auth.password) != user.password:
        raise HTTPException(status_code=401, detail="Bad email or password")
    if user.email_confirmed_at is None:
        raise HTTPException(status_code=400, detail="Email is not yet verified")
    access_token = access_security.create_access_token(user.jwt_subject)
    refresh_token = refresh_security.create_refresh_token(user.jwt_subject)
    response.set_cookie(key="access_token", value=access_token, httponly=True, secure=True, samesite='lax', max_age=int(ACCESS_EXPIRES.total_seconds()))
    response.set_cookie(key="refresh_token", value=refresh_token, httponly=True, secure=True, samesite='lax', max_age=int(REFRESH_EXPIRES.total_seconds()))
    return RefreshToken(access_token=access_token, refresh_token=refresh_token)

This code block coudn't set the cookies, but was able to return JSON / HTML respectively. I ended up removing the htmx decorator, and just return JSON for both web and API, and on the web side use client JS to catch 200 status code and then redirect user.

volfpeter commented 10 months ago

Thanks for the example. The cookies you set on the response in the route will indeed be ignored, because the decorator creates and returns a TemplateResponse -- see the "Tip" in the FastAPI docs here.

I'll think about the right solution to this issue, in the meantime it might be best to return a htmx.templates.TemplateResponse() from the route and never return JSON.

shawnsw commented 10 months ago

Thanks Peter. What if we modify the Jinja class or the hx decorator to accept an additional argument that specifies the cookies to set? This requires changes at several levels but will make the decorator more flexible.

volfpeter commented 10 months ago

It's not that simple:

I'm thinking whether there is a nice and easy to use solution to this issue that doesn't require users to write too much custom code (a function that gets the template response, the route's return value, and the route's context). The hx decorator already gives you this flexibility, since in that case it's the user's job to create the response. I have a feeling that adding similar response customization to the Jinja class would be at least as complex as writing a custom HTMXRenderer that internally creates a customized TemplateResponse.

Very simplified example:

jinja = Jinja(Jinja2Templates("templates"))

def sign_in_renderer(result, *, context, request):
    response = jinja.templates.TemplateResponse("sign_in_payload.html", context={}, request=request)
    response.set_cookie("access_token", value=result.access_token)
    response.set_cookie("refresh_token", value=result.refrest_token)
    return response

I did not mention it before, but with having this code in my own comment, I must warn you that (even accidentally) returning these tokens would be pretty dangerous, please avoid it. You can do that by setting no_data=True on the decorators, so the route will never return JSON. In that case, you can of course skip the whole custom renderer part and just write down the content of sign_in_renderer() in your route. If you need different behavior for HTMX and non-HTMX requests, I'd add a hx_request: Annotated[str | None, Header()] = None dependency to the route - for non-HTMX requests you'll like want to redirect the user or return HTTP 204 (depending on the client).

shawnsw commented 10 months ago

Thanks Peter. My current solution is similar to your example. For this particular scenario it's probably easier to just writing two separate routes, I've wasted hours on this and I don't think it was worth it. 🤣 Edge cases are inevitable but honestly it is still an incredibly useful tool nonetheless. Keep up the good work.

volfpeter commented 10 months ago

Sorry that I couldn't give you a better, more convenient solution (yet). I'll keep the issue open to keep this use-case (~authentication on HTMX routes). I've opened #10 to do some (necessary) convenience upgrades - maybe that will result in some improvements for this scenario as well.

volfpeter commented 9 months ago

PR #11 may give you new options, but I think the discussed solution is the best one, so I hope the issue can be closed.

volfpeter commented 8 months ago

See PR #19 for response header support.