Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid determination of document editor in per-user-encryption setups #1396

Merged
merged 3 commits into from
Mar 4, 2021
Merged

Avoid determination of document editor in per-user-encryption setups #1396

merged 3 commits into from
Mar 4, 2021

Conversation

mario-van-zadel
Copy link

@mario-van-zadel mario-van-zadel commented Mar 2, 2021

Summary

When saving a document with enabled per-user encryption keys, an exception Private Key missing for user: please try to log-out and log-in again is thrown.

This is caused by the determination of the user who is editing the document to be able to trace the document's changes (https://github.com/nextcloud/richdocuments/blob/master/lib/Controller/WopiController.php#L407). Setting a specific user causes the usage of the user-specific encryption key instead of using the incognito mode (like it's done in getFile: https://github.com/nextcloud/richdocuments/blob/master/lib/Controller/WopiController.php#L349).

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Documentation (manuals or wiki) has been updated or is not required

@mario-van-zadel
Copy link
Author

@juliushaertl Do you know how to solve the failing tests? I'm using OCA\Encryption\Util to determine if the master-key is enabled, but it's not part of christophwurst/nextcloud. There seems not to be an isMasterKeyEnabled() method as part of Nextcloud's public API.

@mario-van-zadel mario-van-zadel changed the title Avoid determination of document editor for per-user-encryption setups Avoid determination of document editor in per-user-encryption setups Mar 2, 2021
@juliushaertl
Copy link
Member

I'm using OCA\Encryption\Util to determine if the master-key is enabled, but it's not part of christophwurst/nextcloud. There seems not to be an isMasterKeyEnabled() method as part of Nextcloud's public API.

Yes, the server-side encryption is built in a modular way where the default encryption module (the encryption app that is shipped) is taking care of the implementation in regards to user/master key encryption, so that part is only in the app and not in the server encryption API. That is of course not ideal from an integrating apps perspective since we cannot be sure if the app is enabled or not. Due to that the app is disabled in the tests the autoloader is not able to inject the Util instance into the WopiController constructor. Probably the most suitable way would be to just avoid using dependency injection for the master key check and do something like this for the check:

$isMasterKeyEnabled = false;
try {
	$util = \OC::$server->query(\OCA\Encryption\Util::class);
	$isMasterKeyEnabled = $util->isMasterKeyEnabled();
} catch(\OCP\AppFramework\QueryException $e) {
	// No encryption module enabled
}

Mario Klump added 2 commits March 2, 2021 12:16
… are active.

The incognito mode is also used for fetching documents (see method `getFile`).

Signed-off-by: Mario Klump <mail@marioklump.net>
Signed-off-by: Mario Klump <mail@marioklump.net>
@mario-van-zadel mario-van-zadel marked this pull request as ready for review March 2, 2021 12:04
Signed-off-by: Mario Klump <mail@marioklump.net>

Co-authored-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl juliushaertl merged commit 2ca896a into nextcloud:master Mar 4, 2021
@juliushaertl
Copy link
Member

/backport to stable3.7

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Writing documents with enabled per-user encryption keys fails (Private Key missing for user)
2 participants