-
-
Notifications
You must be signed in to change notification settings - Fork 75
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Branch: refs/heads/master Date: 2023-09-04T12:22:31+02:00 Author: Maurits van Rees (mauritsvanrees) <[email protected]> Commit: plone/plone.session@a093d48 Do not set an auth cookie after password reset, unless the user is authenticated. Otherwise anonymous users will be logged in immediately, even when autologin after password reset is false. Fixes plone/Products.CMFPlone#3835. Files changed: A news/3835.bugfix M plone/session/plugins/session.py M plone/session/tests/testPAS.py Repository: plone.session Branch: refs/heads/master Date: 2023-09-06T16:25:43+02:00 Author: Maurits van Rees (mauritsvanrees) <[email protected]> Commit: plone/plone.session@2c9213c 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 Repository: plone.session Branch: refs/heads/master Date: 2023-09-06T21:44:09-07:00 Author: David Glick (davisagli) <[email protected]> Commit: plone/plone.session@991f77b Merge pull request #44 from plone/maurits-do-not-update-credentials-for-anonymous-issue-3835 Do not set auth cookie after password reset, unless user is authenticated Files changed: A news/3835.bugfix M plone/session/plugins/session.py M plone/session/tests/testPAS.py
- Loading branch information
Showing
1 changed file
with
40 additions
and
19 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) <[email protected]> | ||
| 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 <https://github.com/plone/Products.CMFPlone/issues/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) <[email protected]> | ||
| 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) <[email protected]> | ||
| 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 <https://github.com/plone/Products.CMFPlone/issues/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' | ||
|
|