Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ public function resolve(
throw new GraphQlInputException(__('"id" value should be specified'));
}

$orderId = $this->uidEncoder->decode((string) $this->uidEncoder->encode((string) $value['id']));
// phpcs:ignore Magento2.Functions.DiscouragedFunction
$orderId = (int)base64_decode($value['id']) ?: (int)$value['id'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think that we must be more careful with values provided from FE.

<?php

declare(strict_types=1);

echo "\nit's wrong (string)`1 hi all!` !== (int)`1`\n";
$a = base64_encode('1 hi all!');
var_dump((int)base64_decode($a));


echo "\nit's wrong (string)`1 hi all!` !== (int)`1`\n";
var_dump((int) '1 hi all!');

https://3v4l.org/dAvUu

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rvitaliy
I am sorry but I am not sure that i understand your remark.
Yes, I understood problem which you provided in example but i can't get how it is related with this case in the code.
Perhaps you could explain your point ?
Thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

he means if we trust that $value['id'] is a trusted value
and he explained a way to exploit such a value and how it can be misleading using base64_decode

if you wouldn't add this empty line we would of not seen this :)
we actually have a safer way to do this \Magento\Framework\GraphQl\Query\Uid::decode
you can use it if you want, but it's not really the purpose of this PR


try {
$orderGiftMessage = $this->orderRepository->get($orderId);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\GiftMessageGraphQl\Model\Resolver\Order\Item;

use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\GraphQl\Config\Element\Field;
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;
use Magento\Framework\GraphQl\Query\Resolver\Value;
use Magento\Framework\GraphQl\Query\ResolverInterface;
use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
use Magento\GiftMessage\Api\OrderItemRepositoryInterface;
use Magento\GiftMessage\Helper\Message as GiftMessageHelper;

/**
* Class provides ability to get GiftMessage for order item
*/
class GiftMessage implements ResolverInterface
{
/**
* @var OrderItemRepositoryInterface
*/
private $orderItemRepository;

/**
* @var GiftMessageHelper
*/
private $giftMessageHelper;

/**
* @param OrderItemRepositoryInterface $itemRepository
* @param GiftMessageHelper $giftMessageHelper
*/
public function __construct(
OrderItemRepositoryInterface $itemRepository,
GiftMessageHelper $giftMessageHelper
) {
$this->orderItemRepository = $itemRepository;
$this->giftMessageHelper = $giftMessageHelper;
}

/**
* Return information about Gift message for order item
*
* @param Field $field
* @param ContextInterface $context
* @param ResolveInfo $info
* @param array|null $value
* @param array|null $args
*
* @return array|Value|mixed
* @throws GraphQlInputException
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function resolve(
Field $field,
$context,
ResolveInfo $info,
array $value = null,
array $args = null
) {
if (!isset($value['model'])) {
throw new GraphQlInputException(__('"model" value must be specified'));
}

$orderItem = $value['model'];

if (!$this->giftMessageHelper->isMessagesAllowed('items', $orderItem)) {
return null;
}

if (!$this->giftMessageHelper->isMessagesAllowed('item', $orderItem)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to double-check we really need this part. If we calling isMessageAllowed with type = "items", the second invocation (with type = "item") will be performed eventually. See the following implementation

if ($this->isMessagesAllowed($_type, $item, $store)) {

Please, correct me if I'm wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rogyar
isMessageAllowed with type = "item" need for checking isMessageAllowed on product level because we can have situation where GiftMessages are enable in configs but disable for some products
Thanks

return null;
}

try {
$giftItemMessage = $this->orderItemRepository->get($orderItem->getOrderId(), $orderItem->getItemId());
} catch (LocalizedException $e) {
throw new GraphQlInputException(__('Can\'t load message for order item'));
}

if ($giftItemMessage === null) {
return null;
}

return [
'to' => $giftItemMessage->getRecipient() ?? '',
'from' => $giftItemMessage->getSender() ?? '',
'message'=> $giftItemMessage->getMessage() ?? ''
];
}
}
4 changes: 4 additions & 0 deletions app/code/Magento/GiftMessageGraphQl/etc/schema.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,7 @@ type SalesItemInterface {
type CustomerOrder {
gift_message: GiftMessage @resolver (class: "\\Magento\\GiftMessageGraphQl\\Model\\Resolver\\Order\\GiftMessage") @doc(description: "The entered gift message for the order")
}

interface OrderItemInterface {
gift_message: GiftMessage @resolver(class: "\\Magento\\GiftMessageGraphQl\\Model\\Resolver\\Order\\Item\\GiftMessage") @doc(description: "The selected gift message for the order item")
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,44 @@ public function testGiftMessageForOrder()
}
}

/**
* @magentoConfigFixture default_store sales/gift_options/allow_order 1
* @magentoConfigFixture default_store sales/gift_options/allow_items 1
* @magentoApiDataFixture Magento/Customer/_files/customer.php
* @magentoApiDataFixture Magento/GiftMessage/_files/customer/order_with_message.php
* @throws AuthenticationException
* @throws Exception
*/
public function testGiftMessageForCustomerOrder()
{
$query = <<<QUERY
query {
customer {
orders(filter:{number:{eq:"999999991"}}){
total_count
items
{
gift_message {
from
to
message
}
}
}
}
}
QUERY;
$currentEmail = 'customer@example.com';
$currentPassword = 'password';
$response = $this->graphQlQuery($query, [], '', $this->getCustomerAuthHeaders($currentEmail, $currentPassword));
foreach ($response['customer']['orders']['items'] as $order) {
self::assertArrayHasKey('gift_message', $order);
self::assertSame('Jane Roe', $order['gift_message']['to']);
self::assertSame('John Doe', $order['gift_message']['from']);
self::assertSame('Gift Message Text', $order['gift_message']['message']);
}
}

/**
* @magentoConfigFixture default_store sales/gift_options/allow_order 0
* @magentoConfigFixture default_store sales/gift_options/allow_items 0
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\GraphQl\GiftMessage\Order\Item;

use Exception;
use Magento\Framework\Exception\AuthenticationException;
use Magento\Integration\Api\CustomerTokenServiceInterface;
use Magento\TestFramework\Helper\Bootstrap;
use Magento\TestFramework\TestCase\GraphQlAbstract;

class GiftMessageTest extends GraphQlAbstract
{
/**
* @var CustomerTokenServiceInterface
*/
private $customerTokenService;

protected function setUp(): void
{
parent::setUp();
$this->customerTokenService = Bootstrap::getObjectManager()->get(CustomerTokenServiceInterface::class);
}

/**
* @magentoConfigFixture default_store sales/gift_options/allow_order 1
* @magentoConfigFixture default_store sales/gift_options/allow_items 1
* @magentoApiDataFixture Magento/Customer/_files/customer.php
* @magentoApiDataFixture Magento/GiftMessage/_files/customer/order_with_message.php
* @throws AuthenticationException
* @throws Exception
*/
public function testGiftMessageForOrderItem()
{
$query = <<<QUERY
query {
customer {
orders(filter:{number:{eq:"999999991"}}){
total_count
items {
id
items{
gift_message {
from
to
message
}
}
gift_message {
from
to
message
}
}
}
}
}
QUERY;
$currentEmail = 'customer@example.com';
$currentPassword = 'password';
$response = $this->graphQlQuery($query, [], '', $this->getCustomerAuthHeaders($currentEmail, $currentPassword));
foreach ($response['customer']['orders']['items'][0]['items'] as $item) {
self::assertArrayHasKey('gift_message', $item);
self::assertSame('Luci', $item['gift_message']['to']);
self::assertSame('Jack', $item['gift_message']['from']);
self::assertSame('Good Job!', $item['gift_message']['message']);
}
}

/**
* @param string $email
* @param string $password
* @return array
* @throws AuthenticationException
*/
private function getCustomerAuthHeaders(string $email, string $password): array
{
$customerToken = $this->customerTokenService->createCustomerAccessToken($email, $password);
return ['Authorization' => 'Bearer ' . $customerToken];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,25 @@
$messageModel->setMessage('Gift Message Text');
$message->save($messageModel);

/** @var MessageResource $productMessage */
$productMessage = $objectManager->create(MessageResource::class);
/** @var Message $productMessageModel */
$productMessageModel = $objectManager->create(Message::class);

$productMessageModel->setSender('Jack');
$productMessageModel->setRecipient('Luci');
$productMessageModel->setMessage('Good Job!');
$productMessage->save($productMessageModel);

/** @var Order\Item $orderItem */
$orderItem = $objectManager->create(Order\Item::class);
$orderItem->setProductId($product->getId())
->setQtyOrdered(2)
->setBasePrice($product->getPrice())
->setPrice($product->getPrice())
->setRowTotal($product->getPrice())
->setProductType('simple');
->setProductType('simple')
->setGiftMessageId($productMessageModel->getId());

$order
->setData($orderData)
Expand Down