diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index bd177aab..d3ee2b2e 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -30,6 +30,7 @@ use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; +use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use OCP\Util; use OCP\WorkflowEngine\Events\RegisterOperationsEvent; @@ -53,13 +54,14 @@ public function addStorageWrapper() { * @param IStorage $storage * @return StorageWrapper|IStorage */ - public function addStorageWrapperCallback($mountPoint, IStorage $storage) { + public function addStorageWrapperCallback($mountPoint, IStorage $storage, IMountPoint $mount) { if (!OC::$CLI && $mountPoint !== '/') { /** @var Operation $operation */ $operation = $this->getContainer()->get(Operation::class); return new StorageWrapper([ 'storage' => $storage, 'mountPoint' => $mountPoint, + 'mount' => $mount, 'operation' => $operation, ]); } diff --git a/lib/CacheWrapper.php b/lib/CacheWrapper.php index ecaa37e5..9ec4e9c0 100644 --- a/lib/CacheWrapper.php +++ b/lib/CacheWrapper.php @@ -22,25 +22,24 @@ namespace OCA\FilesAccessControl; use OC\Files\Cache\Wrapper\CacheWrapper as Wrapper; +use OC\Files\Storage\Wrapper\Jail; use OCP\Constants; use OCP\Files\Cache\ICache; use OCP\Files\ForbiddenException; +use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; class CacheWrapper extends Wrapper { protected Operation $operation; protected IStorage $storage; + protected IMountPoint $mountPoint; protected int $mask; - /** - * @param ICache $cache - * @param IStorage $storage - * @param Operation $operation - */ - public function __construct(ICache $cache, IStorage $storage, Operation $operation) { + public function __construct(ICache $cache, Operation $operation, IMountPoint $mountPoint) { parent::__construct($cache); - $this->storage = $storage; + $this->storage = $mountPoint->getStorage(); $this->operation = $operation; + $this->mountPoint = $mountPoint; $this->mask = Constants::PERMISSION_ALL; $this->mask &= ~Constants::PERMISSION_READ; @@ -57,7 +56,13 @@ public function __construct(ICache $cache, IStorage $storage, Operation $operati protected function formatCacheEntry($entry) { if (isset($entry['path']) && isset($entry['permissions'])) { try { - $this->operation->checkFileAccess($this->storage, $entry['path'], $entry['mimetype'] === 'httpd/unix-directory', $entry); + $storage = $this->storage; + $path = $entry['path']; + if ($storage->instanceOfStorage(Jail::class)) { + /** @var Jail $storage */ + $path = $storage->getJailedPath($path); + } + $this->operation->checkFileAccess($path, $this->mountPoint, $entry['mimetype'] === 'httpd/unix-directory', $entry); } catch (ForbiddenException $e) { $entry['permissions'] &= $this->mask; } diff --git a/lib/Operation.php b/lib/Operation.php index 8b215b68..15fc0517 100644 --- a/lib/Operation.php +++ b/lib/Operation.php @@ -77,20 +77,21 @@ public function __construct( * @param array|ICacheEntry|null $cacheEntry * @throws ForbiddenException */ - public function checkFileAccess(IStorage $storage, string $path, bool $isDir = false, $cacheEntry = null): void { - if (!$this->isBlockablePath($storage, $path) || $this->isCreatingSkeletonFiles() || $this->nestingLevel !== 0) { + public function checkFileAccess(string $path, IMountPoint $mountPoint, bool $isDir, $cacheEntry = null): void { + if (!$this->isBlockablePath($mountPoint, $path) || $this->isCreatingSkeletonFiles() || $this->nestingLevel !== 0) { // Allow creating skeletons and theming // https://github.com/nextcloud/files_accesscontrol/issues/5 // https://github.com/nextcloud/files_accesscontrol/issues/12 return; } + $storage = $mountPoint->getStorage(); $this->nestingLevel++; $filePath = $this->translatePath($storage, $path); $ruleMatcher = $this->manager->getRuleMatcher(); $ruleMatcher->setFileInfo($storage, $filePath, $isDir); - $node = $this->getNode($storage, $path, $cacheEntry); + $node = $this->getNode($path, $mountPoint, $cacheEntry); if ($node !== null) { $ruleMatcher->setEntitySubject($this->fileEntity, $node); } @@ -107,24 +108,8 @@ public function checkFileAccess(IStorage $storage, string $path, bool $isDir = f } } - protected function isBlockablePath(IStorage $storage, string $path): bool { - if (property_exists($storage, 'mountPoint')) { - $hasMountPoint = $storage instanceof StorageWrapper; - if (!$hasMountPoint) { - $ref = new ReflectionClass($storage); - $prop = $ref->getProperty('mountPoint'); - $hasMountPoint = $prop->isPublic(); - } - - if ($hasMountPoint) { - /** @var StorageWrapper $storage */ - $fullPath = $storage->mountPoint . ltrim($path, '/'); - } else { - $fullPath = $path; - } - } else { - $fullPath = $path; - } + protected function isBlockablePath(IMountPoint $mountPoint, string $path): bool { + $fullPath = $mountPoint->getMountPoint() . ltrim($path, '/'); if (substr_count($fullPath, '/') < 3) { return false; @@ -289,30 +274,23 @@ public function onEvent(string $eventName, Event $event, IRuleMatcher $ruleMatch /** * @param array|ICacheEntry|null $cacheEntry */ - private function getNode(IStorage $storage, string $path, $cacheEntry = null): ?Node { - /** @var IMountPoint|false $mountPoint */ - $mountPoint = current($this->mountManager->findByStorageId($storage->getId())); - if (!$mountPoint) { + private function getNode(string $path, IMountPoint $mountPoint, $cacheEntry = null): ?Node { + $fullPath = $mountPoint->getMountPoint() . $path; + if (!$cacheEntry) { + $cacheEntry = $mountPoint->getStorage()->getCache()->get($path); + } + if (!$cacheEntry) { return null; } - $fullPath = $mountPoint->getMountPoint() . $path; - if ($cacheEntry) { - // todo: LazyNode? - $info = new FileInfo($fullPath, $mountPoint->getStorage(), $path, $cacheEntry, $mountPoint); - $isDir = $info->getType() === FileInfo::TYPE_FOLDER; - $view = new View(''); - if ($isDir) { - return new Folder($this->rootFolder, $view, $path, $info); - } else { - return new \OC\Files\Node\File($this->rootFolder, $view, $path, $info); - } + // todo: LazyNode? + $info = new FileInfo($fullPath, $mountPoint->getStorage(), $path, $cacheEntry, $mountPoint); + $isDir = $info->getType() === FileInfo::TYPE_FOLDER; + $view = new View(''); + if ($isDir) { + return new Folder($this->rootFolder, $view, $path, $info); } else { - try { - return $this->rootFolder->get($fullPath); - } catch (NotFoundException $e) { - return null; - } + return new \OC\Files\Node\File($this->rootFolder, $view, $path, $info); } } } diff --git a/lib/StorageWrapper.php b/lib/StorageWrapper.php index 29b0786b..a386f7fc 100644 --- a/lib/StorageWrapper.php +++ b/lib/StorageWrapper.php @@ -26,12 +26,14 @@ use OC\Files\Storage\Wrapper\Wrapper; use OCP\Constants; use OCP\Files\ForbiddenException; +use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IWriteStreamStorage; class StorageWrapper extends Wrapper implements IWriteStreamStorage { /** @var Operation */ protected $operation; + protected IMountPoint $mount; /** @var string */ public $mountPoint; @@ -45,6 +47,7 @@ public function __construct($parameters) { parent::__construct($parameters); $this->operation = $parameters['operation']; $this->mountPoint = $parameters['mountPoint']; + $this->mount = $parameters['mount']; $this->mask = Constants::PERMISSION_ALL; $this->mask &= ~Constants::PERMISSION_READ; @@ -57,7 +60,7 @@ public function __construct($parameters) { * @throws ForbiddenException */ protected function checkFileAccess(string $path, bool $isDir = false): void { - $this->operation->checkFileAccess($this, $path, $isDir); + $this->operation->checkFileAccess($path, $this->mount, $isDir); } /* @@ -261,7 +264,7 @@ public function getCache($path = '', $storage = null) { $storage = $this; } $cache = $this->storage->getCache($path, $storage); - return new CacheWrapper($cache, $storage, $this->operation); + return new CacheWrapper($cache, $this->operation, $this->mount); } /** diff --git a/tests/Unit/StorageWrapperTest.php b/tests/Unit/StorageWrapperTest.php index 383b29e2..8e14f74f 100644 --- a/tests/Unit/StorageWrapperTest.php +++ b/tests/Unit/StorageWrapperTest.php @@ -26,6 +26,7 @@ use OCA\FilesAccessControl\Operation; use OCA\FilesAccessControl\StorageWrapper; use OCP\Files\ForbiddenException; +use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -38,12 +39,19 @@ class StorageWrapperTest extends TestCase { /** @var Operation|MockObject */ protected $operation; + /** @var IMountPoint|MockObject */ + protected $mountPoint; + protected function setUp(): void { parent::setUp(); $this->storage = $this->createMock(IStorage::class); $this->operation = $this->createMock(Operation::class); + + $this->mountPoint = $this->createMock(IMountPoint::class); + $this->mountPoint->method('getMountPoint')->willReturn('mountPoint'); + $this->mountPoint->method('getStorage')->willReturn($this->storage); } protected function getInstance(array $methods = []) { @@ -52,6 +60,7 @@ protected function getInstance(array $methods = []) { [ 'storage' => $this->storage, 'mountPoint' => 'mountPoint', + 'mount' => $this->mountPoint, 'operation' => $this->operation, ] ]) @@ -75,7 +84,7 @@ public function testCheckFileAccess(string $path): void { $this->operation->expects($this->once()) ->method('checkFileAccess') - ->with($storage, $path); + ->with($path, $this->mountPoint, false); self::invokePrivate($storage, 'checkFileAccess', [$path]); }