diff --git a/CHANGELOG.md b/CHANGELOG.md index ccda751..3abf6f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `ROLE_DOMAIN_MANAGER` + `ROLE_ADMIN` role identifiers + (`App\Security\Roles`), `role_hierarchy` wiring in `security.yaml` + so `ROLE_ADMIN` implies `ROLE_DOMAIN_MANAGER`, and a + domain-scoped `ManageUserVoter` that grants the `MANAGE_USER` / + `APPROVE_USER` / `BLOCK_USER` attributes when the acting user is a + domain manager in the subject's email domain (or a site-wide + admin). + ([#84](https://github.com/itk-dev/ai-lib/issues/84)). - Initial Symfony 8 application scaffold on the ITK Dev Docker `symfony-8` template (phpfpm 8.4, nginx, MariaDB, Mailpit, Traefik), including dev dependencies for coding standards (`php-cs-fixer`, diff --git a/config/packages/security.yaml b/config/packages/security.yaml index bad06f3..9b0a2da 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -3,6 +3,11 @@ security: password_hashers: Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface: 'auto' + # ADR 006: ROLE_ADMIN implies ROLE_DOMAIN_MANAGER so the same admin + # surfaces serve both site-wide admins and domain-scoped approvers. + role_hierarchy: + ROLE_ADMIN: [ROLE_DOMAIN_MANAGER] + # https://symfony.com/doc/current/security.html#loading-the-user-the-user-provider providers: # used to reload user from session & other features (e.g. switch_user) diff --git a/src/Security/EmailDomain.php b/src/Security/EmailDomain.php new file mode 100644 index 0000000..0ffa6b1 --- /dev/null +++ b/src/Security/EmailDomain.php @@ -0,0 +1,54 @@ +getEmail(); + if (null === $email) { + return null; + } + + $at = strrpos($email, '@'); + if (false === $at || $at === strlen($email) - 1) { + return null; + } + + return strtolower(substr($email, $at + 1)); + } + + /** + * @codeCoverageIgnore + */ + private function __construct() + { + } +} diff --git a/src/Security/Roles.php b/src/Security/Roles.php new file mode 100644 index 0000000..be9b5fd --- /dev/null +++ b/src/Security/Roles.php @@ -0,0 +1,42 @@ +getUser(); + if (!$actor instanceof User) { + return false; + } + + if (!$this->accessDecisionManager->decide($token, [Roles::DOMAIN_MANAGER])) { + return false; + } + + if ($this->accessDecisionManager->decide($token, [Roles::ADMIN])) { + return true; + } + + $actorDomain = EmailDomain::of($actor); + $subjectDomain = EmailDomain::of($subject); + if (null === $actorDomain || null === $subjectDomain) { + return false; + } + + return $actorDomain === $subjectDomain; + } +} diff --git a/tests/Unit/Security/EmailDomainTest.php b/tests/Unit/Security/EmailDomainTest.php new file mode 100644 index 0000000..8de40c1 --- /dev/null +++ b/tests/Unit/Security/EmailDomainTest.php @@ -0,0 +1,55 @@ +setEmail('Alice@Aarhus.DK'); + + self::assertSame('aarhus.dk', EmailDomain::of($user)); + } + + // Verifies null is returned when the user has no email set. + public function testReturnsNullForUserWithoutEmail(): void + { + self::assertNull(EmailDomain::of(new User())); + } + + // Ensures null is returned for input that contains no '@'. + public function testReturnsNullWhenEmailHasNoAtSign(): void + { + $user = new User(); + $user->setEmail('not-an-email'); + + self::assertNull(EmailDomain::of($user)); + } + + // Ensures null is returned for input with an empty domain part (trailing '@'). + public function testReturnsNullWhenEmailEndsWithAtSign(): void + { + $user = new User(); + $user->setEmail('orphan@'); + + self::assertNull(EmailDomain::of($user)); + } + + // Tests that 'user+tag@domain' still resolves to the bare domain. + public function testHandlesSubaddressingByKeepingTheDomainOnly(): void + { + // "user+tag@domain" still has exactly one @; the helper splits on the rightmost. + $user = new User(); + $user->setEmail('alice+sub@aarhus.dk'); + + self::assertSame('aarhus.dk', EmailDomain::of($user)); + } +} diff --git a/tests/Unit/Security/Voter/ManageUserVoterTest.php b/tests/Unit/Security/Voter/ManageUserVoterTest.php new file mode 100644 index 0000000..95c67a0 --- /dev/null +++ b/tests/Unit/Security/Voter/ManageUserVoterTest.php @@ -0,0 +1,173 @@ +voterAllowing(); + $token = $this->tokenFor(null); + + self::assertSame( + VoterInterface::ACCESS_DENIED, + $voter->vote($token, $this->userWithEmail('alice@aarhus.dk'), [ManageUserVoter::MANAGE]), + ); + } + + // Ensures access is denied when the actor lacks ROLE_DOMAIN_MANAGER even within the same domain. + public function testDeniesWhenActorLacksDomainManagerRole(): void + { + $voter = $this->voterWithRoles([]); + $token = $this->tokenFor($this->userWithEmail('charlie@aarhus.dk')); + + self::assertSame( + VoterInterface::ACCESS_DENIED, + $voter->vote($token, $this->userWithEmail('alice@aarhus.dk'), [ManageUserVoter::MANAGE]), + ); + } + + // Verifies ROLE_ADMIN short-circuits the same-domain check and grants across any domain. + public function testGrantsAdminAcrossDomains(): void + { + $voter = $this->voterWithRoles([Roles::DOMAIN_MANAGER, Roles::ADMIN]); + $token = $this->tokenFor($this->userWithEmail('siteadmin@aarhus.dk')); + + self::assertSame( + VoterInterface::ACCESS_GRANTED, + $voter->vote($token, $this->userWithEmail('subject@aalborg.dk'), [ManageUserVoter::MANAGE]), + ); + } + + // Tests that a domain manager can act on a subject in the same email domain. + public function testGrantsDomainManagerWithinSameDomain(): void + { + $voter = $this->voterWithRoles([Roles::DOMAIN_MANAGER]); + $token = $this->tokenFor($this->userWithEmail('manager@aarhus.dk')); + + self::assertSame( + VoterInterface::ACCESS_GRANTED, + $voter->vote($token, $this->userWithEmail('alice@aarhus.dk'), [ManageUserVoter::APPROVE]), + ); + } + + // Ensures a domain manager cannot act on subjects in a different email domain. + public function testDeniesDomainManagerAcrossDifferentDomains(): void + { + $voter = $this->voterWithRoles([Roles::DOMAIN_MANAGER]); + $token = $this->tokenFor($this->userWithEmail('manager@aarhus.dk')); + + self::assertSame( + VoterInterface::ACCESS_DENIED, + $voter->vote($token, $this->userWithEmail('alice@aalborg.dk'), [ManageUserVoter::BLOCK]), + ); + } + + // Verifies the domain comparison is case-insensitive on both actor and subject. + public function testIsCaseInsensitiveOnTheDomainComparison(): void + { + $voter = $this->voterWithRoles([Roles::DOMAIN_MANAGER]); + $token = $this->tokenFor($this->userWithEmail('manager@Aarhus.DK')); + + self::assertSame( + VoterInterface::ACCESS_GRANTED, + $voter->vote($token, $this->userWithEmail('alice@AARHUS.dk'), [ManageUserVoter::MANAGE]), + ); + } + + // Tests that the voter denies when the subject has no email to derive a domain from. + public function testDeniesWhenSubjectHasNoEmail(): void + { + $voter = $this->voterWithRoles([Roles::DOMAIN_MANAGER]); + $token = $this->tokenFor($this->userWithEmail('manager@aarhus.dk')); + $subject = new User(); // email left null + + self::assertSame( + VoterInterface::ACCESS_DENIED, + $voter->vote($token, $subject, [ManageUserVoter::MANAGE]), + ); + } + + // Tests that the voter denies when the actor has no email to derive a domain from. + public function testDeniesWhenActorHasNoEmail(): void + { + $voter = $this->voterWithRoles([Roles::DOMAIN_MANAGER]); + $token = $this->tokenFor(new User()); + + self::assertSame( + VoterInterface::ACCESS_DENIED, + $voter->vote($token, $this->userWithEmail('alice@aarhus.dk'), [ManageUserVoter::MANAGE]), + ); + } + + // Ensures the voter abstains on attributes it doesn't claim to support. + public function testAbstainsOnUnsupportedAttribute(): void + { + $voter = $this->voterAllowing(); + $token = $this->tokenFor($this->userWithEmail('alice@aarhus.dk')); + + self::assertSame( + VoterInterface::ACCESS_ABSTAIN, + $voter->vote($token, $this->userWithEmail('bob@aarhus.dk'), ['SOMETHING_ELSE']), + ); + } + + // Ensures the voter abstains on subjects that aren't User instances. + public function testAbstainsOnUnsupportedSubject(): void + { + $voter = $this->voterAllowing(); + $token = $this->tokenFor($this->userWithEmail('alice@aarhus.dk')); + + self::assertSame( + VoterInterface::ACCESS_ABSTAIN, + $voter->vote($token, new \stdClass(), [ManageUserVoter::MANAGE]), + ); + } + + private function userWithEmail(string $email): User + { + $user = new User(); + $user->setEmail($email); + + return $user; + } + + /** + * @param list $tokenRoles roles the access-decision manager will consider granted + */ + private function voterWithRoles(array $tokenRoles): ManageUserVoter + { + $adm = $this->createMock(AccessDecisionManagerInterface::class); + $adm->method('decide')->willReturnCallback( + static fn (TokenInterface $token, array $attributes): bool => \in_array($attributes[0] ?? null, $tokenRoles, true), + ); + + return new ManageUserVoter($adm); + } + + private function voterAllowing(): ManageUserVoter + { + // For tests that never reach the role check (unsupported attribute / subject + // and "actor is not a User"), the access-decision manager isn't consulted. + return $this->voterWithRoles([Roles::DOMAIN_MANAGER, Roles::ADMIN]); + } + + private function tokenFor(?User $user): TokenInterface + { + $token = $this->createMock(TokenInterface::class); + $token->method('getUser')->willReturn($user); + + return $token; + } +}