Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to access Response in the Route #8

Closed
shawnsw opened this issue Feb 6, 2024 · 10 comments
Closed

Unable to access Response in the Route #8

shawnsw opened this issue Feb 6, 2024 · 10 comments

Comments

@shawnsw
Copy link

shawnsw commented Feb 6, 2024

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
Copy link
Owner

Thanks for reporting the issue and for submitting a PR!

@volfpeter
Copy link
Owner

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

@shawnsw
Copy link
Author

shawnsw commented Feb 6, 2024

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
Copy link
Owner

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
Copy link
Author

shawnsw commented Feb 6, 2024

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.

  • Extend the render Function: Allow it to accept an additional cookies parameter which would be a dictionary of cookies to set.

  • Modify the Decorator: Ensure it correctly handles this cookies parameter and sets the cookies before returning it.

@volfpeter
Copy link
Owner

It's not that simple:

  • First of all, you can't know outside the route what to change on the response. The value always comes from (is created) inside the route/your business logic. Decorators can only get such data from the route's return value or the route's dependencies (route context in hx).
  • Also, you may need to add, change, or delete cookies or headers -- we can not add decorator arguments and all the related scaffolding to every single use-case. It would be a pretty bad solution.

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
Copy link
Author

shawnsw commented Feb 7, 2024

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

See PR #19 for response header support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants