diff --git a/config/services.yaml b/config/services.yaml index 30ced67c..c569b064 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -130,7 +130,8 @@ services: arguments: ['%security.role_hierarchy.roles%'] App\Security\RolePermissionManager: - arguments: ['%kimai.permissions%'] + arguments: + $permissions: '%kimai.permissions%' # ================================================================================ # LDAP diff --git a/src/Security/RolePermissionManager.php b/src/Security/RolePermissionManager.php index 3c6364bb..87191b2b 100644 --- a/src/Security/RolePermissionManager.php +++ b/src/Security/RolePermissionManager.php @@ -19,9 +19,14 @@ class RolePermissionManager * @var string[] */ protected $knownPermissions = []; + /** + * @var RoleService + */ + private $roles; - public function __construct(array $permissions) + public function __construct(RoleService $roles, array $permissions) { + $this->roles = $roles; $this->permissions = $permissions; foreach ($permissions as $role => $perms) { @@ -46,7 +51,7 @@ class RolePermissionManager public function getRoles(): array { - return array_keys($this->permissions); + return $this->roles->getAvailableNames(); } public function getPermissions(): array diff --git a/src/Security/RoleService.php b/src/Security/RoleService.php index 9402b492..4a3a84d7 100644 --- a/src/Security/RoleService.php +++ b/src/Security/RoleService.php @@ -9,12 +9,16 @@ namespace App\Security; -class RoleService +final class RoleService { /** * @var array */ - protected $roles; + private $roles; + /** + * @var string[] + */ + private $roleNames = []; public function __construct(array $roles) { @@ -23,16 +27,20 @@ class RoleService public function getAvailableNames(): array { - $roles = []; - foreach ($this->roles as $key => $value) { - $roles[] = $key; - if (is_array($value)) { - foreach ($value as $name) { - $roles[] = $name; + if (empty($this->roleNames)) { + $roles = []; + foreach ($this->roles as $key => $value) { + $roles[] = $key; + if (is_array($value)) { + foreach ($value as $name) { + $roles[] = $name; + } } } + + $this->roleNames = array_values(array_unique($roles)); } - return array_values(array_unique($roles)); + return $this->roleNames; } } diff --git a/src/Validator/Constraints/RoleValidator.php b/src/Validator/Constraints/RoleValidator.php index 16aabe36..ff6778f7 100644 --- a/src/Validator/Constraints/RoleValidator.php +++ b/src/Validator/Constraints/RoleValidator.php @@ -9,7 +9,7 @@ namespace App\Validator\Constraints; -use App\Entity\User; +use App\Security\RoleService; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidator; use Symfony\Component\Validator\Exception\UnexpectedTypeException; @@ -17,14 +17,14 @@ use Symfony\Component\Validator\Exception\UnexpectedTypeException; class RoleValidator extends ConstraintValidator { /** - * @var string[] + * @var RoleService */ - protected $allowedRoles = [ - User::ROLE_USER, - User::ROLE_TEAMLEAD, - User::ROLE_ADMIN, - User::ROLE_SUPER_ADMIN - ]; + private $service; + + public function __construct(RoleService $service) + { + $this->service = $service; + } /** * {@inheritdoc} @@ -41,8 +41,10 @@ class RoleValidator extends ConstraintValidator $roles = [$roles]; } + $allowedRoles = $this->service->getAvailableNames(); + foreach ($roles as $role) { - if (!is_string($role) || !in_array($role, $this->allowedRoles)) { + if (!is_string($role) || !in_array($role, $allowedRoles)) { $this->context->buildViolation($constraint->message) ->setParameter('{{ value }}', $this->formatValue($role)) ->setCode(Role::ROLE_ERROR) diff --git a/tests/Mocks/Security/RoleServiceFactory.php b/tests/Mocks/Security/RoleServiceFactory.php new file mode 100644 index 00000000..92569cfe --- /dev/null +++ b/tests/Mocks/Security/RoleServiceFactory.php @@ -0,0 +1,31 @@ + [], + User::ROLE_TEAMLEAD => [User::ROLE_USER], + User::ROLE_ADMIN => [User::ROLE_TEAMLEAD], + User::ROLE_SUPER_ADMIN => [User::ROLE_ADMIN], + ]; + } + + return new RoleService($roles); + } +} diff --git a/tests/Validator/Constraints/RoleValidatorTest.php b/tests/Validator/Constraints/RoleValidatorTest.php index bbe3d551..7bda0d4b 100644 --- a/tests/Validator/Constraints/RoleValidatorTest.php +++ b/tests/Validator/Constraints/RoleValidatorTest.php @@ -10,6 +10,7 @@ namespace App\Tests\Validator\Constraints; use App\Entity\User; +use App\Tests\Mocks\Security\RoleServiceFactory; use App\Validator\Constraints\Role; use App\Validator\Constraints\RoleValidator; use Symfony\Component\Validator\Constraints\NotBlank; @@ -22,7 +23,10 @@ class RoleValidatorTest extends ConstraintValidatorTestCase { protected function createValidator() { - return new RoleValidator(); + $factory = new RoleServiceFactory($this); + $roleService = $factory->create(); + + return new RoleValidator($roleService); } public function getValidRoles() diff --git a/tests/Voter/AbstractVoterTest.php b/tests/Voter/AbstractVoterTest.php index 94c9dec4..2ba196bd 100644 --- a/tests/Voter/AbstractVoterTest.php +++ b/tests/Voter/AbstractVoterTest.php @@ -12,6 +12,7 @@ namespace App\Tests\Voter; use App\Entity\User; use App\Security\AclDecisionManager; use App\Security\RolePermissionManager; +use App\Tests\Mocks\Security\RoleServiceFactory; use App\Voter\AbstractVoter; use PHPUnit\Framework\TestCase; @@ -95,6 +96,9 @@ abstract class AbstractVoterTest extends TestCase ]; } - return new RolePermissionManager($permissions); + $factory = new RoleServiceFactory($this); + $roleService = $factory->create(); + + return new RolePermissionManager($roleService, $permissions); } }