Skip to content

Commit c81cc69

Browse files
author
Ievgen Shakhsuvarov
committed
MAGETWO-37037: Customers information leak via checkout
1 parent 49a7b12 commit c81cc69

File tree

3 files changed

+88
-71
lines changed

3 files changed

+88
-71
lines changed

‎app/code/Magento/Multishipping/Model/Checkout/Type/Multishipping.php‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -959,9 +959,6 @@ protected function isAddressIdApplicable($addressId)
959959
/** @var \Magento\Customer\Api\Data\AddressInterface $address */
960960
return $address->getId();
961961
}, $this->getCustomer()->getAddresses());
962-
if (in_array($addressId, $applicableAddressIds)) {
963-
return true;
964-
}
965-
return false;
962+
return in_array($addressId, $applicableAddressIds);
966963
}
967964
}

‎app/code/Magento/Quote/Model/QuoteAddressValidator.php‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,32 @@ class QuoteAddressValidator
1616
protected $addressReporitory;
1717

1818
/**
19-
* Customer factory.
19+
* Customer repository.
2020
*
21-
* @var \Magento\Customer\Model\CustomerFactory
21+
* @var \Magento\Customer\Api\CustomerRepositoryInterface
2222
*/
23-
protected $customerFactory;
23+
protected $customerRepository;
24+
25+
/**
26+
* @var \Magento\Customer\Model\Session
27+
*/
28+
protected $customerSession;
2429

2530
/**
2631
* Constructs a quote shipping address validator service object.
2732
*
2833
* @param \Magento\Customer\Api\AddressRepositoryInterface $addressRepository
29-
* @param \Magento\Customer\Model\CustomerFactory $customerFactory Customer factory.
34+
* @param \Magento\Customer\Api\CustomerRepositoryInterface $customerRepository Customer repository.
35+
* @param \Magento\Customer\Model\Session $customerSession
3036
*/
3137
public function __construct(
3238
\Magento\Customer\Api\AddressRepositoryInterface $addressRepository,
33-
\Magento\Customer\Model\CustomerFactory $customerFactory
39+
\Magento\Customer\Api\CustomerRepositoryInterface $customerRepository,
40+
\Magento\Customer\Model\Session $customerSession
3441
) {
3542
$this->addressReporitory = $addressRepository;
36-
$this->customerFactory = $customerFactory;
43+
$this->customerRepository = $customerRepository;
44+
$this->customerSession = $customerSession;
3745
}
3846

3947
/**
@@ -48,8 +56,7 @@ public function validate(\Magento\Quote\Api\Data\AddressInterface $addressData)
4856
{
4957
//validate customer id
5058
if ($addressData->getCustomerId()) {
51-
$customer = $this->customerFactory->create();
52-
$customer->load($addressData->getCustomerId());
59+
$customer = $this->customerRepository->getById($addressData->getCustomerId());
5360
if (!$customer->getId()) {
5461
throw new \Magento\Framework\Exception\NoSuchEntityException(
5562
__('Invalid customer id %1', $addressData->getCustomerId())
@@ -70,12 +77,24 @@ public function validate(\Magento\Quote\Api\Data\AddressInterface $addressData)
7077
// check correspondence between customer id and address id
7178
if ($addressData->getCustomerId()) {
7279
if ($address->getCustomerId() != $addressData->getCustomerId()) {
73-
throw new \Magento\Framework\Exception\InputException(
74-
__('Address with id %1 belongs to another customer', $addressData->getId())
80+
throw new \Magento\Framework\Exception\NoSuchEntityException(
81+
__('Invalid address id %1', $addressData->getId())
7582
);
7683
}
7784
}
7885
}
86+
87+
if ($addressData->getCustomerAddressId()) {
88+
$applicableAddressIds = array_map(function($address) {
89+
/** @var \Magento\Customer\Api\Data\AddressInterface $address */
90+
return $address->getId();
91+
}, $this->customerSession->getCustomerDataObject()->getAddresses());
92+
if (!in_array($addressData->getCustomerAddressId(), $applicableAddressIds)) {
93+
throw new \Magento\Framework\Exception\NoSuchEntityException(
94+
__('Invalid address id %1', $addressData->getCustomerAddressId())
95+
);
96+
}
97+
}
7998
return true;
8099
}
81100
}

‎app/code/Magento/Quote/Test/Unit/Model/QuoteAddressValidatorTest.php‎

Lines changed: 58 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,15 @@
11
<?php
22
/**
3-
*
43
* Copyright © 2015 Magento. All rights reserved.
54
* See COPYING.txt for license details.
65
*/
76

8-
// @codingStandardsIgnoreFile
9-
107
namespace Magento\Quote\Test\Unit\Model;
118

12-
use \Magento\Quote\Model\QuoteAddressValidator;
13-
149
class QuoteAddressValidatorTest extends \PHPUnit_Framework_TestCase
1510
{
1611
/**
17-
* @var QuoteAddressValidator
12+
* @var \Magento\Quote\Model\QuoteAddressValidator
1813
*/
1914
protected $model;
2015

@@ -31,7 +26,7 @@ class QuoteAddressValidatorTest extends \PHPUnit_Framework_TestCase
3126
/**
3227
* @var \PHPUnit_Framework_MockObject_MockObject
3328
*/
34-
protected $customerFactoryMock;
29+
protected $customerRepositoryMock;
3530

3631
/**
3732
* @var \PHPUnit_Framework_MockObject_MockObject
@@ -41,7 +36,7 @@ class QuoteAddressValidatorTest extends \PHPUnit_Framework_TestCase
4136
/**
4237
* @var \PHPUnit_Framework_MockObject_MockObject
4338
*/
44-
protected $customerMock;
39+
protected $customerSessionMock;
4540

4641
public function setUp()
4742
{
@@ -61,15 +56,20 @@ public function setUp()
6156
'',
6257
false
6358
);
64-
$this->customerFactoryMock = $this->getMock(
65-
'\Magento\Customer\Model\CustomerFactory', ['create', '__wakeup'], [], '', false);
66-
$this->customerMock = $this->getMock('\Magento\Customer\Model\Customer', [], [], '', false);
67-
59+
$this->customerRepositoryMock = $this->getMock(
60+
'\Magento\Customer\Api\CustomerRepositoryInterface',
61+
[],
62+
[],
63+
'',
64+
false
65+
);
66+
$this->customerSessionMock = $this->getMock('\Magento\Customer\Model\Session', [], [], '', false);
6867
$this->model = $this->objectManager->getObject(
6968
'\Magento\Quote\Model\QuoteAddressValidator',
7069
[
7170
'addressRepository' => $this->addressRepositoryMock,
72-
'customerFactory' => $this->customerFactoryMock
71+
'customerRepository' => $this->customerRepositoryMock,
72+
'customerSession' => $this->customerSessionMock
7373
]
7474
);
7575
}
@@ -81,16 +81,12 @@ public function setUp()
8181
public function testValidateInvalidCustomer()
8282
{
8383
$customerId = 100;
84-
85-
$this->customerFactoryMock->expects($this->once())
86-
->method('create')
87-
->will($this->returnValue($this->customerMock));
88-
89-
$this->customerMock->expects($this->once())->method('load')->with($customerId);
90-
$this->customerMock->expects($this->once())->method('getId')->will($this->returnValue(null));
91-
9284
$address = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
85+
$customerMock = $this->getMock('\Magento\Customer\Api\Data\CustomerInterface');
86+
9387
$address->expects($this->atLeastOnce())->method('getCustomerId')->willReturn($customerId);
88+
$this->customerRepositoryMock->expects($this->once())->method('getById')->with($customerId)
89+
->willReturn($customerMock);
9490
$this->model->validate($address);
9591
}
9692

@@ -100,14 +96,13 @@ public function testValidateInvalidCustomer()
10096
*/
10197
public function testValidateInvalidAddress()
10298
{
103-
$this->customerFactoryMock->expects($this->never())->method('create');
104-
$this->customerMock->expects($this->never())->method('load');
99+
$address = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
100+
$this->customerRepositoryMock->expects($this->never())->method('getById');
101+
$address->expects($this->atLeastOnce())->method('getId')->willReturn(101);
105102

106103
$this->addressRepositoryMock->expects($this->once())->method('getById')
107104
->willThrowException(new \Magento\Framework\Exception\NoSuchEntityException());
108105

109-
$address = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
110-
$address->expects($this->atLeastOnce())->method('getId')->willReturn(101);
111106
$this->model->validate($address);
112107
}
113108

@@ -116,70 +111,76 @@ public function testValidateInvalidAddress()
116111
*/
117112
public function testValidateNewAddress()
118113
{
119-
$this->customerFactoryMock->expects($this->never())->method('create');
114+
$this->customerRepositoryMock->expects($this->never())->method('getById');
120115
$this->addressRepositoryMock->expects($this->never())->method('getById');
121-
122116
$address = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
123117
$this->assertTrue($this->model->validate($address));
124118
}
125119

