Skip to content

Commit

Permalink
Merge pull request #10819 from andrewandante/FEAT_add_only_individual…
Browse files Browse the repository at this point in the history 
…_users_inherited_permission

NEW add OnlyTheseMembers Inherited Permission type
  • Loading branch information
@GuySartorelli
GuySartorelli authored Jul 6, 2023
2 parents 730a03d + a03d0fd commit 62bd560
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 19 deletions.
7 changes: 5 additions & 2 deletions src/Forms/ListboxField.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,17 +248,20 @@ public function getValueArray()
}

$canary = reset($validValues);
$targetType = gettype($canary);
if (is_array($value) && count($value) > 0) {
$first = reset($value);
// sanity check the values - make sure strings get strings, ints get ints etc
if (gettype($canary) !== gettype($first)) {
if ($targetType !== gettype($first)) {
$replaced = [];
foreach ($value as $item) {
if (!is_array($item)) {
$item = json_decode($item, true);
}

if (isset($item['Value'])) {
if ($targetType === gettype($item)) {
$replaced[] = $item;
} elseif (isset($item['Value'])) {
$replaced[] = $item['Value'];
}
}
Expand Down
91 changes: 81 additions & 10 deletions src/Security/InheritedPermissions.php
View file
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
View file
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
View file
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
Loading

0 comments on commit 62bd560

Please sign in to comment.