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

Shares should have a least read permission #14570

Merged
merged 1 commit into from
Mar 2, 2015
Merged

Shares should have a least read permission #14570

merged 1 commit into from
Mar 2, 2015

Conversation

rullzer
Copy link
Contributor

@rullzer rullzer commented Feb 27, 2015

  • Throw 400 when a share is created or updated without read permissions
  • Added unit tests

As requested in #14548

@PVince81
Copy link
Contributor

Thanks a lot!

@icewind1991 @DeepDiver1975 @schiesbn

FYI @jnfrmarks @dragotin

@icewind1991
Copy link
Contributor

Looks good 👍

MorrisJobke
MorrisJobke reviewed Mar 1, 2015
View reviewed changes
apps/files_sharing/api/local.php
@@ -279,6 +279,10 @@ public static function createShare($params) {
return new \OC_OCS_Result(null, 400, "unknown share type");
}

if (($permissions & \OCP\Constants::PERMISSION_READ) == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

=== instead? (same below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep should have been ===.
Fixed

@MorrisJobke
Copy link
Contributor

In MySQL env:

00:16:16.081 There was 1 error:
00:16:16.081 
00:16:16.081 1) Test_Files_Sharing_Api::testUpdateShareInvalidPermissions
00:16:16.082 Undefined offset: 1
00:16:16.082 
00:16:16.082 /var/jenkins/workspace/pull-request-analyser-ng-simple/label/SLAVE/apps/files_sharing/tests/api.php:928

@MorrisJobke MorrisJobke added this to the 8.1-current milestone Mar 1, 2015
@rullzer
Shares should have a least read permission
4436a9c 
* Throw 400 when a share is created or updated without read permissions
* Added unit tests
@scrutinizer-notifier
Copy link

The inspection completed: 2 new issues, 2 updated code elements

@ghost
Copy link

ghost commented Mar 1, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/10101/
Test PASSed.

@rullzer
Copy link
Contributor Author

rullzer commented Mar 1, 2015

Fixed issues and unit tests. Seem like or friends are happier now :)

@MorrisJobke
Copy link
Contributor

👍

MorrisJobke added a commit that referenced this pull request Mar 2, 2015
@MorrisJobke
Merge pull request #14570 from rullzer/ocs_sane_permissions
7194952 
Shares should have a least read permission
@MorrisJobke MorrisJobke merged commit 7194952 into owncloud:master Mar 2, 2015
@MorrisJobke MorrisJobke mentioned this pull request Mar 2, 2015
Closed
@PVince81 PVince81 mentioned this pull request Mar 3, 2015
Closed
@rullzer rullzer deleted the ocs_sane_permissions branch May 22, 2015 11:51
@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
3 - To Review feature:sharing
Projects
None yet
Milestone
8.1
Development

Successfully merging this pull request may close these issues.
5 participants
@rullzer @PVince81 @icewind1991 @MorrisJobke @scrutinizer-notifier