From d55c7ae9f794f5cd35593eebf306ec5c4137d9d9 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Sat, 25 Dec 2021 18:09:55 -0800 Subject: [PATCH 01/16] test(deprecation): Fix all warnings from our code Resolve all the deprecation warnings that originate in this package's code that are exposed by running the tests. --- src/plone/restapi/testing.py | 16 ++++++++-------- src/plone/restapi/tests/test_documentation.py | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/plone/restapi/testing.py b/src/plone/restapi/testing.py index 303eed97cc..30197569ab 100644 --- a/src/plone/restapi/testing.py +++ b/src/plone/restapi/testing.py @@ -18,7 +18,7 @@ from plone.registry.interfaces import IRegistry from plone.restapi.tests.dxtypes import INDEXES as DX_TYPES_INDEXES from plone.restapi.tests.helpers import add_catalog_indexes -from plone.testing import z2 +from plone.testing import zope from plone.testing.layer import Layer from plone.uuid.interfaces import IUUIDGenerator from Products.CMFCore.utils import getToolByName @@ -118,7 +118,7 @@ def setUpZope(self, app, configurationContext): xmlconfig.file("testing.zcml", plone.restapi, context=configurationContext) self.loadZCML(package=collective.MockMailHost) - z2.installProduct(app, "plone.restapi") + zope.installProduct(app, "plone.restapi") def setUpPloneSite(self, portal): portal.acl_users.userFolderAddUser( @@ -145,7 +145,7 @@ def setUpPloneSite(self, portal): bases=(PLONE_RESTAPI_DX_FIXTURE,), name="PloneRestApiDXLayer:Integration" ) PLONE_RESTAPI_DX_FUNCTIONAL_TESTING = FunctionalTesting( - bases=(PLONE_RESTAPI_DX_FIXTURE, z2.ZSERVER_FIXTURE), + bases=(PLONE_RESTAPI_DX_FIXTURE, zope.WSGI_SERVER_FIXTURE), name="PloneRestApiDXLayer:Functional", ) @@ -175,7 +175,7 @@ def setUpZope(self, app, configurationContext): xmlconfig.file("configure.zcml", plone.restapi, context=configurationContext) xmlconfig.file("testing.zcml", plone.restapi, context=configurationContext) - z2.installProduct(app, "plone.restapi") + zope.installProduct(app, "plone.restapi") def setUpPloneSite(self, portal): portal.acl_users.userFolderAddUser( @@ -201,7 +201,7 @@ def setUpPloneSite(self, portal): bases=(PLONE_RESTAPI_DX_PAM_FIXTURE,), name="PloneRestApiDXPAMLayer:Integration" ) PLONE_RESTAPI_DX_PAM_FUNCTIONAL_TESTING = FunctionalTesting( - bases=(PLONE_RESTAPI_DX_PAM_FIXTURE, z2.ZSERVER_FIXTURE), + bases=(PLONE_RESTAPI_DX_PAM_FIXTURE, zope.WSGI_SERVER_FIXTURE), name="PloneRestApiDXPAMLayer:Functional", ) @@ -216,7 +216,7 @@ def setUpZope(self, app, configurationContext): xmlconfig.file("configure.zcml", plone.restapi, context=configurationContext) xmlconfig.file("testing.zcml", plone.restapi, context=configurationContext) - z2.installProduct(app, "plone.restapi") + zope.installProduct(app, "plone.restapi") PLONE_RESTAPI_ITERATE_FIXTURE = PloneRestApiDXIterateLayer() @@ -225,7 +225,7 @@ def setUpZope(self, app, configurationContext): name="PloneRestApiDXIterateLayer:Integration", ) PLONE_RESTAPI_ITERATE_FUNCTIONAL_TESTING = FunctionalTesting( - bases=(PLONE_RESTAPI_ITERATE_FIXTURE, z2.ZSERVER_FIXTURE), + bases=(PLONE_RESTAPI_ITERATE_FIXTURE, zope.WSGI_SERVER_FIXTURE), name="PloneRestApiDXIterateLayer:Functional", ) @@ -243,7 +243,7 @@ def setUpPloneSite(self, portal): bases=(PLONE_RESTAPI_BLOCKS_FIXTURE,), name="PloneRestApIBlocksLayer:Integration" ) PLONE_RESTAPI_BLOCKS_FUNCTIONAL_TESTING = FunctionalTesting( - bases=(PLONE_RESTAPI_BLOCKS_FIXTURE, z2.ZSERVER_FIXTURE), + bases=(PLONE_RESTAPI_BLOCKS_FIXTURE, zope.WSGI_SERVER_FIXTURE), name="PloneRestApIBlocksLayer:Functional", ) diff --git a/src/plone/restapi/tests/test_documentation.py b/src/plone/restapi/tests/test_documentation.py index fe6d391de1..90f1b38a3f 100644 --- a/src/plone/restapi/tests/test_documentation.py +++ b/src/plone/restapi/tests/test_documentation.py @@ -27,7 +27,7 @@ from plone.restapi.testing import RelativeSession from plone.restapi.tests.statictime import StaticTime from plone.scale import storage -from plone.testing.z2 import Browser +from plone.testing.zope import Browser from zope.component import createObject from zope.component import getUtility from zope.component.hooks import getSite From 6f4b1b5958b9e943efcbdd164f7e887afd49d9d8 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Sat, 25 Dec 2021 10:44:12 -0800 Subject: [PATCH 02/16] test(auth): Cleanup redundant assertions --- .../restapi/tests/test_functional_auth.py | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/plone/restapi/tests/test_functional_auth.py b/src/plone/restapi/tests/test_functional_auth.py index 59b8b31dec..8a8005f956 100644 --- a/src/plone/restapi/tests/test_functional_auth.py +++ b/src/plone/restapi/tests/test_functional_auth.py @@ -60,8 +60,16 @@ def test_login_with_valid_credentials_returns_token(self): headers={"Accept": "application/json"}, json={"login": TEST_USER_NAME, "password": TEST_USER_PASSWORD}, ) - self.assertEqual(200, response.status_code) - self.assertIn("token", response.json()) + self.assertEqual( + 200, + response.status_code, + "Wrong API login response status code", + ) + self.assertIn( + "token", + response.json(), + "Authentication token missing from API response JSON", + ) def test_api_login_grants_zmi(self): """ @@ -79,16 +87,6 @@ def test_api_login_grants_zmi(self): login_resp.cookies, "Plone session cookie missing from API login POST response", ) - self.assertEqual( - login_resp.status_code, - 200, - "Wrong API login response status code", - ) - self.assertIn( - "token", - login_resp.json(), - "Authentication token missing from API response JSON", - ) zmi_resp = session.get( self.layer["app"].absolute_url() + "/manage_workspace", From 26687663825dd99f7eb4b56706a3b3b16424ff9e Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Sun, 26 Dec 2021 00:07:41 -0800 Subject: [PATCH 03/16] test(auth): Fix cookie presence checks Checking cookies on the individual response only checks for cookies set *by that response*. Cookie checks should probable be done on the session unless there is specific reason to assert which response set them. --- src/plone/restapi/tests/test_functional_auth.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plone/restapi/tests/test_functional_auth.py b/src/plone/restapi/tests/test_functional_auth.py index 8a8005f956..5298da8eb5 100644 --- a/src/plone/restapi/tests/test_functional_auth.py +++ b/src/plone/restapi/tests/test_functional_auth.py @@ -77,14 +77,14 @@ def test_api_login_grants_zmi(self): """ session = requests.Session() self.addCleanup(session.close) - login_resp = session.post( + session.post( self.portal_url + "/@login", headers={"Accept": "application/json"}, json={"login": SITE_OWNER_NAME, "password": TEST_USER_PASSWORD}, ) self.assertIn( "__ac", - login_resp.cookies, + session.cookies, "Plone session cookie missing from API login POST response", ) @@ -183,7 +183,7 @@ def test_cookie_login_grants_api(self): ) self.assertIn( "__ac", - login_resp.history[0].cookies, + session.cookies, "Plone session cookie missing form login POST response", ) self.assertEqual( From 9422195e976a7f6f917491cb368607f836508f8c Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Fri, 24 Dec 2021 15:18:11 -0800 Subject: [PATCH 04/16] fix(auth): Set both classic and API login cookies Use the appropriate PAS interface for setting the cookies used by both classic Plone and Volto's React component. Move the API token generation out of the login API endpoint view and into the PAS plugin where it belongs. Then it will be per the PAS plugin configuration to set the cookie. The plugin then also makes the token available to the login API endpoint view via the request so it can then also return the token in the JSON response. --- src/plone/restapi/pas/plugin.py | 47 ++++++++- src/plone/restapi/services/auth/login.py | 4 +- src/plone/restapi/setuphandlers.py | 6 +- .../restapi/tests/test_functional_auth.py | 99 +++++++++++++------ 4 files changed, 121 insertions(+), 35 deletions(-) diff --git a/src/plone/restapi/pas/plugin.py b/src/plone/restapi/pas/plugin.py index 643c77e35f..ad3198057a 100644 --- a/src/plone/restapi/pas/plugin.py +++ b/src/plone/restapi/pas/plugin.py @@ -13,6 +13,7 @@ from Products.PluggableAuthService.interfaces.plugins import IAuthenticationPlugin from Products.PluggableAuthService.interfaces.plugins import IChallengePlugin from Products.PluggableAuthService.interfaces.plugins import IExtractionPlugin +from Products.PluggableAuthService.interfaces.plugins import ICredentialsUpdatePlugin from Products.PluggableAuthService.plugins.BasePlugin import BasePlugin from zope.component import getUtility from zope.interface import implementer @@ -39,7 +40,12 @@ def addJWTAuthenticationPlugin(self, id_, title=None, REQUEST=None): ) -@implementer(IAuthenticationPlugin, IChallengePlugin, IExtractionPlugin) +@implementer( + IAuthenticationPlugin, + IChallengePlugin, + IExtractionPlugin, + ICredentialsUpdatePlugin, +) class JWTAuthenticationPlugin(BasePlugin): """Plone PAS plugin for authentication with JSON web tokens (JWT).""" @@ -51,6 +57,7 @@ class JWTAuthenticationPlugin(BasePlugin): store_tokens = False _secret = None _tokens = None + cookie_name = "auth_token" # ZMI tab for configuration page manage_options = ( @@ -59,9 +66,11 @@ class JWTAuthenticationPlugin(BasePlugin): security.declareProtected(ManagePortal, "manage_config") manage_config = PageTemplateFile("config", globals(), __name__="manage_config") - def __init__(self, id_, title=None): + def __init__(self, id_, title=None, cookie_name=None): self._setId(id_) self.title = title + if cookie_name: + self.cookie_name = cookie_name # Initiate a challenge to the user to provide credentials. @security.private @@ -95,6 +104,8 @@ def extractCredentials(self, request): return creds creds = {} + + # Prefer the Authorization Bearer header if present auth = request._auth if auth is None: return @@ -102,6 +113,12 @@ def extractCredentials(self, request): creds["token"] = auth.split()[-1] return creds + # Finally, use the cookie if present + cookie = request.get(self.cookie_name, "") + if cookie: + creds["token"] = cookie + return creds + # IAuthenticationPlugin implementation @security.private def authenticateCredentials(self, credentials): @@ -127,6 +144,32 @@ def authenticateCredentials(self, credentials): return (userid, userid) + @security.private + def updateCredentials(self, request, response, login, new_password): + """ + Generate a new token for use both in the Bearer header and the cookie. + """ + # Unfortunately PAS itself is confused as to whether this plugin method should + # get the immutable user ID or the mutable, user-facing user login/name. Real + # usage in the Plone code base also uses both. Do our best to guess which. + user_id = login + data = dict(fullname="") + user = self._getPAS().getUserById(login) + if user is None: + user = self._getPAS().getUser(login) + if user is not None: + user_id = user.getId() + data["fullname"] = user.getProperty("fullname") + token = self.create_token(user_id, data=data) + # Make available on the request for further use such as returning it in the JSON + # body of the response if the current request is for the REST API login view. + request[self.cookie_name] = token + # Make the token available to the client browser for use in UI code such as when + # the login happened through Plone Classic so that the the Volro React + # components can retrieve the token that way and use the Authorization Bearer + # header from then on. + response.setCookie(self.cookie_name, token, path="/") + @security.protected(ManagePortal) @postonly def manage_updateConfig(self, REQUEST): diff --git a/src/plone/restapi/services/auth/login.py b/src/plone/restapi/services/auth/login.py index 47813440d5..b0d65c6648 100644 --- a/src/plone/restapi/services/auth/login.py +++ b/src/plone/restapi/services/auth/login.py @@ -75,9 +75,7 @@ def reply(self): ) login_view._post_login() - payload = {} - payload["fullname"] = user.getProperty("fullname") - return {"token": plugin.create_token(user.getId(), data=payload)} + return {"token": self.request[plugin.cookie_name]} def _find_userfolder(self, userid): """Try to find a user folder that contains a user with the given diff --git a/src/plone/restapi/setuphandlers.py b/src/plone/restapi/setuphandlers.py index 2755e39a5c..ab0c48054a 100644 --- a/src/plone/restapi/setuphandlers.py +++ b/src/plone/restapi/setuphandlers.py @@ -39,7 +39,11 @@ def install_pas_plugin(context): uf._setObject(plugin.getId(), plugin) plugin = uf["jwt_auth"] plugin.manage_activateInterfaces( - ["IAuthenticationPlugin", "IExtractionPlugin"] + [ + "IAuthenticationPlugin", + "IExtractionPlugin", + "ICredentialsUpdatePlugin", + ], ) if uf_parent is uf_parent.getPhysicalRoot(): break diff --git a/src/plone/restapi/tests/test_functional_auth.py b/src/plone/restapi/tests/test_functional_auth.py index 5298da8eb5..52691a138c 100644 --- a/src/plone/restapi/tests/test_functional_auth.py +++ b/src/plone/restapi/tests/test_functional_auth.py @@ -71,9 +71,9 @@ def test_login_with_valid_credentials_returns_token(self): "Authentication token missing from API response JSON", ) - def test_api_login_grants_zmi(self): + def test_api_login_sets_classic_cookie(self): """ - Logging in via the API also grants access to the Zope root ZMI. + Logging in via the API also sets the Plone classic auth cookie. """ session = requests.Session() self.addCleanup(session.close) @@ -88,6 +88,72 @@ def test_api_login_grants_zmi(self): "Plone session cookie missing from API login POST response", ) + def test_classic_login_sets_api_token_cookie(self): + """ + Logging in via Plone classic login form also sets cookie with the API token. + + The cookie that Volto React components will recognize on first request and use + as the Authorization Bearer header for subsequent requests. + """ + session = requests.Session() + self.addCleanup(session.close) + challenge_resp = session.get(self.private_document_url) + self.assertEqual( + challenge_resp.status_code, + 200, + "Wrong Plone login challenge status code", + ) + self.assertTrue( + ' Date: Sun, 26 Dec 2021 20:28:43 -0800 Subject: [PATCH 05/16] fix(auth): API token cookie match token expiration --- src/plone/restapi/pas/plugin.py | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/plone/restapi/pas/plugin.py b/src/plone/restapi/pas/plugin.py index ad3198057a..b9f994a97d 100644 --- a/src/plone/restapi/pas/plugin.py +++ b/src/plone/restapi/pas/plugin.py @@ -2,6 +2,7 @@ from AccessControl.SecurityInfo import ClassSecurityInfo from BTrees.OIBTree import OIBTree from BTrees.OOBTree import OOBTree +from DateTime import DateTime from datetime import datetime from datetime import timedelta from plone.keyring.interfaces import IKeyManager @@ -160,7 +161,7 @@ def updateCredentials(self, request, response, login, new_password): if user is not None: user_id = user.getId() data["fullname"] = user.getProperty("fullname") - token = self.create_token(user_id, data=data) + payload, token = self.create_payload_token(user_id, data=data) # Make available on the request for further use such as returning it in the JSON # body of the response if the current request is for the REST API login view. request[self.cookie_name] = token @@ -168,7 +169,16 @@ def updateCredentials(self, request, response, login, new_password): # the login happened through Plone Classic so that the the Volro React # components can retrieve the token that way and use the Authorization Bearer # header from then on. - response.setCookie(self.cookie_name, token, path="/") + cookie_kwargs = {} + if "exp" in payload: + # Match the token expiration date/time. + cookie_kwargs["expires"] = DateTime(payload["exp"]).toZone("GMT").rfc822() + response.setCookie( + self.cookie_name, + token, + path="/", + **cookie_kwargs, + ) @security.protected(ManagePortal) @postonly @@ -227,7 +237,10 @@ def delete_token(self, token): del self._tokens[userid][token] return True - def create_token(self, userid, timeout=None, data=None): + def create_payload_token(self, userid, timeout=None, data=None): + """ + Create and return both a JWT payload and the signed token. + """ payload = {} payload["sub"] = userid if timeout is None: @@ -244,4 +257,15 @@ def create_token(self, userid, timeout=None, data=None): if userid not in self._tokens: self._tokens[userid] = OIBTree() self._tokens[userid][token] = int(time.time()) + return payload, token + + def create_token(self, userid, timeout=None, data=None): + """ + Create a JWT payload and the signed token, return the token. + """ + _, token = self.create_payload_token( + userid, + timeout=timeout, + data=data, + ) return token From c941bf2875738f5e993867b2f5319d42dbd2c99c Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Sat, 25 Dec 2021 22:40:38 -0800 Subject: [PATCH 06/16] fix(auth): Logout all sessions when any logout When logging out of Plone via the API endpoint or through the classic logout link, all PAS plugins should reset all credentials. --- src/plone/restapi/pas/plugin.py | 12 +++++++ src/plone/restapi/setuphandlers.py | 1 + .../restapi/tests/test_functional_auth.py | 32 +++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/src/plone/restapi/pas/plugin.py b/src/plone/restapi/pas/plugin.py index b9f994a97d..a60f672122 100644 --- a/src/plone/restapi/pas/plugin.py +++ b/src/plone/restapi/pas/plugin.py @@ -15,6 +15,7 @@ from Products.PluggableAuthService.interfaces.plugins import IChallengePlugin from Products.PluggableAuthService.interfaces.plugins import IExtractionPlugin from Products.PluggableAuthService.interfaces.plugins import ICredentialsUpdatePlugin +from Products.PluggableAuthService.interfaces.plugins import ICredentialsResetPlugin from Products.PluggableAuthService.plugins.BasePlugin import BasePlugin from zope.component import getUtility from zope.interface import implementer @@ -46,6 +47,7 @@ def addJWTAuthenticationPlugin(self, id_, title=None, REQUEST=None): IChallengePlugin, IExtractionPlugin, ICredentialsUpdatePlugin, + ICredentialsResetPlugin, ) class JWTAuthenticationPlugin(BasePlugin): """Plone PAS plugin for authentication with JSON web tokens (JWT).""" @@ -180,6 +182,16 @@ def updateCredentials(self, request, response, login, new_password): **cookie_kwargs, ) + @security.private + def resetCredentials(self, request, response): + """ + Expire the token and remove the cookie. + """ + if self.cookie_name in request: + if self.store_tokens: + self.delete_token(request[self.cookie_name]) + response.expireCookie(self.cookie_name, path="/") + @security.protected(ManagePortal) @postonly def manage_updateConfig(self, REQUEST): diff --git a/src/plone/restapi/setuphandlers.py b/src/plone/restapi/setuphandlers.py index ab0c48054a..7e0453ac13 100644 --- a/src/plone/restapi/setuphandlers.py +++ b/src/plone/restapi/setuphandlers.py @@ -43,6 +43,7 @@ def install_pas_plugin(context): "IAuthenticationPlugin", "IExtractionPlugin", "ICredentialsUpdatePlugin", + "ICredentialsResetPlugin", ], ) if uf_parent is uf_parent.getPhysicalRoot(): diff --git a/src/plone/restapi/tests/test_functional_auth.py b/src/plone/restapi/tests/test_functional_auth.py index 52691a138c..01e5e45fb4 100644 --- a/src/plone/restapi/tests/test_functional_auth.py +++ b/src/plone/restapi/tests/test_functional_auth.py @@ -259,6 +259,38 @@ def test_cookie_login_grants_api(self): "Wrong Plone object URL from API response JSON", ) + def test_api_logout_expires_both_cookies(self): + """ + API log out deletes both the API token and Plone classic auth cookie. + """ + session = requests.Session() + self.addCleanup(session.close) + session.post( + self.portal_url + "/@login", + headers={"Accept": "application/json"}, + json={"login": SITE_OWNER_NAME, "password": TEST_USER_PASSWORD}, + ) + transaction.commit() + logout_resp = session.post( + self.portal_url + "/@logout", + headers={"Accept": "application/json"}, + ) + self.assertEqual( + logout_resp.status_code, + 204, + "Wrong API logout response status code", + ) + self.assertNotIn( + "__ac", + session.cookies, + "Plone session cookie remains after API logout POST response", + ) + self.assertNotIn( + "auth_token", + session.cookies, + "API token cookie remains after API logout POST response", + ) + def test_accessing_private_document_with_valid_token_succeeds(self): # login and generate a valid token response = requests.post( From 1228b587b634fa6b9e9905bbf5982a28c33df142 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Sun, 26 Dec 2021 12:13:11 -0800 Subject: [PATCH 07/16] fix(auth): API login for Zope root acl_users user The Plone login handling code depends on the user's password being at the same place in the request as the classic Plone login form puts it in order to set the correct authentication cookie. Without it, when logging in via the Volto UI component as a user from the Zope root `acl_users` (e.g. `admin` or `SITE_OWNER_NAME`), they aren't logged into Plone classic. The other direction is fine, logging in as `admin` to Plone classic results in a new request to the Volto UI being logged in. Fix that edge case by mimicking the request keys of the login form after parsing the login POST JSON body. --- news/1303.feature | 2 ++ src/plone/restapi/services/auth/login.py | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 news/1303.feature diff --git a/news/1303.feature b/news/1303.feature new file mode 100644 index 0000000000..7100014e67 --- /dev/null +++ b/news/1303.feature @@ -0,0 +1,2 @@ +Logging in to or out of Plone classic or the API does the same in the other. +[rpatterson] diff --git a/src/plone/restapi/services/auth/login.py b/src/plone/restapi/services/auth/login.py index b0d65c6648..b51962dcb9 100644 --- a/src/plone/restapi/services/auth/login.py +++ b/src/plone/restapi/services/auth/login.py @@ -28,8 +28,10 @@ def reply(self): if "IDisableCSRFProtection" in dir(plone.protect.interfaces): alsoProvides(self.request, plone.protect.interfaces.IDisableCSRFProtection) - userid = data["login"] - password = data["password"] + # Also add credentials to the request for other code that depends on it. In + # particular, the PAS cookie authentication plugin depends on `__ac_password`. + userid = self.request.form["__ac_name"] = data["login"] + password = self.request.form["__ac_password"] = data["password"] uf = self._find_userfolder(userid) if uf is not None: From 68e9cdb21d43dccd7126add457522948f671469c Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Mon, 3 Jan 2022 16:19:02 -0800 Subject: [PATCH 08/16] fix(log): Log error conditions in the server logs An error that indicates incorrect configuration, set up, or otherwise something an administrator would probably want to know, should also be logged to the server logs. Those log messages should including enough information for an administrator to find the specific instance of the issue (e.g. which Plone portal in a multi-portal ZODB). --- src/plone/restapi/services/auth/login.py | 11 ++++++++++- src/plone/restapi/tests/test_auth.py | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/plone/restapi/services/auth/login.py b/src/plone/restapi/services/auth/login.py index b51962dcb9..21fa9fa6b5 100644 --- a/src/plone/restapi/services/auth/login.py +++ b/src/plone/restapi/services/auth/login.py @@ -7,8 +7,11 @@ from zope.interface import alsoProvides from zope import component +import logging import plone.protect.interfaces +logger = logging.getLogger(__name__) + class Login(Service): """Handles login and returns a JSON web token (JWT).""" @@ -45,10 +48,16 @@ def reply(self): if plugin is None: self.request.response.setStatus(501) + message = "JWT authentication plugin not installed" + logger.error( + "%s: %s", + message, + "/".join(uf.getPhysicalPath()), + ) return dict( error=dict( type="Login failed", - message="JWT authentication plugin not installed.", + message=message, ) ) diff --git a/src/plone/restapi/tests/test_auth.py b/src/plone/restapi/tests/test_auth.py index e619e0ab75..2db395dfca 100644 --- a/src/plone/restapi/tests/test_auth.py +++ b/src/plone/restapi/tests/test_auth.py @@ -86,7 +86,8 @@ def test_login_with_zope_user_fails_without_pas_plugin(self): res = service.reply() self.assertIn("error", res) self.assertEqual( - "JWT authentication plugin not installed.", res["error"]["message"] + "jwt authentication plugin not installed", + res["error"]["message"].lower(), ) self.assertNotIn("token", res) From 75aa8126e7c22254165100234a76ab469df81e93 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Tue, 28 Dec 2021 11:27:15 -0800 Subject: [PATCH 09/16] fix(auth): Install to arbitrary ZODB OFS hierarchy Don't assume that all Plone portals will be installed directly into the Zope root or that all ancestors above the Plone portal will have `./acl_users`. I don't have a case for this, I just noticed it when I was reading the code while working on Zope root auth issues. Also clarify the PAS install plugin process with comments. --- src/plone/restapi/setuphandlers.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/plone/restapi/setuphandlers.py b/src/plone/restapi/setuphandlers.py index 7e0453ac13..1bc174d40c 100644 --- a/src/plone/restapi/setuphandlers.py +++ b/src/plone/restapi/setuphandlers.py @@ -33,8 +33,15 @@ def getNonInstallableProducts(self): # pragma: no cover def install_pas_plugin(context): uf_parent = aq_inner(context) while True: - uf = getToolByName(uf_parent, "acl_users") - if IPluggableAuthService.providedBy(uf) and "jwt_auth" not in uf: + uf = getToolByName(uf_parent, "acl_users", default=None) + + # Skip ancestor contexts to which we don't/can't apply + if uf is None or not IPluggableAuthService.providedBy(uf): + uf_parent = aq_parent(uf_parent) + continue + + # Add the API token plugin if not already installed at this level + if "jwt_auth" not in uf: plugin = JWTAuthenticationPlugin("jwt_auth") uf._setObject(plugin.getId(), plugin) plugin = uf["jwt_auth"] @@ -46,6 +53,8 @@ def install_pas_plugin(context): "ICredentialsResetPlugin", ], ) + + # Go up one more level if uf_parent is uf_parent.getPhysicalRoot(): break uf_parent = aq_parent(uf_parent) From 895eeed7900a3a75ae8c4c9591eee9768dfba3fa Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Tue, 28 Dec 2021 15:33:47 -0800 Subject: [PATCH 10/16] build(checkouts): Fix override base auto-checkout --- plone-5.2.x-performance.cfg | 2 +- plone-6.0.x.cfg | 2 +- site.cfg | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plone-5.2.x-performance.cfg b/plone-5.2.x-performance.cfg index 8285f1ddf0..02d3217951 100644 --- a/plone-5.2.x-performance.cfg +++ b/plone-5.2.x-performance.cfg @@ -1,7 +1,7 @@ [buildout] extends = plone-5.2.x.cfg parts += instance plonesite -auto-checkout = Products.ZCatalog +auto-checkout += Products.ZCatalog [instance] recipe = plone.recipe.zope2instance diff --git a/plone-6.0.x.cfg b/plone-6.0.x.cfg index 1360f78d25..dff3b5fc5f 100644 --- a/plone-6.0.x.cfg +++ b/plone-6.0.x.cfg @@ -5,7 +5,7 @@ extends = base.cfg find-links = https://dist.plone.org/release/6.0.0a3/ versions=versions -auto-checkout = +auto-checkout += Products.CMFPlone [sources] diff --git a/site.cfg b/site.cfg index eb81698e9d..0114638d9c 100644 --- a/site.cfg +++ b/site.cfg @@ -2,7 +2,7 @@ extensions = mr.developer extends = buildout.cfg eggs += plone.restapi -auto-checkout = plone.restapi +auto-checkout += plone.restapi parts = instance plonesite From 03dd65a84c87cf84afad9ede4d70694242030742 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Mon, 3 Jan 2022 16:26:35 -0800 Subject: [PATCH 11/16] fix(auth): Missing JWT plugin activation upgrade Because token generation has been moved into `updateCredentials(...)` [we need an upgrade step](https://github.com/plone/plone.restapi/pull/1303#issuecomment-1002756856) that enables the JWT token plugin for that PAS plugin interface on existing installations in order for authentication to work as before. Also fixes existing plugins outside of a Plone portal that have been configured to use the keyring. I tested this locally by: 1. erasing my local data (ZODB) 2. checking out `master` in the `plone/volto` repo 3. running buildout, including `plonesite` in the API to re-create the portal 4. adding a test user in the Plone portal through the Volto UI 5. add `mr.developer` sources and checkouts in the API buildout 6. disable `plonesite` in the API buildout 7. run buildout to update the code to the PR branches 8. test all the upgrade error conditions around login logout 9. run the `v0006 -> v0007` upgrade step for `plone.restapi:default` 10. confirm all the upgrade error conditions around login logout have been resolved Not that this doesn't address the issue of [existing Zope root `/acl_users/` cookie login set up](https://github.com/plone/plone.restapi/pull/1304#issuecomment-1002759581). --- src/plone/restapi/pas/__init__.py | 32 +++++++++++++++ .../restapi/profiles/default/metadata.xml | 2 +- src/plone/restapi/services/auth/login.py | 22 ++++++++++- src/plone/restapi/setuphandlers.py | 24 +++--------- src/plone/restapi/tests/test_addons.py | 11 ++++-- src/plone/restapi/upgrades/configure.zcml | 9 +++++ src/plone/restapi/upgrades/to0007.py | 39 +++++++++++++++++++ 7 files changed, 115 insertions(+), 24 deletions(-) create mode 100644 src/plone/restapi/upgrades/to0007.py diff --git a/src/plone/restapi/pas/__init__.py b/src/plone/restapi/pas/__init__.py index e69de29bb2..d761a9744b 100644 --- a/src/plone/restapi/pas/__init__.py +++ b/src/plone/restapi/pas/__init__.py @@ -0,0 +1,32 @@ +""" +A JWT token authentication plugin for PluggableAuthService. +""" + +from Products.CMFCore.utils import getToolByName +from Products.CMFPlone import interfaces as plone_ifaces +from Products import PluggableAuthService # noqa, Ensure PAS patch in place +from Products.PluggableAuthService.interfaces import authservice as authservice_ifaces + +import Acquisition + + +def iter_ancestor_pas(context): + """ + Walk up the ZODB OFS returning Pluggableauthservice `./acl_users/` for each level. + """ + uf_parent = Acquisition.aq_inner(context) + while True: + is_plone_site = plone_ifaces.IPloneSiteRoot.providedBy(uf_parent) + uf = getToolByName(uf_parent, "acl_users", default=None) + + # Skip ancestor contexts to which we don't/can't apply + if uf is None or not authservice_ifaces.IPluggableAuthService.providedBy(uf): + uf_parent = Acquisition.aq_parent(uf_parent) + continue + + yield uf, is_plone_site + + # Go up one more level + if uf_parent is uf_parent.getPhysicalRoot(): + break + uf_parent = Acquisition.aq_parent(uf_parent) diff --git a/src/plone/restapi/profiles/default/metadata.xml b/src/plone/restapi/profiles/default/metadata.xml index 81970e34ed..1e4872e498 100644 --- a/src/plone/restapi/profiles/default/metadata.xml +++ b/src/plone/restapi/profiles/default/metadata.xml @@ -1,4 +1,4 @@ - 0006 + 0007 diff --git a/src/plone/restapi/services/auth/login.py b/src/plone/restapi/services/auth/login.py index 21fa9fa6b5..e9014b8b3d 100644 --- a/src/plone/restapi/services/auth/login.py +++ b/src/plone/restapi/services/auth/login.py @@ -86,7 +86,27 @@ def reply(self): ) login_view._post_login() - return {"token": self.request[plugin.cookie_name]} + response = {} + if plugin.cookie_name in self.request: + response["token"] = self.request[plugin.cookie_name] + else: + self.request.response.setStatus(501) + message = ( + "JWT authentication token not created, plugin probably not activated " + "for `ICredentialsUpdatePlugin`" + ) + logger.error( + "%s: %s", + message, + "/".join(plugin.getPhysicalPath()), + ) + return dict( + error=dict( + type="Login failed", + message=message, + ) + ) + return response def _find_userfolder(self, userid): """Try to find a user folder that contains a user with the given diff --git a/src/plone/restapi/setuphandlers.py b/src/plone/restapi/setuphandlers.py index 1bc174d40c..a1a3948172 100644 --- a/src/plone/restapi/setuphandlers.py +++ b/src/plone/restapi/setuphandlers.py @@ -1,11 +1,6 @@ -from Acquisition import aq_inner -from Acquisition import aq_parent +from plone.restapi import pas from plone.restapi.pas.plugin import JWTAuthenticationPlugin -from Products.CMFCore.utils import getToolByName from Products.CMFPlone.interfaces import INonInstallable -from Products.PluggableAuthService.interfaces.authservice import ( - IPluggableAuthService, -) # noqa: E501 from zope.component.hooks import getSite from zope.interface import implementer @@ -31,14 +26,12 @@ def getNonInstallableProducts(self): # pragma: no cover def install_pas_plugin(context): - uf_parent = aq_inner(context) - while True: - uf = getToolByName(uf_parent, "acl_users", default=None) + """ + Install the JWT token PAS plugin in every PAS acl_users here and above. - # Skip ancestor contexts to which we don't/can't apply - if uf is None or not IPluggableAuthService.providedBy(uf): - uf_parent = aq_parent(uf_parent) - continue + Usually this means it is installed into Plone and into the Zope root. + """ + for uf, is_plone_site in pas.iter_ancestor_pas(context): # Add the API token plugin if not already installed at this level if "jwt_auth" not in uf: @@ -54,11 +47,6 @@ def install_pas_plugin(context): ], ) - # Go up one more level - if uf_parent is uf_parent.getPhysicalRoot(): - break - uf_parent = aq_parent(uf_parent) - def post_install_default(context): """Post install of default profile""" diff --git a/src/plone/restapi/tests/test_addons.py b/src/plone/restapi/tests/test_addons.py index 78ab4fa2e9..b19a58a74a 100644 --- a/src/plone/restapi/tests/test_addons.py +++ b/src/plone/restapi/tests/test_addons.py @@ -116,12 +116,15 @@ def _get_upgrade_info(self): # Set need upgrade state self.ps.setLastVersionForProfile("plone.restapi:default", "0002") transaction.commit() + # FIXME: At least the `newVersion` should be extracted from + # `./profiles/default/metadata.xml` so that this test isn't constantly changing + # for unrelated code changes. self.assertEqual( { "available": True, "hasProfile": True, "installedVersion": "0002", - "newVersion": "0006", + "newVersion": "0007", "required": True, }, _get_upgrade_info(self), @@ -135,8 +138,8 @@ def _get_upgrade_info(self): { "available": False, "hasProfile": True, - "installedVersion": "0006", - "newVersion": "0006", + "installedVersion": "0007", + "newVersion": "0007", "required": False, }, _get_upgrade_info(self), @@ -190,7 +193,7 @@ def _get_upgrade_info(self): "available": True, "hasProfile": True, "installedVersion": "0002", - "newVersion": "0006", + "newVersion": "0007", "required": True, }, _get_upgrade_info(self), diff --git a/src/plone/restapi/upgrades/configure.zcml b/src/plone/restapi/upgrades/configure.zcml index 66bb67b0d7..547421dbd6 100644 --- a/src/plone/restapi/upgrades/configure.zcml +++ b/src/plone/restapi/upgrades/configure.zcml @@ -69,4 +69,13 @@ handler="plone.restapi.upgrades.to0006.rename_iface_to_name_in_blocks_behavior" /> + + diff --git a/src/plone/restapi/upgrades/to0007.py b/src/plone/restapi/upgrades/to0007.py new file mode 100644 index 0000000000..c315766a5f --- /dev/null +++ b/src/plone/restapi/upgrades/to0007.py @@ -0,0 +1,39 @@ +""" +GenericSetup profile upgrades from version 0006 to 0007. +""" + +from plone.restapi import pas +from plone.restapi.pas import plugin +from Products.CMFCore.utils import getToolByName +from Products.PluggableAuthService.interfaces import plugins as plugins_ifaces + +import logging + +logger = logging.getLogger(__name__) + + +def enable_new_pas_plugin_interfaces(context): + """ + Enable new PAS plugin interfaces. + + After correcting/completing the PAS plugin interfaces, those interfaces need to be + enabled for existing functionality to continue working. + """ + portal = getToolByName(context, "portal_url").getPortalObject() + for uf, is_plone_site in pas.iter_ancestor_pas(portal): + for jwt_plugin in uf.objectValues(plugin.JWTAuthenticationPlugin.meta_type): + for new_iface in ( + plugins_ifaces.ICredentialsUpdatePlugin, + plugins_ifaces.ICredentialsResetPlugin, + ): + active_plugin_ids = [ + active_plugin_id + for active_plugin_id, _ in uf.plugins.listPlugins(new_iface) + ] + if jwt_plugin.id not in active_plugin_ids: + logger.info( + "Activating PAS interface %s: %s", + new_iface.__name__, + "/".join(jwt_plugin.getPhysicalPath()), + ) + uf.plugins.activatePlugin(new_iface, jwt_plugin.id) From 85670ea1b1a5eb1d3c9e3ff819ea4f018e868fa7 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Wed, 23 Feb 2022 13:46:22 -0800 Subject: [PATCH 12/16] fix(auth): Workaround broken zope root JWT config The existing Zope root JWT plugin configuration is broken. It is configured to use the `IKeyManager` utility but there is no such utility registered in that context. [An upgrade step](https://github.com/plone/plone.restapi/commit/f9097a0215166ea9336d234c11ef732a7d8fceea) has to be run to fix that. If the browser has previously logged into the Volto UI through the React login component, then the `auth_token` cookie will be set. If that browser is then used to access the ZMI, [the presence of the cookie triggers the token decode code path](https://github.com/plone/plone.restapi/commit/9422195e976a7f6f917491cb368607f836508f8c), which in turn causes the exception from the broken plugin configuration. Thus any browser logged into the Volto UI may not be able to access the ZMI in order to run the upgrade step that fixes the plugin configuration. Workaround this trap by logging a helpful error instead of causing an exception. --- src/plone/restapi/pas/plugin.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/plone/restapi/pas/plugin.py b/src/plone/restapi/pas/plugin.py index a60f672122..5fd961faef 100644 --- a/src/plone/restapi/pas/plugin.py +++ b/src/plone/restapi/pas/plugin.py @@ -17,12 +17,15 @@ from Products.PluggableAuthService.interfaces.plugins import ICredentialsUpdatePlugin from Products.PluggableAuthService.interfaces.plugins import ICredentialsResetPlugin from Products.PluggableAuthService.plugins.BasePlugin import BasePlugin +from zope import component from zope.component import getUtility from zope.interface import implementer import jwt +import logging import time +logger = logging.getLogger(__name__) manage_addJWTAuthenticationPlugin = PageTemplateFile( "add_plugin", globals(), __name__="manage_addJWTAuthenticationPlugin" @@ -211,7 +214,15 @@ def manage_updateConfig(self, REQUEST): def _decode_token(self, token, verify=True): if self.use_keyring: - manager = getUtility(IKeyManager) + manager = component.queryUtility(IKeyManager) + if manager is None: + logger.error( + "JWT token plugin configured to use IKeyManager " + "but no utility is registered: %r\n" + "Have you upgraded the `plone.restapi:default` profile?", + "/".join(self.getPhysicalPath()), + ) + return for secret in manager["_system"]: if secret is None: continue From fdc03e0ef423c8cacc8303885da0ad4c31cd475b Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 3 Feb 2022 15:30:38 -0800 Subject: [PATCH 13/16] build(versions): Cleanup unused buildout config Chanced across this while updating the `Products.PluggableAuthService` version. The `./versions.cfg` file isn't referenced anywhere that I can see. A `./versions.cfg` file is a convention in the Plone world so it's misleading to have an unused one lying around. A developer might reasonable assume that a change in that file should take effect. --- Makefile | 1 - versions.cfg | 28 ---------------------------- 2 files changed, 29 deletions(-) delete mode 100644 versions.cfg diff --git a/Makefile b/Makefile index 0c40a7b17b..e570748c6d 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,6 @@ update: ## Update Make and Buildout wget -O requirements.txt https://raw.githubusercontent.com/kitconcept/buildout/5.2/requirements.txt wget -O plone-5.2.x.cfg https://raw.githubusercontent.com/kitconcept/buildout/5.2/plone-5.2.x.cfg wget -O ci.cfg https://raw.githubusercontent.com/kitconcept/buildout/5.2/ci.cfg - wget -O versions.cfg https://raw.githubusercontent.com/kitconcept/buildout/5.2/versions.cfg .installed.cfg: bin/buildout *.cfg bin/buildout diff --git a/versions.cfg b/versions.cfg deleted file mode 100644 index 28a57f5575..0000000000 --- a/versions.cfg +++ /dev/null @@ -1,28 +0,0 @@ -[versions] -# Buildout -setuptools = -zc.buildout = -plone.restapi = - -# code analysis -black = 20.8b1 - -# Error: The requirement ('virtualenv>=20.0.35') is not allowed by your [versions] constraint (20.0.26) -virtualenv = 20.0.35 - -# Error: The requirement ('pep517>=0.9') is not allowed by your [versions] constraint (0.8.2) -pep517 = 0.9.1 - -# Error: The requirement ('importlib-metadata>=1') is not allowed by your [versions] constraint (0.23) -importlib-metadata = 2.0.0 - -# cryptography 3.4 requires a rust compiler installed on the system: -# https://github.com/pyca/cryptography/blob/master/CHANGELOG.rst#34---2021-02-07 -cryptography = 3.3.2 - -# cffi 1.14.3 fails on apple m1 -cffi = 1.14.4 - -# requirement for json widget tests to pass -plone.schema = 1.3.0 -plone.dexterity = 2.9.8 \ No newline at end of file From ea0af6834468d48a7c1f45f67262f3c2c6f729de Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Mon, 27 Dec 2021 22:45:22 -0800 Subject: [PATCH 14/16] fix(auth): Fix broken PAS plugin config at root Outside of the context of a Plone site, there usually isn't a `plone.keyring.interfaces.IKeyManager` but the GenericSetup "various" import step that adds the JWT token plugin to the Zope root `/acl_users` leaves the default keyring plugin setting which results in the following when authenticating to the Zope root: 2021-12-27 11:25:39,451 ERROR [Zope.SiteErrorLog:22][waitress-3] ComponentLookupError: http://localhost:49080/api/acl_users/credentials_cookie_auth/login Traceback (innermost last): Module ZPublisher.WSGIPublisher, line 162, in transaction_pubevents Module ZPublisher.WSGIPublisher, line 372, in publish_module Module ZPublisher.WSGIPublisher, line 266, in publish Module ZPublisher.mapply, line 85, in mapply Module ZPublisher.WSGIPublisher, line 63, in call_object Module Products.PluggableAuthService.plugins.CookieAuthHelper, line 279, in login Module Products.PluggableAuthService.PluggableAuthService, line 1153, in updateCredentials Module plone.restapi.pas.plugin, line 165, in updateCredentials Module plone.restapi.pas.plugin, line 260, in create_payload_token Module plone.restapi.pas.plugin, line 230, in _signing_secret Module zope.component._api, line 165, in getUtility zope.interface.interfaces.ComponentLookupError: (, '') Fix this by doing an interface for the Plone portal and changing that configuration setting if not being installed into a Plone portal. --- src/plone/restapi/setuphandlers.py | 2 ++ src/plone/restapi/upgrades/to0007.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/plone/restapi/setuphandlers.py b/src/plone/restapi/setuphandlers.py index a1a3948172..7b01de357f 100644 --- a/src/plone/restapi/setuphandlers.py +++ b/src/plone/restapi/setuphandlers.py @@ -46,6 +46,8 @@ def install_pas_plugin(context): "ICredentialsResetPlugin", ], ) + if not is_plone_site: + plugin.use_keyring = False def post_install_default(context): diff --git a/src/plone/restapi/upgrades/to0007.py b/src/plone/restapi/upgrades/to0007.py index c315766a5f..0c3af18424 100644 --- a/src/plone/restapi/upgrades/to0007.py +++ b/src/plone/restapi/upgrades/to0007.py @@ -22,6 +22,12 @@ def enable_new_pas_plugin_interfaces(context): portal = getToolByName(context, "portal_url").getPortalObject() for uf, is_plone_site in pas.iter_ancestor_pas(portal): for jwt_plugin in uf.objectValues(plugin.JWTAuthenticationPlugin.meta_type): + if not is_plone_site and jwt_plugin.use_keyring: + logger.info( + "Disabling keyring for plugin outside of Plone: %s", + "/".join(jwt_plugin.getPhysicalPath()), + ) + jwt_plugin.use_keyring = False for new_iface in ( plugins_ifaces.ICredentialsUpdatePlugin, plugins_ifaces.ICredentialsResetPlugin, From 2afb7a4959a85936f6d79165656c665a064bc72c Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Tue, 28 Dec 2021 12:09:01 -0800 Subject: [PATCH 15/16] test(auth): Zope root cookie login form auth Integrate upstream `Products.PluggableAuthService` and `Products.PlonePAS` changes to the Zope root `/acl_users`: - fix some broken plugin configuration on install and upgrade - add a `GenericSetup` profile to convert to a simple cookie login form which fixes logout issues when used with other plugins such as the JWT token plugin Note that the only changes here are adding test coverage confirming the upstream changes have the intended effect when combined with the JWT token plugin. Upstream links: https://github.com/zopefoundation/Products.PluggableAuthService/issues/107#issue-1090137890 https://github.com/zopefoundation/Products.PluggableAuthService/commit/44ac67fa0593fc8daf851feaf648d920fd41eaeb --- base.cfg | 4 + .../restapi/tests/test_functional_auth.py | 115 ++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/base.cfg b/base.cfg index 8f22ca3720..a6ff8a37d3 100644 --- a/base.cfg +++ b/base.cfg @@ -20,6 +20,7 @@ parts = develop = . sources-dir = extras auto-checkout = + Products.PlonePAS # plone.rest allow-hosts = @@ -35,6 +36,8 @@ allow-hosts = [versions] # Do not use a release of plone.restapi: plone.restapi = +# Fix Zope root `/acl_users` logout +Products.PluggableAuthService = 2.7.0 [instance] recipe = plone.recipe.zope2instance @@ -204,6 +207,7 @@ output = ${buildout:directory}/bin/zpretty-run mode = 755 [sources] +Products.PlonePAS = git https://github.com/plone/Products.PlonePAS.git pushurl=git@github.com:plone/Products.PlonePAS.git branch=feat-zope-root-cookie-challenge plone.rest = git git://github.com/plone/plone.rest.git pushurl=git@github.com:plone/plone.rest.git branch=master plone.schema = git git://github.com/plone/plone.schema.git pushurl=git@github.com:plone/plone.schema.git branch=master Products.ZCatalog = git git://github.com/zopefoundation/Products.ZCatalog.git pushurl=git@github.com:zopefoundation/Products.ZCatalog.git \ No newline at end of file diff --git a/src/plone/restapi/tests/test_functional_auth.py b/src/plone/restapi/tests/test_functional_auth.py index 01e5e45fb4..2c1b87c823 100644 --- a/src/plone/restapi/tests/test_functional_auth.py +++ b/src/plone/restapi/tests/test_functional_auth.py @@ -1,3 +1,4 @@ +from plone.app import testing as pa_testing from plone.app.testing import login from plone.app.testing import setRoles from plone.app.testing import SITE_OWNER_NAME @@ -5,6 +6,9 @@ from plone.app.testing import TEST_USER_NAME from plone.app.testing import TEST_USER_PASSWORD from plone.restapi.testing import PLONE_RESTAPI_DX_FUNCTIONAL_TESTING +from plone.testing import zope +from Products.PluggableAuthService.interfaces import plugins as plugins_ifaces +from Products.PluggableAuthService.plugins import CookieAuthHelper import base64 import requests @@ -17,14 +21,28 @@ class TestFunctionalAuth(unittest.TestCase): layer = PLONE_RESTAPI_DX_FUNCTIONAL_TESTING def setUp(self): + """ + Set initial test conditions and convenience attributes. + """ + # Zope root references + self.app = self.layer["app"] + self.root_acl_users = self.app.acl_users + + # Plone portal references self.portal = self.layer["portal"] self.portal_url = self.portal.absolute_url() + + # User permissions and authentication setRoles(self.portal, TEST_USER_ID, ["Manager"]) login(self.portal, SITE_OWNER_NAME) + + # Create a page that can't be publicly accessed self.private_document = self.portal[ self.portal.invokeFactory("Document", id="doc1", title="My Document") ] self.private_document_url = self.private_document.absolute_url() + + # This is a functional fixture, have to commit our changes transaction.commit() def test_login_without_credentials_fails(self): @@ -172,6 +190,66 @@ def test_api_login_grants_zmi(self): "Wrong ZMI view response content", ) + def test_root_cookie_login_sets_api_token(self): + """ + Zope root ZMI uses cookie login form and also sets the API token cookie. + """ + # Install the GenericSetup profile that performs the actual switch + pa_testing.applyProfile(self.portal, "Products.PlonePAS:root-cookie") + transaction.commit() + + # Check the Zope root PAS plugin configuration + self.assertIn( + "credentials_cookie_auth", + self.root_acl_users.objectIds(), + "Cookie auth plugin missing from Zope root `/acl_users`", + ) + cookie_plugin = self.root_acl_users.credentials_cookie_auth + self.assertIs( + type(cookie_plugin.aq_base), + CookieAuthHelper.CookieAuthHelper, + "Wrong Zope root `/acl_users` cookie auth plugin type", + ) + challenge_plugins = self.root_acl_users.plugins.listPlugins( + plugins_ifaces.IChallengePlugin, + ) + _, default_challenge_plugin = challenge_plugins[0] + self.assertEqual( + "/".join(default_challenge_plugin.getPhysicalPath()), + "/".join(cookie_plugin.getPhysicalPath()), + "Wrong Zope root `/acl_users` default challenge plugin", + ) + + # Submit the login form in the browser + browser = zope.Browser(self.app) + browser.open(self.app.absolute_url() + "/manage_main") + login_form = browser.getForm() + login_form.getControl(name="__ac_name").value = SITE_OWNER_NAME + login_form.getControl(name="__ac_password").value = TEST_USER_PASSWORD + login_form.controls[-1].click() + self.assertEqual( + browser.headers["Status"].lower(), + "200 ok", + "Wrong Zope root `/acl_users` cookie login response status", + ) + self.assertEqual( + browser.url, + self.app.absolute_url() + "/manage_main", + "Wrong Zope root `/acl_users` cookie login redirect", + ) + + # Both the Zope root login cookie and the API token cookie are set + self.assertIn( + "__ac", + browser.cookies, + "Zope root auth cookie missing from Zope root basic login form response", + ) + self.assertIn( + "auth_token", + browser.cookies, + "API token cookie missing from Zope root basic login form response", + ) + def test_root_zmi_login_grants_api(self): """ Logging in via the Zope root ZMI also grants access to the API. @@ -291,6 +369,43 @@ def test_api_logout_expires_both_cookies(self): "API token cookie remains after API logout POST response", ) + def test_root_zmi_logout_expires_api_token(self): + """ + Zope root ZMI logout succeeds and deletes all auth cookies. + """ + # Install the GenericSetup profile that performs the actual switch + pa_testing.applyProfile(self.portal, "Products.PlonePAS:root-cookie") + transaction.commit() + + # Submit the login form in the browser + browser = zope.Browser(self.app) + browser.open(self.app.absolute_url() + "/manage_main") + login_form = browser.getForm() + login_form.getControl(name="__ac_name").value = SITE_OWNER_NAME + login_form.getControl(name="__ac_password").value = TEST_USER_PASSWORD + login_form.controls[-1].click() + + # Click the ZMI logout link + browser.raiseHttpErrors = False + logout_link = browser.getLink(url="manage_zmi_logout") + logout_link.click() + browser.raiseHttpErrors = True + self.assertNotIn( + "__ac", + browser.cookies, + "Zope root auth cookie missing from Zope root basic login form response", + ) + self.assertNotIn( + "auth_token", + browser.cookies, + "API token cookie missing from Zope root basic login form response", + ) + + # Confirm the browser is, in fact, logged out + browser.open(self.app.absolute_url() + "/manage_main") + logout_login_form = browser.getForm() + logout_login_form.getControl(name="__ac_name") + def test_accessing_private_document_with_valid_token_succeeds(self): # login and generate a valid token response = requests.post( From 054fe37eaee5b2cb8ca00f622a4b6a2b3df21bf4 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Wed, 9 Mar 2022 09:54:26 -0800 Subject: [PATCH 16/16] build(checkouts): PlonePAS PR merged and released https://github.com/plone/Products.PlonePAS/pull/66 --- base.cfg | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/base.cfg b/base.cfg index a6ff8a37d3..c54d23ed9b 100644 --- a/base.cfg +++ b/base.cfg @@ -20,7 +20,6 @@ parts = develop = . sources-dir = extras auto-checkout = - Products.PlonePAS # plone.rest allow-hosts = @@ -37,7 +36,8 @@ allow-hosts = # Do not use a release of plone.restapi: plone.restapi = # Fix Zope root `/acl_users` logout -Products.PluggableAuthService = 2.7.0 +Products.PluggableAuthService = >=2.7.0 +Products.PlonePAS = >=7.0.0a3 [instance] recipe = plone.recipe.zope2instance @@ -207,7 +207,6 @@ output = ${buildout:directory}/bin/zpretty-run mode = 755 [sources] -Products.PlonePAS = git https://github.com/plone/Products.PlonePAS.git pushurl=git@github.com:plone/Products.PlonePAS.git branch=feat-zope-root-cookie-challenge plone.rest = git git://github.com/plone/plone.rest.git pushurl=git@github.com:plone/plone.rest.git branch=master plone.schema = git git://github.com/plone/plone.schema.git pushurl=git@github.com:plone/plone.schema.git branch=master Products.ZCatalog = git git://github.com/zopefoundation/Products.ZCatalog.git pushurl=git@github.com:zopefoundation/Products.ZCatalog.git \ No newline at end of file