Skip to content

Commit e92817d

Browse files
committed
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: nextcloud/forms#2067 However, if I am right about the security model here, this is a safe workaround, and may actually be correct. Signed-off-by: Matthew Toseland <[email protected]>
1 parent bec14dc commit e92817d

File tree

1 file changed

+4
-3
lines changed

1 file changed

+4
-3
lines changed

Diff for: apps/files_versions/lib/Listener/FileEventsListener.php

+4-3
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,10 @@ private function getPathForNode(Node $node): ?string {
361361

362362
$owner = $node->getOwner()?->getUid();
363363

364-
// If no owner, extract it from the path.
365-
// e.g. /user/files/foobar.txt
366-
if (!$owner) {
364+
// If no owner, extract it from the path, e.g. /user/files/foobar.txt
365+
// Also try this if they are the same, because it might be a group folder that the user does not have access to
366+
// E.g. where filling in a form with a spreadsheet attached
367+
if (!$owner || $owner == $user) {
367368
$parts = explode('/', $node->getPath(), 4);
368369
if (count($parts) === 4) {
369370
$owner = $parts[1];

0 commit comments

Comments
 (0)