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

Can not change encryption directory #6805

Merged
merged 4 commits into from
Mar 2, 2018

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Oct 10, 2017

Fix #6769

@schiessle there is an issue here

  1. You require a path relative to the root storage
  2. You use the view
  3. The view does not allow /../ in paths

Therefor you can only change the root inside your data directory, but not move it out of it.
If that is intended, we should have this commit here and display a better error message

@@ -146,7 +146,7 @@ protected function prepareNewRoot($newRoot) {
'ownCloud will detect this folder as key storage root only if this file exists'
);

if ($result === false) {
if ($result !== true) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently the View returns null, because

if (Filesystem::isValidPath($path)

returns false and therefor nothing is executed.

@MorrisJobke
Copy link
Member

@schiessle Any news for this? Would be good to get it in.

@MorrisJobke MorrisJobke mentioned this pull request Dec 8, 2017
28 tasks
@MorrisJobke
Copy link
Member

Let's move this to 14.

nickvergessen and others added 3 commits March 1, 2018 17:30
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle schiessle force-pushed the can-not-change-encryption-directory branch from 9c4baa0 to 9259b8a Compare March 1, 2018 16:50
@codecov
Copy link

codecov bot commented Mar 1, 2018

Codecov Report

Merging #6805 into master will decrease coverage by 16.83%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##             master    #6805       +/-   ##
=============================================
- Coverage     51.93%   35.09%   -16.84%     
  Complexity    25418    25418               
=============================================
  Files          1609     1609               
  Lines         95329    95329               
  Branches       1378     1378               
=============================================
- Hits          49506    33454    -16052     
- Misses        45823    61875    +16052
Impacted Files Coverage Δ Complexity Δ
core/Command/Encryption/ChangeKeyStorageRoot.php 0% <0%> (-88.55%) 31 <0> (ø)
...s/dav/lib/CalDAV/BirthdayCalendar/EnablePlugin.php 0% <0%> (-100%) 7% <0%> (ø)
lib/private/App/AppStore/Version/Version.php 0% <0%> (-100%) 3% <0%> (ø)
...pps/files/lib/Activity/Settings/FavoriteAction.php 0% <0%> (-100%) 8% <0%> (ø)
apps/files/lib/Activity/Settings/FileChanged.php 0% <0%> (-100%) 8% <0%> (ø)
apps/federation/lib/Hooks.php 0% <0%> (-100%) 4% <0%> (ø)
apps/dav/lib/CalDAV/Activity/Setting/Calendar.php 0% <0%> (-100%) 8% <0%> (ø)
...pFramework/Db/MultipleObjectsReturnedException.php 0% <0%> (-100%) 1% <0%> (ø)
apps/files/lib/Activity/Settings/FileCreated.php 0% <0%> (-100%) 8% <0%> (ø)
core/Command/TwoFactorAuth/Enable.php 0% <0%> (-100%) 4% <0%> (ø)
... and 528 more

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.

tested and works with my changes

@schiessle
Copy link
Member

second reviewer needed

@nickvergessen
Copy link
Member Author

👍 for not my code

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle schiessle merged commit 1953a11 into master Mar 2, 2018
@schiessle schiessle deleted the can-not-change-encryption-directory branch March 2, 2018 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug feature: encryption (server-side)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants