-
-
Notifications
You must be signed in to change notification settings - Fork 955
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
Allow Session scoped cookies. #1387
Allow Session scoped cookies. #1387
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
max_age
type on the __init__
should be Optional[int]
as it can receive None
now.
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
set_cookie = response.headers["set-cookie"] | ||
assert "Max-Age" not in set_cookie | ||
|
||
client.cookies.clear_session_cookies() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to myself: use client.cookies.clear
instead of client.cookies.clear_session_cookies
on #1376
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? afaik this method will keep all other cookies except session-scoped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, yeah... httpx
doesn't have clear_session_cookies
😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @alex-oleshkevich :)
This PR changes the behavior of SessionMiddleware.
If the user sets
max_age
to None or 0 then a cookie will be Session-only and will be destroyed after closing the browser.