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.

Sign up for GitHub

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

Jump to bottom

Setupfs before access a users keys #4005

Closed
wants to merge 1 commit into from
Closed

Setupfs before access a users keys #4005

wants to merge 1 commit into from

Conversation

MorrisJobke
Copy link
Member

@MorrisJobke
Setupfs before access a users keys (#26917)
ec2e26b 
* The file system of a user has to be properly setup before accessing the keys

* Fix unit test execution

* Init mount points for user in more places in Keys\Storage

* Fix encryption key storage tests to properly create required users

* Added test that check if user home is mounted after resolving key

Signed-off-by: Morris Jobke <[email protected]>
@MorrisJobke MorrisJobke added this to the Nextcloud 12.0 milestone Mar 23, 2017
@mention-bot
Copy link

@MorrisJobke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @schiessle, @nickvergessen and @icewind1991 to be potential reviewers.

@MorrisJobke MorrisJobke changed the title Setupfs before access a users keys (#26917) Setupfs before access a users keys Mar 23, 2017
@MorrisJobke
Copy link
Member Author

@icewind1991 @schiessle Please check this one out

Ping

schiessle
schiessle approved these changes May 3, 2017
View reviewed changes
Copy link
Member

@schiessle schiessle left a comment

Choose a reason for hiding this comment

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

I could verify the issue and that this PR fixes it. Code looks good to me

@schiessle
Copy link
Member

I would recommend to backport it at least to Nextcloud 11 and 12.

@MorrisJobke
Copy link
Member Author

Tested and works 👍

@MorrisJobke
Copy link
Member Author

MorrisJobke commented May 4, 2017 •
edited
Loading

@schiessle Could you have a look at the failing tests:

There were 2 errors:
1) Test\Encryption\Keys\StorageTest::testBackupUserKeys with data set #0 (true)
TypeError: Argument 3 passed to OC\Encryption\Keys\Storage::__construct() must implement interface OCP\IUserSession, none given
/drone/src/github.com/nextcloud/server/lib/private/Encryption/Keys/Storage.php:70
/drone/src/github.com/nextcloud/server/tests/lib/Encryption/Keys/StorageTest.php:580
2) Test\Encryption\Keys\StorageTest::testBackupUserKeys with data set #1 (false)
TypeError: Argument 3 passed to OC\Encryption\Keys\Storage::__construct() must implement interface OCP\IUserSession, none given
/drone/src/github.com/nextcloud/server/lib/private/Encryption/Keys/Storage.php:70
/drone/src/github.com/nextcloud/server/tests/lib/Encryption/Keys/StorageTest.php:580

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 4, 2017
@nickvergessen
Copy link
Member

will this kill performance on creating files, etc? Because we set up the filesystem for all recipients to get their keys?

@MorrisJobke
Copy link
Member Author

Out of scope for 12 I think

@MorrisJobke MorrisJobke removed this from the Nextcloud 12.0 milestone May 16, 2017
@rullzer rullzer added this to the Nextcloud 13 milestone May 23, 2017
@MorrisJobke
Copy link
Member Author

@schiessle Could you have a look at the failing tests:

@schiessle 🏓

@MorrisJobke MorrisJobke closed this Aug 8, 2017
@MorrisJobke MorrisJobke deleted the backport-26917 branch August 8, 2017 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schiessle schiessle schiessle approved these changes

@rullzer rullzer Awaiting requested review from rullzer

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@icewind1991 icewind1991 Awaiting requested review from icewind1991

Assignees

@schiessle schiessle

Labels
2. developing Work in progress feature: encryption (server-side) feature: filesystem
Projects
None yet
Milestone
Nextcloud 13
Development

Successfully merging this pull request may close these issues.
5 participants
@MorrisJobke @mention-bot @schiessle @nickvergessen @rullzer