Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(setup): Zope root cookie auth and login form #65

Merged
merged 2 commits into from
Dec 29, 2021

Conversation

rpatterson
Copy link
Member

The GenericSetup "various" import step that originally installs PAS into a Plone portal
also migrates the Zope root /acl_users. Probably an accident over time, but it
results in a cookie auth plugin that doesn't work outside of the Plone portal:

2021-12-27 11:12:02,243 ERROR   [Zope.SiteErrorLog:22][waitress-0] 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.PlonePAS.plugins.cookie_handler, line 106, in login
  Module Products.PluggableAuthService.PluggableAuthService, line 1153, in updateCredentials
  Module Products.PlonePAS.plugins.cookie_handler, line 74, in updateCredentials
  Module zope.component._api, line 165, in getUtility
zope.interface.interfaces.ComponentLookupError: (<InterfaceClass plone.registry.interfaces.IRegistry>, '')

This import step also removes the login_form template which breaks the challenge
response.

Add an interface check to decide whether to install Plone's ExtendedCookieAuthHelper
or PAS's vanilla CookieAuthHelper.

@mister-roboto
Copy link

@rpatterson thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

The GenericSetup "various" import step that originally installs PAS into a Plone portal
also migrates the Zope root `/acl_users`.  Probably an accident over time, but it
results in a cookie auth plugin that doesn't work outside of the Plone portal:

    2021-12-27 11:12:02,243 ERROR   [Zope.SiteErrorLog:22][waitress-0] 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.PlonePAS.plugins.cookie_handler, line 106, in login
      Module Products.PluggableAuthService.PluggableAuthService, line 1153, in updateCredentials
      Module Products.PlonePAS.plugins.cookie_handler, line 74, in updateCredentials
      Module zope.component._api, line 165, in getUtility
    zope.interface.interfaces.ComponentLookupError: (<InterfaceClass plone.registry.interfaces.IRegistry>, '')

This import step also removes the `login_form` template which breaks the challenge
response.

Add an interface check to decide whether to install Plone's `ExtendedCookieAuthHelper`
or PAS's vanilla `CookieAuthHelper`.
@rpatterson
Copy link
Member Author

@jenkins-plone-org please run jobs

@rpatterson
Copy link
Member Author

Everything is green, @jensens. LMK if there's anything I can do to move this forward.

Copy link
Member

@jensens jensens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM. Suggested minor change would be nice to get rid of old format strings.

src/Products/PlonePAS/setuphandlers.py Show resolved Hide resolved
@rpatterson
Copy link
Member Author

LMK if I should merge this myself or if there's otherwise anything I should do to get this merged, @jensens.

@jensens jensens merged commit 7eb1802 into master Dec 29, 2021
@jensens jensens deleted the fix-zope-root-cookie-auth branch December 29, 2021 20:42
@Rudd-O
Copy link

Rudd-O commented Jan 29, 2022

Wow, this is astonishingly good work. Not only is the code clearer, it must have taken a helluva lot of work to understand the old one (in particular with the amount of context needed from back in the Zope 2 days) and get it working again. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants