diff --git a/config/packages/doctrine.yaml b/config/packages/doctrine.yaml index 44ea77f3..72dd35dc 100644 --- a/config/packages/doctrine.yaml +++ b/config/packages/doctrine.yaml @@ -13,6 +13,8 @@ doctrine: class: App\Helpers\UTCDateTimeType big_decimal: class: App\Helpers\BigDecimalType + permission_data: + class: App\Doctrine\Types\PermissionDataType schema_filter: ~^(?!internal)~ # Only enable this when needed diff --git a/src/Doctrine/Types/PermissionDataType.php b/src/Doctrine/Types/PermissionDataType.php new file mode 100644 index 00000000..b33e4545 --- /dev/null +++ b/src/Doctrine/Types/PermissionDataType.php @@ -0,0 +1,34 @@ +getName(), $e); + } + } + + public function getName(): string + { + return 'permission_data'; + } +} \ No newline at end of file diff --git a/src/Entity/UserSystem/Group.php b/src/Entity/UserSystem/Group.php index 94bdb8e1..ce22396e 100644 --- a/src/Entity/UserSystem/Group.php +++ b/src/Entity/UserSystem/Group.php @@ -95,11 +95,17 @@ class Group extends AbstractStructuralDBElement implements HasPermissionsInterfa */ protected $attachments; + /** + * @var PermissionData + * @ORM\Column(type="permission_data", nullable=false, name="permissions") + */ + protected PermissionData $permissions; + /** @var PermissionsEmbed * @ORM\Embedded(class="PermissionsEmbed", columnPrefix="perms_") * @ValidPermission() */ - protected $permissions; + protected $permissions_old; /** @var Collection * @ORM\OneToMany(targetEntity="App\Entity\Parameters\GroupParameter", mappedBy="element", cascade={"persist", "remove"}, orphanRemoval=true) @@ -111,7 +117,8 @@ class Group extends AbstractStructuralDBElement implements HasPermissionsInterfa public function __construct() { parent::__construct(); - $this->permissions = new PermissionsEmbed(); + $this->permissions = new PermissionData(); + $this->permissions_old = new PermissionsEmbed(); $this->users = new ArrayCollection(); } @@ -137,8 +144,12 @@ class Group extends AbstractStructuralDBElement implements HasPermissionsInterfa return $this; } - public function getPermissions(): PermissionsEmbed + public function getPermissions(): PermissionData { + if (!isset($this->permissions)) { + $this->permissions = new PermissionData(); + } + return $this->permissions; } diff --git a/src/Entity/UserSystem/PermissionData.php b/src/Entity/UserSystem/PermissionData.php new file mode 100644 index 00000000..06afd3ac --- /dev/null +++ b/src/Entity/UserSystem/PermissionData.php @@ -0,0 +1,116 @@ + [ + * operation => value, + * ] + */ + protected array $data = []; + + /** + * Creates a new Permission Data Instance using the given data. + * By default, a empty array is used, meaning + */ + public function __construct(array $data = []) + { + $this->data = $data; + } + + /** + * Check if a permission value is set for the given permission and operation (meaning there value is not inherit). + * @param string $permission + * @param string $operation + * @return bool True if the permission value is set, false otherwise + */ + public function isPermissionSet(string $permission, string $operation): bool + { + return isset($this->data[$permission][$operation]); + } + + /** + * Returns the permission value for the given permission and operation. + * @param string $permission + * @param string $operation + * @return bool|null True means allow, false means disallow, null means inherit + */ + public function getPermissionValue(string $permission, string $operation): ?bool + { + if ($this->isPermissionSet($permission, $operation)) { + return $this->data[$permission][$operation]; + } + + //If the value is not set explicitly, return null (meaning inherit) + return null; + } + + /** + * Sets the permission value for the given permission and operation. + * @param string $permission + * @param string $operation + * @param bool|null $value + * @return $this + */ + public function setPermissionValue(string $permission, string $operation, ?bool $value): self + { + if ($value === null) { + //If the value is null, unset the permission value (meaning implicit inherit) + unset($this->data[$permission][$operation]); + } else { + //Otherwise, set the pemission value + if(!isset($this->data[$permission])) { + $this->data[$permission] = []; + } + $this->data[$permission][$operation] = $value; + } + + return $this; + } + + /** + * Creates a new Permission Data Instance using the given JSON encoded data + * @param string $json + * @return static + * @throws \JsonException + */ + public static function fromJSON(string $json): self + { + $data = json_decode($json, true, 512, JSON_THROW_ON_ERROR); + return new self($data); + } + + /** + * Returns an JSON encodable representation of this object. + * @return array|mixed + */ + public function jsonSerialize() + { + $ret = []; + + //Filter out all empty or null values + foreach ($this->data as $permission => $operations) { + $ret[$permission] = array_filter($operations, function ($value) { + return $value !== null; + }); + + //If the permission has no operations, unset it + if (empty($ret[$permission])) { + unset($ret[$permission]); + } + } + + return $ret; + } +} \ No newline at end of file diff --git a/src/Entity/UserSystem/User.php b/src/Entity/UserSystem/User.php index b651d679..869bce89 100644 --- a/src/Entity/UserSystem/User.php +++ b/src/Entity/UserSystem/User.php @@ -265,11 +265,17 @@ class User extends AttachmentContainingDBElement implements UserInterface, HasPe */ protected $currency; + /** + * @var PermissionData + * @ORM\Column(type="permission_data", nullable=false, name="permissions") + */ + protected PermissionData $permissions; + /** @var PermissionsEmbed * @ORM\Embedded(class="PermissionsEmbed", columnPrefix="perms_") * @ValidPermission() */ - protected $permissions; + protected $permissions_old; /** * @var DateTime the time until the password reset token is valid @@ -280,7 +286,7 @@ class User extends AttachmentContainingDBElement implements UserInterface, HasPe public function __construct() { parent::__construct(); - $this->permissions = new PermissionsEmbed(); + $this->permissions = new PermissionData(); $this->u2fKeys = new ArrayCollection(); $this->webauthn_keys = new ArrayCollection(); } @@ -427,8 +433,12 @@ class User extends AttachmentContainingDBElement implements UserInterface, HasPe return $this; } - public function getPermissions(): PermissionsEmbed + public function getPermissions(): PermissionData { + if (!isset($this->permissions)) { + $this->permissions = new PermissionData(); + } + return $this->permissions; } diff --git a/src/Security/Interfaces/HasPermissionsInterface.php b/src/Security/Interfaces/HasPermissionsInterface.php index 323eef3e..10b0a31c 100644 --- a/src/Security/Interfaces/HasPermissionsInterface.php +++ b/src/Security/Interfaces/HasPermissionsInterface.php @@ -42,9 +42,9 @@ declare(strict_types=1); namespace App\Security\Interfaces; -use App\Entity\UserSystem\PermissionsEmbed; +use App\Entity\UserSystem\PermissionData; interface HasPermissionsInterface { - public function getPermissions(): PermissionsEmbed; + public function getPermissions(): PermissionData; } diff --git a/src/Services/PermissionResolver.php b/src/Services/PermissionResolver.php index d6eedfb7..641d93f8 100644 --- a/src/Services/PermissionResolver.php +++ b/src/Services/PermissionResolver.php @@ -81,6 +81,7 @@ class PermissionResolver * Check if a user/group is allowed to do the specified operation for the permission. * * See permissions.yaml for valid permission operation combinations. + * This function does not check, if the permission is valid! * * @param HasPermissionsInterface $user the user/group for which the operation should be checked * @param string $permission the name of the permission for which should be checked @@ -92,18 +93,13 @@ class PermissionResolver public function dontInherit(HasPermissionsInterface $user, string $permission, string $operation): ?bool { //Get the permissions from the user - $perm_list = $user->getPermissions(); - - //Determine bit number using our configuration - $bit = $this->permission_structure['perms'][$permission]['operations'][$operation]['bit']; - - return $perm_list->getPermissionValue($permission, $bit); + return $user->getPermissions()->getPermissionValue($permission, $operation); } /** * Checks if a user is allowed to do the specified operation for the permission. - * In contrast to dontInherit() it tries to resolve the inherit values, of the user, by going upwards in the - * hierachy (user -> group -> parent group -> so on). But even in this case it is possible, that the inherit value + * In contrast to dontInherit() it tries to resolve to inherit values, of the user, by going upwards in the + * hierarchy (user -> group -> parent group -> so on). But even in this case it is possible, that to inherit value * could be resolved, and this function returns null. * * In that case the voter should set it manually to false by using ?? false. @@ -153,10 +149,12 @@ class PermissionResolver //Get the permissions from the user $perm_list = $user->getPermissions(); - //Determine bit number using our configuration - $bit = $this->permission_structure['perms'][$permission]['operations'][$operation]['bit']; + //Check if the permission/operation combination is valid + if (! $this->isValidOperation($permission, $operation)) { + throw new InvalidArgumentException('The permission/operation combination is not valid!'); + } - $perm_list->setPermissionValue($permission, $bit, $new_val); + $perm_list->setPermissionValue($permission, $operation, $new_val); } /** diff --git a/tests/Entity/UserSystem/PermissionDataTest.php b/tests/Entity/UserSystem/PermissionDataTest.php new file mode 100644 index 00000000..22f685b8 --- /dev/null +++ b/tests/Entity/UserSystem/PermissionDataTest.php @@ -0,0 +1,104 @@ +assertNull($perm_data->getPermissionValue('not_existing', 'not_existing')); + $this->assertFalse($perm_data->isPermissionSet('not_existing', 'not_existing')); + + $this->assertNull($perm_data->getPermissionValue('p1', 'op1')); + $this->assertNull($perm_data->getPermissionValue('p1', 'op2')); + $this->assertNull($perm_data->getPermissionValue('p2', 'op1')); + + //Set values + $perm_data->setPermissionValue('p1', 'op1', PermissionData::ALLOW); + $perm_data->setPermissionValue('p1', 'op2', PermissionData::DISALLOW); + $perm_data->setPermissionValue('p2', 'op1', PermissionData::ALLOW); + + //Check that values were set + $this->assertTrue($perm_data->isPermissionSet('p1', 'op1')); + $this->assertTrue($perm_data->isPermissionSet('p1', 'op2')); + $this->assertTrue($perm_data->isPermissionSet('p2', 'op1')); + + //Check that values are correct + $this->assertTrue($perm_data->getPermissionValue('p1', 'op1')); + $this->assertFalse($perm_data->getPermissionValue('p1', 'op2')); + $this->assertTrue($perm_data->getPermissionValue('p2', 'op1')); + + //Set values to null + $perm_data->setPermissionValue('p1', 'op1', null); + $this->assertNull($perm_data->getPermissionValue('p1', 'op1')); + //Values should be unset now + $this->assertFalse($perm_data->isPermissionSet('p1', 'op1')); + } + + public function testJSONSerialization() + { + $perm_data = new PermissionData(); + + $perm_data->setPermissionValue('perm1', 'op1', PermissionData::ALLOW); + $perm_data->setPermissionValue('perm1', 'op2', PermissionData::DISALLOW); + $perm_data->setPermissionValue('perm1', 'op3', PermissionData::ALLOW); + + $perm_data->setPermissionValue('perm2', 'op1', PermissionData::ALLOW); + $perm_data->setPermissionValue('perm2', 'op2', PermissionData::DISALLOW); + + //Ensure that JSON serialization works + $this->assertJsonStringEqualsJsonString(json_encode([ + 'perm1' => [ + 'op1' => true, + 'op2' => false, + 'op3' => true, + ], + 'perm2' => [ + 'op1' => true, + 'op2' => false, + ], + ], JSON_THROW_ON_ERROR), json_encode($perm_data, JSON_THROW_ON_ERROR)); + + //Set values to inherit to ensure they do not show up in the json + $perm_data->setPermissionValue('perm1', 'op3', null); + $perm_data->setPermissionValue('perm2', 'op1', null); + $perm_data->setPermissionValue('perm2', 'op2', null); + + //Ensure that JSON serialization works + $this->assertJsonStringEqualsJsonString(json_encode([ + 'perm1' => [ + 'op1' => true, + 'op2' => false, + ], + ], JSON_THROW_ON_ERROR), json_encode($perm_data, JSON_THROW_ON_ERROR)); + + } + + public function testFromJSON() + { + $json = json_encode([ + 'perm1' => [ + 'op1' => true, + 'op2' => false, + 'op3' => true, + ], + 'perm2' => [ + 'op1' => true, + 'op2' => false, + ], + ], JSON_THROW_ON_ERROR); + + $perm_data = PermissionData::fromJSON($json); + + //Ensure that values were set correctly + $this->assertTrue($perm_data->getPermissionValue('perm1', 'op1')); + $this->assertFalse($perm_data->getPermissionValue('perm2', 'op2')); + } +} diff --git a/tests/Services/PermissionResolverTest.php b/tests/Services/PermissionResolverTest.php index 1ca02b6d..cbb25624 100644 --- a/tests/Services/PermissionResolverTest.php +++ b/tests/Services/PermissionResolverTest.php @@ -43,6 +43,7 @@ declare(strict_types=1); namespace App\Tests\Services; use App\Entity\UserSystem\Group; +use App\Entity\UserSystem\PermissionData; use App\Entity\UserSystem\PermissionsEmbed; use App\Entity\UserSystem\User; use App\Services\PermissionResolver; @@ -69,42 +70,42 @@ class PermissionResolverTest extends WebTestCase $this->service = self::$container->get(PermissionResolver::class); //Set up a mocked user - $user_embed = new PermissionsEmbed(); - $user_embed->setPermissionValue('parts', 0, true) //read - ->setPermissionValue('parts', 2, false) //edit - ->setPermissionValue('parts', 4, null) //create - ->setPermissionValue('parts', 30, null) //move - ->setPermissionValue('parts', 8, null); //delete + $user_perms = new PermissionData(); + $user_perms->setPermissionValue('parts', 'read', true) //read + ->setPermissionValue('parts', 'edit', false) //edit + ->setPermissionValue('parts', 'create', null) //create + ->setPermissionValue('parts', 'move', null) //move + ->setPermissionValue('parts', 'delete', null); //delete $this->user = $this->createMock(User::class); - $this->user->method('getPermissions')->willReturn($user_embed); + $this->user->method('getPermissions')->willReturn($user_perms); $this->user_withoutGroup = $this->createMock(User::class); - $this->user_withoutGroup->method('getPermissions')->willReturn($user_embed); + $this->user_withoutGroup->method('getPermissions')->willReturn($user_perms); $this->user_withoutGroup->method('getGroup')->willReturn(null); //Set up a faked group - $group1_embed = new PermissionsEmbed(); - $group1_embed->setPermissionValue('parts', 6, true) - ->setPermissionValue('parts', 8, false) - ->setPermissionValue('parts', 10, null) - ->setPermissionValue('parts', 0, false) - ->setPermissionValue('parts', 30, true) - ->setPermissionValue('parts', 2, true); + $group1_perms = new PermissionData(); + $group1_perms + ->setPermissionValue('parts', 'delete', false) + ->setPermissionValue('parts', 'search', null) + ->setPermissionValue('parts', 'read', false) + ->setPermissionValue('parts', 'show_history', true) + ->setPermissionValue('parts', 'edit', true); $this->group = $this->createMock(Group::class); - $this->group->method('getPermissions')->willReturn($group1_embed); + $this->group->method('getPermissions')->willReturn($group1_perms); //Set this group for the user $this->user->method('getGroup')->willReturn($this->group); //parent group - $parent_group_embed = new PermissionsEmbed(); - $parent_group_embed->setPermissionValue('parts', 12, true) - ->setPermissionValue('parts', 14, false) - ->setPermissionValue('parts', 16, null); + $parent_group_perms = new PermissionData(); + $parent_group_perms->setPermissionValue('parts', 'all_parts', true) + ->setPermissionValue('parts', 'no_price_parts', false) + ->setPermissionValue('parts', 'obsolete_parts', null); $parent_group = $this->createMock(Group::class); - $parent_group->method('getPermissions')->willReturn($parent_group_embed); + $parent_group->method('getPermissions')->willReturn($parent_group_perms); $this->group->method('getParent')->willReturn($parent_group); }