126120
/**
127-
* @expectedException \Magento\Framework\Exception\InputException
128-
* @expectedExceptionMessage Address with id 100 belongs to another customer
121+
* @expectedException \Magento\Framework\Exception\NoSuchEntityException
122+
* @expectedExceptionMessage Invalid address id 100
129123
*/
130124
public function testValidateWithAddressOfOtherCustomer()
131125
{
132126
$addressCustomer = 100;
133127
$addressId = 100;
134-
135128
$address = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
129+
$customerMock = $this->getMock('\Magento\Customer\Api\Data\CustomerInterface');
130+
131+
$this->customerRepositoryMock->expects($this->once())->method('getById')->with($addressCustomer)
132+
->willReturn($customerMock);
133+
$this->addressRepositoryMock->expects($this->once())->method('getById')->willReturn($this->quoteAddressMock);
134+
$customerMock->expects($this->once())->method('getId')->willReturn(42);
136135
$address->expects($this->atLeastOnce())->method('getId')->willReturn($addressId);
137136
$address->expects($this->atLeastOnce())->method('getCustomerId')->willReturn($addressCustomer);
138137

139-
/** Customer mock */
140-
$this->customerFactoryMock->expects($this->once())
141-
->method('create')
142-
->will($this->returnValue($this->customerMock));
143-
144-
$this->customerMock->expects($this->once())->method('load')->with($addressCustomer);
145-
$this->customerMock->expects($this->once())->method('getId')->will($this->returnValue($addressCustomer));
138+
$this->quoteAddressMock->expects($this->once())->method('getCustomerId')->willReturn(42);
139+
$this->model->validate($address);
140+
}
146141

147-
/** Quote address mock */
148-
$this->addressRepositoryMock->expects($this->once())->method('getById')
149-
->will($this->returnValue($this->quoteAddressMock));
142+
/**
143+
* @expectedException \Magento\Framework\Exception\NoSuchEntityException
144+
* @expectedExceptionMessage Invalid address id 42
145+
*/
146+
public function testValidateWithInvalidCustomerAddressId()
147+
{
148+
$customerAddressId = 42;
149+
$address = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
150+
$customerAddress = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
151+
$customerMock = $this->getMock('\Magento\Customer\Api\Data\CustomerInterface');
150152

151-
$this->quoteAddressMock->expects($this->atLeastOnce())->method('getCustomerId')
152-
->will($this->returnValue(10));
153+
$address->expects($this->atLeastOnce())->method('getCustomerAddressId')->willReturn($customerAddressId);
154+
$this->customerSessionMock->expects($this->once())->method('getCustomerDataObject')->willReturn($customerMock);
155+
$customerMock->expects($this->once())->method('getAddresses')->willReturn([$customerAddress]);
156+
$customerAddress->expects($this->once())->method('getId')->willReturn(43);
153157

154-
/** Validate */
155158
$this->model->validate($address);
156159
}
157160

158161
public function testValidateWithValidAddress()
159162
{
160163
$addressCustomer = 100;
161164
$addressId = 100;
165+
$customerAddressId = 42;
162166

163167
$address = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
164168
$address->expects($this->atLeastOnce())->method('getId')->willReturn($addressId);
165169
$address->expects($this->atLeastOnce())->method('getCustomerId')->willReturn($addressCustomer);
170+
$address->expects($this->atLeastOnce())->method('getCustomerAddressId')->willReturn($customerAddressId);
171+
$customerMock = $this->getMock('\Magento\Customer\Api\Data\CustomerInterface');
172+
$customerAddress = $this->getMock('\Magento\Quote\Api\Data\AddressInterface');
166173

167-
/** Customer mock */
168-
$this->customerFactoryMock->expects($this->once())
169-
->method('create')
170-
->will($this->returnValue($this->customerMock));
174+
$this->customerRepositoryMock->expects($this->once())->method('getById')->willReturn($customerMock);
175+
$customerMock->expects($this->once())->method('getId')->willReturn($addressCustomer);
171176

172-
$this->customerMock->expects($this->once())->method('load')->with($addressCustomer);
173-
$this->customerMock->expects($this->once())->method('getId')->will($this->returnValue($addressCustomer));
177+
$this->addressRepositoryMock->expects($this->once())->method('getById')->willReturn($this->quoteAddressMock);
178+
$this->quoteAddressMock->expects($this->any())->method('getCustomerId')->willReturn($addressCustomer);
174179

175-
/** Quote address mock */
176-
$this->addressRepositoryMock->expects($this->once())->method('getById')
177-
->will($this->returnValue($this->quoteAddressMock));
178-
179-
$this->quoteAddressMock->expects($this->any())->method('getCustomerId')
180-
->will($this->returnValue($addressCustomer));
180+
$this->customerSessionMock->expects($this->once())->method('getCustomerDataObject')->willReturn($customerMock);
181+
$customerMock->expects($this->once())->method('getAddresses')->willReturn([$customerAddress]);
182+
$customerAddress->expects($this->once())->method('getId')->willReturn(42);
181183

182-
/** Validate */
183-
$this->model->validate($address);
184+
$this->assertTrue($this->model->validate($address));
184185
}
185186
}

0 commit comments

Comments
 (0)