Merge branch 'next-7873/update-oauth2-server-dependency' into 'trunk'

NEXT-7873 - bumped league/oauth2-server to major version 8 See merge request shopware/6/product/platform!4379
shopware · Feb 19, 2021 · 656c82d · 656c82d · 7underlines · May 7, 2021
2 parents 90910e4 + b465a32
commit 656c82d
Show file tree

Hide file tree

Showing 14 changed files with 197 additions and 97 deletions.
diff --git a/...og/_unreleased/2021-02-03-bumped-oauth2-server-dependency-to-major-version-8.md b/...og/_unreleased/2021-02-03-bumped-oauth2-server-dependency-to-major-version-8.md
@@ -0,0 +1,24 @@
+---
+title: Bumped oauth2-server dependency to major version 8
+issue: NEXT-7873
+author: Lennart Tinkloh
+author_email: l.tinkloh@shopware.com 
+author_github: lernhart
+---
+# Core
+* Added `src/Core/Framework/Api/OAuth/ClientRepository::validateClient`
+* Added `src/Core/Framework/Api/OAuth/Client/ApiClient::isConfidential`
+* Added `src/Core/Checkout/Payment/Cart/Token/JWTConfigurationFactory`, which creates an injectable JWT configuration object
+* Added parameter `configuration` to `src/Core/Framework/Api/OAuth/BearerTokenValidator` constructor.
+
+___
+# Upgrade Information
+The parameter signature of `src/Core/Framework/Api/OAuth/ClientRepository::getClientEntity` changed due to the major update of the oauth2-server dependency.
+OAuth2-Clients should be validated separately in the new `validateClient` method.
+See: https://github.com/thephpleague/oauth2-server/pull/938
+
+The parameter signature of `src/Core/Checkout/Payment/Cart/Token/JWTFactoryV2` changed.
+It uses the injected configuration object rather than a private key.
+
+The parameter signature of `src/Core/Framework/Api/OAuth/BearerTokenValidator` changed.
+The injected configuration object was added as parameter.
diff --git a/composer.json b/composer.json
@@ -58,10 +58,10 @@
         "google/cloud-storage": "1.17.0",
         "guzzlehttp/guzzle": "~7.2",
         "jdorn/sql-formatter": "1.2.17",
-        "lcobucci/jwt": "3.3.1",
+        "lcobucci/jwt": "4.1.1",
         "league/flysystem": "1.0.63",
         "league/flysystem-aws-s3-v3": "1.0.23",
-        "league/oauth2-server": "7.3.3",
+        "league/oauth2-server": "8.2.4",
         "marc1706/fast-image-size": "1.1.6",
         "nyholm/psr7": "1.2.1",
         "ongr/elasticsearch-dsl": "7.1.3",

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -640,16 +640,6 @@ parameters:
 			count: 1
 			path: src/Core/Checkout/Payment/Cart/PaymentHandler/PaymentHandlerRegistry.php
 
-		-
-			message: "#^Parameter \\#1 \\$keyPath of class League\\\\OAuth2\\\\Server\\\\CryptKey constructor expects string, Lcobucci\\\\JWT\\\\Signer\\\\Key\\|string given\\.$#"
-			count: 1
-			path: src/Core/Checkout/Payment/Cart/Token/JWTFactoryV2.php
-
-		-
-			message: "#^Parameter \\#1 \\$subject of method Lcobucci\\\\JWT\\\\Builder\\:\\:setSubject\\(\\) expects string, string\\|null given\\.$#"
-			count: 1
-			path: src/Core/Checkout/Payment/Cart/Token/JWTFactoryV2.php
-
 		-
 			message: "#^Parameter \\#1 \\$paymentMethodId of method Shopware\\\\Core\\\\Checkout\\\\Payment\\\\PaymentService\\:\\:getPaymentHandlerById\\(\\) expects string, string\\|null given\\.$#"
 			count: 1
@@ -2590,11 +2580,6 @@ parameters:
 			count: 1
 			path: src/Core/Framework/Api/OAuth/AccessToken.php
 
-		-
-			message: "#^Parameter \\#2 \\$clientSecret of method Shopware\\\\Core\\\\Framework\\\\Api\\\\OAuth\\\\ClientRepository\\:\\:getByAccessKey\\(\\) expects string, string\\|null given\\.$#"
-			count: 1
-			path: src/Core/Framework/Api/OAuth/ClientRepository.php
-
 		-
 			message: "#^Parameter \\#1 \\$uuid of static method Shopware\\\\Core\\\\Framework\\\\Uuid\\\\Uuid\\:\\:fromHexToBytes\\(\\) expects string, int\\|string\\|null given\\.$#"
 			count: 1

