Skip to content

Commit

Permalink
fix: remove useless User.sub
Browse files Browse the repository at this point in the history
  • Loading branch information
vincentchalamon committed Mar 26, 2024
1 parent e4fa432 commit ef87db6
Show file tree
Hide file tree
Showing 16 changed files with 41 additions and 84 deletions.
4 changes: 1 addition & 3 deletions api/migrations/Version20230906094949.php
Expand Up @@ -41,11 +41,9 @@ public function up(Schema $schema): void
$this->addSql('COMMENT ON COLUMN review.user_id IS \'(DC2Type:uuid)\'');
$this->addSql('COMMENT ON COLUMN review.book_id IS \'(DC2Type:uuid)\'');
$this->addSql('COMMENT ON COLUMN review.published_at IS \'(DC2Type:datetime_immutable)\'');
$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))');
$this->addSql('CREATE UNIQUE INDEX UNIQ_8D93D649580282DC ON "user" (sub)');
$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))');
$this->addSql('CREATE UNIQUE INDEX UNIQ_8D93D649E7927C74 ON "user" (email)');
$this->addSql('COMMENT ON COLUMN "user".id IS \'(DC2Type:uuid)\'');
$this->addSql('COMMENT ON COLUMN "user".sub IS \'(DC2Type:uuid)\'');
$this->addSql('ALTER TABLE bookmark ADD CONSTRAINT FK_DA62921DA76ED395 FOREIGN KEY (user_id) REFERENCES "user" (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
$this->addSql('ALTER TABLE bookmark ADD CONSTRAINT FK_DA62921D16A2B381 FOREIGN KEY (book_id) REFERENCES book (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
$this->addSql('ALTER TABLE review ADD CONSTRAINT FK_794381C6A76ED395 FOREIGN KEY (user_id) REFERENCES "user" (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
Expand Down
6 changes: 5 additions & 1 deletion api/src/DataFixtures/Factory/ReviewFactory.php
Expand Up @@ -54,7 +54,7 @@ final class ReviewFactory extends ModelFactory
* @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services
*/
public function __construct(
private readonly ResourceHandlerInterface $resourceHandler,
private readonly ?ResourceHandlerInterface $resourceHandler = null,
) {
parent::__construct();
}
Expand All @@ -81,6 +81,10 @@ protected function initialize(): self
return $this
// create the resource on the OIDC server
->afterPersist(function (Review $object): void {
if (!$this->resourceHandler) {
return;
}

// project specification: only create resource on OIDC server for known users (john.doe and chuck.norris)
if (\in_array($object->user?->email, ['john.doe@example.com', 'chuck.norris@example.com'], true)) {
$this->resourceHandler->create($object, $object->user, [
Expand Down
2 changes: 0 additions & 2 deletions api/src/DataFixtures/Factory/UserFactory.php
Expand Up @@ -6,7 +6,6 @@

use App\Entity\User;
use Doctrine\ORM\EntityRepository;
use Symfony\Component\Uid\Uuid;
use Zenstruck\Foundry\ModelFactory;
use Zenstruck\Foundry\Proxy;
use Zenstruck\Foundry\RepositoryProxy;
Expand Down Expand Up @@ -76,7 +75,6 @@ public function withAdmin(): self
protected function getDefaults(): array
{
return [
'sub' => Uuid::v7(),
'email' => self::faker()->unique()->email(),
'firstName' => self::faker()->firstName(),
'lastName' => self::faker()->lastName(),
Expand Down
8 changes: 0 additions & 8 deletions api/src/Entity/User.php
Expand Up @@ -63,14 +63,6 @@ class User implements UserInterface
#[ORM\Id]
private ?Uuid $id = null;

/**
* @see https://schema.org/identifier
*/
#[ApiProperty(types: ['https://schema.org/identifier'])]
#[Groups(groups: ['User:read'])]
#[ORM\Column(type: UuidType::NAME, unique: true)]
public ?Uuid $sub = null;

/**
* @see https://schema.org/email
*/
Expand Down
10 changes: 0 additions & 10 deletions api/src/Security/Core/UserProvider.php
Expand Up @@ -10,7 +10,6 @@
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Symfony\Component\Security\Core\User\AttributesBasedUserProviderInterface;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Uid\Uuid;

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

if (!isset($attributes['sub'])) {
throw new UnsupportedUserException('Property "sub" is missing in token attributes.');
}
try {
$user->sub = Uuid::fromString($attributes['sub']);
} catch (\Throwable $e) {
throw new UnsupportedUserException($e->getMessage(), $e->getCode(), $e);
}

if (!isset($attributes['given_name'])) {
throw new UnsupportedUserException('Property "given_name" is missing in token attributes.');
}
Expand Down
2 changes: 1 addition & 1 deletion api/tests/Api/Admin/BookTest.php
Expand Up @@ -33,7 +33,7 @@ final class BookTest extends ApiTestCase

protected function setup(): void
{
$this->client = self::createClient(['debug' => true]);
$this->client = self::createClient();
}

#[Test]
Expand Down
7 changes: 1 addition & 6 deletions api/tests/Api/Admin/UserTest.php
Expand Up @@ -12,7 +12,6 @@
use App\Tests\Api\Security\TokenGenerator;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Test;
use Symfony\Component\Uid\Uuid;
use Zenstruck\Foundry\FactoryCollection;
use Zenstruck\Foundry\Test\Factories;
use Zenstruck\Foundry\Test\ResetDatabase;
Expand All @@ -27,7 +26,7 @@ final class UserTest extends ApiTestCase

protected function setup(): void
{
$this->client = self::createClient();
$this->client = self::createClient(['debug' => true]);
}

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

$sub = Uuid::v7()->__toString();
$token = self::getContainer()->get(TokenGenerator::class)->generateToken([
'sub' => $sub,
'email' => $user->email,
'given_name' => 'Chuck',
'family_name' => 'NORRIS',
Expand All @@ -176,6 +172,5 @@ public function asAUserIAmUpdatedOnLogin(): void
self::assertEquals('Chuck', $user->firstName);
self::assertEquals('NORRIS', $user->lastName);
self::assertEquals('Chuck NORRIS', $user->getName());
self::assertEquals($sub, $user->sub);
}
}
7 changes: 0 additions & 7 deletions api/tests/Api/Admin/schemas/Review/collection.json
Expand Up @@ -131,12 +131,6 @@
"type": "string",
"pattern": "^https://schema.org/Person$"
},
"sub": {
"externalDocs": {
"url": "https:\/\/schema.org\/identifier"
},
"type": "string"
},
"firstName": {
"description": "The givenName of the person",
"externalDocs": {
Expand All @@ -162,7 +156,6 @@
"required": [
"@id",
"@type",
"sub",
"firstName",
"lastName",
"name"
Expand Down
7 changes: 0 additions & 7 deletions api/tests/Api/Admin/schemas/Review/item.json
Expand Up @@ -38,12 +38,6 @@
"type": "string",
"pattern": "^https://schema.org/Person$"
},
"sub": {
"externalDocs": {
"url": "https:\/\/schema.org\/identifier"
},
"type": "string"
},
"firstName": {
"description": "The givenName of the person",
"externalDocs": {
Expand All @@ -69,7 +63,6 @@
"required": [
"@id",
"@type",
"sub",
"firstName",
"lastName",
"name"
Expand Down
7 changes: 0 additions & 7 deletions api/tests/Api/Admin/schemas/User/collection.json
Expand Up @@ -17,12 +17,6 @@
"type": "string",
"pattern": "^https://schema.org/Person$"
},
"sub": {
"externalDocs": {
"url": "https:\/\/schema.org\/identifier"
},
"type": "string"
},
"firstName": {
"description": "The givenName of the person",
"externalDocs": {
Expand All @@ -48,7 +42,6 @@
"required": [
"@id",
"@type",
"sub",
"firstName",
"lastName",
"name"
Expand Down
7 changes: 0 additions & 7 deletions api/tests/Api/Admin/schemas/User/item.json
Expand Up @@ -18,12 +18,6 @@
"type": "string",
"pattern": "^https://schema.org/Person$"
},
"sub": {
"externalDocs": {
"url": "https:\/\/schema.org\/identifier"
},
"type": "string"
},
"firstName": {
"description": "The givenName of the person",
"externalDocs": {
Expand All @@ -50,7 +44,6 @@
"@context",
"@id",
"@type",
"sub",
"firstName",
"lastName",
"name"
Expand Down
7 changes: 0 additions & 7 deletions api/tests/Api/schemas/Review/collection.json
Expand Up @@ -88,12 +88,6 @@
"type": "string",
"pattern": "^https://schema.org/Person$"
},
"sub": {
"externalDocs": {
"url": "https:\/\/schema.org\/identifier"
},
"type": "string"
},
"firstName": {
"description": "The givenName of the person",
"externalDocs": {
Expand All @@ -119,7 +113,6 @@
"required": [
"@id",
"@type",
"sub",
"firstName",
"lastName",
"name"
Expand Down
7 changes: 0 additions & 7 deletions api/tests/Api/schemas/Review/item.json
Expand Up @@ -38,12 +38,6 @@
"type": "string",
"pattern": "^https://schema.org/Person$"
},
"sub": {
"externalDocs": {
"url": "https:\/\/schema.org\/identifier"
},
"type": "string"
},
"firstName": {
"description": "The givenName of the person",
"externalDocs": {
Expand All @@ -69,7 +63,6 @@
"required": [
"@id",
"@type",
"sub",
"firstName",
"lastName",
"name"
Expand Down
10 changes: 1 addition & 9 deletions api/tests/Security/Core/UserProviderTest.php
Expand Up @@ -15,7 +15,6 @@
use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Uid\Uuid;

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

public static function getInvalidAttributes(): iterable
{
yield 'missing sub' => [[]];
yield 'missing given_name' => [[
'sub' => 'ba86c94b-efeb-4452-a0b4-93ed3c889156',
]];
yield 'missing given_name' => [[]];
yield 'missing family_name' => [[
'sub' => 'ba86c94b-efeb-4452-a0b4-93ed3c889156',
'given_name' => 'John',
]];
}
Expand All @@ -128,7 +123,6 @@ public function itLoadsUserFromAttributes(): void
;

$this->assertSame($this->userMock, $this->provider->loadUserByIdentifier('john.doe@example.com', [
'sub' => 'ba86c94b-efeb-4452-a0b4-93ed3c889156',
'given_name' => 'John',
'family_name' => 'DOE',
]));
Expand All @@ -140,7 +134,6 @@ public function itCreatesAUserFromAttributes(): void
$expectedUser = new User();
$expectedUser->firstName = 'John';
$expectedUser->lastName = 'DOE';
$expectedUser->sub = Uuid::fromString('ba86c94b-efeb-4452-a0b4-93ed3c889156');
$expectedUser->email = 'john.doe@example.com';

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

$this->assertEquals($expectedUser, $this->provider->loadUserByIdentifier('john.doe@example.com', [
'sub' => 'ba86c94b-efeb-4452-a0b4-93ed3c889156',
'given_name' => 'John',
'family_name' => 'DOE',
]));
Expand Down
18 changes: 17 additions & 1 deletion api/tests/State/Processor/ReviewPersistProcessorTest.php
Expand Up @@ -9,6 +9,7 @@
use ApiPlatform\State\ProcessorInterface;
use App\Entity\Review;
use App\Entity\User;
use App\Security\Http\Protection\ResourceHandlerInterface;
use App\State\Processor\ReviewPersistProcessor;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\MockObject\MockObject;
Expand All @@ -25,6 +26,7 @@ final class ReviewPersistProcessorTest extends TestCase
private MockObject|User $userMock;
private MockObject|Review $objectMock;
private ClockInterface|MockObject $clockMock;
private ResourceHandlerInterface|MockObject $resourceHandlerMock;
private ReviewPersistProcessor $processor;

protected function setUp(): void
Expand All @@ -35,12 +37,14 @@ protected function setUp(): void
$this->userMock = $this->createMock(User::class);
$this->objectMock = $this->createMock(Review::class);
$this->clockMock = new MockClock();
$this->resourceHandlerMock = $this->createMock(ResourceHandlerInterface::class);

$this->processor = new ReviewPersistProcessor(
$this->persistProcessorMock,
$this->mercureProcessorMock,
$this->securityMock,
$this->clockMock
$this->clockMock,
$this->resourceHandlerMock
);
}

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

$this->userMock->email = 'john.doe@example.com';
$this->securityMock
->expects($this->once())
->method('getUser')
Expand All @@ -64,6 +69,13 @@ public function itUpdatesReviewDataFromOperationBeforeSaveAndSendMercureUpdates(
->with($expectedData, $operation, [], [])
->willReturn($expectedData)
;
$this->resourceHandlerMock
->expects($this->once())
->method('create')
->with($expectedData, $this->userMock, [
'operation_name' => '/books/{bookId}/reviews/{id}{._format}',
])
;
$this->mercureProcessorMock
->expects($this->exactly(2))
->method('process')
Expand Down Expand Up @@ -105,6 +117,10 @@ public function itUpdatesReviewDataFromContextBeforeSaveAndSendMercureUpdates():
->with($expectedData, $operation, [], $context)
->willReturn($expectedData)
;
$this->resourceHandlerMock
->expects($this->never())
->method('create')
;
$this->mercureProcessorMock
->expects($this->exactly(2))
->method('process')
Expand Down

0 comments on commit ef87db6

Please sign in to comment.