Deleted project role is not recreated on modifyPermissions

enonic / xp

Enonic XP

https://enonic.com

GNU General Public License v3.0

201 stars 34 forks source link

Deleted project role is not recreated on modifyPermissions #8031

Closed alansemenov closed 4 years ago

alansemenov commented 4 years ago

After a project role is manually deleted, it won't be recreated after project permissions are modified. It works (and should still work) when project meta-data is updated, but it should happen on update of permissions as well.

sigdestad commented 4 years ago

We do not need any "automagic" behaviour for this. If people delete such a role, they can re-create it themselves imho?

sigdestad commented 4 years ago

Better just to fail with reasonable error message.

rymsha commented 4 years ago

I disagree that we recreate it if it was manually deleted. I don't understand why metadata updates creates deleted one back - this is a bug.

it opens up room for races (there could be two threads creating same role)
it gives less room for caching (without "recreation" we could assume if project is there - role is there and its nodeId is static)

alansemenov commented 4 years ago

We do not need any "automagic" behaviour for this. If people delete such a role, they can re-create it themselves imho?

You can't recreate a role with specific name pattern (cms.project.project-name.*) required of a project role.

We have to either forbid deleting project roles or recreate them on save. We cannot end up with a totally non-functioning project because someone accidentally removed a project role.

alansemenov commented 4 years ago

it opens up room for races (there could be two threads creating same role)

It's no different than two threads creating two new projects with the same name.

it gives less room for caching (without "recreation" we could assume if project is there - role is there and its nodeId is static)

We can't assume the role is there if it's super easy to delete it in the UI. I suggested to hide the project roles in the UI in the first place, but if we don't then we should at least make it very difficult to delete them.

rymsha commented 4 years ago

The best then would be to forbid deleting these internal roles at least from GUI.

sigdestad commented 4 years ago

Good idea, if deleting project cleans up properly. Let's not implement more than we need now, just make sure UI fails nicely if something is wrong i.e. saying it was unable to find role xyz.

sigdestad commented 4 years ago

Also Alan, I can create roles with any name easily

alansemenov commented 4 years ago

So, to sum up:

still allow deleting project roles
don't recreate project roles on update
give an error when at least one of the 5 project roles is not found. when? when opening the project?

alansemenov commented 4 years ago

We'll fool-proof this issue by disabling deletion of project roles in the UI: https://github.com/enonic/xp/issues/8035 https://github.com/enonic/app-users/issues/324