Skip to content

Commit

Permalink
Correctly look up filename when logged in but inaccessible
Browse files Browse the repository at this point in the history
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 <matthew@toselandcs.co.uk>
  • Loading branch information
toad committed Sep 23, 2024
1 parent bec14dc commit e92817d
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions apps/files_versions/lib/Listener/FileEventsListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down

0 comments on commit e92817d

Please sign in to comment.