[Framework-Team] PLIPs 208 and 217 Ready for Review

Alec Mitchell apm13 at columbia.edu
Fri Jan 18 18:01:42 UTC 2008


Hi Team,

I've got a buildout for the local roles PLIP (208) ready:

https://svn.plone.org/svn/plone/review/plip208-localroles

It includes branches of borg.localrole and a branch of PlonePAS that
installs the borg.localrole plugin in place of the default one.  In
the course of my testing and creating the bundle I discovered a couple
flaws in the PlonePAS local role plugin.

1) getRolesInContext doesn't check if the principal it's looking at is
defined in the current security context as it traversed the
containment chain to find roles.  This is a pretty minor bug, but it
means that if there were e.g. a user named "plone" defined at the Zope
root and given the Manager role locally on the zope root, and another
user named "plone" defined at the portal root which wasn't given the
manager role, getRolesInContext would be incorrectly granting manager
to that user.  borg.localrole performs context checks at each
acquisition level.  The checkLocalRolesAllowed method had the correct
behavior.

2) The local role plugin in PlonePAS was taking responsibility for
returning the user's global roles as well, this seems very
inappropriate, though it is just a matter of convention.  Because I
didn't want to replicate this mis-feature in borg.localrole, I moved
the the inclusion of global roles from the the local role plugin's
getRolesInContext method to the user's getRolesInContext method.

There is one piece missing from the buildout which is a CMFPlone
migration step to replace the old local role manager in existing
sites.  That shouldn't hinder testing or review, but I didn't feel it
was worth branching CMFPlone for a minor thing which isn't needed
during review.


There's also a buildout for the workflow PLIP (217):

https://svn.plone.org/svn/plone/review/plip217-workflow

This adds branches of CMFPlone and CMFPlacefulWorflow.  The CMFPlone
branch includes the changes to the workflow tool, the default adapter,
and tests.  The CMFPlacefulWorkflow branch now uses an adapter in
place of the monkey patch and the code has been significantly
simplified.  There is one caveat, which is that the buildout test
runner seems to have trouble coping with two different
CMFPlacefulWorkflow products in different products folders.  Though it
runs the tests for the correct CMFPlacefulWorkflow (the one in
'products') it tries to use the 'Extensions/Install.py' from the wrong
one (the one in 'parts/plone').  So to run the CMFPlacefulWorkflow
tests, you will need to remove the product from 'parts/plone'.
There's probably some way to fix this, but I'm a buildout novice
really.

Both buildouts are based on the ZopeSkel 'plone3_buildout' template.

Thanks and happy reviewing,
Alec




More information about the Framework-Team mailing list