Merge remote-tracking branch 'upstream/master' into bugfix/scope-named-0-considered-to-be-invalid

Author: Sephster

.github/dependabot.yml

@@ -0,0 +1,12 @@
+version: 2
+updates:
+- package-ecosystem: composer
+  directory: "/"
+  schedule:
+    interval: daily
+    time: "11:00"
+  open-pull-requests-limit: 10
+  ignore:
+  - dependency-name: league/event
+    versions:
+    - 3.0.0

.github/workflows/tests.yml

@@ -36,7 +36,7 @@ jobs:
         run: vendor/bin/phpunit --verbose --coverage-clover=coverage.clover
 
       - name: Code coverage
-        if: ${{ github.ref == 'refs/heads/master' && matrix.php != 8.0 }}
+        if: ${{ github.ref == 'refs/heads/master' && matrix.php != 8.0 && github.repository == 'thephpleague/oauth2-server' }}
         run: |
           wget https://scrutinizer-ci.com/ocular.phar
           php ocular.phar code-coverage:upload --format=php-clover coverage.clover

.gitignore

@@ -1,6 +1,7 @@
 /vendor
 /composer.lock
 phpunit.xml
+.phpunit.result.cache
 .idea
 /examples/vendor
 examples/public.key

CHANGELOG.md

