Add feedback and tests

diff --git a/src/DI.php b/src/DI.php
index 1ba46e23edf37ccfc8dd0f2b331a443f1702bb18..9025feb470983355f0c19f73ed5102b8726042a1 100644 (file)
--- 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 c69f5e26290c79e642202ebcf5758fe3842dad19..eafb88db788ff895c43cf9fa452a9236e7526f0f 100644 (file)
--- 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 b02f0e4c838b4e971efc8b1fd577c55842762fe0..993fda05a50d3aea70f3c032de755b5c445920a3 100644 (file)
--- 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 9887ffeb11a8f5ab8aaa47f4443bc84966517778..84c7d5838d1d6da217869f22d6ee45531ec44c26 100644 (file)
--- 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 (file)
index 0000000..e404694
--- /dev/null
+++ b/tests/src/Security/PermissionSet/Entity/PermissionSetTest.php
@@ -0,0 +1,45 @@
+<?php
+
+namespace Friendica\Test\src\Security\PermissionSet\Entity;
+
+use Friendica\Security\PermissionSet\Entity\PermissionSet;
+use Friendica\Test\MockedTest;
+
+class PermissionSetTest extends MockedTest
+{
+       public function dateAllowedContacts()
+       {
+               return [
+                       'default' => [
+                               '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 (file)
index 0000000..1cb1c16
--- /dev/null
+++ b/tests/src/Security/PermissionSet/Factory/PermissionSetTest.php
@@ -0,0 +1,134 @@
+<?php
+
+namespace Friendica\Test\src\Security\PermissionSet\Factory;
+
+use Friendica\Security\PermissionSet\Factory\PermissionSet;
+use Friendica\Test\MockedTest;
+use Friendica\Util\ACLFormatter;
+use Psr\Log\NullLogger;
+
+class PermissionSetTest extends MockedTest
+{
+       /** @var PermissionSet */
+       protected $permissionSet;
+
+       protected function setUp(): void
+       {
+               parent::setUp();
+
+               $this->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);
+       }
+}