diff --git a/last_commit.txt b/last_commit.txt index 21146272f2..5c400eff78 100644 --- a/last_commit.txt +++ b/last_commit.txt @@ -1,38 +1,59 @@ -Repository: plone.app.event +Repository: plone.session Branch: refs/heads/master -Date: 2023-08-01T00:35:32Z -Author: pre-commit-ci[bot] (pre-commit-ci[bot]) <66853113+pre-commit-ci[bot]@users.noreply.github.com> -Commit: https://github.com/plone/plone.app.event/commit/e87ede2f0fa3a55aa20ae595f8c2eada675e8dec +Date: 2023-09-04T12:22:31+02:00 +Author: Maurits van Rees (mauritsvanrees) +Commit: https://github.com/plone/plone.session/commit/a093d48b1ff81e715e6836170c1f87a3f8da284a -[pre-commit.ci] pre-commit autoupdate +Do not set an auth cookie after password reset, unless the user is authenticated. -updates: -- [github.com/asottile/pyupgrade: v3.8.0 → v3.10.1](https://github.com/asottile/pyupgrade/compare/v3.8.0...v3.10.1) -- [github.com/psf/black: 23.3.0 → 23.7.0](https://github.com/psf/black/compare/23.3.0...23.7.0) -- [github.com/PyCQA/flake8: 6.0.0 → 6.1.0](https://github.com/PyCQA/flake8/compare/6.0.0...6.1.0) -- [github.com/collective/i18ndude: 6.0.0 → 6.1.0](https://github.com/collective/i18ndude/compare/6.0.0...6.1.0) +Otherwise anonymous users will be logged in immediately, even when autologin after password reset is false. +Fixes https://github.com/plone/Products.CMFPlone/issues/3835. Files changed: -M .pre-commit-config.yaml +A news/3835.bugfix +M plone/session/plugins/session.py +M plone/session/tests/testPAS.py -b'diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml\nindex 0d259b7f..f6f04834 100644\n--- a/.pre-commit-config.yaml\n+++ b/.pre-commit-config.yaml\n@@ -7,7 +7,7 @@ ci:\n \n repos:\n - repo: https://github.com/asottile/pyupgrade\n- rev: v3.8.0\n+ rev: v3.10.1\n hooks:\n - id: pyupgrade\n args: [--py38-plus]\n@@ -16,7 +16,7 @@ repos:\n hooks:\n - id: isort\n - repo: https://github.com/psf/black\n- rev: 23.3.0\n+ rev: 23.7.0\n hooks:\n - id: black\n - repo: https://github.com/collective/zpretty\n@@ -32,7 +32,7 @@ repos:\n # """\n ##\n - repo: https://github.com/PyCQA/flake8\n- rev: 6.0.0\n+ rev: 6.1.0\n hooks:\n - id: flake8\n - repo: https://github.com/codespell-project/codespell\n@@ -63,7 +63,7 @@ repos:\n - id: check-python-versions\n args: [\'--only\', \'setup.py,pyproject.toml\']\n - repo: https://github.com/collective/i18ndude\n- rev: "6.0.0"\n+ rev: "6.1.0"\n hooks:\n - id: i18ndude\n \n' +b'diff --git a/news/3835.bugfix b/news/3835.bugfix\nnew file mode 100644\nindex 0000000..28a7ddc\n--- /dev/null\n+++ b/news/3835.bugfix\n@@ -0,0 +1,4 @@\n+Do not set an auth cookie after password reset, unless the user is authenticated.\n+Otherwise anonymous users will be logged in immediately, even when autologin after password reset is false.\n+Fixes `issue 3835 `_.\n+[maurits]\ndiff --git a/plone/session/plugins/session.py b/plone/session/plugins/session.py\nindex 803087c..bd49f83 100644\n--- a/plone/session/plugins/session.py\n+++ b/plone/session/plugins/session.py\n@@ -281,8 +281,14 @@ def updateCredentials(self, request, response, login, new_password):\n authenticated_user = getSecurityManager().getUser()\n if authenticated_user is not None:\n authenticated_id = authenticated_user.getId()\n- # For anonymous, the id is empty\n- if authenticated_id and authenticated_id != user_id:\n+ # For anonymous, the id is empty.\n+ if not authenticated_id:\n+ # We should not update credentials when there are none currently.\n+ # Otherwise for example you are logged in after a password reset,\n+ # even when autologin after password reset is false.\n+ # See https://github.com/plone/Products.CMFPlone/issues/3835\n+ return\n+ if authenticated_id != user_id:\n return\n self._setupSession(user_id, response)\n \ndiff --git a/plone/session/tests/testPAS.py b/plone/session/tests/testPAS.py\nindex 8872042..9e9b2d1 100644\n--- a/plone/session/tests/testPAS.py\n+++ b/plone/session/tests/testPAS.py\n@@ -117,7 +117,10 @@ def testCredentialsUpdateAnonymous(self):\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n session.updateCredentials(request, request.response, "our_user", "password")\n- self.assertIsNotNone(\n+ # The anonymous user should not get a cookie: resetCredentials should\n+ # not do anything when there are no current credentials.\n+ # See https://github.com/plone/Products.CMFPlone/issues/3835\n+ self.assertIsNone(\n request.response.getCookie(session.cookie_name),\n )\n \n@@ -133,7 +136,7 @@ def testRefresh(self):\n logout()\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n- session.updateCredentials(request, request.response, "our_user", "password")\n+ session._setupSession(self.userid, request.response)\n cookie = request.response.getCookie(session.cookie_name)["value"]\n request2 = self.makeRequest(cookie)\n request2.form["type"] = "gif"\n' -Repository: plone.app.event +Repository: plone.session Branch: refs/heads/master -Date: 2023-09-04T09:56:55+02:00 +Date: 2023-09-06T16:25:43+02:00 Author: Maurits van Rees (mauritsvanrees) -Commit: https://github.com/plone/plone.app.event/commit/cd63cfff52f803951bb022133c2905357bfc196a +Commit: https://github.com/plone/plone.session/commit/2c9213c69edc3b5f08e33c167458e7283d071276 + +testRefresh: no need to logout first. + +Also, fix/clarify a few comments in the tests. + +Also, check that calling updateCredentials for ourselves *does* set a cookie. +This seems the most obvious test to do, but we seem to have missed this. + +Files changed: +M plone/session/tests/testPAS.py + +b'diff --git a/plone/session/tests/testPAS.py b/plone/session/tests/testPAS.py\nindex 9e9b2d1..c3cf6ec 100644\n--- a/plone/session/tests/testPAS.py\n+++ b/plone/session/tests/testPAS.py\n@@ -102,7 +102,9 @@ def testExtraction(self):\n self.assertEqual(creds, {})\n \n def testCredentialsUpdateUnknownUser(self):\n- # We are logged in as test user, which we do not want.\n+ # Check that calling updateCredentials for an *unknown* user does not set a\n+ # cookie if there is no cookie with credentials yet (you are anonymous).\n+ # So first logout.\n logout()\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n@@ -112,12 +114,14 @@ def testCredentialsUpdateUnknownUser(self):\n self.assertIsNone(request.response.getCookie(session.cookie_name))\n \n def testCredentialsUpdateAnonymous(self):\n- # We are logged in as test user, which we do not want.\n+ # Check that calling updateCredentials for a *known* user does not set a\n+ # cookie if there is no cookie with credentials yet (you are anonymous).\n+ # So first logout.\n logout()\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n session.updateCredentials(request, request.response, "our_user", "password")\n- # The anonymous user should not get a cookie: resetCredentials should\n+ # The anonymous user should not get a cookie: updateCredentials should\n # not do anything when there are no current credentials.\n # See https://github.com/plone/Products.CMFPlone/issues/3835\n self.assertIsNone(\n@@ -125,15 +129,21 @@ def testCredentialsUpdateAnonymous(self):\n )\n \n def testCredentialsUpdateOtherUser(self):\n- # We are logged in as test user, which we DO want in this test.\n- # The session should not be updated then.\n+ # Check that calling updateCredentials for someone other than the logged in\n+ # user does not set a cookie.\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n session.updateCredentials(request, request.response, "our_user", "password")\n self.assertIsNone(request.response.getCookie(session.cookie_name))\n \n+ def testCredentialsUpdateSameUser(self):\n+ # Check that calling updateCredentials for ourselves *does* set a cookie.\n+ session = self.folder.pas.session\n+ request = self.makeRequest("test string")\n+ session.updateCredentials(request, request.response, self.userid, "password")\n+ self.assertIsNone(request.response.getCookie(session.cookie_name))\n+\n def testRefresh(self):\n- logout()\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n session._setupSession(self.userid, request.response)\n@@ -146,14 +156,14 @@ def testRefresh(self):\n def testUnicodeUserid(self):\n response = MockResponse()\n session = self.folder.pas.session\n- # This step would fail.\n+ # The main thing we test, is that the next call does not give a traceback:\n session._setupSession(self.userid, response)\n \n def testSpecialCharUserid(self):\n unicode_userid = "\xc3\xa3bcd\xc3\xa9fghijk"\n response = MockResponse()\n session = self.folder.pas.session\n- # This step would fail.\n+ # The main thing we test, is that the next call does not give a traceback:\n session._setupSession(unicode_userid, response)\n \n def testCookieInvalidAfterLogout(self):\n' + +Repository: plone.session + + +Branch: refs/heads/master +Date: 2023-09-06T21:44:09-07:00 +Author: David Glick (davisagli) +Commit: https://github.com/plone/plone.session/commit/991f77bf5d4e33f12016234570b6ff6f6da56641 -Merge pull request #382 from plone/pre-commit-ci-update-config +Merge pull request #44 from plone/maurits-do-not-update-credentials-for-anonymous-issue-3835 -[pre-commit.ci] pre-commit autoupdate +Do not set auth cookie after password reset, unless user is authenticated Files changed: -M .pre-commit-config.yaml +A news/3835.bugfix +M plone/session/plugins/session.py +M plone/session/tests/testPAS.py -b'diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml\nindex 0d259b7f..f6f04834 100644\n--- a/.pre-commit-config.yaml\n+++ b/.pre-commit-config.yaml\n@@ -7,7 +7,7 @@ ci:\n \n repos:\n - repo: https://github.com/asottile/pyupgrade\n- rev: v3.8.0\n+ rev: v3.10.1\n hooks:\n - id: pyupgrade\n args: [--py38-plus]\n@@ -16,7 +16,7 @@ repos:\n hooks:\n - id: isort\n - repo: https://github.com/psf/black\n- rev: 23.3.0\n+ rev: 23.7.0\n hooks:\n - id: black\n - repo: https://github.com/collective/zpretty\n@@ -32,7 +32,7 @@ repos:\n # """\n ##\n - repo: https://github.com/PyCQA/flake8\n- rev: 6.0.0\n+ rev: 6.1.0\n hooks:\n - id: flake8\n - repo: https://github.com/codespell-project/codespell\n@@ -63,7 +63,7 @@ repos:\n - id: check-python-versions\n args: [\'--only\', \'setup.py,pyproject.toml\']\n - repo: https://github.com/collective/i18ndude\n- rev: "6.0.0"\n+ rev: "6.1.0"\n hooks:\n - id: i18ndude\n \n' +b'diff --git a/news/3835.bugfix b/news/3835.bugfix\nnew file mode 100644\nindex 0000000..28a7ddc\n--- /dev/null\n+++ b/news/3835.bugfix\n@@ -0,0 +1,4 @@\n+Do not set an auth cookie after password reset, unless the user is authenticated.\n+Otherwise anonymous users will be logged in immediately, even when autologin after password reset is false.\n+Fixes `issue 3835 `_.\n+[maurits]\ndiff --git a/plone/session/plugins/session.py b/plone/session/plugins/session.py\nindex 803087c..bd49f83 100644\n--- a/plone/session/plugins/session.py\n+++ b/plone/session/plugins/session.py\n@@ -281,8 +281,14 @@ def updateCredentials(self, request, response, login, new_password):\n authenticated_user = getSecurityManager().getUser()\n if authenticated_user is not None:\n authenticated_id = authenticated_user.getId()\n- # For anonymous, the id is empty\n- if authenticated_id and authenticated_id != user_id:\n+ # For anonymous, the id is empty.\n+ if not authenticated_id:\n+ # We should not update credentials when there are none currently.\n+ # Otherwise for example you are logged in after a password reset,\n+ # even when autologin after password reset is false.\n+ # See https://github.com/plone/Products.CMFPlone/issues/3835\n+ return\n+ if authenticated_id != user_id:\n return\n self._setupSession(user_id, response)\n \ndiff --git a/plone/session/tests/testPAS.py b/plone/session/tests/testPAS.py\nindex 8872042..c3cf6ec 100644\n--- a/plone/session/tests/testPAS.py\n+++ b/plone/session/tests/testPAS.py\n@@ -102,7 +102,9 @@ def testExtraction(self):\n self.assertEqual(creds, {})\n \n def testCredentialsUpdateUnknownUser(self):\n- # We are logged in as test user, which we do not want.\n+ # Check that calling updateCredentials for an *unknown* user does not set a\n+ # cookie if there is no cookie with credentials yet (you are anonymous).\n+ # So first logout.\n logout()\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n@@ -112,28 +114,39 @@ def testCredentialsUpdateUnknownUser(self):\n self.assertIsNone(request.response.getCookie(session.cookie_name))\n \n def testCredentialsUpdateAnonymous(self):\n- # We are logged in as test user, which we do not want.\n+ # Check that calling updateCredentials for a *known* user does not set a\n+ # cookie if there is no cookie with credentials yet (you are anonymous).\n+ # So first logout.\n logout()\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n session.updateCredentials(request, request.response, "our_user", "password")\n- self.assertIsNotNone(\n+ # The anonymous user should not get a cookie: updateCredentials should\n+ # not do anything when there are no current credentials.\n+ # See https://github.com/plone/Products.CMFPlone/issues/3835\n+ self.assertIsNone(\n request.response.getCookie(session.cookie_name),\n )\n \n def testCredentialsUpdateOtherUser(self):\n- # We are logged in as test user, which we DO want in this test.\n- # The session should not be updated then.\n+ # Check that calling updateCredentials for someone other than the logged in\n+ # user does not set a cookie.\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n session.updateCredentials(request, request.response, "our_user", "password")\n self.assertIsNone(request.response.getCookie(session.cookie_name))\n \n+ def testCredentialsUpdateSameUser(self):\n+ # Check that calling updateCredentials for ourselves *does* set a cookie.\n+ session = self.folder.pas.session\n+ request = self.makeRequest("test string")\n+ session.updateCredentials(request, request.response, self.userid, "password")\n+ self.assertIsNone(request.response.getCookie(session.cookie_name))\n+\n def testRefresh(self):\n- logout()\n session = self.folder.pas.session\n request = self.makeRequest("test string")\n- session.updateCredentials(request, request.response, "our_user", "password")\n+ session._setupSession(self.userid, request.response)\n cookie = request.response.getCookie(session.cookie_name)["value"]\n request2 = self.makeRequest(cookie)\n request2.form["type"] = "gif"\n@@ -143,14 +156,14 @@ def testRefresh(self):\n def testUnicodeUserid(self):\n response = MockResponse()\n session = self.folder.pas.session\n- # This step would fail.\n+ # The main thing we test, is that the next call does not give a traceback:\n session._setupSession(self.userid, response)\n \n def testSpecialCharUserid(self):\n unicode_userid = "\xc3\xa3bcd\xc3\xa9fghijk"\n response = MockResponse()\n session = self.folder.pas.session\n- # This step would fail.\n+ # The main thing we test, is that the next call does not give a traceback:\n session._setupSession(unicode_userid, response)\n \n def testCookieInvalidAfterLogout(self):\n'