From 0754f68295ad72852464cbeb608d579046c910c9 Mon Sep 17 00:00:00 2001 From: Arcane-Ryn <157906738+Arcane-Ryn@users.noreply.github.com> Date: Sat, 10 Aug 2024 15:26:45 -0700 Subject: [PATCH 1/5] fixed cookie refresh bug Issue #824. Before, if a user was logged in with the login_user function when the remember parameter was set to false, their cookies would still be refreshed if the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" configuration option was set to true. This happens because if the login_user function has the remember parameter be false, it doesn't assign session["_rememeber"] any value. When session["_rememeber"] doesn't have any value and the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" configuration option is set to true, the _update_remember_cookie function sets the session["_rememeber"] value to "set". This fix makes it so if the login_user function is given false for the remember parameter, instead of leaving session["_remember"] empty, it sets the value to "unset". --- src/flask_login/login_manager.py | 2 ++ src/flask_login/utils.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/flask_login/login_manager.py b/src/flask_login/login_manager.py index 795e7441..e569ff48 100644 --- a/src/flask_login/login_manager.py +++ b/src/flask_login/login_manager.py @@ -402,6 +402,8 @@ def _update_remember_cookie(self, response): self._set_cookie(response) elif operation == "clear": self._clear_cookie(response) + elif operation == "unset": + session["_remember"] = "unset" return response diff --git a/src/flask_login/utils.py b/src/flask_login/utils.py index 57d49f60..3fd4d2fa 100644 --- a/src/flask_login/utils.py +++ b/src/flask_login/utils.py @@ -198,6 +198,8 @@ def login_user(user, remember=False, duration=None, force=False, fresh=True): raise Exception( f"duration must be a datetime.timedelta, instead got: {duration}" ) from e + else: + session["_remember"] = "unset" current_app.login_manager._update_request_context_with_user(user) user_logged_in.send(current_app._get_current_object(), user=_get_user()) From 824127d22e3dd0e8f97557b26476b7a224d0d777 Mon Sep 17 00:00:00 2001 From: Arcane-Ryn <157906738+Arcane-Ryn@users.noreply.github.com> Date: Sun, 11 Aug 2024 22:05:01 -0700 Subject: [PATCH 2/5] Added unit tests to demonstrate bugfix Added unit tests for each different combination of values for the "remember" and "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" values. --- tests/test_login.py | 98 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/tests/test_login.py b/tests/test_login.py index 50f87872..6e1bd724 100644 --- a/tests/test_login.py +++ b/tests/test_login.py @@ -1660,3 +1660,101 @@ def test_session_protection_modes(self): self.assertEqual("Anonymous", username.data.decode("utf-8")) is_fresh = c.get("/is-fresh") self.assertEqual("False", is_fresh.data.decode("utf-8")) + + +class CookieRefreshTest(unittest.TestCase): + """ + This class tests the bug from issue #824. It makes sure that if a user is + logged in with their "rememeber" parameter as False, that user doesn't + have any cookies, even if the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" + config option is True. + """ + def setUp(self): + self.app = Flask(__name__) + self.login_manager = LoginManager() + self.login_manager.init_app(self.app) + self.app.config["SECRET_KEY"] = "deterministic" + self.app.config["LOGIN_DISABLED"] = False + self.remember_cookie_name = "remember" + self.app.config["REMEMBER_COOKIE_NAME"] = self.remember_cookie_name + self.app.test_client_class = FlaskLoginClient + + @self.app.route("/login-notch0") + def login_notch0(): + self.app.config['REMEMBER_COOKIE_REFRESH_EACH_REQUEST'] = True + login_user(notch, remember=True) + return "Hello" + + @self.app.route("/login-notch1") + def login_notch1(): + self.app.config['REMEMBER_COOKIE_REFRESH_EACH_REQUEST'] = False + login_user(notch, remember=True) + return "Hello" + + @self.app.route("/login-notch2") + def login_notch2(): + self.app.config['REMEMBER_COOKIE_REFRESH_EACH_REQUEST'] = True + login_user(notch, remember=False) + return "Hello" + + @self.app.route("/login-notch3") + def login_notch3(): + self.app.config['REMEMBER_COOKIE_REFRESH_EACH_REQUEST'] = False + login_user(notch, remember=False) + return "Hello" + + # This will help us with the possibility of typos in the tests. Now + # we shouldn't have to check each response to help us set up state + # (such as login pages) to make sure it worked: we will always + # get an exception raised (rather than return a 404 response) + @self.app.errorhandler(404) + def handle_404(e): + raise e + + unittest.TestCase.setUp(self) + + #each of these function tests one of the ways cookie rememberance + #configuration options can be set. + def test_0(self): + with self.app.test_client() as c: + name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname" + path = self.app.config["REMEMBER_COOKIE_PATH"] = "/mypath" + domain = self.app.config["REMEMBER_COOKIE_DOMAIN"] = "localhost.local" + + c.get("/login-notch0") + + cookie = c.get_cookie(name, domain, path) + self.assertIsNotNone(cookie) + + def test_1(self): + with self.app.test_client() as c: + name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname" + path = self.app.config["REMEMBER_COOKIE_PATH"] = "/mypath" + domain = self.app.config["REMEMBER_COOKIE_DOMAIN"] = "localhost.local" + + c.get("/login-notch1") + + cookie = c.get_cookie(name, domain, path) + self.assertIsNotNone(cookie) + + def test_2(self): + with self.app.test_client() as c: + name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname" + path = self.app.config["REMEMBER_COOKIE_PATH"] = "/mypath" + domain = self.app.config["REMEMBER_COOKIE_DOMAIN"] = "localhost.local" + + c.get("/login-notch2") + + cookie = c.get_cookie(name, domain, path) + self.assertIsNone(cookie) + + def test_3(self): + with self.app.test_client() as c: + name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname" + path = self.app.config["REMEMBER_COOKIE_PATH"] = "/mypath" + domain = self.app.config["REMEMBER_COOKIE_DOMAIN"] = "localhost.local" + + c.get("/login-notch3") + cookie = c.get_cookie(name, domain, path) + + self.assertIsNone(cookie) \ No newline at end of file From 8e96789d1263ad3497848886c07a0fb3449778f7 Mon Sep 17 00:00:00 2001 From: Arcane-Ryn <157906738+Arcane-Ryn@users.noreply.github.com> Date: Sun, 11 Aug 2024 22:07:53 -0700 Subject: [PATCH 3/5] newline at end of testing file --- tests/test_login.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_login.py b/tests/test_login.py index 6e1bd724..4fa04273 100644 --- a/tests/test_login.py +++ b/tests/test_login.py @@ -1757,4 +1757,5 @@ def test_3(self): c.get("/login-notch3") cookie = c.get_cookie(name, domain, path) - self.assertIsNone(cookie) \ No newline at end of file + self.assertIsNone(cookie) + From 3e8f77920477862a496522ba54dbb59af6885cdd Mon Sep 17 00:00:00 2001 From: Arcane-Ryn <157906738+Arcane-Ryn@users.noreply.github.com> Date: Sun, 11 Aug 2024 22:19:23 -0700 Subject: [PATCH 4/5] Fixed code formatting --- tests/test_login.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_login.py b/tests/test_login.py index 4fa04273..7a58f1eb 100644 --- a/tests/test_login.py +++ b/tests/test_login.py @@ -1665,10 +1665,11 @@ def test_session_protection_modes(self): class CookieRefreshTest(unittest.TestCase): """ This class tests the bug from issue #824. It makes sure that if a user is - logged in with their "rememeber" parameter as False, that user doesn't - have any cookies, even if the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" + logged in with their "rememeber" parameter as False, that user doesn't + have any cookies, even if the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" config option is True. """ + def setUp(self): self.app = Flask(__name__) self.login_manager = LoginManager() @@ -1681,25 +1682,25 @@ def setUp(self): @self.app.route("/login-notch0") def login_notch0(): - self.app.config['REMEMBER_COOKIE_REFRESH_EACH_REQUEST'] = True + self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = True login_user(notch, remember=True) return "Hello" @self.app.route("/login-notch1") def login_notch1(): - self.app.config['REMEMBER_COOKIE_REFRESH_EACH_REQUEST'] = False + self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = False login_user(notch, remember=True) return "Hello" @self.app.route("/login-notch2") def login_notch2(): - self.app.config['REMEMBER_COOKIE_REFRESH_EACH_REQUEST'] = True + self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = True login_user(notch, remember=False) return "Hello" @self.app.route("/login-notch3") def login_notch3(): - self.app.config['REMEMBER_COOKIE_REFRESH_EACH_REQUEST'] = False + self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = False login_user(notch, remember=False) return "Hello" @@ -1713,8 +1714,8 @@ def handle_404(e): unittest.TestCase.setUp(self) - #each of these function tests one of the ways cookie rememberance - #configuration options can be set. + # each of these function tests one of the ways cookie rememberance + # configuration options can be set. def test_0(self): with self.app.test_client() as c: name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname" @@ -1725,7 +1726,7 @@ def test_0(self): cookie = c.get_cookie(name, domain, path) self.assertIsNotNone(cookie) - + def test_1(self): with self.app.test_client() as c: name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname" @@ -1736,7 +1737,7 @@ def test_1(self): cookie = c.get_cookie(name, domain, path) self.assertIsNotNone(cookie) - + def test_2(self): with self.app.test_client() as c: name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname" @@ -1747,7 +1748,7 @@ def test_2(self): cookie = c.get_cookie(name, domain, path) self.assertIsNone(cookie) - + def test_3(self): with self.app.test_client() as c: name = self.app.config["REMEMBER_COOKIE_NAME"] = "myname" @@ -1758,4 +1759,3 @@ def test_3(self): cookie = c.get_cookie(name, domain, path) self.assertIsNone(cookie) - From 5638c48382ae3e9493d307fc1665cc94d6e2f2e9 Mon Sep 17 00:00:00 2001 From: Arcane-Ryn <157906738+Arcane-Ryn@users.noreply.github.com> Date: Sun, 11 Aug 2024 22:20:58 -0700 Subject: [PATCH 5/5] even more formatting --- tests/test_login.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_login.py b/tests/test_login.py index 7a58f1eb..bb4701df 100644 --- a/tests/test_login.py +++ b/tests/test_login.py @@ -1691,19 +1691,19 @@ def login_notch1(): self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = False login_user(notch, remember=True) return "Hello" - + @self.app.route("/login-notch2") def login_notch2(): self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = True login_user(notch, remember=False) return "Hello" - + @self.app.route("/login-notch3") def login_notch3(): self.app.config["REMEMBER_COOKIE_REFRESH_EACH_REQUEST"] = False login_user(notch, remember=False) return "Hello" - + # This will help us with the possibility of typos in the tests. Now # we shouldn't have to check each response to help us set up state # (such as login pages) to make sure it worked: we will always