plone.restapi
plone.restapi copied to clipboard
plone
feat(auth): Sync classic/API PAS plugin cookies
Build on #1141 to flesh out the rest of the PAS plugin interfaces that the JWT plugin should support, namely those methods that handle log in and log out. With these changes logging in to or out of Plone classic or the Volto UI does the same in the other. This should result in a much less surprising user experience for those users that will end up using both interfaces and makes the JWT plugin more correct and complete. See also the related Volto PR.
Regarding the use case, I understand the intention is to move away from Plone classic as a supported UI. I think it would be a painful mistake to imagine that's going to be a reality for all important use cases and user classes anytime soon. We should expect Volto to be the UI for an increasingly major portion of use cases over time but allow it time to get to all of them. Personally, I wouldn't plan on classic being something we could stop supporting for a couple of major versions of Plone.
@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!
Apologies, I went on auto pilot and stopped running the full test suite locally. I can see these errors locally and I'm working on them now.
I believe I've addressed all your feedback, @tisto. LMK if I missed anything or if there's anything else I can do to move this forward. Thanks for the review!
I just realized that some of my previous local development build improvements make it pretty easy to create a Zope and Plone instance from before my various auth PRs and then test it under all my PRs. Having done so, it confirms that at least some sort of upgrade step is required. Specifically, because token generation has been moved into updateCredentials(...) we need an upgrade step that enables the JWT token plugin for that PAS plugin interface. IOW, this is not ready for merge.
@rpatterson Just out of curiosity, what's your use case? Only authenticated access to Plone Classic? Is there any situation where the anonymous accesses Plone Classic? Does plone api return HTML to Volto?
@rpatterson Just out of curiosity, what's your use case?
I'm on the EEA team, so I'm working under the direction of their leadership. But as I understand it, the use case is to have the experience of using Volto, Plone classic, and the ZMI to be seamless in terms of logging in and out. IOW, it doesn't matter where you log in, you'll be logged in everywhere and ditto for logging out.
Only authenticated access to Plone Classic? Is there any situation where the anonymous accesses Plone Classic? Does plone api return HTML to Volto?
I don't really follow these questions. Can you clarify?
What I was wondering, is what is your site's workflow? Do content managers authenticate to Plone Classic to register content (Page, News...)? After this content consumed by Volto to anonymous users?
@wesleybl @rpatterson Although I'm not on the EEA team which is currently handling this feature request, I think I can add some insight. We currently have a Volto website (eea.europa.eu/ims) running as a subfolder of a Plone 4 classic website. The intention is to have editors seamless authentication between the classic Plone and the Volto part. I think the ZMI auth is a nice to have, but I may be wrong.
...it confirms that at least some sort of upgrade step is required. ... IOW, this is not ready for merge.
I just pushed the upgrade step and related changes. So this should be ready for review and merge.
I just pushed the upgrade step and related changes. So this should be ready for review and merge.
Don't suppose anyone can review this? Is there anything blocking review here? Sorry to pester, but... I'm pestering. ;-) CC: @sneridagh @tiberiuichim @wesleybl
@rpatterson
I added this branch to my existing Plone 6 setup https://github.com/eea/eea-website-backend/tree/master/develop source.ini
And I get this error when accessing http://localhost:8080
2022-02-10 18:36:39 ERROR [Zope.SiteErrorLog:35][waitress-2] ComponentLookupError: http://localhost:8080/manage_main
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 619, in traverse
Module Products.PluggableAuthService.PluggableAuthService, line 244, in validate
Module Products.PluggableAuthService.PluggableAuthService, line 603, in _extractUserIds
Module plone.restapi.pas.plugin, line 133, in authenticateCredentials
Module plone.restapi.pas.plugin, line 214, in _decode_token
Module zope.component._api, line 165, in getUtility
zope.interface.interfaces.ComponentLookupError: (<InterfaceClass plone.keyring.interfaces.IKeyManager>, '')
I added this branch to my existing Plone 6 setup https://github.com/eea/eea-website-backend/tree/master/develop
source.iniAnd I get this error when accessing http://localhost:8080
... zope.interface.interfaces.ComponentLookupError: (<InterfaceClass plone.keyring.interfaces.IKeyManager>, '')
Did you run the upgrade step, @avoinea?
Did you run the upgrade step, @avoinea?
As I can't access anything, including /manage_main I'm not sure how a common site admin will be able to run the upgrade step :thinking:
As I can't access anything, including
/manage_mainI'm not sure how a common site admin will be able to run the upgrade step thinking
Hmm, I don't understand. The exception should only happen once the PAS interfaces are activated but that's exactly what the upgrade step does. Can you try the steps I did to test the upgrade step and see if you can reproduce what you observe in the Volto buildout. Can elaborate on your test steps and how they differ, @avoinea?
@rpatterson These are the steps to reproduce:
- Install https://github.com/eea/eea-website-backend/tree/master/develop#install
- Create a Plone site
- Stop Zope instance
- Add
plone.restapitosources.ini(branchfix-unify-auth-logout) - Run
make develop - Run
make start - Go to http://localhost:8080
- You'll get the
ComponentLookuperror
It seems you can still go to http://localhost:8080/Plone and run the upgrade step. But the error is still there after the upgrade on Zope root.
@rpatterson These are the steps to reproduce:
And the steps I used? How is the interface being activated before the upgrade step in your test bed?
Do I have to also checkout other packages from different branches?
Oh yeah, sorry, @avoinea! When I first started stitching togetherr PRs across the repos involved in Volto, I was maintaining and submitting PRs in plone/volto as well that included the checkouts required to test it all together in that repo. I was told we don't handle it that way, I stopped maintaining such plone/volto branches. I've refreshed it all and pushed to the fix-unify-auth branch, so you should be able to use that to test or as a source for how to test properly in that other project.
I've now run the failing Jenkins job 3 times and it's failing at the same place every time:
==============================================================================
Test Overlays :: These tests are just testing the overlay behavior not the ...
==============================================================================
Scenario: Set default content item of a folder overlay opens | FAIL |
Setup failed:
WebDriverException: Message: chrome not reachable
Also teardown failed:
No browser is open.
------------------------------------------------------------------------------
Test Overlays :: These tests are just testing the overlay behavior... | FAIL |
1 critical test, 0 passed, 1 failed
1 test total, 0 passed, 1 failed
==============================================================================
Output: /home/jenkins/workspace/pull-request-6.0-3.9/parts/test/test_overlays/Scenario_Set_default_content_item_of_a_folder_overlay_opens/output.xml
Failure in test Scenario Set default content item of a folder overlay opens (test_overlays.robot)
Traceback (most recent call last):
File "/srv/python3.9/lib/python3.9/unittest/case.py", line 59, in testPartExecutor
yield
File "/srv/python3.9/lib/python3.9/unittest/case.py", line 592, in run
self._callTestMethod(testMethod)
File "/srv/python3.9/lib/python3.9/unittest/case.py", line 550, in _callTestMethod
method()
File "/home/jenkins/.buildout/eggs/cp39/robotsuite-2.2.1-py3.9.egg/robotsuite/__init__.py", line 475, in runTest
assert last_status == 'PASS', last_message
AssertionError: Setup failed:
WebDriverException: Message: chrome not reachable
Also teardown failed:
No browser is open.
I've tested that overlay in Plone classic in my local development environment and it seems to be working just fine. I can't imagine how this failure could be related to the changes here and I've been told before that the robot tests are intermittent. So can I either get some help understanding how this is related to the changes here or can we ignore that failure and proceed? CC: @avoinea
I've now run the failing Jenkins job 3 times and it's failing at the same place every time:
Apparently, the 4th time is the charm. This kind of intermittent CI false-negative is a big problem for an open source community. Personally, I think intermittent false failures/errors/negatives in CI should be immediately "commented out" or otherwise skipped and some part of whatever resources the community or governance has to work with should be dedicated to resolving intermittent CI issues continuously over the longer term.
At any rate, AFAIK, this is ready to finish review and merge. LMK if I'm missing anything? CC: @avoinea @rpatterson tiberiuichim @wesleybl @sneridagh
Is anyone able to finish reviewing this or help me draw attention to this to help finish review? CC: @avoinea @tiberiuichim @wesleybl @sneridagh
@rpatterson When I already have the Volto site open in the browser and I authenticate in the Plone Site, when I return to the Volto Site, I do not see the authenticated site. Only after I press F5 on Volto site do I see the authenticated site. Could anything be done?