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

Delete response header not setting cookie_samesite as defined in CookieTransport #846

Closed
Hazedd opened this issue Jan 7, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@Hazedd
Copy link
Contributor

Hazedd commented Jan 7, 2022

For my angular project i need to set the cookies SameSite header to 'None' so 'Set-Cookie' is set by the angular app.
I defined the cookie transport:

cookie_transport = CookieTransport(cookie_max_age=3600, cookie_samesite='None')

On a login call the browser is receiving the following cookie:

fastapiusersauth=yofyP7VSA3YZ6seMk3YzX075majNRys4vV7N1kO83a0; HttpOnly; Max-Age=3600; Path=/; SameSite=None; Secure

and the cookie will be set.

On a logout call the browser is receiving the following cookie:

fastapiusersauth=""; expires=Fri, 07 Jan 2022 11:38:53 GMT; Max-Age=0; Path=/; SameSite=lax

Setting cookie is blocked by SameSite attribute lax!

To Reproduce

Steps to reproduce the behavior:
setup fastapi-users cookie backend
setup angular project to call login and logout
look in network calls

Expected behavior

On the delete cookie endpoint return response headers with samesite attribute as defined in the cookie transport.

Configuration

  • Python version : 3.10
  • FastAPI version : 0.70.1
  • FastAPI Users version : 9.2.0

FastAPI Users configuration

see repo for config.

edit:

I have found the issue:

CookieTransport implements a set and delete cookie of starlette. In the set_cookie of starlette all posible options are present (secure and samesite for this issue). The delete_cookie of starlette makes a expired cookie by using the set_cookie on this point only key, expires max_age path domain are used.

Not sure deleted cookies should have those parameters, if so bug is in starlette package.

def delete_cookie(self, key: str, path: str = "/", domain: str = None) -> None:
    self.set_cookie(key, expires=0, max_age=0, path=path, domain=domain)

fix for fastapi-users:

on CookieTransport class make sure the get_logout_response is using the set_cookie methode of starlette and set cookie-value to '' and max_age to 0

async def get_logout_response(self, response: Response) -> Any:
    response.set_cookie(
        self.cookie_name,
        '',
        max_age=0,
        path=self.cookie_path,
        domain=self.cookie_domain,
        secure=self.cookie_secure,
        httponly=self.cookie_httponly,
        samesite=self.cookie_samesite,
    )

commit 113bcf0 PR: #848

@Hazedd Hazedd added the bug Something isn't working label Jan 7, 2022
frankie567 pushed a commit that referenced this issue Jan 10, 2022
* logout response sets proper response headers

logout response is using starlette delete cookie. In starlette the samesite and secure attributes are not in the header but are needed to set the removed cookie client side. Implementing set_cookie with an empty cookie-value and a max_age of 0 will set a new expired cookie by the client.

related issue #846

* fixed linting

Co-authored-by: Pentem <martijn.pentenga@movares.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants