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.

Sign up for GitHub

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

Use the Iterate permissions instead of CMF core permissions #68

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Use the Iterate permissions instead of CMF core permissions #68

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e54b33a
Fix flake8 warning
rpatterson Feb 21, 2019
2f002a2
Use the Iterate permissions instead of CMF core permissions #67
rpatterson Feb 21, 2019
e4d5009
Fix error when checking in from a memebers home folder
rpatterson Feb 25, 2019
50d9d0f
Better permission check
rpatterson Feb 25, 2019
a9ae6b8
Avoid circular dependencies, remove plone.api dependency
rpatterson Mar 5, 2019
c90e2c3
Allow controlling access based on who initiated the check out
rpatterson Mar 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions news/67.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Use the Iterate permissions instead of CMF core permissions to support
customizing the policy. [rpatterson]
12 changes: 9 additions & 3 deletions plone/app/iterate/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,17 @@

from Acquisition import aq_inner
from Acquisition import aq_parent
from AccessControl import SpecialUsers

from plone.app.iterate import interfaces
from plone.app.iterate.event import BeforeCheckoutEvent
from plone.app.iterate.event import CancelCheckoutEvent
from plone.app.iterate.event import CheckoutEvent
from plone.app.iterate.interfaces import ICheckinCheckoutPolicy
from plone.app.iterate.interfaces import IObjectCopier
from plone.app.iterate.util import get_storage
from plone import api

from Products.CMFCore import interfaces as cmf_ifaces
from Products.CMFCore.utils import getToolByName
from zope import component
Expand Down Expand Up @@ -151,10 +155,12 @@ def _removeDuplicateReferences(self, item, backrefs=False):
def _copyBaseline(self, container):
# copy the context from source to the target container
source_container = aq_parent(aq_inner(self.context))
clipboard = source_container.manage_copyObjects([self.context.getId()])
result = container.manage_pasteObjects(clipboard)
with api.env._adopt_user(SpecialUsers.system):
jensens marked this conversation as resolved.
Show resolved Hide resolved
clipboard = source_container.manage_copyObjects(
[self.context.getId()])
result = container.manage_pasteObjects(clipboard)

# get a reference to the working copy
target_id = result[0]['new_id']
target = container._getOb(target_id)
return target
return target
2 changes: 1 addition & 1 deletion plone/app/iterate/browser/configure.zcml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
name="content-checkin"
class=".checkin.Checkin"
template="checkin.pt"
permission="cmf.ModifyPortalContent"
permission="plone.app.iterate.CheckInContent"
/>

<browser:page
Expand Down
15 changes: 9 additions & 6 deletions plone/app/iterate/browser/control.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from plone.memoize.view import memoize
from Products.Five.browser import BrowserView

import Products.CMFCore.permissions
from .. import permissions
Copy link

Choose a reason for hiding this comment

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

Does this work as you expect in Python 3?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why wouldn't it, @Rotonen?

Copy link
Member

Choose a reason for hiding this comment

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

.. is fine

Copy link
Member

Choose a reason for hiding this comment

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

Is fine, but it way better to just use from plone.app.iterate import permissions is not that much typing and you can keep imports sanely

Copy link
Member Author

Choose a reason for hiding this comment

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

I disagree that absolute imports are way better. Relative imports do save some minor refactoring time when package names or structure changes, and if you're typing your imports out they do save some time though you should not be typing your imports out, but the far greater value is when reading code. They make it very clear which dependencies of the code are internal to the package and external to the package even when just skimming the code. This is especially the case for a large ecosystem of packages such as Plone where a developer may be working on 3-6 plone.app.* packages at the same time and it adds cognitive burden to require the developer to remember which plone.app.* package they're reading ATM in order to determine which code dependencies are internal. The core language and the community that develops it, provide this feature for well considered reasons.



class Control(BrowserView):
Expand Down Expand Up @@ -62,11 +62,8 @@ def checkin_allowed(self):
if original is None:
return False

can_modify = checkPermission(
Products.CMFCore.permissions.ModifyPortalContent,
original,
)
if not can_modify:
can_check_in = checkPermission(permissions.CheckinPermission, original)
if not can_check_in:
return False

return True
Expand All @@ -75,6 +72,7 @@ def checkout_allowed(self):
"""Check if a checkout is allowed.
"""
context = aq_inner(self.context)
checkPermission = getSecurityManager().checkPermission

if not interfaces.IIterateAware.providedBy(context):
return False
Expand All @@ -94,6 +92,11 @@ def checkout_allowed(self):
if policy.getBaseline() is not None:
return False

can_check_out = checkPermission(
permissions.CheckoutPermission, context)
if not can_check_out:
return False

return True

@memoize
Expand Down
16 changes: 12 additions & 4 deletions plone/app/iterate/browser/info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@

from AccessControl import getSecurityManager
from DateTime import DateTime

from plone.app.iterate.interfaces import IBaseline
from plone.app.iterate.interfaces import ICheckinCheckoutPolicy
from plone.app.iterate.interfaces import keys
from plone.app.iterate.permissions import CheckoutPermission
from plone.app.iterate import permissions

from plone.memoize.instance import memoize
from Products.CMFCore.permissions import ModifyPortalContent
from Products.CMFCore.utils import getToolByName
Expand Down Expand Up @@ -92,7 +94,7 @@ def properties(self):
return {}

def _getReference(self):
raise NotImplemented
raise NotImplementedError()


class BaselineInfoViewlet(BaseInfoViewlet):
Expand All @@ -104,7 +106,10 @@ def render(self):
working_copy = self.working_copy()
if working_copy is not None and (
sm.checkPermission(ModifyPortalContent, self.context) or
sm.checkPermission(CheckoutPermission, self.context) or
sm.checkPermission(
permissions.CheckoutPermission, self.context) or
sm.checkPermission(
permissions.CheckinPermission, working_copy) or
sm.checkPermission(ModifyPortalContent, working_copy)):
return self.index()
else:
Expand All @@ -127,7 +132,10 @@ def render(self):
baseline = self.baseline()
if baseline is not None and (
sm.checkPermission(ModifyPortalContent, self.context) or
sm.checkPermission(CheckoutPermission, baseline)):
sm.checkPermission(
permissions.CheckoutPermission, baseline) or
sm.checkPermission(
permissions.CheckinPermission, baseline)):
return self.index()
else:
return ''
Expand Down
20 changes: 10 additions & 10 deletions plone/app/iterate/configure.zcml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@
<include package="zope.annotation" />
<include package="Products.CMFCore" file="permissions.zcml" />

<permission
id="plone.app.iterate.CheckInContent"
title="iterate : Check in content"
/>

<permission
id="plone.app.iterate.CheckOutContent"
title="iterate : Check out content"
/>

<include package=".subscribers"/>
<include package=".browser"/>

Expand Down Expand Up @@ -85,16 +95,6 @@
handler=".event.handleDeletion"
/>

<permission
id="plone.app.iterate.CheckInContent"
title="iterate : Check in content"
/>

<permission
id="plone.app.iterate.CheckOutContent"
title="iterate : Check out content"
/>

<include package=".dexterity" zcml:condition="installed plone.app.relationfield" />
<include file="at.zcml" zcml:condition="installed Products.Archetypes" />

Expand Down
9 changes: 8 additions & 1 deletion plone/app/iterate/copier.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,13 @@
from Acquisition import aq_base
from Acquisition import aq_inner
from Acquisition import aq_parent
from AccessControl import SpecialUsers

from plone.app.iterate import interfaces
from plone.app.iterate.base import BaseContentCopier
from plone.app.iterate.interfaces import CheckinException
from plone import api

from Products.Archetypes.Referenceable import Referenceable
from Products.CMFCore.utils import getToolByName
from Products.DCWorkflow.DCWorkflow import DCWorkflowDefinition
Expand Down Expand Up @@ -116,7 +120,10 @@ def _replaceBaseline(self, baseline):
self.context._v_is_cp = 0

wc_id = self.context.getId()
wc_container.manage_delObjects([wc_id])
# Bypass AT security check,
# checking `iterate : Check in content` should be sufficient
with api.env._adopt_user(SpecialUsers.system):
jensens marked this conversation as resolved.
Show resolved Hide resolved
wc_container.manage_delObjects([wc_id])

# move the working copy back to the baseline container
working_copy = aq_base(self.context)
Expand Down
7 changes: 4 additions & 3 deletions plone/app/iterate/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
CheckinPermission = 'iterate : Check in content'
CheckoutPermission = 'iterate : Check out content'

DEFAULT_ROLES = ('Manager', 'Owner', 'Site Administrator', 'Editor')
addPermission(CheckinPermission, default_roles=DEFAULT_ROLES)
addPermission(CheckoutPermission, default_roles=DEFAULT_ROLES)
addPermission(CheckinPermission, default_roles=(
'Manager', 'Site Administrator', 'Reviewer'))
addPermission(CheckoutPermission, default_roles=(
'Manager', 'Owner', 'Site Administrator', 'Editor'))
6 changes: 6 additions & 0 deletions plone/app/iterate/testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,16 @@
'password': 'secret',
'roles': ['Contributor'],
}
REVIEWER = {
'id': 'reviewer',
'password': 'secret',
'roles': ['Reviewer'],
}
USERS_TO_BE_ADDED = (
ADMIN,
EDITOR,
CONTRIBUTOR,
REVIEWER,
)
USERS_WITH_MEMBER_FOLDER = (
EDITOR,
Expand Down
34 changes: 20 additions & 14 deletions plone/app/iterate/tests/browser.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ check out published pages::
True

>>> browser = z2.Browser(app)
>>> browser.handleErrors = False
>>> browser.addHeader('Authorization', 'Basic editor:secret')

>>> browser.open(portal.absolute_url() + '/hello-world')
Expand All @@ -113,19 +114,12 @@ therefore our Editor lacks permissions to modify the original::
...
LinkNotFoundError

The Editor could, however, ask someone to retract the original so he
gains permissions again and check in (and then possibly request for
review)::
The Reviewer can check the working copy back in to update the original
published item::

>>> browser = z2.Browser(app)
>>> browser.addHeader('Authorization',
... 'Basic %s:%s' % (SITE_OWNER_NAME, SITE_OWNER_PASSWORD))
>>> browser.open(portal.absolute_url() + '/hello-world')
>>> browser.getLink("Published").click()
>>> browser.getControl("Retract").click()
>>> browser.getControl("Save").click()
>>> browser = z2.Browser(app)
>>> browser.addHeader('Authorization', 'Basic editor:secret')
>>> browser.handleErrors = False
>>> browser.addHeader('Authorization', 'Basic reviewer:secret')
>>> browser.open(portal.absolute_url() + '/hello-world')
>>> browser.getLink("working copy").click()
>>> browser.getLink("Check in").click()
Expand Down Expand Up @@ -261,8 +255,14 @@ Create a new page to test workflows with::

Checkout::

>>> import transaction
>>> from plone import api
>>> api.user.grant_roles(username='editor', roles=['Contributor'])
>>> transaction.commit()

>>> browser = z2.Browser(app)
>>> browser.addHeader('Authorization', 'Basic contributor:secret')
>>> browser.handleErrors = False
>>> browser.addHeader('Authorization', 'Basic editor:secret')
>>> browser.open(workflow_test_url)
>>> browser.getLink(id='plone-contentmenu-actions-iterate_checkout').click()
>>> browser.contents
Expand All @@ -271,23 +271,27 @@ Checkout::
>>> checkout_form.getControl('Parent folder').selected = True
>>> checkout_form.getControl('Check out').click()
>>> browser.contents
'...This is a working copy of...My workflow test..., made by...contributor...'
'...This is a working copy of...My workflow test..., made by...editor...'
>>> browser.contents
'...state-draft-copy...'
>>> workflow_checkout_url = browser.url

>>> api.user.revoke_roles(username='editor', roles=['Contributor'])
>>> transaction.commit()

Check get info message on original::

>>> browser.open(workflow_test_url)
>>> browser.contents
'...This item is being edited by...contributor...a working copy...'
'...This item is being edited by...editor...a working copy...'

We're going to manually give the contributor user the CheckoutPermission
to check it's used when displaying the info messages. In our workflow
once the checked out item is submitted the contributor no longer has
permission to modify it but we still want them to see the info messages::

>>> browser = z2.Browser(app)
>>> browser.handleErrors = False
>>> browser.addHeader('Authorization',
... 'Basic %s:%s' % (SITE_OWNER_NAME, SITE_OWNER_PASSWORD))

Expand All @@ -297,6 +301,7 @@ permission to modify it but we still want them to see the info messages::
>>> browser.getControl('Save Changes').click()

>>> browser = z2.Browser(app)
>>> browser.handleErrors = False
>>> browser.addHeader('Authorization', 'Basic contributor:secret')
>>> browser.open(workflow_checkout_url)
>>> browser.getLink(id='workflow-transition-submit-copy-for-publication')\
Expand All @@ -314,6 +319,7 @@ move permissions in our workflow so this should not appear in the action menu.
http://code.google.com/p/dexterity/issues/detail?id=258 ::

>>> browser = z2.Browser(app)
>>> browser.handleErrors = False
>>> browser.addHeader('Authorization', 'Basic editor:secret')
>>> browser.open(workflow_checkout_url)
>>> browser.getLink(id='plone-contentmenu-actions-copy')
Expand Down