Skip to content

Commit

Permalink
bug #365 Secure checkout complete action (mamazu, lchrusciel)
Browse files Browse the repository at this point in the history
This PR was merged into the 1.0-dev branch.

Discussion
----------

This PR contains the improved implementation of #351. Current implementation will support following scenarios:

1. User is logged in - valid
2. User is not logged in and customer exists, but doesn't have an account - valid
3. User is logged in and customer exists and they are related - valid
4. User is not logged in and customer exists, but has an account - exception, he needs to log in
5. User is logged in but different mail is provided - exception

Commits
-------

ae0575e Fixed the bug and added tests to prevent it
d4c81f6 Fixed phpspec
836982c Removed redundant comparision
069e714 Refactored to use the user provider
2d6deaa Renaming the exception
98f9981 Fixed tests
ef6dc51 Fixed routes in PHPUnit Test
2fcec7d Changed the code to match the requirements
d1e99a2 Fixed phpspec tests
ae7433f Fixed tests for logged in customers
52e4e05 Adding check if a token exists
0408b78 Codestyle
2e64316 Improve logged in shop user provider
a8e8762 Provide logged in safe customer provider implementation
  • Loading branch information
Zales0123 authored Jan 4, 2019
2 parents 84dc0a8 + a8e8762 commit 0b18c2e
Show file tree
Hide file tree
Showing 26 changed files with 454 additions and 216 deletions.
2 changes: 2 additions & 0 deletions doc/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,8 @@ paths:
description: "Invalid input, validation failed."
schema:
$ref: "#/definitions/GeneralError"
403:
description: "Not logged in or wrong email"
/taxon-products-by-slug/{slug}:
get:
tags:
Expand Down
30 changes: 21 additions & 9 deletions spec/Handler/CompleteOrderHandlerSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@

