X Tutup
Skip to content

Members of excluded groups should not get any information about sharees#33728

Merged
PVince81 merged 1 commit intomasterfrom
fix/api-sharees-security
Dec 4, 2018
Merged

Members of excluded groups should not get any information about sharees#33728
PVince81 merged 1 commit intomasterfrom
fix/api-sharees-security

Conversation

@micbar
Copy link
Contributor

@micbar micbar commented Dec 3, 2018 •
edited
Loading

Description

Original issue https://github.com/owncloud/enterprise/issues/3024

Members of excluded groups should not get any information about sharees. They cannot use the autocomplete field in the UI, but directly using the api works.

Related Issue

Motivation and Context

Members of excluded groups should not get any information about sharees. This is a privacy issue.

How Has This Been Tested?

Local Test:

  • Create User 1:
  • Create Group "restricted":
  • Make User 1 member of group "restricted"
  • Try to call http://localhost/ocs/v1.php/apps/files_sharing/api/v1/sharees?format=json&search=admin&perPage=400&itemType=file
  • Result should return no users

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@micbar micbar force-pushed the fix/api-sharees-security branch from 8d9da1f to ffc9039 Compare December 3, 2018 14:44
@codecov
Copy link

codecov bot commented Dec 3, 2018

Codecov Report

Merging #33728 into master will decrease coverage by 15.87%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #33728       +/-   ##
=============================================
- Coverage     64.35%   48.47%   -15.88%     
=============================================
  Files          1195      109     -1086     
  Lines         69240    10498    -58742     
  Branches       1276     1276               
=============================================
- Hits          44560     5089    -39471     
+ Misses        24308     5037    -19271     
  Partials        372      372
Flag Coverage Δ Complexity Δ
#javascript 52.98% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 38.67% <ø> (-27%) 0 <ø> (-18307)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 59.45% <0%> (-21.64%) 0% <0%> (ø)
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1077 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1947c92...ffc9039. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 3, 2018 •
edited
Loading

Codecov Report

Merging #33728 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #33728      +/-   ##
============================================
+ Coverage     64.35%   64.35%   +<.01%     
- Complexity    18307    18308       +1     
============================================
  Files          1195     1195              
  Lines         69240    69245       +5     
  Branches       1276     1276              
============================================
+ Hits          44560    44565       +5     
  Misses        24308    24308              
  Partials        372      372
Flag Coverage Δ Complexity Δ
#javascript 52.98% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.67% <100%> (ø) 18308 <0> (+1) ⬆️
Impacted Files Coverage Δ Complexity Δ
...files_sharing/lib/Controller/ShareesController.php 89.32% <100%> (+0.17%) 105 <0> (+1) ⬆️
...eratedfilesharing/lib/Controller/OcmController.php 66.26% <0%> (+0.2%) 30% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74b72e1...bff85a1. Read the comment docs.

@phil-davis phil-davis mentioned this pull request Dec 3, 2018
Merged
@micbar micbar force-pushed the fix/api-sharees-security branch from ffc9039 to 6613855 Compare December 3, 2018 16:36
PVince81
PVince81 reviewed Dec 3, 2018
View reviewed changes
@PVince81 PVince81 added this to the development milestone Dec 3, 2018
@PVince81
Copy link
Contributor

PVince81 commented Dec 3, 2018

@micbar when you make changes to a PR please always post a comment to explain what and why you changed and ping for review

@micbar
Copy link
Contributor Author

micbar commented Dec 3, 2018 •
edited
Loading

@micbar when you make changes to a PR please always post a comment to explain what and why you changed and ping for review

Sorry, was in the middle of pushing. Did it 5 min. ago.

@micbar
Copy link
Contributor Author

micbar commented Dec 3, 2018

Should I squash the commits into a single one?

@micbar micbar force-pushed the fix/api-sharees-security branch from a4f4156 to 619998b Compare December 3, 2018 20:27
PVince81
PVince81 approved these changes Dec 3, 2018
View reviewed changes
Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81
Copy link
Contributor

PVince81 commented Dec 3, 2018

@micbar please backport

@micbar micbar mentioned this pull request Dec 3, 2018
Merged
9 tasks
@PVince81
Copy link
Contributor

PVince81 commented Dec 3, 2018

restarted failed build

DeepDiver1975
DeepDiver1975 reviewed Dec 3, 2018
View reviewed changes
@micbar micbar force-pushed the fix/api-sharees-security branch from 619998b to bff85a1 Compare December 4, 2018 07:47
@micbar
Copy link
Contributor Author

micbar commented Dec 4, 2018

CI green, but status not updated.

@PVince81 PVince81 merged commit 591d90d into master Dec 4, 2018
@delete-merged-branch delete-merged-branch bot deleted the fix/api-sharees-security branch December 4, 2018 09:41
@phil-davis
Copy link
Contributor

phil-davis commented Dec 13, 2018

Backport stable10 #33736 has been merged

@lock lock bot locked as resolved and limited conversation to collaborators Dec 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 left review comments

+1 more reviewer

@PVince81 PVince81 PVince81 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@micbar micbar

Labels

4 - To release

Projects

None yet

Milestone

development

Development

Successfully merging this pull request may close these issues.

4 participants

@micbar @PVince81 @phil-davis @DeepDiver1975
X Tutup