diff --git a/src/Core/Checkout/DependencyInjection/payment.xml b/src/Core/Checkout/DependencyInjection/payment.xml
@@ -42,8 +42,18 @@
             <argument type="service" id="order_customer.repository"/>
         </service>
 
-        <service id="Shopware\Core\Checkout\Payment\Cart\Token\JWTFactoryV2">
+        <service id="shopware.jwt_signer" class="Lcobucci\JWT\Signer\Rsa\Sha256"/>
+
+        <service id="shopware.jwt_config" class="Lcobucci\JWT\Configuration">
+            <factory class="Shopware\Core\Checkout\Payment\Cart\Token\JWTConfigurationFactory"
+                     method="createJWTConfiguration"/>
+            <argument type="service" id="shopware.jwt_signer"/>
             <argument type="service" id="shopware.private_key"/>
+            <argument type="service" id="shopware.public_key"/>
+        </service>
+
+        <service id="Shopware\Core\Checkout\Payment\Cart\Token\JWTFactoryV2">
+            <argument type="service" id="shopware.jwt_config"/>
         </service>
 
         <service id="Shopware\Core\Checkout\Payment\Cart\PaymentHandler\PaymentHandlerRegistry">

diff --git a/src/Core/Checkout/Payment/Cart/Token/JWTConfigurationFactory.php b/src/Core/Checkout/Payment/Cart/Token/JWTConfigurationFactory.php
@@ -0,0 +1,39 @@
+<?php declare(strict_types=1);
+
+namespace Shopware\Core\Checkout\Payment\Cart\Token;
+
+use Lcobucci\JWT\Configuration;
+use Lcobucci\JWT\Decoder;
+use Lcobucci\JWT\Encoder;
+use Lcobucci\JWT\Signer;
+use Lcobucci\JWT\Signer\Key\InMemory;
+use Lcobucci\JWT\Validation\Constraint\SignedWith;
+use League\OAuth2\Server\CryptKey;
+
+class JWTConfigurationFactory
+{
+    public static function createJWTConfiguration(
+        Signer $signer,
+        CryptKey $privateKey,
+        CryptKey $publicKey,
+        ?Encoder $encoder = null,
+        ?Decoder $decoder = null
+    ): Configuration {
+        $privateKey = InMemory::file($privateKey->getKeyPath(), $privateKey->getPassPhrase() ?? '');
+        $publicKey = InMemory::file($publicKey->getKeyPath(), $publicKey->getPassPhrase() ?? '');
+
+        $configuration = Configuration::forAsymmetricSigner(
+            $signer,
+            $privateKey,
+            $publicKey,
+            $encoder,
+            $decoder
+        );
+
+        // add basic constraint for token signature validation
+        $constraint = new SignedWith($signer, $publicKey);
+        $configuration->setValidationConstraints($constraint);
+
+        return $configuration;
+    }
+}
diff --git a/src/Core/Checkout/Payment/Cart/Token/JWTFactoryV2.php b/src/Core/Checkout/Payment/Cart/Token/JWTFactoryV2.php
@@ -2,48 +2,41 @@
 
 namespace Shopware\Core\Checkout\Payment\Cart\Token;
 
