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

Prevent LOCK/UNLOCK methods from public endpoint #34351

Merged
merged 2 commits into from
Feb 1, 2019
Merged

Prevent LOCK/UNLOCK methods from public endpoint #34351

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
a5b9de6
Prevent LOCK/UNLOCK methods from public endpoint
Jan 31, 2019
7a66731
Remove most public link locking tests
Jan 31, 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
15 changes: 11 additions & 4 deletions apps/dav/lib/Files/FileLocksBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use Sabre\DAV\Tree;
use OCP\AppFramework\Utility\ITimeFactory;
use OCA\DAV\Connector\Sabre\ObjectTree;
use OCA\DAV\Connector\Sabre\Exception\Forbidden;

class FileLocksBackend implements BackendInterface {

Expand All @@ -40,13 +41,13 @@ class FileLocksBackend implements BackendInterface {
/** @var ITimeFactory */
private $timeFactory;
/** @var bool */
private $hideLockTokenInList;
private $isPublicEndpoint;

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

/**
Expand Down Expand Up @@ -134,7 +135,7 @@ public function getLocks($uri, $returnChildLocks) {
}
}

if (!$this->hideLockTokenInList) {
if (!$this->isPublicEndpoint) {
$lockInfo->token = $lock->getToken();
$lockInfo->owner = $lock->getOwner();
}
Expand All @@ -160,6 +161,9 @@ public function getLocks($uri, $returnChildLocks) {
* @return bool
*/
public function lock($uri, Locks\LockInfo $lockInfo) {
if ($this->isPublicEndpoint) {
throw new Forbidden('Forbidden to lock from public endpoint');
}
try {
$node = $this->tree->getNodeForPath($uri);
} catch (NotFound $e) {
Expand Down Expand Up @@ -197,6 +201,9 @@ public function lock($uri, Locks\LockInfo $lockInfo) {
* @return bool
*/
public function unlock($uri, Locks\LockInfo $lockInfo) {
if ($this->isPublicEndpoint) {
throw new Forbidden('Forbidden to unlock from public endpoint');
}
try {
$node = $this->tree->getNodeForPath($uri);
} catch (NotFound $e) {
Expand Down
44 changes: 44 additions & 0 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 @@ -314,6 +314,50 @@ public function testLock() {
$this->assertTrue($this->plugin->lock('file-to-be-locked.txt', $lockInfo));
}

/**
* @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden
*/
public function testLockPublic() {
$lockInfo = new LockInfo();
$lockInfo->token = '123-456-7890';
$lockInfo->scope = LockInfo::SHARED;
$lockInfo->owner = 'Alice Wonder';
$lockInfo->timeout = 800;
$lockInfo->created = self::CREATION_TIME;

$this->storageOfFileToBeLocked
->expects($this->never())
->method('lockNodePersistent');

$timeFactory = $this->createMock(ITimeFactory::class);
$timeFactory->method('getTime')->willReturn(self::CURRENT_TIME);

$this->plugin = new FileLocksBackend($this->tree, true, $timeFactory, true);
$this->plugin->lock('file-to-be-locked.txt', $lockInfo);
}

/**
* @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden
*/
public function testUnlockPublic() {
$lockInfo = new LockInfo();
$lockInfo->token = '123-456-7890';
$lockInfo->scope = LockInfo::SHARED;
$lockInfo->owner = 'Alice Wonder';
$lockInfo->timeout = 800;
$lockInfo->created = self::CREATION_TIME;

$this->storageOfFileToBeLocked
->expects($this->never())
->method('lockNodePersistent');

$timeFactory = $this->createMock(ITimeFactory::class);
$timeFactory->method('getTime')->willReturn(self::CURRENT_TIME);

$this->plugin = new FileLocksBackend($this->tree, true, $timeFactory, true);
$this->plugin->unlock('file-to-be-locked.txt', $lockInfo);
}

public function testUnlock() {
$lockInfo = new LockInfo();
$lockInfo->token = '123-456-7890';
Expand Down
10 changes: 4 additions & 6 deletions tests/acceptance/features/apiWebdavLocks/publicLink.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,9 @@ Feature: persistent-locking in case of a public link
| new | shared |
| new | exclusive |

Scenario: public tries to lock a folder inside an exclusively locked folder
Scenario: Public locking is forbidden
Given 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 | exclusive |
When the public locks "/CHILD" in the last public shared folder using the WebDAV API setting following properties
| lockscope | shared |
Then the HTTP status code should be "423"
And the value of the item "//d:no-conflicting-lock/d:href" in the response should be ""
| lockscope | exclusive |
Then the HTTP status code should be "403"
And the value of the item "//s:message" in the response should be "Forbidden to lock from public endpoint"
117 changes: 0 additions & 117 deletions tests/acceptance/features/apiWebdavLocks/publicLinkLockdiscovery.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,120 +102,3 @@ Feature: LOCKDISCOVERY for public links
| shared |
| exclusive |

Scenario Outline: lockdiscovery root of public link when root is locked by public
Given user "user0" has created a public link share of folder "PARENT" with change permission
And the public has locked the last public shared folder setting following properties
| lockscope | <lock-scope> |
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 be "opaquelocktoken:"
And the value of the item "//d:timeout" in the response should match "/Second-\d+/"
Examples:
| lock-scope |
| shared |
| exclusive |

Scenario Outline: lockdiscovery subfolder of public link when root is locked by public
Given user "user0" has created a public link share of folder "PARENT" with change permission
And the public has locked the last public shared folder setting following properties
| lockscope | <lock-scope> |
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 be "opaquelocktoken:"
And the value of the item "//d:timeout" in the response should match "/Second-\d+/"
Examples:
| lock-scope |
| shared |
| exclusive |

Scenario Outline: lockdiscovery subfolder of public link when subfolder is locked by public
Given user "user0" has created a public link share of folder "PARENT" with change permission
And the public has locked "CHILD" in the last public shared folder setting following properties
| lockscope | <lock-scope> |
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 be "opaquelocktoken:"
And the value of the item "//d:timeout" in the response should match "/Second-\d+/"
Examples:
| lock-scope |
| shared |
| exclusive |

Scenario: lockdiscovery root of public link when root is locked by public and user
Given 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 | shared |
And the public has locked the last public shared folder setting following properties
| lockscope | shared |
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: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 be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[1]/d:timeout" in the response should match "/Second-\d+/"
And the value of the item "//d:activelock[2]/d:timeout" in the response should match "/Second-\d+/"

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
And user "user0" has locked folder "PARENT" setting following properties
| lockscope | shared |
And the public has locked "CHILD" in the last public shared folder setting following properties
| lockscope | shared |
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: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 be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[1]/d:timeout" in the response should match "/Second-\d+/"
And the value of the item "//d:activelock[2]/d:timeout" in the response should match "/Second-\d+/"

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
And user "user0" has locked folder "PARENT" setting following properties
| lockscope | shared |
And the public has locked "/" in the last public shared folder setting following properties
| lockscope | shared |
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: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 be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[1]/d:timeout" in the response should match "/Second-\d+/"
And the value of the item "//d:activelock[2]/d:timeout" in the response should match "/Second-\d+/"

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
And user "user0" has created folder "PARENT/CHILD/GRANDCHILD"
And user "user0" has locked folder "PARENT" setting following properties
| lockscope | shared |
And the public has locked "/GRANDCHILD" in the last public shared folder setting following properties
| lockscope | shared |
When the public gets the following properties of entry "/GRANDCHILD" in the last created public link using the WebDAV API
| 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 be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[1]/d:timeout" in the response should match "/Second-\d+/"
And the value of the item "//d:activelock[2]/d:timeout" in the response should match "/Second-\d+/"

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
And user "user0" has locked folder "PARENT" setting following properties
| lockscope | shared |
And the public has locked "/child.txt" in the last public shared folder setting following properties
| lockscope | shared |
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: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 be "opaquelocktoken:"
And the value of the item "//d:activelock[2]/d:locktoken/d:href" in the response should be "opaquelocktoken:"
And the value of the item "//d:activelock[1]/d:timeout" in the response should match "/Second-\d+/"
And the value of the item "//d:activelock[2]/d:timeout" in the response should match "/Second-\d+/"
20 changes: 0 additions & 20 deletions tests/acceptance/features/apiWebdavLocks/resharedShares.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,23 +106,3 @@ Feature: lock should propagate correctly if a share is reshared
| new | shared |
| new | exclusive |

Scenario Outline: upload to a reshared share that was locked by the public
Given using <dav-path> DAV path
And user "user0" has shared folder "PARENT" with user "user1"
And user "user1" has shared folder "PARENT (2)" with user "user2"
And user "user2" has created a public link share of folder "PARENT (2)" with change permission
And the public locks "/CHILD" in the last public shared folder using the WebDAV API setting following properties
| lockscope | shared |
When user "user2" uploads file "filesForUpload/textfile.txt" to "/PARENT (2)/CHILD/textfile.txt" using the WebDAV API
Then the HTTP status code should be "423"
When user "user1" uploads file "filesForUpload/textfile.txt" to "/PARENT (2)/CHILD/textfile.txt" using the WebDAV API
Then the HTTP status code should be "423"
When user "user0" uploads file "filesForUpload/textfile.txt" to "/PARENT/CHILD/textfile.txt" using the WebDAV API
Then the HTTP status code should be "423"
And as "user0" file "/PARENT/textfile.txt" should not exist
Examples:
| dav-path | lock-scope |
| old | shared |
| old | exclusive |
| new | shared |
| new | exclusive |
27 changes: 0 additions & 27 deletions tests/acceptance/features/apiWebdavLocks/setTimeout.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,30 +117,3 @@ Feature: set timeouts of LOCKS
| new | second--1 | /Second-\d{5}$/ |
| new | second-0 | /Second-\d{4}$/ |

Scenario Outline: as public set timeout on folder as owner check it
Given using <dav-path> DAV path
And user "user0" has created a public link share of folder "PARENT"
When the public locks the last public shared folder using the WebDAV API setting following properties
| lockscope | shared |
| timeout | <timeout> |
And user "user0" gets the following properties of folder "PARENT" using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:timeout" in the response should match "<result>"
When user "user0" gets the following properties of folder "PARENT/CHILD" using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:timeout" in the response should match "<result>"
When user "user0" gets the following properties of folder "PARENT/parent.txt" using the WebDAV API
| d:lockdiscovery |
Then the value of the item "//d:timeout" in the response should match "<result>"
Examples:
| dav-path | timeout | result |
| old | second-999 | /Second-\d{3}$/ |
| old | second-99999999 | /Second-\d{5}$/ |
| old | infinite | /Second-\d{5}$/ |
| old | second--1 | /Second-\d{5}$/ |
| old | second-0 | /Second-\d{4}$/ |
| new | second-999 | /Second-\d{3}$/ |
| new | second-99999999 | /Second-\d{5}$/ |
| new | infinite | /Second-\d{5}$/ |
| new | second--1 | /Second-\d{5}$/ |
| new | second-0 | /Second-\d{4}$/ |
Loading