Skip to content

Commit aeaaec5

Browse files
author
He, Joan(johe)
committed
Merge pull request #336 from magento-extensibility/develop
[Extensibility] Sprint 52
2 parents fca00a7 + 81aca8e commit aeaaec5

File tree

10 files changed

+203
-57
lines changed

10 files changed

+203
-57
lines changed

‎app/code/Magento/Backend/App/AbstractAction.php

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ abstract class AbstractAction extends \Magento\Framework\App\Action\Action
2222
*/
2323
const SESSION_NAMESPACE = 'adminhtml';
2424

25+
/**
26+
* Authorization level of a basic admin session
27+
*/
28+
const ADMIN_RESOURCE = 'Magento_Backend::admin';
29+
2530
/**
2631
* Array of actions which can be processed without secret key validation
2732
*
@@ -97,7 +102,7 @@ public function __construct(Action\Context $context)
97102
*/
98103
protected function _isAllowed()
99104
{
100-
return true;
105+
return $this->_authorization->isAllowed(self::ADMIN_RESOURCE);
101106
}
102107

103108
/**
@@ -228,14 +233,10 @@ public function dispatch(\Magento\Framework\App\RequestInterface $request)
228233
*/
229234
protected function _isUrlChecked()
230235
{
231-
return !$this->_actionFlag->get(
232-
'',
233-
self::FLAG_IS_URLS_CHECKED
234-
) && !$this->getRequest()->getParam(
235-
'forwarded'
236-
) && !$this->_getSession()->getIsUrlNotice(
237-
true
238-
) && !$this->_canUseBaseUrl;
236+
return !$this->_actionFlag->get('', self::FLAG_IS_URLS_CHECKED)
237+
&& !$this->getRequest()->isForwarded()
238+
&& !$this->_getSession()->getIsUrlNotice(true)
239+
&& !$this->_canUseBaseUrl;
239240
}
240241

