Skip to content

Commit

Permalink
Merge pull request #28637 from owncloud/stable10-validation-plugin
Browse files Browse the repository at this point in the history 
Stable10 validation plugin
  • Loading branch information
Vincent Petry committed Aug 10, 2017
2 parents 18dfd26 + 975b2f4 commit 72c43e6
Show file tree
Hide file tree
Showing 8 changed files with 260 additions and 54 deletions.
1 change: 1 addition & 0 deletions apps/dav/lib/Connector/Sabre/ServerFactory.php
View file
Expand Up @@ -101,6 +101,7 @@ public function createServer($baseUri,

// Load plugins
$server->addPlugin(new \OCA\DAV\Connector\Sabre\MaintenancePlugin($this->config));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\ValidateRequestPlugin('webdav'));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin($this->config));
$server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend));
// FIXME: The following line is a workaround for legacy components relying on being able to send a GET to /
Expand Down
101 changes: 101 additions & 0 deletions apps/dav/lib/Connector/Sabre/ValidateRequestPlugin.php
View file
@@ -0,0 +1,101 @@
<?php
/**
* @author Piotr Mrowczynski <piotr@owncloud.com>
*
* @copyright Copyright (c) 2017, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace OCA\DAV\Connector\Sabre;

use Sabre\DAV\Exception\ServiceUnavailable;
use Sabre\DAV\ServerPlugin;

/**
* Class ValidateRequestPlugin is a plugin which examines correctness of
* the webdav request, checking if accessing a service e.g. "dav"
* the request has proper headers/structure.
*
* @see https://github.com/owncloud/core/issues/28200
* @package OCA\DAV\Connector\Sabre
*/
class ValidateRequestPlugin extends ServerPlugin {

/**
* Reference to main server object
*
* @var \Sabre\DAV\Server
*/
private $server;

/**
* Service type as decided by resolveService($service) in remote.php
*
* @var string
*/
private $service;

/**
* This plugin ensures that all request directed to specific
* services (type $service as decided by resolveService($service) in remote.php)
* contain correct headers and their structure is correct
*
* Currently supported:
* 'webdav'
* 'dav'
*
* @var string $service
*/
public function __construct($service) {
$this->service = $service;
}

/**
* This initializes the plugin.
*
* This function is called by \Sabre\DAV\Server, after
* addPlugin is called.
*
* This method should set up the required event subscriptions.
*
* @param \Sabre\DAV\Server $server
* @return void
*/
public function initialize(\Sabre\DAV\Server $server) {
$this->server = $server;
$this->server->on('beforeMethod:PUT', [$this, 'checkValidityPut'], 100);
}

/**
* This method is called before any HTTP method and returns http status code 503
* in case the request is incorrect
*
* @throws ServiceUnavailable
* @return bool
*/
public function checkValidityPut() {
$request = $this->server->httpRequest;

// Verify if optional OC headers are routed in the proper endpoint
if (($request->hasHeader('HTTP_OC_CHUNKED') || $request->hasHeader('Oc-Chunked'))
&& ($this->service !== 'webdav')) {
// Headers not allowed in new dav endpoint
throw new ServiceUnavailable('Specified header (HTTP_OC_CHUNKED/OC-Chunked header) is allowed only in webdav endpoint');
}

return true;
}
}
2 changes: 2 additions & 0 deletions apps/dav/lib/Server.php
View file
Expand Up @@ -50,6 +50,7 @@
use OCA\DAV\Connector\Sabre\TagsPlugin;
use OCA\DAV\AppInfo\PluginManager;
use OCA\DAV\Connector\Sabre\MaintenancePlugin;
use OCA\DAV\Connector\Sabre\ValidateRequestPlugin;