@@ -5,8 +5,16 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Added
+- The server will now validate redirect uris according to rfc8252 (PR #1203)
+- Events emitted now include the refresh token and access token payloads (PR #1211)
+
+### Changed
+- Keys are now validated using `openssl_pkey_get_private()` and openssl_pkey_get_public()` instead of regex matching (PR #1215)
+
 ### Fixed
 - The server will now only recognise and handle an authorization header if the value of the header is non-empty. This is to circumvent issues where some common frameworks set this header even if no value is present (PR #1170)
+- Added type validation for redirect uri, client ID, client secret, scopes, auth code, state, username, and password inputs (PR #1210)
 
 ## [8.2.4] - released 2020-12-10
 ### Fixed

composer.json

@@ -7,7 +7,7 @@
         "php": "^7.2 || ^8.0",
         "ext-openssl": "*",
         "league/event": "^2.2",
-        "lcobucci/jwt": "^3.4 || ^4.0",
+        "lcobucci/jwt": "^3.4 || ~4.0.0",
         "psr/http-message": "^1.0.1",
         "defuse/php-encryption": "^2.2.1",
         "ext-json": "*"

src/AuthorizationValidators/BearerTokenValidator.php

@@ -12,12 +12,9 @@
 use DateTimeZone;
 use Lcobucci\Clock\SystemClock;
 use Lcobucci\JWT\Configuration;
-use Lcobucci\JWT\Encoding\CannotDecodeContent;
 use Lcobucci\JWT\Signer\Key\InMemory;
 use Lcobucci\JWT\Signer\Key\LocalFileReference;
 use Lcobucci\JWT\Signer\Rsa\Sha256;
-use Lcobucci\JWT\Token\InvalidTokenStructure;
-use Lcobucci\JWT\Token\UnsupportedHeaderFound;
 use Lcobucci\JWT\Validation\Constraint\SignedWith;
 use Lcobucci\JWT\Validation\Constraint\ValidAt;
 use Lcobucci\JWT\Validation\RequiredConstraintsViolated;
@@ -95,18 +92,18 @@ public function validateAuthorization(ServerRequestInterface $request)
         $jwt = \trim((string) \preg_replace('/^(?:\s+)?Bearer\s/', '', $header[0]));
 
         try {
-            // Attempt to parse and validate the JWT
+            // Attempt to parse the JWT
             $token = $this->jwtConfiguration->parser()->parse($jwt);
+        } catch (\Lcobucci\JWT\Exception $exception) {
+            throw OAuthServerException::accessDenied($exception->getMessage(), null, $exception);
+        }
 
+        try {
+            // Attempt to validate the JWT
             $constraints = $this->jwtConfiguration->validationConstraints();
-
-            try {
-                $this->jwtConfiguration->validator()->assert($token, ...$constraints);
-            } catch (RequiredConstraintsViolated $exception) {
-                throw OAuthServerException::accessDenied('Access token could not be verified');
-            }
-        } catch (CannotDecodeContent | InvalidTokenStructure | UnsupportedHeaderFound $exception) {
-            throw OAuthServerException::accessDenied($exception->getMessage(), null, $exception);
+            $this->jwtConfiguration->validator()->assert($token, ...$constraints);
+        } catch (RequiredConstraintsViolated $exception) {
+            throw OAuthServerException::accessDenied('Access token could not be verified');
         }
 
         $claims = $token->claims();

src/CryptKey.php

@@ -16,9 +16,12 @@
 
 class CryptKey
 {
+    /** @deprecated left for backward compatibility check */
     const RSA_KEY_PATTERN =
         '/^(-----BEGIN (RSA )?(PUBLIC|PRIVATE) KEY-----)\R.*(-----END (RSA )?(PUBLIC|PRIVATE) KEY-----)\R?$/s';
 
+    private const FILE_PREFIX = 'file://';
+
     /**
      * @var string
      */
@@ -36,36 +39,43 @@ class CryptKey
      */
     public function __construct($keyPath, $passPhrase = null, $keyPermissionsCheck = true)
     {
-        if ($rsaMatch = \preg_match(static::RSA_KEY_PATTERN, $keyPath)) {
-            $keyPath = $this->saveKeyToFile($keyPath);
-        } elseif ($rsaMatch === false) {
-            throw new \RuntimeException(
-                \sprintf('PCRE error [%d] encountered during key match attempt', \preg_last_error())
-            );
-        }
+        $this->passPhrase = $passPhrase;
 
-        if (\strpos($keyPath, 'file://') !== 0) {
-            $keyPath = 'file://' . $keyPath;
-        }
+        if (\is_file($keyPath)) {
+            if (\strpos($keyPath, self::FILE_PREFIX) !== 0) {
+                $keyPath = self::FILE_PREFIX . $keyPath;
+            }
 
-        if (!\file_exists($keyPath) || !\is_readable($keyPath)) {
-            throw new LogicException(\sprintf('Key path "%s" does not exist or is not readable', $keyPath));
+            if (!\is_readable($keyPath)) {
+                throw new LogicException(\sprintf('Key path "%s" does not exist or is not readable', $keyPath));
+            }
+            $isFileKey = true;
+            $contents = \file_get_contents($keyPath);
+            $this->keyPath = $keyPath;
+        } else {
+            $isFileKey = false;
+            $contents = $keyPath;
+            $this->keyPath = $this->saveKeyToFile($keyPath);
         }
 
         if ($keyPermissionsCheck === true) {
             // Verify the permissions of the key
-            $keyPathPerms = \decoct(\fileperms($keyPath) & 0777);
+            $keyPathPerms = \decoct(\fileperms($this->keyPath) & 0777);
             if (\in_array($keyPathPerms, ['400', '440', '600', '640', '660'], true) === false) {
-                \trigger_error(\sprintf(
-                    'Key file "%s" permissions are not correct, recommend changing to 600 or 660 instead of %s',
-                    $keyPath,
-                    $keyPathPerms
-                ), E_USER_NOTICE);
+                \trigger_error(
+                    \sprintf(
+                        'Key file "%s" permissions are not correct, recommend changing to 600 or 660 instead of %s',
+                        $this->keyPath,
+                        $keyPathPerms
+                    ),
+                    E_USER_NOTICE
+                );
             }
         }
 
-        $this->keyPath = $keyPath;
-        $this->passPhrase = $passPhrase;
+        if (!$this->isValidKey($contents, $this->passPhrase ?? '')) {
+            throw new LogicException('Unable to read key' . ($isFileKey ? " from file $keyPath" : ''));
+        }
     }
 
     /**
@@ -81,7 +91,7 @@ private function saveKeyToFile($key)
         $keyPath = $tmpDir . '/' . \sha1($key) . '.key';
 
         if (\file_exists($keyPath)) {
-            return 'file://' . $keyPath;
+            return self::FILE_PREFIX . $keyPath;
         }
 
         if (\file_put_contents($keyPath, $key) === false) {
@@ -96,7 +106,30 @@ private function saveKeyToFile($key)
             // @codeCoverageIgnoreEnd
         }
 
-        return 'file://' . $keyPath;
+        return self::FILE_PREFIX . $keyPath;
+    }
+
+    /**
+     * Validate key contents.
+     *
+     * @param string $contents
+     * @param string $passPhrase
+     *
+     * @return bool
+     */
+    private function isValidKey($contents, $passPhrase)
+    {
+        $pkey = \openssl_pkey_get_private($contents, $passPhrase) ?: \openssl_pkey_get_public($contents);
+        if ($pkey === false) {
+            return false;
+        }
+        $details = \openssl_pkey_get_details($pkey);
+
+        return $details !== false && \in_array(
+            $details['type'] ?? -1,
+            [OPENSSL_KEYTYPE_RSA, OPENSSL_KEYTYPE_EC],
+            true
+        );
     }
 
     /**

src/Grant/AbstractGrant.php

@@ -24,6 +24,7 @@
 use League\OAuth2\Server\Entities\ScopeEntityInterface;
 use League\OAuth2\Server\Exception\OAuthServerException;
 use League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException;
+use League\OAuth2\Server\RedirectUriValidators\RedirectUriValidator;
 use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface;
 use League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface;
 use League\OAuth2\Server\Repositories\ClientRepositoryInterface;
@@ -191,6 +192,10 @@ protected function validateClient(ServerRequestInterface $request)
         $redirectUri = $this->getRequestParameter('redirect_uri', $request, null);
 
         if ($redirectUri !== null) {
+            if (!\is_string($redirectUri)) {
+                throw OAuthServerException::invalidRequest('redirect_uri');
+            }
+
             $this->validateRedirectUri($redirectUri, $client, $request);
         }
 
@@ -238,12 +243,16 @@ protected function getClientCredentials(ServerRequestInterface $request)
 
         $clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser);
 
-        if (\is_null($clientId)) {
+        if (!\is_string($clientId)) {
             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];
     }
 
@@ -262,14 +271,8 @@ protected function validateRedirectUri(
         ClientEntityInterface $client,
         ServerRequestInterface $request
     ) {
-        if (\is_string($client->getRedirectUri())
-            && (\strcmp($client->getRedirectUri(), $redirectUri) !== 0)
-        ) {
-            $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
-            throw OAuthServerException::invalidClient($request);
-        } elseif (\is_array($client->getRedirectUri())
-            && \in_array($redirectUri, $client->getRedirectUri(), true) === false
-        ) {
+        $validator = new RedirectUriValidator($client->getRedirectUri());
+        if (!$validator->validateRedirectUri($redirectUri)) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
             throw OAuthServerException::invalidClient($request);
         }
@@ -287,10 +290,16 @@ protected function validateRedirectUri(
      */
     public function validateScopes($scopes, $redirectUri = null)
     {
-        if (!\is_array($scopes)) {
+        if ($scopes === null) {
+            $scopes = [];
+        } elseif (\is_string($scopes)) {
             $scopes = $this->convertScopesQueryStringToArray($scopes);
         }
 
+        if (!\is_array($scopes)) {
+            throw OAuthServerException::invalidRequest('scope');
+        }
+
         $validScopes = [];
 
         foreach ($scopes as $scopeItem) {
@@ -313,7 +322,7 @@ public function validateScopes($scopes, $redirectUri = null)
      *
      * @return array
      */
-    private function convertScopesQueryStringToArray($scopes)
+    private function convertScopesQueryStringToArray(string $scopes)
     {
         return \array_filter(\explode(self::SCOPE_DELIMITER_STRING, \trim($scopes)), function ($scope) {
             return $scope !== '';

src/Grant/AuthCodeGrant.php

@@ -20,7 +20,9 @@
 use League\OAuth2\Server\Exception\OAuthServerException;
 use League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface;
 use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
+use League\OAuth2\Server\RequestAccessTokenEvent;
 use League\OAuth2\Server\RequestEvent;
+use League\OAuth2\Server\RequestRefreshTokenEvent;
 use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
 use League\OAuth2\Server\ResponseTypes\RedirectResponse;
 use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface;
@@ -106,7 +108,7 @@ public function respondToAccessTokenRequest(
 
         $encryptedAuthCode = $this->getRequestParameter('code', $request, null);
 
-        if ($encryptedAuthCode === null) {
+        if (!\is_string($encryptedAuthCode)) {
             throw OAuthServerException::invalidRequest('code');
         }
 
@@ -162,14 +164,14 @@ public function respondToAccessTokenRequest(
 
         // Issue and persist new access token
         $accessToken = $this->issueAccessToken($accessTokenTTL, $client, $authCodePayload->user_id, $scopes);
-        $this->getEmitter()->emit(new RequestEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request));
+        $this->getEmitter()->emit(new RequestAccessTokenEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request, $accessToken));
         $responseType->setAccessToken($accessToken);
 
         // Issue and persist new refresh token if given
         $refreshToken = $this->issueRefreshToken($accessToken);
 
         if ($refreshToken !== null) {
-            $this->getEmitter()->emit(new RequestEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request));
+            $this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken));
             $responseType->setRefreshToken($refreshToken);
         }
 
@@ -260,6 +262,10 @@ public function validateAuthorizationRequest(ServerRequestInterface $request)
         $redirectUri = $this->getQueryStringParameter('redirect_uri', $request);
 
         if ($redirectUri !== null) {
+            if (!\is_string($redirectUri)) {
+                throw OAuthServerException::invalidRequest('redirect_uri');
+            }
+
             $this->validateRedirectUri($redirectUri, $client, $request);
         } elseif (empty($client->getRedirectUri()) ||
             (\is_array($client->getRedirectUri()) && \count($client->getRedirectUri()) !== 1)) {

src/Grant/ClientCredentialsGrant.php

@@ -13,6 +13,7 @@
 
 use DateInterval;
 use League\OAuth2\Server\Exception\OAuthServerException;
+use League\OAuth2\Server\RequestAccessTokenEvent;
 use League\OAuth2\Server\RequestEvent;
 use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface;
 use Psr\Http\Message\ServerRequestInterface;
@@ -52,7 +53,7 @@ public function respondToAccessTokenRequest(
         $accessToken = $this->issueAccessToken($accessTokenTTL, $client, null, $finalizedScopes);
 
         // Send event to emitter
-        $this->getEmitter()->emit(new RequestEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request));
+        $this->getEmitter()->emit(new RequestAccessTokenEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request, $accessToken));
 
         // Inject access token into response type
         $responseType->setAccessToken($accessToken);