241242
/**

‎app/code/Magento/Backend/App/Action/Plugin/Authentication.php

Lines changed: 16 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -147,46 +147,25 @@ protected function _processNotLoggedInUser(\Magento\Framework\App\RequestInterfa
147147
if ($request->getPost('login') && $this->_performLogin($request)) {
148148
$isRedirectNeeded = $this->_redirectIfNeededAfterLogin($request);
149149
}
150-
if (!$isRedirectNeeded && !$request->getParam('forwarded')) {
150+
if (!$isRedirectNeeded && !$request->isForwarded()) {
151151
if ($request->getParam('isIframe')) {
152-
$request->setParam(
153-
'forwarded',
154-
true
155-
)->setRouteName(
156-
'adminhtml'
157-
)->setControllerName(
158-
'auth'
159-
)->setActionName(
160-
'deniedIframe'
161-
)->setDispatched(
162-
false
163-
);
152+
$request->setForwarded(true)
153+
->setRouteName('adminhtml')
154+
->setControllerName('auth')
155+
->setActionName('deniedIframe')
156+
->setDispatched(false);
164157
} elseif ($request->getParam('isAjax')) {
165-
$request->setParam(
166-
'forwarded',
167-
true
168-
)->setRouteName(
169-
'adminhtml'
170-
)->setControllerName(
171-
'auth'
172-
)->setActionName(
173-
'deniedJson'
174-
)->setDispatched(
175-
false
176-
);
158+
$request->setForwarded(true)
159+
->setRouteName('adminhtml')
160+
->setControllerName('auth')
161+
->setActionName('deniedJson')
162+
->setDispatched(false);
177163
} else {
178-
$request->setParam(
179-
'forwarded',
180-
true
181-
)->setRouteName(
182-
'adminhtml'
183-
)->setControllerName(
184-
'auth'
185-
)->setActionName(
186-
'login'
187-
)->setDispatched(
188-
false
189-
);
164+
$request->setForwarded(true)
165+
->setRouteName('adminhtml')
166+
->setControllerName('auth')
167+
->setActionName('login')
168+
->setDispatched(false);
190169
}
191170
}
192171
}

‎app/code/Magento/Backend/Test/Unit/App/Action/Plugin/AuthenticationTest.php

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,78 @@ public function testAroundDispatchProlongStorage()
8484

8585
$this->assertEquals($expectedResult, $this->plugin->aroundDispatch($subject, $proceed, $request));
8686
}
87+
88+
/**
89+
* Calls aroundDispatch to access protected method _processNotLoggedInUser
90+
*
91+
* Data provider supplies different possibilities of request parameters and properties
92+
* @dataProvider processNotLoggedInUserDataProvider
93+
*/
94+
public function testProcessNotLoggedInUser($isIFrameParam, $isAjaxParam, $isForwardedFlag)
95+
{
96+
$subject = $this->getMockBuilder('Magento\Backend\Controller\Adminhtml\Index')
97+
->disableOriginalConstructor()
98+
->getMock();
99+
$request = $this->getMockBuilder('Magento\Framework\App\Request\Http')
100+
->disableOriginalConstructor()
101+
->getMock();
102+
$storage = $this->getMockBuilder('Magento\Backend\Model\Auth\Session')
103+
->disableOriginalConstructor()
104+
->getMock();
105+
106+
// Stubs to control the flow of execution in aroundDispatch
107+
$this->auth->expects($this->any())->method('getAuthStorage')->will($this->returnValue($storage));
108+
$request->expects($this->once())->method('getActionName')->will($this->returnValue('non/open/action/name'));
109+
$this->auth->expects($this->any())->method('getUser')->willReturn(false);
110+
$this->auth->expects($this->once())->method('isLoggedIn')->will($this->returnValue(false));
111+
$request->expects($this->any())->method('getPost')->willReturn(false);
112+
113+
// Test cases and expectations based on provided data
114+
$request->expects($this->once())->method('isForwarded')->willReturn($isForwardedFlag);
115+
$getParamCalls = 0;
116+
$actionName = '';
117+
118+
// If forwarded flag is set, getParam never gets called
119+
if (!$isForwardedFlag) {
120+
if ($isIFrameParam) {
121+
$getParamCalls = 1;
122+
$actionName = 'deniedIframe';
123+
} else if ($isAjaxParam) {
124+
$getParamCalls = 2;
125+
$actionName = 'deniedJson';
126+
} else {
127+
$getParamCalls = 2;
128+
$actionName = 'login';
129+
}
130+
}
131+
132+
$requestParams = [
133+
['isIframe', null, $isIFrameParam],
134+
['isAjax', null, $isAjaxParam]
135+
];
136+
137+
$setterCalls = $isForwardedFlag ? 0 : 1;
138+
$request->expects($this->exactly($getParamCalls))->method('getParam')->willReturnMap($requestParams);
139+
$request->expects($this->exactly($setterCalls))->method('setForwarded')->with(true)->willReturnSelf();
140+
$request->expects($this->exactly($setterCalls))->method('setRouteName')->with('adminhtml')->willReturnSelf();
141+
$request->expects($this->exactly($setterCalls))->method('setControllerName')->with('auth')->willReturnSelf();
142+
$request->expects($this->exactly($setterCalls))->method('setActionName')->with($actionName)->willReturnSelf();
143+
$request->expects($this->exactly($setterCalls))->method('setDispatched')->with(false)->willReturnSelf();
144+
145+
$expectedResult = 'expectedResult';
146+
$proceed = function ($request) use ($expectedResult) {
147+
return $expectedResult;
148+
};
149+
$this->assertEquals($expectedResult, $this->plugin->aroundDispatch($subject, $proceed, $request));
150+
}
151+
152+
public function processNotLoggedInUserDataProvider()
153+
{
154+
return [
155+
'iFrame' => [true, false, false],
156+
'Ajax' => [false, true, false],
157+
'Neither iFrame nor Ajax' => [false, false, false],
158+
'Forwarded request' => [true, true, true]
159+
];
160+
}
87161
}

