Skip to content

Fix proper permissions for multiple file access - #8891

Merged
rullzer merged 2 commits into
masterfrom
fix_8890
Mar 22, 2018
Merged

Fix proper permissions for multiple file access#8891
rullzer merged 2 commits into
masterfrom
fix_8890

Conversation

@rullzer

Copy link
Copy Markdown
Member

Fixes#8890

In case you have access to a file via multiple ways, for example:

  1. the file is shared with you with permission read only
  2. the folder containing the file is shared with your read/write

Requesting the getById function on the userFolder would give back two
entries but both with the same permissions. Depending on the node you
picked this is not right.

@icewind1991 please have a look. I realize this is a little less optimized. But it is now correct I think.

@rullzerrullzer added this to the Nextcloud 14 milestone Mar 19, 2018
icewind1991
icewind1991 previously requested changes Mar 20, 2018
Comment threadlib/private/Files/Node/Folder.php Outdated
));
}, $mountsContainingFile);

$nodes = array_filter($nodes, function ($x) {

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.

You can leave out the callback argument to filter out any falseish values

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Ah fair enough. was late yesterday.

@icewind1991

Copy link
Copy Markdown
Member

Looks good otherwise

@rullzer

Copy link
Copy Markdown
MemberAuthor

I'll fix the tests later as well

Fixes#8890
In case you have access to a file via multiple ways, for example:
1. the file is shared with you with permission read only
2. the folder containing the file is shared with your read/write
Requesting the getById function on the userFolder would give back two
entries but both with the same permissions. Depending on the node you
picked this is not right.
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@codecov

codecovBot commented Mar 22, 2018

Copy link
Copy Markdown

Codecov Report

Merging #8891 into master will decrease coverage by 1.33%.
The diff coverage is 83.33%.

@@ Coverage Diff @@## master #8891 +/- ##
============================================
- Coverage 53.3% 51.96% -1.34% - Complexity 23969 25270 +1301 
============================================
Files 1509 1603 +94 Lines 89488 94878 +5390 Branches 1388 1388 ============================================
+ Hits 47703 49305 +1602 - Misses 41785 45573 +3788
Impacted FilesCoverage ΔComplexity Δ
lib/private/Files/Node/Folder.php89.25% <83.33%> (ø)62 <2> (+1)⬆️
.../tests/Unit/Collaboration/CommentersSorterTest.php25.55% <0%> (-66.45%)6% <0%> (ø)
apps/sharebymail/tests/SettingsTest.php52.17% <0%> (-47.83%)3% <0%> (ø)
...ps/comments/tests/Unit/AppInfo/ApplicationTest.php69.56% <0%> (-30.44%)4% <0%> (ø)
lib/private/Files/ObjectStore/S3ObjectTrait.php75% <0%> (-25%)5% <0%> (ø)
lib/public/AppFramework/ApiController.php71.42% <0%> (-22.33%)3% <0%> (ø)
.../dav/lib/Connector/Sabre/ExceptionLoggerPlugin.php78.94% <0%> (-21.06%)6% <0%> (ø)
apps/comments/tests/Unit/JSSettingsHelperTest.php80.76% <0%> (-19.24%)2% <0%> (ø)
lib/public/AppFramework/OCSController.php80.95% <0%> (-19.05%)7% <0%> (ø)
apps/dav/lib/CalDAV/Activity/Provider/Base.php79.36% <0%> (-18.68%)19% <0%> (ø)
... and 138 more

@rullzer

Copy link
Copy Markdown
MemberAuthor

Review time!

@ChristophWurstChristophWurst left a comment

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.

Change makes sense and code looks good 👍 from me

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@rullzer@icewind1991@ChristophWurst