Merge pull request #3593 from magento-qwerty/2.3-bugfixes-020119

Fixed issues:
- MAGETWO-95945: Add a code mess rule for improper session and cookies usages
- MAGETWO-95928: Remove RequestAwareBlockMethod and update Technical Vision

app/code/Magento/Widget/Block/Adminhtml/Widget.php

@@ -16,8 +16,6 @@ class Widget extends \Magento\Backend\Block\Widget\Form\Container
 {
     /**
      * @inheritdoc
-     *
-     * @SuppressWarnings(PHPMD.RequestAwareBlockMethod)
      */
     protected function _construct()
     {

dev/tests/static/framework/Magento/CodeMessDetector/Rule/Design/CookieAndSessionMisuse.php

@@ -0,0 +1,184 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+
+namespace Magento\CodeMessDetector\Rule\Design;
+
+use PDepend\Source\AST\ASTClass;
+use PHPMD\AbstractNode;
+use PHPMD\AbstractRule;
+use PHPMD\Node\ClassNode;
+use PHPMD\Rule\ClassAware;
+
+/**
+ * Session and Cookies must be used only in HTML Presentation layer.
+ */
+class CookieAndSessionMisuse extends AbstractRule implements ClassAware
+{
+    /**
+     * Is given class a controller?
+     *
+     * @param \ReflectionClass $class
+     * @return bool
+     */
+    private function isController(\ReflectionClass $class): bool
+    {
+        return $class->isSubclassOf(\Magento\Framework\App\ActionInterface::class);
+    }
+
+    /**
+     * Is given class a block?
+     *
+     * @param \ReflectionClass $class
+     * @return bool
+     */
+    private function isBlock(\ReflectionClass $class): bool
+    {
+        return $class->isSubclassOf(\Magento\Framework\View\Element\BlockInterface::class);
+    }
+
+    /**
+     * Is given class an HTML UI data provider?
+     *
+     * @param \ReflectionClass $class
+     * @return bool
+     */
+    private function isUiDataProvider(\ReflectionClass $class): bool
+    {
+        return $class->isSubclassOf(
+            \Magento\Framework\View\Element\UiComponent\DataProvider\DataProviderInterface::class
+        );
+    }
+
+    /**
+     * Is given class an HTML UI Document?
+     *
+     * @param \ReflectionClass $class
+     * @return bool
+     */
+    private function isUiDocument(\ReflectionClass $class): bool
+    {
+        return $class->isSubclassOf(\Magento\Framework\View\Element\UiComponent\DataProvider\Document::class)
+            || $class->getName() === \Magento\Framework\View\Element\UiComponent\DataProvider\Document::class;
+    }
+
+    /**
+     * Is given class a plugin for controllers?
+     *
+     * @param \ReflectionClass $class
+     * @return bool
+     */
+    private function isControllerPlugin(\ReflectionClass $class): bool
+    {
+        foreach ($class->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) {
+            if (preg_match('/^(after|around|before).+/i', $method->getName())) {
+                try {
+                    $argument = $method->getParameters()[0]->getClass();
+                } catch (\Throwable $exception) {
+                    //Non-existing class (autogenerated perhaps) or doesn't have an argument.
+                    continue;
+                }
+                if ($argument) {
+                    $isAction = $argument->isSubclassOf(\Magento\Framework\App\ActionInterface::class)
+                        || $argument->getName() === \Magento\Framework\App\ActionInterface::class;
+                    if ($isAction) {
+                        return true;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Is given class a plugin for blocks?
+     *
+     * @param \ReflectionClass $class
+     * @return bool
+     */
+    private function isBlockPlugin(\ReflectionClass $class): bool
+    {
+        foreach ($class->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) {
+            if (preg_match('/^(after|around|before).+/i', $method->getName())) {
+                try {
+                    $argument = $method->getParameters()[0]->getClass();
+                } catch (\Throwable $exception) {
+                    //Non-existing class (autogenerated perhaps) or doesn't have an argument.
+                    continue;
+                }
+                if ($argument) {
+                    $isBlock = $argument->isSubclassOf(\Magento\Framework\View\Element\BlockInterface::class)
+                        || $argument->getName() === \Magento\Framework\View\Element\BlockInterface::class;
+                    if ($isBlock) {
+                        return true;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Whether given class depends on classes to pay attention to.
+     *
+     * @param \ReflectionClass $class
+     * @return bool
+     */
+    private function doesUseRestrictedClasses(\ReflectionClass $class): bool
+    {
+        $constructor = $class->getConstructor();
+        if ($constructor) {
+            foreach ($constructor->getParameters() as $argument) {
+                try {
+                    if ($class = $argument->getClass()) {
+                        if ($class->isSubclassOf(\Magento\Framework\Session\SessionManagerInterface::class)
+                            || $class->getName() === \Magento\Framework\Session\SessionManagerInterface::class
+                            || $class->isSubclassOf(\Magento\Framework\Stdlib\Cookie\CookieReaderInterface::class)
+                            || $class->getName() === \Magento\Framework\Stdlib\Cookie\CookieReaderInterface::class
+                        ) {
+                            return true;
+                        }
+                    }
+                } catch (\ReflectionException $exception) {
+                    //Failed to load the argument's class information
+                    continue;
+                }
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * @inheritdoc
+     *
+     * @param ClassNode|ASTClass $node
+     */
+    public function apply(AbstractNode $node)
+    {
+        try {
+            $class = new \ReflectionClass($node->getFullQualifiedName());
+        } catch (\Throwable $exception) {
+            //Failed to load class, nothing we can do
+            return;
+        }
+
+        if ($this->doesUseRestrictedClasses($class)) {
+            if (!$this->isController($class)
+                && !$this->isBlock($class)
+                && !$this->isUiDataProvider($class)
+                && !$this->isUiDocument($class)
+                && !$this->isControllerPlugin($class)
+                && !$this->isBlockPlugin($class)
+            ) {
+                $this->addViolation($node, [$node->getFullQualifiedName()]);
+            }
+        }
+    }
+}

dev/tests/static/framework/Magento/CodeMessDetector/resources/rulesets/design.xml

@@ -58,32 +58,30 @@ class PostOrder implements ActionInterface
             ]]>
         </example>
     </rule>
-    <rule name="RequestAwareBlockMethod"
-          class="Magento\CodeMessDetector\Rule\Design\RequestAwareBlockMethod"
-          message="{0} uses request object directly. Add user input validation and suppress this warning.">
+    <rule name="CookieAndSessionMisuse"
+          class="Magento\CodeMessDetector\Rule\Design\CookieAndSessionMisuse"
+          message= "The class {0} uses sessions or cookies while not being a part of HTML Presentation layer">
         <description>
             <![CDATA[
-Blocks must not depend on being used with certain controllers.
-If you use request object in a block directly you must validate all user input inside the block.
+Sessions and cookies must only be used in classes directly responsible for HTML presentation because Web APIs do not
+rely on cookies and sessions. If you need to get current user use Magento\Authorization\Model\UserContextInterface
             ]]>
         </description>
         <priority>2</priority>
         <properties />
         <example>
             <![CDATA[
-class MyOrder extends AbstractBlock
+class OrderProcessor
 {
+    public function __construct(SessionManagerInterface $session) {
+        $this->session = $session;
+    }
 
-    .......
-
-    public function getOrder()
+    public function place(OrderInterface $order)
     {
-        $orderId = $this->getRequest()->getParam('order_id');
-        //Validate customer having such order.
-        if (!$this->hasOrder($this->getCustomerId(), $orderId)) {
-            ...deny access...
-        }
-        .....
+        //Will not be present if processing a WebAPI request
+        $currentOrder = $this->session->get('current_order');
+        ...
     }
 }
             ]]>

dev/tests/static/testsuite/Magento/Test/Php/_files/phpmd/ruleset.xml

@@ -48,6 +48,6 @@
     <!-- Magento Specific Rules -->
     <rule ref="Magento/CodeMessDetector/resources/rulesets/design.xml/FinalImplementation" />
     <rule ref="Magento/CodeMessDetector/resources/rulesets/design.xml/AllPurposeAction" />
-    <rule ref="Magento/CodeMessDetector/resources/rulesets/design.xml/RequestAwareBlockMethod" />
+    <rule ref="Magento/CodeMessDetector/resources/rulesets/design.xml/CookieAndSessionMisuse" />
 
 </ruleset>