fix permissions of mountpoints - take 2 #4685
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 70a0e9c. Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
A moveable mount point (What a SharedStorage is) always has DELETE + UPDATE permissions. Since you can either delete (unshare) or update (rename) it. Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
|
@rullzer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @icewind1991, @schiessle and @LukasReschke to be potential reviewers. |
|
Mmmm the integration tests are not happy... lets see why |
|
Ah go it... |
rullzer
commented
May 4, 2017
| $data['permissions'] |= \OCP\Constants::PERMISSION_DELETE; | ||
| if ($internalPath === '') { | ||
| if ($mount instanceof MoveableMount) { | ||
| $data['permissions'] = $data['permissions'] | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE; |
There was a problem hiding this comment.
@icewind1991 because you added the PERMISSION_UPDATE here it fails for shared files.
- user0 has
foo.txt - user0 shares
foo.txtto user1 with READ+SHARE permissions - user1 shares
foo.txtto user2 with READ+SHARE+UPDATE permissions
This now works because well the mountpoint gets update permissions.
MorrisJobke
reviewed
May 4, 2017
| @@ -210,7 +210,7 @@ public function testGetPermissions() { | |||
| // the read permissions (1) | |||
| // the delete permission (8), to enable unshare | |||
| $rootInfo = \OC\Files\Filesystem::getFileInfo($this->folder); | |||
| $this->assertSame(9, $rootInfo->getPermissions()); | |||
| $this->assertSame(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_UPDATE, $rootInfo->getPermissions()); | |||
There was a problem hiding this comment.
Then also update the comment above
Signed-off-by: Robin Appelman <robin@icewind.nl>
Codecov Report
@@ Coverage Diff @@
## master #4685 +/- ##
============================================
+ Coverage 54.28% 54.28% +<.01%
Complexity 22098 22098
============================================
Files 1361 1361
Lines 84710 84712 +2
Branches 1324 1324
============================================
+ Hits 45982 45984 +2
Misses 38728 38728
|
icewind1991
approved these changes
May 15, 2017
|
Damn ... I have overseen this failure: @rullzer could you have a look? |
|
Looks like an actual bug :/ |
|
Yeah please revert. As we need to fix that properly our users can increase shared file permissions |
Done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#4623 + fix for the test.