From 3038fafc7b4ae5205b0478fc6bb527a028fc1a62 Mon Sep 17 00:00:00 2001 From: Marko Ivancic Date: Thu, 6 Feb 2025 14:59:29 +0100 Subject: [PATCH] Remove globals from unit tests --- ...AuthenticatedGetClientFromRequestTrait.php | 58 ------------------- src/Helpers/Client.php | 11 ++-- src/Services/AuthenticationService.php | 9 +-- .../src/Entities/AccessTokenEntityTest.php | 2 - tests/unit/src/Forms/ClientFormTest.php | 7 --- tests/unit/src/Helpers/ClientTest.php | 7 +-- .../Validators/BearerTokenValidatorTest.php | 6 -- .../Services/AuthenticationServiceTest.php | 14 +---- 8 files changed, 14 insertions(+), 100 deletions(-) delete mode 100644 src/Controllers/Traits/AuthenticatedGetClientFromRequestTrait.php diff --git a/src/Controllers/Traits/AuthenticatedGetClientFromRequestTrait.php b/src/Controllers/Traits/AuthenticatedGetClientFromRequestTrait.php deleted file mode 100644 index 2c95bf00..00000000 --- a/src/Controllers/Traits/AuthenticatedGetClientFromRequestTrait.php +++ /dev/null @@ -1,58 +0,0 @@ -getQueryParams(); - $clientId = empty($params['client_id']) ? null : (string)$params['client_id']; - - if (!is_string($clientId)) { - throw new Error\BadRequest('Client id is missing.'); - } - $authedUser = null; - if (!$this->authContextService->isSspAdmin()) { - $authedUser = $this->authContextService->getAuthUserId(); - } - $client = $this->clientRepository->findById($clientId, $authedUser); - if (!$client) { - throw OidcServerException::invalidClient($request); - } - - return $client; - } -} diff --git a/src/Helpers/Client.php b/src/Helpers/Client.php index 8928154f..a2afddbc 100644 --- a/src/Helpers/Client.php +++ b/src/Helpers/Client.php @@ -5,9 +5,8 @@ namespace SimpleSAML\Module\oidc\Helpers; use Psr\Http\Message\ServerRequestInterface; -use SimpleSAML\Error\BadRequest; -use SimpleSAML\Error\NotFound; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; +use SimpleSAML\Module\oidc\Exceptions\OidcException; use SimpleSAML\Module\oidc\Repositories\ClientRepository; class Client @@ -18,9 +17,7 @@ public function __construct(protected Http $http) /** * @throws \JsonException - * @throws \SimpleSAML\Error\BadRequest - * @throws \SimpleSAML\Error\NotFound - * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException + * @throws \SimpleSAML\Module\oidc\Exceptions\OidcException */ public function getFromRequest( ServerRequestInterface $request, @@ -30,13 +27,13 @@ public function getFromRequest( $clientId = empty($params['client_id']) ? null : (string)$params['client_id']; if (!is_string($clientId)) { - throw new BadRequest('Client ID is missing.'); + throw new OidcException('Client ID is missing.'); } $client = $clientRepository->findById($clientId); if (!$client) { - throw new NotFound('Client not found.'); + throw new OidcException('Client not found.'); } return $client; diff --git a/src/Services/AuthenticationService.php b/src/Services/AuthenticationService.php index 06ffd8ef..7d8c7afa 100644 --- a/src/Services/AuthenticationService.php +++ b/src/Services/AuthenticationService.php @@ -28,6 +28,7 @@ use SimpleSAML\Module\oidc\Controllers\EndSessionController; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; use SimpleSAML\Module\oidc\Entities\UserEntity; +use SimpleSAML\Module\oidc\Exceptions\OidcException; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory; use SimpleSAML\Module\oidc\Factories\ProcessingChainFactory; @@ -78,12 +79,11 @@ public function __construct( * * @return array * @throws Error\AuthSource - * @throws Error\BadRequest - * @throws Error\NotFound * @throws Exception * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException * @throws Error\UnserializableException * @throws \JsonException + * @throws \SimpleSAML\Module\oidc\Exceptions\OidcException */ public function processRequest( ServerRequestInterface $request, @@ -117,13 +117,14 @@ public function processRequest( /** - * @param array|null $state + * @param array|null $state * * @return UserEntity * @throws Error\NotFound * @throws Exception * @throws \JsonException * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException + * @throws \SimpleSAML\Module\oidc\Exceptions\OidcException */ public function getAuthenticateUser( ?array $state, @@ -164,7 +165,7 @@ public function getAuthenticateUser( $client = $this->clientRepository->findById((string)$state['Oidc']['RelyingPartyMetadata']['id']); if (!$client) { - throw new Error\NotFound('Client not found.'); + throw new OidcException('Client not found.'); } $this->addRelyingPartyAssociation($client, $user); diff --git a/tests/unit/src/Entities/AccessTokenEntityTest.php b/tests/unit/src/Entities/AccessTokenEntityTest.php index 1bb4bebf..b7d6d1c1 100644 --- a/tests/unit/src/Entities/AccessTokenEntityTest.php +++ b/tests/unit/src/Entities/AccessTokenEntityTest.php @@ -18,8 +18,6 @@ /** * @covers \SimpleSAML\Module\oidc\Entities\AccessTokenEntity - * - * @backupGlobals enabled */ class AccessTokenEntityTest extends TestCase { diff --git a/tests/unit/src/Forms/ClientFormTest.php b/tests/unit/src/Forms/ClientFormTest.php index 4f24c893..59700f2e 100644 --- a/tests/unit/src/Forms/ClientFormTest.php +++ b/tests/unit/src/Forms/ClientFormTest.php @@ -40,13 +40,6 @@ public function setUp(): void $this->serverRequestMock = $this->createMock(ServerRequest::class); } - public static function setUpBeforeClass(): void - { - // To make lib/SimpleSAML/Utils/HTTP::getSelfURL() work... - global $_SERVER; - $_SERVER['REQUEST_URI'] = '/'; - } - public static function validateOriginProvider(): array { return [ diff --git a/tests/unit/src/Helpers/ClientTest.php b/tests/unit/src/Helpers/ClientTest.php index 7916e678..b79b052a 100644 --- a/tests/unit/src/Helpers/ClientTest.php +++ b/tests/unit/src/Helpers/ClientTest.php @@ -8,9 +8,8 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; -use SimpleSAML\Error\BadRequest; -use SimpleSAML\Error\NotFound; use SimpleSAML\Module\oidc\Entities\ClientEntity; +use SimpleSAML\Module\oidc\Exceptions\OidcException; use SimpleSAML\Module\oidc\Helpers\Client; use SimpleSAML\Module\oidc\Helpers\Http; use SimpleSAML\Module\oidc\Repositories\ClientRepository; @@ -56,7 +55,7 @@ public function testCanGetFromRequest(): void public function testGetFromRequestThrowsIfNoClientId(): void { - $this->expectException(BadRequest::class); + $this->expectException(OidcException::class); $this->expectExceptionMessage('Client ID'); $this->sut()->getFromRequest($this->requestMock, $this->clientRepositoryMock); @@ -64,7 +63,7 @@ public function testGetFromRequestThrowsIfNoClientId(): void public function testGetFromRequestThrowsIfClientNotFound(): void { - $this->expectException(NotFound::class); + $this->expectException(OidcException::class); $this->expectExceptionMessage('Client not found'); $this->httpMock->expects($this->once())->method('getAllRequestParams') diff --git a/tests/unit/src/Server/Validators/BearerTokenValidatorTest.php b/tests/unit/src/Server/Validators/BearerTokenValidatorTest.php index a47b3174..daa8bf19 100644 --- a/tests/unit/src/Server/Validators/BearerTokenValidatorTest.php +++ b/tests/unit/src/Server/Validators/BearerTokenValidatorTest.php @@ -24,8 +24,6 @@ /** * @covers \SimpleSAML\Module\oidc\Server\Validators\BearerTokenValidator - * - * @backupGlobals enabled */ class BearerTokenValidatorTest extends TestCase { @@ -60,10 +58,6 @@ public function setUp(): void */ public static function setUpBeforeClass(): void { - // To make lib/SimpleSAML/Utils/HTTP::getSelfURL() work... - global $_SERVER; - $_SERVER['REQUEST_URI'] = ''; - $tempDir = sys_get_temp_dir(); // Plant certdir config for JsonWebTokenBuilderService (since we don't inject it) diff --git a/tests/unit/src/Services/AuthenticationServiceTest.php b/tests/unit/src/Services/AuthenticationServiceTest.php index 962ba1db..fd9b2024 100644 --- a/tests/unit/src/Services/AuthenticationServiceTest.php +++ b/tests/unit/src/Services/AuthenticationServiceTest.php @@ -17,9 +17,9 @@ use SimpleSAML\Auth\State; use SimpleSAML\Error\Exception; use SimpleSAML\Error\NoState; -use SimpleSAML\Error\NotFound; use SimpleSAML\Module\oidc\Entities\ClientEntity; use SimpleSAML\Module\oidc\Entities\UserEntity; +use SimpleSAML\Module\oidc\Exceptions\OidcException; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory; use SimpleSAML\Module\oidc\Factories\ProcessingChainFactory; @@ -84,16 +84,6 @@ class AuthenticationServiceTest extends TestCase protected MockObject $requestParamsResolverMock; protected MockObject $userEntityFactoryMock; - /** - * @return void - */ - public static function setUpBeforeClass(): void - { - // To make lib/SimpleSAML/Utils/HTTP::getSelfURL() work... - global $_SERVER; - $_SERVER['REQUEST_URI'] = ''; - } - /** * @throws \PHPUnit\Framework\MockObject\Exception */ @@ -280,7 +270,7 @@ public static function getUserState(): array 'AuthorizationRequestParameters' => self::AUTHZ_REQUEST_PARAMS, ], ], - NotFound::class, + OidcException::class, '/Client not found./', ], ];