Consider reworking default access rules for editing pages.

[amolswnz]

Module version(s) affected

5.2.0

Description

Users with "View any page" can publish, unpublish, archive, edit any page.

How to reproduce

1. Create a Group
2. In Permissions tab select "Access to 'Pages' section"
3. Select "View any page"
4. Create a user in this group
5. Login with the newly created user
6. User is able to do all functions on pages which have same permissions as "Edit any page"

Possible Solution

This code seems to be the culprit in SiteTree.php

public function canEdit($member = null)
{
    ...
    // Check inherited permissions
    return static::getPermissionChecker()
        ->canEdit($this->ID, $member);

Additional Context

No response

Validations

• Check that there isn't already an issue that reports the same bug
• Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

[andrewandante]

I have tested and reproduced - you don't even need the View any page permission, you only need the Access to 'Pages' section permission and you have full edit rights on the pages.

Only having the View any page permission does correctly prevent your access though.

Edit: have just tested with files, the problem is not present there.

[andrewandante]

Tracing this through, I think the default permission set is to allow anyone you can log-in to the CMS to be able to edit pages on the site:

So falling through the canEdit() checks, it falls back to:

  return static::getPermissionChecker()
        ->canEdit($this->ID, $member);

which then eventually falls back to:

SiteConfig::current_site_config()->canEditPages($member);

which goes to Logged-In Users as the default type, set here: https://github.com/silverstripe/silverstripe-siteconfig/blob/5/code/SiteConfig.php#L49

class SiteConfig extends DataObject implements PermissionProvider, TemplateGlobalProvider
{
    private static $db = [
        "Title" => "Varchar(255)",
        "Tagline" => "Varchar(255)",
        "CanViewType" => "Enum('Anyone, LoggedInUsers, OnlyTheseUsers, OnlyTheseMembers', 'Anyone')",
        "CanEditType" => "Enum('LoggedInUsers, OnlyTheseUsers, OnlyTheseMembers', 'LoggedInUsers')",
        "CanCreateTopLevelType" => "Enum('LoggedInUsers, OnlyTheseUsers, OnlyTheseMembers', 'LoggedInUsers')",
    ];

So - this is working as intended, it seems - though the implication is that VIEW ANY PAGE without explicit EDIT ANY PAGE permission is that they should not be able to edit, where in fact, by default, they can. In essence, the VIEW ANY PAGE permission doesn't do anything, and in fact disguises the true access a user has, by default. This is the Bad Thing™️

In my mind there are a few options here:

a) change the default CanEditType to a group, set it to content-authors
b) explicitly check against users who have only the VIEW ANY PAGE permission and deny them
c) remove the SiteConfig thing entirely

There are probably others, but that's just off the top of my head.

In the meantime, the workaround is to change that top-level setting so that it's not inherited down.

[GuySartorelli]

Thank you for the investigation @andrewandante! As you noted, this is working as currently intended - though it's not the first time someone has reported this, so it's clearly a source of confusion and perhaps it's time we think about changing how this works.

@amolswnz If this wasn't working as intended, it would have been a security problem and should have been reported through the security process - please read that so you know what to do next time.

[GuySartorelli]

For now I'm going to change the title of this so that anyone who stumbles across it doesn't freak out.

[andrewandante]

I wonder if there's a way to rethink how permissions are represented on a page.

Even with the "inherited from above" permission, it's not that obvious actually who has permissions on that page.

Would be cool to see some sort of icon, or descriptive line, that tells the users exactly what the "computed" access is for the page. Something along the lines of:

• "This page is only viewable and editable by administrators"
• "This page is editable by anyone with CMS access (inherited from root)"
• "This page is only editable by select users (configured on this page) and viewable by anyone with CMS access (inherited from root)"

I'm pretty sure that's hard to surface though 😅

[amolswnz]

SiteConfig "Who can edit pages on this site?" is a double up on permissions. If we select "Only these groups", it lists "Groups with global edit permissions" which is saying that all groups with "Edit" permissions can edit pages on site 🔁