Skip to content

Commit 06d33fe

Browse files
pmarkipmarki
committed
Added unit tests + prevent Path Traversal in Delete controller
1 parent 9299024 commit 06d33fe

File tree

3 files changed

+370
-1
lines changed

3 files changed

+370
-1
lines changed

‎app/code/Magento/ImportExport/Controller/Adminhtml/Export/File/Delete.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public function execute()
6262
$resultRedirect = $this->resultRedirectFactory->create();
6363
$resultRedirect->setPath('adminhtml/export/index');
6464
$fileName = $this->getRequest()->getParam('filename');
65-
if (empty($fileName)) {
65+
if (empty($fileName) || preg_match('/\.\.(\\\|\/)/', $fileName) !== 0) {
6666
$this->messageManager->addErrorMessage(__('Please provide valid export file name'));
6767

6868
return $resultRedirect;

‎app/code/Magento/ImportExport/Test/Unit/Controller/Adminhtml/Export/File/DeleteTest.php

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\ImportExport\Controller\Adminhtml\Export\File;
7+
8+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
9+
10+
/**
11+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
12+
*/
13+
class DeleteTest extends \PHPUnit\Framework\TestCase
14+
{
15+
/**
16+
* @var \Magento\Backend\App\Action\Context|\PHPUnit_Framework_MockObject_MockObject
17+
*/
18+
protected $context;
19+
20+
/**
21+
* @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
22+
*/
23+
protected $objectManagerHelper;
24+
25+
/**
26+
* @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject
27+
*/
28+
protected $request;
29+
30+
/**
31+
* @var \Magento\Framework\Controller\Result\Raw|\PHPUnit_Framework_MockObject_MockObject
32+
*/
33+
protected $redirect;
34+
35+
/**
36+
* @var \Magento\Framework\Controller\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject
37+
*/
38+
protected $resultRedirectFactory;
39+
40+
/**
41+
* @var \Magento\Framework\Filesystem|\PHPUnit_Framework_MockObject_MockObject
42+
*/
43+
protected $fileSystem;
44+
45+
/**
46+
* @var \Magento\Framework\Filesystem\DriverInterface|\PHPUnit_Framework_MockObject_MockObject
47+
*/
48+
protected $file;
49+
50+
/**
51+
* @var \Magento\ImportExport\Controller\Adminhtml\Export\File\Delete|\PHPUnit_Framework_MockObject_MockObject
52+
*/
53+
protected $deleteController;
54+
55+
/**
56+
* @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject
57+
*/
58+
protected $messageManager;
59+
60+
/**
61+
* @var \Magento\Framework\Filesystem\Directory\ReadInterface|\PHPUnit_Framework_MockObject_MockObject
62+
*/
63+
protected $directory;
64+
65+
/**
66+
* Set up
67+
*/
68+
protected function setUp()
69+
{
70+
$this->request = $this->getMockBuilder(\Magento\Framework\App\Request\Http::class)
71+
->disableOriginalConstructor()
72+
->getMock();
73+
74+
$this->fileSystem = $this->getMockBuilder(\Magento\Framework\Filesystem::class)
75+
->disableOriginalConstructor()
76+
->getMock();
77+
78+
$this->directory = $this->getMockBuilder(\Magento\Framework\Filesystem\Directory\ReadInterface::class)
79+
->disableOriginalConstructor()
80+
->getMock();
81+
82+
$this->file = $this->getMockBuilder(\Magento\Framework\Filesystem\DriverInterface::class)
83+
->disableOriginalConstructor()
84+
->getMock();
85+
86+
$this->messageManager = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class)
87+
->disableOriginalConstructor()
88+
->getMock();
89+
90+
$this->context = $this->createPartialMock(
91+
\Magento\Backend\App\Action\Context::class,
92+
['getRequest', 'getResultRedirectFactory', 'getMessageManager']
93+
);
94+
95+
$this->redirect = $this->createPartialMock(\Magento\Backend\Model\View\Result\Redirect::class, ['setPath']);
96+
97+
$this->resultRedirectFactory = $this->createPartialMock(
98+
\Magento\Framework\Controller\Result\RedirectFactory::class,
99+
['create']
100+
);
101+
$this->resultRedirectFactory->expects($this->any())->method('create')->willReturn($this->redirect);
102+
$this->context->expects($this->any())->method('getRequest')->willReturn($this->request);
103+
$this->context->expects($this->any())
104+
->method('getResultRedirectFactory')
105+
->willReturn($this->resultRedirectFactory);
106+
107+
$this->context->expects($this->any())
108+
->method('getMessageManager')
109+
->willReturn($this->messageManager);
110+
111+
112+
$this->objectManagerHelper = new ObjectManagerHelper($this);
113+
$this->deleteController = $this->objectManagerHelper->getObject(
114+
Delete::class,
115+
[
116+
'context' => $this->context,
117+
'filesystem' => $this->fileSystem,
118+
'file' => $this->file
119+
]
120+
);
121+
}
122+
123+
/**
124+
* Tests download controller with different file names in request.
125+
*/
126+
public function testExecuteSuccess()
127+
{
128+
$this->request->method('getParam')
129+
->with('filename')
130+
->willReturn('sampleFile');
131+
132+
$this->fileSystem->expects($this->once())->method('getDirectoryRead')->will($this->returnValue($this->directory));
133+
$this->directory->expects($this->once())->method('isFile')->willReturn(true);
134+
$this->file->expects($this->once())->method('deleteFile')->willReturn(true);
135+
$this->messageManager->expects($this->once())->method('addSuccessMessage');
136+
137+
$this->deleteController->execute();
138+
}
139+
140+
/**
141+
* Tests download controller with different file names in request.
142+
143+
*/
144+
public function testExecuteFileDoesntExists()
145+
{
146+
$this->request->method('getParam')
147+
->with('filename')
148+
->willReturn('sampleFile');
149+
150+
$this->fileSystem->expects($this->once())->method('getDirectoryRead')->will($this->returnValue($this->directory));
151+
$this->directory->expects($this->once())->method('isFile')->willReturn(false);
152+
$this->messageManager->expects($this->once())->method('addErrorMessage');
153+
154+
$this->deleteController->execute();
155+
}
156+
157+
/**
158+
* Test execute() with invalid file name
159+
* @param string $requestFilename
160+
* @dataProvider executeDataProvider
161+
*/
162+
public function testExecuteInvalidFileName($requestFilename)
163+
{
164+
$this->request->method('getParam')->with('filename')->willReturn($requestFilename);
165+
$this->messageManager->expects($this->once())->method('addErrorMessage');
166+
167+
$this->deleteController->execute();
168+
}
169+
170+
/**
171+
* @return array
172+
*/
173+
public function executeDataProvider()
174+
{
175+
return [
176+
'Relative file name' => ['../.htaccess'],
177+
'Empty file name' => [''],
178+
'Null file name' => [null],
179+
];
180+
}
181+
}

‎app/code/Magento/ImportExport/Test/Unit/Controller/Adminhtml/Export/File/DownloadTest.php

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\ImportExport\Controller\Adminhtml\Export\File;
7+
8+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
9+
10+
/**
11+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
12+
*/
13+
class DownloadTest extends \PHPUnit\Framework\TestCase
14+
{
15+
/**
16+
* @var \Magento\Backend\App\Action\Context|\PHPUnit_Framework_MockObject_MockObject
17+
*/
18+
protected $context;
19+
20+
/**
21+
* @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
22+
*/
23+
protected $objectManagerHelper;
24+
25+
/**
26+
* @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject
27+
*/
28+
protected $request;
29+
30+
/**
31+
* @var \Magento\Framework\Controller\Result\Raw|\PHPUnit_Framework_MockObject_MockObject
32+
*/
33+
protected $redirect;
34+
35+
/**
36+
* @var \Magento\Framework\Controller\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject
37+
*/
38+
protected $resultRedirectFactory;
39+
40+
/**
41+
* @var \Magento\Framework\Filesystem|\PHPUnit_Framework_MockObject_MockObject
42+
*/
43+
protected $fileSystem;
44+
45+
/**
46+
* @var \Magento\Framework\App\Response\Http\FileFactory|\PHPUnit_Framework_MockObject_MockObject
47+
*/
48+
protected $fileFactory;
49+
50+
/**
51+
* @var \Magento\ImportExport\Controller\Adminhtml\Export\File\Download|\PHPUnit_Framework_MockObject_MockObject
52+
*/
53+
protected $downloadController;
54+
55+
/**
56+
* @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject
57+
*/
58+
protected $messageManager;
59+
60+
/**
61+
* @var \Magento\Framework\Filesystem\Directory\ReadInterface|\PHPUnit_Framework_MockObject_MockObject
62+
*/
63+
protected $directory;
64+
65+
/**
66+
* Set up
67+
*/
68+
protected function setUp()
69+
{
70+
$this->request = $this->getMockBuilder(\Magento\Framework\App\Request\Http::class)
71+
->disableOriginalConstructor()
72+
->getMock();
73+
74+
$this->fileSystem = $this->getMockBuilder(\Magento\Framework\Filesystem::class)
75+
->disableOriginalConstructor()
76+
->getMock();
77+
78+
$this->directory = $this->getMockBuilder(\Magento\Framework\Filesystem\Directory\ReadInterface::class)
79+
->disableOriginalConstructor()
80+
->getMock();
81+
82+
$this->fileFactory = $this->getMockBuilder(\Magento\Framework\App\Response\Http\FileFactory::class)
83+
->disableOriginalConstructor()
84+
->getMock();
85+
86+
$this->messageManager = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class)
87+
->disableOriginalConstructor()
88+
->getMock();
89+
90+
$this->context = $this->createPartialMock(
91+
\Magento\Backend\App\Action\Context::class,
92+
['getRequest', 'getResultRedirectFactory', 'getMessageManager']
93+
);
94+
95+
$this->redirect = $this->createPartialMock(
96+
\Magento\Backend\Model\View\Result\Redirect::class,
97+
['setPath']
98+
);
99+
100+
$this->resultRedirectFactory = $this->createPartialMock(
101+
\Magento\Framework\Controller\Result\RedirectFactory::class,
102+
['create']
103+
);
104+
$this->resultRedirectFactory->expects($this->any())
105+
->method('create')
106+
->willReturn($this->redirect);
107+
108+
$this->context->expects($this->any())
109+
->method('getRequest')
110+
->willReturn($this->request);
111+
112+
$this->context->expects($this->any())
113+
->method('getResultRedirectFactory')
114+
->willReturn($this->resultRedirectFactory);
115+
116+
$this->context->expects($this->any())
117+
->method('getMessageManager')
118+
->willReturn($this->messageManager);
119+
120+
$this->objectManagerHelper = new ObjectManagerHelper($this);
121+
$this->downloadController = $this->objectManagerHelper->getObject(
122+
Download::class,
123+
[
124+
'context' => $this->context,
125+
'filesystem' => $this->fileSystem,
126+
'fileFactory' => $this->fileFactory
127+
]
128+
);
129+
}
130+
131+
/**
132+
* Tests download controller with successful file downloads
133+
*/
134+
public function testExecuteSuccess()
135+
{
136+
$this->request->method('getParam')
137+
->with('filename')
138+
->willReturn('sampleFile.csv');
139+
140+
$this->fileSystem->expects($this->once())->method('getDirectoryRead')->will($this->returnValue($this->directory));
141+
$this->directory->expects($this->once())->method('isFile')->willReturn(true);
142+
$this->fileFactory->expects($this->once())->method('create');
143+
144+
$this->downloadController->execute();
145+
}
146+
147+
/**
148+
* Tests download controller with file that doesn't exist
149+
150+
*/
151+
public function testExecuteFileDoesntExists()
152+
{
153+
$this->request->method('getParam')
154+
->with('filename')
155+
->willReturn('sampleFile');
156+
157+
$this->fileSystem->expects($this->once())->method('getDirectoryRead')->will($this->returnValue($this->directory));
158+
$this->directory->expects($this->once())->method('isFile')->willReturn(false);
159+
$this->messageManager->expects($this->once())->method('addErrorMessage');
160+
161+
$this->downloadController->execute();
162+
}
163+
164+
/**
165+
* Test execute() with invalid file name
166+
* @param string $requestFilename
167+
* @dataProvider executeDataProvider
168+
*/
169+
public function testExecuteInvalidFileName($requestFilename)
170+
{
171+
$this->request->method('getParam')->with('filename')->willReturn($requestFilename);
172+
$this->messageManager->expects($this->once())->method('addErrorMessage');
173+
174+
$this->downloadController->execute();
175+
}
176+
177+
/**
178+
* @return array
179+
*/
180+
public function executeDataProvider()
181+
{
182+
return [
183+
'Relative file name' => ['../.htaccess'],
184+
'Empty file name' => [''],
185+
'Null file name' => [null],
186+
];
187+
}
188+
}

0 commit comments

Comments
 (0)