From e4b1506ac53ec163780cf639ffb2a808acec6ba1 Mon Sep 17 00:00:00 2001 From: myrmidex Date: Wed, 24 Jun 2026 23:26:46 +0200 Subject: [PATCH 1/4] 50 - Stamp authenticated user as Scenario owner on write --- migrations/Version20260623183021.php | 33 ++++ src/Entity/Scenario.php | 21 +++ src/State/ScenarioOwnerProcessor.php | 50 ++++++ tests/Entity/BucketPersistenceTest.php | 19 +++ tests/Entity/ScenarioOwnerTest.php | 49 ++++++ tests/Entity/ScenarioPersistenceTest.php | 20 +++ tests/Entity/StreamPersistenceTest.php | 17 ++ tests/Functional/BucketApiTest.php | 10 +- tests/Functional/ProjectionPreviewApiTest.php | 10 +- tests/Functional/ScenarioApiTest.php | 10 +- tests/Functional/ScenarioOwnerApiTest.php | 156 ++++++++++++++++++ tests/Functional/StreamApiTest.php | 10 +- tests/Repository/BucketRepositoryTest.php | 10 ++ .../SingleOverflowPerScenarioTest.php | 18 ++ 14 files changed, 417 insertions(+), 16 deletions(-) create mode 100644 migrations/Version20260623183021.php create mode 100644 src/State/ScenarioOwnerProcessor.php create mode 100644 tests/Entity/ScenarioOwnerTest.php create mode 100644 tests/Functional/ScenarioOwnerApiTest.php diff --git a/migrations/Version20260623183021.php b/migrations/Version20260623183021.php new file mode 100644 index 0000000..834c02e --- /dev/null +++ b/migrations/Version20260623183021.php @@ -0,0 +1,33 @@ +addSql('ALTER TABLE scenario ADD owner_id UUID NOT NULL'); + $this->addSql('ALTER TABLE scenario ADD CONSTRAINT FK_3E45C8D87E3C61F9 FOREIGN KEY (owner_id) REFERENCES "user" (id) ON DELETE CASCADE NOT DEFERRABLE'); + $this->addSql('CREATE INDEX IDX_3E45C8D87E3C61F9 ON scenario (owner_id)'); + } + + public function down(Schema $schema): void + { + // this down() migration is auto-generated, please modify it to your needs + $this->addSql('ALTER TABLE scenario DROP CONSTRAINT FK_3E45C8D87E3C61F9'); + $this->addSql('DROP INDEX IDX_3E45C8D87E3C61F9'); + $this->addSql('ALTER TABLE scenario DROP owner_id'); + } +} diff --git a/src/Entity/Scenario.php b/src/Entity/Scenario.php index 9d9abc1..adc33bc 100644 --- a/src/Entity/Scenario.php +++ b/src/Entity/Scenario.php @@ -24,6 +24,15 @@ class Scenario #[ORM\Column(type: 'uuid')] private UuidV7 $id; + /** + * Set server-side by App\State\ScenarioOwnerProcessor on write; must be + * assigned via setOwner() before persist. The non-nullable type means + * getOwner() throws if read before that invariant is satisfied. + */ + #[ORM\ManyToOne(targetEntity: User::class)] + #[ORM\JoinColumn(nullable: false, onDelete: 'CASCADE')] + private User $owner; + #[ORM\Column(enumType: DistributionMode::class)] private DistributionMode $distributionMode = DistributionMode::EVEN; @@ -44,6 +53,11 @@ class Scenario return $this->id; } + public function getOwner(): User + { + return $this->owner; + } + public function getDistributionMode(): DistributionMode { return $this->distributionMode; @@ -59,6 +73,13 @@ class Scenario return $this->description; } + public function setOwner(User $owner): static + { + $this->owner = $owner; + + return $this; + } + public function setDistributionMode(DistributionMode $distributionMode): static { $this->distributionMode = $distributionMode; diff --git a/src/State/ScenarioOwnerProcessor.php b/src/State/ScenarioOwnerProcessor.php new file mode 100644 index 0000000..899ec9d --- /dev/null +++ b/src/State/ScenarioOwnerProcessor.php @@ -0,0 +1,50 @@ + + */ +#[AsDecorator('api_platform.doctrine.orm.state.persist_processor')] +final class ScenarioOwnerProcessor implements ProcessorInterface +{ + /** + * @param ProcessorInterface $decorated + */ + public function __construct( + #[AutowireDecorated] + private readonly ProcessorInterface $decorated, + private readonly Security $security, + ) { + } + + public function process( + mixed $data, + Operation $operation, + array $uriVariables = [], + array $context = [], + ): mixed { + if ($data instanceof Scenario) { + // The Scenario Post operation is gated by `is_granted('ROLE_USER')` + // (see #[ApiResource] on App\Entity\Scenario), so by the time this + // processor runs the security pipeline guarantees a logged-in User. + $user = $this->security->getUser(); + \assert($user instanceof User); + + $data->setOwner($user); + } + + return $this->decorated->process($data, $operation, $uriVariables, $context); + } +} diff --git a/tests/Entity/BucketPersistenceTest.php b/tests/Entity/BucketPersistenceTest.php index da4cf94..ac7d0a8 100644 --- a/tests/Entity/BucketPersistenceTest.php +++ b/tests/Entity/BucketPersistenceTest.php @@ -4,6 +4,7 @@ namespace App\Tests\Entity; use App\Entity\Bucket; use App\Entity\Scenario; +use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; @@ -21,10 +22,25 @@ final class BucketPersistenceTest extends KernelTestCase $this->em = self::getContainer()->get(EntityManagerInterface::class); } + private function persistOwner(): User + { + static $counter = 0; + ++$counter; + + $owner = new User(); + $owner->setEmail(\sprintf('bucket-persistence-test-%d@example.com', $counter)); + $owner->setPassword('irrelevant-hash'); + + $this->em->persist($owner); + + return $owner; + } + public function testItPersistsABucketLinkedToItsScenario(): void { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -95,6 +111,7 @@ final class BucketPersistenceTest extends KernelTestCase { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -134,6 +151,7 @@ final class BucketPersistenceTest extends KernelTestCase { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -174,6 +192,7 @@ final class BucketPersistenceTest extends KernelTestCase { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Entity/ScenarioOwnerTest.php b/tests/Entity/ScenarioOwnerTest.php new file mode 100644 index 0000000..ad05883 --- /dev/null +++ b/tests/Entity/ScenarioOwnerTest.php @@ -0,0 +1,49 @@ +em = self::getContainer()->get(EntityManagerInterface::class); + } + + public function testOwnerRoundTripsThroughPersistence(): void + { + $owner = new User(); + $owner->setEmail('scenario-owner-test@example.com'); + $owner->setPassword('irrelevant-hash'); + + $this->em->persist($owner); + + $scenario = new Scenario(); + $scenario->setName('Household Budget'); + $scenario->setOwner($owner); + + $this->em->persist($scenario); + $this->em->flush(); + + $scenarioId = $scenario->getId(); + $ownerId = $owner->getId(); + + $this->em->clear(); + + $reloaded = $this->em->find(Scenario::class, $scenarioId); + + self::assertNotNull($reloaded, 'Scenario must be retrievable from the database after an identity-map clear.'); + self::assertInstanceOf(User::class, $reloaded->getOwner()); + self::assertTrue( + $ownerId->equals($reloaded->getOwner()->getId()), + 'The owner set before persistence must survive a flush + clear + reload round-trip.', + ); + } +} diff --git a/tests/Entity/ScenarioPersistenceTest.php b/tests/Entity/ScenarioPersistenceTest.php index 3e56db4..3fee263 100644 --- a/tests/Entity/ScenarioPersistenceTest.php +++ b/tests/Entity/ScenarioPersistenceTest.php @@ -3,6 +3,7 @@ namespace App\Tests\Entity; use App\Entity\Scenario; +use App\Entity\User; use App\Enum\DistributionMode; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; @@ -18,10 +19,25 @@ final class ScenarioPersistenceTest extends KernelTestCase $this->em = self::getContainer()->get(EntityManagerInterface::class); } + private function persistOwner(): User + { + static $counter = 0; + ++$counter; + + $owner = new User(); + $owner->setEmail(\sprintf('scenario-persistence-test-%d@example.com', $counter)); + $owner->setPassword('irrelevant-hash'); + + $this->em->persist($owner); + + return $owner; + } + public function testItPersistsAScenarioWithAUuidId(): void { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -42,6 +58,7 @@ final class ScenarioPersistenceTest extends KernelTestCase { $scenario = new Scenario(); $scenario->setName('Default Mode Scenario'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -65,6 +82,7 @@ final class ScenarioPersistenceTest extends KernelTestCase $scenario = new Scenario(); $scenario->setName('Priority Mode Scenario'); $scenario->setDistributionMode(DistributionMode::PRIORITY); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -93,6 +111,7 @@ final class ScenarioPersistenceTest extends KernelTestCase $scenario = new Scenario(); $scenario->setName('Described Scenario'); $scenario->setDescription('Covers rent, groceries, and the emergency buffer.'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -111,6 +130,7 @@ final class ScenarioPersistenceTest extends KernelTestCase { $scenario = new Scenario(); $scenario->setName('Undescribed Scenario'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Entity/StreamPersistenceTest.php b/tests/Entity/StreamPersistenceTest.php index c94dbb1..9a4c2ae 100644 --- a/tests/Entity/StreamPersistenceTest.php +++ b/tests/Entity/StreamPersistenceTest.php @@ -5,6 +5,7 @@ namespace App\Tests\Entity; use App\Entity\Bucket; use App\Entity\Scenario; use App\Entity\Stream; +use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; use App\Enum\StreamFrequency; @@ -22,10 +23,25 @@ final class StreamPersistenceTest extends KernelTestCase $this->em = self::getContainer()->get(EntityManagerInterface::class); } + private function persistOwner(): User + { + static $counter = 0; + ++$counter; + + $owner = new User(); + $owner->setEmail(\sprintf('stream-persistence-test-%d@example.com', $counter)); + $owner->setPassword('irrelevant-hash'); + + $this->em->persist($owner); + + return $owner; + } + public function testItPersistsAStreamLinkedToItsScenarioAndBucket(): void { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $bucket = new Bucket(); @@ -119,6 +135,7 @@ final class StreamPersistenceTest extends KernelTestCase { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Functional/BucketApiTest.php b/tests/Functional/BucketApiTest.php index 2eed148..33897f5 100644 --- a/tests/Functional/BucketApiTest.php +++ b/tests/Functional/BucketApiTest.php @@ -22,6 +22,7 @@ final class BucketApiTest extends WebTestCase private KernelBrowser $client; private EntityManagerInterface $em; + private User $user; protected function setUp(): void { @@ -36,11 +37,11 @@ final class BucketApiTest extends WebTestCase ]); $hasher = new UserPasswordHasher($factory); - $user = new User(); - $user->setEmail(self::EMAIL); - $user->setPassword($hasher->hashPassword($user, self::PASSWORD)); + $this->user = new User(); + $this->user->setEmail(self::EMAIL); + $this->user->setPassword($hasher->hashPassword($this->user, self::PASSWORD)); - $this->em->persist($user); + $this->em->persist($this->user); $this->em->flush(); } @@ -62,6 +63,7 @@ final class BucketApiTest extends WebTestCase { $scenario = new Scenario(); $scenario->setName($name); + $scenario->setOwner($this->user); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Functional/ProjectionPreviewApiTest.php b/tests/Functional/ProjectionPreviewApiTest.php index 5447b33..110e399 100644 --- a/tests/Functional/ProjectionPreviewApiTest.php +++ b/tests/Functional/ProjectionPreviewApiTest.php @@ -27,6 +27,7 @@ final class ProjectionPreviewApiTest extends WebTestCase private KernelBrowser $client; private EntityManagerInterface $em; + private User $user; protected function setUp(): void { @@ -41,11 +42,11 @@ final class ProjectionPreviewApiTest extends WebTestCase ]); $hasher = new UserPasswordHasher($factory); - $user = new User(); - $user->setEmail(self::EMAIL); - $user->setPassword($hasher->hashPassword($user, self::PASSWORD)); + $this->user = new User(); + $this->user->setEmail(self::EMAIL); + $this->user->setPassword($hasher->hashPassword($this->user, self::PASSWORD)); - $this->em->persist($user); + $this->em->persist($this->user); $this->em->flush(); } @@ -67,6 +68,7 @@ final class ProjectionPreviewApiTest extends WebTestCase { $scenario = new Scenario(); $scenario->setName($name); + $scenario->setOwner($this->user); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Functional/ScenarioApiTest.php b/tests/Functional/ScenarioApiTest.php index 56ff79a..d0f58d2 100644 --- a/tests/Functional/ScenarioApiTest.php +++ b/tests/Functional/ScenarioApiTest.php @@ -18,6 +18,7 @@ final class ScenarioApiTest extends WebTestCase private KernelBrowser $client; private EntityManagerInterface $em; + private User $user; protected function setUp(): void { @@ -32,11 +33,11 @@ final class ScenarioApiTest extends WebTestCase ]); $hasher = new UserPasswordHasher($factory); - $user = new User(); - $user->setEmail(self::EMAIL); - $user->setPassword($hasher->hashPassword($user, self::PASSWORD)); + $this->user = new User(); + $this->user->setEmail(self::EMAIL); + $this->user->setPassword($hasher->hashPassword($this->user, self::PASSWORD)); - $this->em->persist($user); + $this->em->persist($this->user); $this->em->flush(); } @@ -58,6 +59,7 @@ final class ScenarioApiTest extends WebTestCase { $scenario = new Scenario(); $scenario->setName($name); + $scenario->setOwner($this->user); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Functional/ScenarioOwnerApiTest.php b/tests/Functional/ScenarioOwnerApiTest.php new file mode 100644 index 0000000..367fc12 --- /dev/null +++ b/tests/Functional/ScenarioOwnerApiTest.php @@ -0,0 +1,156 @@ +client = static::createClient(); + + /** @var EntityManagerInterface $em */ + $em = static::getContainer()->get(EntityManagerInterface::class); + $this->em = $em; + + $factory = new PasswordHasherFactory([ + User::class => new NativePasswordHasher(cost: 4), + ]); + $hasher = new UserPasswordHasher($factory); + + $this->caller = new User(); + $this->caller->setEmail(self::EMAIL); + $this->caller->setPassword($hasher->hashPassword($this->caller, self::PASSWORD)); + $this->em->persist($this->caller); + + $this->otherUser = new User(); + $this->otherUser->setEmail(self::OTHER_EMAIL); + $this->otherUser->setPassword($hasher->hashPassword($this->otherUser, 'some-other-password')); + $this->em->persist($this->otherUser); + + $this->em->flush(); + } + + private function login(): void + { + $this->client->jsonRequest('POST', '/api/login', [ + 'username' => self::EMAIL, + 'password' => self::PASSWORD, + ]); + + self::assertLessThan( + 300, + $this->client->getResponse()->getStatusCode(), + 'Test setup precondition: login must succeed before exercising the Scenario API.', + ); + } + + public function testPostCreatesAScenarioOwnedByTheAuthenticatedUser(): void + { + $this->login(); + + $this->client->request( + 'POST', + '/api/scenarios', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode(['name' => 'Holiday Fund']), + ); + + self::assertSame( + 201, + $this->client->getResponse()->getStatusCode(), + 'POST /api/scenarios with a valid body must return 201 Created.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(\App\Entity\Scenario::class)->findOneBy(['name' => 'Holiday Fund']); + + self::assertNotNull($persisted, 'A successful POST /api/scenarios must persist the new Scenario.'); + self::assertNotNull($persisted->getOwner(), 'The persisted Scenario must have an owner set.'); + self::assertTrue( + $this->caller->getId()->equals($persisted->getOwner()->getId()), + 'The persisted Scenario must be owned by the authenticated caller, not left null or assigned elsewhere.', + ); + } + + public function testPostIgnoresAClientSuppliedOwner(): void + { + $this->login(); + + $this->client->request( + 'POST', + '/api/scenarios', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode([ + 'name' => 'Spoofed Owner Fund', + 'owner' => '/api/users/'.$this->otherUser->getId(), + ]), + ); + + self::assertSame( + 201, + $this->client->getResponse()->getStatusCode(), + 'POST /api/scenarios with a valid body must return 201 Created even when a client-supplied owner is present in the payload.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(\App\Entity\Scenario::class)->findOneBy(['name' => 'Spoofed Owner Fund']); + + self::assertNotNull($persisted, 'A successful POST /api/scenarios must persist the new Scenario.'); + self::assertNotNull($persisted->getOwner(), 'The persisted Scenario must have an owner set.'); + self::assertTrue( + $this->caller->getId()->equals($persisted->getOwner()->getId()), + 'A client-supplied owner in the request body must be ignored — the owner must always be the authenticated caller.', + ); + self::assertFalse( + $this->otherUser->getId()->equals($persisted->getOwner()->getId()), + 'The spoofed owner IRI in the request body must never be honoured.', + ); + } + + public function testPostWithMissingNameIsStillRejectedAsUnprocessable(): void + { + $this->login(); + + $this->client->request( + 'POST', + '/api/scenarios', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode([]), + ); + + self::assertSame( + 422, + $this->client->getResponse()->getStatusCode(), + 'POST /api/scenarios without a name must still return 422 Unprocessable Entity — the owner-setting persist processor must not interfere with existing validation order.', + ); + } +} diff --git a/tests/Functional/StreamApiTest.php b/tests/Functional/StreamApiTest.php index f145fcd..f9e9f41 100644 --- a/tests/Functional/StreamApiTest.php +++ b/tests/Functional/StreamApiTest.php @@ -22,6 +22,7 @@ final class StreamApiTest extends WebTestCase private KernelBrowser $client; private EntityManagerInterface $em; + private User $user; protected function setUp(): void { @@ -36,11 +37,11 @@ final class StreamApiTest extends WebTestCase ]); $hasher = new UserPasswordHasher($factory); - $user = new User(); - $user->setEmail(self::EMAIL); - $user->setPassword($hasher->hashPassword($user, self::PASSWORD)); + $this->user = new User(); + $this->user->setEmail(self::EMAIL); + $this->user->setPassword($hasher->hashPassword($this->user, self::PASSWORD)); - $this->em->persist($user); + $this->em->persist($this->user); $this->em->flush(); } @@ -62,6 +63,7 @@ final class StreamApiTest extends WebTestCase { $scenario = new Scenario(); $scenario->setName($name); + $scenario->setOwner($this->user); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Repository/BucketRepositoryTest.php b/tests/Repository/BucketRepositoryTest.php index ca9ba6f..6670cc6 100644 --- a/tests/Repository/BucketRepositoryTest.php +++ b/tests/Repository/BucketRepositoryTest.php @@ -4,6 +4,7 @@ namespace App\Tests\Repository; use App\Entity\Bucket; use App\Entity\Scenario; +use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; use App\Repository\BucketRepository; @@ -141,8 +142,17 @@ final class BucketRepositoryTest extends KernelTestCase private function persistScenario(string $name = 'Household Budget'): Scenario { + static $counter = 0; + ++$counter; + + $owner = new User(); + $owner->setEmail(\sprintf('bucket-repository-test-%d@example.com', $counter)); + $owner->setPassword('irrelevant-hash'); + $this->em->persist($owner); + $scenario = new Scenario(); $scenario->setName($name); + $scenario->setOwner($owner); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Validator/SingleOverflowPerScenarioTest.php b/tests/Validator/SingleOverflowPerScenarioTest.php index 1d07767..d9ac68f 100644 --- a/tests/Validator/SingleOverflowPerScenarioTest.php +++ b/tests/Validator/SingleOverflowPerScenarioTest.php @@ -4,6 +4,7 @@ namespace App\Tests\Validator; use App\Entity\Bucket; use App\Entity\Scenario; +use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; use App\Validator\SingleOverflowPerScenario; @@ -24,10 +25,25 @@ final class SingleOverflowPerScenarioTest extends KernelTestCase $this->validator = self::getContainer()->get(ValidatorInterface::class); } + private function persistOwner(): User + { + static $counter = 0; + ++$counter; + + $owner = new User(); + $owner->setEmail(\sprintf('single-overflow-per-scenario-test-%d@example.com', $counter)); + $owner->setPassword('irrelevant-hash'); + + $this->em->persist($owner); + + return $owner; + } + public function testSecondOverflowBucketOnTheSameScenarioRaisesAViolation(): void { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -54,6 +70,7 @@ final class SingleOverflowPerScenarioTest extends KernelTestCase { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); @@ -77,6 +94,7 @@ final class SingleOverflowPerScenarioTest extends KernelTestCase { $scenario = new Scenario(); $scenario->setName('Household Budget'); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); -- 2.45.2 From 064805eb091936ad4d234921499a6088549df408 Mon Sep 17 00:00:00 2001 From: myrmidex Date: Thu, 25 Jun 2026 19:59:40 +0200 Subject: [PATCH 2/4] 50 - Scope Scenario reads to the authenticated owner --- src/Doctrine/ScenarioOwnerExtension.php | 69 ++++++++++++ tests/Functional/ScenarioOwnerApiTest.php | 121 +++++++++++++++++++++- 2 files changed, 186 insertions(+), 4 deletions(-) create mode 100644 src/Doctrine/ScenarioOwnerExtension.php diff --git a/src/Doctrine/ScenarioOwnerExtension.php b/src/Doctrine/ScenarioOwnerExtension.php new file mode 100644 index 0000000..62b9bf6 --- /dev/null +++ b/src/Doctrine/ScenarioOwnerExtension.php @@ -0,0 +1,69 @@ +addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); + } + + public function applyToItem( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + array $identifiers, + ?Operation $operation = null, + array $context = [], + ): void { + $this->addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); + } + + private function addOwnerWhere( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + ): void { + // This extension fires for every resource's queries — only act on Scenario. + if (Scenario::class !== $resourceClass) { + return; + } + + $user = $this->security->getUser(); + if (!$user instanceof User) { + return; + } + + $rootAlias = $queryBuilder->getRootAliases()[0]; + $param = $queryNameGenerator->generateParameterName('currentUser'); + + $queryBuilder + ->andWhere(\sprintf('%s.owner = :%s', $rootAlias, $param)) + ->setParameter($param, $user); + } +} diff --git a/tests/Functional/ScenarioOwnerApiTest.php b/tests/Functional/ScenarioOwnerApiTest.php index 367fc12..5aee74c 100644 --- a/tests/Functional/ScenarioOwnerApiTest.php +++ b/tests/Functional/ScenarioOwnerApiTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Functional; +use App\Entity\Scenario; use App\Entity\User; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\KernelBrowser; @@ -84,10 +85,9 @@ final class ScenarioOwnerApiTest extends WebTestCase $this->em->clear(); - $persisted = $this->em->getRepository(\App\Entity\Scenario::class)->findOneBy(['name' => 'Holiday Fund']); + $persisted = $this->em->getRepository(Scenario::class)->findOneBy(['name' => 'Holiday Fund']); self::assertNotNull($persisted, 'A successful POST /api/scenarios must persist the new Scenario.'); - self::assertNotNull($persisted->getOwner(), 'The persisted Scenario must have an owner set.'); self::assertTrue( $this->caller->getId()->equals($persisted->getOwner()->getId()), 'The persisted Scenario must be owned by the authenticated caller, not left null or assigned elsewhere.', @@ -119,10 +119,9 @@ final class ScenarioOwnerApiTest extends WebTestCase $this->em->clear(); - $persisted = $this->em->getRepository(\App\Entity\Scenario::class)->findOneBy(['name' => 'Spoofed Owner Fund']); + $persisted = $this->em->getRepository(Scenario::class)->findOneBy(['name' => 'Spoofed Owner Fund']); self::assertNotNull($persisted, 'A successful POST /api/scenarios must persist the new Scenario.'); - self::assertNotNull($persisted->getOwner(), 'The persisted Scenario must have an owner set.'); self::assertTrue( $this->caller->getId()->equals($persisted->getOwner()->getId()), 'A client-supplied owner in the request body must be ignored — the owner must always be the authenticated caller.', @@ -133,6 +132,120 @@ final class ScenarioOwnerApiTest extends WebTestCase ); } + public function testGetItemOnAScenarioOwnedBySomeoneElseReturnsNotFound(): void + { + $othersScenario = new Scenario(); + $othersScenario->setName('Someone Else\'s Fund'); + $othersScenario->setOwner($this->otherUser); + $this->em->persist($othersScenario); + $this->em->flush(); + + $this->login(); + + $this->client->request('GET', '/api/scenarios/'.$othersScenario->getId(), server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 404, + $this->client->getResponse()->getStatusCode(), + 'GET /api/scenarios/{id} for a scenario owned by another user must return 404, not 403 — no existence oracle for scenarios you do not own.', + ); + } + + public function testGetCollectionOnlyReturnsScenariosOwnedByTheCaller(): void + { + $mine = new Scenario(); + $mine->setName('My Fund'); + $mine->setOwner($this->caller); + $this->em->persist($mine); + + $othersScenario = new Scenario(); + $othersScenario->setName('Someone Else\'s Fund'); + $othersScenario->setOwner($this->otherUser); + $this->em->persist($othersScenario); + + $this->em->flush(); + + $this->login(); + + $this->client->request('GET', '/api/scenarios', server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + $response = $this->client->getResponse(); + + self::assertSame( + 200, + $response->getStatusCode(), + 'GET /api/scenarios must return 200 for an authenticated user.', + ); + + $body = json_decode((string) $response->getContent(), true); + + self::assertIsArray($body, 'The collection response must be a JSON-LD/Hydra object.'); + self::assertArrayHasKey( + 'member', + $body, + 'A Hydra collection response must expose its items under the member key.', + ); + + $names = array_column($body['member'], 'name'); + + self::assertContains( + 'My Fund', + $names, + 'GET /api/scenarios must include scenarios owned by the authenticated caller.', + ); + self::assertNotContains( + 'Someone Else\'s Fund', + $names, + 'GET /api/scenarios must NOT include scenarios owned by other users.', + ); + } + + public function testAnonymousGetCollectionIsRejectedRatherThanReturningUnfilteredScenarios(): void + { + // The ownership extension no-ops for anonymous requests (no user to scope to), + // so the resource-level `is_granted('ROLE_USER')` security MUST reject the + // request before the query runs. If that wall ever relaxes, an anonymous caller + // would receive an unfiltered listing — this test fails closed if that happens. + $othersScenario = new Scenario(); + $othersScenario->setName('Someone Else\'s Fund'); + $othersScenario->setOwner($this->otherUser); + $this->em->persist($othersScenario); + $this->em->flush(); + + $this->client->request('GET', '/api/scenarios', server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 401, + $this->client->getResponse()->getStatusCode(), + 'GET /api/scenarios without authentication must be rejected by the security layer, not served an unfiltered collection.', + ); + } + + public function testAnonymousGetItemIsRejectedRatherThanResolvingScenario(): void + { + $othersScenario = new Scenario(); + $othersScenario->setName('Someone Else\'s Fund'); + $othersScenario->setOwner($this->otherUser); + $this->em->persist($othersScenario); + $this->em->flush(); + + $this->client->request('GET', '/api/scenarios/'.$othersScenario->getId(), server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 401, + $this->client->getResponse()->getStatusCode(), + 'GET /api/scenarios/{id} without authentication must be rejected by the security layer.', + ); + } + public function testPostWithMissingNameIsStillRejectedAsUnprocessable(): void { $this->login(); -- 2.45.2 From 6dd8e2207fcddf760b838fb5a594eebf34f78a9f Mon Sep 17 00:00:00 2001 From: myrmidex Date: Thu, 25 Jun 2026 20:34:45 +0200 Subject: [PATCH 3/4] 50 - Scope Bucket reads to the owning Scenario's owner --- src/Doctrine/BucketOwnerExtension.php | 74 +++++ src/State/PreviewProcessor.php | 12 + tests/Functional/BucketOwnerApiTest.php | 282 ++++++++++++++++++ tests/Functional/ProjectionPreviewApiTest.php | 62 +++- tests/State/PreviewProcessorTest.php | 51 ++++ 5 files changed, 478 insertions(+), 3 deletions(-) create mode 100644 src/Doctrine/BucketOwnerExtension.php create mode 100644 tests/Functional/BucketOwnerApiTest.php diff --git a/src/Doctrine/BucketOwnerExtension.php b/src/Doctrine/BucketOwnerExtension.php new file mode 100644 index 0000000..23cbc8b --- /dev/null +++ b/src/Doctrine/BucketOwnerExtension.php @@ -0,0 +1,74 @@ +addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); + } + + public function applyToItem( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + array $identifiers, + ?Operation $operation = null, + array $context = [], + ): void { + $this->addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); + } + + private function addOwnerWhere( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + ): void { + // This extension fires for every resource's queries — only act on Bucket. + if (Bucket::class !== $resourceClass) { + return; + } + + $user = $this->security->getUser(); + if (!$user instanceof User) { + return; + } + + $rootAlias = $queryBuilder->getRootAliases()[0]; + $scenarioAlias = $queryNameGenerator->generateJoinAlias('scenario'); + $param = $queryNameGenerator->generateParameterName('currentUser'); + + $queryBuilder + ->innerJoin(\sprintf('%s.scenario', $rootAlias), $scenarioAlias) + ->andWhere(\sprintf('%s.owner = :%s', $scenarioAlias, $param)) + ->setParameter($param, $user); + } +} diff --git a/src/State/PreviewProcessor.php b/src/State/PreviewProcessor.php index afe3108..43e0fba 100644 --- a/src/State/PreviewProcessor.php +++ b/src/State/PreviewProcessor.php @@ -7,10 +7,12 @@ use ApiPlatform\State\ProcessorInterface; use App\ApiResource\ProjectionPreviewInput; use App\ApiResource\ProjectionPreviewOutput; use App\Entity\Scenario; +use App\Entity\User; use App\Repository\BucketRepository; use App\Repository\ScenarioRepository; use App\Service\Allocation\AllocateIncome; use Doctrine\Common\Collections\ArrayCollection; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Uid\Uuid; @@ -22,6 +24,7 @@ class PreviewProcessor implements ProcessorInterface public function __construct( private readonly ScenarioRepository $scenarioRepository, private readonly BucketRepository $bucketRepository, + private readonly Security $security, ) { } @@ -43,6 +46,15 @@ class PreviewProcessor implements ProcessorInterface throw new NotFoundHttpException(); } + // This operation is gated by `is_granted('ROLE_USER')` on the Preview + // resource, so the security pipeline guarantees a logged-in User here. + $currentUser = $this->security->getUser(); + \assert($currentUser instanceof User); + + if (!$currentUser->getId()->equals($scenario->getOwner()->getId())) { + throw new NotFoundHttpException(); + } + return $this->buildOutput($scenario, $data->amount); } diff --git a/tests/Functional/BucketOwnerApiTest.php b/tests/Functional/BucketOwnerApiTest.php new file mode 100644 index 0000000..25b4e5e --- /dev/null +++ b/tests/Functional/BucketOwnerApiTest.php @@ -0,0 +1,282 @@ +client = static::createClient(); + + /** @var EntityManagerInterface $em */ + $em = static::getContainer()->get(EntityManagerInterface::class); + $this->em = $em; + + $factory = new PasswordHasherFactory([ + User::class => new NativePasswordHasher(cost: 4), + ]); + $hasher = new UserPasswordHasher($factory); + + $this->caller = new User(); + $this->caller->setEmail(self::EMAIL); + $this->caller->setPassword($hasher->hashPassword($this->caller, self::PASSWORD)); + $this->em->persist($this->caller); + + $this->otherUser = new User(); + $this->otherUser->setEmail(self::OTHER_EMAIL); + $this->otherUser->setPassword($hasher->hashPassword($this->otherUser, 'some-other-password')); + $this->em->persist($this->otherUser); + + $this->em->flush(); + } + + private function login(): void + { + $this->client->jsonRequest('POST', '/api/login', [ + 'username' => self::EMAIL, + 'password' => self::PASSWORD, + ]); + + self::assertLessThan( + 300, + $this->client->getResponse()->getStatusCode(), + 'Test setup precondition: login must succeed before exercising the Bucket API.', + ); + } + + private function persistScenarioFor(User $owner, string $name): Scenario + { + $scenario = new Scenario(); + $scenario->setName($name); + $scenario->setOwner($owner); + + $this->em->persist($scenario); + $this->em->flush(); + + return $scenario; + } + + private function persistBucket(Scenario $scenario, string $name, int $priority): Bucket + { + $bucket = new Bucket(); + $bucket->setScenario($scenario); + $bucket->setName($name); + $bucket->setPriority($priority); + + $this->em->persist($bucket); + $this->em->flush(); + + return $bucket; + } + + public function testGetItemOnABucketOwnedBySomeoneElseReturnsNotFound(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $othersBucket = $this->persistBucket($othersScenario, 'Their Rent', 1); + + $this->login(); + + $this->client->request('GET', '/api/buckets/'.$othersBucket->getId(), server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 404, + $this->client->getResponse()->getStatusCode(), + 'GET /api/buckets/{id} for a bucket whose scenario is owned by another user must return 404, not 403 — ' + .'no existence oracle for buckets you do not own, scoped transitively via scenario.owner.', + ); + } + + public function testGetCollectionOnlyReturnsBucketsWhoseScenarioIsOwnedByTheCaller(): void + { + $myScenario = $this->persistScenarioFor($this->caller, 'My Fund'); + $this->persistBucket($myScenario, 'My Rent', 1); + + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $this->persistBucket($othersScenario, 'Their Rent', 1); + + $this->login(); + + $this->client->request('GET', '/api/buckets', server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + $response = $this->client->getResponse(); + + self::assertSame( + 200, + $response->getStatusCode(), + 'GET /api/buckets must return 200 for an authenticated user.', + ); + + $body = json_decode((string) $response->getContent(), true); + + self::assertIsArray($body, 'The collection response must be a JSON-LD/Hydra object.'); + self::assertArrayHasKey( + 'member', + $body, + 'A Hydra collection response must expose its items under the member key.', + ); + + $names = array_column($body['member'], 'name'); + + self::assertContains( + 'My Rent', + $names, + 'GET /api/buckets must include buckets whose scenario is owned by the authenticated caller.', + ); + self::assertNotContains( + 'Their Rent', + $names, + 'GET /api/buckets must NOT include buckets whose scenario is owned by another user, ' + .'even though the bucket itself has no direct owner column — ownership is transitive via scenario.owner.', + ); + } + + /** + * PINS CURRENT (PRE-#50-STORY-3) BEHAVIOUR — flagged for review, not a locked contract. + * + * The ticket asks for 404 when POSTing a bucket under a scenario IRI the caller + * does not own. Empirically, today this is 400: API Platform's IRI denormalizer + * (`AbstractItemNormalizer::getResourceFromIri`) raises "Item not found" as soon as + * the `scenario` relation can't be resolved — Scenario's own ownership extension + * (#50 Story 2) already makes a foreign scenario IRI unresolvable to a non-owner, + * so this 400 already happens BEFORE any Bucket-specific extension code runs. + * + * This test asserts the OBSERVED 400, not the ticket's desired 404. If a 404 is + * required, it needs a normalizing guard (e.g. catching the IRI-resolution failure + * and re-throwing as NotFoundHttpException) — that's an implementation decision, + * flagged to the user rather than assumed. + */ + public function testPostUnderAForeignScenarioCurrentlyReturnsBadRequestNotFound(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + + $this->login(); + + $this->client->request( + 'POST', + '/api/buckets', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode([ + 'scenario' => '/api/scenarios/'.$othersScenario->getId(), + 'name' => 'Sneaky Bucket', + 'priority' => 1, + ]), + ); + + $response = $this->client->getResponse(); + + self::assertSame( + 400, + $response->getStatusCode(), + 'POST /api/buckets under a scenario IRI the caller does not own currently resolves to 400 ' + .'(unresolvable IRI denormalization failure), not 201 — pinning the observed behaviour. ' + .'See class docblock above this test: a 404 may require an explicit normalizing guard.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(Bucket::class)->findOneBy(['name' => 'Sneaky Bucket']); + + self::assertNull( + $persisted, + 'A rejected POST under a foreign scenario must never persist a Bucket.', + ); + } + + public function testPostUnderTheCallersOwnScenarioStillSucceeds(): void + { + $myScenario = $this->persistScenarioFor($this->caller, 'My Fund'); + + $this->login(); + + $this->client->request( + 'POST', + '/api/buckets', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode([ + 'scenario' => '/api/scenarios/'.$myScenario->getId(), + 'name' => 'Groceries', + 'priority' => 1, + ]), + ); + + self::assertSame( + 201, + $this->client->getResponse()->getStatusCode(), + 'POST /api/buckets under a scenario the caller owns must still succeed — the ownership scoping ' + .'must not regress the happy path.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(Bucket::class)->findOneBy(['name' => 'Groceries']); + + self::assertNotNull( + $persisted, + 'A successful POST /api/buckets under the caller\'s own scenario must persist the new Bucket.', + ); + } + + public function testAnonymousGetCollectionIsRejectedRatherThanReturningUnfilteredBuckets(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $this->persistBucket($othersScenario, 'Their Rent', 1); + + $this->client->request('GET', '/api/buckets', server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 401, + $this->client->getResponse()->getStatusCode(), + 'GET /api/buckets without authentication must be rejected by the security layer, not served an ' + .'unfiltered collection — the ownership extension no-ops for anonymous requests, so the resource-level ' + .'security wall must reject first.', + ); + } + + public function testAnonymousGetItemIsRejectedRatherThanResolvingBucket(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $othersBucket = $this->persistBucket($othersScenario, 'Their Rent', 1); + + $this->client->request('GET', '/api/buckets/'.$othersBucket->getId(), server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 401, + $this->client->getResponse()->getStatusCode(), + 'GET /api/buckets/{id} without authentication must be rejected by the security layer.', + ); + } +} diff --git a/tests/Functional/ProjectionPreviewApiTest.php b/tests/Functional/ProjectionPreviewApiTest.php index 110e399..c425523 100644 --- a/tests/Functional/ProjectionPreviewApiTest.php +++ b/tests/Functional/ProjectionPreviewApiTest.php @@ -25,9 +25,12 @@ final class ProjectionPreviewApiTest extends WebTestCase private const EMAIL = 'projection-preview-api-test@example.com'; private const PASSWORD = 'correct-horse-battery-staple'; + private const OTHER_EMAIL = 'projection-preview-api-other@example.com'; + private KernelBrowser $client; private EntityManagerInterface $em; private User $user; + private User $otherUser; protected function setUp(): void { @@ -45,8 +48,13 @@ final class ProjectionPreviewApiTest extends WebTestCase $this->user = new User(); $this->user->setEmail(self::EMAIL); $this->user->setPassword($hasher->hashPassword($this->user, self::PASSWORD)); - $this->em->persist($this->user); + + $this->otherUser = new User(); + $this->otherUser->setEmail(self::OTHER_EMAIL); + $this->otherUser->setPassword($hasher->hashPassword($this->otherUser, 'some-other-password')); + $this->em->persist($this->otherUser); + $this->em->flush(); } @@ -64,11 +72,11 @@ final class ProjectionPreviewApiTest extends WebTestCase ); } - private function persistScenario(string $name = 'Household Budget'): Scenario + private function persistScenario(string $name = 'Household Budget', ?User $owner = null): Scenario { $scenario = new Scenario(); $scenario->setName($name); - $scenario->setOwner($this->user); + $scenario->setOwner($owner ?? $this->user); $this->em->persist($scenario); $this->em->flush(); @@ -257,6 +265,54 @@ final class ProjectionPreviewApiTest extends WebTestCase ); } + public function testPreviewForAScenarioOwnedBySomeoneElseReturnsNotFound(): void + { + $othersScenario = $this->persistScenario('Someone Else\'s Fund', $this->otherUser); + + $this->login(); + + $this->client->request( + 'POST', + $this->previewUri($othersScenario), + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode(['amount' => 8000]), + ); + + self::assertSame( + 404, + $this->client->getResponse()->getStatusCode(), + 'POST .../projections/preview for a scenario owned by another user must return 404, not 403 — ' + .'no existence oracle for scenarios you do not own, matching the rest of #50.', + ); + } + + public function testPreviewForTheCallersOwnScenarioStillSucceeds(): void + { + $myScenario = $this->persistScenario('My Fund', $this->user); + + $this->login(); + + $this->client->request( + 'POST', + $this->previewUri($myScenario), + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode(['amount' => 8000]), + ); + + self::assertSame( + 200, + $this->client->getResponse()->getStatusCode(), + 'POST .../projections/preview for the caller\'s own scenario must still succeed — ownership ' + .'enforcement must not regress the happy path.', + ); + } + public function testPreviewWithMissingAmountIsRejectedAsUnprocessable(): void { $scenario = $this->persistScenario(); diff --git a/tests/State/PreviewProcessorTest.php b/tests/State/PreviewProcessorTest.php index 1af3811..7b3d6fc 100644 --- a/tests/State/PreviewProcessorTest.php +++ b/tests/State/PreviewProcessorTest.php @@ -2,9 +2,12 @@ namespace App\Tests\State; +use ApiPlatform\Metadata\Post; +use App\ApiResource\ProjectionPreviewInput; use App\ApiResource\ProjectionPreviewOutput; use App\Entity\Bucket; use App\Entity\Scenario; +use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; use App\Repository\BucketRepository; @@ -12,6 +15,8 @@ use App\Repository\ScenarioRepository; use App\State\PreviewProcessor; use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; +use Symfony\Bundle\SecurityBundle\Security; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** * Pure-unit complement to tests/Functional/ProjectionPreviewApiTest.php. @@ -45,6 +50,7 @@ final class PreviewProcessorTest extends TestCase $this->provider = new PreviewProcessor( $this->createStub(ScenarioRepository::class), $this->bucketRepository, + $this->createStub(Security::class), ); } @@ -127,6 +133,51 @@ final class PreviewProcessorTest extends TestCase self::assertInstanceOf(ProjectionPreviewOutput::class, $output); } + /** + * SEAM PINNED (#50 Story 4): unlike Stories 1-3, the preview path does NOT go + * through a Doctrine query extension — it goes through this processor's own + * `process()`. Ownership therefore has to be enforced HERE, not via a query + * filter. This test calls `process()` directly (not `buildOutput()`) because + * the owner check needs the full scenario-resolution flow, including the + * current authenticated user — pinning the behavioural outcome regardless of + * exactly where inside process() the comparison lives. + * + * Convention followed: `Symfony\Bundle\SecurityBundle\Security` is the + * project's established way to obtain the current user in a processor (see + * `App\State\ScenarioOwnerProcessor`) — assumed here as the collaborator the + * processor will be constructed with, consistent with that existing pattern. + */ + public function testProcessThrowsNotFoundWhenTheScenarioIsOwnedBySomeoneElse(): void + { + $owner = (new User())->setEmail('owner@example.com')->setPassword('irrelevant-hash'); + $intruder = (new User())->setEmail('intruder@example.com')->setPassword('irrelevant-hash'); + + $scenario = $this->makeScenario()->setOwner($owner); + + $scenarioRepository = $this->createStub(ScenarioRepository::class); + $scenarioRepository->method('find')->willReturn($scenario); + + $security = $this->createStub(Security::class); + $security->method('getUser')->willReturn($intruder); + + $processor = new PreviewProcessor( + $scenarioRepository, + $this->createStub(BucketRepository::class), + $security, + ); + + $input = new ProjectionPreviewInput(); + $input->amount = 1000; + + $this->expectException(NotFoundHttpException::class); + + $processor->process( + $input, + new Post(), + ['scenario' => (string) $scenario->getId()], + ); + } + private function makeScenario(): Scenario { return (new Scenario())->setName('Household Budget'); -- 2.45.2 From 473b88b54e6361f1b7048ffa60c7ae1507b632bf Mon Sep 17 00:00:00 2001 From: myrmidex Date: Thu, 25 Jun 2026 23:52:58 +0200 Subject: [PATCH 4/4] 50 - Scope streams to the owner transitively via scenario --- src/Doctrine/AbstractOwnerScopeExtension.php | 94 ++++ src/Doctrine/BucketOwnerExtension.php | 65 +-- src/Doctrine/ScenarioOwnerExtension.php | 64 +-- src/Doctrine/StreamOwnerExtension.php | 39 ++ tests/Concerns/CreatesUsers.php | 30 ++ tests/Entity/BucketPersistenceTest.php | 18 +- tests/Entity/ScenarioPersistenceTest.php | 18 +- tests/Entity/StreamPersistenceTest.php | 18 +- tests/Functional/StreamOwnerApiTest.php | 412 ++++++++++++++++++ tests/Repository/BucketRepositoryTest.php | 14 +- .../SingleOverflowPerScenarioTest.php | 18 +- 11 files changed, 615 insertions(+), 175 deletions(-) create mode 100644 src/Doctrine/AbstractOwnerScopeExtension.php create mode 100644 src/Doctrine/StreamOwnerExtension.php create mode 100644 tests/Concerns/CreatesUsers.php create mode 100644 tests/Functional/StreamOwnerApiTest.php diff --git a/src/Doctrine/AbstractOwnerScopeExtension.php b/src/Doctrine/AbstractOwnerScopeExtension.php new file mode 100644 index 0000000..954c559 --- /dev/null +++ b/src/Doctrine/AbstractOwnerScopeExtension.php @@ -0,0 +1,94 @@ +.owner = :currentUser`. Non-owned rows become unresolvable, + * so API Platform's provider 404s naturally on item reads and silently filters + * them out of collections — no 403, no existence oracle. + * + * Subclasses declare which resource they guard (getResourceClass) and which alias + * carries the `owner` column (ownerAlias) — resolving it directly for a resource + * that owns the column, or JOINing toward the owning resource and returning that + * join's alias for transitively-owned resources. + */ +abstract class AbstractOwnerScopeExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface +{ + public function __construct( + private readonly Security $security, + ) { + } + + public function applyToCollection( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + ?Operation $operation = null, + array $context = [], + ): void { + $this->addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); + } + + public function applyToItem( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + array $identifiers, + ?Operation $operation = null, + array $context = [], + ): void { + $this->addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); + } + + /** + * The resource class this extension scopes. The extension fires for every + * resource's queries, so it must no-op for anything but its own. + * + * @return class-string + */ + abstract protected function getResourceClass(): string; + + /** + * Returns the query alias whose `owner` column identifies the resource's + * owner. Implementations that own the column return the root alias; those + * owned transitively JOIN toward the owning resource (mutating the builder) + * and return that join's alias. + */ + abstract protected function ownerAlias( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $rootAlias, + ): string; + + private function addOwnerWhere( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + ): void { + if ($this->getResourceClass() !== $resourceClass) { + return; + } + + $user = $this->security->getUser(); + if (!$user instanceof User) { + return; + } + + $rootAlias = $queryBuilder->getRootAliases()[0]; + $ownerAlias = $this->ownerAlias($queryBuilder, $queryNameGenerator, $rootAlias); + $param = $queryNameGenerator->generateParameterName('currentUser'); + + $queryBuilder + ->andWhere(\sprintf('%s.owner = :%s', $ownerAlias, $param)) + ->setParameter($param, $user); + } +} diff --git a/src/Doctrine/BucketOwnerExtension.php b/src/Doctrine/BucketOwnerExtension.php index 23cbc8b..bf9f30d 100644 --- a/src/Doctrine/BucketOwnerExtension.php +++ b/src/Doctrine/BucketOwnerExtension.php @@ -2,73 +2,32 @@ namespace App\Doctrine; -use ApiPlatform\Doctrine\Orm\Extension\QueryCollectionExtensionInterface; -use ApiPlatform\Doctrine\Orm\Extension\QueryItemExtensionInterface; use ApiPlatform\Doctrine\Orm\Util\QueryNameGeneratorInterface; -use ApiPlatform\Metadata\Operation; use App\Entity\Bucket; -use App\Entity\User; use Doctrine\ORM\QueryBuilder; -use Symfony\Bundle\SecurityBundle\Security; /** * Scopes every Bucket read to the authenticated owner. A Bucket has no direct * owner column — ownership is transitive via its Scenario — so this JOINs the - * `scenario` relation and appends `WHERE scenario.owner = :currentUser`. - * Non-owned rows become unresolvable, so API Platform's provider 404s naturally - * on item reads and silently filters them out of collections — no 403, no - * existence oracle. + * `scenario` relation and filters on that join's `owner`. + * + * @see AbstractOwnerScopeExtension for the 404-not-403 mechanism. */ -final class BucketOwnerExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface +final class BucketOwnerExtension extends AbstractOwnerScopeExtension { - public function __construct( - private readonly Security $security, - ) { + protected function getResourceClass(): string + { + return Bucket::class; } - public function applyToCollection( + protected function ownerAlias( QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, - string $resourceClass, - ?Operation $operation = null, - array $context = [], - ): void { - $this->addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); - } - - public function applyToItem( - QueryBuilder $queryBuilder, - QueryNameGeneratorInterface $queryNameGenerator, - string $resourceClass, - array $identifiers, - ?Operation $operation = null, - array $context = [], - ): void { - $this->addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); - } - - private function addOwnerWhere( - QueryBuilder $queryBuilder, - QueryNameGeneratorInterface $queryNameGenerator, - string $resourceClass, - ): void { - // This extension fires for every resource's queries — only act on Bucket. - if (Bucket::class !== $resourceClass) { - return; - } - - $user = $this->security->getUser(); - if (!$user instanceof User) { - return; - } - - $rootAlias = $queryBuilder->getRootAliases()[0]; + string $rootAlias, + ): string { $scenarioAlias = $queryNameGenerator->generateJoinAlias('scenario'); - $param = $queryNameGenerator->generateParameterName('currentUser'); + $queryBuilder->innerJoin(\sprintf('%s.scenario', $rootAlias), $scenarioAlias); - $queryBuilder - ->innerJoin(\sprintf('%s.scenario', $rootAlias), $scenarioAlias) - ->andWhere(\sprintf('%s.owner = :%s', $scenarioAlias, $param)) - ->setParameter($param, $user); + return $scenarioAlias; } } diff --git a/src/Doctrine/ScenarioOwnerExtension.php b/src/Doctrine/ScenarioOwnerExtension.php index 62b9bf6..69ff909 100644 --- a/src/Doctrine/ScenarioOwnerExtension.php +++ b/src/Doctrine/ScenarioOwnerExtension.php @@ -2,68 +2,28 @@ namespace App\Doctrine; -use ApiPlatform\Doctrine\Orm\Extension\QueryCollectionExtensionInterface; -use ApiPlatform\Doctrine\Orm\Extension\QueryItemExtensionInterface; use ApiPlatform\Doctrine\Orm\Util\QueryNameGeneratorInterface; -use ApiPlatform\Metadata\Operation; use App\Entity\Scenario; -use App\Entity\User; use Doctrine\ORM\QueryBuilder; -use Symfony\Bundle\SecurityBundle\Security; /** - * Scopes every Scenario read to the authenticated owner by appending - * `WHERE owner = :currentUser`. Non-owned rows become unresolvable, so - * API Platform's provider 404s naturally — no 403, no existence oracle. + * Scopes every Scenario read to the authenticated owner. Scenario owns the + * `owner` column directly, so the filter applies straight to the root alias. + * + * @see AbstractOwnerScopeExtension for the 404-not-403 mechanism. */ -final class ScenarioOwnerExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface +final class ScenarioOwnerExtension extends AbstractOwnerScopeExtension { - public function __construct( - private readonly Security $security, - ) { + protected function getResourceClass(): string + { + return Scenario::class; } - public function applyToCollection( + protected function ownerAlias( QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, - string $resourceClass, - ?Operation $operation = null, - array $context = [], - ): void { - $this->addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); - } - - public function applyToItem( - QueryBuilder $queryBuilder, - QueryNameGeneratorInterface $queryNameGenerator, - string $resourceClass, - array $identifiers, - ?Operation $operation = null, - array $context = [], - ): void { - $this->addOwnerWhere($queryBuilder, $queryNameGenerator, $resourceClass); - } - - private function addOwnerWhere( - QueryBuilder $queryBuilder, - QueryNameGeneratorInterface $queryNameGenerator, - string $resourceClass, - ): void { - // This extension fires for every resource's queries — only act on Scenario. - if (Scenario::class !== $resourceClass) { - return; - } - - $user = $this->security->getUser(); - if (!$user instanceof User) { - return; - } - - $rootAlias = $queryBuilder->getRootAliases()[0]; - $param = $queryNameGenerator->generateParameterName('currentUser'); - - $queryBuilder - ->andWhere(\sprintf('%s.owner = :%s', $rootAlias, $param)) - ->setParameter($param, $user); + string $rootAlias, + ): string { + return $rootAlias; } } diff --git a/src/Doctrine/StreamOwnerExtension.php b/src/Doctrine/StreamOwnerExtension.php new file mode 100644 index 0000000..cd347d4 --- /dev/null +++ b/src/Doctrine/StreamOwnerExtension.php @@ -0,0 +1,39 @@ +generateJoinAlias('scenario'); + $queryBuilder->innerJoin(\sprintf('%s.scenario', $rootAlias), $scenarioAlias); + + return $scenarioAlias; + } +} diff --git a/tests/Concerns/CreatesUsers.php b/tests/Concerns/CreatesUsers.php new file mode 100644 index 0000000..42beebc --- /dev/null +++ b/tests/Concerns/CreatesUsers.php @@ -0,0 +1,30 @@ +em. + */ +trait CreatesUsers +{ + private function persistOwner(): User + { + static $counter = 0; + ++$counter; + + $prefix = strtolower(str_replace('\\', '-', static::class)); + + $owner = new User(); + $owner->setEmail(\sprintf('%s-%d@example.com', $prefix, $counter)); + $owner->setPassword('irrelevant-hash'); + + $this->em->persist($owner); + + return $owner; + } +} diff --git a/tests/Entity/BucketPersistenceTest.php b/tests/Entity/BucketPersistenceTest.php index ac7d0a8..529b567 100644 --- a/tests/Entity/BucketPersistenceTest.php +++ b/tests/Entity/BucketPersistenceTest.php @@ -4,9 +4,9 @@ namespace App\Tests\Entity; use App\Entity\Bucket; use App\Entity\Scenario; -use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; +use App\Tests\Concerns\CreatesUsers; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; @@ -14,6 +14,8 @@ use Symfony\Component\Uid\UuidV7; final class BucketPersistenceTest extends KernelTestCase { + use CreatesUsers; + private EntityManagerInterface $em; protected function setUp(): void @@ -22,20 +24,6 @@ final class BucketPersistenceTest extends KernelTestCase $this->em = self::getContainer()->get(EntityManagerInterface::class); } - private function persistOwner(): User - { - static $counter = 0; - ++$counter; - - $owner = new User(); - $owner->setEmail(\sprintf('bucket-persistence-test-%d@example.com', $counter)); - $owner->setPassword('irrelevant-hash'); - - $this->em->persist($owner); - - return $owner; - } - public function testItPersistsABucketLinkedToItsScenario(): void { $scenario = new Scenario(); diff --git a/tests/Entity/ScenarioPersistenceTest.php b/tests/Entity/ScenarioPersistenceTest.php index 3fee263..bf8b332 100644 --- a/tests/Entity/ScenarioPersistenceTest.php +++ b/tests/Entity/ScenarioPersistenceTest.php @@ -3,14 +3,16 @@ namespace App\Tests\Entity; use App\Entity\Scenario; -use App\Entity\User; use App\Enum\DistributionMode; +use App\Tests\Concerns\CreatesUsers; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Uid\UuidV7; final class ScenarioPersistenceTest extends KernelTestCase { + use CreatesUsers; + private EntityManagerInterface $em; protected function setUp(): void @@ -19,20 +21,6 @@ final class ScenarioPersistenceTest extends KernelTestCase $this->em = self::getContainer()->get(EntityManagerInterface::class); } - private function persistOwner(): User - { - static $counter = 0; - ++$counter; - - $owner = new User(); - $owner->setEmail(\sprintf('scenario-persistence-test-%d@example.com', $counter)); - $owner->setPassword('irrelevant-hash'); - - $this->em->persist($owner); - - return $owner; - } - public function testItPersistsAScenarioWithAUuidId(): void { $scenario = new Scenario(); diff --git a/tests/Entity/StreamPersistenceTest.php b/tests/Entity/StreamPersistenceTest.php index 9a4c2ae..327a3bc 100644 --- a/tests/Entity/StreamPersistenceTest.php +++ b/tests/Entity/StreamPersistenceTest.php @@ -5,16 +5,18 @@ namespace App\Tests\Entity; use App\Entity\Bucket; use App\Entity\Scenario; use App\Entity\Stream; -use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; use App\Enum\StreamFrequency; use App\Enum\StreamType; +use App\Tests\Concerns\CreatesUsers; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; final class StreamPersistenceTest extends KernelTestCase { + use CreatesUsers; + private EntityManagerInterface $em; protected function setUp(): void @@ -23,20 +25,6 @@ final class StreamPersistenceTest extends KernelTestCase $this->em = self::getContainer()->get(EntityManagerInterface::class); } - private function persistOwner(): User - { - static $counter = 0; - ++$counter; - - $owner = new User(); - $owner->setEmail(\sprintf('stream-persistence-test-%d@example.com', $counter)); - $owner->setPassword('irrelevant-hash'); - - $this->em->persist($owner); - - return $owner; - } - public function testItPersistsAStreamLinkedToItsScenarioAndBucket(): void { $scenario = new Scenario(); diff --git a/tests/Functional/StreamOwnerApiTest.php b/tests/Functional/StreamOwnerApiTest.php new file mode 100644 index 0000000..ed445a4 --- /dev/null +++ b/tests/Functional/StreamOwnerApiTest.php @@ -0,0 +1,412 @@ +client = static::createClient(); + + /** @var EntityManagerInterface $em */ + $em = static::getContainer()->get(EntityManagerInterface::class); + $this->em = $em; + + $factory = new PasswordHasherFactory([ + User::class => new NativePasswordHasher(cost: 4), + ]); + $hasher = new UserPasswordHasher($factory); + + $this->caller = new User(); + $this->caller->setEmail(self::EMAIL); + $this->caller->setPassword($hasher->hashPassword($this->caller, self::PASSWORD)); + $this->em->persist($this->caller); + + $this->otherUser = new User(); + $this->otherUser->setEmail(self::OTHER_EMAIL); + $this->otherUser->setPassword($hasher->hashPassword($this->otherUser, 'some-other-password')); + $this->em->persist($this->otherUser); + + $this->em->flush(); + } + + private function login(): void + { + $this->client->jsonRequest('POST', '/api/login', [ + 'username' => self::EMAIL, + 'password' => self::PASSWORD, + ]); + + self::assertLessThan( + 300, + $this->client->getResponse()->getStatusCode(), + 'Test setup precondition: login must succeed before exercising the Stream API.', + ); + } + + private function persistScenarioFor(User $owner, string $name): Scenario + { + $scenario = new Scenario(); + $scenario->setName($name); + $scenario->setOwner($owner); + + $this->em->persist($scenario); + $this->em->flush(); + + return $scenario; + } + + private function persistBucket(Scenario $scenario, string $name, int $priority): Bucket + { + $bucket = new Bucket(); + $bucket->setScenario($scenario); + $bucket->setName($name); + $bucket->setPriority($priority); + + $this->em->persist($bucket); + $this->em->flush(); + + return $bucket; + } + + private function persistStream(Scenario $scenario, string $name, ?Bucket $bucket = null): Stream + { + $stream = new Stream(); + $stream->setScenario($scenario); + $stream->setBucket($bucket); + $stream->setName($name); + $stream->setAmount(1000); + $stream->setType(StreamType::INCOME); + $stream->setFrequency(StreamFrequency::MONTHLY); + $stream->setStartDate(new \DateTimeImmutable('2026-01-01')); + + $this->em->persist($stream); + $this->em->flush(); + + return $stream; + } + + /** + * Minimal valid Stream POST payload, scoped under the given scenario IRI + * (and optionally a bucket IRI). Mirrors the entity's required fields: + * name, amount, type, frequency, startDate, scenario. + * + * @return array + */ + private function streamPayload(string $scenarioIri, ?string $bucketIri = null, string $name = 'Paycheck'): array + { + $payload = [ + 'scenario' => $scenarioIri, + 'name' => $name, + 'amount' => 1000, + 'type' => StreamType::INCOME->value, + 'frequency' => StreamFrequency::MONTHLY->value, + 'startDate' => '2026-01-01', + ]; + + if (null !== $bucketIri) { + $payload['bucket'] = $bucketIri; + } + + return $payload; + } + + public function testGetItemOnAStreamOwnedBySomeoneElseReturnsNotFound(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $othersStream = $this->persistStream($othersScenario, 'Their Paycheck'); + + $this->login(); + + $this->client->request('GET', '/api/streams/'.$othersStream->getId(), server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 404, + $this->client->getResponse()->getStatusCode(), + 'GET /api/streams/{id} for a stream whose scenario is owned by another user must return 404, not 403 — ' + .'no existence oracle for streams you do not own, scoped transitively via scenario.owner.', + ); + } + + public function testGetCollectionOnlyReturnsStreamsWhoseScenarioIsOwnedByTheCaller(): void + { + $myScenario = $this->persistScenarioFor($this->caller, 'My Fund'); + $this->persistStream($myScenario, 'My Paycheck'); + + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $this->persistStream($othersScenario, 'Their Paycheck'); + + $this->login(); + + $this->client->request('GET', '/api/streams', server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + $response = $this->client->getResponse(); + + self::assertSame( + 200, + $response->getStatusCode(), + 'GET /api/streams must return 200 for an authenticated user.', + ); + + $body = json_decode((string) $response->getContent(), true); + + self::assertIsArray($body, 'The collection response must be a JSON-LD/Hydra object.'); + self::assertArrayHasKey( + 'member', + $body, + 'A Hydra collection response must expose its items under the member key.', + ); + + $names = array_column($body['member'], 'name'); + + self::assertContains( + 'My Paycheck', + $names, + 'GET /api/streams must include streams whose scenario is owned by the authenticated caller.', + ); + self::assertNotContains( + 'Their Paycheck', + $names, + 'GET /api/streams must NOT include streams whose scenario is owned by another user, ' + .'even though the stream itself has no direct owner column — ownership is transitive via scenario.owner.', + ); + } + + public function testAnonymousGetCollectionIsRejectedRatherThanReturningUnfilteredStreams(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $this->persistStream($othersScenario, 'Their Paycheck'); + + $this->client->request('GET', '/api/streams', server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 401, + $this->client->getResponse()->getStatusCode(), + 'GET /api/streams without authentication must be rejected by the security layer, not served an ' + .'unfiltered collection — the ownership extension no-ops for anonymous requests, so the resource-level ' + .'security wall must reject first.', + ); + } + + public function testAnonymousGetItemIsRejectedRatherThanResolvingStream(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $othersStream = $this->persistStream($othersScenario, 'Their Paycheck'); + + $this->client->request('GET', '/api/streams/'.$othersStream->getId(), server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ]); + + self::assertSame( + 401, + $this->client->getResponse()->getStatusCode(), + 'GET /api/streams/{id} without authentication must be rejected by the security layer.', + ); + } + + /** + * PINS OBSERVED (PRE-#50-STORY-5) BEHAVIOUR — flagged for review, not a locked contract. + * + * Mirrors BucketOwnerApiTest::testPostUnderAForeignScenarioCurrentlyReturnsBadRequestNotFound. + * A foreign scenario IRI is already unresolvable to a non-owner (Scenario's own + * ownership extension, #50 Story 2), so API Platform's IRI denormalizer raises an + * "Item not found" failure during deserialization, surfaced as 400 — BEFORE any + * Stream-specific extension code runs. This test asserts the OBSERVED 400, not a + * desired 404. If a 404 contract is required, it needs an explicit normalizing + * guard — an implementation decision, not assumed here. + */ + public function testPostUnderAForeignScenarioCurrentlyReturnsBadRequest(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + + $this->login(); + + $this->client->request( + 'POST', + '/api/streams', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode($this->streamPayload( + '/api/scenarios/'.$othersScenario->getId(), + name: 'Sneaky Stream', + )), + ); + + self::assertSame( + 400, + $this->client->getResponse()->getStatusCode(), + 'POST /api/streams under a scenario IRI the caller does not own currently resolves to 400 ' + .'(unresolvable IRI denormalization failure), not 201 — pinning the observed behaviour. ' + .'See class docblock above this test: a 404 may require an explicit normalizing guard.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(Stream::class)->findOneBy(['name' => 'Sneaky Stream']); + + self::assertNull( + $persisted, + 'A rejected POST under a foreign scenario must never persist a Stream.', + ); + } + + /** + * THE WRINKLE — Story 5's distinguishing case. Unlike Bucket (which only points at + * a scenario), Stream POST can carry a SEPARATE bucket IRI. Even when the scenario + * IRI belongs to the caller, a foreign bucket IRI must not be acceptable — otherwise + * a non-owner could attach a stream to a bucket they don't own (cross-tenant write + * via the optional bucket leg). This test pins WHATEVER status the app currently + * returns; if it is 201, that is a real ownership gap — the bucket relation resolves + * despite the caller not owning it, and the create must be rejected before persisting. + */ + public function testPostUnderOwnScenarioButForeignBucketIsRejected(): void + { + $myScenario = $this->persistScenarioFor($this->caller, 'My Fund'); + + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $othersBucket = $this->persistBucket($othersScenario, 'Their Rent', 1); + + $this->login(); + + $this->client->request( + 'POST', + '/api/streams', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode($this->streamPayload( + '/api/scenarios/'.$myScenario->getId(), + bucketIri: '/api/buckets/'.$othersBucket->getId(), + name: 'Cross Tenant Stream', + )), + ); + + self::assertSame( + 400, + $this->client->getResponse()->getStatusCode(), + 'POST /api/streams under the caller\'s OWN scenario but referencing a bucket IRI owned by ANOTHER ' + .'user must be rejected, not created — a non-owner must not be able to attach a stream to someone ' + .'else\'s bucket. Pinning the OBSERVED 400 (not Stream-owned logic — an incidental side-effect of ' + .'BucketOwnerExtension making the foreign bucket IRI unresolvable during denormalization, same ' + .'mechanism as the foreign-scenario case above). If this ever regresses to 403, that is an ' + .'existence-oracle leak (confirms the bucket exists but isn\'t yours) — exactly what #50 exists to ' + .'prevent. If it regresses to 201, that is a real ownership gap: the bucket relation resolved ' + .'despite the caller not owning it.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(Stream::class)->findOneBy(['name' => 'Cross Tenant Stream']); + + self::assertNull( + $persisted, + 'A rejected POST referencing a foreign bucket must never persist a Stream.', + ); + } + + public function testPostUnderTheCallersOwnScenarioAndOwnBucketSucceeds(): void + { + $myScenario = $this->persistScenarioFor($this->caller, 'My Fund'); + $myBucket = $this->persistBucket($myScenario, 'My Rent', 1); + + $this->login(); + + $this->client->request( + 'POST', + '/api/streams', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode($this->streamPayload( + '/api/scenarios/'.$myScenario->getId(), + bucketIri: '/api/buckets/'.$myBucket->getId(), + name: 'Paycheck', + )), + ); + + self::assertSame( + 201, + $this->client->getResponse()->getStatusCode(), + 'POST /api/streams under the caller\'s own scenario AND own bucket must succeed — the ownership ' + .'scoping must not regress the happy path.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(Stream::class)->findOneBy(['name' => 'Paycheck']); + + self::assertNotNull( + $persisted, + 'A successful POST /api/streams under the caller\'s own scenario and bucket must persist the new Stream.', + ); + } + + public function testPostUnderTheCallersOwnScenarioWithNoBucketSucceeds(): void + { + $myScenario = $this->persistScenarioFor($this->caller, 'My Fund'); + + $this->login(); + + $this->client->request( + 'POST', + '/api/streams', + server: [ + 'CONTENT_TYPE' => 'application/ld+json', + 'HTTP_ACCEPT' => 'application/ld+json', + ], + content: json_encode($this->streamPayload( + '/api/scenarios/'.$myScenario->getId(), + name: 'Unassigned Income', + )), + ); + + self::assertSame( + 201, + $this->client->getResponse()->getStatusCode(), + 'POST /api/streams under the caller\'s own scenario with NO bucket (null is valid — bucket is ' + .'optional) must succeed.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(Stream::class)->findOneBy(['name' => 'Unassigned Income']); + + self::assertNotNull( + $persisted, + 'A successful POST /api/streams with a null bucket must persist the new Stream.', + ); + } +} diff --git a/tests/Repository/BucketRepositoryTest.php b/tests/Repository/BucketRepositoryTest.php index 6670cc6..c5a97e5 100644 --- a/tests/Repository/BucketRepositoryTest.php +++ b/tests/Repository/BucketRepositoryTest.php @@ -4,15 +4,17 @@ namespace App\Tests\Repository; use App\Entity\Bucket; use App\Entity\Scenario; -use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; use App\Repository\BucketRepository; +use App\Tests\Concerns\CreatesUsers; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; final class BucketRepositoryTest extends KernelTestCase { + use CreatesUsers; + private EntityManagerInterface $em; private BucketRepository $repository; @@ -142,17 +144,9 @@ final class BucketRepositoryTest extends KernelTestCase private function persistScenario(string $name = 'Household Budget'): Scenario { - static $counter = 0; - ++$counter; - - $owner = new User(); - $owner->setEmail(\sprintf('bucket-repository-test-%d@example.com', $counter)); - $owner->setPassword('irrelevant-hash'); - $this->em->persist($owner); - $scenario = new Scenario(); $scenario->setName($name); - $scenario->setOwner($owner); + $scenario->setOwner($this->persistOwner()); $this->em->persist($scenario); $this->em->flush(); diff --git a/tests/Validator/SingleOverflowPerScenarioTest.php b/tests/Validator/SingleOverflowPerScenarioTest.php index d9ac68f..c451c99 100644 --- a/tests/Validator/SingleOverflowPerScenarioTest.php +++ b/tests/Validator/SingleOverflowPerScenarioTest.php @@ -4,9 +4,9 @@ namespace App\Tests\Validator; use App\Entity\Bucket; use App\Entity\Scenario; -use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; +use App\Tests\Concerns\CreatesUsers; use App\Validator\SingleOverflowPerScenario; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; @@ -15,6 +15,8 @@ use Symfony\Component\Validator\Validator\ValidatorInterface; final class SingleOverflowPerScenarioTest extends KernelTestCase { + use CreatesUsers; + private EntityManagerInterface $em; private ValidatorInterface $validator; @@ -25,20 +27,6 @@ final class SingleOverflowPerScenarioTest extends KernelTestCase $this->validator = self::getContainer()->get(ValidatorInterface::class); } - private function persistOwner(): User - { - static $counter = 0; - ++$counter; - - $owner = new User(); - $owner->setEmail(\sprintf('single-overflow-per-scenario-test-%d@example.com', $counter)); - $owner->setPassword('irrelevant-hash'); - - $this->em->persist($owner); - - return $owner; - } - public function testSecondOverflowBucketOnTheSameScenarioRaisesAViolation(): void { $scenario = new Scenario(); -- 2.45.2