diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index c4b3baf8..5476e546 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -26,6 +26,7 @@ jobs: php-versions: ['8.1'] databases: ['sqlite', 'mysql', 'pgsql'] server-versions: ['stable31'] + richdocuments-versions: ['stable31'] primary-storage: ['local', 'minio'] name: php${{ matrix.php-versions }}-${{ matrix.databases }}-${{ matrix.server-versions }}-${{ matrix.primary-storage}} @@ -74,6 +75,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout app (richdocuments) + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + path: apps/richdocuments + repository: nextcloud/richdocuments + ref: ${{ matrix.richdocuments-versions }} + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@c541c155eee45413f5b09a52248675b1a2575231 # v2.31.1 with: @@ -91,6 +100,14 @@ jobs: working-directory: apps/${{ env.APP_NAME }} run: composer i --no-dev + - name: Set up behat dependencies + working-directory: apps/${{ env.APP_NAME }}/tests/Integration + run: composer i + + - name: Set up dependencies (richdocuments) + working-directory: apps/richdocuments + run: composer i --no-dev + - name: Set up Nextcloud for S3 primary storage if: matrix.primary-storage == 'minio' run: | diff --git a/.github/workflows/phpunit-mariadb.yml b/.github/workflows/phpunit-mariadb.yml index bd5d4423..3dcc8405 100644 --- a/.github/workflows/phpunit-mariadb.yml +++ b/.github/workflows/phpunit-mariadb.yml @@ -70,21 +70,23 @@ jobs: matrix: php-versions: ${{ fromJson(needs.matrix.outputs.php-version) }} server-versions: ${{ fromJson(needs.matrix.outputs.server-max) }} - mariadb-versions: ['10.6', '10.11'] + mariadb-versions: ['10.6', '11.4'] + richdocuments-versions: ['main'] name: MariaDB ${{ matrix.mariadb-versions }} PHP ${{ matrix.php-versions }} Nextcloud ${{ matrix.server-versions }} services: mariadb: - image: ghcr.io/nextcloud/continuous-integration-mariadb-${{ matrix.mariadb-versions }}:latest + image: ghcr.io/nextcloud/continuous-integration-mariadb-${{ matrix.mariadb-versions }}:latest # zizmor: ignore[unpinned-images] ports: - 4444:3306/tcp env: - MYSQL_ROOT_PASSWORD: rootpassword - options: --health-cmd="mysqladmin ping" --health-interval 5s --health-timeout 2s --health-retries 5 + MARIADB_ROOT_PASSWORD: rootpassword + options: --health-cmd="mariadb-admin ping" --health-interval 5s --health-timeout 2s --health-retries 5 steps: - name: Set app env + if: ${{ env.APP_NAME == '' }} run: | # Split and keep last echo "APP_NAME=${GITHUB_REPOSITORY##*/}" >> $GITHUB_ENV @@ -103,8 +105,16 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout app (richdocuments) + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + path: apps/richdocuments + repository: nextcloud/richdocuments + ref: ${{ matrix.richdocuments-versions }} + - name: Set up php ${{ matrix.php-versions }} - uses: shivammathur/setup-php@c541c155eee45413f5b09a52248675b1a2575231 # v2.31.1 + uses: shivammathur/setup-php@0f7f1d08e3e32076e51cae65eb0b0c871405b16e # v2.34.1 with: php-version: ${{ matrix.php-versions }} # https://docs.nextcloud.com/server/stable/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation @@ -132,9 +142,13 @@ jobs: if: steps.check_composer.outputs.files_exists == 'true' working-directory: apps/${{ env.APP_NAME }} run: | - composer remove nextcloud/ocp --dev + composer remove nextcloud/ocp --dev --no-scripts composer i + - name: Set up dependencies (richdocuments) + working-directory: apps/richdocuments + run: composer i --no-dev + - name: Set up Nextcloud env: DB_PORT: 4444 diff --git a/.github/workflows/phpunit-mysql.yml b/.github/workflows/phpunit-mysql.yml index d0bf8649..27b52f02 100644 --- a/.github/workflows/phpunit-mysql.yml +++ b/.github/workflows/phpunit-mysql.yml @@ -32,7 +32,7 @@ jobs: id: versions uses: icewind1991/nextcloud-version-matrix@58becf3b4bb6dc6cef677b15e2fd8e7d48c0908f # v1.3.1 with: - matrix: '{"mysql-versions": ["8.4"]}' + matrix: '{"mysql-versions": ["8.4"], "richdocuments-versions": ["stable31"]}' changes: runs-on: ubuntu-latest-low @@ -101,6 +101,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout app (richdocuments) + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + path: apps/richdocuments + repository: nextcloud/richdocuments + ref: ${{ matrix.richdocuments-versions }} + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@c541c155eee45413f5b09a52248675b1a2575231 # v2.31.1 with: @@ -133,6 +141,10 @@ jobs: composer remove nextcloud/ocp --dev composer i + - name: Set up dependencies (richdocuments) + working-directory: apps/richdocuments + run: composer i --no-dev + - name: Set up Nextcloud env: DB_PORT: 4444 diff --git a/.github/workflows/phpunit-oci.yml b/.github/workflows/phpunit-oci.yml index cc1429da..e47044e7 100644 --- a/.github/workflows/phpunit-oci.yml +++ b/.github/workflows/phpunit-oci.yml @@ -70,6 +70,7 @@ jobs: matrix: php-versions: ${{ fromJson(needs.matrix.outputs.php-version) }} server-versions: ${{ fromJson(needs.matrix.outputs.server-max) }} + richdocuments-versions: ['stable31'] name: OCI PHP ${{ matrix.php-versions }} Nextcloud ${{ matrix.server-versions }} @@ -114,6 +115,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout app (richdocuments) + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + path: apps/richdocuments + repository: nextcloud/richdocuments + ref: ${{ matrix.richdocuments-versions }} + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@c541c155eee45413f5b09a52248675b1a2575231 # v2.31.1 with: @@ -141,6 +150,10 @@ jobs: composer remove nextcloud/ocp --dev composer i + - name: Set up dependencies (richdocuments) + working-directory: apps/richdocuments + run: composer i --no-dev + - name: Set up Nextcloud env: DB_PORT: 1521 diff --git a/.github/workflows/phpunit-pgsql.yml b/.github/workflows/phpunit-pgsql.yml index 643dab30..3e798079 100644 --- a/.github/workflows/phpunit-pgsql.yml +++ b/.github/workflows/phpunit-pgsql.yml @@ -70,6 +70,7 @@ jobs: matrix: php-versions: ${{ fromJson(needs.matrix.outputs.php-version) }} server-versions: ${{ fromJson(needs.matrix.outputs.server-max) }} + richdocuments-versions: ['stable31'] name: PostgreSQL PHP ${{ matrix.php-versions }} Nextcloud ${{ matrix.server-versions }} @@ -104,6 +105,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout app (richdocuments) + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + path: apps/richdocuments + repository: nextcloud/richdocuments + ref: ${{ matrix.richdocuments-versions }} + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@c541c155eee45413f5b09a52248675b1a2575231 # v2.31.1 with: @@ -131,6 +140,10 @@ jobs: composer remove nextcloud/ocp --dev composer i + - name: Set up dependencies (richdocuments) + working-directory: apps/richdocuments + run: composer i --no-dev + - name: Set up Nextcloud env: DB_PORT: 4444 diff --git a/.github/workflows/phpunit-sqlite.yml b/.github/workflows/phpunit-sqlite.yml index fca86835..f62c6278 100644 --- a/.github/workflows/phpunit-sqlite.yml +++ b/.github/workflows/phpunit-sqlite.yml @@ -70,6 +70,7 @@ jobs: matrix: php-versions: ${{ fromJson(needs.matrix.outputs.php-version) }} server-versions: ${{ fromJson(needs.matrix.outputs.server-max) }} + richdocuments-versions: ['stable31'] name: SQLite PHP ${{ matrix.php-versions }} Nextcloud ${{ matrix.server-versions }} @@ -93,6 +94,14 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Checkout app (richdocuments) + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + path: apps/richdocuments + repository: nextcloud/richdocuments + ref: ${{ matrix.richdocuments-versions }} + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@c541c155eee45413f5b09a52248675b1a2575231 # v2.31.1 with: @@ -120,6 +129,10 @@ jobs: composer remove nextcloud/ocp --dev composer i + - name: Set up dependencies (richdocuments) + working-directory: apps/richdocuments + run: composer i --no-dev + - name: Set up Nextcloud env: DB_PORT: 4444 diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 50ded6f6..a061e246 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -17,6 +17,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; @@ -40,13 +41,14 @@ public function addStorageWrapper(): void { * @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 b0c3b45a..8ed22666 100644 --- a/lib/CacheWrapper.php +++ b/lib/CacheWrapper.php @@ -9,20 +9,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 readonly int $mask; + protected readonly ?IStorage $storage; public function __construct( ICache $cache, - protected readonly IStorage $storage, + protected readonly IMountPoint $mountPoint, protected readonly Operation $operation, ) { parent::__construct($cache); + $this->storage = $mountPoint->getStorage(); $this->mask = Constants::PERMISSION_ALL & ~Constants::PERMISSION_READ & ~Constants::PERMISSION_CREATE @@ -33,7 +37,14 @@ public function __construct( 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 */ + $jailedPath = $storage->getJailedPath($path); + $path = $jailedPath ?? $path; + } + $this->operation->checkFileAccess($path, $this->mountPoint, $entry['mimetype'] === 'httpd/unix-directory', $entry); } catch (ForbiddenException) { $entry['permissions'] &= $this->mask; } diff --git a/lib/Operation.php b/lib/Operation.php index 44fb14c2..769ef4d8 100644 --- a/lib/Operation.php +++ b/lib/Operation.php @@ -18,8 +18,8 @@ use OCP\Files\ForbiddenException; use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountManager; +use OCP\Files\Mount\IMountPoint; use OCP\Files\Node; -use OCP\Files\NotFoundException; use OCP\Files\Storage\IStorage; use OCP\IL10N; use OCP\IURLGenerator; @@ -28,7 +28,6 @@ use OCP\WorkflowEngine\IRuleMatcher; use OCP\WorkflowEngine\ISpecificOperation; use Psr\Log\LoggerInterface; -use ReflectionClass; use UnexpectedValueException; class Operation implements IComplexOperation, ISpecificOperation { @@ -49,20 +48,24 @@ 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(); + if ($storage === null) { + return; + } $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); } @@ -79,24 +82,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; @@ -288,41 +275,22 @@ public function onEvent(string $eventName, Event $event, IRuleMatcher $ruleMatch // Noop } - /** - * @param array|ICacheEntry|null $cacheEntry - */ - private function getNode(IStorage $storage, string $path, $cacheEntry = null): ?Node { - $mountPoint = null; - $mounts = $this->mountManager->findByStorageId($storage->getId()); - foreach ($mounts as $mount) { - // we don't want to root mount; - if (strlen($mount->getMountPoint()) > 2) { - $mountPoint = $mount; - break; - } + private function getNode(string $path, IMountPoint $mountPoint, ICacheEntry|array|null $cacheEntry = null): ?Node { + $fullPath = $mountPoint->getMountPoint() . $path; + if (!$cacheEntry) { + $cacheEntry = $mountPoint->getStorage()?->getCache()->get($path); } - - if (!$mountPoint) { + if (!$cacheEntry) { return null; } - $fullPath = $mountPoint->getMountPoint() . $path; - if ($cacheEntry) { - // todo: LazyNode? - $info = new FileInfo($fullPath, $mountPoint->getStorage(), $path, $cacheEntry, $mountPoint); - $isDir = $info->getType() === \OCP\Files\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); - } - } else { - try { - return $this->rootFolder->get($fullPath); - } catch (NotFoundException $e) { - return null; - } + // todo: LazyNode? + $info = new FileInfo($fullPath, $mountPoint->getStorage(), $path, $cacheEntry, $mountPoint); + $isDir = $info->getType() === \OCP\Files\FileInfo::TYPE_FOLDER; + $view = new View(''); + if ($isDir) { + return new Folder($this->rootFolder, $view, $path, $info); } + return new \OC\Files\Node\File($this->rootFolder, $view, $path, $info); } } diff --git a/lib/StorageWrapper.php b/lib/StorageWrapper.php index 40c94604..d58451c9 100644 --- a/lib/StorageWrapper.php +++ b/lib/StorageWrapper.php @@ -8,12 +8,12 @@ namespace OCA\FilesAccessControl; -use OC\Files\Cache\Cache; use OC\Files\Storage\Storage; use OC\Files\Storage\Wrapper\Wrapper; use OCP\Constants; use OCP\Files\Cache\ICache; use OCP\Files\ForbiddenException; +use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IWriteStreamStorage; @@ -21,6 +21,7 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { protected readonly Operation $operation; public readonly string $mountPoint; protected readonly int $mask; + protected readonly IMountPoint $mount; /** * @param array $parameters @@ -29,6 +30,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 & ~Constants::PERMISSION_READ @@ -41,7 +43,7 @@ public function __construct($parameters) { * @throws ForbiddenException */ protected function checkFileAccess(string $path, ?bool $isDir = null): void { - $this->operation->checkFileAccess($this, $path, is_bool($isDir) ? $isDir : $this->is_dir($path)); + $this->operation->checkFileAccess($path, $this->mount, is_bool($isDir) ? $isDir : $this->is_dir($path)); } /* @@ -145,7 +147,7 @@ public function getPermissions($path): int { * see http://php.net/manual/en/function.file_get_contents.php * * @param string $path - * @return string + * @return string|false * @throws ForbiddenException */ public function file_get_contents($path): string|false { @@ -157,11 +159,11 @@ public function file_get_contents($path): string|false { * see http://php.net/manual/en/function.file_put_contents.php * * @param string $path - * @param string $data - * @return bool + * @param mixed $data + * @return int|float|false * @throws ForbiddenException */ - public function file_put_contents($path, $data): int|float|false { + public function file_put_contents(string $path, mixed $data): int|float|false { $this->checkFileAccess($path, false); return $this->storage->file_put_contents($path, $data); } @@ -213,7 +215,7 @@ public function copy($source, $target): bool { * * @param string $path * @param string $mode - * @return resource + * @return resource|false * @throws ForbiddenException */ public function fopen($path, $mode) { @@ -240,14 +242,14 @@ public function touch($path, $mtime = null): bool { * * @param string $path * @param Storage (optional) the storage to pass to the cache - * @return Cache + * @return ICache */ public function getCache($path = '', $storage = null): ICache { if (!$storage) { $storage = $this; } $cache = $this->storage->getCache($path, $storage); - return new CacheWrapper($cache, $storage, $this->operation); + return new CacheWrapper($cache, $this->mount, $this->operation); } /** @@ -256,7 +258,7 @@ public function getCache($path = '', $storage = null): ICache { * For now the returned array can hold the parameter url - in future more attributes might follow. * * @param string $path - * @return array + * @return array|false * @throws ForbiddenException */ public function getDirectDownload($path): array|false { @@ -304,7 +306,7 @@ public function writeStream(string $path, $stream, ?int $size = null): int { $this->checkFileAccess($path, false); } - $result = $this->storage->writeStream($path, $stream, $size); + $result = parent::writeStream($path, $stream, $size); if (!$this->isPartFile($path)) { return $result; } @@ -324,4 +326,8 @@ private function isPartFile(string $path): bool { $extension = pathinfo($path, PATHINFO_EXTENSION); return $extension === 'part'; } + + public function getMount(): IMountPoint { + return $this->mount; + } } diff --git a/psalm.xml b/psalm.xml index 4e5bf0bb..82e912be 100644 --- a/psalm.xml +++ b/psalm.xml @@ -26,6 +26,10 @@ + + + + diff --git a/tests/Integration/features/bootstrap/FeatureContext.php b/tests/Integration/features/bootstrap/FeatureContext.php index 41e87c81..be307711 100644 --- a/tests/Integration/features/bootstrap/FeatureContext.php +++ b/tests/Integration/features/bootstrap/FeatureContext.php @@ -45,6 +45,8 @@ class FeatureContext implements Context { protected string $tagId = ''; protected array $createdUsers = []; + protected array $changedConfigs = []; + /** * FeatureContext constructor. */ @@ -61,6 +63,12 @@ public function cleanUpBetweenTests() { $this->setCurrentUser('admin'); $this->sendingTo('DELETE', '/apps/files_accesscontrol_testing'); $this->assertStatusCode($this->response, 200); + + foreach ($this->changedConfigs as $appId => $configs) { + foreach ($configs as $config) { + $this->sendingTo('DELETE', '/apps/provisioning_api/api/v1/config/apps/' . $appId . '/' . $config); + } + } } /** @@ -133,6 +141,20 @@ public function userSharesFile(string $sharer, string $file, string $sharee): vo ]); } + /** + * @Given /^user "([^"]*)" shares file "([^"]*)" publicly$/ + */ + public function userSharesFilePublicly(string $sharer, string $file): void { + $this->setCurrentUser($sharer); + $this->sendingToWith('POST', '/apps/files_sharing/api/v1/shares', [ + 'path' => $file, + 'permissions' => 19, + 'shareType' => 3, + ]); + $responseBody = json_decode($this->response->getBody()->getContents(), true, flags: JSON_THROW_ON_ERROR); + $this->lastShareData = $responseBody['ocs']['data']; + } + // ChecksumsContext /** * @Then The webdav response should have a status code :statusCode @@ -150,6 +172,18 @@ public function theWebdavResponseShouldHaveAStatusCode($statusCode) { } } + + #[\Behat\Step\Given('the following :appId app config is set')] + public function setAppConfig(string $appId, TableNode $formData): void { + $this->setCurrentUser('admin'); + foreach ($formData->getRows() as $row) { + $this->sendingToWith('POST', '/apps/provisioning_api/api/v1/config/apps/' . $appId . '/' . $row[0], [ + 'value' => $row[1], + ]); + $this->changedConfigs[$appId][] = $row[0]; + } + } + /** * User management */ diff --git a/tests/Integration/features/bootstrap/WebDav.php b/tests/Integration/features/bootstrap/WebDav.php index 9e90e57b..98c4e05e 100644 --- a/tests/Integration/features/bootstrap/WebDav.php +++ b/tests/Integration/features/bootstrap/WebDav.php @@ -19,6 +19,7 @@ trait WebDav { /** @var int */ private $storedFileID = null; private array $trashedFiles = []; + protected array $lastShareData = []; /** * @Given /^using dav path "([^"]*)"$/ @@ -140,7 +141,7 @@ public function downloadFileWithRange($fileSource, $range) { * @param string $range */ public function downloadPublicFileWithRange($range) { - $token = $this->lastShareData->data->token; + $token = $this->lastShareData['token']; $fullUrl = $this->baseUrl . 'public.php/webdav'; $client = new GClient(); @@ -150,7 +151,11 @@ public function downloadPublicFileWithRange($range) { 'Range' => $range ]; - $this->response = $client->request('GET', $fullUrl, $options); + try { + $this->response = $client->request('GET', $fullUrl, $options); + } catch (\GuzzleHttp\Exception\ClientException $e) { + $this->response = $e->getResponse(); + } } /** @@ -158,7 +163,7 @@ public function downloadPublicFileWithRange($range) { * @param string $range */ public function downloadPublicFileInsideAFolderWithRange($path, $range) { - $token = $this->lastShareData->data->token; + $token = $this->lastShareData['token']; $fullUrl = $this->baseUrl . 'public.php/webdav' . "$path"; $client = new GClient(); @@ -169,7 +174,11 @@ public function downloadPublicFileInsideAFolderWithRange($path, $range) { ]; $options['auth'] = [$token, '']; - $this->response = $client->request('GET', $fullUrl, $options); + try { + $this->response = $client->request('GET', $fullUrl, $options); + } catch (\GuzzleHttp\Exception\ClientException $e) { + $this->response = $e->getResponse(); + } } /** @@ -189,8 +198,13 @@ public function downloadedContentShouldBe($content) { */ public function checkPropForFile($file, $prefix, $prop, $value) { $elementList = $this->propfindFile($this->currentUser, $file, "<$prefix:$prop/>"); - $property = $elementList['/' . $this->getDavFilesPath($this->currentUser) . $file][200]["{DAV:}$prop"]; - Assert::assertEquals($property, $value); + if ($prefix === 'oc') { + $prefix = '{http://owncloud.org/ns}'; + } else { + $prefix = '{DAV:}'; + } + $property = $elementList['/' . $this->getDavFilesPath($this->currentUser) . $file][200]["$prefix$prop"]; + Assert::assertEquals($value, $property); } /** diff --git a/tests/Integration/features/mimetypes.feature b/tests/Integration/features/mimetypes.feature index 607a6780..ec39d98f 100644 --- a/tests/Integration/features/mimetypes.feature +++ b/tests/Integration/features/mimetypes.feature @@ -5,7 +5,7 @@ Given as user "test1" And using new dav path - Scenario: Can properly block path detected mimetypes for application/javscript + Scenario: Can properly block path detected mimetypes for application/javascript And user "admin" creates global flow with 200 | name | Admin flow | | class | OCA\FilesAccessControl\Operation | diff --git a/tests/Integration/features/sharing-user.feature b/tests/Integration/features/sharing-user.feature index 6c0da385..e30a9167 100644 --- a/tests/Integration/features/sharing-user.feature +++ b/tests/Integration/features/sharing-user.feature @@ -172,3 +172,35 @@ Feature: Sharing user And The webdav response should have a status code "404" And user "test2" should see following elements | /nextcloud2.txt | + + Scenario: Downloading is still blocked when Secure View is enabled + Given the following files app config is set + | watermark_enabled | yes | + Given User "test1" uploads file "data/textfile.txt" to "/foobar.txt" + And The webdav response should have a status code "201" + And user "test1" shares file "/foobar.txt" with user "test2" + And as user "test2" + When File "/foobar.txt" should have prop "oc:permissions" equal to "SRGDNVW" + When Downloading file "/foobar.txt" + Then The webdav response should have a status code "200" + When Downloading file "/foobar.txt" with range "1-4" + Then The webdav response should have a status code "200" + And user "test1" shares file "/foobar.txt" publicly + And as user "test2" + When Downloading last public shared file with range "1-4" + Then The webdav response should have a status code "200" + And user "admin" creates global flow with 200 + | name | Admin flow | + | class | OCA\FilesAccessControl\Operation | + | entity | OCA\WorkflowEngine\Entity\File | + | events | [] | + | operation | deny | + | checks-0 | {"class":"OCA\\\\WorkflowEngine\\\\Check\\\\FileMimeType", "operator": "is", "value": "text/plain"} | + And as user "test2" + When File "/foobar.txt" should have prop "oc:permissions" equal to "SRD" + When Downloading file "/foobar.txt" + Then The webdav response should have a status code "404" + When Downloading file "/foobar.txt" with range "1-4" + Then The webdav response should have a status code "404" + When Downloading last public shared file with range "1-4" + Then The webdav response should have a status code "404" diff --git a/tests/Integration/run.sh b/tests/Integration/run.sh index a8d5910d..e6ab34b1 100755 --- a/tests/Integration/run.sh +++ b/tests/Integration/run.sh @@ -16,6 +16,7 @@ composer install cp -R ./app "../../../${APP_NAME}_testing" ${ROOT_DIR}/occ app:enable $APP_NAME ${ROOT_DIR}/occ app:enable --force "${APP_NAME}_testing" +${ROOT_DIR}/occ app:enable --force richdocuments ${ROOT_DIR}/occ app:list | grep $APP_NAME export TEST_SERVER_URL="http://localhost:8080/" diff --git a/tests/Unit/StorageWrapperTest.php b/tests/Unit/StorageWrapperTest.php index 204232d5..9c6ca852 100644 --- a/tests/Unit/StorageWrapperTest.php +++ b/tests/Unit/StorageWrapperTest.php @@ -11,6 +11,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; @@ -19,11 +20,20 @@ class StorageWrapperTest extends TestCase { protected IStorage&MockObject $storage; protected Operation&MockObject $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 = []): StorageWrapper&MockObject { @@ -32,6 +42,7 @@ protected function getInstance(array $methods = []): StorageWrapper&MockObject { [ 'storage' => $this->storage, 'mountPoint' => 'mountPoint', + 'mount' => $this->mountPoint, 'operation' => $this->operation, ] ]) @@ -54,7 +65,7 @@ public function testCheckFileAccess(string $path, bool $isDir): void { $this->operation->expects($this->once()) ->method('checkFileAccess') - ->with($storage, $path); + ->with($path, $this->mountPoint, $isDir); self::invokePrivate($storage, 'checkFileAccess', [$path, $isDir]); } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index b17e249b..62960cba 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -6,18 +6,10 @@ - - - - - fileEntity]]> - - - @@ -31,12 +23,4 @@ - - - - - - - - diff --git a/tests/stubs/oc_files_cache_cache.php b/tests/stubs/oc_files_cache_cache.php new file mode 100644 index 00000000..2a6c7a08 --- /dev/null +++ b/tests/stubs/oc_files_cache_cache.php @@ -0,0 +1,77 @@ +cache = $cache; + } + public function getNumericStorageId() { + } + public function getIncomplete() { + } + public function getPathById($id) { + } + public function getAll() { + } + public function get($file) { + } + public function getFolderContents($folder) { + } + public function getFolderContentsById($fileId) { + } + public function put($file, array $data) { + } + public function insert($file, array $data) { + } + public function update($id, array $data) { + } + public function getId($file) { + } + public function getParentId($file) { + } + public function inCache($file) { + } + public function remove($file) { + } + public function move($source, $target) { + } + public function moveFromCache(ICache $sourceCache, $sourcePath, $targetPath) { + } + public function clear() { + } + public function getStatus($file) { + } + public function search($pattern) { + } + public function searchByMime($mimetype) { + } + public function searchQuery(ISearchQuery $query) { + } + public function correctFolderSize($path, $data = null, $isBackgroundScan = false) { + } + public function copyFromCache(ICache $sourceCache, ICacheEntry $sourceEntry, string $targetPath): int { + } + public function normalize($path) { + } + public function getQueryFilterForStorage(): ISearchOperator { + } + public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { + } + public static function cacheEntryFromData($data, IMimeTypeLoader $mimetypeLoader) { + } + } +} diff --git a/tests/stubs/oc_files_cache_wrapper_cachewrapper.php b/tests/stubs/oc_files_cache_wrapper_cachewrapper.php new file mode 100644 index 00000000..2d99ba2e --- /dev/null +++ b/tests/stubs/oc_files_cache_wrapper_cachewrapper.php @@ -0,0 +1,21 @@ + $class + * @psalm-return T|null + */ + public function getInstanceOfStorage(string $class): ?IStorage { + } + + /** + * Pass any methods custom to specific storage implementations to the wrapped storage + * + * @param string $method + * @param array $args + * @return mixed + */ + public function __call($method, $args) { + } + + public function getDirectDownload($path): array|false { + } + + public function getAvailability(): array { + } + + public function setAvailability($isAvailable): void { + } + + public function verifyPath($path, $fileName): void { + } + + public function copyFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath): bool { + } + + public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath): bool { + } + + public function getMetaData($path): ?array { + } + + public function acquireLock($path, $type, ILockingProvider $provider): void { + } + + public function releaseLock($path, $type, ILockingProvider $provider): void { + } + + public function changeLock($path, $type, ILockingProvider $provider): void { + } + + public function needsPartFile(): bool { + } + + public function writeStream(string $path, $stream, ?int $size = null): int { + } + + public function getDirectoryContent($directory): \Traversable { + } + + public function isWrapperOf(IStorage $storage): bool { + } + + public function setOwner(?string $user): void { + } + } +}