Fix: files_versions store

[LinneyS]

In version 7.5.2 of Nextcloud ONLYOFFICE integration app we have changed file saving (ONLYOFFICE/onlyoffice-nextcloud@1468eff):
we call \OC_Util::setupFS with the owner of the file
https://github.com/ONLYOFFICE/onlyoffice-nextcloud/blob/f77c15953d9956a74f45cd0a5acef215d495a2ab/controller/callbackcontroller.php#L498

and call \OC_User::setUserId with the author of the changes
https://github.com/ONLYOFFICE/onlyoffice-nextcloud/blob/f77c15953d9956a74f45cd0a5acef215d495a2ab/controller/callbackcontroller.php#L468

In general, the save is successful. But an error occurs when editing a shared file from a group folder or from external storage. And in the logs there is an error from the File Versions application
ONLYOFFICE/onlyoffice-nextcloud#660

This fix makes the save work.

Hi @juliushaertl @PVince81
can you help?

Signed-off-by: Sergey Linnik sergey.linnik@onlyoffice.com

Thanks

[juliusknorr]

I think this is just hiding the symptoms and would stop the version storage in that case. The issue seems to be more that the versions backend is using the filesystem owner to construct the files view and that is a user that might not have access to the file.

Minimal reproduction case similar to the onlyoffice codebase:

• have a groupfolder that only admin can access
• Create a file in that groupfolder (Readme.md) and share that file with user2
• Call the following script without a user session

<?php
	require_once __DIR__ . '/../lib/versioncheck.php';
	require_once __DIR__ . '/../lib/base.php';
	OC_Util::setupFS('admin');
	OC_User::setUserId('user2');

	$userFolder = \OC::$server->getRootFolder()->getUserFolder('admin');
	$file = $userFolder->get('/groupfolder/Readme.md');
	$file->putContent('adf' . time());

Not sure where to hook in there properly to solve this, as the different user ids seem rather something our codebase is not really ready for.

@LinneyS Can you elaborate a bit more on the use case using a different user for getting the file (setupFs) and overwriting the current user?

[LinneyS]

Hi @juliushaertl
In version 7.4.0 we tried to add permission extension. For example, when granting read access, we add the ability to expand access to "comment only". In this case, when using ONLYOFFICE, the file is opened in a special mode where you cannot edit the entire file, but you can add a comment to the text. For Nextcloud, it will still be like saving a file from a user with the "read" access right.

[juliusknorr]

Suggested change

	$eventDispatcher = \OC::$server->get(IEventDispatcher::class);
	$fileInfo = $files_view->getFileInfo($filename);
	if ($fileInfo === false) {
	return false;
	}
	$fileInfo = $files_view->getFileInfo($filename);
	if ($fileInfo === false) {
	$uid = \OC\Files\Filesystem::getView()->getMount('')->getStorage()->getOwner('');
	$files_view = new View('/'.$uid .'/files');
	$fileInfo = $files_view->getFileInfo($filename);
	}
	if ($fileInfo === false) {
	logger('files_versions')->error('Not storing version as the file could not be found for the owner or user: ' . $filename);
	return false;
	}

[juliusknorr]

I think we can make this pick up the previous user from the setupFs call with the change mentioned in the comment, but am still unsure if that is a proper way or if the usage of setupFs and a different current user should actually be avoided. Would be nice to also get some insight from @icewind1991 here.

@LinneyS Can you maybe point to where author of the changes is actually exposed? For the version itself we don't store the user at all, so I'd be curious why exactly you set the different user, where the information is used further on.

[icewind1991]

I think #33804 is the best approach atm.

In the long term this should be moved over to the newer fs hooks that provide the Node instead of just the path.

[PVince81]

#33804 is merged

should we close this PR as obsolete then ?