Skip to content

Commit

Permalink
Block reading children of upload-only folder on GET request
Browse files Browse the repository at this point in the history
  • Loading branch information
VicDeo committed Aug 29, 2019
1 parent 4aa17f5 commit cbd4705
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
14 changes: 8 additions & 6 deletions apps/dav/lib/Files/PublicFiles/PublicSharedRootNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ public function __construct(IShare $share, IRequest $request) {
*/
public function getChildren() {
// Within a PROPFIND request we return no listing in case the share is a file drop folder
if ($this->isPropfind() && $this->isFileDropFolder()) {
if ($this->isFileDropFolder()
&& ($this->isPropfind() || $this->isGet())
) {
return [];
}

Expand Down Expand Up @@ -184,11 +186,7 @@ public function getACL() {
'principal' => '{DAV:}owner',
'protected' => true,
],
[
'privilege' => '{DAV:}read',
'principal' => 'principals/system/public',
'protected' => true,
]

];

if ($this->checkPermissions(Constants::PERMISSION_UPDATE)) {
Expand Down Expand Up @@ -227,6 +225,10 @@ private function isPropfind() {
return $this->request->getMethod() === 'PROPFIND';
}

private function isGet() {
return $this->request->getMethod() === 'GET';
}

/**
* An anonymous upload folder aka file drop folder has only the create permission
* @return bool
Expand Down
12 changes: 12 additions & 0 deletions apps/dav/tests/unit/Files/PublicFiles/PublicSharedRootNodeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
namespace OCA\DAV\Tests\Unit\Files\PublicFiles;

use OCA\DAV\Files\PublicFiles\PublicSharedRootNode;
use OCP\Constants;
use OCP\Files\NotFoundException;
use OCP\IRequest;
use OCP\Share\IShare;
Expand All @@ -39,4 +40,15 @@ public function testNoLongerExistingResource() {
$publicSharedRoot = new PublicSharedRootNode($share, $request);
$publicSharedRoot->getChildren();
}

public function testNoChildrenOnGetForPublicUpload() {
$share = $this->createMock(IShare::class);
$request = $this->createMock(IRequest::class);
$share->method('getPermissions')->willReturn(Constants::PERMISSION_CREATE);
$request->method('getMethod')->willReturn('GET');

$publicSharedRoot = new PublicSharedRootNode($share, $request);
$children = $publicSharedRoot->getChildren();
$this->assertEmpty($children);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ Feature: sharing
| path | FOLDER |
| permissions | create |
Then the public download of file "/test.txt" from inside the last public shared folder using the old public WebDAV API should fail with HTTP status code "404"
And the public should be able to download the range "bytes=0-7" of file "/test.txt" from inside the last public shared folder using the new public WebDAV API and the content should be "ownCloud"
#And the public download of file "/test.txt" from inside the last public shared folder using the new public WebDAV API should fail with HTTP status code "404"
And the public download of file "/test.txt" from inside the last public shared folder using the new public WebDAV API should fail with HTTP status code "404"

@public_link_share-feature-required
Scenario: Downloading from share after the share source was deleted
Expand Down

0 comments on commit cbd4705

Please sign in to comment.