Skip to content

fix(dav): align public request path handling #66022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
leftybournes merged 1 commit into from
Oct 30, 2026
Merged

fix(dav): align public request path handling #66022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions apps/dav/appinfo/v2/publicremote.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
$filesDropPlugin = new FilesDropPlugin();

/** @var string $baseuri defined in public.php */
$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($baseuri, $requestUri, $authBackend, $bearerAuthBackend, $linkCheckPlugin, $filesDropPlugin) {
$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($authBackend, $bearerAuthBackend, $linkCheckPlugin, $filesDropPlugin) {
// GET must be allowed for e.g. showing images and allowing Zip downloads
if ($server->httpRequest->getMethod() !== 'GET') {
// If this is *not* a GET request we only allow access to public DAV from AJAX or when Server2Server is allowed
Expand All @@ -110,18 +110,19 @@
}
$isReadable = $share->getPermissions() & Constants::PERMISSION_READ;
$fileId = $share->getNodeId();
$isUpload = str_starts_with($server->httpRequest->getPath(), 'uploads/');

// FIXME: should not add storage wrappers outside of preSetup, need to find a better way
/** @psalm-suppress InternalMethod */
$previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false);

/** @psalm-suppress MissingClosureParamType */
Filesystem::addStorageWrapper('sharePermissions', function (string $mountPoint, IStorage $storage) use ($requestUri, $baseuri, $share) {
Filesystem::addStorageWrapper('sharePermissions', function (string $mountPoint, IStorage $storage) use ($isUpload, $share) {
$mask = $share->getPermissions() | Constants::PERMISSION_SHARE;

// For chunked uploads it is necessary to have read and delete permission,
// so the temporary directory, chunks and destination file can be read and delete after the assembly.
if (str_starts_with(substr($requestUri, strlen($baseuri) - 1), '/uploads/')) {
if ($isUpload) {
$mask |= Constants::PERMISSION_READ | Constants::PERMISSION_DELETE;
}

Expand Down
5 changes: 3 additions & 2 deletions apps/dav/lib/Connector/Sabre/PublicAuth.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,16 @@ public function check(RequestInterface $request, ResponseInterface $response): a
* @throws NotFound
*/
private function getToken(): string {
$path = $this->request->getPathInfo() ?: '';
// Normalized the same way as Sabre resolves the requested node
$path = \Sabre\Uri\normalize($this->request->getRawPathInfo());
// ['', 'dav', 'files', 'token']
$splittedPath = explode('/', $path);

if (count($splittedPath) < 4 || $splittedPath[3] === '') {
throw new NotFound();
}

return $splittedPath[3];
return rawurldecode($splittedPath[3]);
}

/**
Expand Down
68 changes: 52 additions & 16 deletions apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,24 +79,60 @@ protected function tearDown(): void {
}

public function testGetToken(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$result = self::invokePrivate($this->auth, 'getToken');

$this->assertSame('GX9HSGQrGE', $result);
}

public static function dataGetTokenNormalizesPath(): array {
return [
'parent segment' => ['/dav/uploads/tokenA/../tokenB/upload-id/1', 'tokenB'],
'nested parent segments' => ['/dav/files/tokenA/../../uploads/tokenB/upload-id/1', 'tokenB'],
'current segment' => ['/dav/uploads/./tokenB/upload-id/1', 'tokenB'],
'encoded dots are not a parent segment' => ['/dav/uploads/%2e%2e/tokenB', '..'],
'encoded token' => ['/dav/files/token%41', 'tokenA'],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataGetTokenNormalizesPath')]
public function testGetTokenNormalizesPath(string $rawPath, string $expected): void {
$this->request->method('getRawPathInfo')
->willReturn($rawPath);

$this->assertSame($expected, self::invokePrivate($this->auth, 'getToken'));
}

public function testCheckTokenUsesNormalizedToken(): void {
$this->request->method('getRawPathInfo')
->willReturn('/dav/uploads/tokenA/../tokenB/upload-id/1');

$share = $this->createMock(IShare::class);
$share->method('isPasswordProtected')->willReturn(true);

$this->shareManager->expects($this->once())
->method('getShareByToken')
->with('tokenB')
->willReturn($share);

$this->session->method('exists')->with('public_link_authenticated')->willReturn(false);

$this->expectException(\Sabre\DAV\Exception\NotAuthenticated::class);
self::invokePrivate($this->auth, 'checkToken');
}

public function testGetTokenInvalid(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files');

$this->expectException(\Sabre\DAV\Exception\NotFound::class);
self::invokePrivate($this->auth, 'getToken');
}

public function testCheckTokenValidShare(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -113,7 +149,7 @@ public function testCheckTokenValidShare(): void {
}

public function testCheckTokenInvalidShare(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$this->shareManager
Expand All @@ -127,7 +163,7 @@ public function testCheckTokenInvalidShare(): void {
}

public function testCheckTokenAlreadyAuthenticated(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -146,7 +182,7 @@ public function testCheckTokenAlreadyAuthenticated(): void {
}

public function testCheckTokenPasswordNotAuthenticated(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -166,7 +202,7 @@ public function testCheckTokenPasswordNotAuthenticated(): void {
}

public function testCheckTokenPasswordAuthenticatedWrongShare(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -187,7 +223,7 @@ public function testCheckTokenPasswordAuthenticatedWrongShare(): void {
}

public function testNoShare(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$this->shareManager->expects($this->once())
Expand All @@ -201,7 +237,7 @@ public function testNoShare(): void {
}

public function testShareNoPassword(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -219,7 +255,7 @@ public function testShareNoPassword(): void {
}

public function testSharePasswordFancyShareType(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -238,7 +274,7 @@ public function testSharePasswordFancyShareType(): void {
}

public function testSharePasswordRemote(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -257,7 +293,7 @@ public function testSharePasswordRemote(): void {
}

public function testSharePasswordLinkValidPassword(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -282,7 +318,7 @@ public function testSharePasswordLinkValidPassword(): void {
}

public function testSharePasswordMailValidPassword(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand All @@ -307,7 +343,7 @@ public function testSharePasswordMailValidPassword(): void {
}

public function testInvalidSharePasswordLinkValidSession(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand Down Expand Up @@ -337,7 +373,7 @@ public function testInvalidSharePasswordLinkValidSession(): void {
}

public function testSharePasswordLinkInvalidSession(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand Down Expand Up @@ -367,7 +403,7 @@ public function testSharePasswordLinkInvalidSession(): void {
}

public function testSharePasswordMailInvalidSession(): void {
$this->request->method('getPathInfo')
$this->request->method('getRawPathInfo')
->willReturn('/dav/files/GX9HSGQrGE');

$share = $this->createMock(IShare::class);
Expand Down
Loading