Skip to content

Commit

Permalink
Merge pull request #17 from magento-api/MAGETWO-31999-github-829-oAut…
Browse files Browse the repository at this point in the history
…h-issue

[API] MAGETWO-31999: oAuth issue [from github]
  • Loading branch information
vpelipenko committed Jan 13, 2015
2 parents 2f03eec + 4b4d193 commit 7581dd6
Show file tree
Hide file tree
Showing 30 changed files with 200 additions and 77 deletions.
4 changes: 2 additions & 2 deletions app/code/Magento/Checkout/Model/Type/Onepage.php
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ protected function _validateCustomerData(array $data)
$quote = $this->getQuote();
$isCustomerNew = !$quote->getCustomerId();
$customer = $quote->getCustomer();
$customerData = $this->extensibleDataObjectConverter->toFlatArray($customer);
$customerData = $this->extensibleDataObjectConverter->toFlatArray($customer, [], '\Magento\Customer\Api\Data\CustomerInterface');

/** @var Form $customerForm */
$customerForm = $this->_formFactory->create(
Expand Down Expand Up @@ -594,7 +594,7 @@ protected function _validateCustomerData(array $data)
$this->_objectCopyService->copyFieldsetToTarget(
'customer_account',
'to_quote',
$this->extensibleDataObjectConverter->toFlatArray($customer),
$this->extensibleDataObjectConverter->toFlatArray($customer, [], '\Magento\Customer\Api\Data\CustomerInterface'),
$quote
);

Expand Down
6 changes: 5 additions & 1 deletion app/code/Magento/Customer/Block/Adminhtml/Edit/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ protected function _prepareForm()
$form->addField('id', 'hidden', ['name' => 'customer_id']);
$customer = $this->_customerRepository->getById($customerId);
$form->setValues(
$this->_extensibleDataObjectConverter->toFlatArray($customer)
$this->_extensibleDataObjectConverter->toFlatArray(
$customer,
[],
'\Magento\Customer\Api\Data\CustomerInterface'
)
)->addValues(
['customer_id' => $customerId]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,11 @@ protected function _customizeFieldset($fieldset)
);
$form->getElement('website_id')->setRenderer($renderer);

$accountData = $this->_extensibleDataObjectConverter->toFlatArray($this->_getCustomerDataObject());
$accountData = $this->_extensibleDataObjectConverter->toFlatArray(
$this->_getCustomerDataObject(),
[],
'\Magento\Customer\Api\Data\CustomerInterface'
);

if ($this->_getCustomerDataObject()->getId()) {
$customerFormFields = $this->_addEditCustomerFormFields($fieldset);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ protected function _validateCustomer($response)
$customerForm = $this->_formFactory->create(
'customer',
'adminhtml_customer',
$this->_extensibleDataObjectConverter->toFlatArray($customer),
$this->_extensibleDataObjectConverter->toFlatArray(
$customer,
[],
'\Magento\Customer\Api\Data\CustomerInterface'
),
true
);
$customerForm->setInvisibleIgnored(true);
Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/Customer/Model/AccountManagement.php
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ protected function createPasswordHash($password)
public function validate(\Magento\Customer\Api\Data\CustomerInterface $customer)
{
$customerErrors = $this->validator->validateData(
$this->extensibleDataObjectConverter->toFlatArray($customer),
$this->extensibleDataObjectConverter->toFlatArray($customer, [], '\Magento\Customer\Api\Data\CustomerInterface'),
[],
'customer'
);
Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/Customer/Model/Address/Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function __construct(ExtensibleDataObjectConverter $extensibleDataObjectC
*/
public function toFlatArray($addressDataObject)
{
$flatAddressArray = $this->extensibleDataObjectConverter->toFlatArray($addressDataObject);
$flatAddressArray = $this->extensibleDataObjectConverter->toFlatArray($addressDataObject, [], '\Magento\Customer\Api\Data\AddressInterface');
//preserve street
$street = $addressDataObject->getStreet();
if (!empty($street) && is_array($street)) {
Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/Customer/Model/Customer/Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function __construct(ExtensibleDataObjectConverter $extensibleDataObjectC
*/
public function toFlatArray(CustomerInterface $customer)
{
$flatArray = $this->extensibleDataObjectConverter->toNestedArray($customer);
$flatArray = $this->extensibleDataObjectConverter->toNestedArray($customer, [], '\Magento\Customer\Api\Data\CustomerInterface');
unset($flatArray["addresses"]);
return ConvertArray::toFlatArray($flatArray);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ public function save(\Magento\Customer\Api\Data\CustomerInterface $customer, $pa
{
$this->validate($customer);
$customerData = $this->extensibleDataObjectConverter->toFlatArray(
$this->customerBuilder->populate($customer)->setAddresses([])->create()
$this->customerBuilder->populate($customer)->setAddresses([])->create(),
[],
'\Magento\Customer\Api\Data\CustomerInterface'
);
$customerModel = $this->customerFactory->create(['data' => $customerData]);
$storeId = $customerModel->getStoreId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public function loadData($printQuery = false, $logQuery = false)
$groups = $searchResults->getItems();
foreach ($groups as $group) {
$groupItem = new \Magento\Framework\Object();
$groupItem->addData($this->simpleDataObjectConverter->toFlatArray($group));
$groupItem->addData($this->simpleDataObjectConverter->toFlatArray($group, '\Magento\Customer\Api\Data\GroupInterface'));
$this->_addItem($groupItem);
}
$this->_setIsLoaded();
Expand Down
17 changes: 17 additions & 0 deletions app/code/Magento/Integration/Model/Oauth/Consumer.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,17 @@ class Consumer extends \Magento\Framework\Model\AbstractModel implements Consume
*/
protected $_keyLengthFactory;

/**
* @var \Magento\Integration\Helper\Oauth\Data
*/
protected $dataHelper;

/**
* @param \Magento\Framework\Model\Context $context
* @param \Magento\Framework\Registry $registry
* @param \Magento\Integration\Model\Oauth\Consumer\Validator\KeyLengthFactory $keyLengthFactory
* @param \Magento\Framework\Url\Validator $urlValidator
* @param \Magento\Integration\Helper\Oauth\Data $dataHelper
* @param \Magento\Framework\Model\Resource\AbstractResource $resource
* @param \Magento\Framework\Data\Collection\Db $resourceCollection
* @param array $data
Expand All @@ -52,12 +58,14 @@ public function __construct(
\Magento\Framework\Registry $registry,
\Magento\Integration\Model\Oauth\Consumer\Validator\KeyLengthFactory $keyLengthFactory,
\Magento\Framework\Url\Validator $urlValidator,
\Magento\Integration\Helper\Oauth\Data $dataHelper,
\Magento\Framework\Model\Resource\AbstractResource $resource = null,
\Magento\Framework\Data\Collection\Db $resourceCollection = null,
array $data = []
) {
$this->_keyLengthFactory = $keyLengthFactory;
$this->_urlValidator = $urlValidator;
$this->dataHelper = $dataHelper;
parent::__construct($context, $registry, $resource, $resourceCollection, $data);
}

Expand Down Expand Up @@ -166,4 +174,13 @@ public function getCreatedAt()
{
return $this->getData('created_at');
}

/**
* {@inheritdoc}
*/
public function isValidForTokenExchange()
{
$expiry = $this->dataHelper->getConsumerExpirationPeriod();
return $expiry > $this->getResource()->getTimeInSecondsSinceCreation($this->getId());
}
}
22 changes: 2 additions & 20 deletions app/code/Magento/Integration/Model/Oauth/Token/Provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,16 @@ class Provider implements TokenProviderInterface
*/
protected $_tokenFactory;

/**
* @var \Magento\Integration\Helper\Oauth\Data
*/
protected $_dataHelper;

/**
* @var \Magento\Framework\Stdlib\DateTime\DateTime
*/
protected $_date;

/**
* @param \Magento\Integration\Model\Oauth\Consumer\Factory $consumerFactory
* @param \Magento\Integration\Model\Oauth\TokenFactory $tokenFactory
* @param \Magento\Integration\Helper\Oauth\Data $dataHelper
* @param \Magento\Framework\Stdlib\DateTime\DateTime $date
*/
public function __construct(
\Magento\Integration\Model\Oauth\Consumer\Factory $consumerFactory,
\Magento\Integration\Model\Oauth\TokenFactory $tokenFactory,
\Magento\Integration\Helper\Oauth\Data $dataHelper,
\Magento\Framework\Stdlib\DateTime\DateTime $date
\Magento\Integration\Model\Oauth\TokenFactory $tokenFactory
) {
$this->_consumerFactory = $consumerFactory;
$this->_tokenFactory = $tokenFactory;
$this->_dataHelper = $dataHelper;
$this->_date = $date;
}

/**
Expand All @@ -56,9 +40,7 @@ public function __construct(
public function validateConsumer($consumer)
{
// Must use consumer within expiration period.
$consumerTS = strtotime($consumer->getCreatedAt());
$expiry = $this->_dataHelper->getConsumerExpirationPeriod();
if ($this->_date->timestamp() - $consumerTS > $expiry) {
if (!$consumer->isValidForTokenExchange()) {
throw new \Magento\Framework\Oauth\Exception(
'Consumer key has expired'
);
Expand Down
18 changes: 18 additions & 0 deletions app/code/Magento/Integration/Model/Resource/Oauth/Consumer.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,22 @@ public function _afterDelete(\Magento\Framework\Model\AbstractModel $object)
$adapter->delete($this->getTable('oauth_token'), ['consumer_id' => $object->getId()]);
return parent::_afterDelete($object);
}

/**
* Compute time in seconds since consumer was created.
*
* @param int $consumerId - The consumer id
* @return int - time lapsed in seconds
*/
public function getTimeInSecondsSinceCreation($consumerId)
{
$adapter = $this->_getReadAdapter();
$select = $adapter->select()
->from($this->getMainTable())
->reset(\Zend_Db_Select::COLUMNS)
->columns('CURRENT_TIMESTAMP() - created_at')
->where('entity_id = ?', $consumerId);

return $adapter->fetchOne($select);
}
}
2 changes: 1 addition & 1 deletion app/code/Magento/Integration/etc/adminhtml/system.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
<label>Consumer Settings</label>
<field id="expiration_period" translate="label" type="text" sortOrder="30" showInDefault="1" showInWebsite="0" showInStore="0">
<label>Expiration Period</label>
<comment>Disable consumer key/secret credentials if not used within X seconds.</comment>
<comment>Consumer key/secret will expire if not used within X seconds after Oauth token exchange starts.</comment>
</field>
<field id="post_maxredirects" translate="label" type="text" sortOrder="30" showInDefault="1" showInWebsite="0" showInStore="0">
<label>OAuth consumer credentials HTTP Post maxredirects</label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public function getFormValues()
} catch (\Exception $e) {
/** If customer does not exist do nothing. */
}
$data = isset($customer) ? $this->_extensibleDataObjectConverter->toFlatArray($customer) : [];
$data = isset($customer) ? $this->_extensibleDataObjectConverter->toFlatArray($customer, [], '\Magento\Customer\Api\Data\CustomerInterface') : [];
foreach ($this->getQuote()->getData() as $key => $value) {
if (strpos($key, 'customer_') === 0) {
$data[substr($key, 9)] = $value;
Expand Down
4 changes: 3 additions & 1 deletion app/code/Magento/Sales/Model/Quote.php
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,9 @@ public function setCustomer(\Magento\Customer\Api\Data\CustomerInterface $custom
$this->setCustomerId($customer->getId());
$customerData = $this->objectFactory->create(
$this->extensibleDataObjectConverter->toFlatArray(
$this->customerBuilder->populate($customer)->setAddresses([])->create()
$this->customerBuilder->populate($customer)->setAddresses([])->create(),
[],
'\Magento\Customer\Api\Data\CustomerInterface'
)
);
$this->_objectCopyService->copyFieldsetToTarget('customer_account', 'to_quote', $customerData, $this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public function testGetAccessTokenConsumerMismatch()

/**
* @expectedException \Exception
* @expectedExceptionMessage HTTP/1.1 401
* @expectedExceptionMessage HTTP/1.1 400
*/
public function testAccessApiInvalidAccessToken()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ public function testSaveActionExistingGroup()
$simpleDataObjectConverter = Bootstrap::getObjectManager()
->get('Magento\Framework\Api\SimpleDataObjectConverter');
$customerGroupData = $simpleDataObjectConverter->toFlatArray(
$this->groupRepository->getById($groupId)
$this->groupRepository->getById($groupId),
'Magento\Customer\Api\Data\GroupInterface'
);
ksort($customerGroupData);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -599,8 +599,16 @@ public function testCreateNonexistingCustomer()
'aPassword',
true
);
$attributesBefore = $this->extensibleDataObjectConverter->toFlatArray($existingCustomer);
$attributesAfter = $this->extensibleDataObjectConverter->toFlatArray($customerAfter);
$attributesBefore = $this->extensibleDataObjectConverter->toFlatArray(
$existingCustomer,
[],
'\Magento\Customer\Api\Data\CustomerInterface'
);
$attributesAfter = $this->extensibleDataObjectConverter->toFlatArray(
$customerAfter,
[],
'\Magento\Customer\Api\Data\CustomerInterface'
);
// ignore 'updated_at'
unset($attributesBefore['updated_at']);
unset($attributesAfter['updated_at']);
Expand Down Expand Up @@ -670,7 +678,10 @@ public function testCreateCustomerInServiceVsInModel()
$simpleDataObjectConverter = Bootstrap::getObjectManager()
->get('Magento\Framework\Api\SimpleDataObjectConverter');

$dataInService = $simpleDataObjectConverter->toFlatArray($savedCustomer);
$dataInService = $simpleDataObjectConverter->toFlatArray(
$savedCustomer,
'Magento\Customer\Api\Data\CustomerInterface'
);
$expectedDifferences = [
'created_at',
'updated_at',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public function testGetCustomerAttributeMetadata()
'id' => 1,
'website_id' => 1,
'store_id' => 1,
'group_id' => '1',
'group_id' => 1,
'firstname' => 'John',
'lastname' => 'Smith',
'email' => 'customer@example.com',
Expand All @@ -132,7 +132,11 @@ public function testGetCustomerAttributeMetadata()
$customer = $this->customerRepository->getById(1);
$this->assertNotNull($customer);

$attributes = $this->_extensibleDataObjectConverter->toFlatArray($customer);
$attributes = $this->_extensibleDataObjectConverter->toFlatArray(
$customer,
[],
'\Magento\Customer\Api\Data\CustomerInterface'
);
$this->assertNotEmpty($attributes);

foreach ($attributes as $attributeCode => $attributeValue) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,16 @@ public function testUpdateCustomer()
$this->assertEquals('Admin', $customerAfter->getCreatedIn());
$passwordFromFixture = 'password';
$this->accountManagement->authenticate($customerAfter->getEmail(), $passwordFromFixture);
$attributesBefore = $this->converter->toFlatArray($customerBefore);
$attributesAfter = $this->converter->toFlatArray($customerAfter);
$attributesBefore = $this->converter->toFlatArray(
$customerBefore,
[],
'\Magento\Customer\Api\Data\CustomerInterface'
);
$attributesAfter = $this->converter->toFlatArray(
$customerAfter,
[],
'\Magento\Customer\Api\Data\CustomerInterface'
);
// ignore 'updated_at'
unset($attributesBefore['updated_at']);
unset($attributesAfter['updated_at']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function testGetProductWeeeAttributes()
['metadataService' => $customerMetadataService]
);
$expected = $this->_extensibleDataObjectConverter->toFlatArray(
$customerRepository->getById(1)
$customerRepository->getById(1), [], '\Magento\Customer\Api\Data\CustomerInterface'
);
$customerBuilder->populateWithArray($expected);
$customerDataSet = $customerBuilder->create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ private function _setupStoreMode($customerData, $isSingleStoreMode, $canModifyCu
'adminhtml_customer',
$this->extensibleDataObjectConverterMock->toFlatArray(
$customerObject,
[],
'\Magento\Customer\Api\Data\CustomerInterface'
)
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php
/**
* Copyright © 2015 Magento. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Framework\Oauth;

class OauthInputExceptionTest extends \PHPUnit_Framework_TestCase
{
public function testGetAggregatedErrorMessage()
{
$exception = new OauthInputException();
foreach (['field1', 'field2'] as $param) {
$exception->addError(OauthInputException::REQUIRED_FIELD, ['fieldName' => $param]);
}
$exception->addError('Message with period.');

$this->assertEquals(
'field1 is a required field, field2 is a required field, Message with period',
$exception->getAggregatedErrorMessage()
);
}

public function testGetAggregatedErrorMessageNoError()
{
$exception = new OauthInputException();
$this->assertEquals('', $exception->getAggregatedErrorMessage());
}
}
Loading

0 comments on commit 7581dd6

Please sign in to comment.