Skip to content

Commit 7af65aa

Browse files
soyukasoyuka
committed
fix(graphql): property security might be cached w/ different objects
1 parent 60747cc commit 7af65aa

File tree

6 files changed

+288
-1
lines changed

6 files changed

+288
-1
lines changed

‎src/GraphQl/Serializer/ItemNormalizer.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ public function normalize(mixed $object, ?string $format = null, array $context
8989

9090
if ($this->isCacheKeySafe($context)) {
9191
$context['cache_key'] = $this->getCacheKey($format, $context);
92+
} else {
93+
$context['cache_key'] = false;
9294
}
9395

9496
unset($context['operation_name'], $context['operation']); // Remove operation and operation_name only when cache key has been created

‎tests/Fixtures/TestBundle/Document/SecuredDummyCollection.php

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\Document;
15+
16+
use ApiPlatform\Metadata\ApiProperty;
17+
use ApiPlatform\Metadata\ApiResource;
18+
use ApiPlatform\Metadata\GraphQl\Query;
19+
use ApiPlatform\Metadata\GraphQl\QueryCollection;
20+
use ApiPlatform\Metadata\NotExposed;
21+
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
22+
23+
/**
24+
* Secured resource.
25+
*/
26+
#[ApiResource(
27+
operations: [
28+
new NotExposed(),
29+
],
30+
graphQlOperations: [
31+
new Query(),
32+
new QueryCollection(),
33+
],
34+
security: 'is_granted(\'ROLE_USER\')'
35+
)]
36+
#[ODM\Document]
37+
class SecuredDummyCollection
38+
{
39+
#[ODM\Id(strategy: 'AUTO', type: 'integer')]
40+
public ?int $id = null;
41+
42+
/**
43+
* @var string The title
44+
*/
45+
#[ODM\Field]
46+
public string $title;
47+
48+
/**
49+
* @var string Secret property, only readable/writable by owners
50+
*/
51+
#[ApiProperty(security: 'object == null or object.owner == user', securityPostDenormalize: 'object.owner == user')]
52+
#[ODM\Field]
53+
public ?string $ownerOnlyProperty = null;
54+
55+
/**
56+
* @var string The owner
57+
*/
58+
#[ODM\Field]
59+
public string $owner;
60+
}

‎tests/Fixtures/TestBundle/Document/SecuredDummyCollectionParent.php

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\Document;
15+
16+
use ApiPlatform\Metadata\ApiResource;
17+
use ApiPlatform\Metadata\GraphQl\Query;
18+
use ApiPlatform\Metadata\GraphQl\QueryCollection;
19+
use ApiPlatform\Metadata\NotExposed;
20+
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
21+
22+
/**
23+
* Secured resource.
24+
*/
25+
#[ApiResource(
26+
operations: [
27+
new NotExposed(),
28+
],
29+
graphQlOperations: [
30+
new Query(),
31+
new QueryCollection(),
32+
],
33+
security: 'is_granted(\'ROLE_USER\')'
34+
)]
35+
#[ODM\Document]
36+
class SecuredDummyCollectionParent
37+
{
38+
#[ODM\Id]
39+
#[ODM\Field(type: 'id')]
40+
public ?string $id = null;
41+
42+
#[ODM\ReferenceOne(targetDocument: SecuredDummyCollection::class, inversedBy: 'parents')]
43+
#[ODM\Field(nullable: false)]
44+
public SecuredDummyCollection $child;
45+
}

‎tests/Fixtures/TestBundle/Entity/SecuredDummyCollection.php

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;
15+
16+
use ApiPlatform\Metadata\ApiProperty;
17+
use ApiPlatform\Metadata\ApiResource;
18+
use ApiPlatform\Metadata\GraphQl\Query;
19+
use ApiPlatform\Metadata\GraphQl\QueryCollection;
20+
use ApiPlatform\Metadata\NotExposed;
21+
use Doctrine\ORM\Mapping as ORM;
22+
23+
/**
24+
* Secured resource.
25+
*/
26+
#[ApiResource(
27+
operations: [
28+
new NotExposed(),
29+
],
30+
graphQlOperations: [
31+
new Query(),
32+
new QueryCollection(),
33+
],
34+
security: 'is_granted(\'ROLE_USER\')'
35+
)]
36+
#[ORM\Entity]
37+
class SecuredDummyCollection
38+
{
39+
#[ORM\Column(type: 'integer')]
40+
#[ORM\Id]
41+
#[ORM\GeneratedValue(strategy: 'AUTO')]
42+
public ?int $id = null;
43+
44+
/**
45+
* @var string The title
46+
*/
47+
#[ORM\Column]
48+
public string $title;
49+
50+
/**
51+
* @var string Secret property, only readable/writable by owners
52+
*/
53+
#[ApiProperty(security: 'object == null or object.owner == user', securityPostDenormalize: 'object.owner == user')]
54+
#[ORM\Column]
55+
public ?string $ownerOnlyProperty = null;
56+
57+
/**
58+
* @var string The owner
59+
*/
60+
#[ORM\Column]
61+
public string $owner;
62+
}

