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

Replace expires cookie attribute with max-age attribute #823

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Unreleased
- Use `datetime.now(timezone.utc)` instead of deprecated `datetime.utcnow`. #758
- Never look at the `X-Forwarded-For` header, always use `request.remote_addr`,
requiring the developer to configure `ProxyFix` appropriately. #700
- Replace `expires` attribute with `max-age` in "remember_me" cookie. #823
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should read as additive since it's no longer replacing.



Version 0.6.3
Expand Down
20 changes: 3 additions & 17 deletions src/flask_login/login_manager.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
from datetime import datetime
from datetime import timedelta
from datetime import timezone

from flask import abort
from flask import current_app
from flask import flash
Expand Down Expand Up @@ -417,29 +413,19 @@ def _set_cookie(self, response):
samesite = config.get("REMEMBER_COOKIE_SAMESITE", COOKIE_SAMESITE)

if "_remember_seconds" in session:
duration = timedelta(seconds=session["_remember_seconds"])
# Convert float to int
duration = int(session["_remember_seconds"])
else:
duration = config.get("REMEMBER_COOKIE_DURATION", COOKIE_DURATION)

# prepare data
data = encode_cookie(str(session["_user_id"]))

if isinstance(duration, int):
duration = timedelta(seconds=duration)

try:
expires = datetime.now(timezone.utc) + duration
except TypeError as e:
raise Exception(
"REMEMBER_COOKIE_DURATION must be a datetime.timedelta,"
f" instead got: {duration}"
) from e

# actually set it
response.set_cookie(
cookie_name,
value=data,
expires=expires,
max_age=duration,
domain=domain,
path=path,
secure=secure,
Expand Down
6 changes: 1 addition & 5 deletions src/flask_login/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,7 @@ def login_user(user, remember=False, duration=None, force=False, fresh=True):
session["_remember"] = "set"
if duration is not None:
try:
# equal to timedelta.total_seconds() but works with Python 2.6
session["_remember_seconds"] = (
duration.microseconds
+ (duration.seconds + duration.days * 24 * 3600) * 10**6
) / 10.0**6
session["_remember_seconds"] = duration.total_seconds()
except AttributeError as e:
raise Exception(
f"duration must be a datetime.timedelta, instead got: {duration}"
Expand Down
67 changes: 18 additions & 49 deletions tests/test_login.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import unittest
from collections.abc import Hashable
from contextlib import contextmanager
from datetime import datetime
from datetime import timedelta
from datetime import timezone
from unittest.mock import ANY
from unittest.mock import Mock
from unittest.mock import patch

from flask import Blueprint
from flask import Flask
Expand Down Expand Up @@ -638,11 +634,8 @@ def test_remember_me_uses_custom_cookie_parameters(self):
c.get("/login-notch-remember")
cookie = c.get_cookie(name, domain, path)
self.assertIsNotNone(cookie)
self.assertIsNotNone(cookie.expires)
expected_date = datetime.now(timezone.utc) + duration
difference = expected_date - cookie.expires
self.assertLess(difference, timedelta(seconds=10))
self.assertGreater(difference, timedelta(seconds=-10))
self.assertIsNotNone(cookie.max_age)
self.assertEqual(cookie.max_age, duration.total_seconds())

def test_remember_me_custom_duration_uses_custom_cookie(self):
name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname"
Expand All @@ -654,11 +647,8 @@ def test_remember_me_custom_duration_uses_custom_cookie(self):
c.get("/login-notch-remember-custom")
cookie = c.get_cookie(name, domain, path)
self.assertIsNotNone(cookie)
self.assertIsNotNone(cookie.expires)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be removed. Likewise below.

expected_date = datetime.now(timezone.utc) + duration
difference = expected_date - cookie.expires
self.assertLess(difference, timedelta(seconds=10))
self.assertGreater(difference, timedelta(seconds=-10))
self.assertIsNotNone(cookie.max_age)
self.assertEqual(cookie.max_age, duration.total_seconds())

def test_remember_me_accepts_duration_as_int(self):
self.app.config["REMEMBER_COOKIE_DURATION"] = 172800
Expand All @@ -670,11 +660,8 @@ def test_remember_me_accepts_duration_as_int(self):
self.assertEqual(result.status_code, 200)
cookie = c.get_cookie(name, domain)
self.assertIsNotNone(cookie)
self.assertIsNotNone(cookie.expires)
expected_date = datetime.now(timezone.utc) + duration
difference = expected_date - cookie.expires
self.assertLess(difference, timedelta(seconds=10))
self.assertGreater(difference, timedelta(seconds=-10))
self.assertIsNotNone(cookie.max_age)
self.assertEqual(cookie.max_age, duration.total_seconds())

def test_remember_me_with_invalid_duration_returns_500_response(self):
self.app.config["REMEMBER_COOKIE_DURATION"] = "123"
Expand All @@ -693,19 +680,6 @@ def login_notch_remember_custom_invalid():
result = c.get("/login-notch-remember-custom-invalid")
self.assertEqual(result.status_code, 500)

def test_set_cookie_with_invalid_duration_raises_exception(self):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this being removed?

self.app.config["REMEMBER_COOKIE_DURATION"] = "123"

with self.assertRaises(Exception) as cm:
with self.app.test_request_context():
session["_user_id"] = 2
self.login_manager._set_cookie(None)

expected_exception_message = (
"REMEMBER_COOKIE_DURATION must be a datetime.timedelta, instead got: 123"
)
self.assertIn(expected_exception_message, str(cm.exception))

def test_set_cookie_with_invalid_custom_duration_raises_exception(self):
with self.assertRaises(Exception) as cm:
with self.app.test_request_context():
Expand All @@ -723,28 +697,23 @@ def test_remember_me_no_refresh_every_request(self):
c = self.app.test_client()
c.get("/login-notch-remember")
cookie1 = c.get_cookie("remember", domain, path)
self.assertIsNotNone(cookie1.expires)
self.assertIsNotNone(cookie1.max_age)
self._delete_session(c)
c.get("/username")
cookie2 = c.get_cookie("remember", domain, path)
self.assertEqual(cookie1.expires, cookie2.expires)
self.assertEqual(cookie1.max_age, cookie2.max_age)

def test_remember_me_refresh_each_request(self):
with patch("flask_login.login_manager.datetime") as mock_dt:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise here.

now = datetime.now(timezone.utc)
mock_dt.now = Mock(return_value=now)

domain = self.app.config["REMEMBER_COOKIE_DOMAIN"] = "localhost.local"
path = self.app.config["REMEMBER_COOKIE_PATH"] = "/"
self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = True
c = self.app.test_client()
c.get("/login-notch-remember")
cookie1 = c.get_cookie("remember", domain, path)
self.assertIsNotNone(cookie1.expires)
mock_dt.now.return_value = now + timedelta(seconds=1)
c.get("/username")
cookie2 = c.get_cookie("remember", domain, path)
self.assertNotEqual(cookie1.expires, cookie2.expires)
domain = self.app.config["REMEMBER_COOKIE_DOMAIN"] = "localhost.local"
path = self.app.config["REMEMBER_COOKIE_PATH"] = "/"
self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = True
c = self.app.test_client()
c.get("/login-notch-remember")
cookie1 = c.get_cookie("remember", domain, path)
self.assertIsNotNone(cookie1.max_age)
c.get("/username")
cookie2 = c.get_cookie("remember", domain, path)
self.assertEqual(cookie1.max_age, cookie2.max_age)

def test_remember_me_is_unfresh(self):
with self.app.test_client() as c:
Expand Down