From 0fb259a601fd59949ad8fca9cc7fe89549a9c4e6 Mon Sep 17 00:00:00 2001 From: myrmidex Date: Thu, 25 Jun 2026 19:59:40 +0200 Subject: [PATCH] 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();