‎app/code/Magento/Captcha/Controller/Adminhtml/Refresh/Refresh.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,14 @@ public function execute()
2727
$this->getResponse()->representJson(json_encode(['imgSrc' => $captchaModel->getImgSrc()]));
2828
$this->_actionFlag->set('', self::FLAG_NO_POST_DISPATCH, true);
2929
}
30+
31+
/**
32+
* Check if user has permissions to access this controller
33+
*
34+
* @return bool
35+
*/
36+
protected function _isAllowed()
37+
{
38+
return true;
39+
}
3040
}

‎lib/internal/Magento/Framework/App/Test/Unit/Config/_files/invalidRoutesXmlArray.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@
9898
'</route></router></config>',
9999
[
100100
"Element 'route', attribute 'id': [facet 'pattern'] The value 'dc' is not accepted by the " .
101-
"pattern '[A-Za-z_]{3,}'.",
101+
"pattern '[A-Za-z0-9_]{3,}'.",
102102
"Element 'route', attribute 'id': 'dc' is not a valid value of the atomic type 'routeIdType'.",
103103
"Element 'route', attribute 'id': Warning: No precomputed value available, the value was either " .
104104
"invalid or something strange happend."

‎lib/internal/Magento/Framework/App/etc/routes.xsd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@
9292
</xs:documentation>
9393
</xs:annotation>
9494
<xs:restriction base="xs:string">
95-
<xs:pattern value="[A-Za-z_]{3,}" />
95+
<xs:pattern value="[A-Za-z0-9_]{3,}" />
9696
</xs:restriction>
9797
</xs:simpleType>
9898

‎lib/internal/Magento/Framework/App/etc/routes_merged.xsd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@
9292
</xs:documentation>
9393
</xs:annotation>
9494
<xs:restriction base="xs:string">
95-
<xs:pattern value="[A-Za-z_]{3,}" />
95+
<xs:pattern value="[A-Za-z0-9_]{3,}" />
9696
</xs:restriction>
9797
</xs:simpleType>
9898

‎lib/internal/Magento/Framework/Data/Collection/Filesystem.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,10 @@ public function getAllIds()
698698
*/
699699
public function filterCallbackLike($field, $filterValue, $row)
700700
{
701-
$filterValueRegex = str_replace('%', '(.*?)', str_replace('\'', '', preg_quote($filterValue, '/')));
701+
$filterValue = trim(stripslashes($filterValue), '\'');
702+
$filterValue = trim($filterValue, '%');
703+
$filterValueRegex = '(.*?)' . preg_quote($filterValue, '/') . '(.*?)';
704+
702705
return (bool)preg_match("/^{$filterValueRegex}\$/i", $row[$field]);
703706
}
704707

‎lib/internal/Magento/Framework/Data/Test/Unit/Collection/FilesystemTest.php

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,67 @@ public function setUp()
1717
$this->model = $objectManager->getObject('Magento\Framework\Data\Collection\Filesystem');
1818
}
1919

