Skip to content

Commit

Permalink
[EngCom] Public Pull Requests - 2.3-develop
Browse files Browse the repository at this point in the history
 - merged latest code from mainline branch
  • Loading branch information
magento-engcom-team authored Jan 15, 2019
2 parents 30ce26d + ec25977 commit 458fceb
Show file tree
Hide file tree
Showing 5 changed files with 198 additions and 71 deletions.
2 changes: 0 additions & 2 deletions app/code/Magento/Widget/Block/Adminhtml/Widget.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ class Widget extends \Magento\Backend\Block\Widget\Form\Container
{
/**
* @inheritdoc
*
* @SuppressWarnings(PHPMD.RequestAwareBlockMethod)
*/
protected function _construct()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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()]);
}
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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');
...
}
}
]]>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>

0 comments on commit 458fceb

Please sign in to comment.