50 - Scope Bucket reads to the owning Scenario's owner

This commit is contained in:
myrmidex 2026-06-25 20:34:45 +02:00
parent 0fb259a601
commit 2993385b48
5 changed files with 478 additions and 3 deletions

View file

@ -0,0 +1,74 @@
<?php
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.
*/
final class BucketOwnerExtension 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);
}
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);
}
}

View file

@ -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);
}

View file

@ -0,0 +1,282 @@
<?php
namespace App\Tests\Functional;
use App\Entity\Bucket;
use App\Entity\Scenario;
use App\Entity\User;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactory;
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasher;
final class BucketOwnerApiTest extends WebTestCase
{
private const EMAIL = 'bucket-owner-api-test@example.com';
private const PASSWORD = 'correct-horse-battery-staple';
private const OTHER_EMAIL = 'bucket-owner-api-other@example.com';
private KernelBrowser $client;
private EntityManagerInterface $em;
private User $caller;
private User $otherUser;
protected function setUp(): void
{
$this->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.',
);
}
}

View file

@ -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();

View file

@ -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');