kenaug74/Sylius
1
0
Fork 0
mirror of https://github.com/Sylius/Sylius.git synced 2026-07-03 03:37:12 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Check payment request ownership on POST

Browse source
This commit is contained in:
TheMilek 2026-06-01 14:02:20 +02:00
parent 19dbca7440
commit 0af90b6e96
No known key found for this signature in database
GPG key ID: A3F43F426B5286AA
5 changed files with 297 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
UPGRADE-API-2.1.md
View file

@ -4,12 +4,12 @@
### Payment request ownership enforcement
`GET /api/v2/shop/payment-requests/{hash}` and `PUT /api/v2/shop/payment-requests/{hash}` now enforce ownership:
`GET /api/v2/shop/payment-requests/{hash}`, `PUT /api/v2/shop/payment-requests/{hash}` and `POST /api/v2/shop/orders/{tokenValue}/payment-requests` now enforce ownership:
- An unauthenticated request receives `404 Not Found`, unless the underlying order is a guest order. An order qualifies as a guest order when it has no customer, when its customer has no associated user account, or when it was placed through the guest checkout flow (`Order::isCreatedByGuest()` returns `true`).
- An authenticated shop user can only access payment requests belonging to their own orders; requests for another customer's payment request also receive `404 Not Found`.
Previously both operations could be performed by any authenticated user or even an anonymous user who knew the payment request hash, which constituted a broken object-level authorization (IDOR) vulnerability.
Previously these operations could be performed by any authenticated user or even an anonymous user who knew the payment request hash (or, for the creation endpoint, the order token value), which constituted a broken object-level authorization (IDOR) vulnerability.
**No action is required** unless your integration assumed that these endpoints were publicly accessible or shared across customers.
@ -17,6 +17,10 @@ Previously both operations could be performed by any authenticated user or even
A new `UserContextInterface` argument is appended to the constructor. Omitting it is deprecated and will be required in Sylius 3.0; without it the ownership check is skipped and the provider falls back to its previous behavior.
### `Sylius\Bundle\ApiBundle\CommandHandler\Payment\AddPaymentRequestHandler` constructor
A new, nullable `UserContextInterface` argument is appended to the constructor. When it is not provided, the ownership check is skipped and the handler falls back to its previous behavior.
# UPGRADE FROM `2.1.10` TO `2.1.11`
### Constructor signature changes

35
src/Sylius/Bundle/ApiBundle/CommandHandler/Payment/AddPaymentRequestHandler.php
View file

@ -14,18 +14,23 @@ declare(strict_types=1);
namespace Sylius\Bundle\ApiBundle\CommandHandler\Payment;
use Sylius\Bundle\ApiBundle\Command\Payment\AddPaymentRequest;
use Sylius\Bundle\ApiBundle\Context\UserContextInterface;
use Sylius\Bundle\ApiBundle\Exception\PaymentMethodNotFoundException;
use Sylius\Bundle\ApiBundle\Exception\PaymentNotFoundException;
use Sylius\Bundle\PaymentBundle\Provider\DefaultActionProviderInterface;
use Sylius\Bundle\PaymentBundle\Provider\DefaultPayloadProviderInterface;
use Sylius\Component\Core\Model\CustomerInterface;
use Sylius\Component\Core\Model\OrderInterface;
use Sylius\Component\Core\Model\PaymentInterface;
use Sylius\Component\Core\Model\PaymentMethodInterface;
use Sylius\Component\Core\Model\ShopUserInterface;
use Sylius\Component\Core\Repository\PaymentMethodRepositoryInterface;
use Sylius\Component\Core\Repository\PaymentRepositoryInterface;
use Sylius\Component\Payment\Factory\PaymentRequestFactoryInterface;
use Sylius\Component\Payment\Model\PaymentRequestInterface;
use Sylius\Component\Payment\Repository\PaymentRequestRepositoryInterface;
use Symfony\Component\Messenger\Attribute\AsMessageHandler;
use Symfony\Component\Security\Core\User\UserInterface;
/** @experimental */
#[AsMessageHandler]
@ -44,6 +49,7 @@ final class AddPaymentRequestHandler
private PaymentRequestRepositoryInterface $paymentRequestRepository,
private DefaultActionProviderInterface $defaultActionProvider,
private DefaultPayloadProviderInterface $defaultPayloadProvider,
private ?UserContextInterface $userContext = null,
) {
}
@ -64,6 +70,10 @@ final class AddPaymentRequestHandler
throw new PaymentNotFoundException();
}
if ($this->userContext !== null && !$this->isAccessibleByCurrentUser($payment, $this->userContext->getUser())) {
throw new PaymentNotFoundException();
}
/** @var PaymentMethodInterface|null $paymentMethod */
$paymentMethod = $this->paymentMethodRepository->findOneBy(['code' => $addPaymentRequest->paymentMethodCode]);
if (null === $paymentMethod) {
@ -76,4 +86,29 @@ final class AddPaymentRequestHandler
return $paymentRequest;
}
private function isAccessibleByCurrentUser(PaymentInterface $payment, ?UserInterface $user): bool
{
$order = $payment->getOrder();
if (!$order instanceof OrderInterface) {
return false;
}
if ($user instanceof ShopUserInterface) {
$customer = $user->getCustomer();
return $customer instanceof CustomerInterface && $order->getCustomer() === $customer;
}
return $this->isGuestOrder($order);
}
private function isGuestOrder(OrderInterface $order): bool
{
$customer = $order->getCustomer();
return null === $customer ||
null === $customer->getUser() ||
$order->isCreatedByGuest();
}
}

