Skip to content

Commit

Permalink
Merge branch 'next-7873/update-oauth2-server-dependency' into 'trunk'
Browse files Browse the repository at this point in the history
NEXT-7873 - bumped league/oauth2-server to major version 8

See merge request shopware/6/product/platform!4379
  • Loading branch information
ssltg committed Feb 19, 2021
2 parents 90910e4 + b465a32 commit 656c82d
Show file tree
Hide file tree
Showing 14 changed files with 197 additions and 97 deletions.
@@ -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.
4 changes: 2 additions & 2 deletions composer.json
Expand Up @@ -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",
Expand Down
15 changes: 0 additions & 15 deletions phpstan-baseline.neon
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion src/Core/Checkout/DependencyInjection/payment.xml
Expand Up @@ -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">
Expand Down
39 changes: 39 additions & 0 deletions 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;
}
}
79 changes: 36 additions & 43 deletions src/Core/Checkout/Payment/Cart/Token/JWTFactoryV2.php
Expand Up @@ -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();
}

/**
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Core/Framework/Api/OAuth/AccessTokenRepository.php
Expand Up @@ -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');
Expand Down
23 changes: 16 additions & 7 deletions src/Core/Framework/Api/OAuth/BearerTokenValidator.php
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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;
Expand All @@ -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'])
Expand All @@ -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');
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/Core/Framework/Api/OAuth/Client/ApiClient.php
Expand Up @@ -35,4 +35,9 @@ public function getIdentifier(): string
{
return $this->identifier;
}

public function isConfidential(): bool
{
return true;
}
}
36 changes: 28 additions & 8 deletions src/Core/Framework/Api/OAuth/ClientRepository.php
Expand Up @@ -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
{
Expand All @@ -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);
}
Expand All @@ -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);
}

Expand Down

5 comments on commit 656c82d

@thomaspeissl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lernhart
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @treffner.
Authentication should work as it worked before.
The breaks come from the updated dependencies (league/oauth2-server, lcobucci/jwt) and are only for developers, extending/modifying the Shopware OAuth workflow.

@thomaspeissl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lernhart Ah thanks for the fast response.
I was worried because of the wording in the changelog "This comes with a break in our current oauth2 core implementation.".

@lernhart
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@treffner
Maybe this is a bit misleading, but the upgrade.md should only have changes regarding the plugin developer in general.

@thomaspeissl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lernhart
Ah ok, well I can't login with client credentials anymore. But I'm not sure if this commit introduced the regression. I opened an issue #1846

Please sign in to comment.