Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/dav/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function getCapabilities() {
$capabilities = [
'dav' => [
'chunking' => '1.0',
'public_shares_chunking' => false,
'public_shares_chunking' => true,
]
];
if ($this->config->getSystemValueBool('bulkupload.enabled', true)) {
Expand Down
47 changes: 19 additions & 28 deletions apps/dav/lib/Files/Sharing/FilesDropPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ private function isChunkedUpload(RequestInterface $request): bool {
public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
$isChunkedUpload = $this->isChunkedUpload($request);

// For the final MOVE request of a chunked upload it is necessary to modify the Destination header.
if ($isChunkedUpload && $request->getMethod() !== 'MOVE') {
// For the PUT and MOVE requests of a chunked upload it is necessary to modify the Destination header.
if ($isChunkedUpload && $request->getMethod() !== 'MOVE' && $request->getMethod() !== 'PUT') {
return;
}

Expand All @@ -87,6 +87,23 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
throw new MethodNotAllowed('Only PUT, MKCOL and MOVE are allowed on files drop');
}

// Extract the attributes for the file request
$isFileRequest = false;
$attributes = $this->share->getAttributes();
if ($attributes !== null) {
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
}

// Retrieve the nickname from the request
$nickname = $request->hasHeader('X-NC-Nickname')
? trim(urldecode($request->getHeader('X-NC-Nickname')))
: null;

// We need a valid nickname for file requests
if ($isFileRequest && !$nickname) {
throw new BadRequest('A nickname header is required for file requests');
}

// If this is a folder creation request
// let's stop there and let the onMkcol handle it
if ($request->getMethod() === 'MKCOL') {
Expand All @@ -113,32 +130,6 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
$rootPath = substr($path, 0, strpos($path, $token) + strlen($token));
// e.g /Folder/image.jpg
$relativePath = substr($path, strlen($rootPath));
$isRootUpload = substr_count($relativePath, '/') === 1;

// Extract the attributes for the file request
$isFileRequest = false;
$attributes = $this->share->getAttributes();
if ($attributes !== null) {
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
}

// Retrieve the nickname from the request
$nickname = $request->hasHeader('X-NC-Nickname')
? trim(urldecode($request->getHeader('X-NC-Nickname')))
: null;

// We need a valid nickname for file requests
if ($isFileRequest && !$nickname) {
throw new BadRequest('A nickname header is required for file requests');
}

// We're only allowing the upload of
// long path with subfolders if a nickname is set.
// This prevents confusion when uploading files and help
// classify them by uploaders.
if (!$nickname && !$isRootUpload) {
throw new BadRequest('A nickname header is required when uploading subfolders');
}

if ($nickname) {
try {
Expand Down
6 changes: 3 additions & 3 deletions apps/dav/tests/unit/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function testGetCapabilities(): void {
$expected = [
'dav' => [
'chunking' => '1.0',
'public_shares_chunking' => false,
'public_shares_chunking' => true,
],
];
$this->assertSame($expected, $capabilities->getCapabilities());
Expand All @@ -50,7 +50,7 @@ public function testGetCapabilitiesWithBulkUpload(): void {
$expected = [
'dav' => [
'chunking' => '1.0',
'public_shares_chunking' => false,
'public_shares_chunking' => true,
'bulkupload' => '1.0',
],
];
Expand All @@ -71,7 +71,7 @@ public function testGetCapabilitiesWithAbsence(): void {
$expected = [
'dav' => [
'chunking' => '1.0',
'public_shares_chunking' => false,
'public_shares_chunking' => true,
'absence-supported' => true,
'absence-replacement' => true,
],
Expand Down
47 changes: 44 additions & 3 deletions apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCP\Share\IAttributes;
use OCP\Share\IShare;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\DAV\Exception\BadRequest;
use Sabre\DAV\Server;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
Expand All @@ -23,6 +24,7 @@ class FilesDropPluginTest extends TestCase {
private FilesDropPlugin $plugin;

private Folder&MockObject $node;
private IAttributes&MockObject $attributes;
private IShare&MockObject $share;
private Server&MockObject $server;
private RequestInterface&MockObject $request;
Expand All @@ -45,10 +47,10 @@ protected function setUp(): void {
$this->request = $this->createMock(RequestInterface::class);
$this->response = $this->createMock(ResponseInterface::class);

$attributes = $this->createMock(IAttributes::class);
$this->attributes = $this->createMock(IAttributes::class);
$this->share->expects($this->any())
->method('getAttributes')
->willReturn($attributes);
->willReturn($this->attributes);

$this->share
->method('getToken')
Expand Down Expand Up @@ -104,7 +106,7 @@ public function testFileAlreadyExistsValid(): void {
$this->plugin->beforeMethod($this->request, $this->response);
}

public function testMKCOL(): void {
public function testFileDropMKCOLWithoutNickname(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);

Expand All @@ -116,6 +118,45 @@ public function testMKCOL(): void {
$this->plugin->beforeMethod($this->request, $this->response);
}

public function testFileRequestNoMKCOLWithoutNickname(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);

$this->request->method('getMethod')
->willReturn('MKCOL');

$this->attributes
->method('getAttribute')
->with('fileRequest', 'enabled')
->willReturn(true);

$this->expectException(BadRequest::class);

$this->plugin->beforeMethod($this->request, $this->response);
}

public function testFileRequestMKCOLWithNickname(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);

$this->request->method('getMethod')
->willReturn('MKCOL');

$this->attributes
->method('getAttribute')
->with('fileRequest', 'enabled')
->willReturn(true);

$this->request->method('hasHeader')
->with('X-NC-Nickname')
->willReturn(true);
$this->request->method('getHeader')
->with('X-NC-Nickname')
->willReturn('nickname');

$this->plugin->beforeMethod($this->request, $this->response);
}

public function testSubdirPut(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);
Expand Down
14 changes: 13 additions & 1 deletion build/integration/features/bootstrap/FilesDropContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function droppingFileWithAs($path, $content, $nickname) {
/**
* @When Creating folder :folder in drop
*/
public function creatingFolderInDrop($folder) {
public function creatingFolderInDrop($folder, $nickname = null) {
$client = new Client();
$options = [];
if (count($this->lastShareData->data->element) > 0) {
Expand All @@ -73,10 +73,22 @@ public function creatingFolderInDrop($folder) {
'X-REQUESTED-WITH' => 'XMLHttpRequest',
];

if ($nickname) {
$options['headers']['X-NC-NICKNAME'] = $nickname;
}

try {
$this->response = $client->request('MKCOL', $fullUrl, $options);
} catch (\GuzzleHttp\Exception\ClientException $e) {
$this->response = $e->getResponse();
}
}


/**
* @When Creating folder :folder in drop as :nickName
*/
public function creatingFolderInDropWithNickname($folder, $nickname) {
return $this->creatingFolderInDrop($folder, $nickname);
}
}
36 changes: 33 additions & 3 deletions build/integration/filesdrop_features/filesdrop.feature
Original file line number Diff line number Diff line change
Expand Up @@ -33,40 +33,70 @@ Feature: FilesDrop
And Downloading file "/drop/a (2).txt"
Then Downloaded content should be "def"

Scenario: Files drop forbid directory without a nickname
Scenario: Files request forbid directory without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
And Updating last share with
| permissions | 4 |
When Dropping file "/folder/a.txt" with "abc"
Then the HTTP status code should be "400"

Scenario: Files drop allows MKCOL
Scenario: Files drop allow MKCOL without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
And Updating last share with
| permissions | 4 |
When Creating folder "folder" in drop
Then the HTTP status code should be "201"

Scenario: Files request forbid MKCOL without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
And Updating last share with
| permissions | 4 |
When Creating folder "folder" in drop
Then the HTTP status code should be "400"

Scenario: Files request allows MKCOL with a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
And Updating last share with
| permissions | 4 |
When Creating folder "folder" in drop as "nickname"
Then the HTTP status code should be "201"

Scenario: Files drop forbid subfolder creation without a nickname
Scenario: Files request forbid subfolder creation without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
And Updating last share with
| permissions | 4 |
When dropping file "/folder/a.txt" with "abc"
Expand Down
Loading