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

Allow Session scoped cookies. #1387

Merged
merged 6 commits into from Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/middleware.md
Expand Up @@ -98,7 +98,7 @@ The following arguments are supported:

* `secret_key` - Should be a random string.
* `session_cookie` - Defaults to "session".
* `max_age` - Session expiry time in seconds. Defaults to 2 weeks.
* `max_age` - Session expiry time in seconds. Defaults to 2 weeks. If set to None or 0 seconds then the session will be cleared on tab close.
alex-oleshkevich marked this conversation as resolved.
Show resolved Hide resolved
* `same_site` - SameSite flag prevents the browser from sending session cookie along with cross-site requests. Defaults to `'lax'`.
* `https_only` - Indicate that Secure flag should be set (can be used with HTTPS only). Defaults to `False`.

Expand Down
12 changes: 6 additions & 6 deletions starlette/middleware/sessions.py
Expand Up @@ -55,12 +55,12 @@ async def send_wrapper(message: Message) -> None:
data = b64encode(json.dumps(scope["session"]).encode("utf-8"))
data = self.signer.sign(data)
headers = MutableHeaders(scope=message)
header_value = "%s=%s; path=%s; Max-Age=%d; %s" % (
self.session_cookie,
data.decode("utf-8"),
path,
self.max_age,
self.security_flags,
header_value = "{session_cookie}={data}; path={path}; {max_age}{security_flags}".format( # noqa E501
session_cookie=self.session_cookie,
data=data.decode("utf-8"),
path=path,
max_age=f"Max-Age={self.max_age}; " if self.max_age else "",
security_flags=self.security_flags,
)
headers.append("Set-Cookie", header_value)
elif not initial_session_was_empty:
Expand Down
17 changes: 17 additions & 0 deletions tests/middleware/test_session.py
Expand Up @@ -125,3 +125,20 @@ def test_invalid_session_cookie(test_client_factory):
# we expect it to not raise an exception if we provide a bogus session cookie
response = client.get("/view_session", cookies={"session": "invalid"})
assert response.json() == {"session": {}}


def test_session_cookie(test_client_factory):
app = create_app()
app.add_middleware(SessionMiddleware, secret_key="example", max_age=None)
client = test_client_factory(app)

response = client.post("/update_session", json={"some": "data"})
assert response.json() == {"session": {"some": "data"}}

# check cookie max-age
set_cookie = response.headers["set-cookie"]
assert "Max-Age" not in set_cookie

client.cookies.clear_session_cookies()
Copy link
Sponsor Member

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

Copy link
Member Author

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.

Copy link
Sponsor Member

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 😞

response = client.get("/view_session")
assert response.json() == {"session": {}}