Skip to content

Commit 49ceb88

Browse files
joanhejoanhe
authored
Merge pull request #3621 from magento-trigger/MC-5899
[trigger] MC-5899: Wishlist - Insecure Direct Object Reference
2 parents 8e2ca94 + efa0ded commit 49ceb88

File tree

5 files changed

+162
-9
lines changed

5 files changed

+162
-9
lines changed

‎app/code/Magento/Wishlist/Model/Rss/Wishlist.php

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,8 @@ public function __construct(
118118
*/
119119
public function isAllowed()
120120
{
121-
return $this->scopeConfig->isSetFlag(
122-
'rss/wishlist/active',
123-
ScopeInterface::SCOPE_STORE
124-
);
121+
return $this->scopeConfig->isSetFlag('rss/wishlist/active', ScopeInterface::SCOPE_STORE)
122+
&& $this->getWishlist()->getCustomerId() === $this->wishlistHelper->getCustomer()->getId();
125123
}
126124

127125
/**
@@ -185,8 +183,8 @@ public function getRssData()
185183
}
186184
} else {
187185
$data = [
188-
'title' => __('We cannot retrieve the Wish List.'),
189-
'description' => __('We cannot retrieve the Wish List.'),
186+
'title' => __('We cannot retrieve the Wish List.')->render(),
187+
'description' => __('We cannot retrieve the Wish List.')->render(),
190188
'link' => $this->urlBuilder->getUrl(),
191189
'charset' => 'UTF-8',
192190
];
@@ -202,7 +200,7 @@ public function getRssData()
202200
*/
203201
public function getCacheKey()
204202
{
205-
return 'rss_wishlist_data';
203+
return 'rss_wishlist_data_' . $this->getWishlist()->getId();
206204
}
207205

208206
/**
@@ -224,7 +222,7 @@ public function getHeader()
224222
{
225223
$customerId = $this->getWishlist()->getCustomerId();
226224
$customer = $this->customerFactory->create()->load($customerId);
227-
$title = __('%1\'s Wishlist', $customer->getName());
225+
$title = __('%1\'s Wishlist', $customer->getName())->render();
228226
$newUrl = $this->urlBuilder->getUrl(
229227
'wishlist/shared/index',
230228
['code' => $this->getWishlist()->getSharingCode()]

‎app/code/Magento/Wishlist/Test/Unit/Model/Rss/WishlistTest.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,15 +278,35 @@ protected function processWishlistItemDescription($wishlistModelMock, $staticArg
278278

279279
public function testIsAllowed()
280280
{
281+
$customerId = 1;
282+
$customerServiceMock = $this->createMock(\Magento\Customer\Api\Data\CustomerInterface::class);
283+
$wishlist = $this->getMockBuilder(\Magento\Wishlist\Model\Wishlist::class)->setMethods(
284+
['getId', '__wakeup', 'getCustomerId', 'getItemCollection', 'getSharingCode']
285+
)->disableOriginalConstructor()->getMock();
286+
$wishlist->expects($this->once())->method('getCustomerId')->willReturn($customerId);
287+
$this->wishlistHelperMock->expects($this->any())->method('getWishlist')
288+
->will($this->returnValue($wishlist));
289+
$this->wishlistHelperMock->expects($this->any())
290+
->method('getCustomer')
291+
->will($this->returnValue($customerServiceMock));
292+
$customerServiceMock->expects($this->once())->method('getId')->willReturn($customerId);
281293
$this->scopeConfig->expects($this->once())->method('isSetFlag')
282294
->with('rss/wishlist/active', \Magento\Store\Model\ScopeInterface::SCOPE_STORE)
283295
->will($this->returnValue(true));
296+
284297
$this->assertTrue($this->model->isAllowed());
285298
}
286299

287300
public function testGetCacheKey()
288301
{
289-
$this->assertEquals('rss_wishlist_data', $this->model->getCacheKey());
302+
$wishlistId = 1;
303+
$wishlist = $this->getMockBuilder(\Magento\Wishlist\Model\Wishlist::class)->setMethods(
304+
['getId', '__wakeup', 'getCustomerId', 'getItemCollection', 'getSharingCode']
305+
)->disableOriginalConstructor()->getMock();
306+
$wishlist->expects($this->once())->method('getId')->willReturn($wishlistId);
307+
$this->wishlistHelperMock->expects($this->any())->method('getWishlist')
308+
->will($this->returnValue($wishlist));
309+
$this->assertEquals('rss_wishlist_data_1', $this->model->getCacheKey());
290310
}
291311

292312
public function testGetCacheLifetime()

‎dev/tests/integration/testsuite/Magento/Rss/Controller/Feed/IndexTest.php

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Rss\Controller\Feed;
9+
10+
class IndexTest extends \Magento\TestFramework\TestCase\AbstractBackendController
11+
{
12+
/**
13+
* @var \Magento\Rss\Model\UrlBuilder
14+
*/
15+
private $urlBuilder;
16+
17+
/**
18+
* @var \Magento\Customer\Api\CustomerRepositoryInterface
19+
*/
20+
private $customerRepository;
21+
22+
/**
23+
* @var \Magento\Wishlist\Model\Wishlist
24+
*/
25+
private $wishlist;
26+
27+
/**
28+
* @var
29+
*/
30+
private $customerSession;
31+
32+
protected function setUp()
33+
{
34+
parent::setUp();
35+
$this->urlBuilder = $this->_objectManager->get(\Magento\Rss\Model\UrlBuilder::class);
36+
$this->customerRepository = $this->_objectManager->get(
37+
\Magento\Customer\Api\CustomerRepositoryInterface::class
38+
);
39+
$this->wishlist = $this->_objectManager->get(\Magento\Wishlist\Model\Wishlist::class);
40+
$this->customerSession = $this->_objectManager->get(\Magento\Customer\Model\Session::class);
41+
}
42+
43+
/**
44+
* Check Rss response.
45+
*
46+
* @magentoAppIsolation enabled
47+
* @magentoDataFixture Magento/Wishlist/_files/two_wishlists_for_two_diff_customers.php
48+
* @magentoConfigFixture current_store rss/wishlist/active 1
49+
* @magentoConfigFixture current_store rss/config/active 1
50+
*/
51+
public function testRssResponse()
52+
{
53+
$firstCustomerId = 1;
54+
$this->customerSession->setCustomerId($firstCustomerId);
55+
$customer = $this->customerRepository->getById($firstCustomerId);
56+
$customerEmail = $customer->getEmail();
57+
$wishlistId = $this->wishlist->loadByCustomerId($firstCustomerId)->getId();
58+
$this->dispatch($this->getLink($firstCustomerId, $customerEmail, $wishlistId));
59+
$body = $this->getResponse()->getBody();
60+
$this->assertContains('<title>John Smith\'s Wishlist</title>', $body);
61+
}
62+
63+
/**
64+
* Check Rss with incorrect wishlist id.
65+
*
66+
* @magentoAppIsolation enabled
67+
* @magentoDataFixture Magento/Wishlist/_files/two_wishlists_for_two_diff_customers.php
68+
* @magentoConfigFixture current_store rss/wishlist/active 1
69+
* @magentoConfigFixture current_store rss/config/active 1
70+
*/
71+
public function testRssResponseWithIncorrectWishlistId()
72+
{
73+
$firstCustomerId = 1;
74+
$secondCustomerId = 2;
75+
$this->customerSession->setCustomerId($firstCustomerId);
76+
$customer = $this->customerRepository->getById($firstCustomerId);
77+
$customerEmail = $customer->getEmail();
78+
$wishlistId = $this->wishlist->loadByCustomerId($secondCustomerId, true)->getId();
79+
$this->dispatch($this->getLink($firstCustomerId, $customerEmail, $wishlistId));
80+
$body = $this->getResponse()->getBody();
81+
$this->assertContains('<title>404 Not Found</title>', $body);
82+
}
83+
84+
private function getLink($customerId, $customerEmail, $wishlistId)
85+
{
86+
87+
return 'rss/feed/index/type/wishlist/data/'
88+
. base64_encode($customerId . ',' . $customerEmail)
89+
. '/wishlist_id/' . $wishlistId;
90+
}
91+
}

