From 03659c69df47bd16d82b6884f90d2542c0fb7f29 Mon Sep 17 00:00:00 2001 From: Bojan Zivanovic Date: Thu, 4 Oct 2018 17:37:05 +0200 Subject: [PATCH] Issue #2995325 by bojanz, mglaman: Order::getCustomer() should guard against deleted users --- modules/order/src/Entity/Order.php | 17 +++++++++++++--- modules/order/src/Entity/OrderInterface.php | 10 ++++++---- .../OrderReceiptSubscriber.php | 3 ++- modules/order/src/Form/OrderForm.php | 3 ++- modules/order/src/Form/OrderReassignForm.php | 4 ++-- modules/order/src/OrderAssignment.php | 2 +- .../Commerce/Condition/OrderCustomerRole.php | 3 +-- .../FieldWidget/BillingProfileWidget.php | 2 +- .../tests/src/Kernel/Entity/OrderTest.php | 20 ++++++++++++++++--- .../Condition/OrderCustomerRoleTest.php | 19 +----------------- modules/payment/src/Form/PaymentAddForm.php | 2 +- modules/payment/src/PaymentOptionsBuilder.php | 2 +- 12 files changed, 49 insertions(+), 38 deletions(-) diff --git a/modules/order/src/Entity/Order.php b/modules/order/src/Entity/Order.php index df3d896c0d..7427531585 100644 --- a/modules/order/src/Entity/Order.php +++ b/modules/order/src/Entity/Order.php @@ -9,6 +9,7 @@ use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\BaseFieldDefinition; +use Drupal\user\Entity\User; use Drupal\user\UserInterface; use Drupal\profile\Entity\ProfileInterface; @@ -120,7 +121,12 @@ public function setStoreId($store_id) { * {@inheritdoc} */ public function getCustomer() { - return $this->get('uid')->entity; + $customer = $this->get('uid')->entity; + // Handle deleted customers. + if (!$customer) { + $customer = User::getAnonymousUser(); + } + return $customer; } /** @@ -516,10 +522,15 @@ public function preSave(EntityStorageInterface $storage) { } } - if (!$this->getEmail() && $customer = $this->getCustomer()) { + $customer = $this->getCustomer(); + // The customer has been deleted, clear the reference. + if ($this->getCustomerId() && $customer->isAnonymous()) { + $this->set('uid', 0); + } + // Maintain the order email. + if (!$this->getEmail() && $customer->isAuthenticated()) { $this->setEmail($customer->getEmail()); } - // Maintain the completed timestamp. $state = $this->getState()->value; $original_state = isset($this->original) ? $this->original->getState()->value : ''; diff --git a/modules/order/src/Entity/OrderInterface.php b/modules/order/src/Entity/OrderInterface.php index 48ee9ee12d..84a9621057 100644 --- a/modules/order/src/Entity/OrderInterface.php +++ b/modules/order/src/Entity/OrderInterface.php @@ -76,8 +76,10 @@ public function setStoreId($store_id); /** * Gets the customer user. * - * @return \Drupal\user\UserInterface|null - * The customer user entity, or NULL in case the order is anonymous, + * @return \Drupal\user\UserInterface + * The customer user entity. If the order is anonymous (customer + * unspecified or deleted), an anonymous user will be returned. Use + * $customer->isAnonymous() to check. */ public function getCustomer(); @@ -94,8 +96,8 @@ public function setCustomer(UserInterface $account); /** * Gets the customer user ID. * - * @return int|null - * The customer user ID, or NULL in case the order is anonymous. + * @return int + * The customer user ID ('0' if anonymous). */ public function getCustomerId(); diff --git a/modules/order/src/EventSubscriber/OrderReceiptSubscriber.php b/modules/order/src/EventSubscriber/OrderReceiptSubscriber.php index 850e336341..442799f819 100644 --- a/modules/order/src/EventSubscriber/OrderReceiptSubscriber.php +++ b/modules/order/src/EventSubscriber/OrderReceiptSubscriber.php @@ -130,7 +130,8 @@ public function sendOrderReceipt(WorkflowTransitionEvent $event) { }); // Replicated logic from EmailAction and contact's MailHandler. - if ($customer = $order->getCustomer()) { + $customer = $order->getCustomer(); + if ($customer->isAuthenticated()) { $langcode = $customer->getPreferredLangcode(); } else { diff --git a/modules/order/src/Form/OrderForm.php b/modules/order/src/Form/OrderForm.php index 66099e74d1..473e771ac4 100644 --- a/modules/order/src/Form/OrderForm.php +++ b/modules/order/src/Form/OrderForm.php @@ -117,10 +117,11 @@ public function form(array $form, FormStateInterface $form_state) { $form['meta']['store'] = $this->fieldAsReadOnly($this->t('Store'), $store_link); } // Move uid/mail widgets to the sidebar, or provide read-only alternatives. + $customer = $order->getCustomer(); if (isset($form['uid'])) { $form['uid']['#group'] = 'customer'; } - elseif ($customer = $order->getCustomer()) { + elseif ($customer->isAuthenticated()) { $customer_link = $customer->toLink()->toString(); $form['customer']['uid'] = $this->fieldAsReadOnly($this->t('Customer'), $customer_link); } diff --git a/modules/order/src/Form/OrderReassignForm.php b/modules/order/src/Form/OrderReassignForm.php index 85b8c5452d..e445b8c7b8 100644 --- a/modules/order/src/Form/OrderReassignForm.php +++ b/modules/order/src/Form/OrderReassignForm.php @@ -49,13 +49,13 @@ public function getFormId() { * {@inheritdoc} */ public function buildForm(array $form, FormStateInterface $form_state) { - if (!$this->order->getCustomerId()) { + $customer = $this->order->getCustomer(); + if ($customer->isAnonymous()) { $current_customer = $this->t('anonymous user with the email %email', [ '%email' => $this->order->getEmail(), ]); } else { - $customer = $this->order->getCustomer(); // If the display name has been altered to not be the email address, // show the email as well. if ($customer->getDisplayName() != $customer->getEmail()) { diff --git a/modules/order/src/OrderAssignment.php b/modules/order/src/OrderAssignment.php index 29b0903a6e..d2fce2620e 100644 --- a/modules/order/src/OrderAssignment.php +++ b/modules/order/src/OrderAssignment.php @@ -42,7 +42,7 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager, Eve * {@inheritdoc} */ public function assign(OrderInterface $order, UserInterface $account) { - if (!empty($order->getCustomerId())) { + if ($order->getCustomer()->isAuthenticated()) { // Skip orders which already have a customer. return; } diff --git a/modules/order/src/Plugin/Commerce/Condition/OrderCustomerRole.php b/modules/order/src/Plugin/Commerce/Condition/OrderCustomerRole.php index e5de7b9d7e..db0dce50c1 100644 --- a/modules/order/src/Plugin/Commerce/Condition/OrderCustomerRole.php +++ b/modules/order/src/Plugin/Commerce/Condition/OrderCustomerRole.php @@ -63,9 +63,8 @@ public function evaluate(EntityInterface $entity) { /** @var \Drupal\commerce_order\Entity\OrderInterface $order */ $order = $entity; $customer = $order->getCustomer(); - $roles = $customer ? $customer->getRoles() : ['anonymous']; - return (bool) array_intersect($this->configuration['roles'], $roles); + return (bool) array_intersect($this->configuration['roles'], $customer->getRoles()); } } diff --git a/modules/order/src/Plugin/Field/FieldWidget/BillingProfileWidget.php b/modules/order/src/Plugin/Field/FieldWidget/BillingProfileWidget.php index 5fe768c1a1..1bc6e73ad1 100644 --- a/modules/order/src/Plugin/Field/FieldWidget/BillingProfileWidget.php +++ b/modules/order/src/Plugin/Field/FieldWidget/BillingProfileWidget.php @@ -81,7 +81,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen else { $profile = $this->entityTypeManager->getStorage('profile')->create([ 'type' => 'customer', - 'uid' => $order->getCustomerId(), + 'uid' => $order->getCustomer(), ]); } diff --git a/modules/order/tests/src/Kernel/Entity/OrderTest.php b/modules/order/tests/src/Kernel/Entity/OrderTest.php index 3607333ba8..b2d5a41e73 100644 --- a/modules/order/tests/src/Kernel/Entity/OrderTest.php +++ b/modules/order/tests/src/Kernel/Entity/OrderTest.php @@ -10,6 +10,7 @@ use Drupal\commerce_price\Price; use Drupal\profile\Entity\Profile; use Drupal\Tests\commerce\Kernel\CommerceKernelTestBase; +use Drupal\user\UserInterface; /** * Tests the Order entity. @@ -135,6 +136,7 @@ public function testOrder() { $another_order_item->save(); $another_order_item = $this->reloadEntity($another_order_item); + /** @var \Drupal\commerce_order\Entity\OrderInterface $order */ $order = Order::create([ 'type' => 'default', 'state' => 'completed', @@ -149,15 +151,22 @@ public function testOrder() { $this->assertEquals($this->store->id(), $order->getStoreId()); $order->setStoreId(0); $this->assertEquals(NULL, $order->getStore()); - $order->setStoreId([$this->store->id()]); + $order->setStoreId($this->store->id()); $this->assertEquals($this->store, $order->getStore()); $this->assertEquals($this->store->id(), $order->getStoreId()); + $this->assertInstanceOf(UserInterface::class, $order->getCustomer()); + $this->assertTrue($order->getCustomer()->isAnonymous()); + $this->assertEquals(0, $order->getCustomerId()); $order->setCustomer($this->user); $this->assertEquals($this->user, $order->getCustomer()); $this->assertEquals($this->user->id(), $order->getCustomerId()); - $order->setCustomerId(0); - $this->assertEquals(NULL, $order->getCustomer()); + $this->assertTrue($order->getCustomer()->isAuthenticated()); + // Non-existent/deleted user ID. + $order->setCustomerId(888); + $this->assertInstanceOf(UserInterface::class, $order->getCustomer()); + $this->assertTrue($order->getCustomer()->isAnonymous()); + $this->assertEquals(888, $order->getCustomerId()); $order->setCustomerId($this->user->id()); $this->assertEquals($this->user, $order->getCustomer()); $this->assertEquals($this->user->id(), $order->getCustomerId()); @@ -241,6 +250,11 @@ public function testOrder() { $order->setCompletedTime(635879900); $this->assertEquals(635879900, $order->getCompletedTime()); + + // Confirm that saving the order clears an invalid customer ID. + $order->setCustomerId(888); + $order->save(); + $this->assertEquals(0, $order->getCustomerId()); } /** diff --git a/modules/order/tests/src/Unit/Plugin/Commerce/Condition/OrderCustomerRoleTest.php b/modules/order/tests/src/Unit/Plugin/Commerce/Condition/OrderCustomerRoleTest.php index 143bfccad4..10bb07512c 100644 --- a/modules/order/tests/src/Unit/Plugin/Commerce/Condition/OrderCustomerRoleTest.php +++ b/modules/order/tests/src/Unit/Plugin/Commerce/Condition/OrderCustomerRoleTest.php @@ -16,24 +16,7 @@ class OrderCustomerRoleTest extends UnitTestCase { /** * ::covers evaluate. */ - public function testAnonymousCustomer() { - $condition = new OrderCustomerRole([ - 'roles' => ['authenticated'], - ], 'order_customer_role', ['entity_type' => 'commerce_order']); - $order = $this->prophesize(OrderInterface::class); - $order->getEntityTypeId()->willReturn('commerce_order'); - $order->getCustomer()->willReturn(NULL); - $order = $order->reveal(); - - $this->assertFalse($condition->evaluate($order)); - $condition->setConfiguration(['roles' => ['anonymous', 'authenticated']]); - $this->assertTrue($condition->evaluate($order)); - } - - /** - * ::covers evaluate. - */ - public function testAuthenticatedCustomer() { + public function testEvaluate() { $condition = new OrderCustomerRole([ 'roles' => ['merchant'], ], 'order_customer_role', ['entity_type' => 'commerce_order']); diff --git a/modules/payment/src/Form/PaymentAddForm.php b/modules/payment/src/Form/PaymentAddForm.php index 9699e184b7..02f7305cd1 100644 --- a/modules/payment/src/Form/PaymentAddForm.php +++ b/modules/payment/src/Form/PaymentAddForm.php @@ -101,7 +101,7 @@ protected function buildPaymentGatewayForm(array $form, FormStateInterface $form // @todo // Support adding payments to anonymous orders, by adding support for // creating payment methods directly on this form. - if (!$this->order->getCustomerId()) { + if ($this->order->getCustomer()->isAnonymous()) { throw new AccessDeniedHttpException(); } diff --git a/modules/payment/src/PaymentOptionsBuilder.php b/modules/payment/src/PaymentOptionsBuilder.php index b2a943b0dd..d58864a267 100644 --- a/modules/payment/src/PaymentOptionsBuilder.php +++ b/modules/payment/src/PaymentOptionsBuilder.php @@ -51,7 +51,7 @@ public function buildOptions(OrderInterface $order, array $payment_gateways = [] $options = []; // 1) Add options to reuse stored payment methods for known customers. $customer = $order->getCustomer(); - if ($customer) { + if ($customer->isAuthenticated()) { $billing_countries = $order->getStore()->getBillingCountries(); /** @var \Drupal\commerce_payment\PaymentMethodStorageInterface $payment_method_storage */ $payment_method_storage = $this->entityTypeManager->getStorage('commerce_payment_method');