Frustration with permissions/security

211 views
Skip to first unread message

Roman

unread,
Oct 12, 2012, 5:01:05 AM10/12/12
to silverst...@googlegroups.com
Hello SilverStripe Developers

I'm posting this, because I'm actively using SS3 for client-work and I'm
having several gripes with permissions. As a pretext I have to mention,
that I usually don't give full Admin rights to clients. My permissions I
give to clients mostly look like the "Content Authors" settings that are
already present with a fresh SilverStripe install.

The first and foremost problem is, that Groups like the "Content
Authors" (SS3 default) are basically useless, because of the "PING" bug:
http://open.silverstripe.org/ticket/7915
(this can currently be circumvented by giving access to all CMS sections)

Then there was this issue, where users weren't able to access
SiteConfig, even when they had the proper permission set ("Manage site
configuration"):
http://open.silverstripe.org/ticket/7902
(this will be fixed in 3.0.3)

But there are plenty other things that bother me:

*Non-Admin users don't have CRUD permissions on DataObjects*
This might be a sensible default, but it's really cumbersome to override
the `canDelete` `canView` `canEdit` methods for every DataObject. More
so, there doesn't seem to be a useful existing permission to check in
these methods. If I check for `CMS_ACCESS_CMSMain` (which seems
reasonable), it works fine for the "Content Authors" account. If I chose
to switch to the "Access to all CMS Sections" permission (for example to
work around the "ping" bug), then the `CMS_ACCESS_CMSMain` permission is
gone. Instead the user will have `CMS_ACCESS_LeftAndMain`... and
apparently permissions don't cascade. While in reality,
`CMS_ACCESS_LeftAndMain` includes `CMS_ACCESS_CMSMain`, it doesn't work
when I check with `Requirements::check('CMS_ACCESS_CMSMain');`.

I know that I could create my own permissions and check for these. But
this all seems to be a lot of additional work which just wasn't there
with SS2.x

*Non-admin users don't have any Edit/Remove/Delete from Files Buttons on
Images*
When adding images to a page using the UploadField, then non-admin users
don't see any of the buttons to actually remove an image. So while they
can upload an image on a newly created page, they can't remove it or
replace it afterwards. This is only true for users who have the "Access
to all CMS sections" permission set. It works for the "Content Authors"
group... which leads me to the conclusion, that there is something
similar going on as described above: Most likely, UploadField checks the
`CMS_ACCESS_CMSMain` permission to decide whether or not to display the
buttons. But when "Access to all CMS sections" is granted to the user,
he actually doesn't have that permission, instead he gets
`CMS_ACCESS_LeftAndMain`.

These are the main gripes with security for me at the moment. I could go
an create some tickets for these issues, but I'd like to hear what the
developer community has to say about this. Are you guys working on the
permission system for an upcoming version of SilverStripe? Wouldn't it
be a good idea to have an existing permission to check whether or not a
user is logged in to the backend and can edit pages? Other ideas?

Best regards
- Roman

P.S. Sorry this turned out to be quite a lengthy post :(

Ingo Schommer

unread,
Oct 12, 2012, 9:01:41 AM10/12/12
to silverst...@googlegroups.com
Hey Roman,

On 12/10/2012, at 11:01 AM, Roman <bumm...@gmail.com> wrote:
> *Non-Admin users don't have CRUD permissions on DataObjects*
> This might be a sensible default, but it's really cumbersome to override
> the `canDelete` `canView` `canEdit` methods for every DataObject. More
> so, there doesn't seem to be a useful existing permission to check in
> these methods. If I check for `CMS_ACCESS_CMSMain` (which seems
> reasonable), it works fine for the "Content Authors" account. If I chose
> to switch to the "Access to all CMS Sections" permission (for example to
> work around the "ping" bug), then the `CMS_ACCESS_CMSMain` permission is
> gone. Instead the user will have `CMS_ACCESS_LeftAndMain`... and
> apparently permissions don't cascade. While in reality,
> `CMS_ACCESS_LeftAndMain` includes `CMS_ACCESS_CMSMain`, it doesn't work
> when I check with `Requirements::check('CMS_ACCESS_CMSMain');`.

There's Permission::$implied_permissions which seems to go in
the direction you're suggesting, although it was never functional …
which is a bit embarrassing. We should implement or remove it.

I don't see the point of distinguishing between ADMIN and "access to all CMS sections",
and would actually prefer to remove CMS_ACCESS_LeftAndMain.
We could replace it with a 1:1 mapping of all the CMS_ACCESS_* permission codes.
The UI could still look the same, but the checkbox would only trigger
all the other checkboxes, rather than saving a value itself.
This would mean that groups need to have permissions to newly added
CMS interfaces assigned manually, but that's acceptable IMHO.
>
> I know that I could create my own permissions and check for these. But
> this all seems to be a lot of additional work which just wasn't there
> with SS2.x
I don't think this aspect changed a whole lot since 2.x
(both requiring ADMIN for DataObject->can*() and the lacking CMS_ACCESS_* inheritance).
>
> *Non-admin users don't have any Edit/Remove/Delete from Files Buttons on
> Images*
> When adding images to a page using the UploadField, then non-admin users
> don't see any of the buttons to actually remove an image. So while they
> can upload an image on a newly created page, they can't remove it or
> replace it afterwards. This is only true for users who have the "Access
> to all CMS sections" permission set. It works for the "Content Authors"
> group... which leads me to the conclusion, that there is something
> similar going on as described above: Most likely, UploadField checks the
> `CMS_ACCESS_CMSMain` permission to decide whether or not to display the
> buttons. But when "Access to all CMS sections" is granted to the user,
> he actually doesn't have that permission, instead he gets
> `CMS_ACCESS_LeftAndMain`.
Actually, it checks File->can*(), which looks for the CMS_Access_AssetAdmin permission.
This isn't realistic, since files can be uploaded from all over the place (AssetAdmin, ModelAdmin, CMSMain, …).
Either we introduce a UPLOAD_FILES permission, or remove the permission checks
on File->can*() in favour of generic access checks via the controller routing.
This was the 2.4 behaviour by the way.

Thanks for your detailed and thoughtful post!
Ingo

Roman

unread,
Oct 12, 2012, 9:16:32 AM10/12/12
to silverst...@googlegroups.com
Hi Ingo

Thanks for the quick feedback.
If I think about it, your proposal about adding all permissions
separately (eg. just the selected ones or ALL the CMS panel permissions
when "access to all CMS sections" was chosen) seems to be a good solution.

Not sure if file-permission checks are a sensitive default. Maybe even a
user without any login (and therefore no permissions) would want to
upload a file? I think this should rather be handled on the controller
level?

Is there any way I can help here? Should I post a ticket for any of
these issues?

Cheers - Roman

On 10/12/12 3:01 PM, Ingo Schommer wrote:
> Hey Roman,
>
> On 12/10/2012, at 11:01 AM, Roman <bumm...@gmail.com> wrote:
>> *Non-Admin users don't have CRUD permissions on DataObjects*
>> This might be a sensible default, but it's really cumbersome to override
>> the `canDelete` `canView` `canEdit` methods for every DataObject. More
>> so, there doesn't seem to be a useful existing permission to check in
>> these methods. If I check for `CMS_ACCESS_CMSMain` (which seems
>> reasonable), it works fine for the "Content Authors" account. If I chose
>> to switch to the "Access to all CMS Sections" permission (for example to
>> work around the "ping" bug), then the `CMS_ACCESS_CMSMain` permission is
>> gone. Instead the user will have `CMS_ACCESS_LeftAndMain`... and
>> apparently permissions don't cascade. While in reality,
>> `CMS_ACCESS_LeftAndMain` includes `CMS_ACCESS_CMSMain`, it doesn't work
>> when I check with `Requirements::check('CMS_ACCESS_CMSMain');`.
> There's Permission::$implied_permissions which seems to go in
> the direction you're suggesting, although it was never functional �
> This isn't realistic, since files can be uploaded from all over the place (AssetAdmin, ModelAdmin, CMSMain, �).

Ingo Schommer

unread,
Oct 12, 2012, 9:42:30 AM10/12/12
to silverst...@googlegroups.com
Hello Roman,

I've created a pull request for the file issue at
Since it's an API change, this will have to wait until 3.1 though.

Ingo

the direction you're suggesting, although it was never functional …
This isn't realistic, since files can be uploaded from all over the place (AssetAdmin, ModelAdmin, CMSMain, …).

Either we introduce a UPLOAD_FILES permission, or remove the permission checks
on File->can*() in favour of generic access checks via the controller routing.
This was the 2.4 behaviour by the way.

Thanks for your detailed and thoughtful post!
Ingo


--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To post to this group, send email to silverst...@googlegroups.com.
To unsubscribe from this group, send email to silverstripe-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/silverstripe-dev?hl=en.


Marcus Nyeholt

unread,
Oct 12, 2012, 9:01:54 PM10/12/12
to silverst...@googlegroups.com

> Non-Admin users don't have CRUD permissions on DataObjects*
> This might be a sensible default, but it's really cumbersome to override
> the `canDelete` `canView` `canEdit` methods for every DataObject. More
> so, there doesn't seem to be a useful existing permission to check in
> these methods.

It's this very reason I created the restricted objects module - https://github.com/nyeholt/silverstripe-restrictedobjects . 

The way SS does access control is in two different ways, which somewhat cross over - the global permission definitions (eg CS_ACCESS_Main) and the per object permissions (can*() methods) which occasionally use the global permission definitions in their checks, but aren't always really considered. Basically, I find it to be a bit of a mess because none of the underlying querying methods actually check access before allowing you to do things (eg read queries don't check permissions, nor do write operations) unless you explicitly do it yourself. 

The way the restricted objects work is to break access down to a per-object level, and unless a user has an explicit 'read' permission they can't load the object. Permissions are the lowest level item (eg Read, Write, CreateChildren, Publish, Delete), which can be bundled together into a Role (eg Editor might have CreateChildren, Read, Write permissions, Manager might have those + Publish and Delete). A User or Group is then assigned a role to a particular object (or inherited down a tree of objects), which means they then have those lower level permissions for all those bits of content. 

This doesn't do anything at the CMS Section level for permissions, but provides a much more flexible way of doing permissions for any data object type. 
Reply all
Reply to author
Forward
0 new messages