Move to dedicated ScopeEntity Factory (#261)

cicnavi · web-flow · commit 897b9a52387f · 2024-10-21T15:31:32.000+02:00
* Move to dedicated ScopeEntity Factory

---------

Co-authored-by: Marko Ivančić &lt;marko.ivancic@srce.hr&gt;
diff --git a/src/Entities/ScopeEntity.php b/src/Entities/ScopeEntity.php
@@ -27,41 +27,15 @@ class ScopeEntity implements ScopeEntityInterface
     use EntityTrait;
 
     /**
-     * @var string|null
+     * @param string[] $claims
      */
-    private ?string $icon = null;
-
-    /**
-     * @var string|null
-     */
-    private ?string $description = null;
-
-    /**
-     * @var array<string>
-     */
-    private array $claims;
-
-    private function __construct()
-    {
-    }
-
-    /**
-     * @param array<string> $claims
-     */
-    public static function fromData(
+    public function __construct(
         string $identifier,
-        string $description = null,
-        string $icon = null,
-        array $claims = [],
-    ): self {
-        $scope = new self();
-
-        $scope->identifier = $identifier;
-        $scope->description = $description;
-        $scope->icon = $icon;
-        $scope->claims = $claims;
-
-        return $scope;
+        protected ?string $description = null,
+        protected ?string $icon = null,
+        protected array $claims = [],
+    ) {
+        $this->identifier = $identifier;
     }
 
     public function getIcon(): ?string
diff --git a/src/Factories/Entities/AccessTokenEntityFactory.php b/src/Factories/Entities/AccessTokenEntityFactory.php
@@ -9,7 +9,6 @@
 use League\OAuth2\Server\Entities\ClientEntityInterface as OAuth2ClientEntityInterface;
 use SimpleSAML\Module\oidc\Entities\AccessTokenEntity;
 use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface;
-use SimpleSAML\Module\oidc\Entities\ScopeEntity;
 use SimpleSAML\Module\oidc\Helpers;
 use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
 use SimpleSAML\Module\oidc\Services\JsonWebTokenBuilderService;
@@ -20,6 +19,7 @@ public function __construct(
         protected readonly Helpers $helpers,
         protected readonly CryptKey $privateKey,
         protected readonly JsonWebTokenBuilderService $jsonWebTokenBuilderService,
+        protected readonly ScopeEntityFactory $scopeEntityFactory,
     ) {
     }
 
@@ -71,7 +71,7 @@ public function fromState(array $state): AccessTokenEntity
         }
 
         /** @psalm-var string $scope */
-        $scopes = array_map(fn(string $scope) => ScopeEntity::fromData($scope), $stateScopes);
+        $scopes = array_map(fn(string $scope) => $this->scopeEntityFactory->fromData($scope), $stateScopes);
 
         $id = $state['id'];
         $expiryDateTime = $this->helpers->dateTime()->getUtc($state['expires_at']);
diff --git a/src/Factories/Entities/AuthCodeEntityFactory.php b/src/Factories/Entities/AuthCodeEntityFactory.php
@@ -8,14 +8,14 @@
 use League\OAuth2\Server\Entities\ClientEntityInterface as OAuth2ClientEntityInterface;
 use SimpleSAML\Module\oidc\Entities\AuthCodeEntity;
 use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface;
-use SimpleSAML\Module\oidc\Entities\ScopeEntity;
 use SimpleSAML\Module\oidc\Helpers;
 use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
 
 class AuthCodeEntityFactory
 {
     public function __construct(
         protected readonly Helpers $helpers,
+        protected readonly ScopeEntityFactory $scopeEntityFactory,
     ) {
     }
 
@@ -68,9 +68,9 @@ public function fromState(array $state): AuthCodeEntity
 
         $scopes = array_map(
             /**
-             * @return ScopeEntity
+             * @return \SimpleSAML\Module\oidc\Entities\ScopeEntity
              */
-            fn(string $scope) => ScopeEntity::fromData($scope),
+            fn(string $scope) => $this->scopeEntityFactory->fromData($scope),
             $stateScopes,
         );
 
diff --git a/src/Factories/Entities/ScopeEntityFactory.php b/src/Factories/Entities/ScopeEntityFactory.php
@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SimpleSAML\Module\oidc\Factories\Entities;
+
+use SimpleSAML\Module\oidc\Entities\ScopeEntity;
+
+class ScopeEntityFactory
+{
+    /**
+     * @param string[] $claims
+     */
+    public function fromData(
+        string $identifier,
+        string $description = null,
+        string $icon = null,
+        array $claims = [],
+    ): ScopeEntity {
+        return new ScopeEntity(
+            $identifier,
+            $description,
+            $icon,
+            $claims,
+        );
+    }
+}
diff --git a/src/Repositories/ScopeRepository.php b/src/Repositories/ScopeRepository.php
@@ -20,12 +20,21 @@
 use League\OAuth2\Server\Repositories\ScopeRepositoryInterface;
 use SimpleSAML\Module\oidc\Entities\ClientEntity;
 use SimpleSAML\Module\oidc\Entities\ScopeEntity;
+use SimpleSAML\Module\oidc\Factories\Entities\ScopeEntityFactory;
+use SimpleSAML\Module\oidc\ModuleConfig;
 
 use function array_key_exists;
 use function in_array;
 
 class ScopeRepository extends AbstractDatabaseRepository implements ScopeRepositoryInterface
 {
+    public function __construct(
+        ModuleConfig $moduleConfig,
+        protected readonly ScopeEntityFactory $scopeEntityFactory,
+    ) {
+        parent::__construct($moduleConfig);
+    }
+
     public function getTableName(): ?string
     {
         return null;
@@ -52,7 +61,7 @@ public function getScopeEntityByIdentifier($identifier): ScopeEntity|ScopeEntity
         /** @var string[] $claims */
         $claims = $scope['claims'] ?? [];
 
-        return ScopeEntity::fromData(
+        return $this->scopeEntityFactory->fromData(
             $identifier,
             $description,
             $icon,
diff --git 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\ScopeEntityFactory;
 use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory;
 use SimpleSAML\Module\oidc\Factories\FederationFactory;
 use SimpleSAML\Module\oidc\Factories\FormFactory;
@@ -216,7 +217,13 @@ public function __construct()
         );
         $this->services[UserRepository::class] = $userRepository;
 
-        $authCodeEntityFactory = new AuthCodeEntityFactory($helpers);
+        $scopeEntityFactory = new ScopeEntityFactory();
+        $this->services[ScopeEntityFactory::class] = $scopeEntityFactory;
+
+        $authCodeEntityFactory = new AuthCodeEntityFactory(
+            $helpers,
+            $scopeEntityFactory,
+        );
         $this->services[AuthCodeEntityFactory::class] = $authCodeEntityFactory;
 
         $authCodeRepository = new AuthCodeRepository(
@@ -238,6 +245,7 @@ public function __construct()
             $helpers,
             $privateKey,
             $jsonWebTokenBuilderService,
+            $scopeEntityFactory,
         );
         $this->services[AccessTokenEntityFactory::class] = $accessTokenEntityFactory;
 
@@ -258,7 +266,7 @@ public function __construct()
         );
         $this->services[RefreshTokenRepository::class] = $refreshTokenRepository;
 
-        $scopeRepository = new ScopeRepository($moduleConfig);
+        $scopeRepository = new ScopeRepository($moduleConfig, $scopeEntityFactory);
         $this->services[ScopeRepository::class] = $scopeRepository;
 
         $allowedOriginRepository = new AllowedOriginRepository($moduleConfig);
diff --git 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\ScopeEntityFactory;
 use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory;
 use SimpleSAML\Module\oidc\Helpers;
 use SimpleSAML\Module\oidc\ModuleConfig;
@@ -124,6 +125,7 @@ public function setUp(): void
             new Helpers(),
             $this->privateKey,
             $this->createMock(JsonWebTokenBuilderService::class),
+            new ScopeEntityFactory(),
         );
     }
 
diff --git a/tests/unit/src/Entities/ScopeEntityTest.php b/tests/unit/src/Entities/ScopeEntityTest.php
@@ -9,26 +9,26 @@
 
 class ScopeEntityTest extends TestCase
 {
-    protected function prepareMockedInstance(
+    protected function mock(
         string $id = 'id',
         string $description = 'description',
         string $icon = 'icon',
         array $attributes = ['attrid' => 'attrval'],
     ): ScopeEntity {
-        return ScopeEntity::fromData($id, $description, $icon, $attributes);
+        return new ScopeEntity($id, $description, $icon, $attributes);
     }
 
     public function testItIsInitializable(): void
     {
         $this->assertInstanceOf(
             ScopeEntity::class,
-            $this->prepareMockedInstance(),
+            $this->mock(),
         );
     }
 
     public function testCanGetProperties(): void
     {
-        $scopeEntity = $this->prepareMockedInstance();
+        $scopeEntity = $this->mock();
         $this->assertSame('id', $scopeEntity->getIdentifier());
         $this->assertSame('description', $scopeEntity->getDescription());
         $this->assertSame('icon', $scopeEntity->getIcon());
diff --git a/tests/unit/src/Repositories/AuthCodeRepositoryTest.php b/tests/unit/src/Repositories/AuthCodeRepositoryTest.php
@@ -72,7 +72,7 @@ protected function setUp(): void
         $this->clientRepositoryMock = $this->createMock(ClientRepository::class);
         $this->clientRepositoryMock->method('findById')->willReturn($this->clientEntityMock);
 
-        $this->scopes = [ScopeEntity::fromData('openid')];
+        $this->scopes = [new ScopeEntity('openid')];
 
         $this->authCodeEntityFactoryMock = $this->createMock(AuthCodeEntityFactory::class);
 
@@ -96,10 +96,6 @@ public function testGetTableName(): void
      */
     public function testAddAndFound(): void
     {
-        $scopes = [
-            ScopeEntity::fromData('openid'),
-        ];
-
         $authCode = new AuthCodeEntity(
             self::AUTH_CODE_ID,
             $this->clientEntityMock,
diff --git a/tests/unit/src/Repositories/ScopeRepositoryTest.php b/tests/unit/src/Repositories/ScopeRepositoryTest.php
@@ -18,6 +18,7 @@
 use PHPUnit\Framework\TestCase;
 use SimpleSAML\Configuration;
 use SimpleSAML\Module\oidc\Entities\ScopeEntity;
+use SimpleSAML\Module\oidc\Factories\Entities\ScopeEntityFactory;
 use SimpleSAML\Module\oidc\ModuleConfig;
 use SimpleSAML\Module\oidc\Repositories\ScopeRepository;
 use SimpleSAML\Module\oidc\Services\DatabaseMigration;
@@ -48,11 +49,11 @@ public static function setUpBeforeClass(): void
      */
     public function testGetScopeEntityByIdentifier(): void
     {
-        $scopeRepository = new ScopeRepository(new ModuleConfig());
+        $scopeRepository = new ScopeRepository(new ModuleConfig(), new ScopeEntityFactory());
 
         $scope = $scopeRepository->getScopeEntityByIdentifier('openid');
 
-        $expected = ScopeEntity::fromData(
+        $expected = new ScopeEntity(
             'openid',
             'openid',
         );
@@ -65,7 +66,7 @@ public function testGetScopeEntityByIdentifier(): void
      */
     public function testGetUnknownScope(): void
     {
-        $scopeRepository = new ScopeRepository(new ModuleConfig());
+        $scopeRepository = new ScopeRepository(new ModuleConfig(), new ScopeEntityFactory());
 
         $this->assertNull($scopeRepository->getScopeEntityByIdentifier('none'));
     }
@@ -75,17 +76,17 @@ public function testGetUnknownScope(): void
      */
     public function testFinalizeScopes(): void
     {
-        $scopeRepository = new ScopeRepository(new ModuleConfig());
+        $scopeRepository = new ScopeRepository(new ModuleConfig(), new ScopeEntityFactory());
         $scopes = [
-            ScopeEntity::fromData('openid'),
-            ScopeEntity::fromData('basic'),
+            new ScopeEntity('openid'),
+            new ScopeEntity('basic'),
         ];
         $client = ClientRepositoryTest::getClient('clientid');
 
         $finalizedScopes = $scopeRepository->finalizeScopes($scopes, 'any', $client);
 
         $expectedScopes = [
-            ScopeEntity::fromData('openid'),
+            new ScopeEntity('openid'),
         ];
         $this->assertEquals($expectedScopes, $finalizedScopes);
     }
diff --git a/tests/unit/src/Server/RequestRules/Rules/RequiredOpenIdScopeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/RequiredOpenIdScopeRuleTest.php
@@ -44,8 +44,8 @@ protected function setUp(): void
         $this->stateResult = new Result(StateRule::class, '123');
         $this->requestStub = $this->createStub(ServerRequestInterface::class);
         $this->scopeEntities = [
-            'openid' => ScopeEntity::fromData('openid'),
-            'profile' => ScopeEntity::fromData('profile'),
+            'openid' => new ScopeEntity('openid'),
+            'profile' => new ScopeEntity('profile'),
         ];
         $this->scopeResult = new Result(ScopeRule::class, $this->scopeEntities);
         $this->loggerServiceStub = $this->createStub(LoggerService::class);
@@ -108,7 +108,7 @@ public function testCheckRuleThrowsWhenOpenIdScopeIsNotPresent()
         $resultBag->add($this->redirectUriResult);
         $resultBag->add($this->stateResult);
         $invalidScopeEntities = [
-            'profile' => ScopeEntity::fromData('profile'),
+            'profile' => new ScopeEntity('profile'),
         ];
         $resultBag->add(new Result(ScopeRule::class, $invalidScopeEntities));
 
diff --git a/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php
@@ -59,8 +59,8 @@ protected function setUp(): void
         $this->stateResult = new Result(StateRule::class, '123');
         $this->requestStub = $this->createStub(ServerRequestInterface::class);
         $this->scopeEntities = [
-            'openid' => ScopeEntity::fromData('openid'),
-            'profile' => ScopeEntity::fromData('profile'),
+            'openid' => new ScopeEntity('openid'),
+            'profile' => new ScopeEntity('profile'),
         ];
         $this->loggerServiceStub = $this->createStub(LoggerService::class);
         $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class);
diff --git a/tests/unit/src/Server/ResponseTypes/IdTokenResponseTest.php b/tests/unit/src/Server/ResponseTypes/IdTokenResponseTest.php
@@ -76,8 +76,8 @@ protected function setUp(): void
             ['cn'  => ['Homer Simpson'], 'mail' => ['myEmail@example.com'],],
         );
         $this->scopes = [
-            ScopeEntity::fromData('openid'),
-            ScopeEntity::fromData('email'),
+            new ScopeEntity('openid'),
+            new ScopeEntity('email'),
         ];
         $this->expiration = (new DateTimeImmutable())->setTimestamp(time() + 3600);
 
@@ -184,7 +184,7 @@ public function testItCanGenerateResponseWithIndividualRequestedClaims(): void
                 ],
             );
         $this->accessTokenEntityMock->method('getScopes')->willReturn(
-            [ScopeEntity::fromData('openid')],
+            [new ScopeEntity('openid')],
         );
         $idTokenResponse->setAccessToken($this->accessTokenEntityMock);
         $response = $idTokenResponse->generateHttpResponse(new Response());
@@ -198,7 +198,7 @@ public function testNoExtraParamsForNonOidcRequest(): void
     {
         $this->accessTokenEntityMock->method('getRequestedClaims')->willReturn([]);
         $this->accessTokenEntityMock->method('getScopes')->willReturn(
-            [ScopeEntity::fromData('profile')],
+            [new ScopeEntity('profile')],
         );
         $idTokenResponse = $this->prepareMockedInstance();
         $idTokenResponse->setAccessToken($this->accessTokenEntityMock);
diff --git a/tests/unit/src/Server/Validators/BearerTokenValidatorTest.php b/tests/unit/src/Server/Validators/BearerTokenValidatorTest.php

Original file line number	Diff line number	Diff line change
`@@ -9,26 +9,26 @@`
`9`	`9`
`10`	`10`	`class ScopeEntityTest extends TestCase`
`11`	`11`	`{`
`12`		`- protected function prepareMockedInstance(`
	`12`	`+ protected function mock(`
`13`	`13`	`string $id = 'id',`
`14`	`14`	`string $description = 'description',`
`15`	`15`	`string $icon = 'icon',`
`16`	`16`	`array $attributes = ['attrid' => 'attrval'],`
`17`	`17`	`): ScopeEntity {`
`18`		`- return ScopeEntity::fromData($id, $description, $icon, $attributes);`
	`18`	`+ return new ScopeEntity($id, $description, $icon, $attributes);`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`public function testItIsInitializable(): void`
`22`	`22`	`{`
`23`	`23`	`$this->assertInstanceOf(`
`24`	`24`	`ScopeEntity::class,`
`25`		`- $this->prepareMockedInstance(),`
	`25`	`+ $this->mock(),`
`26`	`26`	`);`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`public function testCanGetProperties(): void`
`30`	`30`	`{`
`31`		`- $scopeEntity = $this->prepareMockedInstance();`
	`31`	`+ $scopeEntity = $this->mock();`
`32`	`32`	`$this->assertSame('id', $scopeEntity->getIdentifier());`
`33`	`33`	`$this->assertSame('description', $scopeEntity->getDescription());`
`34`	`34`	`$this->assertSame('icon', $scopeEntity->getIcon());`