Skip to content

DAV permissions for read only single file shares indicate updatable #53041

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.

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

Open
juliusknorr opened this issue May 22, 2025 · 4 comments
Open
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 32-feedback bug

Comments

@juliusknorr
Copy link
Member

This seems like a bug that i noticed when trying to check for the permissions using @nextcloud/files where it turned out my file action did not hide on read only shares.

Steps to reproduce

  • as userA: Share a single file as view only (permission: 1) to userB
  • as userB: Check the webdav response and see <oc:permissions>SGDNV</oc:permissions> indicating that the file can be updated

Additional context

  • This does not happen with folder shares, there both the root mount and files within have correct webdav permissions exposed
  • I already spend some time stepping through code
    • Permissions in the FileInfo in
      $permissions = $info->getPermissions();
      are already wrong with 11 (unsure where that is originating from, but my guess is somewhere in
      public function getDirectoryContent($directory, $mimetype_filter = '', ?\OCP\Files\FileInfo $directoryInfo = null) {
      $this->assertPathLength($directory);
      if (!Filesystem::isValidPath($directory)) {
      return [];
      }
      $path = $this->getAbsolutePath($directory);
      $path = Filesystem::normalizePath($path);
      $mount = $this->getMount($directory);
      $storage = $mount->getStorage();
      $internalPath = $mount->getInternalPath($path);
      if (!$storage) {
      return [];
      }
      $cache = $storage->getCache($internalPath);
      $user = \OC_User::getUser();
      if (!$directoryInfo) {
      $data = $this->getCacheEntry($storage, $internalPath, $directory);
      if (!$data instanceof ICacheEntry || !isset($data['fileid'])) {
      return [];
      }
      } else {
      $data = $directoryInfo;
      }
      if (!($data->getPermissions() & Constants::PERMISSION_READ)) {
      return [];
      }
      $folderId = $data->getId();
      $contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter
      $sharingDisabled = \OCP\Util::isSharingDisabledForUser();
      $fileNames = array_map(function (ICacheEntry $content) {
      return $content->getName();
      }, $contents);
      /**
      * @var \OC\Files\FileInfo[] $fileInfos
      */
      $fileInfos = array_map(function (ICacheEntry $content) use ($path, $storage, $mount, $sharingDisabled) {
      if ($sharingDisabled) {
      $content['permissions'] = $content['permissions'] & ~\OCP\Constants::PERMISSION_SHARE;
      }
      $ownerId = $storage->getOwner($content['path']);
      if ($ownerId !== false) {
      $owner = $this->getUserObjectForOwner($ownerId);
      } else {
      $owner = null;
      }
      return new FileInfo($path . '/' . $content['name'], $storage, $content['path'], $content, $mount, $owner);
      }, $contents);
      $files = array_combine($fileNames, $fileInfos);
      //add a folder for any mountpoint in this directory and add the sizes of other mountpoints to the folders
      $mounts = Filesystem::getMountManager()->findIn($path);
      // make sure nested mounts are sorted after their parent mounts
      // otherwise doesn't propagate the etag across storage boundaries correctly
      usort($mounts, function (IMountPoint $a, IMountPoint $b) {
      return $a->getMountPoint() <=> $b->getMountPoint();
      });
      $dirLength = strlen($path);
      foreach ($mounts as $mount) {
      $mountPoint = $mount->getMountPoint();
      $subStorage = $mount->getStorage();
      if ($subStorage) {
      $subCache = $subStorage->getCache('');
      $rootEntry = $subCache->get('');
      if (!$rootEntry) {
      $subScanner = $subStorage->getScanner();
      try {
      $subScanner->scanFile('');
      } catch (\OCP\Files\StorageNotAvailableException $e) {
      continue;
      } catch (\OCP\Files\StorageInvalidException $e) {
      continue;
      } catch (\Exception $e) {
      // sometimes when the storage is not available it can be any exception
      $this->logger->error('Exception while scanning storage "' . $subStorage->getId() . '"', [
      'exception' => $e,
      'app' => 'core',
      ]);
      continue;
      }
      $rootEntry = $subCache->get('');
      }
      if ($rootEntry && ($rootEntry->getPermissions() & Constants::PERMISSION_READ)) {
      $relativePath = trim(substr($mountPoint, $dirLength), '/');
      if ($pos = strpos($relativePath, '/')) {
      //mountpoint inside subfolder add size to the correct folder
      $entryName = substr($relativePath, 0, $pos);
      // Create parent folders if the mountpoint is inside a subfolder that doesn't exist yet
      if (!isset($files[$entryName])) {
      try {
      [$storage, ] = $this->resolvePath($path . '/' . $entryName);
      // make sure we can create the mountpoint folder, even if the user has a quota of 0
      if ($storage->instanceOfStorage(Quota::class)) {
      $storage->enableQuota(false);
      }
      if ($this->mkdir($path . '/' . $entryName) !== false) {
      $info = $this->getFileInfo($path . '/' . $entryName);
      if ($info !== false) {
      $files[$entryName] = $info;
      }
      }
      if ($storage->instanceOfStorage(Quota::class)) {
      $storage->enableQuota(true);
      }
      } catch (\Exception $e) {
      // Creating the parent folder might not be possible, for example due to a lack of permissions.
      $this->logger->debug('Failed to create non-existent parent', ['exception' => $e, 'path' => $path . '/' . $entryName]);
      }
      }
      if (isset($files[$entryName])) {
      $files[$entryName]->addSubEntry($rootEntry, $mountPoint);
      }
      } else { //mountpoint in this folder, add an entry for it
      $rootEntry['name'] = $relativePath;
      $rootEntry['type'] = $rootEntry['mimetype'] === 'httpd/unix-directory' ? 'dir' : 'file';
      $permissions = $rootEntry['permissions'];
      // do not allow renaming/deleting the mount point if they are not shared files/folders
      // for shared files/folders we use the permissions given by the owner
      if ($mount instanceof MoveableMount) {
      $rootEntry['permissions'] = $permissions | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE;
      } else {
      $rootEntry['permissions'] = $permissions & (\OCP\Constants::PERMISSION_ALL - (\OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE));
      }
      $rootEntry['path'] = substr(Filesystem::normalizePath($path . '/' . $rootEntry['name']), strlen($user) + 2); // full path without /$user/
      // if sharing was disabled for the user we remove the share permissions
      if ($sharingDisabled) {
      $rootEntry['permissions'] = $rootEntry['permissions'] & ~\OCP\Constants::PERMISSION_SHARE;
      }
      $ownerId = $subStorage->getOwner('');
      if ($ownerId !== false) {
      $owner = $this->getUserObjectForOwner($ownerId);
      } else {
      $owner = null;
      }
      $files[$rootEntry->getName()] = new FileInfo($path . '/' . $rootEntry['name'], $subStorage, '', $rootEntry, $mount, $owner);
      }
      }
      }
      }
      if ($mimetype_filter) {
      $files = array_filter($files, function (FileInfo $file) use ($mimetype_filter) {
      if (strpos($mimetype_filter, '/')) {
      return $file->getMimetype() === $mimetype_filter;
      } else {
      return $file->getMimePart() === $mimetype_filter;
      }
      });
      }
      return array_values($files);
      }
      )
