Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEW add OnlyTheseMembers Inherited Permission type #10819

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 81 additions & 10 deletions src/Security/InheritedPermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use SilverStripe\Versioned\Versioned;
use Psr\SimpleCache\CacheInterface;
use SilverStripe\Core\Cache\MemberCacheFlusher;
use SilverStripe\Dev\Deprecation;

/**
* Calculates batch permissions for nested objects for:
Expand All @@ -25,37 +26,42 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher
/**
* Delete permission
*/
const DELETE = 'delete';
public const DELETE = 'delete';

/**
* View permission
*/
const VIEW = 'view';
public const VIEW = 'view';

/**
* Edit permission
*/
const EDIT = 'edit';
public const EDIT = 'edit';

/**
* Anyone canView permission
*/
const ANYONE = 'Anyone';
public const ANYONE = 'Anyone';

/**
* Restrict to logged in users
*/
const LOGGED_IN_USERS = 'LoggedInUsers';
public const LOGGED_IN_USERS = 'LoggedInUsers';

/**
* Restrict to specific groups
*/
const ONLY_THESE_USERS = 'OnlyTheseUsers';
public const ONLY_THESE_USERS = 'OnlyTheseUsers';

/**
* Restrict to specific members
*/
public const ONLY_THESE_MEMBERS = 'OnlyTheseMembers';

/**
* Inherit from parent
*/
const INHERIT = 'Inherit';
public const INHERIT = 'Inherit';

/**
* Class name
Expand Down Expand Up @@ -359,19 +365,27 @@ protected function batchPermissionCheckForStage(
if ($member && $member->ID) {
if (!Permission::checkMember($member, 'ADMIN')) {
// Determine if this member matches any of the group or other rules
$groupJoinTable = $this->getJoinTable($type);
$groupJoinTable = $this->getGroupJoinTable($type);
$memberJoinTable = $this->getMemberJoinTable($type);
$uninheritedPermissions = $stageRecords
->where([
"(\"$typeField\" IN (?, ?) OR " . "(\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL))"
"(\"$typeField\" IN (?, ?)"
. " OR (\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL)"
. " OR (\"$typeField\" = ? AND \"$memberJoinTable\".\"{$baseTable}ID\" IS NOT NULL)"
. ")"
=> [
self::ANYONE,
self::LOGGED_IN_USERS,
self::ONLY_THESE_USERS
self::ONLY_THESE_USERS,
self::ONLY_THESE_MEMBERS,
]
])
->leftJoin(
$groupJoinTable,
"\"$groupJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " . "\"$groupJoinTable\".\"GroupID\" IN ($groupIDsSQLList)"
)->leftJoin(
$memberJoinTable,
"\"$memberJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " . "\"$memberJoinTable\".\"MemberID\" = {$member->ID}"
)->column('ID');
} else {
$uninheritedPermissions = $stageRecords->column('ID');
Expand Down Expand Up @@ -628,10 +642,24 @@ protected function getPermissionField($type)
* Get join table for type
* Defaults to those provided by {@see InheritedPermissionsExtension)
*
* @deprecated 5.1.0 Use getGroupJoinTable() instead
* @param string $type
* @return string
*/
protected function getJoinTable($type)
{
Deprecation::notice('5.1.0', 'Use getGroupJoinTable() instead');
return $this->getGroupJoinTable($type);
}

/**
* Get group join table for type
* Defaults to those provided by {@see InheritedPermissionsExtension)
*
* @param string $type
* @return string
*/
protected function getGroupJoinTable($type)
{
switch ($type) {
case self::DELETE:
Expand All @@ -645,6 +673,27 @@ protected function getJoinTable($type)
}
}

/**
* Get member join table for type
* Defaults to those provided by {@see InheritedPermissionsExtension)
*
* @param string $type
* @return string
*/
protected function getMemberJoinTable($type)
{
switch ($type) {
case self::DELETE:
// Delete uses edit type - Drop through
case self::EDIT:
return $this->getEditorMembersTable();
case self::VIEW:
return $this->getViewerMembersTable();
default:
throw new InvalidArgumentException("Invalid argument type $type");
}
}

/**
* Determine default permission for a givion check
*
Expand Down Expand Up @@ -716,6 +765,28 @@ protected function getViewerGroupsTable()
return "{$table}_ViewerGroups";
}

/**
* Get table to use for editor members relation
*
* @return string
*/
protected function getEditorMembersTable()
{
$table = DataObject::getSchema()->tableName($this->baseClass);
return "{$table}_EditorMembers";
}

/**
* Get table to use for viewer members relation
*
* @return string
*/
protected function getViewerMembersTable()
{
$table = DataObject::getSchema()->tableName($this->baseClass);
return "{$table}_ViewerMembers";
}

