diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 00000000..cf6d4202 --- /dev/null +++ b/mypy.ini @@ -0,0 +1,45 @@ +# Couldn't get `foo.*` module wildcards to work in `./pyproject.toml` +[mypy-ordereddict] +ignore_missing_imports = True +[mypy-transaction] +ignore_missing_imports = True +[mypy-zope.*] +ignore_missing_imports = True +[mypy-ZODB.*] +ignore_missing_imports = True +[mypy-BTrees.*] +ignore_missing_imports = True +[mypy-zExceptions.*] +ignore_missing_imports = True +[mypy-AccessControl.*] +ignore_missing_imports = True +[mypy-Acquisition.*] +ignore_missing_imports = True +[mypy-OFS.*] +ignore_missing_imports = TRUE +[mypy-ZPublisher.*] +ignore_missing_imports = True +[mypy-App.*] +ignore_missing_imports = True +[mypy-AuthEncoding.*] +ignore_missing_imports = True +[mypy-DateTime.*] +ignore_missing_imports = True +[mypy-Products.*] +ignore_missing_imports = True +[mypy-plone.*] +ignore_missing_imports = True + +# For some reason installing `types-*` didn't work +[mypy-six.*] +ignore_missing_imports = True +[mypy-past.*] +ignore_missing_imports = True +[mypy-PIL.*] +ignore_missing_imports = True + +# Zope interfaces break MyPy expectations and unfortunately I couldn't find a way to +# ignore only the specific error MyPy reports: +# `./src/Products/PlonePAS/interfaces/capabilities.py:57: error: Method must have at least one argument` +[mypy-*.interfaces.*] +ignore_errors = True \ No newline at end of file diff --git a/news/zope-root-cookie.feature b/news/zope-root-cookie.feature new file mode 100644 index 00000000..b2d8f892 --- /dev/null +++ b/news/zope-root-cookie.feature @@ -0,0 +1,4 @@ +Add separate `GenericSetup` profile to switch the Zope root `/acl_users` to use a simple +cookie login form. Useful when Zope root login and logout need to synchronize +authentication state between multiple plugins, which is not possible with HTTP `Basic +...` authentication. [rpatterson] (#65) diff --git a/pyproject.toml b/pyproject.toml index 05b615de..c4f4af63 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,8 @@ +[tool.pylint.'MESSAGES CONTROL'] +disable = [ + "wrong-import-order", +] + [tool.towncrier] filename = "CHANGES.rst" directory = "news/" diff --git a/setup.cfg b/setup.cfg index 06aa277c..cdfdbc2c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,3 +4,11 @@ create-wheel = yes # When Python 2-3 compatible: [bdist_wheel] universal = 1 + +[flake8] +# Match Black's defaults +# https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#flake8 +max-line-length = 88 +extend-ignore = E203 +aggressive = 3 +experimental = true diff --git a/src/Products/PlonePAS/configure.zcml b/src/Products/PlonePAS/configure.zcml index d7c003ae..a6a123e4 100644 --- a/src/Products/PlonePAS/configure.zcml +++ b/src/Products/PlonePAS/configure.zcml @@ -2,7 +2,6 @@ i18n_domain="plone" xmlns="http://namespaces.zope.org/zope" xmlns:five="http://namespaces.zope.org/five" - xmlns:genericsetup="http://namespaces.zope.org/genericsetup" xmlns:i18n="http://namespaces.zope.org/i18n"> @@ -10,17 +9,6 @@ - - - - - - - diff --git a/src/Products/PlonePAS/interfaces/memberdata.py b/src/Products/PlonePAS/interfaces/memberdata.py index 024c73cc..2eca85fc 100644 --- a/src/Products/PlonePAS/interfaces/memberdata.py +++ b/src/Products/PlonePAS/interfaces/memberdata.py @@ -7,4 +7,4 @@ class IMemberDataTool(interfaces.IMemberDataTool): """ -__all__ = (IMemberDataTool,) +__all__ = ("IMemberDataTool", ) diff --git a/src/Products/PlonePAS/interfaces/membership.py b/src/Products/PlonePAS/interfaces/membership.py index 1f8a757a..e36e5265 100644 --- a/src/Products/PlonePAS/interfaces/membership.py +++ b/src/Products/PlonePAS/interfaces/membership.py @@ -9,4 +9,4 @@ def getMemberInfo(memberId=None): location, etc """ -__all__ = (IMembershipTool, ) +__all__ = ("IMembershipTool", ) diff --git a/src/Products/PlonePAS/patch.py b/src/Products/PlonePAS/patch.py index c5f24c37..3eaea2fe 100644 --- a/src/Products/PlonePAS/patch.py +++ b/src/Products/PlonePAS/patch.py @@ -15,7 +15,7 @@ def call(self, __name__, *args, **kw): ADDED = '__PlonePAS_is_added_method__' ORIG_NAME = '__PlonePAS_original_method_name__' -_marker = dict() +_marker = dict() # type: ignore def isWrapperMethod(meth): diff --git a/src/Products/PlonePAS/profiles.zcml b/src/Products/PlonePAS/profiles.zcml index 84103954..a8993a9f 100644 --- a/src/Products/PlonePAS/profiles.zcml +++ b/src/Products/PlonePAS/profiles.zcml @@ -10,5 +10,29 @@ description="Extension profile for default PlonePAS setup." provides="Products.GenericSetup.interfaces.EXTENSION" /> + + + + + + + + + diff --git a/src/Products/PlonePAS/profiles/default/metadata.xml b/src/Products/PlonePAS/profiles/default/metadata.xml index dab4dc0c..1062ac87 100644 --- a/src/Products/PlonePAS/profiles/default/metadata.xml +++ b/src/Products/PlonePAS/profiles/default/metadata.xml @@ -1,4 +1,4 @@ - 4 + 5 diff --git a/src/Products/PlonePAS/profiles/root-cookie/metadata.xml b/src/Products/PlonePAS/profiles/root-cookie/metadata.xml new file mode 100644 index 00000000..cf4492ac --- /dev/null +++ b/src/Products/PlonePAS/profiles/root-cookie/metadata.xml @@ -0,0 +1,4 @@ + + + 1 + diff --git a/src/Products/PlonePAS/profiles/root-cookie/plone-pas-zope-root-cookie.txt b/src/Products/PlonePAS/profiles/root-cookie/plone-pas-zope-root-cookie.txt new file mode 100644 index 00000000..a0844b6c --- /dev/null +++ b/src/Products/PlonePAS/profiles/root-cookie/plone-pas-zope-root-cookie.txt @@ -0,0 +1,2 @@ +Change the Zope root `/acl_users` to use a simple cookie login form instead of HTTP +`Basic ...` for authentication. diff --git a/src/Products/PlonePAS/setuphandlers.py b/src/Products/PlonePAS/setuphandlers.py index 084ca86b..94586119 100644 --- a/src/Products/PlonePAS/setuphandlers.py +++ b/src/Products/PlonePAS/setuphandlers.py @@ -1,4 +1,8 @@ # -*- coding: utf-8 -*- +""" +Custom GenericSetup import steps for PAS in Plone. +""" + from Acquisition import aq_base from Acquisition import aq_parent from Products.CMFCore.utils import getToolByName @@ -513,3 +517,39 @@ def setupPlonePAS(context): addRolesToPlugIn(site) setupGroups(site) setLoginFormInCookieAuth(site) + + +def set_up_zope_root_cookie_auth(context): + """ + Change the Zope root `/acl_users` to use a simple cookie login form. + """ + portal = aq_parent(context) + root = portal.getPhysicalRoot() + root_acl_users = getToolByName(root, "acl_users") + + # Enable the cookie plugin for all interfaces + activatePluginInterfaces(root, "credentials_cookie_auth") + # Ensure that the cookie login form is used to challenge for authentication + credentials_cookie_auth = root_acl_users._getOb( # pylint: disable=protected-access + "credentials_cookie_auth", + ) + root_acl_users.plugins.movePluginsTop( + IChallengePlugin, + [credentials_cookie_auth.id], + ) + # Disable the HTTP `Basic ...` authentication plugin + for plugin_iface in ( + IChallengePlugin, + # Apparently, the `HTTPBasicAuthHelper` plugin"s `ICredentialsResetPlugin` + # implementation interferes with deleting/expiring cookies, specifically the + # `__ac` cookie in this case. I first tried moving that plugin to the top and + # bottom for that interface, but the cookies still remained after logout. Only + # deactivating it worked. + ICredentialsResetPlugin, + ): + activated_plugin_ids = root_acl_users.plugins.listPluginIds(plugin_iface) + if "credentials_basic_auth" in activated_plugin_ids: + root_acl_users.plugins.deactivatePlugin( + plugin_iface, + "credentials_basic_auth", + ) diff --git a/src/Products/PlonePAS/sheet.py b/src/Products/PlonePAS/sheet.py index 7a83c7e2..281dca1b 100644 --- a/src/Products/PlonePAS/sheet.py +++ b/src/Products/PlonePAS/sheet.py @@ -42,6 +42,7 @@ def validate(self, property_type, value): inspector = self.tmap[property_type] return inspector(value) + PropertySchema = PropertySchemaTypeMap() PropertySchema.addType( 'string', diff --git a/src/Products/PlonePAS/tests/test_setup.py b/src/Products/PlonePAS/tests/test_setup.py index 473f5106..ae851272 100644 --- a/src/Products/PlonePAS/tests/test_setup.py +++ b/src/Products/PlonePAS/tests/test_setup.py @@ -1,4 +1,8 @@ # -*- coding: utf-8 -*- +""" +Test set up specific to Plone through thea GenericSetup profile installation. +""" + from plone.app import testing as pa_testing from plone.testing import zope from zope.component import hooks @@ -13,6 +17,9 @@ class PortalSetupTest(unittest.TestCase): + """ + Test set up specific to Plone through thea GenericSetup profile installation. + """ layer = testing.PRODUCTS_PLONEPAS_FUNCTIONAL_TESTING @@ -22,6 +29,7 @@ def setUp(self): """ self.app = self.layer["app"] self.root_acl_users = self.app.acl_users + self.portal = self.layer["portal"] def test_zope_root_default_challenge(self): """ @@ -63,6 +71,10 @@ def test_zope_root_cookie_login(self): """ The Zope root `/acl_users` cookie login works. """ + # Install the GenericSetup profile that performs the actual switch + pa_testing.applyProfile(self.portal, 'Products.PlonePAS:root-cookie') + transaction.commit() + # Make the cookie plugin the default auth challenge self.assertIn( "credentials_cookie_auth", @@ -75,15 +87,6 @@ def test_zope_root_cookie_login(self): CookieAuthHelper.CookieAuthHelper, "Wrong Zope root `/acl_users` cookie auth plugin type", ) - self.root_acl_users.plugins.activatePlugin( - plugins_ifaces.IChallengePlugin, - cookie_plugin.id, - ) - self.root_acl_users.plugins.movePluginsTop( - plugins_ifaces.IChallengePlugin, - [cookie_plugin.id], - ) - transaction.commit() challenge_plugins = self.root_acl_users.plugins.listPlugins( plugins_ifaces.IChallengePlugin, ) @@ -120,7 +123,9 @@ def test_zope_root_cookie_login(self): # Submit the login form in the browser login_form = browser.getForm() login_form.getControl(name="__ac_name").value = pa_testing.SITE_OWNER_NAME - login_form.getControl(name="__ac_password").value = pa_testing.TEST_USER_PASSWORD + login_form.getControl( + name="__ac_password" + ).value = pa_testing.TEST_USER_PASSWORD login_form.controls[-1].click() self.assertEqual( browser.headers["Status"].lower(), diff --git a/src/Products/PlonePAS/tools/membership.py b/src/Products/PlonePAS/tools/membership.py index 857a2e00..29c103a3 100644 --- a/src/Products/PlonePAS/tools/membership.py +++ b/src/Products/PlonePAS/tools/membership.py @@ -45,7 +45,7 @@ default_portrait = 'defaultUser.png' logger = logging.getLogger('PlonePAS') -_marker = dict() +_marker = dict() # type: ignore def _unicodify_structure(value, charset=_marker): diff --git a/src/Products/PlonePAS/upgrades.py b/src/Products/PlonePAS/upgrades.py new file mode 100644 index 00000000..d9353256 --- /dev/null +++ b/src/Products/PlonePAS/upgrades.py @@ -0,0 +1,59 @@ +""" +Upgrade steps specific to Plone's use of PAS. +""" + +from Products.PlonePAS.plugins import cookie_handler +from Products.PluggableAuthService.plugins import CookieAuthHelper + +import logging + +logger = logging.getLogger(__name__) + + +def from4to5_fix_zope_root(context): + """ + Fix broken Zope root `/acl_users/` plugins. + """ + root = context.getPhysicalRoot() + pas = root.acl_users.manage_addProduct['PluggableAuthService'] + # Identify which interfaces should be considered PAS plugin interfaces + plugin_ifaces = [ + plugin_type_info["interface"] + for plugin_type_info in root.acl_users.plugins.listPluginTypeInfo() + ] + broken_meta_type = cookie_handler.ExtendedCookieAuthHelper.meta_type + broken_plugins = root.acl_users.objectValues(broken_meta_type) + for broken_plugin in broken_plugins: + # Collect properties from old/broken plugin + kwargs = dict( + id=broken_plugin.id, + title=broken_plugin.title, + cookie_name=broken_plugin.cookie_name, + ) + # Which PAS plugin interfaces has this plugin been activated for + active_ifaces = [ + plugin_iface + for plugin_iface in plugin_ifaces + if plugin_iface.providedBy(broken_plugin) + and broken_plugin.id in root.acl_users.plugins.listPluginIds(plugin_iface) + ] + # Delete the old/broken plugin + logger.info( + "Deleting broken %r plugin: %r", + broken_meta_type, + "/".join(broken_plugin.getPhysicalPath()), + ) + root.acl_users.manage_delObjects([broken_plugin.id]) + # Add the correct plugin + logger.info( + "Adding working %r plugin: %r", + CookieAuthHelper.CookieAuthHelper.meta_type, + "/".join(broken_plugin.getPhysicalPath()), + ) + pas.addCookieAuthHelper(**kwargs) + # Restore activated plugin interfaces + for plugin_iface in active_ifaces: + root.acl_users.plugins.activatePlugin( + plugin_iface, + kwargs["id"], + )