feat(setup): Zope root cookie login form profile

[rpatterson]

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.

The use case for which I built this is Volto, specifically in the JWT token plugin from
plone.restapi,
but I think I've generalized things correctly to be useful in other such situations.

[rpatterson]

Hmm, it doesn't look like any actions ran, including the prompt to run the Jenkins jobs. Might something have been broken in this repo since my last PR? I've re-reviewed the 4 commits here and don't see anything that could have affected that. CC: @jensens @thet

[rpatterson]

@jenkins-plone-org please run jobs

[rpatterson]

@jenkins-plone-org please run jobs

This didn't seem to work. I ran the same Jenkins jobs that were run for my last PR manually:

• https://jenkins.plone.org/job/pull-request-6.0-3.7/951/console
• https://jenkins.plone.org/job/pull-request-6.0-3.8/946/console
• https://jenkins.plone.org/job/pull-request-6.0-3.9/859/console

But they all failed with the same error that seems unrelated to any code changes:

...
[EnvInject] - Injecting environment variables from a build step.
[EnvInject] - [ERROR] - The given properties file path 'vars.properties' doesn't exist.
[EnvInject] - [ERROR] - Missing file path was resolved from pattern 'vars.properties' .
[EnvInject] - [ERROR] - Problems occurs on injecting env vars as a build step: java.io.IOException: The given properties file path 'vars.properties' doesn't exist.
Build step 'Inject environment variables' changed build result to FAILURE
Build step 'Inject environment variables' marked build as failure
Recording test results
ERROR: Step ‘Publish JUnit test result report’ failed: No test report files were found. Configuration error?
...

So I'm guessing that there's something broken in the Plone CI world ATM and that's why the checks have been disabled. LMK if I've got that wrong or there's anything I can do to get this reviewed.

[rpatterson]

@jenkins-plone-org please run jobs

This didn't seem to work. I ran the same Jenkins jobs that were run for my last PR manually:

Is Jenkins broken, @ericof ?

[ericof]

@plone/ai-team, @fredvd: Do you know anything about it?

[mauritsvanrees]

