diff --git a/src/Entities/UserEntity.php b/src/Entities/UserEntity.php index 74233aa0..e62a73c3 100644 --- a/src/Entities/UserEntity.php +++ b/src/Entities/UserEntity.php @@ -16,85 +16,22 @@ namespace SimpleSAML\Module\oidc\Entities; -use DateTime; +use DateTimeImmutable; use League\OAuth2\Server\Entities\UserEntityInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\ClaimSetInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\MementoInterface; -use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; -use SimpleSAML\Module\oidc\Utils\TimestampGenerator; /** * @psalm-suppress PropertyNotSetInConstructor */ class UserEntity implements UserEntityInterface, MementoInterface, ClaimSetInterface { - /** - * @var string - */ - private string $identifier; - - /** - * @var array - */ - private array $claims; - - /** - * @var DateTime - */ - private DateTime $createdAt; - - /** - * @var DateTime - */ - private DateTime $updatedAt; - - private function __construct() - { - } - - /** - * @throws \Exception - */ - public static function fromData(string $identifier, array $claims = []): self - { - $user = new self(); - - $user->identifier = $identifier; - $user->createdAt = TimestampGenerator::utc(); - $user->updatedAt = $user->createdAt; - $user->claims = $claims; - - return $user; - } - - /** - * @throws \Exception - * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException - */ - public static function fromState(array $state): self - { - $user = new self(); - - if ( - !is_string($state['id']) || - !is_string($state['claims']) || - !is_string($state['updated_at']) || - !is_string($state['created_at']) - ) { - throw OidcServerException::serverError('Invalid user entity data'); - } - - $user->identifier = $state['id']; - $claims = json_decode($state['claims'], true, 512, JSON_INVALID_UTF8_SUBSTITUTE); - - if (!is_array($claims)) { - throw OidcServerException::serverError('Invalid user entity data'); - } - $user->claims = $claims; - $user->updatedAt = TimestampGenerator::utc($state['updated_at']); - $user->createdAt = TimestampGenerator::utc($state['created_at']); - - return $user; + public function __construct( + private readonly string $identifier, + private readonly DateTimeImmutable $createdAt, + private DateTimeImmutable $updatedAt, + private array $claims = [], + ) { } /** @@ -120,23 +57,24 @@ public function getClaims(): array return $this->claims; } - /** - * @throws \Exception - */ public function setClaims(array $claims): self { $this->claims = $claims; - $this->updatedAt = TimestampGenerator::utc(); - return $this; } - public function getUpdatedAt(): DateTime + public function getUpdatedAt(): DateTimeImmutable { return $this->updatedAt; } - public function getCreatedAt(): DateTime + public function setUpdatedAt(DateTimeImmutable $updatedAt): self + { + $this->updatedAt = $updatedAt; + return $this; + } + + public function getCreatedAt(): DateTimeImmutable { return $this->createdAt; } diff --git a/src/Factories/Entities/UserEntityFactory.php b/src/Factories/Entities/UserEntityFactory.php new file mode 100644 index 00000000..9b2671f5 --- /dev/null +++ b/src/Factories/Entities/UserEntityFactory.php @@ -0,0 +1,60 @@ +helpers->dateTime()->getUtc(); + + return new UserEntity( + $identifier, + $createdAt, + $updatedAt, + $claims, + ); + } + + /** + * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException + */ + public function fromState(array $state): UserEntity + { + if ( + !is_string($state['id']) || + !is_string($state['claims']) || + !is_string($state['updated_at']) || + !is_string($state['created_at']) + ) { + throw OidcServerException::serverError('Invalid user entity data'); + } + + $identifier = $state['id']; + $claims = json_decode($state['claims'], true, 512, JSON_INVALID_UTF8_SUBSTITUTE); + + if (!is_array($claims)) { + throw OidcServerException::serverError('Invalid user entity data'); + } + $updatedAt = $this->helpers->dateTime()->getUtc($state['updated_at']); + $createdAt = $this->helpers->dateTime()->getUtc($state['created_at']); + + return new UserEntity( + $identifier, + $createdAt, + $updatedAt, + $claims, + ); + } +} diff --git a/src/Repositories/UserRepository.php b/src/Repositories/UserRepository.php index 3bd27077..4c2772a7 100644 --- a/src/Repositories/UserRepository.php +++ b/src/Repositories/UserRepository.php @@ -16,17 +16,29 @@ namespace SimpleSAML\Module\oidc\Repositories; +use DateTimeImmutable; use Exception; use League\OAuth2\Server\Entities\ClientEntityInterface as OAuth2ClientEntityInterface; use League\OAuth2\Server\Entities\UserEntityInterface; use League\OAuth2\Server\Repositories\UserRepositoryInterface; use SimpleSAML\Module\oidc\Entities\UserEntity; +use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory; +use SimpleSAML\Module\oidc\Helpers; +use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\Interfaces\IdentityProviderInterface; class UserRepository extends AbstractDatabaseRepository implements UserRepositoryInterface, IdentityProviderInterface { final public const TABLE_NAME = 'oidc_user'; + public function __construct( + ModuleConfig $moduleConfig, + protected readonly Helpers $helpers, + protected readonly UserEntityFactory $userEntityFactory, + ) { + parent::__construct($moduleConfig); + } + public function getTableName(): string { return $this->database->applyPrefix(self::TABLE_NAME); @@ -57,7 +69,7 @@ public function getUserEntityByIdentifier(string $identifier): ?UserEntity return null; } - return UserEntity::fromState($row); + return $this->userEntityFactory->fromState($row); } /** @@ -95,8 +107,10 @@ public function delete(UserEntity $user): void ); } - public function update(UserEntity $user): void + public function update(UserEntity $user, ?DateTimeImmutable $updatedAt = null): void { + $user->setUpdatedAt($updatedAt ?? $this->helpers->dateTime()->getUtc()); + $stmt = sprintf( "UPDATE %s SET claims = :claims, updated_at = :updated_at, created_at = :created_at WHERE id = :id", $this->getTableName(), diff --git a/src/Services/AuthenticationService.php b/src/Services/AuthenticationService.php index 41ba00fe..18744788 100644 --- a/src/Services/AuthenticationService.php +++ b/src/Services/AuthenticationService.php @@ -29,6 +29,7 @@ use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; use SimpleSAML\Module\oidc\Entities\UserEntity; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; +use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory; use SimpleSAML\Module\oidc\Factories\ProcessingChainFactory; use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; @@ -66,6 +67,7 @@ public function __construct( private readonly StateService $stateService, private readonly Helpers $helpers, private readonly RequestParamsResolver $requestParamsResolver, + private readonly UserEntityFactory $userEntityFactory, ) { $this->userIdAttr = $this->moduleConfig->getUserIdentifierAttribute(); } @@ -140,13 +142,14 @@ public function getAuthenticateUser( } $userId = (string)$claims[$this->userIdAttr][0]; + $user = $this->userRepository->getUserEntityByIdentifier($userId); if ($user) { $user->setClaims($claims); $this->userRepository->update($user); } else { - $user = UserEntity::fromData($userId, $claims); + $user = $this->userEntityFactory->fromData($userId, $claims); $this->userRepository->add($user); } diff --git a/src/Services/Container.php b/src/Services/Container.php index b4400a87..2b2d3ba8 100644 --- a/src/Services/Container.php +++ b/src/Services/Container.php @@ -43,6 +43,7 @@ use SimpleSAML\Module\oidc\Factories\Entities\ClaimSetEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\ClientEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; +use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory; use SimpleSAML\Module\oidc\Factories\FederationFactory; use SimpleSAML\Module\oidc\Factories\FormFactory; use SimpleSAML\Module\oidc\Factories\Grant\AuthCodeGrantFactory; @@ -205,7 +206,14 @@ public function __construct() $clientRepository = new ClientRepository($moduleConfig, $clientEntityFactory); $this->services[ClientRepository::class] = $clientRepository; - $userRepository = new UserRepository($moduleConfig); + $userEntityFactory = new UserEntityFactory($helpers); + $this->services[UserEntityFactory::class] = $userEntityFactory; + + $userRepository = new UserRepository( + $moduleConfig, + $helpers, + $userEntityFactory, + ); $this->services[UserRepository::class] = $userRepository; $authCodeEntityFactory = new AuthCodeEntityFactory($helpers); @@ -277,6 +285,7 @@ public function __construct() $stateService, $helpers, $requestParamsResolver, + $userEntityFactory, ); $this->services[AuthenticationService::class] = $authenticationService; diff --git a/tests/integration/src/Repositories/Traits/RevokeTokenByAuthCodeIdTraitTest.php b/tests/integration/src/Repositories/Traits/RevokeTokenByAuthCodeIdTraitTest.php index bdf69e72..bddee782 100644 --- a/tests/integration/src/Repositories/Traits/RevokeTokenByAuthCodeIdTraitTest.php +++ b/tests/integration/src/Repositories/Traits/RevokeTokenByAuthCodeIdTraitTest.php @@ -18,6 +18,7 @@ use SimpleSAML\Module\oidc\Entities\UserEntity; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\ClientEntityFactory; +use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory; use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\AbstractDatabaseRepository; @@ -171,9 +172,14 @@ public function getDatabase(): Database $this->mock->getDatabase()->write('DELETE from ' . $clientRepositoryMock->getTableName()); $clientRepositoryMock->add($client); - - $user = UserEntity::fromData(self::USER_ID); - $userRepositoryMock = new UserRepository($moduleConfig); + $createUpdatedAt = new \DateTimeImmutable(); + $helpers = new Helpers(); + $user = new UserEntity(self::USER_ID, $createUpdatedAt, $createUpdatedAt, []); + $userRepositoryMock = new UserRepository( + $moduleConfig, + $helpers, + new UserEntityFactory($helpers), + ); $this->mock->getDatabase()->write('DELETE from ' . $userRepositoryMock->getTableName()); $userRepositoryMock->add($user); } diff --git a/tests/unit/src/Entities/UserEntityTest.php b/tests/unit/src/Entities/UserEntityTest.php index 68340bd5..1d6e5e79 100644 --- a/tests/unit/src/Entities/UserEntityTest.php +++ b/tests/unit/src/Entities/UserEntityTest.php @@ -4,6 +4,8 @@ namespace SimpleSAML\Test\Module\oidc\unit\Entities; +use DateTimeImmutable; +use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use SimpleSAML\Module\oidc\Entities\UserEntity; @@ -14,6 +16,13 @@ class UserEntityTest extends TestCase { protected array $state; + protected string $identifier = 'id'; + + protected array $claims = []; + + protected Stub $createdAt; + protected Stub $updatedAt; + protected function setUp(): void { $this->state = [ @@ -22,15 +31,28 @@ protected function setUp(): void 'updated_at' => '1970-01-01 00:00:00', 'created_at' => '1970-01-01 00:00:00', ]; + + $this->createdAt = $this->createStub(DateTimeImmutable::class); + $this->updatedAt = $this->createStub(DateTimeImmutable::class); } - /** - * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException - */ - protected function prepareMockedInstance(array $state = null): UserEntity - { - $state ??= $this->state; - return UserEntity::fromState($state); + protected function mock( + ?string $identifier = null, + ?array $claims = null, + ?Stub $createdAt = null, + ?Stub $updatedAt = null, + ): UserEntity { + $identifier ??= $this->identifier; + $claims ??= $this->claims; + $createdAt ??= $this->createdAt; + $updatedAt ??= $this->updatedAt; + + return new UserEntity( + $identifier, + $createdAt, + $updatedAt, + $claims, + ); } /** @@ -41,12 +63,7 @@ public function testItIsInitializable(): void { $this->assertInstanceOf( UserEntity::class, - $this->prepareMockedInstance(), - ); - - $this->assertInstanceOf( - UserEntity::class, - UserEntity::fromData('id'), + $this->mock(), ); } @@ -56,14 +73,9 @@ public function testItIsInitializable(): void */ public function testCanGetProperties(): void { - $userEntity = $this->prepareMockedInstance(); + $userEntity = $this->mock(); $this->assertSame($userEntity->getIdentifier(), 'id'); $this->assertSame($userEntity->getClaims(), []); - $this->assertSame($userEntity->getCreatedAt()->format('Y-m-d H:i:s'), '1970-01-01 00:00:00'); - $this->assertSame($userEntity->getUpdatedAt()->format('Y-m-d H:i:s'), '1970-01-01 00:00:00'); - - $userEntity->setClaims(['claim']); - $this->assertSame($userEntity->getClaims(), ['claim']); } /** @@ -72,12 +84,12 @@ public function testCanGetProperties(): void public function testCanGetState(): void { $this->assertSame( - $this->prepareMockedInstance()->getState(), + $this->mock()->getState(), [ 'id' => 'id', 'claims' => json_encode([]), - 'updated_at' => '1970-01-01 00:00:00', - 'created_at' => '1970-01-01 00:00:00', + 'updated_at' => '', + 'created_at' => '', ], ); } diff --git a/tests/unit/src/Repositories/UserRepositoryTest.php b/tests/unit/src/Repositories/UserRepositoryTest.php index 4cf35d99..c1016ab1 100644 --- a/tests/unit/src/Repositories/UserRepositoryTest.php +++ b/tests/unit/src/Repositories/UserRepositoryTest.php @@ -15,9 +15,14 @@ */ namespace SimpleSAML\Test\Module\oidc\unit\Repositories; +use DateTimeImmutable; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use SimpleSAML\Configuration; use SimpleSAML\Module\oidc\Entities\UserEntity; +use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\UserRepository; use SimpleSAML\Module\oidc\Services\DatabaseMigration; @@ -28,6 +33,9 @@ class UserRepositoryTest extends TestCase { protected static UserRepository $repository; + protected Stub $helpersStub; + protected MockObject $userEntityFactoryMock; + protected MockObject $userEntityMock; /** * @throws \Exception @@ -47,8 +55,15 @@ protected function setUp(): void (new DatabaseMigration())->migrate(); $moduleConfig = new ModuleConfig(); - - self::$repository = new UserRepository($moduleConfig); + $this->helpersStub = $this->createStub(Helpers::class); + $this->userEntityFactoryMock = $this->createMock(UserEntityFactory::class); + $this->userEntityMock = $this->createMock(UserEntity::class); + + self::$repository = new UserRepository( + $moduleConfig, + $this->helpersStub, + $this->userEntityFactoryMock, + ); } public function testGetTableName(): void @@ -60,9 +75,18 @@ public function testGetTableName(): void * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException * @throws \Exception */ - public function testAddAndFound(): void + public function testCanAddFindDelete(): void { - self::$repository->add(UserEntity::fromData('uniqueid')); + $createdUpdatedAt = new DateTimeImmutable(); + self::$repository->add(new UserEntity('uniqueid', $createdUpdatedAt, $createdUpdatedAt)); + + $this->userEntityMock->method('getIdentifier')->willReturn('uniqueid'); + $this->userEntityFactoryMock->expects($this->once()) + ->method('fromState') + ->with($this->callback(function (array $state) { + return $state['id'] === 'uniqueid'; + })) + ->willReturn($this->userEntityMock); $user = self::$repository->getUserEntityByIdentifier('uniqueid'); $this->assertNotNull($user); @@ -93,15 +117,11 @@ public function testUpdate(): void $this->assertNotSame($user, $user2); } - /** - * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException - */ - public function testDelete(): void + public function testCanDelete(): void { - $user = self::$repository->getUserEntityByIdentifier('uniqueid'); - self::$repository->delete($user); - $user = self::$repository->getUserEntityByIdentifier('uniqueid'); - - $this->assertNull($user); + $this->userEntityMock->method('getIdentifier')->willReturn('uniqueid'); + $this->assertNotNull(self::$repository->getUserEntityByIdentifier('uniqueid')); + self::$repository->delete($this->userEntityMock); + $this->assertNull(self::$repository->getUserEntityByIdentifier('uniqueid')); } } diff --git a/tests/unit/src/Server/ResponseTypes/IdTokenResponseTest.php b/tests/unit/src/Server/ResponseTypes/IdTokenResponseTest.php index 74e101d3..8fdeb83c 100644 --- a/tests/unit/src/Server/ResponseTypes/IdTokenResponseTest.php +++ b/tests/unit/src/Server/ResponseTypes/IdTokenResponseTest.php @@ -68,10 +68,13 @@ class IdTokenResponseTest extends TestCase protected function setUp(): void { $this->certFolder = dirname(__DIR__, 5) . '/docker/ssp/'; - $this->userEntity = UserEntity::fromData(self::SUBJECT, [ - 'cn' => ['Homer Simpson'], - 'mail' => ['myEmail@example.com'], - ]); + $createdUpdatedAt = new DateTimeImmutable(); + $this->userEntity = new UserEntity( + self::SUBJECT, + $createdUpdatedAt, + $createdUpdatedAt, + ['cn' => ['Homer Simpson'], 'mail' => ['myEmail@example.com'],], + ); $this->scopes = [ ScopeEntity::fromData('openid'), ScopeEntity::fromData('email'), diff --git a/tests/unit/src/Services/AuthenticationServiceTest.php b/tests/unit/src/Services/AuthenticationServiceTest.php index 871bb675..29623db4 100644 --- a/tests/unit/src/Services/AuthenticationServiceTest.php +++ b/tests/unit/src/Services/AuthenticationServiceTest.php @@ -21,6 +21,7 @@ use SimpleSAML\Module\oidc\Entities\ClientEntity; use SimpleSAML\Module\oidc\Entities\UserEntity; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; +use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory; use SimpleSAML\Module\oidc\Factories\ProcessingChainFactory; use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; @@ -81,6 +82,7 @@ class AuthenticationServiceTest extends TestCase protected MockObject $helpersMock; protected MockObject $clientHelperMock; protected MockObject $requestParamsResolverMock; + protected MockObject $userEntityFactoryMock; /** * @return void @@ -134,6 +136,8 @@ protected function setUp(): void $this->requestParamsResolverMock = $this->createMock(RequestParamsResolver::class); $this->requestParamsResolverMock->method('getAll')->with($this->serverRequestMock) ->willReturn(self::AUTHZ_REQUEST_PARAMS); + + $this->userEntityFactoryMock = $this->createMock(UserEntityFactory::class); } /** @@ -156,6 +160,7 @@ public function mock(): AuthenticationService $this->stateServiceMock, $this->helpersMock, $this->requestParamsResolverMock, + $this->userEntityFactoryMock, ], )->onlyMethods([]) ->getMock(); @@ -185,6 +190,14 @@ public function testItCreatesNewUser(): void $clientId = 'client123'; $this->clientRepositoryMock->method('findById')->willReturn($this->clientEntityMock); $this->clientEntityMock->expects($this->once())->method('getIdentifier')->willReturn($clientId); + + $this->userEntityMock->method('getIdentifier')->willReturn(self::USERNAME); + $this->userEntityMock->method('getClaims')->willReturn(self::USER_ENTITY_ATTRIBUTES); + + $this->userEntityFactoryMock->expects($this->once())->method('fromData') + ->with(self::USERNAME, self::USER_ENTITY_ATTRIBUTES) + ->willReturn($this->userEntityMock); + $userEntity = $this->mock()->getAuthenticateUser(self::STATE); $this->assertSame( @@ -403,6 +416,7 @@ public function testItProcessesRequest(bool $isAuthnPer): void $this->stateServiceMock, $this->helpersMock, $this->requestParamsResolverMock, + $this->userEntityFactoryMock, ]) ->onlyMethods(['runAuthProcs', 'prepareStateArray']) ->getMock(); @@ -493,6 +507,7 @@ public function testItRunAuthProcs(): void $this->stateServiceMock, $this->helpersMock, $this->requestParamsResolverMock, + $this->userEntityFactoryMock, ) extends AuthenticationService { public function runAuthProcsPublic(array &$state): void {