@juliusknorr juliusknorr added bug 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels May 22, 2025
@juliusknorr
Copy link
Member Author

juliusknorr commented May 22, 2025

One step further it actually seems like

if ($mount instanceof MoveableMount) {
$rootEntry['permissions'] = $permissions | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE;
} else {
is setting the update permission for movable mounts to support renames but somewhat conflicts the the file still not being updatable. However on the storage/file info permissions we do not have separate ones.

For WebDAV there are and we have a similar logic in

$storage = $info->getStorage();
if ($info->getInternalPath() === '' && $info->getMountPoint() instanceof IMovableMount) {
$rootEntry = $storage->getCache()->get('');

@juliusknorr
Copy link
Member Author

juliusknorr commented May 22, 2025

Ok so it seems we just loose the difference between move, rename and update permissions that the webdav response has in https://github.com/nextcloud-libraries/nextcloud-files/blob/main/lib/dav/dav.ts#L173C14-L202

@skjnldsv @susnux I think that is something we should handle in @nextcloud/files. Would it make sense to still expose the permission string in the node object? Any other preferences how we could expose this?

Edit: use case and catched when trying to hide a file action for files that user can not edit: https://github.com/nextcloud/files_lock/pull/683/files#diff-a09b7f67f5f992f7ef9d9299f6fac774fb218572c9c2960eb2c30293067a8843R25

@susnux
Copy link
Contributor

susnux commented May 22, 2025

So needs to be updatable only:

  • is file && 'W'
  • is folder && 'NV'

if a file has 'NV' then we can only rename or move it, maybe a new permission here.

@juliusknorr
Copy link
Member Author

I'm not sure what other checks are using the updatable permission as well.

Also for an incoming single file share it could be read only (1) as storage permission but the share itself would still be NV, so not sure how to differentiate that between that and a file with 'NVW'

Currently I see no way around separate options to check if the file is renamable/movable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 32-feedback bug
Projects
None yet
Development

No branches or pull requests

3 participants