Sylius / CustomerReorderPlugin

Plugin that allows customers to reorder previously placed order.
20 stars 25 forks source link

[🔐Security] Avoid reordering order from a different customer than themself #37

Closed jacquesbh closed 5 years ago

jacquesbh commented 6 years ago

I'm sure there is a different way, and probably better way to perform this verification.

But for now I prefer to fix security issues quickly and open an issue to improve the fix than doing nothing.

Thanks :)

P.S. The tests are missing. I didn't go into the testing part of Sylius. So, I may need help on this. Maybe if a core team member could write the test(s) in this branch, I'll be happy to learn from them. ❤️

bartoszpietrzak1994 commented 6 years ago

You can use

@Given there is a customer :name identified by an email :email and a password :password

step placed in Sylius\Behat\Context\Setup\CustomerContext

and then

@Given /^there is (?:a|another) (customer "[^"]+") that placed (an order "[^"]+")$/

step placed in Sylius\Behat\Context\Setup\OrderContext class.

In that way, you will be able to create a new customer and then place an order with given number.

Then, you will have to create another customer and write a new step simulating CustomerReorderAction call when the second user is set as a Customer in CustomerContext and the id parameter is the id of the order placed by the first customer.

I would also recommend you to move checks that you've written to a separate service (i. e. OrderCustomerRelationChecker) and test them using PHPSpec.

Zales0123 commented 5 years ago

The explained security issue has been resolved in #40, thank you very much, Jacques, for your contribution!

jacquesbh commented 5 years ago

… no problem.