From 0d7674acd5362518594a7f291e4fa454065b923e Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Mon, 23 Sep 2024 12:51:53 +0100 Subject: [PATCH] Correctly look up filename when logged in but inaccessible If the user is not logged in, and we can't find the file owner, we guess the file owner from the path. Most likely the username in the path has some access to the file, so we can find the filename. However, in the case where we are logged in, and the file is a group share, both the username and the owner will be the same UID. Which is fine as long as we have access to the file. But in the case where we are actually writing a spreadsheet update for a form submission, the logged in user may not have access to the file. However, it is still a legitimate write by the forms app, so it is safe to use the owner in the path, just as if we were logged out. I'm not 100% sure about the security implications here and request review. However it provides a simple workaround for our use case: - Form with an attached spreadsheet - User is logged in - User has access to the form but not the spreadsheet - The spreadsheet is in a group folder - The versioning app is enabled The alternative is to fix the underlying problem in the forms app, which would be a much bigger diff. See the forms bug here: https://github.com/nextcloud/forms/issues/2067 However, if I am right about the security model here, this is a safe workaround, and may actually be correct. --- apps/files_versions/lib/Listener/FileEventsListener.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/files_versions/lib/Listener/FileEventsListener.php b/apps/files_versions/lib/Listener/FileEventsListener.php index c078f4bc34755..aa70aa80ebd36 100644 --- a/apps/files_versions/lib/Listener/FileEventsListener.php +++ b/apps/files_versions/lib/Listener/FileEventsListener.php @@ -361,9 +361,10 @@ private function getPathForNode(Node $node): ?string { $owner = $node->getOwner()?->getUid(); - // If no owner, extract it from the path. - // e.g. /user/files/foobar.txt - if (!$owner) { + // If no owner, extract it from the path, e.g. /user/files/foobar.txt + // Also try this if they are the same, because it might be a group folder that the user does not have access to + // E.g. where filling in a form with a spreadsheet attached + if (!$owner || $owner == $user) { $parts = explode('/', $node->getPath(), 4); if (count($parts) === 4) { $owner = $parts[1];