final class CompleteOrderHandlerSpec extends ObjectBehavior
{
function let(OrderRepositoryInterface $orderRepository, CustomerProviderInterface $customerProvider, StateMachineFactoryInterface $stateMachineFactory): void
{
function let(
OrderRepositoryInterface $orderRepository,
CustomerProviderInterface $customerProvider,
StateMachineFactoryInterface $stateMachineFactory
): void {
$this->beConstructedWith($orderRepository, $customerProvider, $stateMachineFactory);
}

function it_handles_order_completion_for_existing_customer(
function it_handles_order_completion(
CustomerInterface $customer,
CustomerProviderInterface $customerProvider,
OrderInterface $order,
Expand All @@ -30,6 +33,7 @@ function it_handles_order_completion_for_existing_customer(
StateMachineInterface $stateMachine
): void {
$orderRepository->findOneBy(['tokenValue' => 'ORDERTOKEN'])->willReturn($order);

$customerProvider->provide('example@customer.com')->willReturn($customer);

$stateMachineFactory->get($order, OrderCheckoutTransitions::GRAPH)->willReturn($stateMachine);
Expand All @@ -51,6 +55,7 @@ function it_handles_order_completion_with_notes(
StateMachineInterface $stateMachine
): void {
$orderRepository->findOneBy(['tokenValue' => 'ORDERTOKEN'])->willReturn($order);

$customerProvider->provide('example@customer.com')->willReturn($customer);

$stateMachineFactory->get($order, OrderCheckoutTransitions::GRAPH)->willReturn($stateMachine);
Expand All @@ -63,14 +68,18 @@ function it_handles_order_completion_with_notes(
$this->handle(new CompleteOrder('ORDERTOKEN', 'example@customer.com', 'Some notes'));
}

function it_throws_an_exception_if_order_does_not_exist(OrderRepositoryInterface $orderRepository): void
{
function it_throws_an_exception_if_order_does_not_exist(
OrderRepositoryInterface $orderRepository
): void {
$orderRepository->findOneBy(['tokenValue' => 'ORDERTOKEN'])->willReturn(null);

$this->shouldThrow(\InvalidArgumentException::class)->during('handle', [new CompleteOrder('ORDERTOKEN', 'example@customer.com')]);
$this
->shouldThrow(\InvalidArgumentException::class)
->during('handle', [new CompleteOrder('ORDERTOKEN', 'example@customer.com')])
;
}

function it_throws_an_exception_if_order_cannot_be_addressed(
function it_throws_an_exception_if_order_cannot_be_completed(
StateMachineFactoryInterface $stateMachineFactory,
OrderInterface $order,
OrderRepositoryInterface $orderRepository,
Expand All @@ -79,8 +88,11 @@ function it_throws_an_exception_if_order_cannot_be_addressed(
$orderRepository->findOneBy(['tokenValue' => 'ORDERTOKEN'])->willReturn($order);

$stateMachineFactory->get($order, OrderCheckoutTransitions::GRAPH)->willReturn($stateMachine);
$stateMachine->can('complete')->willReturn(false);
$stateMachine->can(OrderCheckoutTransitions::TRANSITION_COMPLETE)->willReturn(false);

$this->shouldThrow(\InvalidArgumentException::class)->during('handle', [new CompleteOrder('ORDERTOKEN', 'example@customer.com')]);
$this
->shouldThrow(\InvalidArgumentException::class)
->during('handle', [new CompleteOrder('ORDERTOKEN', 'example@customer.com')])
;
}
}
45 changes: 0 additions & 45 deletions spec/Provider/CustomerProviderSpec.php

This file was deleted.

65 changes: 65 additions & 0 deletions spec/Provider/LoggedInShopUserProviderSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

declare(strict_types=1);

namespace spec\Sylius\ShopApiPlugin\Provider;

use PhpSpec\ObjectBehavior;
use Sylius\Component\Core\Model\ShopUserInterface;
use Sylius\Component\User\Model\UserInterface;
use Sylius\ShopApiPlugin\Provider\LoggedInShopUserProviderInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\TokenNotFoundException;

final class LoggedInShopUserProviderSpec extends ObjectBehavior
{
function let(TokenStorageInterface $tokenStorage): void
{
$this->beConstructedWith($tokenStorage);
}

function it_is_reviewer_subject_provider(): void
{
$this->shouldImplement(LoggedInShopUserProviderInterface::class);
}

function it_throws_an_error_if_there_is_no_shop_user_logged_in(
TokenStorageInterface $tokenStorage,
TokenInterface $token,
UserInterface $anotherUser
): void {
$tokenStorage->getToken()->willReturn(null, $token);
$token->getUser()->willReturn(null, $anotherUser);

$this->shouldThrow(TokenNotFoundException::class)->during('provide');
$this->shouldThrow(TokenNotFoundException::class)->during('provide');
$this->shouldThrow(TokenNotFoundException::class)->during('provide');
}

function it_returns_the_logged_in_user_if_there_is_one(
TokenStorageInterface $tokenStorage,
TokenInterface $token,
ShopUserInterface $shopUser
): void {
$token->getUser()->willReturn($shopUser);
$tokenStorage->getToken()->willReturn($token);

$this->provide()->shouldReturn($shopUser);
}

function it_checks_if_shop_user_is_logged_in(
TokenStorageInterface $tokenStorage,
TokenInterface $token,
ShopUserInterface $shopUser,
UserInterface $anotherUser
): void {
$tokenStorage->getToken()->willReturn(null, $token);
$token->getUser()->willReturn(null, $anotherUser, $shopUser);

$this->isUserLoggedIn()->shouldReturn(false);
$this->isUserLoggedIn()->shouldReturn(false);
$this->isUserLoggedIn()->shouldReturn(false);
$this->isUserLoggedIn()->shouldReturn(true);
}
}
46 changes: 0 additions & 46 deletions spec/Provider/LoggedInUserProviderSpec.php

This file was deleted.

102 changes: 102 additions & 0 deletions spec/Provider/ShopUserAwareCustomerProviderSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?php

declare(strict_types=1);

namespace spec\Sylius\ShopApiPlugin\Provider;

use PhpSpec\ObjectBehavior;
use Sylius\Component\Core\Model\CustomerInterface;
use Sylius\Component\Core\Model\ShopUserInterface;
use Sylius\Component\Core\Repository\CustomerRepositoryInterface;
use Sylius\Component\Resource\Factory\FactoryInterface;
use Sylius\ShopApiPlugin\Exception\WrongUserException;
use Sylius\ShopApiPlugin\Provider\CustomerProviderInterface;
use Sylius\ShopApiPlugin\Provider\LoggedInShopUserProviderInterface;

final class ShopUserAwareCustomerProviderSpec extends ObjectBehavior
{
function let(
CustomerRepositoryInterface $customerRepository,
FactoryInterface $customerFactory,
LoggedInShopUserProviderInterface $loggedInShopUserProvider
): void {
$this->beConstructedWith($customerRepository, $customerFactory, $loggedInShopUserProvider);
}

function it_is_customer_provider(): void
{
$this->shouldImplement(CustomerProviderInterface::class);
}

function it_provides_customer_from_reposiotory_if_it_does_not_have_related_shop_user(
CustomerRepositoryInterface $customerRepository,
CustomerInterface $customer,
LoggedInShopUserProviderInterface $loggedInShopUserProvider
): void {
$loggedInShopUserProvider->isUserLoggedIn()->willReturn(false);

$customerRepository->findOneBy(['email' => 'example@customer.com'])->willReturn($customer);

$customer->getUser()->willReturn(null);

$this->provide('example@customer.com')->shouldReturn($customer);
}

function it_creates_new_customer_if_it_does_not_exists(
CustomerRepositoryInterface $customerRepository,
FactoryInterface $customerFactory,
CustomerInterface $customer,
LoggedInShopUserProviderInterface $loggedInShopUserProvider
): void {
$loggedInShopUserProvider->isUserLoggedIn()->willReturn(false);
$customerRepository->findOneBy(['email' => 'example@customer.com'])->willReturn(null);
$customerFactory->createNew()->willReturn($customer);

$customer->setEmail('example@customer.com')->shouldBeCalled();
$customerRepository->add($customer)->shouldBeCalled();

$this->provide('example@customer.com')->shouldReturn($customer);
}

function it_provides_customer_from_reposiotory_if_it_has_related_shop_user_and_user_is_logged_in(
CustomerInterface $customer,
LoggedInShopUserProviderInterface $loggedInShopUserProvider,
ShopUserInterface $shopUser
): void {
$loggedInShopUserProvider->isUserLoggedIn()->willReturn(true);
$loggedInShopUserProvider->provide()->willReturn($shopUser);

$shopUser->getCustomer()->willReturn($customer);
$customer->getEmail()->willReturn('example@customer.com');

$this->provide('example@customer.com')->shouldReturn($customer);
}

function it_throws_an_exception_if_requested_customer_is_not_logged_in_but_has_related_shop_user(
CustomerRepositoryInterface $customerRepository,
CustomerInterface $customer,
LoggedInShopUserProviderInterface $loggedInShopUserProvider,
ShopUserInterface $shopUser
): void {
$customerRepository->findOneBy(['email' => 'example@customer.com'])->willReturn($customer);
$loggedInShopUserProvider->isUserLoggedIn()->willReturn(false);

$customer->getUser()->willReturn($shopUser);

$this->shouldThrow(WrongUserException::class)->during('provide', ['example@customer.com']);
}

function it_throws_an_exception_if_requested_customer_is_logged_in_but_customer_is_related_to_another_shop_user(
CustomerInterface $customer,
LoggedInShopUserProviderInterface $loggedInShopUserProvider,
ShopUserInterface $shopUser
): void {
$loggedInShopUserProvider->isUserLoggedIn()->willReturn(true);
$loggedInShopUserProvider->provide()->willReturn($shopUser);

$shopUser->getCustomer()->willReturn($customer);
$customer->getEmail()->willReturn('anotherCustomer@customer.com');

$this->shouldThrow(WrongUserException::class)->during('provide', ['example@customer.com']);
}
}
8 changes: 4 additions & 4 deletions spec/Validator/Address/AddressExistsValidatorSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use Sylius\Component\Core\Model\CustomerInterface;
use Sylius\Component\Core\Model\ShopUserInterface;
use Sylius\Component\Core\Repository\AddressRepositoryInterface;
use Sylius\ShopApiPlugin\Provider\LoggedInUserProviderInterface;
use Sylius\ShopApiPlugin\Provider\LoggedInShopUserProviderInterface;
use Sylius\ShopApiPlugin\Validator\Constraints\AddressExists;
use Symfony\Component\Validator\Context\ExecutionContextInterface;

Expand All @@ -19,7 +19,7 @@ final class AddressExistsValidatorSpec extends ObjectBehavior
function let(
ExecutionContextInterface $executionContext,
AddressRepositoryInterface $addressRepository,
LoggedInUserProviderInterface $currentUserProvider
LoggedInShopUserProviderInterface $currentUserProvider
): void {
$this->beConstructedWith($addressRepository, $currentUserProvider);

Expand All @@ -30,7 +30,7 @@ function it_does_not_add_constraint_if_address_exists_and_its_owned_by_current_u
AddressInterface $address,
ShopUserInterface $shopUser,
CustomerInterface $customerOwner,
LoggedInUserProviderInterface $currentUserProvider,
LoggedInShopUserProviderInterface $currentUserProvider,
AddressRepositoryInterface $addressRepository,
ExecutionContextInterface $executionContext
): void {
Expand Down Expand Up @@ -61,7 +61,7 @@ function it_adds_constraint_if_address_does_not_exits_exists(
function it_adds_constraint_if_current_user_is_not_address_owner(
AddressInterface $address,
AddressRepositoryInterface $addressRepository,
LoggedInUserProviderInterface $currentUserProvider,
LoggedInShopUserProviderInterface $currentUserProvider,
ShopUserInterface $shopUser,
CustomerInterface $customerOwner,
ExecutionContextInterface $executionContext
Expand Down
Loading

0 comments on commit 0b18c2e

Please sign in to comment.