feat(dav): allow uploading folders to public shares

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
This commit is contained in:
John Molakvoæ (skjnldsv) 2025-05-11 14:56:59 +02:00
parent 56897b6f3c
commit 4495794a0b
No known key found for this signature in database
GPG key ID: 60C25B8C072916CF
7 changed files with 316 additions and 63 deletions

View file

@ -26,19 +26,19 @@ class ChecksumUpdatePlugin extends ServerPlugin {
return 'checksumupdate';
}
/** @return string[] */
public function getHTTPMethods($path): array {
$tree = $this->server->tree;
/** @return string[] */
public function getHTTPMethods($path): array {
$tree = $this->server->tree;
if ($tree->nodeExists($path)) {
$node = $tree->getNodeForPath($path);
if ($node instanceof File) {
return ['PATCH'];
if ($tree->nodeExists($path)) {
$node = $tree->getNodeForPath($path);
if ($node instanceof File) {
return ['PATCH'];
}
}
}
return [];
}
return [];
}
/** @return string[] */
public function getFeatures(): array {

View file

@ -176,13 +176,14 @@ class Directory extends Node implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuot
public function getChild($name, $info = null, ?IRequest $request = null, ?IL10N $l10n = null) {
$storage = $this->info->getStorage();
$allowDirectory = false;
// Checking if we're in a file drop
// If we are, then only PUT and MKCOL are allowed (see plugin)
// so we are safe to return the directory without a risk of
// leaking files and folders structure.
if ($storage instanceof PublicShareWrapper) {
$share = $storage->getShare();
$allowDirectory =
// Only allow directories for file drops
($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ &&
// And only allow it for directories which are a direct child of the share root
$this->info->getId() === $share->getNodeId();
$allowDirectory = ($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ;
}
// For file drop we need to be allowed to read the directory with the nickname

View file

@ -36,57 +36,136 @@ class FilesDropPlugin extends ServerPlugin {
/**
* This initializes the plugin.
*
* @param \Sabre\DAV\Server $server Sabre server
*
* @return void
* @throws MethodNotAllowed
* It is ONLY initialized by the server on a file drop request.
*/
public function initialize(\Sabre\DAV\Server $server): void {
$server->on('beforeMethod:*', [$this, 'beforeMethod'], 999);
$server->on('method:MKCOL', [$this, 'onMkcol']);
$this->enabled = false;
}
public function beforeMethod(RequestInterface $request, ResponseInterface $response): void {
public function onMkcol(RequestInterface $request, ResponseInterface $response) {
if (!$this->enabled || $this->share === null || $this->view === null) {
return;
}
// Only allow file drop
if ($request->getMethod() !== 'PUT') {
throw new MethodNotAllowed('Only PUT is allowed on files drop');
// If this is a folder creation request we need
// to fake a success so we can pretend every
// folder now exists.
$response->setStatus(201);
return false;
}
public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
if (!$this->enabled || $this->share === null || $this->view === null) {
return;
}
// Always upload at the root level
$path = explode('/', $request->getPath());
$path = array_pop($path);
// Retrieve the nickname from the request
$nickname = $request->hasHeader('X-NC-Nickname')
? trim(urldecode($request->getHeader('X-NC-Nickname')))
: null;
//
if ($request->getMethod() !== 'PUT') {
// If uploading subfolders we need to ensure they get created
// within the nickname folder
if ($request->getMethod() === 'MKCOL') {
if (!$nickname) {
throw new MethodNotAllowed('A nickname header is required when uploading subfolders');
}
} else {
throw new MethodNotAllowed('Only PUT is allowed on files drop');
}
}
// If this is a folder creation request
// let's stop there and let the onMkcol handle it
if ($request->getMethod() === 'MKCOL') {
return;
}
// Now if we create a file, we need to create the
// full path along the way. We'll only handle conflict
// resolution on file conflicts, but not on folders.
// e.g files/dCP8yn3N86EK9sL/Folder/image.jpg
$path = $request->getPath();
$token = $this->share->getToken();
// e.g files/dCP8yn3N86EK9sL
$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();
$nickName = $request->hasHeader('X-NC-Nickname') ? urldecode($request->getHeader('X-NC-Nickname')) : null;
if ($attributes !== null) {
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
}
// We need a valid nickname for file requests
if ($isFileRequest && ($nickName == null || trim($nickName) === '')) {
throw new MethodNotAllowed('Nickname is required for file requests');
if ($isFileRequest && !$nickname) {
throw new MethodNotAllowed('A nickname header is required for file requests');
}
// If this is a file request we need to create a folder for the user
if ($isFileRequest) {
// Check if the folder already exists
if (!($this->view->file_exists($nickName) === true)) {
$this->view->mkdir($nickName);
}
// 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 MethodNotAllowed('A nickname header is required when uploading subfolders');
}
// If we have a nickname, let's put everything inside
if ($nickname) {
// Put all files in the subfolder
$path = $nickName . '/' . $path;
$relativePath = '/' . $nickname . '/' . $relativePath;
$relativePath = str_replace('//', '/', $relativePath);
}
$newName = \OC_Helper::buildNotExistingFileNameForView('/', $path, $this->view);
$url = $request->getBaseUrl() . '/files/' . $this->share->getToken() . $newName;
// Create the folders along the way
$folders = $this->getPathSegments(dirname($relativePath));
foreach ($folders as $folder) {
if ($folder === '') {
continue;
} // skip empty parts
if (!$this->view->file_exists($folder)) {
$this->view->mkdir($folder);
}
}
// Finally handle conflicts on the end files
$noConflictPath = \OC_Helper::buildNotExistingFileNameForView(dirname($relativePath), basename($relativePath), $this->view);
$path = '/files/' . $token . '/' . $noConflictPath;
$url = $request->getBaseUrl() . str_replace('//', '/', $path);
$request->setUrl($url);
}
private function getPathSegments(string $path): array {
// Normalize slashes and remove trailing slash
$path = rtrim(str_replace('\\', '/', $path), '/');
// Handle absolute paths starting with /
$isAbsolute = str_starts_with($path, '/');
$segments = explode('/', $path);
// Add back the leading slash for the first segment if needed
$result = [];
$current = $isAbsolute ? '/' : '';
foreach ($segments as $segment) {
if ($segment === '') {
// skip empty parts
continue;
}
$current = rtrim($current, '/') . '/' . $segment;
$result[] = $current;
}
return $result;
}
}

View file

@ -46,9 +46,6 @@ class FilesDropPluginTest extends TestCase {
$this->request = $this->createMock(RequestInterface::class);
$this->response = $this->createMock(ResponseInterface::class);
$this->response->expects($this->never())
->method($this->anything());
$attributes = $this->createMock(IAttributes::class);
$this->share->expects($this->any())
->method('getAttributes')
@ -60,13 +57,19 @@ class FilesDropPluginTest extends TestCase {
}
public function testInitialize(): void {
$this->server->expects($this->once())
$this->server->expects($this->at(0))
->method('on')
->with(
$this->equalTo('beforeMethod:*'),
$this->equalTo([$this->plugin, 'beforeMethod']),
$this->equalTo(999)
);
$this->server->expects($this->at(1))
->method('on')
->with(
$this->equalTo('method:MKCOL'),
$this->equalTo([$this->plugin, 'onMkcol']),
);
$this->plugin->initialize($this->server);
}
@ -136,7 +139,7 @@ class FilesDropPluginTest extends TestCase {
$this->plugin->beforeMethod($this->request, $this->response);
}
public function testNoMKCOL(): void {
public function testNoMKCOLWithoutNickname(): void {
$this->plugin->enable();
$this->plugin->setView($this->view);
$this->plugin->setShare($this->share);
@ -149,7 +152,27 @@ class FilesDropPluginTest extends TestCase {
$this->plugin->beforeMethod($this->request, $this->response);
}
public function testNoSubdirPut(): void {
public function testMKCOLWithNickname(): void {
$this->plugin->enable();
$this->plugin->setView($this->view);
$this->plugin->setShare($this->share);
$this->request->method('getMethod')
->willReturn('MKCOL');
$this->request->method('hasHeader')
->with('X-NC-Nickname')
->willReturn(true);
$this->request->method('getHeader')
->with('X-NC-Nickname')
->willReturn('nickname');
$this->expectNotToPerformAssertions();
$this->plugin->beforeMethod($this->request, $this->response);
}
public function testSubdirPut(): void {
$this->plugin->enable();
$this->plugin->setView($this->view);
$this->plugin->setShare($this->share);
@ -157,6 +180,13 @@ class FilesDropPluginTest extends TestCase {
$this->request->method('getMethod')
->willReturn('PUT');
$this->request->method('hasHeader')
->with('X-NC-Nickname')
->willReturn(true);
$this->request->method('getHeader')
->with('X-NC-Nickname')
->willReturn('nickname');
$this->request->method('getPath')
->willReturn('/files/token/folder/file.txt');
@ -165,7 +195,7 @@ class FilesDropPluginTest extends TestCase {
$this->view->method('file_exists')
->willReturnCallback(function ($path) {
if ($path === 'file.txt' || $path === '/file.txt') {
if ($path === 'file.txt' || $path === '/folder/file.txt') {
return true;
} else {
return false;
@ -174,8 +204,70 @@ class FilesDropPluginTest extends TestCase {
$this->request->expects($this->once())
->method('setUrl')
->with($this->equalTo('https://example.com/files/token/file (2).txt'));
->with($this->equalTo('https://example.com/files/token/nickname/folder/file.txt'));
$this->plugin->beforeMethod($this->request, $this->response);
}
public function testRecursiveFolderCreation(): void {
$this->plugin->enable();
$this->plugin->setView($this->view);
$this->plugin->setShare($this->share);
$this->request->method('getMethod')
->willReturn('PUT');
$this->request->method('hasHeader')
->with('X-NC-Nickname')
->willReturn(true);
$this->request->method('getHeader')
->with('X-NC-Nickname')
->willReturn('nickname');
$this->request->method('getPath')
->willReturn('/files/token/folder/subfolder/file.txt');
$this->request->method('getBaseUrl')
->willReturn('https://example.com');
$this->view->method('file_exists')
->willReturn(false);
$this->view->expects($this->exactly(4))
->method('file_exists')
->withConsecutive(
['/nickname'],
['/nickname/folder'],
['/nickname/folder/subfolder'],
['/nickname/folder/subfolder/file.txt']
)
->willReturnOnConsecutiveCalls(
false,
false,
false,
false,
);
$this->view->expects($this->exactly(3))
->method('mkdir')
->withConsecutive(
['/nickname'],
['/nickname/folder'],
['/nickname/folder/subfolder'],
);
$this->request->expects($this->once())
->method('setUrl')
->with($this->equalTo('https://example.com/files/token/nickname/folder/subfolder/file.txt'));
$this->plugin->beforeMethod($this->request, $this->response);
}
public function testOnMkcol(): void {
$this->plugin->enable();
$this->plugin->setView($this->view);
$this->plugin->setShare($this->share);
$this->response->expects($this->once())
->method('setStatus')
->with(201);
$response = $this->plugin->onMkcol($this->request, $this->response);
$this->assertFalse($response);
}
}

View file

@ -15,7 +15,7 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
/**
* @When Dropping file :path with :content
*/
public function droppingFileWith($path, $content, $nickName = null) {
public function droppingFileWith($path, $content, $nickname = null) {
$client = new Client();
$options = [];
if (count($this->lastShareData->data->element) > 0) {
@ -28,11 +28,11 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
$fullUrl = str_replace('//', '/', $base . "/public.php/dav/files/$token/$path");
$options['headers'] = [
'X-REQUESTED-WITH' => 'XMLHttpRequest'
'X-REQUESTED-WITH' => 'XMLHttpRequest',
];
if ($nickName) {
$options['headers']['X-NC-NICKNAME'] = $nickName;
if ($nickname) {
$options['headers']['X-NC-NICKNAME'] = $nickname;
}
$options['body'] = \GuzzleHttp\Psr7\Utils::streamFor($content);
@ -48,15 +48,15 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
/**
* @When Dropping file :path with :content as :nickName
*/
public function droppingFileWithAs($path, $content, $nickName) {
$this->droppingFileWith($path, $content, $nickName);
public function droppingFileWithAs($path, $content, $nickname) {
$this->droppingFileWith($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) {
@ -69,13 +69,25 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
$fullUrl = str_replace('//', '/', $base . "/public.php/dav/files/$token/$folder");
$options['headers'] = [
'X-REQUESTED-WITH' => 'XMLHttpRequest'
'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);
}
}

View file

@ -262,7 +262,11 @@ trait WebDav {
'Accept' => 'application/zip'
];
$this->response = $client->request('GET', $fullUrl, $options);
try {
$this->response = $client->request('GET', $fullUrl, $options);
} catch (\GuzzleHttp\Exception\ClientException $e) {
$this->response = $e->getResponse();
}
}
/**

View file

@ -33,7 +33,7 @@ Feature: FilesDrop
And Downloading file "/drop/a (2).txt"
Then Downloaded content should be "def"
Scenario: Files drop ignores directory
Scenario: Files drop forbid directory without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
@ -44,10 +44,9 @@ Feature: FilesDrop
And Updating last share with
| permissions | 4 |
When Dropping file "/folder/a.txt" with "abc"
And Downloading file "/drop/a.txt"
Then Downloaded content should be "abc"
Then the HTTP status code should be "405"
Scenario: Files drop forbis MKCOL
Scenario: Files drop forbid MKCOL without a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
@ -60,6 +59,32 @@ Feature: FilesDrop
When Creating folder "folder" in drop
Then the HTTP status code should be "405"
Scenario: Files drop 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 |
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
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 dropping file "/folder/a.txt" with "abc"
Then the HTTP status code should be "405"
Scenario: Files request drop
Given user "user0" exists
And As an "user0"
@ -71,7 +96,7 @@ Feature: FilesDrop
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
| shareWith | |
When Dropping file "/folder/a.txt" with "abc" as "Alice"
And Downloading file "/drop/Alice/a.txt"
And Downloading file "/drop/Alice/folder/a.txt"
Then Downloaded content should be "abc"
Scenario: Put file same file multiple times via files drop
@ -86,7 +111,47 @@ Feature: FilesDrop
| shareWith | |
When Dropping file "/folder/a.txt" with "abc" as "Mallory"
And Dropping file "/folder/a.txt" with "def" as "Mallory"
And Downloading file "/drop/Mallory/a.txt"
# Ensure folder structure and that we only checked
# for files duplicates, but merged the existing folders
Then as "user0" the folder "/drop/Mallory" exists
Then as "user0" the folder "/drop/Mallory/folder" exists
Then as "user0" the folder "/drop/Mallory (2)" does not exist
Then as "user0" the folder "/drop/Mallory/folder (2)" does not exist
Then as "user0" the file "/drop/Mallory/folder/a.txt" exists
Then as "user0" the file "/drop/Mallory/folder/a (2).txt" exists
And Downloading file "/drop/Mallory/folder/a.txt"
Then Downloaded content should be "abc"
And Downloading file "/drop/Mallory/a (2).txt"
And Downloading file "/drop/Mallory/folder/a (2).txt"
Then Downloaded content should be "def"
Scenario: Files drop prevents GET
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 | 4 |
| permissions | 4 |
| shareWith | |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
When Dropping file "/folder/a.txt" with "abc" as "Mallory"
When as "user0" the file "/drop/Mallory/folder/a.txt" exists
And Downloading public folder "Mallory"
Then the HTTP status code should be "405"
And Downloading public folder "Mallory/folder"
Then the HTTP status code should be "405"
And Downloading public file "Mallory/folder/a.txt"
Then the HTTP status code should be "405"
Scenario: Files drop requires nickname if file request is enabled
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 | 4 |
| permissions | 4 |
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
| shareWith | |
When Dropping file "/folder/a.txt" with "abc"
Then the HTTP status code should be "405"