Skip to content

WIP: fix(encryption): store key by fileId for CollectiveMountPoint#59343

Draft
mejo- wants to merge 1 commit into
masterfrom
fix/encryption_key_by_file_id
Draft

WIP: fix(encryption): store key by fileId for CollectiveMountPoint#59343
mejo- wants to merge 1 commit into
masterfrom
fix/encryption_key_by_file_id

Conversation

@mejo-
Copy link
Copy Markdown
Member

@mejo- mejo- commented Mar 31, 2026

Summary

In Collectives, we can have different paths to a file by user (when a user changes their user_folder user setting). This leads to inaccessible files with server-side encryption as the encryption keys are stored by file path. In order to fix this, we should switch to fileId based key storage instead of file path based key storage.

TODO

  • Decide whether we want to always store encryption keys by fileId, or just for ISystemMountPoint or just for CollectiveMountPoint
  • Decide whether we change all the interfaces and functions to pass $fileId through encryption storage wrapper or whether it's acceptable to use $this->rootView->getFileInfo() in Util.php.
  • [ ]

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Jonas <jonas@freesources.org>
@icewind1991
Copy link
Copy Markdown
Member

I agree that storing keys by fileid would be an improvement, as it also removes the need to move the keys on renames etc.

Decide whether we want to always store encryption keys by fileId, or just for ISystemMountPoint or just for CollectiveMountPoint

I would be in favor of doing it for all storages, but that obviously makes migration/backwards compatibility harder

Decide whether we change all the interfaces and functions to pass $fileId through encryption storage wrapper or whether it's acceptable to use $this->rootView->getFileInfo() in Util.php

Ideally the fileid would be passed, (or just pass the full FileInfo instead of paths trough the whole stack)

}

if ($fileId !== null) {
$keyPath = $root . '/files_encryption/keys_by_fileId/' . $fileId . '/';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will run into problems with the number of files in a single directory. Some form of nested directory structure will need to be used to spread things out a bit

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an idea how we could nest the the files?

What do you think about a 3-level split of a 12-digit zero-padded version of the fileId? I.e. fileId 123 would go to 0000/0000/0123/123/, 987654321 would go to 0009/8765/4321/987654321/, and so on.

$padded = str_pad((string)$fileId, 12, '0', STR_PAD_LEFT); 
$path = substr($padded, 0, 4) . '/' . substr($padded, 4, 4) . '/' . substr($padded, 8, 4) . '/' . (string)$fileId;

@github-project-automation github-project-automation Bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Productivity team Apr 27, 2026
@mejo- mejo- moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 📝 Productivity team Apr 27, 2026
@susnux susnux added the bug label May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 📄 To do (~10 entries)

Development

Successfully merging this pull request may close these issues.

4 participants