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

Add Jail and PermissionsMask storage wrappers #12426

Merged
merged 2 commits into from
Nov 27, 2014
Merged

Conversation

icewind1991
Copy link
Contributor

Extracted from #12086

  • \OC\Files\Storage\Wrapper\Jail: limits all access of the storage to a specific folder
  • \OC\Files\Storage\Wrapper\PermissionsMask: applies a permissions mask on top of a storage

Moved to a separate PR to reduce the size of the indivual PR and makes them easier to review/merge and because @PVince81's metadata work could use the cache wrapper stuff introduced here.

cc @PVince81 @DeepDiver1975

/**
* find a folder in the cache which has not been fully scanned
*
* If multiply incomplete folders are in the cache, the one with the highest id will be returned,
Copy link
Contributor

Choose a reason for hiding this comment

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

"multiple" (I've seen this typo before in other classes 😄)

@DeepDiver1975
Copy link
Member

@icewind1991 same failing unit test as in #12420 (comment) - possible fix: 5d4b7e0 - THX

@MorrisJobke
Copy link
Contributor

@icewind1991 Fix is in master. Maybe rebase ;)

@icewind1991 icewind1991 force-pushed the jail-mask-wrappers branch 2 times, most recently from 3fa9389 to bf838cb Compare November 26, 2014 14:32
}

public function isUpdatable($path) {
return $this->checkMask(\OCP\PERMISSION_UPDATE) and parent::isUpdatable($path);
Copy link
Contributor

Choose a reason for hiding this comment

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

deprecated as of #12421 please use \OCP\Constants::PERMISSION_* instead

@icewind1991
Copy link
Contributor Author

Updated the permissions constants

public function setUp() {
$this->sourceStorage = new \OC\Files\Storage\Temporary(array());
$this->instance = $this->getMaskedStorage(Constants::PERMISSION_ALL);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

please call parent setUp() and tearDown() !

@icewind1991
Copy link
Contributor Author

@owncloud-bot retest this please

@ghost
Copy link

ghost commented Nov 27, 2014

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

@icewind1991
Copy link
Contributor Author

@nickvergessen @DeepDiver1975 @MorrisJobke can this be merged?

/**
* Mask the permissions of a storage
*
* Note that the read permissions cant be masked
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain the masking a little bit more ?

@PVince81
Copy link
Contributor

Please add a better explanation for the PermissionsMask, then it will be good to go from my point of view 👍

@nickvergessen
Copy link
Contributor

Will run tests on windows in a bit

@nickvergessen
Copy link
Contributor

No new failing tests on windows

@scrutinizer-notifier
Copy link

A new inspection was created.

@icewind1991
Copy link
Contributor Author

Extended phpdocs

@ghost
Copy link

ghost commented Nov 27, 2014

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

Build result: FAILURE

[...truncated 18 lines...] > git rev-parse origin/pr/12426/merge^{commit} # timeout=10Checking out Revision f8ef5c812777fcd34621cd40aad7bbbb81ad6b99 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f f8ef5c812777fcd34621cd40aad7bbbb81ad6b99 > git rev-list bf383df9f1e066833f6db9761c0c32d7ea853fde # timeout=10First time build. Skipping changelog. > git remote # timeout=10 > git submodule init # timeout=10 > git submodule sync # timeout=10 > git config --get remote.origin.url # timeout=10 > git submodule update --init --recursiveCleaning workspace > git rev-parse --verify HEAD # timeout=10Resetting working tree > git reset --hard # timeout=10 > git clean -fdx # timeout=10 > git submodule foreach --recursive git reset --hard # timeout=10 > git submodule foreach --recursive git clean -fdx # timeout=10Triggering pull-request-analyser-ng-simple » vm-slave-02pull-request-analyser-ng-simple » vm-slave-02 completed with result FAILUREStarted calculate disk usage of buildFinished Calculation of disk usage of build in 0 secondsStarted calculate disk usage of workspaceFinished Calculation of disk usage of workspace in 7 second
💣 Test FAILed. 💣

@icewind1991
Copy link
Contributor Author

Jenkins messing up again, only did doc changes since last success -> merging

icewind1991 added a commit that referenced this pull request Nov 27, 2014
Add Jail and PermissionsMask storage wrappers
@icewind1991 icewind1991 merged commit 05a069c into master Nov 27, 2014
@icewind1991 icewind1991 deleted the jail-mask-wrappers branch November 27, 2014 15:00
@PVince81 PVince81 mentioned this pull request Nov 28, 2014
22 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Aug 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants