Ticket #5817 (closed Bug: fixed)

Opened 8 years ago

Last modified 7 years ago

Localrole fixing script does not call reindexObjectSecurity

Reported by: asmith Owned by: alecm
Priority: major Milestone: 2.5.4
Component: Upgrade/Migration Version:
Keywords: Cc:

Description

When migrating Plone 2.1.2 site to Plone 2.5.1 Group names migrate correctly BUT - all folders and files with a share group assigned are incorrectly migrated with the prefix "group_"

eg. the group "contracting" is migrated correctly

but the folder with the group sharing permission of "contracting" will be incorrectly labeled as "group_contracting"

Change History

comment:1 Changed 8 years ago by limi

Yes, we corrected this problem manually on plone.org, but we need to supply that script with Plone 2.5.x so people can fix it too.

The script can be found at  https://svn.plone.org/svn/plone/PloneOrg/Extensions/stripGRUFLocalRolePrefix.py but we should ship it with Plone too.

comment:2 Changed 8 years ago by alecm

  • Owner changed from somebody to wichert

comment:3 Changed 8 years ago by hannosch

  • Status changed from new to closed
  • Resolution set to fixed

The script and some small documentation is included in 2.5.1.

comment:4 Changed 7 years ago by edouard_mercier

  • Status changed from closed to reopened
  • Keywords local role added
  • Resolution fixed deleted
  • Priority changed from major to critical

According to me the script 'stripGRUFLocalRolePrefix.py ' misses the call to 'reindexObjectSecurity', in order for the local roles to actual take effect. Without that call, the local role groups are properly removed the 'group_' prefix, but this is not enough!

comment:5 Changed 7 years ago by hannosch

  • Milestone changed from 2.5.1 to 2.5.x

comment:6 Changed 7 years ago by limi

  • Keywords group local role removed
  • Priority changed from critical to major
  • Summary changed from Groups in Plone 2.5.1 - incorretly named. to Localrole fixing script does not call reindexObjectSecurity
  • Milestone changed from 2.5.x to 2.5.3

This should be fixed for 2.5.3, if possible.

comment:7 follow-up: ↓ 8 Changed 7 years ago by alecm

  • Status changed from reopened to closed
  • Resolution set to fixed

(In [14345]) Reindex security recursively after group name update in stripGRUFLocalRolePrefix. Fixes #5817

comment:8 in reply to: ↑ 7 Changed 7 years ago by keul

  • Status changed from closed to reopened
  • Resolution fixed deleted

Replying to alecm:

(In [14345]) Reindex security recursively after group name update in stripGRUFLocalRolePrefix. Fixes #5817

Sorry but the reindexObjectSecurity version on Plone 2.5.3 isn't working for me. Calling self.reindexObjectSecurity() in the "do" method call the reindex on the Zope App object, and so raise an error!

I moved the calling inside the removePrefix, in something like this

if obj.objectIds():
    obj.reindexObjectSecurity()
    removePrefix(obj, root=False)

I dont't know is this is a waste of time (can I call reindexObjectSecurity only on the "Plone Site" object?)

comment:9 Changed 7 years ago by wichert

  • Owner changed from wichert to alecm
  • Status changed from reopened to new

comment:10 Changed 7 years ago by alecm

  • Status changed from new to closed
  • Resolution set to fixed

Calling it on the object will work, but the whole process will be much slower. Installing the external method in your Plone site instead of in the zope root will fix the problem. Even if you install it in the zope root and call it from the plone site, it may make use the portal context as self, rather than the container, though I'm not positive how external methods determine self. In any case installing it in the plone site, which is the intended usage, will work. Your fix would work too, but it's unnecessary.

comment:11 Changed 7 years ago by keul

  • Status changed from closed to reopened
  • Resolution fixed deleted

Sorry for reopen... again!

The first time I tested this, the external method was in the root of the Plone site, not in the Zope root. But is wasn't working because the of this:

    for id in folder.objectIds():
        obj = folder._getOb(id)

        if root == True and not obj.meta_type == 'Plone Site':
            continue

The script look if one (or more) of the child of root is a "Plone Site", not the root itself. So the script will do absolutly nothing if launched on the Plone Site object.

comment:12 Changed 7 years ago by keul

This can be a valid patch of my previous code?

if obj.objectIds():
    removePrefix(obj, root=False)
    if obj.meta_type == 'Plone Site':
        obj.reindexObjectSecurity()

comment:13 Changed 7 years ago by limi

  • Milestone changed from 2.5.3 to 2.5.x

comment:14 Changed 7 years ago by nd51

I can confirm too that this fix doesn't work (on Plone 2.5.3/Zope2.8.9.1). If you copy the external method into the Plone root and run it, it says "done" but doesn't actually fix the local roles. I also copied it to a subdirectory to see if running it from there would help. It consumed CPU cycles for a couple of minutes, but still hasn't fixed anything. This is quite a serious bug as it makes people get stuck with Plone 2.1.x until they fix it, add roles back by hand or rename groups.

comment:15 Changed 7 years ago by nd51

To make this work for us, I commented out offending lines 19 and 20 :

# if root == True and not obj.meta_type == 'Plone Site': # continue

This may slow it down but at least it actually works. Now that 3.0 is out and 2.1 is deprecated, this bug really needs fixing because it blocks upgrading from 2.1 to 2.5, and many large users of Plone are still on 2.1. Its not good if everyone has to hand edit this script before running it. And that assumes they were even lucky enough to stumble upon this ticket when trying to figure out why all their group roles are screwed after migration.

I intend to apply to get checkin permissions to the core in order to fix such bugs myself, but in the meantime it'd be nice if someone else could put this (or similar) fix in. :-)

comment:16 Changed 7 years ago by alecm

  • Status changed from reopened to closed
  • Resolution set to fixed

(In [17212]) Remove silly root check from stripGRUFLocalRolePrefix script fixes #5817

Note: See TracTickets for help on using tickets.