20-
public function testFilterCallbackLike()
20+
/**
21+
* @param $field
22+
* @param $filterValue
23+
* @param $row
24+
* @param $expected
25+
*
26+
* @dataProvider testFilterCallbackLikeDataProvider
27+
*/
28+
public function testFilterCallbackLike($field, $filterValue, $row, $expected)
2129
{
22-
$field = 'field';
23-
$row = [$field => 'beginning_filter_target_end',];
24-
$filterValueSuccess = new \Zend_Db_Expr('%filter_target%');
25-
$filterValueFailure = new \Zend_Db_Expr('%not_found_in_the_row%');
30+
$filterValue = new \Zend_Db_Expr($filterValue);
2631

27-
$this->assertTrue($this->model->filterCallbackLike($field, $filterValueSuccess, $row));
28-
$this->assertFalse($this->model->filterCallbackLike($field, $filterValueFailure, $row));
32+
$this->assertEquals($expected, $this->model->filterCallbackLike($field, $filterValue, $row));
33+
}
34+
35+
/**
36+
* @return array
37+
*/
38+
public function testFilterCallbackLikeDataProvider()
39+
{
40+
$field = 'field';
41+
$testValue = '\'\'\'test\'\'\'Filter\'\'\'Value\'\'\'';
42+
return [
43+
[$field, '\'%test%\'', [$field => $testValue,], true],
44+
[$field, '%\'test%', [$field => $testValue,], true],
45+
[$field, '%\'test\'%', [$field => $testValue,], true],
46+
[$field, '%\'\'test%', [$field => $testValue,], true],
47+
[$field, '%\'\'test\'\'%', [$field => $testValue,], true],
48+
[$field, '%\'\'\'test%', [$field => $testValue,], true],
49+
[$field, '%\'\'\'test\'\'\'%', [$field => $testValue,], true],
50+
[$field, '%\'\'\'\'test%', [$field => $testValue,], false],
51+
52+
[$field, '\'%Value%\'', [$field => $testValue,], true],
53+
[$field, '%Value\'%', [$field => $testValue,], true],
54+
[$field, '%\'Value\'%', [$field => $testValue,], true],
55+
[$field, '%Value\'\'%', [$field => $testValue,], true],
56+
[$field, '%\'\'Value\'\'%', [$field => $testValue,], true],
57+
[$field, '%Value\'\'\'%', [$field => $testValue,], true],
58+
[$field, '%\'\'\'Value\'\'\'%', [$field => $testValue,], true],
59+
[$field, '%Value%\'\'\'\'%', [$field => $testValue,], false],
60+
61+
[$field, '\'%\'\'\'test\'\'\'Filter\'\'\'Value\'\'\'%\'', [$field => $testValue,], true],
62+
[$field, '\'\'\'%\'\'\'test\'\'\'Filter\'\'\'Value\'\'\'%\'\'\'', [$field => $testValue,], true],
63+
[$field, '%test\'\'\'Filter\'\'\'Value%', [$field => $testValue,], true],
64+
[$field, '%test\'\'\'Filter\'\'\'Value\'\'\'%', [$field => $testValue,], true],
65+
[$field, '%\'\'\'test\'\'\'Filter\'\'\'Value%', [$field => $testValue,], true],
66+
[$field, '%\'\'\'Filter\'\'\'Value\'\'\'%', [$field => $testValue,], true],
67+
[$field, '%Filter\'\'\'Value\'\'\'%', [$field => $testValue,], true],
68+
[$field, '%\'\'\'Filter\'\'\'Value%', [$field => $testValue,], true],
69+
[$field, '%Filter\'\'\'Value%', [$field => $testValue,], true],
70+
[$field, '%Filter\'\'\'\'Value%', [$field => $testValue,], false],
71+
72+
[$field, '\'%\'\'\'Filter\'\'\'%\'', [$field => $testValue,], true],
73+
[$field, '%Filter\'\'\'%', [$field => $testValue,], true],
74+
[$field, '%\'\'\'Filter%', [$field => $testValue,], true],
75+
[$field, '%\'Filter%', [$field => $testValue,], true],
76+
[$field, '%Filter\'%', [$field => $testValue,], true],
77+
[$field, '%Filter%', [$field => $testValue,], true],
78+
[$field, '%Filter\'\'\'\'%', [$field => $testValue,], false],
79+
80+
[$field, '\'%no_match_value%\'', [$field => $testValue,], false],
81+
];
2982
}
3083
}

‎lib/internal/Magento/Framework/HTTP/PhpEnvironment/Request.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ class Request extends \Zend\Http\PhpEnvironment\Request
6767
*/
6868
protected $dispatched = false;
6969

70+
/**
71+
* Flag for whether the request is forwarded or not
72+
*
73+
* @var bool
74+
*/
75+
protected $forwarded;
7076

7177
/**
7278
* @var CookieReaderInterface
@@ -690,4 +696,24 @@ public function getBaseUrl()
690696
$url = str_replace('\\', '/', $url);
691697
return $url;
692698
}
699+
700+
/**
701+
* @return bool
702+
* @codeCoverageIgnore
703+
*/
704+
public function isForwarded()
705+
{
706+
return $this->forwarded;
707+
}
708+
709+
/**
710+
* @param bool $forwarded
711+
* @return $this
712+
* @codeCoverageIgnore
713+
*/
714+
public function setForwarded($forwarded)
715+
{
716+
$this->forwarded = $forwarded;
717+
return $this;
718+
}
693719
}

0 commit comments

Comments
 (0)