‎tests/Fixtures/TestBundle/Entity/SecuredDummyCollectionParent.php

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;
15+
16+
use ApiPlatform\Metadata\ApiResource;
17+
use ApiPlatform\Metadata\GraphQl\Query;
18+
use ApiPlatform\Metadata\GraphQl\QueryCollection;
19+
use ApiPlatform\Metadata\NotExposed;
20+
use Doctrine\ORM\Mapping as ORM;
21+
22+
/**
23+
* Secured resource.
24+
*/
25+
#[ApiResource(
26+
operations: [
27+
new NotExposed(),
28+
],
29+
graphQlOperations: [
30+
new Query(),
31+
new QueryCollection(),
32+
],
33+
security: 'is_granted(\'ROLE_USER\')'
34+
)]
35+
#[ORM\Entity]
36+
class SecuredDummyCollectionParent
37+
{
38+
#[ORM\Column(type: 'integer')]
39+
#[ORM\Id]
40+
#[ORM\GeneratedValue(strategy: 'AUTO')]
41+
public ?int $id = null;
42+
43+
#[ORM\ManyToOne]
44+
#[ORM\JoinColumn(nullable: false)]
45+
public SecuredDummyCollection $child;
46+
}

‎tests/Functional/GraphQl/SecurityTest.php

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,24 @@
1616
use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
1717
use ApiPlatform\Tests\Fixtures\TestBundle\Document\SecuredDummy as DocumentSecuredDummy;
1818
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\SecuredDummy;
19+
use ApiPlatform\Tests\Fixtures\TestBundle\Document\SecuredDummyCollection as DocumentSecuredDummyCollection;
20+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\SecuredDummyCollection;
21+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\SecuredDummyCollectionParent;
1922
use ApiPlatform\Tests\RecreateSchemaTrait;
2023
use ApiPlatform\Tests\SetupClassResourcesTrait;
2124

2225
final class SecurityTest extends ApiTestCase
2326
{
2427
use RecreateSchemaTrait;
2528
use SetupClassResourcesTrait;
29+
protected static ?bool $alwaysBootKernel = false;
2630

2731
/**
2832
* @return class-string[]
2933
*/
3034
public static function getResources(): array
3135
{
32-
return [SecuredDummy::class];
36+
return [SecuredDummy::class, SecuredDummyCollection::class, SecuredDummyCollectionParent::class];
3337
}
3438

3539
public function testQueryItem(): void
@@ -113,4 +117,72 @@ public function loadFixtures(string $resourceClass): void
113117
$manager->persist($s);
114118
$manager->flush();
115119
}
120+
121+
public function testQueryCollection(): void
122+
{
123+
$resource = $this->isMongoDB() ? DocumentSecuredDummyCollection::class : SecuredDummyCollection::class;
124+
$this->recreateSchema([$resource, $resource.'Parent']);
125+
$this->loadFixturesQueryCollection($resource);
126+
$client = self::createClient();
127+
128+
$response = $client->request('POST', '/graphql', ['headers' => ['Authorization' => 'Basic ZHVuZ2xhczprZXZpbg=='], 'json' => [
129+
'query' => <<<QUERY
130+
{
131+
securedDummyCollectionParents {
132+
edges {
133+
node {
134+
child {
135+
title, ownerOnlyProperty, owner
136+
}
137+
}
138+
}
139+
}
140+
}
141+
QUERY,
142+
]]);
143+
144+
$d = $response->toArray();
145+
$this->assertNull($d['data']['securedDummyCollectionParents']['edges'][1]['node']['child']['ownerOnlyProperty']);
146+
}
147+
148+
public function loadFixturesQueryCollection(string $resourceClass): void
149+
{
150+
$parentResourceClass = $resourceClass.'Parent';
151+
$container = static::$kernel->getContainer();
152+
$registry = $this->isMongoDB() ? $container->get('doctrine_mongodb') : $container->get('doctrine');
153+
$manager = $registry->getManager();
154+
$s = new $resourceClass();
155+
$s->title = 'Foo';
156+
$s->ownerOnlyProperty = 'Foo by dunglas';
157+
$s->owner = 'dunglas';
158+
$manager->persist($s);
159+
$p = new $parentResourceClass();
160+
$p->child = $s;
161+
$manager->persist($p);
162+
$s = new $resourceClass();
163+
$s->title = 'Bar';
164+
$s->ownerOnlyProperty = 'Bar by admin';
165+
$s->owner = 'admin';
166+
$manager->persist($s);
167+
$p = new $parentResourceClass();
168+
$p->child = $s;
169+
$manager->persist($p);
170+
$s = new $resourceClass();
171+
$s->title = 'Baz';
172+
$s->ownerOnlyProperty = 'Baz by dunglas';
173+
$s->owner = 'dunglas';
174+
$manager->persist($s);
175+
$p = new $parentResourceClass();
176+
$p->child = $s;
177+
$manager->persist($p);
178+
$s = new $resourceClass();
179+
$s->ownerOnlyProperty = 'Bat by admin';
180+
$s->owner = 'admin';
181+
$s->title = 'Bat';
182+
$manager->persist($s);
183+
$p = new $parentResourceClass();
184+
$p->child = $s;
185+
$manager->persist($p);
186+
$manager->flush();
187+
}
116188
}

0 commit comments

Comments
 (0)