From 7e6d9d5999621e8988ae38afcef914d3f077b38a Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Mon, 27 Dec 2021 18:15:37 -0800 Subject: [PATCH 1/7] feat(setup): Zope root Basic -> cookie login form Improve the Zope root ZMI login UX, avoid all the HTTP `Authorization: Basic ...` edge cases and hassles. Switch the default authentication challenge for the Zope root `/acl_users` from HTTP `Authorization: Basic ...` to the cookie auth plugins basic login form. This should be a much better UX overall and shouldn't cause any fundamental issues. One can still use HTTP `Authorization: Basic ...` manually by adding credentials to the URL: http://admin:secret@localhost:8080/manage_main But may cause issues where tests expect the HTTP `Authorization: Basic ...` challenge response or existing uses where new Zope instances are created as a part of normal use (SAAS?). We could also consider adding an upgrade step to make this change to existing installations but that would be disruptive to any existing installations that require HTTP `Authorization: Basic ...`. I can't imagine why that would be, but we should probably expect those use cases to come out of the woodwork once an upgrade step is released. --- src/Products/PlonePAS/setuphandlers.py | 11 ++++++--- src/Products/PlonePAS/tests/test_setup.py | 28 ++++++++++------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/Products/PlonePAS/setuphandlers.py b/src/Products/PlonePAS/setuphandlers.py index 084ca86b..daabb2f7 100644 --- a/src/Products/PlonePAS/setuphandlers.py +++ b/src/Products/PlonePAS/setuphandlers.py @@ -324,9 +324,14 @@ def migrate_root_uf(self): pas = uf.manage_addProduct['PluggableAuthService'] plone_pas = uf.manage_addProduct['PlonePAS'] # Setup authentication plugins - setupAuthPlugins(parent, pas, plone_pas, - deactivate_basic_reset=False, - deactivate_cookie_challenge=True) + setupAuthPlugins( + parent, + pas, + plone_pas, + deactivate_basic_reset=False, + # Switch from HTTP `Authorization: Basic ...` to cookie login form + deactivate_cookie_challenge=False, + ) # Activate *all* interfaces for user manager. IUserAdder is not # activated for some reason by default. diff --git a/src/Products/PlonePAS/tests/test_setup.py b/src/Products/PlonePAS/tests/test_setup.py index 473f5106..ca502dca 100644 --- a/src/Products/PlonePAS/tests/test_setup.py +++ b/src/Products/PlonePAS/tests/test_setup.py @@ -23,11 +23,11 @@ def setUp(self): self.app = self.layer["app"] self.root_acl_users = self.app.acl_users - def test_zope_root_default_challenge(self): + def test_zope_root_basic_challenge(self): """ - The Zope root `/acl_users` default challenge plugin works. + The Zope root `/acl_users` basic challenge plugin works. """ - # Check the Zope root PAS plugin configuration + # Make the basic plugin the default auth challenge self.assertIn( "credentials_basic_auth", self.root_acl_users.objectIds(), @@ -39,6 +39,11 @@ def test_zope_root_default_challenge(self): HTTPBasicAuthHelper.HTTPBasicAuthHelper, "Wrong Zope root `/acl_users` basic auth plugin type", ) + self.root_acl_users.plugins.movePluginsTop( + plugins_ifaces.IChallengePlugin, + [basic_plugin.id], + ) + transaction.commit() challenge_plugins = self.root_acl_users.plugins.listPlugins( plugins_ifaces.IChallengePlugin, ) @@ -56,14 +61,14 @@ def test_zope_root_default_challenge(self): self.assertEqual( browser.headers["Status"].lower(), "401 unauthorized", - "Wrong Zope root `/acl_users` default challenge response status", + "Wrong Zope root `/acl_users` basic challenge response status", ) - def test_zope_root_cookie_login(self): + def test_zope_root_default_login(self): """ - The Zope root `/acl_users` cookie login works. + The Zope root `/acl_users` default login works. """ - # Make the cookie plugin the default auth challenge + # Check the Zope root PAS plugin configuration self.assertIn( "credentials_cookie_auth", self.root_acl_users.objectIds(), @@ -75,15 +80,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, ) From 660343de91afc53a27ae8e77b8df1c795295f0b2 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Sun, 13 Feb 2022 14:31:03 -0800 Subject: [PATCH 2/7] style(lint): Cleanup/ignore common linter errors These are linters that my editor uses by default because they are common linters, so might as well fix what we can and ignore the rest. As much as possible, I placed ignores that should apply across the code base in the appropriate configuration file rather than inline comments. --- mypy.ini | 45 +++++++++++++++++++ pyproject.toml | 5 +++ setup.cfg | 8 ++++ .../PlonePAS/interfaces/memberdata.py | 2 +- .../PlonePAS/interfaces/membership.py | 2 +- src/Products/PlonePAS/patch.py | 2 +- src/Products/PlonePAS/plugins/ufactory.py | 2 +- src/Products/PlonePAS/sheet.py | 2 + src/Products/PlonePAS/tests/test_setup.py | 11 ++++- src/Products/PlonePAS/tools/membership.py | 2 +- 10 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 mypy.ini 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/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/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/plugins/ufactory.py b/src/Products/PlonePAS/plugins/ufactory.py index 9be8b467..107b7e93 100644 --- a/src/Products/PlonePAS/plugins/ufactory.py +++ b/src/Products/PlonePAS/plugins/ufactory.py @@ -19,7 +19,7 @@ try: from collections import OrderedDict except ImportError: - from ordereddict import OrderedDict + from ordereddict import OrderedDict # type: ignore manage_addPloneUserFactoryForm = DTMLFile('../zmi/PloneUserFactoryForm', globals()) diff --git a/src/Products/PlonePAS/sheet.py b/src/Products/PlonePAS/sheet.py index 7a83c7e2..bbd2f48c 100644 --- a/src/Products/PlonePAS/sheet.py +++ b/src/Products/PlonePAS/sheet.py @@ -8,6 +8,7 @@ from Products.CMFCore.interfaces import ISiteRoot from Products.PlonePAS.interfaces.propertysheets import IMutablePropertySheet from Products.PluggableAuthService.UserPropertySheet import UserPropertySheet +from past.builtins import long from zope.component import getUtility from zope.interface import implementer @@ -42,6 +43,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 ca502dca..dc2ef98f 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 @@ -116,7 +123,9 @@ def test_zope_root_default_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): From 955a276681e7ba3b19d9c411f00bac5f291db698 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Sun, 13 Feb 2022 15:06:20 -0800 Subject: [PATCH 3/7] refactor(setup): More sensible import step define --- src/Products/PlonePAS/configure.zcml | 12 ------------ src/Products/PlonePAS/profiles.zcml | 10 ++++++++++ 2 files changed, 10 insertions(+), 12 deletions(-) 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/profiles.zcml b/src/Products/PlonePAS/profiles.zcml index 84103954..2b93de24 100644 --- a/src/Products/PlonePAS/profiles.zcml +++ b/src/Products/PlonePAS/profiles.zcml @@ -10,5 +10,15 @@ description="Extension profile for default PlonePAS setup." provides="Products.GenericSetup.interfaces.EXTENSION" /> + + + + + + From 13c1766e9bcfc0da8306c7a33e2dd6e828affe10 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Sun, 13 Feb 2022 11:44:36 -0800 Subject: [PATCH 4/7] feat(setup): Zope root cookie login form profile Move the change of the default Zope root configuration from HTTP Basic auth to the cookie login form [into a separate GenericSetup upgrade step to make the change optional](https://github.com/plone/plone.restapi/pull/1304#issuecomment-1004702508). This reverts commit 132c2c390801ff16393f214c1501252b240cb62a. --- news/zope-root-cookie.feature | 4 ++ src/Products/PlonePAS/profiles.zcml | 16 ++++++ .../profiles/root-cookie/metadata.xml | 4 ++ .../plone-pas-zope-root-cookie.txt | 2 + src/Products/PlonePAS/setuphandlers.py | 54 ++++++++++++++++--- src/Products/PlonePAS/tests/test_setup.py | 24 ++++----- 6 files changed, 84 insertions(+), 20 deletions(-) create mode 100644 news/zope-root-cookie.feature create mode 100644 src/Products/PlonePAS/profiles/root-cookie/metadata.xml create mode 100644 src/Products/PlonePAS/profiles/root-cookie/plone-pas-zope-root-cookie.txt 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/src/Products/PlonePAS/profiles.zcml b/src/Products/PlonePAS/profiles.zcml index 2b93de24..74286bdd 100644 --- a/src/Products/PlonePAS/profiles.zcml +++ b/src/Products/PlonePAS/profiles.zcml @@ -21,4 +21,20 @@ + + + + + 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 daabb2f7..de6315af 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 @@ -324,14 +328,9 @@ def migrate_root_uf(self): pas = uf.manage_addProduct['PluggableAuthService'] plone_pas = uf.manage_addProduct['PlonePAS'] # Setup authentication plugins - setupAuthPlugins( - parent, - pas, - plone_pas, - deactivate_basic_reset=False, - # Switch from HTTP `Authorization: Basic ...` to cookie login form - deactivate_cookie_challenge=False, - ) + setupAuthPlugins(parent, pas, plone_pas, + deactivate_basic_reset=False, + deactivate_cookie_challenge=True) # Activate *all* interfaces for user manager. IUserAdder is not # activated for some reason by default. @@ -518,3 +517,42 @@ 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. + """ + # Only run step if a flag file is present, IOW not for every profile + if context.readDataFile("plone-pas-zope-root-cookie.txt") is None: + return + portal = context.getSite() + 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/tests/test_setup.py b/src/Products/PlonePAS/tests/test_setup.py index dc2ef98f..ae851272 100644 --- a/src/Products/PlonePAS/tests/test_setup.py +++ b/src/Products/PlonePAS/tests/test_setup.py @@ -29,12 +29,13 @@ 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_basic_challenge(self): + def test_zope_root_default_challenge(self): """ - The Zope root `/acl_users` basic challenge plugin works. + The Zope root `/acl_users` default challenge plugin works. """ - # Make the basic plugin the default auth challenge + # Check the Zope root PAS plugin configuration self.assertIn( "credentials_basic_auth", self.root_acl_users.objectIds(), @@ -46,11 +47,6 @@ def test_zope_root_basic_challenge(self): HTTPBasicAuthHelper.HTTPBasicAuthHelper, "Wrong Zope root `/acl_users` basic auth plugin type", ) - self.root_acl_users.plugins.movePluginsTop( - plugins_ifaces.IChallengePlugin, - [basic_plugin.id], - ) - transaction.commit() challenge_plugins = self.root_acl_users.plugins.listPlugins( plugins_ifaces.IChallengePlugin, ) @@ -68,14 +64,18 @@ def test_zope_root_basic_challenge(self): self.assertEqual( browser.headers["Status"].lower(), "401 unauthorized", - "Wrong Zope root `/acl_users` basic challenge response status", + "Wrong Zope root `/acl_users` default challenge response status", ) - def test_zope_root_default_login(self): + def test_zope_root_cookie_login(self): """ - The Zope root `/acl_users` default login works. + The Zope root `/acl_users` cookie login works. """ - # Check the Zope root PAS plugin configuration + # 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", self.root_acl_users.objectIds(), From 1e9cd05d9118e2387501e8925583731d56927d80 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 24 Feb 2022 13:44:32 -0800 Subject: [PATCH 5/7] fix(setup): Existing broken Zope root cookie [A previous PR fixed the broken Zope root cookie plugin for new installs](https://github.com/plone/Products.PlonePAS/pull/65/commits/17deb979ea4a7c6047cee1cd2a2accdbf7046384) but didn't include an upgrade step for existing Zope instances/ZODBs. The issue is only revealed when `IChallengePlugin` is activated for the broken plugins, such as when the `Products.PlonePAS:root-cookie` profile is installed, and [a `Manager` tries to login to](https://github.com/plone/Products.PlonePAS/pull/66#issuecomment-1041746695) the [Zope root ZMI](http://localhost:8080/manage_main). Add an upgrade step that fixes the issue for existing instances/ZODBs. --- src/Products/PlonePAS/profiles.zcml | 6 ++ .../PlonePAS/profiles/default/metadata.xml | 2 +- src/Products/PlonePAS/upgrades.py | 59 +++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 src/Products/PlonePAS/upgrades.py diff --git a/src/Products/PlonePAS/profiles.zcml b/src/Products/PlonePAS/profiles.zcml index 74286bdd..e6330eb0 100644 --- a/src/Products/PlonePAS/profiles.zcml +++ b/src/Products/PlonePAS/profiles.zcml @@ -20,6 +20,12 @@ + - 4 + 5 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"], + ) From 22ba99f7ee9074d6cbbab0ec680eb4582ef4efce Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Fri, 25 Feb 2022 01:49:22 -0800 Subject: [PATCH 6/7] fix(setup): Use new pre/post profile handlers [Per feedback](https://github.com/plone/Products.PlonePAS/pull/66#discussion_r808029387), use the new (to me) `post_handler` feature provided by `GenericSetup` as it is much better than littering the import step registry with one-off, profile-specific import steps. --- src/Products/PlonePAS/profiles.zcml | 10 +--------- src/Products/PlonePAS/setuphandlers.py | 5 +---- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/Products/PlonePAS/profiles.zcml b/src/Products/PlonePAS/profiles.zcml index e6330eb0..a8993a9f 100644 --- a/src/Products/PlonePAS/profiles.zcml +++ b/src/Products/PlonePAS/profiles.zcml @@ -33,14 +33,6 @@ 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" /> diff --git a/src/Products/PlonePAS/setuphandlers.py b/src/Products/PlonePAS/setuphandlers.py index de6315af..94586119 100644 --- a/src/Products/PlonePAS/setuphandlers.py +++ b/src/Products/PlonePAS/setuphandlers.py @@ -523,10 +523,7 @@ def set_up_zope_root_cookie_auth(context): """ Change the Zope root `/acl_users` to use a simple cookie login form. """ - # Only run step if a flag file is present, IOW not for every profile - if context.readDataFile("plone-pas-zope-root-cookie.txt") is None: - return - portal = context.getSite() + portal = aq_parent(context) root = portal.getPhysicalRoot() root_acl_users = getToolByName(root, "acl_users") From 9810cdead931b46029be31fb9566ad9421627b87 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Tue, 8 Mar 2022 12:52:50 -0800 Subject: [PATCH 7/7] style(lint): Backout Python 2 compat lint cleanup [Per feedback](https://github.com/plone/Products.PlonePAS/pull/66#discussion_r820449115), back out the changes that are related to Python 2 compatibility since we no longer support versions before Python 3.6. I briefly evaluated actually removing the Python 2 compatibility for these lines, but I note that `six` is still in `install_requires` for the package/dist dependency metadata so I think there's significant work to be done to cleanup no longer needed compatibility code and I don't want to hold up this PR. --- src/Products/PlonePAS/plugins/ufactory.py | 2 +- src/Products/PlonePAS/sheet.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Products/PlonePAS/plugins/ufactory.py b/src/Products/PlonePAS/plugins/ufactory.py index 107b7e93..9be8b467 100644 --- a/src/Products/PlonePAS/plugins/ufactory.py +++ b/src/Products/PlonePAS/plugins/ufactory.py @@ -19,7 +19,7 @@ try: from collections import OrderedDict except ImportError: - from ordereddict import OrderedDict # type: ignore + from ordereddict import OrderedDict manage_addPloneUserFactoryForm = DTMLFile('../zmi/PloneUserFactoryForm', globals()) diff --git a/src/Products/PlonePAS/sheet.py b/src/Products/PlonePAS/sheet.py index bbd2f48c..281dca1b 100644 --- a/src/Products/PlonePAS/sheet.py +++ b/src/Products/PlonePAS/sheet.py @@ -8,7 +8,6 @@ from Products.CMFCore.interfaces import ISiteRoot from Products.PlonePAS.interfaces.propertysheets import IMutablePropertySheet from Products.PluggableAuthService.UserPropertySheet import UserPropertySheet -from past.builtins import long from zope.component import getUtility from zope.interface import implementer