diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 7bef99433..7949ac1d8 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -2,8 +2,4 @@ parameters: level: 8 paths: - src - - tests - ignoreErrors: - - - message: '#Call to an undefined method League\\OAuth2\\Server\\ResponseTypes\\ResponseTypeInterface::getAccessToken\(\)\.#' - path: tests/Grant/ClientCredentialsGrantTest.php + - tests \ No newline at end of file diff --git a/src/CryptKey.php b/src/CryptKey.php index 944c56a05..45011b1cf 100644 --- a/src/CryptKey.php +++ b/src/CryptKey.php @@ -43,13 +43,13 @@ class CryptKey implements CryptKeyInterface public function __construct(string $keyPath, protected ?string $passPhrase = null, bool $keyPermissionsCheck = true) { - if (strpos($keyPath, self::FILE_PREFIX) !== 0 && $this->isValidKey($keyPath, $this->passPhrase ?? '')) { + if (!str_starts_with($keyPath, self::FILE_PREFIX) && $this->isValidKey($keyPath, $this->passPhrase ?? '')) { $this->keyContents = $keyPath; $this->keyPath = ''; // There's no file, so no need for permission check. $keyPermissionsCheck = false; } elseif (is_file($keyPath)) { - if (strpos($keyPath, self::FILE_PREFIX) !== 0) { + if (!str_starts_with($keyPath, self::FILE_PREFIX)) { $keyPath = self::FILE_PREFIX . $keyPath; } diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index aa5d0cb6a..dd8901bc1 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -269,9 +269,9 @@ public function generateHttpResponse(ResponseInterface $response, bool $useFragm if ($this->redirectUri !== null) { if ($useFragment === true) { - $this->redirectUri .= (strstr($this->redirectUri, '#') === false) ? '#' : '&'; + $this->redirectUri .= (!str_contains($this->redirectUri, '#')) ? '#' : '&'; } else { - $this->redirectUri .= (strstr($this->redirectUri, '?') === false) ? '?' : '&'; + $this->redirectUri .= (!str_contains($this->redirectUri, '?')) ? '?' : '&'; } return $response->withStatus(302)->withHeader('Location', $this->redirectUri . http_build_query($payload)); @@ -310,7 +310,7 @@ public function getHttpHeaders(): array // include the "WWW-Authenticate" response header field // matching the authentication scheme used by the client. if ($this->errorType === 'invalid_client' && $this->requestHasAuthorizationHeader()) { - $authScheme = strpos($this->serverRequest->getHeader('Authorization')[0], 'Bearer') === 0 ? 'Bearer' : 'Basic'; + $authScheme = str_starts_with($this->serverRequest->getHeader('Authorization')[0], 'Bearer') ? 'Bearer' : 'Basic'; $headers['WWW-Authenticate'] = $authScheme . ' realm="OAuth"'; } diff --git a/src/Grant/AbstractAuthorizeGrant.php b/src/Grant/AbstractAuthorizeGrant.php index 4fe552c17..22b58c4e2 100644 --- a/src/Grant/AbstractAuthorizeGrant.php +++ b/src/Grant/AbstractAuthorizeGrant.php @@ -18,16 +18,15 @@ use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface; use function http_build_query; -use function strstr; abstract class AbstractAuthorizeGrant extends AbstractGrant { /** - * @param mixed[] $params + * @param array $params */ public function makeRedirectUri(string $uri, array $params = [], string $queryDelimiter = '?'): string { - $uri .= (strstr($uri, $queryDelimiter) === false) ? $queryDelimiter : '&'; + $uri .= str_contains($uri, $queryDelimiter) ? '&' : $queryDelimiter; return $uri . http_build_query($params); } diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index d35b4fd10..167ab3f04 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -49,10 +49,8 @@ use function base64_decode; use function bin2hex; use function explode; -use function is_null; use function is_string; use function random_bytes; -use function strpos; use function substr; use function trim; @@ -129,9 +127,9 @@ public function setRefreshTokenTTL(DateInterval $refreshTokenTTL): void /** * Set the private key */ - public function setPrivateKey(CryptKeyInterface $key): void + public function setPrivateKey(CryptKeyInterface $privateKey): void { - $this->privateKey = $key; + $this->privateKey = $privateKey; } public function setDefaultScope(string $scope): void @@ -139,9 +137,9 @@ public function setDefaultScope(string $scope): void $this->defaultScope = $scope; } - public function revokeRefreshTokens(bool $revokeRefreshTokens): void + public function revokeRefreshTokens(bool $willRevoke): void { - $this->revokeRefreshTokens = $revokeRefreshTokens; + $this->revokeRefreshTokens = $willRevoke; } /** @@ -161,13 +159,9 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity $client = $this->getClientEntityOrFail($clientId, $request); // If a redirect URI is provided ensure it matches what is pre-registered - $redirectUri = $this->getRequestParameter('redirect_uri', $request, null); + $redirectUri = $this->getRequestParameter('redirect_uri', $request); if ($redirectUri !== null) { - if (!is_string($redirectUri)) { - throw OAuthServerException::invalidRequest('redirect_uri'); - } - $this->validateRedirectUri($redirectUri, $client, $request); } @@ -183,6 +177,7 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity * doesn't actually enforce non-null returns/exception-on-no-client so * getClientEntity might return null. By contrast, this method will * always either return a ClientEntityInterface or throw. + * @throws OAuthServerException */ protected function getClientEntityOrFail(string $clientId, ServerRequestInterface $request): ClientEntityInterface { @@ -200,7 +195,8 @@ protected function getClientEntityOrFail(string $clientId, ServerRequestInterfac * Gets the client credentials from the request from the request body or * the Http Basic Authorization header * - * @return mixed[] + * @return array{0:non-empty-string,1:string} + * @throws OAuthServerException */ protected function getClientCredentials(ServerRequestInterface $request): array { @@ -208,17 +204,13 @@ protected function getClientCredentials(ServerRequestInterface $request): array $clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser); - if (is_null($clientId)) { + if ($clientId === null) { throw OAuthServerException::invalidRequest('client_id'); } $clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword); - if ($clientSecret !== null && !is_string($clientSecret)) { - throw OAuthServerException::invalidRequest('client_secret'); - } - - return [$clientId, $clientSecret]; + return [$clientId, $clientSecret ?? '']; } /** @@ -279,19 +271,43 @@ public function validateScopes(string|array|null $scopes, string $redirectUri = */ private function convertScopesQueryStringToArray(string $scopes): array { - return array_filter(explode(self::SCOPE_DELIMITER_STRING, trim($scopes)), function ($scope) { - return $scope !== ''; - }); + return array_filter(explode(self::SCOPE_DELIMITER_STRING, trim($scopes)), static fn($scope) => $scope !== ''); } /** - * Retrieve request parameter. + * Parse request parameter. + * @param array $request + * @return non-empty-string|null + * @throws OAuthServerException */ - protected function getRequestParameter(string $parameter, ServerRequestInterface $request, mixed $default = null): mixed + private static function parseParam(string $parameter, array $request, ?string $default = null): ?string { - $requestParameters = (array) $request->getParsedBody(); + $value = $request[$parameter] ?? ''; + if (is_scalar($value)) { + $value = trim((string)$value); + } else { + throw OAuthServerException::invalidRequest($parameter); + } + + if ($value === '') { + $value = $default === null ? null : trim($default); + if ($value === '') { + $value = null; + } + } + + return $value; + } - return $requestParameters[$parameter] ?? $default; + /** + * Retrieve request parameter. + * + * @return non-empty-string|null + * @throws OAuthServerException + */ + protected function getRequestParameter(string $parameter, ServerRequestInterface $request, ?string $default = null): ?string + { + return self::parseParam($parameter, (array) $request->getParsedBody(), $default); } /** @@ -301,7 +317,7 @@ protected function getRequestParameter(string $parameter, ServerRequestInterface * not exist, or is otherwise an invalid HTTP Basic header, return * [null, null]. * - * @return string[]|null[] + * @return array{0:non-empty-string,1:string}|array{0:null,1:null} */ protected function getBasicAuthCredentials(ServerRequestInterface $request): array { @@ -310,7 +326,7 @@ protected function getBasicAuthCredentials(ServerRequestInterface $request): arr } $header = $request->getHeader('Authorization')[0]; - if (strpos($header, 'Basic ') !== 0) { + if (!str_starts_with($header, 'Basic ')) { return [null, null]; } @@ -320,35 +336,47 @@ protected function getBasicAuthCredentials(ServerRequestInterface $request): arr return [null, null]; } - if (strpos($decoded, ':') === false) { + if (!str_contains($decoded, ':')) { return [null, null]; // HTTP Basic header without colon isn't valid } - return explode(':', $decoded, 2); + [$username, $password] = explode(':', $decoded, 2); + + if ($username === '') { + return [null, null]; + } + + return [$username, $password]; } /** * Retrieve query string parameter. + * @return non-empty-string|null + * @throws OAuthServerException */ - protected function getQueryStringParameter(string $parameter, ServerRequestInterface $request, mixed $default = null): mixed + protected function getQueryStringParameter(string $parameter, ServerRequestInterface $request, ?string $default = null): ?string { - return isset($request->getQueryParams()[$parameter]) ? $request->getQueryParams()[$parameter] : $default; + return self::parseParam($parameter, $request->getQueryParams(), $default); } /** * Retrieve cookie parameter. + * @return non-empty-string|null + * @throws OAuthServerException */ - protected function getCookieParameter(string $parameter, ServerRequestInterface $request, mixed $default = null): mixed + protected function getCookieParameter(string $parameter, ServerRequestInterface $request, ?string $default = null): ?string { - return isset($request->getCookieParams()[$parameter]) ? $request->getCookieParams()[$parameter] : $default; + return self::parseParam($parameter, $request->getCookieParams(), $default); } /** * Retrieve server parameter. + * @return non-empty-string|null + * @throws OAuthServerException */ - protected function getServerParameter(string $parameter, ServerRequestInterface $request, mixed $default = null): mixed + protected function getServerParameter(string $parameter, ServerRequestInterface $request, ?string $default = null): ?string { - return isset($request->getServerParams()[$parameter]) ? $request->getServerParams()[$parameter] : $default; + return self::parseParam($parameter, $request->getServerParams(), $default); } /** diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 83bddac18..8a24a8e95 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -41,7 +41,6 @@ use function implode; use function in_array; use function is_array; -use function is_string; use function json_decode; use function json_encode; use function preg_match; @@ -106,9 +105,9 @@ public function respondToAccessTokenRequest( $this->validateClient($request); } - $encryptedAuthCode = $this->getRequestParameter('code', $request, null); + $encryptedAuthCode = $this->getRequestParameter('code', $request); - if (!is_string($encryptedAuthCode)) { + if ($encryptedAuthCode === null) { throw OAuthServerException::invalidRequest('code'); } @@ -128,7 +127,7 @@ public function respondToAccessTokenRequest( throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the authorization code', $e); } - $codeVerifier = $this->getRequestParameter('code_verifier', $request, null); + $codeVerifier = $this->getRequestParameter('code_verifier', $request); // If a code challenge isn't present but a code verifier is, reject the request to block PKCE downgrade attack if (!isset($authCodePayload->code_challenge) && $codeVerifier !== null) { @@ -219,7 +218,7 @@ private function validateAuthorizationCode( } // The redirect URI is required in this request - $redirectUri = $this->getRequestParameter('redirect_uri', $request, null); + $redirectUri = $this->getRequestParameter('redirect_uri', $request); if ($authCodePayload->redirect_uri !== '' && $redirectUri === null) { throw OAuthServerException::invalidRequest('redirect_uri'); } diff --git a/src/Grant/DeviceCodeGrant.php b/src/Grant/DeviceCodeGrant.php index 4a34ade78..3804e2027 100644 --- a/src/Grant/DeviceCodeGrant.php +++ b/src/Grant/DeviceCodeGrant.php @@ -304,9 +304,7 @@ protected function generateUserCode(int $length = 8): string return $userCode; // @codeCoverageIgnoreStart - } catch (TypeError $e) { - throw OAuthServerException::serverError('An unexpected error has occurred', $e); - } catch (Error $e) { + } catch (TypeError | Error $e) { throw OAuthServerException::serverError('An unexpected error has occurred', $e); } catch (Exception $e) { // If you get this message, the CSPRNG failed hard. diff --git a/src/Grant/PasswordGrant.php b/src/Grant/PasswordGrant.php index 1b92dc1da..f5d8da322 100644 --- a/src/Grant/PasswordGrant.php +++ b/src/Grant/PasswordGrant.php @@ -26,8 +26,6 @@ use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; use Psr\Http\Message\ServerRequestInterface; -use function is_string; - /** * Password grant class. */ @@ -84,17 +82,11 @@ public function respondToAccessTokenRequest( */ protected function validateUser(ServerRequestInterface $request, ClientEntityInterface $client): UserEntityInterface { - $username = $this->getRequestParameter('username', $request); - - if (!is_string($username)) { - throw OAuthServerException::invalidRequest('username'); - } + $username = $this->getRequestParameter('username', $request) + ?? throw OAuthServerException::invalidRequest('username'); - $password = $this->getRequestParameter('password', $request); - - if (!is_string($password)) { - throw OAuthServerException::invalidRequest('password'); - } + $password = $this->getRequestParameter('password', $request) + ?? throw OAuthServerException::invalidRequest('password'); $user = $this->userRepository->getUserEntityByUserCredentials( $username, diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php index aff772a95..a632990c7 100644 --- a/src/Grant/RefreshTokenGrant.php +++ b/src/Grant/RefreshTokenGrant.php @@ -26,7 +26,6 @@ use function implode; use function in_array; -use function is_string; use function json_decode; use function time; @@ -103,11 +102,8 @@ public function respondToAccessTokenRequest( */ protected function validateOldRefreshToken(ServerRequestInterface $request, string $clientId): array { - $encryptedRefreshToken = $this->getRequestParameter('refresh_token', $request); - - if (!is_string($encryptedRefreshToken)) { - throw OAuthServerException::invalidRequest('refresh_token'); - } + $encryptedRefreshToken = $this->getRequestParameter('refresh_token', $request) + ?? throw OAuthServerException::invalidRequest('refresh_token'); // Validate refresh token try { diff --git a/src/ResponseTypes/BearerTokenResponse.php b/src/ResponseTypes/BearerTokenResponse.php index c33bdc712..dd49b99ba 100644 --- a/src/ResponseTypes/BearerTokenResponse.php +++ b/src/ResponseTypes/BearerTokenResponse.php @@ -73,7 +73,7 @@ public function generateHttpResponse(ResponseInterface $response): ResponseInter * AuthorizationServer::getResponseType() to pull in your version of * this class rather than the default. * - * @return mixed[] + * @return array */ protected function getExtraParams(AccessTokenEntityInterface $accessToken): array { diff --git a/src/ResponseTypes/DeviceCodeResponse.php b/src/ResponseTypes/DeviceCodeResponse.php index 339c75bd3..91a8df69a 100644 --- a/src/ResponseTypes/DeviceCodeResponse.php +++ b/src/ResponseTypes/DeviceCodeResponse.php @@ -84,7 +84,7 @@ public function includeVerificationUriComplete(): void * AuthorizationServer::getResponseType() to pull in your version of * this class rather than the default. * - * @return mixed[] + * @return array */ protected function getExtraParams(DeviceCodeEntityInterface $deviceCode): array { diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php index 27922b427..93db59f2a 100644 --- a/tests/Exception/OAuthServerExceptionTest.php +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -123,7 +123,7 @@ public function testHasPrevious(): void $previous = new Exception('This is the previous'); $exceptionWithPrevious = OAuthServerException::accessDenied(null, null, $previous); - $previousMessage = $exceptionWithPrevious->getPrevious() !== null ? $exceptionWithPrevious->getPrevious()->getMessage() : null; + $previousMessage = $exceptionWithPrevious->getPrevious()?->getMessage(); self::assertSame('This is the previous', $previousMessage); } diff --git a/tests/Grant/ClientCredentialsGrantTest.php b/tests/Grant/ClientCredentialsGrantTest.php index 264e026e2..69f756c37 100644 --- a/tests/Grant/ClientCredentialsGrantTest.php +++ b/tests/Grant/ClientCredentialsGrantTest.php @@ -59,6 +59,8 @@ public function testRespondToRequest(): void ]); $responseType = new StubResponseType(); + + /** @var StubResponseType $response */ $response = $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); self::assertNotEmpty($response->getAccessToken()->getIdentifier()); diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index 515629247..c2b943197 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -23,6 +23,7 @@ use LeagueTests\Stubs\StubResponseType; use LeagueTests\Stubs\UserEntity; use LogicException; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; class ImplicitGrantTest extends TestCase @@ -282,7 +283,7 @@ public function testAccessTokenRepositoryUniqueConstraintCheck(): void $accessToken->setClient($client); $accessToken->setUserIdentifier('userId'); - /** @var AccessTokenRepositoryInterface|\PHPUnit\Framework\MockObject\MockObject $accessTokenRepositoryMock */ + /** @var AccessTokenRepositoryInterface|MockObject $accessTokenRepositoryMock */ $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken); @@ -321,7 +322,7 @@ public function testAccessTokenRepositoryFailToPersist(): void $authRequest->setGrantTypeId('authorization_code'); $authRequest->setUser(new UserEntity()); - /** @var AccessTokenRepositoryInterface|\PHPUnit\Framework\MockObject\MockObject $accessTokenRepositoryMock */ + /** @var AccessTokenRepositoryInterface|MockObject $accessTokenRepositoryMock */ $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); $accessTokenRepositoryMock->method('persistNewAccessToken')->willThrowException(OAuthServerException::serverError('something bad happened')); @@ -353,7 +354,7 @@ public function testAccessTokenRepositoryFailToPersistUniqueNoInfiniteLoop(): vo $authRequest->setGrantTypeId('authorization_code'); $authRequest->setUser(new UserEntity()); - /** @var AccessTokenRepositoryInterface|\PHPUnit\Framework\MockObject\MockObject $accessTokenRepositoryMock */ + /** @var AccessTokenRepositoryInterface|MockObject $accessTokenRepositoryMock */ $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); $accessTokenRepositoryMock->method('persistNewAccessToken')->willThrowException(UniqueTokenIdentifierConstraintViolationException::create()); diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php index e3ff7bd0e..b37001a80 100644 --- a/tests/Grant/RefreshTokenGrantTest.php +++ b/tests/Grant/RefreshTokenGrantTest.php @@ -595,7 +595,7 @@ public function testRespondToRequestFinalizeScopes(): void 'client_id' => 'foo', 'client_secret' => 'bar', 'refresh_token' => $encryptedOldRefreshToken, - 'scope' => ['foo', 'bar'], + 'scope' => 'foo bar', ]); $responseType = new StubResponseType(); @@ -654,7 +654,7 @@ public function testRevokedRefreshToken(): void 'client_id' => 'foo', 'client_secret' => 'bar', 'refresh_token' => $encryptedOldRefreshToken, - 'scope' => ['foo'], + 'scope' => 'foo', ]); $grant = new RefreshTokenGrant($refreshTokenRepositoryMock); @@ -719,7 +719,7 @@ public function testUnrevokedRefreshToken(): void 'client_id' => 'foo', 'client_secret' => 'bar', 'refresh_token' => $encryptedOldRefreshToken, - 'scope' => ['foo'], + 'scope' => 'foo', ]); $grant = new RefreshTokenGrant($refreshTokenRepositoryMock); diff --git a/tests/Stubs/StubResponseType.php b/tests/Stubs/StubResponseType.php index 0f8f371a1..02f6f14e8 100644 --- a/tests/Stubs/StubResponseType.php +++ b/tests/Stubs/StubResponseType.php @@ -35,7 +35,7 @@ public function setRefreshToken(RefreshTokenEntityInterface $refreshToken): void } /** - * @throws \League\OAuth2\Server\Exception\OAuthServerException + * @throws OAuthServerException */ public function validateAccessToken(ServerRequestInterface $request): ServerRequestInterface {