Skip to content

Commit 5af717d

Browse files
vincentchalamonvincentchalamon
committed
fix: remove useless User.sub
1 parent bfb38b9 commit 5af717d

File tree

16 files changed

+41
-84
lines changed

16 files changed

+41
-84
lines changed

‎api/migrations/Version20230906094949.php

+1-3
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,9 @@ public function up(Schema $schema): void
4141
$this->addSql('COMMENT ON COLUMN review.user_id IS \'(DC2Type:uuid)\'');
4242
$this->addSql('COMMENT ON COLUMN review.book_id IS \'(DC2Type:uuid)\'');
4343
$this->addSql('COMMENT ON COLUMN review.published_at IS \'(DC2Type:datetime_immutable)\'');
44-
$this->addSql('CREATE TABLE "user" (id UUID NOT NULL, sub UUID NOT NULL, email VARCHAR(255) NOT NULL, first_name VARCHAR(255) NOT NULL, last_name VARCHAR(255) NOT NULL, roles JSON NOT NULL, PRIMARY KEY(id))');
45-
$this->addSql('CREATE UNIQUE INDEX UNIQ_8D93D649580282DC ON "user" (sub)');
44+
$this->addSql('CREATE TABLE "user" (id UUID NOT NULL, email VARCHAR(255) NOT NULL, first_name VARCHAR(255) NOT NULL, last_name VARCHAR(255) NOT NULL, roles JSON NOT NULL, PRIMARY KEY(id))');
4645
$this->addSql('CREATE UNIQUE INDEX UNIQ_8D93D649E7927C74 ON "user" (email)');
4746
$this->addSql('COMMENT ON COLUMN "user".id IS \'(DC2Type:uuid)\'');
48-
$this->addSql('COMMENT ON COLUMN "user".sub IS \'(DC2Type:uuid)\'');
4947
$this->addSql('ALTER TABLE bookmark ADD CONSTRAINT FK_DA62921DA76ED395 FOREIGN KEY (user_id) REFERENCES "user" (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
5048
$this->addSql('ALTER TABLE bookmark ADD CONSTRAINT FK_DA62921D16A2B381 FOREIGN KEY (book_id) REFERENCES book (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
5149
$this->addSql('ALTER TABLE review ADD CONSTRAINT FK_794381C6A76ED395 FOREIGN KEY (user_id) REFERENCES "user" (id) NOT DEFERRABLE INITIALLY IMMEDIATE');

‎api/src/DataFixtures/Factory/ReviewFactory.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ final class ReviewFactory extends ModelFactory
5454
* @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services
5555
*/
5656
public function __construct(
57-
private readonly ResourceHandlerInterface $resourceHandler,
57+
private readonly ?ResourceHandlerInterface $resourceHandler = null,
5858
) {
5959
parent::__construct();
6060
}
@@ -81,6 +81,10 @@ protected function initialize(): self
8181
return $this
8282
// create the resource on the OIDC server
8383
->afterPersist(function (Review $object): void {
84+
if (!$this->resourceHandler) {
85+
return;
86+
}
87+
8488
// project specification: only create resource on OIDC server for known users (john.doe and chuck.norris)
8589
if (\in_array($object->user?->email, ['[email protected]', '[email protected]'], true)) {
8690
$this->resourceHandler->create($object, $object->user, [

‎api/src/DataFixtures/Factory/UserFactory.php

-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
use App\Entity\User;
88
use Doctrine\ORM\EntityRepository;
9-
use Symfony\Component\Uid\Uuid;
109
use Zenstruck\Foundry\ModelFactory;
1110
use Zenstruck\Foundry\Proxy;
1211
use Zenstruck\Foundry\RepositoryProxy;
@@ -76,7 +75,6 @@ public function withAdmin(): self
7675
protected function getDefaults(): array
7776
{
7877
return [
79-
'sub' => Uuid::v7(),
8078
'email' => self::faker()->unique()->email(),
8179
'firstName' => self::faker()->firstName(),
8280
'lastName' => self::faker()->lastName(),

‎api/src/Entity/User.php

-8
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,6 @@ class User implements UserInterface
6363
#[ORM\Id]
6464
private ?Uuid $id = null;
6565

66-
/**
67-
* @see https://schema.org/identifier
68-
*/
69-
#[ApiProperty(types: ['https://schema.org/identifier'])]
70-
#[Groups(groups: ['User:read'])]
71-
#[ORM\Column(type: UuidType::NAME, unique: true)]
72-
public ?Uuid $sub = null;
73-
7466
/**
7567
* @see https://schema.org/email
7668
*/

‎api/src/Security/Core/UserProvider.php

-10
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
1111
use Symfony\Component\Security\Core\User\AttributesBasedUserProviderInterface;
1212
use Symfony\Component\Security\Core\User\UserInterface;
13-
use Symfony\Component\Uid\Uuid;
1413

1514
/**
1615
* @implements AttributesBasedUserProviderInterface<UserInterface|User>
@@ -46,15 +45,6 @@ public function loadUserByIdentifier(string $identifier, array $attributes = [])
4645
$user = $this->repository->findOneBy(['email' => $identifier]) ?: new User();
4746
$user->email = $identifier;
4847

49-
if (!isset($attributes['sub'])) {
50-
throw new UnsupportedUserException('Property "sub" is missing in token attributes.');
51-
}
52-
try {
53-
$user->sub = Uuid::fromString($attributes['sub']);
54-
} catch (\Throwable $e) {
55-
throw new UnsupportedUserException($e->getMessage(), $e->getCode(), $e);
56-
}
57-
5848
if (!isset($attributes['given_name'])) {
5949
throw new UnsupportedUserException('Property "given_name" is missing in token attributes.');
6050
}

‎api/tests/Api/Admin/BookTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ final class BookTest extends ApiTestCase
3333

3434
protected function setup(): void
3535
{
36-
$this->client = self::createClient(['debug' => true]);
36+
$this->client = self::createClient();
3737
}
3838

3939
#[Test]

‎api/tests/Api/Admin/UserTest.php

+1-6
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use App\Tests\Api\Security\TokenGenerator;
1313
use PHPUnit\Framework\Attributes\DataProvider;
1414
use PHPUnit\Framework\Attributes\Test;
15-
use Symfony\Component\Uid\Uuid;
1615
use Zenstruck\Foundry\FactoryCollection;
1716
use Zenstruck\Foundry\Test\Factories;
1817
use Zenstruck\Foundry\Test\ResetDatabase;
@@ -27,7 +26,7 @@ final class UserTest extends ApiTestCase
2726

2827
protected function setup(): void
2928
{
30-
$this->client = self::createClient();
29+
$this->client = self::createClient(['debug' => true]);
3130
}
3231

3332
#[Test]
@@ -156,12 +155,9 @@ public function asAUserIAmUpdatedOnLogin(): void
156155
$user = UserFactory::createOne([
157156
'firstName' => 'John',
158157
'lastName' => 'DOE',
159-
'sub' => Uuid::fromString('b5c5bff1-5b5f-4a73-8fc8-4ea8f18586a9'),
160158
])->disableAutoRefresh();
161159

162-
$sub = Uuid::v7()->__toString();
163160
$token = self::getContainer()->get(TokenGenerator::class)->generateToken([
164-
'sub' => $sub,
165161
'email' => $user->email,
166162
'given_name' => 'Chuck',
167163
'family_name' => 'NORRIS',
@@ -176,6 +172,5 @@ public function asAUserIAmUpdatedOnLogin(): void
176172
self::assertEquals('Chuck', $user->firstName);
177173
self::assertEquals('NORRIS', $user->lastName);
178174
self::assertEquals('Chuck NORRIS', $user->getName());
179-
self::assertEquals($sub, $user->sub);
180175
}
181176
}

‎api/tests/Api/Admin/schemas/Review/collection.json

-7
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,6 @@
131131
"type": "string",
132132
"pattern": "^https://schema.org/Person$"
133133
},
134-
"sub": {
135-
"externalDocs": {
136-
"url": "https:\/\/schema.org\/identifier"
137-
},
138-
"type": "string"
139-
},
140134
"firstName": {
141135
"description": "The givenName of the person",
142136
"externalDocs": {
@@ -162,7 +156,6 @@
162156
"required": [
163157
"@id",
164158
"@type",
165-
"sub",
166159
"firstName",
167160
"lastName",
168161
"name"

‎api/tests/Api/Admin/schemas/Review/item.json

-7
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,6 @@
3838
"type": "string",
3939
"pattern": "^https://schema.org/Person$"
4040
},
41-
"sub": {
42-
"externalDocs": {
43-
"url": "https:\/\/schema.org\/identifier"
44-
},
45-
"type": "string"
46-
},
4741
"firstName": {
4842
"description": "The givenName of the person",
4943
"externalDocs": {
@@ -69,7 +63,6 @@
6963
"required": [
7064
"@id",
7165
"@type",
72-
"sub",
7366
"firstName",
7467
"lastName",
7568
"name"

‎api/tests/Api/Admin/schemas/User/collection.json

-7
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,6 @@
1717
"type": "string",
1818
"pattern": "^https://schema.org/Person$"
1919
},
20-
"sub": {
21-
"externalDocs": {
22-
"url": "https:\/\/schema.org\/identifier"
23-
},
24-
"type": "string"
25-
},
2620
"firstName": {
2721
"description": "The givenName of the person",
2822
"externalDocs": {
@@ -48,7 +42,6 @@
4842
"required": [
4943
"@id",
5044
"@type",
51-
"sub",
5245
"firstName",
5346
"lastName",
5447
"name"

‎api/tests/Api/Admin/schemas/User/item.json

-7
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,6 @@
1818
"type": "string",
1919
"pattern": "^https://schema.org/Person$"
2020
},
21-
"sub": {
22-
"externalDocs": {
23-
"url": "https:\/\/schema.org\/identifier"
24-
},
25-
"type": "string"
26-
},
2721
"firstName": {
2822
"description": "The givenName of the person",
2923
"externalDocs": {
@@ -50,7 +44,6 @@
5044
"@context",
5145
"@id",
5246
"@type",
53-
"sub",
5447
"firstName",
5548
"lastName",
5649
"name"

‎api/tests/Api/schemas/Review/collection.json

-7
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,6 @@
8888
"type": "string",
8989
"pattern": "^https://schema.org/Person$"
9090
},
91-
"sub": {
92-
"externalDocs": {
93-
"url": "https:\/\/schema.org\/identifier"
94-
},
95-
"type": "string"
96-
},
9791
"firstName": {
9892
"description": "The givenName of the person",
9993
"externalDocs": {
@@ -119,7 +113,6 @@
119113
"required": [
120114
"@id",
121115
"@type",
122-
"sub",
123116
"firstName",
124117
"lastName",
125118
"name"

‎api/tests/Api/schemas/Review/item.json

-7
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,6 @@
3838
"type": "string",
3939
"pattern": "^https://schema.org/Person$"
4040
},
41-
"sub": {
42-
"externalDocs": {
43-
"url": "https:\/\/schema.org\/identifier"
44-
},
45-
"type": "string"
46-
},
4741
"firstName": {
4842
"description": "The givenName of the person",
4943
"externalDocs": {
@@ -69,7 +63,6 @@
6963
"required": [
7064
"@id",
7165
"@type",
72-
"sub",
7366
"firstName",
7467
"lastName",
7568
"name"

‎api/tests/Security/Core/UserProviderTest.php

+1-9
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use PHPUnit\Framework\TestCase;
1616
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
1717
use Symfony\Component\Security\Core\User\UserInterface;
18-
use Symfony\Component\Uid\Uuid;
1918

2019
final class UserProviderTest extends TestCase
2120
{
@@ -102,12 +101,8 @@ public function itCannotLoadUserIfAttributeIsMissing(array $attributes): void
102101

103102
public static function getInvalidAttributes(): iterable
104103
{
105-
yield 'missing sub' => [[]];
106-
yield 'missing given_name' => [[
107-
'sub' => 'ba86c94b-efeb-4452-a0b4-93ed3c889156',
108-
]];
104+
yield 'missing given_name' => [[]];
109105
yield 'missing family_name' => [[
110-
'sub' => 'ba86c94b-efeb-4452-a0b4-93ed3c889156',
111106
'given_name' => 'John',
112107
]];
113108
}
@@ -128,7 +123,6 @@ public function itLoadsUserFromAttributes(): void
128123
;
129124

130125
$this->assertSame($this->userMock, $this->provider->loadUserByIdentifier('[email protected]', [
131-
'sub' => 'ba86c94b-efeb-4452-a0b4-93ed3c889156',
132126
'given_name' => 'John',
133127
'family_name' => 'DOE',
134128
]));
@@ -140,7 +134,6 @@ public function itCreatesAUserFromAttributes(): void
140134
$expectedUser = new User();
141135
$expectedUser->firstName = 'John';
142136
$expectedUser->lastName = 'DOE';
143-
$expectedUser->sub = Uuid::fromString('ba86c94b-efeb-4452-a0b4-93ed3c889156');
144137
$expectedUser->email = '[email protected]';
145138

146139
$this->repositoryMock
@@ -156,7 +149,6 @@ public function itCreatesAUserFromAttributes(): void
156149
;
157150

158151
$this->assertEquals($expectedUser, $this->provider->loadUserByIdentifier('[email protected]', [
159-
'sub' => 'ba86c94b-efeb-4452-a0b4-93ed3c889156',
160152
'given_name' => 'John',
161153
'family_name' => 'DOE',
162154
]));

‎api/tests/State/Processor/ReviewPersistProcessorTest.php

+17-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use ApiPlatform\State\ProcessorInterface;
1010
use App\Entity\Review;
1111
use App\Entity\User;
12+
use App\Security\Http\Protection\ResourceHandlerInterface;
1213
use App\State\Processor\ReviewPersistProcessor;
1314
use PHPUnit\Framework\Attributes\Test;
1415
use PHPUnit\Framework\MockObject\MockObject;
@@ -25,6 +26,7 @@ final class ReviewPersistProcessorTest extends TestCase
2526
private MockObject|User $userMock;
2627
private MockObject|Review $objectMock;
2728
private ClockInterface|MockObject $clockMock;
29+
private ResourceHandlerInterface|MockObject $resourceHandlerMock;
2830
private ReviewPersistProcessor $processor;
2931

3032
protected function setUp(): void
@@ -35,12 +37,14 @@ protected function setUp(): void
3537
$this->userMock = $this->createMock(User::class);
3638
$this->objectMock = $this->createMock(Review::class);
3739
$this->clockMock = new MockClock();
40+
$this->resourceHandlerMock = $this->createMock(ResourceHandlerInterface::class);
3841

3942
$this->processor = new ReviewPersistProcessor(
4043
$this->persistProcessorMock,
4144
$this->mercureProcessorMock,
4245
$this->securityMock,
43-
$this->clockMock
46+
$this->clockMock,
47+
$this->resourceHandlerMock
4448
);
4549
}
4650

@@ -53,6 +57,7 @@ public function itUpdatesReviewDataFromOperationBeforeSaveAndSendMercureUpdates(
5357
$expectedData->user = $this->userMock;
5458
$expectedData->publishedAt = $this->clockMock->now();
5559

60+
$this->userMock->email = '[email protected]';
5661
$this->securityMock
5762
->expects($this->once())
5863
->method('getUser')
@@ -64,6 +69,13 @@ public function itUpdatesReviewDataFromOperationBeforeSaveAndSendMercureUpdates(
6469
->with($expectedData, $operation, [], [])
6570
->willReturn($expectedData)
6671
;
72+
$this->resourceHandlerMock
73+
->expects($this->once())
74+
->method('create')
75+
->with($expectedData, $this->userMock, [
76+
'operation_name' => '/books/{bookId}/reviews/{id}{._format}',
77+
])
78+
;
6779
$this->mercureProcessorMock
6880
->expects($this->exactly(2))
6981
->method('process')
@@ -105,6 +117,10 @@ public function itUpdatesReviewDataFromContextBeforeSaveAndSendMercureUpdates():
105117
->with($expectedData, $operation, [], $context)
106118
->willReturn($expectedData)
107119
;
120+
$this->resourceHandlerMock
121+
->expects($this->never())
122+
->method('create')
123+
;
108124
$this->mercureProcessorMock
109125
->expects($this->exactly(2))
110126
->method('process')

0 commit comments

Comments
 (0)