class Server {

Expand Down Expand Up @@ -80,6 +81,7 @@ public function __construct(IRequest $request, $baseUri) {

$config = \OC::$server->getConfig();
$this->server->addPlugin(new MaintenancePlugin($config));
$this->server->addPlugin(new ValidateRequestPlugin('dav'));
$this->server->addPlugin(new BlockLegacyClientPlugin($config));
$authPlugin = new Plugin();
$authPlugin->addBackend(new PublicAuth());
Expand Down
74 changes: 74 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/ValidateRequestPluginTest.php
View file
@@ -0,0 +1,74 @@
<?php
/**
* @author Piotr Mrowczynski <piotr@owncloud.com>
*
* @copyright Copyright (c) 2017, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace OCA\DAV\Tests\unit\Connector\Sabre;

use OCA\DAV\Connector\Sabre\ValidateRequestPlugin;
use Test\TestCase;
use OCP\IConfig;

/**
* Class ValidateRequestPluginTest
*
* @package OCA\DAV\Tests\unit\Connector\Sabre
*/
class ValidateRequestPluginTest extends TestCase {
/** @var \Sabre\DAV\Server */
private $server;

/** @var ValidateRequestPlugin */
private $plugin;

public function setUp() {
parent::setUp();

$this->server = new \Sabre\DAV\Server();
}

/**
* @expectedException \Sabre\DAV\Exception\ServiceUnavailable
* @expectedExceptionMessage Specified header (HTTP_OC_CHUNKED/OC-Chunked header) is allowed only in webdav endpoint
*/
public function testOldChunkingInNewEndpoint1() {
$headers['HTTP_OC_CHUNKED'] = 1;

$this->plugin = new ValidateRequestPlugin('dav');
$this->server->httpRequest = new \Sabre\HTTP\Request('PUT', 'http://server/root_url/remote.php/dav/files/user/testnew.txt', $headers);
$this->plugin->initialize($this->server);

$this->plugin->checkValidityPut();
}

/**
* @expectedException \Sabre\DAV\Exception\ServiceUnavailable
* @expectedExceptionMessage Specified header (HTTP_OC_CHUNKED/OC-Chunked header) is allowed only in webdav endpoint
*/
public function testOldChunkingInNewEndpoint2() {
$headers['OC-Chunked'] = 1;

$this->plugin = new ValidateRequestPlugin('dav');
$this->server->httpRequest = new \Sabre\HTTP\Request('PUT', 'http://server/root_url/remote.php/dav/files/user/testnew.txt', $headers);
$this->plugin->initialize($this->server);

$this->plugin->checkValidityPut();
}

}
22 changes: 13 additions & 9 deletions tests/integration/features/bootstrap/Checksums.php
View file
Expand Up @@ -159,14 +159,18 @@ public function theOcChecksumHeaderShouldNotBeThere()
* @param string $checksum
*/
public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) {
$num -= 1;
$data = \GuzzleHttp\Stream\Stream::factory($data);
$file = $destination . '-chunking-42-' . $total . '-' . $num;
$this->response = $this->makeDavRequest($user,
'PUT',
$file,
['OC-Checksum' => $checksum, 'OC-Chunked' => '1'],
$data,
"files");
try {
$num -= 1;
$data = \GuzzleHttp\Stream\Stream::factory($data);
$file = $destination . '-chunking-42-' . $total . '-' . $num;
$this->response = $this->makeDavRequest($user,
'PUT',
$file,
['OC-Checksum' => $checksum, 'OC-Chunked' => '1'],
$data,
"files");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}
}
57 changes: 30 additions & 27 deletions tests/integration/features/bootstrap/WebDav.php
View file
Expand Up @@ -680,19 +680,6 @@ public function uploadWithAllMechanisms($user, $source, $destination, $overwrite
}
}
if ($dav === 'new') {
// old chunking style applied to new endpoint 🙈
if (!$overwriteMode) {
$suffix = '-' . $dav . 'dav-oldchunking';
}
try {
// FIXME: prepending new dav path because the chunking utility functions are messed up
$this->userUploadsAFileToWithChunks($user, $source, '/files/' . $user . '/' . ltrim($destination, '/') . $suffix, 'old');
$responses[] = $this->response;
} catch (ServerException $e) {
$responses[] = $e->getResponse();
}

// new chunking style applied to new endpoint
if (!$overwriteMode) {
$suffix = '-' . $dav . 'dav-newchunking';
}
Expand Down Expand Up @@ -834,29 +821,41 @@ public function userCreatedAFolder($user, $destination){
*/
public function userUploadsChunkedFile($user, $num, $total, $data, $destination)
{
$num -= 1;
$data = \GuzzleHttp\Stream\Stream::factory($data);
$file = $destination . '-chunking-42-' . $total . '-' . $num;
$this->makeDavRequest($user, 'PUT', $file, ['OC-Chunked' => '1'], $data, "uploads");
try {
$num -= 1;
$data = \GuzzleHttp\Stream\Stream::factory($data);
$file = $destination . '-chunking-42-' . $total . '-' . $num;
$this->makeDavRequest($user, 'PUT', $file, ['OC-Chunked' => '1'], $data, "uploads");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}

/**
* @Given user :user creates a new chunking upload with id :id
*/
public function userCreatesANewChunkingUploadWithId($user, $id)
{
$destination = '/uploads/'.$user.'/'.$id;
$this->makeDavRequest($user, 'MKCOL', $destination, [], null, "uploads");
try {
$destination = '/uploads/'.$user.'/'.$id;
$this->makeDavRequest($user, 'MKCOL', $destination, [], null, "uploads");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}

/**
* @Given user :user uploads new chunk file :num with :data to id :id
*/
public function userUploadsNewChunkFileOfWithToId($user, $num, $data, $id)
{
$data = \GuzzleHttp\Stream\Stream::factory($data);
$destination = '/uploads/'. $user .'/'. $id .'/' . $num;
$this->makeDavRequest($user, 'PUT', $destination, [], $data, "uploads");
try {
$data = \GuzzleHttp\Stream\Stream::factory($data);
$destination = '/uploads/'. $user .'/'. $id .'/' . $num;
$this->makeDavRequest($user, 'PUT', $destination, [], $data, "uploads");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}

/**
Expand Down Expand Up @@ -885,11 +884,15 @@ public function userUploadsNewChunkFileOfWithToIdWithChecksum($user, $num, $data
*/
public function userMovesNewChunkFileWithIdToMychunkedfile($user, $id, $dest)
{
$source = '/uploads/' . $user . '/' . $id . '/.file';
$destination = $this->baseUrlWithoutOCSAppendix() . $this->getDavFilesPath($user) . $dest;
$this->makeDavRequest($user, 'MOVE', $source, [
'Destination' => $destination
], null, "uploads");
try {
$source = '/uploads/' . $user . '/' . $id . '/.file';
$destination = $this->baseUrlWithoutOCSAppendix() . $this->getDavFilesPath($user) . $dest;
$this->makeDavRequest($user, 'MOVE', $source, [
'Destination' => $destination
], null, "uploads");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}


Expand Down
25 changes: 7 additions & 18 deletions tests/integration/features/checksums.feature
View file
Expand Up @@ -115,24 +115,6 @@ Feature: checksums
And user "user0" downloads the file "/myChecksumFileCopy.txt"
Then The header checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f"

Scenario: Uploading a chunked file with checksum should return the checksum in the propfind using new dav path
Given using new dav path
And user "user0" exists
And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
When user "user0" request the checksum of "/myChecksumFile.txt" via propfind
Then The webdav checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df"

Scenario: Uploading a chunked file with checksum should return the checksum in the download header using new dav path
Given using new dav path
And user "user0" exists
And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
When user "user0" downloads the file "/myChecksumFile.txt"
Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8"

@local_storage
Scenario: Downloading a file from local storage has correct checksum using new dav path
Given using new dav path
Expand Down Expand Up @@ -183,3 +165,10 @@ Feature: checksums
Then The following headers should be set
| OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 |

## Validation Plugin or Old Endpoint Specific

Scenario: Uploading a old method chunked file with checksum should fail using new dav path
Given using new dav path
And user "user0" exists
When user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
Then the HTTP status code should be "503"
32 changes: 32 additions & 0 deletions tests/integration/features/webdav-related-new-endpoint.feature
View file
Expand Up @@ -548,3 +548,35 @@ Feature: webdav-related-new-endpoint
And as "user1" the folder "/folderB/ONE/TWO" exists
And User "user1" checks id of file "/folderB/ONE"

## Validation Plugin or Old Endpoint Specific

Scenario: New chunked upload MKDIR using old dav path should fail
Given using old dav path
And user "user0" exists
When user "user0" creates a new chunking upload with id "chunking-42"
Then the HTTP status code should be "409"

Scenario: New chunked upload PUT using old dav path should fail
Given using new dav path
And user "user0" exists
And user "user0" creates a new chunking upload with id "chunking-42"
When using old dav path
And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42"
Then the HTTP status code should be "404"

Scenario: New chunked upload MOVE using old dav path should fail
Given using new dav path
And user "user0" exists
And user "user0" creates a new chunking upload with id "chunking-42"
And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42"
And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42"
And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42"
When using old dav path
And user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt"
Then the HTTP status code should be "404"

Scenario: Upload to new dav path using old way should fail
Given using new dav path
And user "user0" exists
When user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChunkedFile.txt"
Then the HTTP status code should be "503"

0 comments on commit 72c43e6

Please sign in to comment.