/**
* Gets the permission from cache
*
Expand Down
10 changes: 8 additions & 2 deletions src/Security/InheritedPermissionsExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,21 @@
* @property string $CanEditType
* @method ManyManyList ViewerGroups()
* @method ManyManyList EditorGroups()
* @method ManyManyList ViewerMembers()
* @method ManyManyList EditorMembers()
*/
class InheritedPermissionsExtension extends DataExtension
{
private static array $db = [
'CanViewType' => "Enum('Anyone, LoggedInUsers, OnlyTheseUsers, Inherit', 'Inherit')",
'CanEditType' => "Enum('LoggedInUsers, OnlyTheseUsers, Inherit', 'Inherit')",
'CanViewType' => "Enum('Anyone, LoggedInUsers, OnlyTheseUsers, OnlyTheseMembers, Inherit', 'Inherit')",
'CanEditType' => "Enum('LoggedInUsers, OnlyTheseUsers, OnlyTheseMembers, Inherit', 'Inherit')",
];

private static array $many_many = [
'ViewerGroups' => Group::class,
'EditorGroups' => Group::class,
'ViewerMembers' => Member::class,
'EditorMembers' => Member::class,
];

private static array $defaults = [
Expand All @@ -33,5 +37,7 @@ class InheritedPermissionsExtension extends DataExtension
private static array $cascade_duplicates = [
'ViewerGroups',
'EditorGroups',
'ViewerMembers',
'EditorMembers',
];
}
41 changes: 36 additions & 5 deletions tests/php/Security/InheritedPermissionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,20 @@ protected function tearDown(): void
public function testEditPermissions()
{
$editor = $this->objFromFixture(Member::class, 'editor');
$freddie = $this->objFromFixture(Member::class, 'oneFileFreddie');

$about = $this->objFromFixture(TestPermissionNode::class, 'about');
$aboutStaff = $this->objFromFixture(TestPermissionNode::class, 'about-staff');
$history = $this->objFromFixture(TestPermissionNode::class, 'history');
$products = $this->objFromFixture(TestPermissionNode::class, 'products');
$product1 = $this->objFromFixture(TestPermissionNode::class, 'products-product1');
$product4 = $this->objFromFixture(TestPermissionNode::class, 'products-product4');
$freddiesFile = $this->objFromFixture(TestPermissionNode::class, 'freddies-file');

// Test logged out users cannot edit
Member::actAs(null, function () use ($aboutStaff) {
Member::actAs(null, function () use ($aboutStaff, $freddiesFile) {
$this->assertFalse($aboutStaff->canEdit());
$this->assertFalse($freddiesFile->canEdit());
});

// Can't edit a page that is locked to admins
Expand All @@ -96,6 +99,10 @@ public function testEditPermissions()
// Test that root node respects root permissions
$this->assertTrue($history->canEdit($editor));

// Test that only Freddie can edit Freddie's file
$this->assertFalse($freddiesFile->canEdit($editor));
$this->assertTrue($freddiesFile->canEdit($freddie));

TestPermissionNode::getInheritedPermissions()->clearCache();
$this->rootPermissions->setCanEdit(false);

Expand All @@ -106,17 +113,20 @@ public function testEditPermissions()
public function testDeletePermissions()
{
$editor = $this->objFromFixture(Member::class, 'editor');
$freddie = $this->objFromFixture(Member::class, 'oneFileFreddie');

$about = $this->objFromFixture(TestPermissionNode::class, 'about');
$aboutStaff = $this->objFromFixture(TestPermissionNode::class, 'about-staff');
$history = $this->objFromFixture(TestPermissionNode::class, 'history');
$products = $this->objFromFixture(TestPermissionNode::class, 'products');
$product1 = $this->objFromFixture(TestPermissionNode::class, 'products-product1');
$product4 = $this->objFromFixture(TestPermissionNode::class, 'products-product4');
$freddiesFile = $this->objFromFixture(TestPermissionNode::class, 'freddies-file');

// Test logged out users cannot edit
Member::actAs(null, function () use ($aboutStaff) {
Member::actAs(null, function () use ($aboutStaff, $freddiesFile) {
$this->assertFalse($aboutStaff->canDelete());
$this->assertFalse($freddiesFile->canDelete());
});

// Can't edit a page that is locked to admins
Expand All @@ -134,6 +144,10 @@ public function testDeletePermissions()
// Test that root node respects root permissions
$this->assertTrue($history->canDelete($editor));

// Test that only Freddie can delete Freddie's file
$this->assertFalse($freddiesFile->canDelete($editor));
$this->assertTrue($freddiesFile->canDelete($freddie));

TestPermissionNode::getInheritedPermissions()->clearCache();
$this->rootPermissions->setCanEdit(false);

Expand All @@ -150,14 +164,17 @@ public function testViewPermissions()
$secretNested = $this->objFromFixture(TestPermissionNode::class, 'secret-nested');
$protected = $this->objFromFixture(TestPermissionNode::class, 'protected');
$protectedChild = $this->objFromFixture(TestPermissionNode::class, 'protected-child');
$editor = $this->objFromFixture(Member::class, 'editor');
$restricted = $this->objFromFixture(TestPermissionNode::class, 'restricted-page');
$freddiesFile = $this->objFromFixture(TestPermissionNode::class, 'freddies-file');

$editor = $this->objFromFixture(Member::class, 'editor');
$admin = $this->objFromFixture(Member::class, 'admin');
$freddie = $this->objFromFixture(Member::class, 'oneFileFreddie');

// Not logged in user can only access Inherit or Anyone pages
Member::actAs(
null,
function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm) {
function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm, $freddiesFile) {
$this->assertTrue($history->canView());
$this->assertTrue($contact->canView());
$this->assertTrue($contactForm->canView());
Expand All @@ -166,6 +183,7 @@ function () use ($protectedChild, $secretNested, $protected, $secret, $history,
$this->assertFalse($secretNested->canView());
$this->assertFalse($protected->canView());
$this->assertFalse($protectedChild->canView());
$this->assertFalse($freddiesFile->canView());
}
);

Expand All @@ -180,6 +198,10 @@ function () use ($protectedChild, $secretNested, $protected, $secret, $history,
// Check root permissions
$this->assertTrue($history->canView($editor));

// Test that only Freddie can view Freddie's file
$this->assertFalse($freddiesFile->canView($editor));
$this->assertTrue($freddiesFile->canView($freddie));

TestPermissionNode::getInheritedPermissions()->clearCache();
$this->rootPermissions->setCanView(false);

Expand All @@ -198,12 +220,16 @@ public function testUnstagedViewPermissions()
$secretNested = $this->objFromFixture(UnstagedNode::class, 'secret-nested');
$protected = $this->objFromFixture(UnstagedNode::class, 'protected');
$protectedChild = $this->objFromFixture(UnstagedNode::class, 'protected-child');
$freddiesFile = $this->objFromFixture(UnstagedNode::class, 'freddies-file');

$editor = $this->objFromFixture(Member::class, 'editor');
$freddie = $this->objFromFixture(Member::class, 'oneFileFreddie');


// Not logged in user can only access Inherit or Anyone pages
Member::actAs(
null,
function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm) {
function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm, $freddiesFile) {
$this->assertTrue($history->canView());
$this->assertTrue($contact->canView());
$this->assertTrue($contactForm->canView());
Expand All @@ -212,6 +238,7 @@ function () use ($protectedChild, $secretNested, $protected, $secret, $history,
$this->assertFalse($secretNested->canView());
$this->assertFalse($protected->canView());
$this->assertFalse($protectedChild->canView());
$this->assertFalse($freddiesFile->canView());
}
);

Expand All @@ -226,6 +253,10 @@ function () use ($protectedChild, $secretNested, $protected, $secret, $history,
// Check root permissions
$this->assertTrue($history->canView($editor));

// Test that only Freddie can view Freddie's file
$this->assertFalse($freddiesFile->canView($editor));
$this->assertTrue($freddiesFile->canView($freddie));

UnstagedNode::getInheritedPermissions()->clearCache();
$this->rootPermissions->setCanView(false);

Expand Down
16 changes: 16 additions & 0 deletions tests/php/Security/InheritedPermissionsTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ SilverStripe\Security\Member:
Groups: =>SilverStripe\Security\Group.allsections
securityadmin:
Groups: =>SilverStripe\Security\Group.securityadmins
oneFileFreddie:
FirstName: Freddie
Surname: Fantastic
Groups: =>SilverStripe\Security\Group.editors

SilverStripe\Security\Tests\InheritedPermissionsTest\TestPermissionNode:
about:
Expand Down Expand Up @@ -104,6 +108,12 @@ SilverStripe\Security\Tests\InheritedPermissionsTest\TestPermissionNode:
Title: Restricted Page
CanViewType: OnlyTheseUsers
ViewerGroups: =>SilverStripe\Security\Group.allsections
freddies-file:
Title: Freddies File
CanViewType: OnlyTheseMembers
CanEditType: OnlyTheseMembers
ViewerMembers: =>SilverStripe\Security\Member.oneFileFreddie
EditorMembers: =>SilverStripe\Security\Member.oneFileFreddie

SilverStripe\Security\Tests\InheritedPermissionsTest\UnstagedNode:
about:
Expand Down Expand Up @@ -175,3 +185,9 @@ SilverStripe\Security\Tests\InheritedPermissionsTest\UnstagedNode:
Title: Restricted Page
CanViewType: OnlyTheseUsers
ViewerGroups: =>SilverStripe\Security\Group.allsections
freddies-file:
Title: Freddies File
CanViewType: OnlyTheseMembers
CanEditType: OnlyTheseMembers
ViewerMembers: =>SilverStripe\Security\Member.oneFileFreddie
EditorMembers: =>SilverStripe\Security\Member.oneFileFreddie