-use Lcobucci\JWT\Builder;
-use Lcobucci\JWT\Parser;
-use Lcobucci\JWT\Signer\Hmac\Sha256;
-use Lcobucci\JWT\Signer\Key;
-use League\OAuth2\Server\CryptKey;
+use Lcobucci\JWT\Configuration;
+use Lcobucci\JWT\UnencryptedToken;
 use Shopware\Core\Checkout\Payment\Exception\InvalidTokenException;
 use Shopware\Core\Framework\Uuid\Uuid;
 
 class JWTFactoryV2 implements TokenFactoryInterfaceV2
 {
     /**
-     * @var CryptKey
+     * @var Configuration
      */
-    protected $privateKey;
+    protected $configuration;
 
-    /**
-     * @param Key|CryptKey|string $privateKey
-     */
-    public function __construct($privateKey)
+    public function __construct(Configuration $configuration)
     {
-        if (!$privateKey instanceof CryptKey) {
-            $privateKey = new CryptKey($privateKey);
-        }
-
-        $this->privateKey = $privateKey;
+        $this->configuration = $configuration;
     }
 
     public function generateToken(TokenStruct $tokenStruct): string
     {
-        $jwtToken = (new Builder())
-            ->setId(Uuid::randomHex(), true)
-            ->setIssuedAt(time())
-            ->setNotBefore(time())
-            ->setExpiration(time() + $tokenStruct->getExpires())
-            ->setSubject($tokenStruct->getTransactionId())
-            ->set('pmi', $tokenStruct->getPaymentMethodId())
-            ->set('ful', $tokenStruct->getFinishUrl())
-            ->set('eul', $tokenStruct->getErrorUrl())
-            ->sign(new Sha256(), new Key($this->privateKey->getKeyPath(), $this->privateKey->getPassPhrase()))
-            ->getToken();
+        $expires = (new \DateTimeImmutable('@' . time()))->modify(
+            \sprintf('+%d seconds', $tokenStruct->getExpires())
+        );
 
-        return (string) $jwtToken;
+        $jwtToken = $this->configuration->builder()
+            ->identifiedBy(Uuid::randomHex())
+            ->issuedAt(new \DateTimeImmutable('@' . time()))
+            ->canOnlyBeUsedAfter(new \DateTimeImmutable('@' . time()))
+            ->expiresAt($expires)
+            ->relatedTo($tokenStruct->getTransactionId() ?? '')
+            ->withClaim('pmi', $tokenStruct->getPaymentMethodId())
+            ->withClaim('ful', $tokenStruct->getFinishUrl())
+            ->withClaim('eul', $tokenStruct->getErrorUrl())
+            ->getToken($this->configuration->signer(), $this->configuration->signingKey());
+
+        return $jwtToken->toString();
     }
 
     /**
@@ -52,30 +45,30 @@ public function generateToken(TokenStruct $tokenStruct): string
     public function parseToken(string $token): TokenStruct
     {
         try {
-            $jwtToken = (new Parser())->parse($token);
-        } catch (\InvalidArgumentException $e) {
+            /** @var UnencryptedToken $jwtToken */
+            $jwtToken = $this->configuration->parser()->parse($token);
+        } catch (\Throwable $e) {
             throw new InvalidTokenException($token);
         }
 
