Merge pull request #66 from plone/feat-zope-root-cookie-challenge

feat(setup): Zope root cookie login form profile

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

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)

pyproject.toml

@@ -1,3 +1,8 @@
+[tool.pylint.'MESSAGES CONTROL']
+disable = [
+    "wrong-import-order",
+]
+
 [tool.towncrier]
 filename = "CHANGES.rst"
 directory = "news/"

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

src/Products/PlonePAS/configure.zcml

@@ -2,25 +2,13 @@
     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">
 
   <include package=".browser" />
   <include package=".tools" />
   <include file="profiles.zcml" />
   <include file="exportimport.zcml" />
 
-  <genericsetup:importStep
-      description="Configure PlonePas"
-      handler="Products.PlonePAS.setuphandlers.setupPlonePAS"
-      name="plonepas"
-      title="PlonePAS setup">
-    <depends name="componentregistry" />
-    <depends name="controlpanel" />
-    <depends name="memberdata-properties" />
-    <depends name="rolemap" />
-  </genericsetup:importStep>
-
   <five:deprecatedManageAddDelete class=".plugins.cookie_handler.ExtendedCookieAuthHelper" />
   <five:deprecatedManageAddDelete class=".plugins.role.GroupAwareRoleManager" />
 

src/Products/PlonePAS/interfaces/memberdata.py

@@ -7,4 +7,4 @@ class IMemberDataTool(interfaces.IMemberDataTool):
     """
 
 
-__all__ = (IMemberDataTool,)
+__all__ = ("IMemberDataTool", )

src/Products/PlonePAS/interfaces/membership.py

@@ -9,4 +9,4 @@ def getMemberInfo(memberId=None):
         location, etc
         """
 
-__all__ = (IMembershipTool, )
+__all__ = ("IMembershipTool", )

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):

src/Products/PlonePAS/profiles.zcml

@@ -10,5 +10,29 @@
       description="Extension profile for default PlonePAS setup."
       provides="Products.GenericSetup.interfaces.EXTENSION"
       />
+  <genericsetup:importStep
+      description="Configure PlonePas"
+      handler="Products.PlonePAS.setuphandlers.setupPlonePAS"
+      name="plonepas"
+      title="PlonePAS setup">
+    <depends name="componentregistry" />
+    <depends name="controlpanel" />
+    <depends name="memberdata-properties" />
+    <depends name="rolemap" />
+  </genericsetup:importStep>
+  <genericsetup:upgradeStep
+      title="Fix existing broken Zope root `/acl_users` plugins"
+      profile="Products.PlonePAS:PlonePAS"
+      source="4"
+      destination="5"
+      handler=".upgrades.from4to5_fix_zope_root" />
+
+  <genericsetup:registerProfile
+      name="root-cookie"
+      title="Zope Root Cookie Login"
+      description="Change the Zope root `/acl_users` to use a simple cookie login form
+		   instead of HTTP `Basic ...` for authentication."
+      provides="Products.GenericSetup.interfaces.EXTENSION"
+      post_handler=".setuphandlers.set_up_zope_root_cookie_auth" />
 
 </configure>

src/Products/PlonePAS/profiles/default/metadata.xml

@@ -1,4 +1,4 @@
 <?xml version="1.0"?>
 <metadata>
-  <version>4</version>
+  <version>5</version>
 </metadata>

src/Products/PlonePAS/profiles/root-cookie/metadata.xml

@@ -0,0 +1,4 @@
+<?xml version="1.0"?>
+<metadata>
+  <version>1</version>
+</metadata>

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.

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",
+            )

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',

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(),

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):

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"],
+            )