Skip to content
Draft
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
16 changes: 15 additions & 1 deletion lib/ACL/ACLStorageWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,25 @@ class ACLStorageWrapper extends Wrapper implements IConstructableStorage {
private readonly bool $inShare;
private readonly int $folderId;
private readonly int $storageId;
private readonly bool $isTeamSpace;

/**
* @param array{storage: Storage, acl_manager: ACLManager, in_share: bool, folder_id: int, storage_id: int} $arguments
* @param array{storage: Storage, acl_manager: ACLManager, in_share: bool, folder_id: int, storage_id: int, is_team_space: bool} $arguments
*/
public function __construct(array $arguments) {
parent::__construct($arguments);
$this->aclManager = $arguments['acl_manager'];
$this->inShare = $arguments['in_share'];
$this->folderId = $arguments['folder_id'];
$this->storageId = $arguments['storage_id'];
$this->isTeamSpace = $arguments['is_team_space'];
}

private function getACLPermissionsForPath(string $path): int {
if ($this->isProtectedTeamSpacePath($path)) {
return Constants::PERMISSION_ALL;
}

$permissions = $this->aclManager->getACLPermissionsForPath($this->folderId, $this->storageId, $path);

// if there is no read permissions, than deny everything
Expand All @@ -47,6 +53,10 @@ private function getACLPermissionsForPath(string $path): int {
return $canRead ? $permissions : 0;
}

private function isProtectedTeamSpacePath(string $path): bool {
return $this->isTeamSpace && ($path === '.system' || str_starts_with($path, '.system/'));
}

private function checkPermissions(string $path, int $permissions): bool {
return ($this->getACLPermissionsForPath($path) & $permissions) === $permissions;
}
Expand Down Expand Up @@ -169,6 +179,10 @@ public function unlink(string $path): bool {
* This check is fairly expensive so we only do it for the actual delete and not metadata operations
*/
private function canDeleteTree(string $path): int {
if ($this->isProtectedTeamSpacePath($path)) {
return Constants::PERMISSION_DELETE;
}

return $this->aclManager->getPermissionsForTree($this->folderId, $this->storageId, $path) & Constants::PERMISSION_DELETE;
}

Expand Down
15 changes: 14 additions & 1 deletion lib/DAV/ACLPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ private function getParents(string $path): array {
return $paths;
}

private function isProtectedTeamSpacePath(GroupMountPoint $mount, string $path): bool {
$folder = $this->folderManager->getFolder($mount->getFolderId());
if ($folder === null || !$folder->isTeamSpace()) {
return false;
}

$path = ltrim($path, '/');
return $path === '.system' || str_starts_with($path, '.system/');
}

public function propFind(PropFind $propFind, INode $node): void {
if (!$node instanceof Node) {
return;
Expand Down Expand Up @@ -227,7 +237,7 @@ public function propPatch(string $path, PropPatch $propPatch): void {
}

// Mapping the old property to the new property.
$propPatch->handle(self::ACL_LIST, function (array $rawRules) use ($path): bool {
$propPatch->handle(self::ACL_LIST, function (array $rawRules) use ($path, $mount): bool {
if ($this->server === null) {
return false;
}
Expand All @@ -238,6 +248,9 @@ public function propPatch(string $path, PropPatch $propPatch): void {
}

$fileInfo = $node->getFileInfo();
if ($this->isProtectedTeamSpacePath($mount, $fileInfo->getInternalPath())) {
throw new BadRequest($this->l10n->t('Advanced permissions cannot be changed for the reserved .system directory.'));
}

$fileInfoId = $fileInfo->getId();
if ($fileInfoId === null) {
Expand Down
32 changes: 32 additions & 0 deletions lib/Migration/Version2300000Date20260922000000.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\GroupFolders\Migration;

use Closure;
use OCP\DB\IQueryBuilder;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

class Version2300000Date20260922000000 extends SimpleMigrationStep {
public function __construct(
private readonly IDBConnection $connection,
) {
}

#[\Override]
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
$query = $this->connection->getQueryBuilder();
$query->update('group_folders')
->set('acl', $query->createNamedParameter(true, IQueryBuilder::PARAM_BOOL))

Check failure on line 28 in lib/Migration/Version2300000Date20260922000000.php

View workflow job for this annotation

GitHub Actions / static-phpstan-analysis

Access to constant PARAM_BOOL on an unknown class OCP\DB\IQueryBuilder.
->where($query->expr()->isNotNull('team_circle_id'));
$query->executeStatement();
}
}
2 changes: 2 additions & 0 deletions lib/Mount/FolderStorageManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ private function getBaseStorageForFolderSeparate(
'acl_manager' => $aclManager,
'in_share' => $inShare,
'folder_id' => $folderId,
'is_team_space' => $folder->isTeamSpace(),
// already loaded with the folder; avoids a storages lookup per folder
'storage_id' => $folder->storageId,
]);
Expand Down Expand Up @@ -251,6 +252,7 @@ private function getBaseStorageForFolderRootJail(
'acl_manager' => $aclManager,
'in_share' => $inShare,
'folder_id' => $folderId,
'is_team_space' => $folder->isTeamSpace(),
// already loaded; the same id MountProvider uses for rule lookups (incl. legacy root-jail)
'storage_id' => $folder->storageId,
]);
Expand Down
1 change: 1 addition & 0 deletions lib/TeamSpace/TeamSpaceService.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public function createTeamSpace(string $circleId, string $mountPoint, int $quota
}

$this->folderManager->addApplicableGroup($folderId, $circleId);
$this->folderManager->setFolderACL($folderId, true);
$this->folderManager->setManageACL($folderId, 'circle', $circleId, true);

$this->folderManager->setTeamCircleId($folderId, $circleId);
Expand Down
1 change: 1 addition & 0 deletions tests/ACL/ACLScannerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public function testScanAclStorage(): void {
'in_share' => false,
'folder_id' => 0,
'storage_id' => $cache->getNumericStorageId(),
'is_team_space' => false,
]);

$scanner = $aclStorage->getScanner();
Expand Down
31 changes: 31 additions & 0 deletions tests/ACL/ACLStorageWrapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,40 @@
'in_share' => false,
'folder_id' => 0,
'storage_id' => $this->source->getCache()->getNumericStorageId(),
'is_team_space' => true,
]);
}

public function testTeamSpaceSystemDirectoryBypassesACL(): void {
$this->source->mkdir('.system');
$this->source->touch('.system/app-data.json');
$this->source->mkdir('.system-copy');

$this->aclPermissions['.system'] = 0;
$this->aclPermissions['.system/app-data.json'] = 0;
$this->aclPermissions['.system-copy'] = 0;

$this->assertTrue($this->storage->isReadable('.system'));
$this->assertTrue($this->storage->isReadable('.system/app-data.json'));
$this->assertFalse($this->storage->isReadable('.system-copy'));
}

public function testRegularGroupFolderSystemDirectoryHonorsACL(): void {
$this->source->mkdir('.system');
$this->aclPermissions['.system'] = 0;

$storage = new ACLStorageWrapper([

Check failure on line 62 in tests/ACL/ACLStorageWrapperTest.php

View workflow job for this annotation

GitHub Actions / static-phpstan-analysis

Parameter #1 $arguments of class OCA\GroupFolders\ACL\ACLStorageWrapper constructor expects array{storage: OC\Files\Storage\Storage, acl_manager: OCA\GroupFolders\ACL\ACLManager, in_share: bool, folder_id: int, storage_id: int, is_team_space: bool}, array{storage: OCP\Files\Storage\IStorage, acl_manager: OCA\GroupFolders\ACL\ACLManager&PHPUnit\Framework\MockObject\MockObject, in_share: false, folder_id: 0, storage_id: int, is_team_space: false} given.
'storage' => $this->source,
'acl_manager' => $this->aclManager,
'in_share' => false,
'folder_id' => 0,
'storage_id' => $this->source->getCache()->getNumericStorageId(),
'is_team_space' => false,
]);

$this->assertFalse($storage->isReadable('.system'));
}

public function testNoReadImpliesNothing(): void {
$this->source->mkdir('foo');
$this->aclPermissions['foo'] = Constants::PERMISSION_ALL - Constants::PERMISSION_READ;
Expand Down
1 change: 1 addition & 0 deletions tests/TeamSpace/TeamSpaceServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ static function (string $path) use (&$scannedPaths): void {
);
$this->folderManager->expects($this->once())->method('setFolderQuota')->with(42, 1024);
$this->folderManager->expects($this->once())->method('addApplicableGroup')->with(42, 'team-1');
$this->folderManager->expects($this->once())->method('setFolderACL')->with(42, true);
$this->folderManager->expects($this->once())->method('setManageACL')->with(42, 'circle', 'team-1', true);
$this->folderManager->expects($this->once())->method('setTeamCircleId')->with(42, 'team-1');

Expand Down
Loading