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

[stable10] Hide lock token in public webdav responses #34270

Merged
merged 2 commits into from
Jan 28, 2019
Merged

[stable10] Hide lock token in public webdav responses #34270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
301235c
Hide lock token in public webdav responses
Jan 28, 2019
0d286aa
adjust acceptance tests
individual-it Jan 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/dav/appinfo/v1/publicwebdav.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function ($event) use ($share) {
);

return new \OC\Files\View($ownerView->getAbsolutePath($path));
});
}, true);

$server->addPlugin(new \OCA\DAV\Connector\Sabre\AutorenamePlugin());
$server->addPlugin($linkCheckPlugin);
Expand Down
6 changes: 4 additions & 2 deletions apps/dav/lib/Connector/Sabre/ServerFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,14 @@ public function __construct(
* @param string $requestUri
* @param BackendInterface $authBackend
* @param callable $viewCallBack callback that should return the view for the dav endpoint
* @param bool $isPublicAccess whether DAV is accessed through a public link
* @return Server
*/
public function createServer($baseUri,
$requestUri,
BackendInterface $authBackend,
callable $viewCallBack) {
callable $viewCallBack,
$isPublicAccess = false) {
// Fire up server
$objectTree = new \OCA\DAV\Connector\Sabre\ObjectTree();
$server = new \OCA\DAV\Connector\Sabre\Server($objectTree);
Expand All @@ -118,7 +120,7 @@ public function createServer($baseUri,
$server->addPlugin(new \OCA\DAV\Connector\Sabre\DummyGetResponsePlugin());
$server->addPlugin(new \OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin('webdav', $this->logger));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\LockPlugin());
$server->addPlugin(new \Sabre\DAV\Locks\Plugin(new FileLocksBackend($server->tree, true, $this->timeFactory)));
$server->addPlugin(new \Sabre\DAV\Locks\Plugin(new FileLocksBackend($server->tree, true, $this->timeFactory, $isPublicAccess)));

if (BrowserErrorPagePlugin::isBrowserRequest($this->request)) {
$server->addPlugin(new BrowserErrorPagePlugin());
Expand Down
10 changes: 8 additions & 2 deletions apps/dav/lib/Files/FileLocksBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,14 @@ class FileLocksBackend implements BackendInterface {
private $useV1;
/** @var ITimeFactory */
private $timeFactory;
/** @var bool */
private $hideLockTokenInList;

public function __construct($tree, $useV1, $timeFactory) {
public function __construct($tree, $useV1, $timeFactory, $hideLockTokenInList = false) {
$this->tree = $tree;
$this->useV1 = $useV1;
$this->timeFactory = $timeFactory;
$this->hideLockTokenInList = $hideLockTokenInList;
}

/**
Expand Down Expand Up @@ -130,7 +133,10 @@ public function getLocks($uri, $returnChildLocks) {
$lockInfo->uri = "files/$uid/$fileName";
}
}
$lockInfo->token = $lock->getToken();

if (!$this->hideLockTokenInList) {
$lockInfo->token = $lock->getToken();
}
$lockInfo->created = $lock->getCreatedAt();
$lockInfo->depth = $lock->getDepth();
$lockInfo->owner = $lock->getOwner();
Expand Down
4 changes: 2 additions & 2 deletions apps/dav/tests/unit/Files/FileLocksBackendTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,14 @@ public function testGetLocksPublic($storageLockPath, $storageGetLocksPath, $lock
$timeFactory = $this->createMock(ITimeFactory::class);
$timeFactory->method('getTime')->willReturn(self::CURRENT_TIME);

$this->plugin = new FileLocksBackend($this->tree, true, $timeFactory);
$this->plugin = new FileLocksBackend($this->tree, true, $timeFactory, true);

// "/public/share" is a public share
// "/public" has the locks
// we query "public.php/webdav/sub" inside that share
$locks = $this->plugin->getLocks($lockPluginGetLockPath, true);
$lockInfo = new LockInfo();
$lockInfo->token = '123-456-7890';
$lockInfo->token = null; // hidden in public endpoint
$lockInfo->scope = LockInfo::EXCLUSIVE;
$lockInfo->uri = $responseLockRoot;
$lockInfo->owner = 'Alice Wonder';
Expand Down
18 changes: 17 additions & 1 deletion tests/acceptance/features/apiWebdavLocks/folder.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,20 @@ Feature: lock folders
| old | shared |
| old | exclusive |
| new | shared |
| new | exclusive |
| new | exclusive |

Scenario Outline: lockdiscovery of a locked folder
Given using <dav-path> DAV path
And user "user0" has created a public link share of folder "PARENT" with change permission
And user "user0" has locked folder "PARENT" setting following properties
| lockscope | <lock-scope> |
When user "user0" gets the following properties of folder "PARENT" using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "<lock-root>"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:[a-z0-9-]+$/"
Examples:
| dav-path | lock-scope | lock-root |
| old | shared | /%base_path%\/remote.php\/webdav\/PARENT$/ |
| old | exclusive | /%base_path%\/remote.php\/webdav\/PARENT$/ |
| new | shared | /%base_path%\/remote.php\/dav\/files\/user0\/PARENT$/ |
| new | exclusive | /%base_path%\/remote.php\/dav\/files\/user0\/PARENT$/ |
65 changes: 21 additions & 44 deletions tests/acceptance/features/apiWebdavLocks/publicLink.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:lockroot/d:href" in the response should be ""
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -98,9 +96,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/CHILD" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:lockroot/d:href" in the response should be ""
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -113,9 +109,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/CHILD" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/CHILD$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:lockroot/d:href" in the response should be ""
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -128,9 +122,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/CHILD/child.txt" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/CHILD$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:lockroot/d:href" in the response should be ""
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -143,9 +135,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/CHILD/child.txt" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:lockroot/d:href" in the response should be ""
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -158,9 +148,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/CHILD/child.txt" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/CHILD\/child.txt$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:lockroot/d:href" in the response should be ""
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -173,9 +161,8 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/child.txt" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:lockroot/d:href" in the response should be ""
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"

Examples:
| lock-scope |
| shared |
Expand All @@ -188,7 +175,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -201,7 +188,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/CHILD" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -214,7 +201,7 @@ Feature: persistent-locking in case of a public link
When the public gets the following properties of entry "/CHILD" in the last created public link using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/CHILD$/"
And the value of the item "//d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
And the value of the item "//d:locktoken/d:href" in the response should be "opaquelocktoken:"
Examples:
| lock-scope |
| shared |
Expand All @@ -230,10 +217,8 @@ Feature: persistent-locking in case of a public link
| d:lockdiscovery |
Then the value of the item "//d:activelock[1]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:activelock[2]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be ""
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"

Scenario: lockdiscovery subfolder of public link when root is locked by user and subfolder is locked by public
Given user "user0" has created a public link share of folder "PARENT" with change permission
Expand All @@ -245,10 +230,8 @@ Feature: persistent-locking in case of a public link
| d:lockdiscovery |
Then the value of the item "//d:activelock[1]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:activelock[2]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/CHILD$/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be ""
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"

Scenario: lockdiscovery root of public link when user has locked folder above public link and public has locked root of public link
Given user "user0" has created a public link share of folder "PARENT/CHILD" with change permission
Expand All @@ -260,10 +243,8 @@ Feature: persistent-locking in case of a public link
| d:lockdiscovery |
Then the value of the item "//d:activelock[1]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:activelock[2]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be ""
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"

Scenario: lockdiscovery subfolder of public link when user has locked folder above public link and public has locked subfolder of public link
Given user "user0" has created a public link share of folder "PARENT/CHILD" with change permission
Expand All @@ -276,10 +257,8 @@ Feature: persistent-locking in case of a public link
| d:lockdiscovery |
Then the value of the item "//d:activelock[1]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:activelock[2]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/GRANDCHILD$/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be ""
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"

Scenario: lockdiscovery file in public link when user has locked folder above public link and public has locked file inside of public link
Given user "user0" has created a public link share of folder "PARENT/CHILD" with change permission
Expand All @@ -291,7 +270,5 @@ Feature: persistent-locking in case of a public link
| d:lockdiscovery |
Then the value of the item "//d:activelock[1]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/$/"
And the value of the item "//d:activelock[2]/d:lockroot/d:href" in the response should match "/%base_path%\/public.php\/webdav\/child.txt$/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
#see https://github.com/owncloud/core/pull/34229#issuecomment-457186641
#And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be ""
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should match "/^opaquelocktoken:/"
And the value of the item "//d:activelock[1]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"