From 03164d00e8c6c36df0df6193f704393fa213beb2 Mon Sep 17 00:00:00 2001 From: Philipp Date: Thu, 7 Oct 2021 19:46:24 +0200 Subject: [PATCH] Add feedback and tests --- src/DI.php | 15 +- src/Model/ProfileField.php | 7 +- .../Depository/PermissionSet.php | 53 ++++--- .../PermissionSet/Factory/PermissionSet.php | 46 +++--- .../Entity/PermissionSetTest.php | 45 ++++++ .../Factory/PermissionSetTest.php | 134 ++++++++++++++++++ 6 files changed, 250 insertions(+), 50 deletions(-) create mode 100644 tests/src/Security/PermissionSet/Entity/PermissionSetTest.php create mode 100644 tests/src/Security/PermissionSet/Factory/PermissionSetTest.php diff --git a/src/DI.php b/src/DI.php index 1ba46e23e..9025feb47 100644 --- a/src/DI.php +++ b/src/DI.php @@ -22,7 +22,6 @@ namespace Friendica; use Dice\Dice; -use Friendica\Security\PermissionSet\Depository\PermissionSet; use Psr\Log\LoggerInterface; /** @@ -443,20 +442,14 @@ abstract class DI return self::$dice->create(Repository\Introduction::class); } - /** - * @return PermissionSet - */ - public static function permissionSet() + public static function permissionSet(): Security\PermissionSet\Depository\PermissionSet { - return self::$dice->create(PermissionSet::class); + return self::$dice->create(Security\PermissionSet\Depository\PermissionSet::class); } - /** - * @return \Friendica\Security\PermissionSet\Factory\PermissionSet - */ - public static function permissionSetFactory() + public static function permissionSetFactory(): Security\PermissionSet\Factory\PermissionSet { - return self::$dice->create(\Friendica\Security\PermissionSet\Factory\PermissionSet::class); + return self::$dice->create(Security\PermissionSet\Factory\PermissionSet::class); } /** diff --git a/src/Model/ProfileField.php b/src/Model/ProfileField.php index c69f5e262..eafb88db7 100644 --- a/src/Model/ProfileField.php +++ b/src/Model/ProfileField.php @@ -23,6 +23,7 @@ namespace Friendica\Model; use Friendica\BaseModel; use Friendica\Database\Database; +use Friendica\Security\PermissionSet\Depository\PermissionSet as PermissionSetDepository; use Friendica\Security\PermissionSet\Entity\PermissionSet; use Psr\Log\LoggerInterface; @@ -46,10 +47,10 @@ class ProfileField extends BaseModel /** @var PermissionSet */ private $permissionset; - /** @var \Friendica\Security\PermissionSet\Depository\PermissionSet */ + /** @var PermissionSetDepository */ private $permissionSetDepository; - public function __construct(Database $dba, LoggerInterface $logger,\Friendica\Security\PermissionSet\Depository\PermissionSet $permissionSetDepository, array $data = []) + public function __construct(Database $dba, LoggerInterface $logger, PermissionSetDepository $permissionSetDepository, array $data = []) { parent::__construct($dba, $logger, $data); @@ -62,7 +63,7 @@ class ProfileField extends BaseModel switch ($name) { case 'permissionset': - $this->permissionset = $this->permissionset ?? $this->permissionSetDepository->selectOneById($this->psid); + $this->permissionset = $this->permissionset ?? $this->permissionSetDepository->selectOneForUser($this->uid, $this->psid); $return = $this->permissionset; break; diff --git a/src/Security/PermissionSet/Depository/PermissionSet.php b/src/Security/PermissionSet/Depository/PermissionSet.php index b02f0e4c8..993fda05a 100644 --- a/src/Security/PermissionSet/Depository/PermissionSet.php +++ b/src/Security/PermissionSet/Depository/PermissionSet.php @@ -70,6 +70,24 @@ class PermissionSet extends BaseDepository return new Collection\PermissionSets(parent::_select($condition, $params)->getArrayCopy()); } + /** + * Converts a given PermissionSet into a DB compatible row array + * + * @param Entity\PermissionSet $permissionSet + * + * @return array + */ + protected function convertToTableRow(Entity\PermissionSet $permissionSet): array + { + return [ + 'uid' => $permissionSet->uid, + 'allow_cid' => $this->aclFormatter->toString($permissionSet->allow_cid), + 'allow_gid' => $this->aclFormatter->toString($permissionSet->allow_gid), + 'deny_cid' => $this->aclFormatter->toString($permissionSet->deny_cid), + 'deny_gid' => $this->aclFormatter->toString($permissionSet->deny_gid), + ]; + } + /** * @param int $id * @@ -159,6 +177,23 @@ class PermissionSet extends BaseDepository return $this->selectOrCreate($this->factory->createFromString($uid)); } + /** + * Fetch one PermissionSet with check for ownership + * + * @param int $uid The user id + * @param int $id The unique id of the PermissionSet + * + * @return Entity\PermissionSet + * @throws NotFoundException in case either the id is invalid or the PermissionSet does not relay to the given user + */ + public function selectOneForUser(int $uid, int $id): Entity\PermissionSet + { + return $this->selectOne([ + 'id' => $id, + 'uid' => $uid, + ]); + } + /** * Selects or creates a PermissionSet based on it's fields * @@ -172,16 +207,8 @@ class PermissionSet extends BaseDepository return $permissionSet; } - $fields = [ - 'uid' => $permissionSet->uid, - 'allow_cid' => $this->aclFormatter->toString($permissionSet->allow_cid), - 'allow_gid' => $this->aclFormatter->toString($permissionSet->allow_gid), - 'deny_cid' => $this->aclFormatter->toString($permissionSet->deny_cid), - 'deny_gid' => $this->aclFormatter->toString($permissionSet->deny_gid), - ]; - try { - return $this->selectOne($fields); + return $this->selectOne($this->convertToTableRow($permissionSet)); } catch (NotFoundException $exception) { return $this->save($permissionSet); } @@ -189,13 +216,7 @@ class PermissionSet extends BaseDepository public function save(Entity\PermissionSet $permissionSet): Entity\PermissionSet { - $fields = [ - 'uid' => $permissionSet->uid, - 'allow_cid' => $this->aclFormatter->toString($permissionSet->allow_cid), - 'allow_gid' => $this->aclFormatter->toString($permissionSet->allow_gid), - 'deny_cid' => $this->aclFormatter->toString($permissionSet->deny_cid), - 'deny_gid' => $this->aclFormatter->toString($permissionSet->deny_gid), - ]; + $fields = $this->convertToTableRow($permissionSet); if ($permissionSet->id) { $this->db->update(self::$table_name, $fields, ['id' => $permissionSet->id]); diff --git a/src/Security/PermissionSet/Factory/PermissionSet.php b/src/Security/PermissionSet/Factory/PermissionSet.php index 9887ffeb1..84c7d5838 100644 --- a/src/Security/PermissionSet/Factory/PermissionSet.php +++ b/src/Security/PermissionSet/Factory/PermissionSet.php @@ -20,39 +20,45 @@ class PermissionSet extends BaseFactory implements ICanCreateFromTableRow $this->formatter = $formatter; } + /** + * @inheritDoc + */ public function createFromTableRow(array $row): Entity\PermissionSet { return new Entity\PermissionSet( $row['uid'], - $this->formatter->expand($row['allow_cid'] ?? []), - $this->formatter->expand($row['allow_gid'] ?? []), - $this->formatter->expand($row['deny_cid'] ?? []), - $this->formatter->expand($row['deny_gid'] ?? []), + $this->formatter->expand($row['allow_cid'] ?? ''), + $this->formatter->expand($row['allow_gid'] ?? ''), + $this->formatter->expand($row['deny_cid'] ?? ''), + $this->formatter->expand($row['deny_gid'] ?? ''), $row['id'] ?? null ); } + /** + * Creates a new PermissionSet based on it's fields + * + * @param int $uid + * @param string $allow_cid + * @param string $allow_gid + * @param string $deny_cid + * @param string $deny_gid + * + * @return Entity\PermissionSet + */ public function createFromString( int $uid, string $allow_cid = '', string $allow_gid = '', string $deny_cid = '', - string $deny_gid = '') + string $deny_gid = ''): Entity\PermissionSet { - return new Entity\PermissionSet( - $uid, - $this->formatter->expand($allow_cid), - $this->formatter->expand($allow_gid), - $this->formatter->expand($deny_cid), - $this->formatter->expand($deny_gid) - ); - } - - public function createPrototypeForUser(int $uid, string $allowCid): Entity\PermissionSet - { - return new Entity\PermissionSet( - $uid, - $this->formatter->expand($allowCid) - ); + return $this->createFromTableRow([ + 'uid' => $uid, + 'allow_cid' => $allow_cid, + 'allow_gid' => $allow_gid, + 'deny_cid' => $deny_cid, + 'deny_gid' => $deny_gid, + ]); } } diff --git a/tests/src/Security/PermissionSet/Entity/PermissionSetTest.php b/tests/src/Security/PermissionSet/Entity/PermissionSetTest.php new file mode 100644 index 000000000..e404694d4 --- /dev/null +++ b/tests/src/Security/PermissionSet/Entity/PermissionSetTest.php @@ -0,0 +1,45 @@ + [ + 'id' => 10, + 'allow_cid' => ['1', '2'], + 'allow_gid' => ['3', '4'], + 'deny_cid' => ['5', '6', '7'], + 'deny_gid' => ['8'], + 'update_cid' => ['10'], + ], + ]; + } + + /** + * Test if the call "withAllowedContacts()" creates a clone + * + * @dataProvider dateAllowedContacts + */ + public function testWithAllowedContacts(int $id, array $allow_cid, array $allow_gid, array $deny_cid, array $deny_gid, array $update_cid) + { + $permissionSetOrig = new PermissionSet( + $id, + $allow_cid, + $allow_gid, + $deny_cid, + $deny_gid + ); + + $permissionSetNew = $permissionSetOrig->withAllowedContacts($update_cid); + + self::assertNotSame($permissionSetOrig, $permissionSetNew); + self::assertEquals($update_cid, $permissionSetNew->allow_cid); + self::assertEquals($allow_cid, $permissionSetOrig->allow_cid); + } +} diff --git a/tests/src/Security/PermissionSet/Factory/PermissionSetTest.php b/tests/src/Security/PermissionSet/Factory/PermissionSetTest.php new file mode 100644 index 000000000..1cb1c16ad --- /dev/null +++ b/tests/src/Security/PermissionSet/Factory/PermissionSetTest.php @@ -0,0 +1,134 @@ +permissionSet = new PermissionSet(new NullLogger(), new ACLFormatter()); + } + + public function dataInput() + { + return [ + 'new' => [ + 'input' => [ + 'uid' => 12, + 'allow_cid' => '<1>,<2>', + 'allow_gid' => '<3>,<4>', + 'deny_cid' => '<6>', + 'deny_gid' => '<8>', + ], + 'assertion' => [ + 'id' => null, + 'uid' => 12, + 'allow_cid' => ['1', '2'], + 'allow_gid' => ['3', '4'], + 'deny_cid' => ['6'], + 'deny_gid' => ['8'], + ], + ], + 'full' => [ + 'input' => [ + 'id' => 3, + 'uid' => 12, + 'allow_cid' => '<1>,<2>', + 'allow_gid' => '<3>,<4>', + 'deny_cid' => '<6>', + 'deny_gid' => '<8>', + ], + 'assertion' => [ + 'id' => 3, + 'uid' => 12, + 'allow_cid' => ['1', '2'], + 'allow_gid' => ['3', '4'], + 'deny_cid' => ['6'], + 'deny_gid' => ['8'], + ], + ], + 'mini' => [ + 'input' => [ + 'id' => null, + 'uid' => 12, + ], + 'assertion' => [ + 'id' => null, + 'uid' => 12, + 'allow_cid' => [], + 'allow_gid' => [], + 'deny_cid' => [], + 'deny_gid' => [], + ], + ], + 'wrong' => [ + 'input' => [ + 'id' => 3, + 'uid' => 12, + 'allow_cid' => '<1,<2>', + ], + 'assertion' => [ + 'id' => 3, + 'uid' => 12, + 'allow_cid' => ['2'], + 'allow_gid' => [], + 'deny_cid' => [], + 'deny_gid' => [], + ], + ] + ]; + } + + protected function assertPermissionSet(\Friendica\Security\PermissionSet\Entity\PermissionSet $permissionSet, array $assertion) + { + self::assertEquals($assertion['id'] ?? null, $permissionSet->id); + self::assertNotNull($permissionSet->uid); + self::assertEquals($assertion['uid'], $permissionSet->uid); + self::assertEquals($assertion['allow_cid'], $permissionSet->allow_cid); + self::assertEquals($assertion['allow_gid'], $permissionSet->allow_gid); + self::assertEquals($assertion['deny_cid'], $permissionSet->deny_cid); + self::assertEquals($assertion['deny_gid'], $permissionSet->deny_gid); + } + + /** + * Test the createFromTableRow method + * + * @dataProvider dataInput + */ + public function testCreateFromTableRow(array $input, array $assertion) + { + $permissionSet = $this->permissionSet->createFromTableRow($input); + + $this->assertPermissionSet($permissionSet, $assertion); + } + + /** + * Test the createFromString method + * + * @dataProvider dataInput + */ + public function testCreateFromString(array $input, array $assertion) + { + $permissionSet = $this->permissionSet->createFromString( + $input['uid'], + $input['allow_cid'] ?? '', + $input['allow_gid'] ?? '', + $input['deny_cid'] ?? '', + $input['deny_gid'] ?? '' + ); + + unset($assertion['id']); + + $this->assertPermissionSet($permissionSet, $assertion); + } +}