plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
246 stars 186 forks source link

Resource Registries visible to Site Administrator #961

Closed hvelarde closed 6 years ago

hvelarde commented 9 years ago

IMO, this is huge risk as the resource registry is pretty technical and is going to be easy for a Site Administrator to mess things up and ending with something broken:

selection_004

This option should only be visible to Managers.

polyester commented 9 years ago

Debatable. Having access to LESS variables is very useful to site administrators. Taking that away would probably mean more people would be given "manager" role.

It probably comes down to semantics and definition of roles. <insert obligatory 'with great power comes great responsability' Spiderman quote>

hvelarde commented 9 years ago

then Site Administrator role should have access only to those features; I just tested and, for instance, I was able to delete the Plone bundle (luckily, that seems not to work probably because is protected in code, @vangheem?)

if we decide to leave that option there, then the Site Administrator shouldn't be able to do any harm to the site: no access to changing to development mode or to remove resources or LESS variables, only to change them. also, we need to add more help to the screens so the user knows what to expect and how to proceed.

another example: I was able to change the value of the font used on the toolbar, but it isn't obvious that the user has to rebuild the Plone resource bundle after that.

the Site Administrator role is mostly a non-technical user that have no idea of what RequireJS and LESS are, and they mostly wouldn't care neither.

polyester commented 9 years ago

It feels inconsistent to me to allow Site Admins to access the Theming (which, using Diazo, can do quite some damage as well as good) and not to the LESS variables of the default theme. They could then just copy over the theme and do exactly the same TTW.

So, I do see a good need to document the functions (and dangers) of using Diazo, and using LESS. Pull requests very much welcome!!

But unless you want to forbid Site Admins to have access to theming, they will still be able to disable a site. Although maybe a "whoops, I messed up, reset everything to default" panic option would be nice.

jensens commented 6 years ago

Permission is https://github.com/plone/Products.CMFPlone/blob/0d8155f4d0d526f7b0cd67a9a3750dd5fb66ed20/Products/CMFPlone/controlpanel/browser/configure.zcml#L139 If this is a problem, we can introduce a new permission just for this controlpanel in order to have fine grained control. Please submit a PR.

hvelarde commented 6 years ago

I think @polyester made a point about messing around the theme; anyway, I'm not interested in this and I still consider the resource registries must die in the future.