aboutsummaryrefslogtreecommitdiffstats
path: root/apps/dav
diff options
context:
space:
mode:
authorRoeland Jago Douma <roeland@famdouma.nl>2017-01-09 21:20:55 +0100
committerRoeland Jago Douma <roeland@famdouma.nl>2017-02-27 17:07:18 +0100
commitc75b5a56140605308237d94038947b1b27941521 (patch)
tree3f5fd18273e0e5cd0f9bd6e677c3391736ad2d3a /apps/dav
parent019574014702657fa34acac4a631f78e27c20db8 (diff)
downloadnextcloud-server-c75b5a56140605308237d94038947b1b27941521.tar.gz
nextcloud-server-c75b5a56140605308237d94038947b1b27941521.zip
Properly handle groups with a /
If a group contains a slash the principal URI becomes principals/groups/foo/bar. Now the URI is plit on '/' so this creates issues ;) Fixes #2957 * Add tests for groups with / Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Diffstat (limited to 'apps/dav')
-rw-r--r--apps/dav/lib/Connector/Sabre/Principal.php2
-rw-r--r--apps/dav/lib/DAV/GroupPrincipalBackend.php6
-rw-r--r--apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php45
-rw-r--r--apps/dav/tests/unit/DAV/GroupPrincipalTest.php29
4 files changed, 48 insertions, 34 deletions
diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php
index a592ed80918..9da416312d0 100644
--- a/apps/dav/lib/Connector/Sabre/Principal.php
+++ b/apps/dav/lib/Connector/Sabre/Principal.php
@@ -150,7 +150,7 @@ class Principal implements BackendInterface {
$groups = $this->groupManager->getUserGroups($user);
$groups = array_map(function($group) {
/** @var IGroup $group */
- return 'principals/groups/' . $group->getGID();
+ return 'principals/groups/' . urlencode($group->getGID());
}, $groups);
return $groups;
diff --git a/apps/dav/lib/DAV/GroupPrincipalBackend.php b/apps/dav/lib/DAV/GroupPrincipalBackend.php
index f8dc1e60881..f6295d3b386 100644
--- a/apps/dav/lib/DAV/GroupPrincipalBackend.php
+++ b/apps/dav/lib/DAV/GroupPrincipalBackend.php
@@ -76,14 +76,14 @@ class GroupPrincipalBackend implements BackendInterface {
* @return array
*/
public function getPrincipalByPath($path) {
- $elements = explode('/', $path);
+ $elements = explode('/', $path, 3);
if ($elements[0] !== 'principals') {
return null;
}
if ($elements[1] !== 'groups') {
return null;
}
- $name = $elements[2];
+ $name = urldecode($elements[2]);
$group = $this->groupManager->get($name);
if (!is_null($group)) {
@@ -179,7 +179,7 @@ class GroupPrincipalBackend implements BackendInterface {
protected function groupToPrincipal($group) {
$groupId = $group->getGID();
$principal = [
- 'uri' => "principals/groups/$groupId",
+ 'uri' => 'principals/groups/' . urlencode($groupId),
'{DAV:}displayname' => $groupId,
];
diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
index 03856807026..8d7485769fa 100644
--- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
@@ -25,6 +25,8 @@
namespace OCA\DAV\Tests\unit\Connector\Sabre;
+use OC\User\User;
+use OCP\IGroup;
use OCP\IGroupManager;
use \Sabre\DAV\PropPatch;
use OCP\IUserManager;
@@ -39,10 +41,8 @@ class PrincipalTest extends TestCase {
private $groupManager;
public function setUp() {
- $this->userManager = $this->getMockBuilder('\OCP\IUserManager')
- ->disableOriginalConstructor()->getMock();
- $this->groupManager = $this->getMockBuilder('\OCP\IGroupManager')
- ->disableOriginalConstructor()->getMock();
+ $this->userManager = $this->createMock(IUserManager::class);
+ $this->groupManager = $this->createMock(IGroupManager::class);
$this->connector = new \OCA\DAV\Connector\Sabre\Principal(
$this->userManager,
@@ -56,8 +56,7 @@ class PrincipalTest extends TestCase {
}
public function testGetPrincipalsByPrefixWithUsers() {
- $fooUser = $this->getMockBuilder('\OC\User\User')
- ->disableOriginalConstructor()->getMock();
+ $fooUser = $this->createMock(User::class);
$fooUser
->expects($this->exactly(1))
->method('getUID')
@@ -70,8 +69,7 @@ class PrincipalTest extends TestCase {
->expects($this->exactly(1))
->method('getEMailAddress')
->will($this->returnValue(''));
- $barUser = $this->getMockBuilder('\OC\User\User')
- ->disableOriginalConstructor()->getMock();
+ $barUser = $this->createMock(User::class);
$barUser
->expects($this->exactly(1))
->method('getUID')
@@ -113,8 +111,7 @@ class PrincipalTest extends TestCase {
}
public function testGetPrincipalsByPathWithoutMail() {
- $fooUser = $this->getMockBuilder('\OC\User\User')
- ->disableOriginalConstructor()->getMock();
+ $fooUser = $this->createMock(User::class);
$fooUser
->expects($this->exactly(1))
->method('getUID')
@@ -134,8 +131,7 @@ class PrincipalTest extends TestCase {
}
public function testGetPrincipalsByPathWithMail() {
- $fooUser = $this->getMockBuilder('\OC\User\User')
- ->disableOriginalConstructor()->getMock();
+ $fooUser = $this->createMock(User::class);
$fooUser
->expects($this->exactly(1))
->method('getEMailAddress')
@@ -171,8 +167,7 @@ class PrincipalTest extends TestCase {
}
public function testGetGroupMemberSet() {
- $fooUser = $this->getMockBuilder('\OC\User\User')
- ->disableOriginalConstructor()->getMock();
+ $fooUser = $this->createMock(User::class);
$fooUser
->expects($this->exactly(1))
->method('getUID')
@@ -202,13 +197,15 @@ class PrincipalTest extends TestCase {
}
public function testGetGroupMembership() {
- $fooUser = $this->getMockBuilder('\OC\User\User')
- ->disableOriginalConstructor()->getMock();
- $group = $this->getMockBuilder('\OCP\IGroup')
- ->disableOriginalConstructor()->getMock();
- $group->expects($this->once())
+ $fooUser = $this->createMock(User::class);
+ $group1 = $this->createMock(IGroup::class);
+ $group1->expects($this->once())
->method('getGID')
->willReturn('group1');
+ $group2 = $this->createMock(IGroup::class);
+ $group2->expects($this->once())
+ ->method('getGID')
+ ->willReturn('foo/bar');
$this->userManager
->expects($this->once())
->method('get')
@@ -217,12 +214,15 @@ class PrincipalTest extends TestCase {
$this->groupManager
->expects($this->once())
->method('getUserGroups')
+ ->with($fooUser)
->willReturn([
- $group
+ $group1,
+ $group2,
]);
$expectedResponse = [
- 'principals/groups/group1'
+ 'principals/groups/group1',
+ 'principals/groups/foo%2Fbar',
];
$response = $this->connector->getGroupMembership('principals/users/foo');
$this->assertSame($expectedResponse, $response);
@@ -259,8 +259,7 @@ class PrincipalTest extends TestCase {
}
public function testFindByUri() {
- $fooUser = $this->getMockBuilder('\OC\User\User')
- ->disableOriginalConstructor()->getMock();
+ $fooUser = $this->createMock(User::class);
$fooUser
->expects($this->exactly(1))
->method('getUID')
diff --git a/apps/dav/tests/unit/DAV/GroupPrincipalTest.php b/apps/dav/tests/unit/DAV/GroupPrincipalTest.php
index e532ed164e9..180fc31040a 100644
--- a/apps/dav/tests/unit/DAV/GroupPrincipalTest.php
+++ b/apps/dav/tests/unit/DAV/GroupPrincipalTest.php
@@ -23,6 +23,7 @@
namespace OCA\DAV\Tests\unit\DAV;
+use OC\Group\Group;
use OCA\DAV\DAV\GroupPrincipalBackend;
use OCP\IGroupManager;
use PHPUnit_Framework_MockObject_MockObject;
@@ -37,8 +38,7 @@ class GroupPrincipalTest extends \Test\TestCase {
private $connector;
public function setUp() {
- $this->groupManager = $this->getMockBuilder('\OCP\IGroupManager')
- ->disableOriginalConstructor()->getMock();
+ $this->groupManager = $this->createMock(IGroupManager::class);
$this->connector = new GroupPrincipalBackend($this->groupManager);
parent::setUp();
@@ -126,6 +126,22 @@ class GroupPrincipalTest extends \Test\TestCase {
$this->assertSame(null, $response);
}
+ public function testGetPrincipalsByPathGroupWithSlash() {
+ $group1 = $this->mockGroup('foo/bar');
+ $this->groupManager
+ ->expects($this->once())
+ ->method('get')
+ ->with('foo/bar')
+ ->will($this->returnValue($group1));
+
+ $expectedResponse = [
+ 'uri' => 'principals/groups/foo%2Fbar',
+ '{DAV:}displayname' => 'foo/bar'
+ ];
+ $response = $this->connector->getPrincipalByPath('principals/groups/foo/bar');
+ $this->assertSame($expectedResponse, $response);
+ }
+
public function testGetGroupMemberSet() {
$response = $this->connector->getGroupMemberSet('principals/groups/foo');
$this->assertSame([], $response);
@@ -153,15 +169,14 @@ class GroupPrincipalTest extends \Test\TestCase {
}
/**
- * @return PHPUnit_Framework_MockObject_MockObject
+ * @return Group|\PHPUnit_Framework_MockObject_MockObject
*/
private function mockGroup($gid) {
- $fooUser = $this->getMockBuilder('\OC\Group\Group')
- ->disableOriginalConstructor()->getMock();
- $fooUser
+ $fooGroup = $this->createMock(Group::class);
+ $fooGroup
->expects($this->exactly(1))
->method('getGID')
->will($this->returnValue($gid));
- return $fooUser;
+ return $fooGroup;
}
}