‎dev/tests/integration/testsuite/Magento/Wishlist/_files/two_wishlists_for_two_diff_customers.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
require __DIR__ . '/../../../Magento/Customer/_files/two_customers.php';
9+
require __DIR__ . '/../../../Magento/Catalog/_files/product_simple.php';
10+
11+
$firstCustomerIdFromFixture = 1;
12+
$wishlistForFirstCustomer = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
13+
\Magento\Wishlist\Model\Wishlist::class
14+
);
15+
$wishlistForFirstCustomer->loadByCustomerId($firstCustomerIdFromFixture, true);
16+
$item = $wishlistForFirstCustomer->addNewItem($product, new \Magento\Framework\DataObject([]));
17+
$wishlistForFirstCustomer->save();
18+
19+
$secondCustomerIdFromFixture = 2;
20+
$wishlistForSecondCustomer = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
21+
\Magento\Wishlist\Model\Wishlist::class
22+
);
23+
$wishlistForSecondCustomer->loadByCustomerId($secondCustomerIdFromFixture, true);
24+
$item = $wishlistForSecondCustomer->addNewItem($product, new \Magento\Framework\DataObject([]));
25+
$wishlistForSecondCustomer->save();

‎dev/tests/integration/testsuite/Magento/Wishlist/_files/two_wishlists_for_two_diff_customers_rollback.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
/** @var \Magento\Framework\ObjectManagerInterface $objectManager */
9+
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
10+
11+
/** @var \Magento\Wishlist\Model\Wishlist $wishlist */
12+
$wishlist = $objectManager->create(\Magento\Wishlist\Model\Wishlist::class);
13+
$wishlist->loadByCustomerId(1);
14+
$wishlist->delete();
15+
$wishlist->loadByCustomerId(2);
16+
$wishlist->delete();
17+
18+
require __DIR__ . '/../../../Magento/Customer/_files/two_customers_rollback.php';
19+
require __DIR__ . '/../../../Magento/Catalog/_files/product_simple_rollback.php';

0 commit comments

Comments
 (0)