1
src/Sylius/Bundle/ApiBundle/Resources/config/services/command_handlers.xml
View file

@ -218,6 +218,7 @@
<argument type="service" id="sylius.repository.payment_request" />
<argument type="service" id="sylius.provider.payment_request.default_action" />
<argument type="service" id="sylius.provider.payment_request.default_payload" />
<argument type="service" id="sylius_api.context.user.token_based" />
<tag name="messenger.message_handler" bus="sylius.command_bus" />
</service>

93
src/Sylius/Bundle/ApiBundle/tests/CommandHandler/AddPaymentRequestHandlerTest.php
View file

@ -19,12 +19,16 @@ use Prophecy\PhpUnit\ProphecyTrait;
use Prophecy\Prophecy\ObjectProphecy;
use Sylius\Bundle\ApiBundle\Command\Payment\AddPaymentRequest;
use Sylius\Bundle\ApiBundle\CommandHandler\Payment\AddPaymentRequestHandler;
use Sylius\Bundle\ApiBundle\Context\UserContextInterface;
use Sylius\Bundle\ApiBundle\Exception\PaymentMethodNotFoundException;
use Sylius\Bundle\ApiBundle\Exception\PaymentNotFoundException;
use Sylius\Bundle\PaymentBundle\Provider\DefaultActionProviderInterface;
use Sylius\Bundle\PaymentBundle\Provider\DefaultPayloadProviderInterface;
use Sylius\Component\Core\Model\CustomerInterface;
use Sylius\Component\Core\Model\OrderInterface;
use Sylius\Component\Core\Model\PaymentInterface;
use Sylius\Component\Core\Model\PaymentMethodInterface;
use Sylius\Component\Core\Model\ShopUserInterface;
use Sylius\Component\Core\Repository\PaymentMethodRepositoryInterface;
use Sylius\Component\Core\Repository\PaymentRepositoryInterface;
use Sylius\Component\Payment\Factory\PaymentRequestFactoryInterface;
@ -47,6 +51,8 @@ final class AddPaymentRequestHandlerTest extends TestCase
private DefaultPayloadProviderInterface|ObjectProphecy $defaultPayloadProvider;
private ObjectProphecy|UserContextInterface $userContext;
private AddPaymentRequestHandler $addPaymentRequestHandler;
protected function setUp(): void
@ -57,6 +63,7 @@ final class AddPaymentRequestHandlerTest extends TestCase
$this->paymentRequestRepository = $this->prophesize(PaymentRequestRepositoryInterface::class);
$this->defaultActionProvider = $this->prophesize(DefaultActionProviderInterface::class);
$this->defaultPayloadProvider = $this->prophesize(DefaultPayloadProviderInterface::class);
$this->userContext = $this->prophesize(UserContextInterface::class);
$this->addPaymentRequestHandler = new AddPaymentRequestHandler(
$this->paymentMethodRepository->reveal(),
@ -65,6 +72,7 @@ final class AddPaymentRequestHandlerTest extends TestCase
$this->paymentRequestRepository->reveal(),
$this->defaultActionProvider->reveal(),
$this->defaultPayloadProvider->reveal(),
$this->userContext->reveal(),
);
}
@ -78,27 +86,110 @@ final class AddPaymentRequestHandlerTest extends TestCase
$this->addPaymentRequestHandler->__invoke(new AddPaymentRequest('token', 1, 'bank_transfer'));
}
#[Test]
public function it_throws_an_exception_if_the_order_is_not_owned_by_the_logged_in_shop_user(): void
{
self::expectException(PaymentNotFoundException::class);
$payment = $this->prophesize(PaymentInterface::class);
$order = $this->prophesize(OrderInterface::class);
$shopUser = $this->prophesize(ShopUserInterface::class);
$orderCustomer = $this->prophesize(CustomerInterface::class)->reveal();
$userCustomer = $this->prophesize(CustomerInterface::class)->reveal();
$this->paymentRepository->findOneByOrderToken(1, 'token')->willReturn($payment->reveal());
$payment->getOrder()->willReturn($order->reveal());
$order->getCustomer()->willReturn($orderCustomer);
$this->userContext->getUser()->willReturn($shopUser->reveal());
$shopUser->getCustomer()->willReturn($userCustomer);
$this->paymentMethodRepository->findOneBy(['code' => 'bank_transfer'])->shouldNotBeCalled();
$this->addPaymentRequestHandler->__invoke(new AddPaymentRequest('token', 1, 'bank_transfer'));
}
#[Test]
public function it_throws_an_exception_if_an_anonymous_user_creates_a_payment_request_for_a_customer_order(): void
{
self::expectException(PaymentNotFoundException::class);
$payment = $this->prophesize(PaymentInterface::class);
$order = $this->prophesize(OrderInterface::class);
$customer = $this->prophesize(CustomerInterface::class);
$shopUser = $this->prophesize(ShopUserInterface::class);
$this->paymentRepository->findOneByOrderToken(1, 'token')->willReturn($payment->reveal());
$payment->getOrder()->willReturn($order->reveal());
$order->getCustomer()->willReturn($customer->reveal());
$customer->getUser()->willReturn($shopUser->reveal());
$order->isCreatedByGuest()->willReturn(false);
$this->userContext->getUser()->willReturn(null);
$this->paymentMethodRepository->findOneBy(['code' => 'bank_transfer'])->shouldNotBeCalled();
$this->addPaymentRequestHandler->__invoke(new AddPaymentRequest('token', 1, 'bank_transfer'));
}
#[Test]
public function it_throws_an_exception_if_there_is_no_payment_method_for_given_code(): void
{
self::expectException(PaymentMethodNotFoundException::class);
$payment = $this->prophesize(PaymentInterface::class);
$order = $this->prophesize(OrderInterface::class);
$this->paymentRepository->findOneByOrderToken(1, 'token')->willReturn($payment->reveal());
$payment->getOrder()->willReturn($order->reveal());
$order->getCustomer()->willReturn(null);
$this->userContext->getUser()->willReturn(null);
$this->paymentMethodRepository->findOneBy(['code' => 'bank_transfer'])->willReturn(null);
$this->addPaymentRequestHandler->__invoke(new AddPaymentRequest('token', 1, 'bank_transfer'));
}
#[Test]
public function it_creates_a_payment_request(): void
public function it_creates_a_payment_request_for_a_guest_order_requested_by_an_anonymous_user(): void
{
$payment = $this->prophesize(PaymentInterface::class);
$order = $this->prophesize(OrderInterface::class);
$paymentMethod = $this->prophesize(PaymentMethodInterface::class);
$paymentRequest = $this->prophesize(PaymentRequestInterface::class);
$this->paymentRepository->findOneByOrderToken(1, 'token')->willReturn($payment->reveal());
$payment->getOrder()->willReturn($order->reveal());
$order->getCustomer()->willReturn(null);
$this->userContext->getUser()->willReturn(null);
$this->paymentMethodRepository->findOneBy(['code' => 'bank_transfer'])->willReturn($paymentMethod->reveal());
$this->defaultActionProvider->getAction($paymentRequest)->willReturn('authorize');
$this->defaultPayloadProvider->getPayload($paymentRequest)->willReturn(['foo' => 'bar']);
$this->paymentRequestFactory->create($payment->reveal(), $paymentMethod->reveal())->willReturn($paymentRequest->reveal());
$paymentRequest->setAction('authorize')->shouldBeCalled();
$paymentRequest->setPayload(['foo' => 'bar'])->shouldBeCalled();
self::assertSame(
$paymentRequest->reveal(),
$this->addPaymentRequestHandler->__invoke(
new AddPaymentRequest('token', 1, 'bank_transfer'),
),
);
}
#[Test]
public function it_creates_a_payment_request_for_an_order_owned_by_the_logged_in_shop_user(): void
{
$payment = $this->prophesize(PaymentInterface::class);
$order = $this->prophesize(OrderInterface::class);
$shopUser = $this->prophesize(ShopUserInterface::class);
$customer = $this->prophesize(CustomerInterface::class)->reveal();
$paymentMethod = $this->prophesize(PaymentMethodInterface::class);
$paymentRequest = $this->prophesize(PaymentRequestInterface::class);
$this->paymentRepository->findOneByOrderToken(1, 'token')->willReturn($payment->reveal());
$payment->getOrder()->willReturn($order->reveal());
$order->getCustomer()->willReturn($customer);
$this->userContext->getUser()->willReturn($shopUser->reveal());
$shopUser->getCustomer()->willReturn($customer);
$this->paymentMethodRepository->findOneBy(['code' => 'bank_transfer'])->willReturn($paymentMethod->reveal());
$this->defaultActionProvider->getAction($paymentRequest)->willReturn('authorize');
$this->defaultPayloadProvider->getPayload($paymentRequest)->willReturn(['foo' => 'bar']);

163
tests/Api/Shop/PaymentRequestsTest.php
View file

@ -15,6 +15,7 @@ namespace Sylius\Tests\Api\Shop;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Test;
use Sylius\Component\Core\Model\PaymentInterface;
use Sylius\Component\Payment\Model\PaymentRequestInterface;
use Sylius\Tests\Api\JsonApiTestCase;
use Sylius\Tests\Api\Utils\OrderPlacerTrait;
@ -487,6 +488,168 @@ final class PaymentRequestsTest extends JsonApiTestCase
$this->assertResponseCode($this->client->getResponse(), Response::HTTP_OK);
}
#[Test]
public function it_does_not_allow_an_anonymous_customer_to_create_a_payment_request_for_an_order_owned_by_a_customer(): void
{
$payment = $this->loadOrderOwnedByOliver();
$this->client->request(
method: 'POST',
uri: sprintf('/api/v2/shop/orders/%s/payment-requests', $payment->getOrder()->getTokenValue()),
server: $this->headerBuilder()->withJsonLdAccept()->withJsonLdContentType()->build(),
content: json_encode([
'paymentId' => $payment->getId(),
'paymentMethodCode' => $payment->getMethod()->getCode(),
'payload' => [
'target_path' => 'https://attacker.tld/target-path',
'after_path' => 'https://attacker.tld/after-path',
],
], \JSON_THROW_ON_ERROR),
);
$this->assertResponseCode($this->client->getResponse(), Response::HTTP_NOT_FOUND);
}
#[Test]
public function it_does_not_allow_a_customer_to_create_a_payment_request_for_an_order_owned_by_another_customer(): void
{
$payment = $this->loadOrderOwnedByOliver();
$this->client->request(
method: 'POST',
uri: sprintf('/api/v2/shop/orders/%s/payment-requests', $payment->getOrder()->getTokenValue()),
server: $this->headerBuilder()->withJsonLdAccept()->withJsonLdContentType()->withShopUserAuthorization('dave@doe.com')->build(),
content: json_encode([
'paymentId' => $payment->getId(),
'paymentMethodCode' => $payment->getMethod()->getCode(),
'payload' => [
'target_path' => 'https://attacker.tld/target-path',
'after_path' => 'https://attacker.tld/after-path',
],
], \JSON_THROW_ON_ERROR),
);
$this->assertResponseCode($this->client->getResponse(), Response::HTTP_NOT_FOUND);
}
#[Test]
public function it_does_not_allow_a_shop_user_to_create_a_payment_request_for_a_guest_order(): void
{
$payment = $this->loadGuestOrder();
$this->client->request(
method: 'POST',
uri: sprintf('/api/v2/shop/orders/%s/payment-requests', $payment->getOrder()->getTokenValue()),
server: $this->headerBuilder()->withJsonLdAccept()->withJsonLdContentType()->withShopUserAuthorization('oliver@doe.com')->build(),
content: json_encode([
'paymentId' => $payment->getId(),
'paymentMethodCode' => $payment->getMethod()->getCode(),
'payload' => [
'target_path' => 'https://myshop.tld/target-path',
'after_path' => 'https://myshop.tld/after-path',
],
], \JSON_THROW_ON_ERROR),
);
$this->assertResponseCode($this->client->getResponse(), Response::HTTP_NOT_FOUND);
}
#[Test]
public function it_allows_a_customer_to_create_a_payment_request_for_their_own_order(): void
{
$payment = $this->loadOrderOwnedByOliver();
$this->client->request(
method: 'POST',
uri: sprintf('/api/v2/shop/orders/%s/payment-requests', $payment->getOrder()->getTokenValue()),
server: $this->headerBuilder()->withJsonLdAccept()->withJsonLdContentType()->withShopUserAuthorization('oliver@doe.com')->build(),
content: json_encode([
'paymentId' => $payment->getId(),
'paymentMethodCode' => $payment->getMethod()->getCode(),
'payload' => [
'target_path' => 'https://myshop.tld/target-path',
'after_path' => 'https://myshop.tld/after-path',
],
], \JSON_THROW_ON_ERROR),
);
$this->assertResponseCode($this->client->getResponse(), Response::HTTP_CREATED);
}
#[Test]
public function it_allows_an_anonymous_user_to_create_a_payment_request_for_a_guest_order(): void
{
$payment = $this->loadGuestOrder();
$this->client->request(
method: 'POST',
uri: sprintf('/api/v2/shop/orders/%s/payment-requests', $payment->getOrder()->getTokenValue()),
server: $this->headerBuilder()->withJsonLdAccept()->withJsonLdContentType()->build(),
content: json_encode([
'paymentId' => $payment->getId(),
'paymentMethodCode' => $payment->getMethod()->getCode(),
'payload' => [
'target_path' => 'https://myshop.tld/target-path',
'after_path' => 'https://myshop.tld/after-path',
],
], \JSON_THROW_ON_ERROR),
);
$this->assertResponseCode($this->client->getResponse(), Response::HTTP_CREATED);
}
#[Test]
public function it_allows_an_anonymous_user_to_create_a_payment_request_for_an_order_created_as_guest_by_a_registered_customer(): void
{
$payment = $this->loadOrderCreatedAsGuestByRegisteredCustomer();
$this->client->request(
method: 'POST',
uri: sprintf('/api/v2/shop/orders/%s/payment-requests', $payment->getOrder()->getTokenValue()),
server: $this->headerBuilder()->withJsonLdAccept()->withJsonLdContentType()->build(),
content: json_encode([
'paymentId' => $payment->getId(),
'paymentMethodCode' => $payment->getMethod()->getCode(),
'payload' => [
'target_path' => 'https://myshop.tld/target-path',
'after_path' => 'https://myshop.tld/after-path',
],
], \JSON_THROW_ON_ERROR),
);
$this->assertResponseCode($this->client->getResponse(), Response::HTTP_CREATED);
}
private function loadOrderOwnedByOliver(): PaymentInterface
{
return $this->loadOrderPayment('payment_request/order_with_customer.yaml');
}
private function loadGuestOrder(): PaymentInterface
{
return $this->loadOrderPayment('payment_request/order.yaml');
}
private function loadOrderCreatedAsGuestByRegisteredCustomer(): PaymentInterface
{
return $this->loadOrderPayment('payment_request/order_with_customer_created_as_guest.yaml');
}
private function loadOrderPayment(string $orderFixture): PaymentInterface
{
$fixtures = $this->loadFixturesFromFiles([
'authentication/shop_user.yaml',
'channel/channel.yaml',
'payment_method.yaml',
$orderFixture,
]);
/** @var PaymentInterface $payment */
$payment = $fixtures['payment'];
return $payment;
}
private function loadPaymentRequestOwnedByOliver(): PaymentRequestInterface
{
$fixtures = $this->loadFixturesFromFiles([