Merge pull request #1407 from eugene-borovov/fix-passport-compatibility

Parse request parameters
thephpleague · May 10, 2024 · 7596935 · 7596935
2 parents d1dc453 + df39bf6
commit 7596935
Show file tree

Hide file tree

Showing 16 changed files with 95 additions and 84 deletions.
diff --git 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
diff --git 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
@@ -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
@@ -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<array-key,mixed> $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
@@ -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,19 +127,19 @@ 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
     {
         $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,25 +195,22 @@ 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
     {
         [$basicAuthUser, $basicAuthPassword] = $this->getBasicAuthCredentials($request);
 
         $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<array-key, mixed> $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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<array-key,mixed>
      */
     protected function getExtraParams(AccessTokenEntityInterface $accessToken): array
     {

diff --git 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<array-key,mixed>
      */
     protected function getExtraParams(DeviceCodeEntityInterface $deviceCode): array
     {