-        if (!$jwtToken->verify(new Sha256(), $this->privateKey->getKeyPath())) {
+        if (!$this->configuration->validator()->validate($jwtToken, ...$this->configuration->validationConstraints())) {
             throw new InvalidTokenException($token);
         }
-        $errorUrl = null;
-        if ($jwtToken->hasClaim('eul')) {
-            $errorUrl = $jwtToken->getClaim('eul');
-        }
 
-        $tokenStruct = new TokenStruct(
-            $jwtToken->getClaim('jti'),
+        $errorUrl = $jwtToken->claims()->get('eul');
+
+        /** @var \DateTimeImmutable $expires */
+        $expires = $jwtToken->claims()->get('exp');
+
+        return new TokenStruct(
+            $jwtToken->claims()->get('jti'),
             $token,
-            $jwtToken->getClaim('pmi'),
-            $jwtToken->getClaim('sub'),
-            $jwtToken->getClaim('ful'),
-            $jwtToken->getClaim('exp'),
+            $jwtToken->claims()->get('pmi'),
+            $jwtToken->claims()->get('sub'),
+            $jwtToken->claims()->get('ful'),
+            $expires->getTimestamp(),
             $errorUrl
         );
-
-        return $tokenStruct;
     }
 
     public function invalidateToken(string $token): bool

diff --git a/src/Core/Framework/Api/OAuth/AccessTokenRepository.php b/src/Core/Framework/Api/OAuth/AccessTokenRepository.php
@@ -14,7 +14,7 @@ class AccessTokenRepository implements AccessTokenRepositoryInterface
      */
     public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null): AccessTokenEntityInterface
     {
-        $token = new AccessToken($clientEntity, [], $userIdentifier);
+        $token = new AccessToken($clientEntity, $scopes, $userIdentifier);
 
         if ($clientEntity instanceof ApiClient && $clientEntity->getIdentifier() === 'administration') {
             $token->setIdentifier('administration');

diff --git a/src/Core/Framework/Api/OAuth/BearerTokenValidator.php b/src/Core/Framework/Api/OAuth/BearerTokenValidator.php
@@ -3,7 +3,8 @@
 namespace Shopware\Core\Framework\Api\OAuth;
 
 use Doctrine\DBAL\Connection;
-use Lcobucci\JWT\Parser;
+use Lcobucci\JWT\Configuration;
+use Lcobucci\JWT\UnencryptedToken;
 use League\OAuth2\Server\AuthorizationValidators\AuthorizationValidatorInterface;
 use League\OAuth2\Server\Exception\OAuthServerException;
 use Psr\Http\Message\ServerRequestInterface;
@@ -22,15 +23,22 @@ class BearerTokenValidator implements AuthorizationValidatorInterface
      */
     private $decorated;
 
+    /**
+     * @var Configuration
+     */
+    private $configuration;
+
     public function __construct(
         AuthorizationValidatorInterface $decorated,
-        Connection $connection
+        Connection $connection,
+        Configuration $configuration
     ) {
         $this->decorated = $decorated;
         $this->connection = $connection;
+        $this->configuration = $configuration;
     }
 
-    /**
+    /**A
      * {@inheritdoc}
      */
     public function validateAuthorization(ServerRequestInterface $request)
@@ -41,10 +49,11 @@ public function validateAuthorization(ServerRequestInterface $request)
 
         $jwt = trim(preg_replace('/^(?:\s+)?Bearer\s/', '', $header[0]) ?? '');
 
-        $token = (new Parser())->parse($jwt);
+        /** @var UnencryptedToken $token */
+        $token = $this->configuration->parser()->parse($jwt);
 
         if ($userId = $request->getAttribute(PlatformRequest::ATTRIBUTE_OAUTH_USER_ID)) {
-            $this->validateAccessTokenIssuedAt($token->getClaim('iat', 0), $userId);
+            $this->validateAccessTokenIssuedAt($token->claims()->get('iat', 0), $userId);
         }
 
         return $request;
@@ -54,7 +63,7 @@ public function validateAuthorization(ServerRequestInterface $request)
      * @throws OAuthServerException
      * @throws \Doctrine\DBAL\DBALException
      */
-    private function validateAccessTokenIssuedAt(int $tokenIssuedAt, string $userId): void
+    private function validateAccessTokenIssuedAt(\DateTimeImmutable $tokenIssuedAt, string $userId): void
     {
         $lastUpdatedPasswordAt = $this->connection->createQueryBuilder()
             ->select(['last_updated_password_at'])
@@ -74,7 +83,7 @@ private function validateAccessTokenIssuedAt(int $tokenIssuedAt, string $userId)
 
         $lastUpdatedPasswordAt = strtotime($lastUpdatedPasswordAt);
 
-        if ($tokenIssuedAt <= $lastUpdatedPasswordAt) {
+        if ($tokenIssuedAt->getTimestamp() <= $lastUpdatedPasswordAt) {
             throw OAuthServerException::accessDenied('Access token is expired');
         }
     }

diff --git a/src/Core/Framework/Api/OAuth/Client/ApiClient.php b/src/Core/Framework/Api/OAuth/Client/ApiClient.php
@@ -35,4 +35,9 @@ public function getIdentifier(): string
     {
         return $this->identifier;
     }
+
+    public function isConfidential(): bool
+    {
+        return true;
+    }
 }
diff --git a/src/Core/Framework/Api/OAuth/ClientRepository.php b/src/Core/Framework/Api/OAuth/ClientRepository.php
@@ -9,6 +9,7 @@
 use Shopware\Core\Framework\Api\OAuth\Client\ApiClient;
 use Shopware\Core\Framework\Api\Util\AccessKeyHelper;
 use Shopware\Core\Framework\Uuid\Uuid;
+use Symfony\Component\HttpFoundation\RequestStack;
 
 class ClientRepository implements ClientRepositoryInterface
 {
@@ -17,21 +18,38 @@ class ClientRepository implements ClientRepositoryInterface
      */
     private $connection;
 
-    public function __construct(Connection $connection)
+    /**
+     * @var RequestStack
+     */
+    private $requestStack;
+
+    public function __construct(Connection $connection, RequestStack $requestStack)
     {
         $this->connection = $connection;
+        $this->requestStack = $requestStack;
     }
 
     /**
-     * Get a client.
-     *
-     * @param string      $clientIdentifier   The client's identifier
-     * @param string|null $grantType          The grant type used (if sent)
-     * @param string|null $clientSecret       The client's secret (if sent)
-     * @param bool        $mustValidateSecret If true the client must attempt to validate the secret if the client                                        is confidential
+     * do not validate oauth client here, as this would mean two requests:
+     * 1. to validate the client
+     * 2. to actually fetch the client
+     * instead, the client_credentials grant will throw en exception if the client secret is invalid on its own
      */
-    public function getClientEntity($clientIdentifier, $grantType = null, $clientSecret = null, $mustValidateSecret = true): ?ClientEntityInterface
+    public function validateClient($clientIdentifier, $clientSecret, $grantType): bool
+    {
+        return true;
+    }
+
+    public function getClientEntity($clientIdentifier): ?ClientEntityInterface
     {
+        $request = $this->requestStack->getMasterRequest();
+
+        if (!$request) {
+            return null;
+        }
+
+        $grantType = $request->request->get('grant_type');
+
         if ($grantType === 'password' && $clientIdentifier === 'administration') {
             return new ApiClient('administration', true);
         }
@@ -41,6 +59,8 @@ public function getClientEntity($clientIdentifier, $grantType = null, $clientSec
         }
 
         if ($grantType === 'client_credentials') {
+            $clientSecret = $request->request->get('client_secret');
+
             return $this->getByAccessKey($clientIdentifier, $clientSecret);
         }