Yes, Jenkins is broken since this weekend. :-(
I created an issue yesterday: plone/jenkins.plone.org#292

[rpatterson]

Yes, Jenkins is broken since this weekend. :-( I created an issue yesterday: plone/jenkins.plone.org#292

Thanks for reaching out with the update, @mauritsvanrees @ericof, I'm sure you're way too busy!

[fredvd]

@rpatterson Jenkins is up and running again. The plugin configuration of jenkins somehow got borked after a Jenkins update this weekend. We're still looking into how that could happen, but all tests/jobs are active again.

[mister-roboto]

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

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

[fredvd]

Sorry, shouldn't close the PR. :-$ Thought this was the other issue on jenkins.plone.org repo.

[fredvd]

@jenkins-plone-org please run jobs

[rpatterson]

Alright, @jensens, @thet or anyone else, I think this is ready fore review! LMK if you need anything more from me.

[jensens]

LGTM and well thought.

[rpatterson]

Alright, all checks are passing, so I think this is ready for merge, @jensens or whoever.

[mauritsvanrees]

I tried it out on an existing Plone 6 database where several Plone Sites exist already. I added a new Plone Site. Then go to portal_setup and install the Products.PlonePAS:root-cookie profile. Then as anonymous user go to http://localhost:8080/manage_main. This raises an Unauhtorized, which kicks in the challenge plugin. But this fails with a traceback:

2022-02-16 16:34:44,995 ERROR   [Zope.SiteErrorLog:252][waitress-2] 1645025684.9947740.3392464506199683 http://localhost:8080/acl_users/credentials_cookie_auth/login_form
Traceback (innermost last):
  Module ZPublisher.WSGIPublisher, line 167, in transaction_pubevents
  Module ZPublisher.WSGIPublisher, line 376, in publish_module
  Module ZPublisher.WSGIPublisher, line 255, in publish
  Module ZPublisher.BaseRequest, line 529, in traverse
  Module ZPublisher.HTTPResponse, line 976, in debugError
zExceptions.NotFound: Cannot locate object at: http://localhost:8080/acl_users/credentials_cookie_auth/login_form

So a login form is missing.

[rpatterson]

@jenkins-plone-org please run jobs

[rpatterson]

Hmm, I swear I had coverage for that, but yeah, I just checked an existing ZODB and I am able to reproduce this. Digging ino it and thanks for the catch, @mauritsvanrees!

I didn't have test coverage for this after all as this is an upgrade step issue. So I rigged up a semi-manual test bed for reproducing upgrade/migration issues, and used that to reproduce this issue and add a Products.PlonePAS:PlonePAS upgrade step. It's now working for me. Can you resume testing, @mauritsvanrees, and run that upgrade step before trying out the new Products.PlonePAS:root-cookie profile?

And with that I'm back to a place where I think we're ready to finish reviews and merge. LMK what else anyone needs from me!

[rpatterson]

Is anyone able to finish reviewing this or help me draw attention to this to help finish review? CC: @avoinea, @thet, @jensens, @fredvd, @mauritsvanrees

[jensens]

LGTM. There is some code for Python 2.7 compat left, but that is probably inherited and not part of this PR. Anyway, it would be nice to have it cleaned up along the way. OTOH it should not be a blocker for a merge here.

[mauritsvanrees]

I tried it on a completely fresh database:

• create a Plone Site
• via portal_setup install the root-cookie profile
• it works

Existing database:

• go to a Plone site
• run the PlonePAS upgrade
• apply the root-cookie profile
• it works

So this is merge-worthy.

The only thing I wonder is: why is there an upgrade step for the default profile? I would think it is the responsibility of the root-cookie profile to make it work both in a completely fresh database and in an existing database and existing site. The way you have it now works, but I don't completely see the logic.
Does my reservation make sense?
Minor point, since it works, but still.

[rpatterson]

So this is merge-worthy.

Woot! Thanks for double checking the existing ZODB cases.

The only thing I wonder is: why is there an upgrade step for the default profile? I would think it is the responsibility of the root-cookie profile to make it work both in a completely fresh database and in an existing database and existing site.

That particular upgrade step fixes broken Zope root /acl_users plugins created by the default profile, so I think it's more correct to have it registered to the profile that created the broken plugins. Make sense, @mauritsvanrees?

[rpatterson]

@jenkins-plone-org please run jobs

[mauritsvanrees]

That particular upgrade step fixes broken Zope root /acl_users plugins created by the default profile, so I think it's more correct to have it registered to the profile that created the broken plugins. Make sense, @mauritsvanrees?

Ah, I see. I missed that the setup was already fixed in a previous PR.

Fine to merge when green.

[rpatterson]

Fine to merge when green.

K, everything is green. Should I merge? If not, who should? CC: @mauritsvanrees @jensens

[mauritsvanrees]

I will merge.
Thanks a lot for your work!

[rpatterson]

I will merge. Thanks a lot for your work!

Thanks for merging! May I ask what the process is for release, @mauritsvanrees. I have another PR in another repo that depends on this so I just want to know what to track so I can move that other PR forward once this is released.

[mauritsvanrees]

The process is:

• Once merged, Products.PlonePAS is automatically added to the coredev checkouts by mr.roboto in a fake commit.
• When I am preparing a Plone release, I look in checkouts.cfg and make releases of those packages.

Or indeed you ask, if you need it sooner. :-)
I have released Products.PlonePAS 7.0.0a3.

[mauritsvanrees]

Wait, I am making some mistakes here. Hold on.

[mauritsvanrees]

Ah no, it is fine. I did get an error at first on install:

Getting distribution for 'Products.PlonePAS==7.0.0a3'.
ERROR: Directory '/Users/maurits/community/plone-coredev/6.0/src/Products.PlonePAS/src' is not installable. Neither 'setup.py' nor 'pyproject.toml' found.

Strange. But second time it works. It should not have been using the src checkout anymore anyway. But it works now.

[rpatterson]

Or indeed you ask, if you need it sooner. :-) I have released Products.PlonePAS 7.0.0a3.

Hehe, awesome and thanks much!

Strange. But second time it works. It should not have been using the src checkout anymore anyway. But it works now.

Odd indeed. Did you have any inkling that this could be related to my changes?

[mauritsvanrees]

Odd indeed. Did you have any inkling that this could be related to my changes?

No. For a moment I thought the creation of the release had somehow failed, and for another moment I thought I had forgotten to pull your changes before creating a release. But all was well after all.

[avoinea]

@mauritsvanrees @rpatterson Should we add the upgrade step also to plone.app.upgrade?

[mauritsvanrees]

@mauritsvanrees @rpatterson Should we add the upgrade step also to plone.app.upgrade?

In the MigrationTool we have a list of add-ons. A different name may have been better: it is really a list of Plone core profiles, although technically a community add-on could register itself in this with some Python code. Anyway, at the end of running @@plone-upgrade, all profiles in this list get upgraded.

We should add the Products.PlonePAS profile to this list. In my opinion, all core packages that provide their own upgrade steps, should be in this list. That is what I intended it for. Obviously, the list is missing a few profiles now. I can have a look at updating it. It is on my list to have a look at anyway, for this issue: plone/Products.CMFPlone#3346.