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>
This commit is contained in:
parent
0195740147
commit
c75b5a5614
|
@ -150,7 +150,7 @@ class Principal implements BackendInterface {
|
||||||
$groups = $this->groupManager->getUserGroups($user);
|
$groups = $this->groupManager->getUserGroups($user);
|
||||||
$groups = array_map(function($group) {
|
$groups = array_map(function($group) {
|
||||||
/** @var IGroup $group */
|
/** @var IGroup $group */
|
||||||
return 'principals/groups/' . $group->getGID();
|
return 'principals/groups/' . urlencode($group->getGID());
|
||||||
}, $groups);
|
}, $groups);
|
||||||
|
|
||||||
return $groups;
|
return $groups;
|
||||||
|
|
|
@ -76,14 +76,14 @@ class GroupPrincipalBackend implements BackendInterface {
|
||||||
* @return array
|
* @return array
|
||||||
*/
|
*/
|
||||||
public function getPrincipalByPath($path) {
|
public function getPrincipalByPath($path) {
|
||||||
$elements = explode('/', $path);
|
$elements = explode('/', $path, 3);
|
||||||
if ($elements[0] !== 'principals') {
|
if ($elements[0] !== 'principals') {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
if ($elements[1] !== 'groups') {
|
if ($elements[1] !== 'groups') {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
$name = $elements[2];
|
$name = urldecode($elements[2]);
|
||||||
$group = $this->groupManager->get($name);
|
$group = $this->groupManager->get($name);
|
||||||
|
|
||||||
if (!is_null($group)) {
|
if (!is_null($group)) {
|
||||||
|
@ -179,7 +179,7 @@ class GroupPrincipalBackend implements BackendInterface {
|
||||||
protected function groupToPrincipal($group) {
|
protected function groupToPrincipal($group) {
|
||||||
$groupId = $group->getGID();
|
$groupId = $group->getGID();
|
||||||
$principal = [
|
$principal = [
|
||||||
'uri' => "principals/groups/$groupId",
|
'uri' => 'principals/groups/' . urlencode($groupId),
|
||||||
'{DAV:}displayname' => $groupId,
|
'{DAV:}displayname' => $groupId,
|
||||||
];
|
];
|
||||||
|
|
||||||
|
|
|
@ -25,6 +25,8 @@
|
||||||
|
|
||||||
namespace OCA\DAV\Tests\unit\Connector\Sabre;
|
namespace OCA\DAV\Tests\unit\Connector\Sabre;
|
||||||
|
|
||||||
|
use OC\User\User;
|
||||||
|
use OCP\IGroup;
|
||||||
use OCP\IGroupManager;
|
use OCP\IGroupManager;
|
||||||
use \Sabre\DAV\PropPatch;
|
use \Sabre\DAV\PropPatch;
|
||||||
use OCP\IUserManager;
|
use OCP\IUserManager;
|
||||||
|
@ -39,10 +41,8 @@ class PrincipalTest extends TestCase {
|
||||||
private $groupManager;
|
private $groupManager;
|
||||||
|
|
||||||
public function setUp() {
|
public function setUp() {
|
||||||
$this->userManager = $this->getMockBuilder('\OCP\IUserManager')
|
$this->userManager = $this->createMock(IUserManager::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
$this->groupManager = $this->createMock(IGroupManager::class);
|
||||||
$this->groupManager = $this->getMockBuilder('\OCP\IGroupManager')
|
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
|
|
||||||
$this->connector = new \OCA\DAV\Connector\Sabre\Principal(
|
$this->connector = new \OCA\DAV\Connector\Sabre\Principal(
|
||||||
$this->userManager,
|
$this->userManager,
|
||||||
|
@ -56,8 +56,7 @@ class PrincipalTest extends TestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testGetPrincipalsByPrefixWithUsers() {
|
public function testGetPrincipalsByPrefixWithUsers() {
|
||||||
$fooUser = $this->getMockBuilder('\OC\User\User')
|
$fooUser = $this->createMock(User::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
$fooUser
|
$fooUser
|
||||||
->expects($this->exactly(1))
|
->expects($this->exactly(1))
|
||||||
->method('getUID')
|
->method('getUID')
|
||||||
|
@ -70,8 +69,7 @@ class PrincipalTest extends TestCase {
|
||||||
->expects($this->exactly(1))
|
->expects($this->exactly(1))
|
||||||
->method('getEMailAddress')
|
->method('getEMailAddress')
|
||||||
->will($this->returnValue(''));
|
->will($this->returnValue(''));
|
||||||
$barUser = $this->getMockBuilder('\OC\User\User')
|
$barUser = $this->createMock(User::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
$barUser
|
$barUser
|
||||||
->expects($this->exactly(1))
|
->expects($this->exactly(1))
|
||||||
->method('getUID')
|
->method('getUID')
|
||||||
|
@ -113,8 +111,7 @@ class PrincipalTest extends TestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testGetPrincipalsByPathWithoutMail() {
|
public function testGetPrincipalsByPathWithoutMail() {
|
||||||
$fooUser = $this->getMockBuilder('\OC\User\User')
|
$fooUser = $this->createMock(User::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
$fooUser
|
$fooUser
|
||||||
->expects($this->exactly(1))
|
->expects($this->exactly(1))
|
||||||
->method('getUID')
|
->method('getUID')
|
||||||
|
@ -134,8 +131,7 @@ class PrincipalTest extends TestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testGetPrincipalsByPathWithMail() {
|
public function testGetPrincipalsByPathWithMail() {
|
||||||
$fooUser = $this->getMockBuilder('\OC\User\User')
|
$fooUser = $this->createMock(User::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
$fooUser
|
$fooUser
|
||||||
->expects($this->exactly(1))
|
->expects($this->exactly(1))
|
||||||
->method('getEMailAddress')
|
->method('getEMailAddress')
|
||||||
|
@ -171,8 +167,7 @@ class PrincipalTest extends TestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testGetGroupMemberSet() {
|
public function testGetGroupMemberSet() {
|
||||||
$fooUser = $this->getMockBuilder('\OC\User\User')
|
$fooUser = $this->createMock(User::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
$fooUser
|
$fooUser
|
||||||
->expects($this->exactly(1))
|
->expects($this->exactly(1))
|
||||||
->method('getUID')
|
->method('getUID')
|
||||||
|
@ -202,13 +197,15 @@ class PrincipalTest extends TestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testGetGroupMembership() {
|
public function testGetGroupMembership() {
|
||||||
$fooUser = $this->getMockBuilder('\OC\User\User')
|
$fooUser = $this->createMock(User::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
$group1 = $this->createMock(IGroup::class);
|
||||||
$group = $this->getMockBuilder('\OCP\IGroup')
|
$group1->expects($this->once())
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
$group->expects($this->once())
|
|
||||||
->method('getGID')
|
->method('getGID')
|
||||||
->willReturn('group1');
|
->willReturn('group1');
|
||||||
|
$group2 = $this->createMock(IGroup::class);
|
||||||
|
$group2->expects($this->once())
|
||||||
|
->method('getGID')
|
||||||
|
->willReturn('foo/bar');
|
||||||
$this->userManager
|
$this->userManager
|
||||||
->expects($this->once())
|
->expects($this->once())
|
||||||
->method('get')
|
->method('get')
|
||||||
|
@ -217,12 +214,15 @@ class PrincipalTest extends TestCase {
|
||||||
$this->groupManager
|
$this->groupManager
|
||||||
->expects($this->once())
|
->expects($this->once())
|
||||||
->method('getUserGroups')
|
->method('getUserGroups')
|
||||||
|
->with($fooUser)
|
||||||
->willReturn([
|
->willReturn([
|
||||||
$group
|
$group1,
|
||||||
|
$group2,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$expectedResponse = [
|
$expectedResponse = [
|
||||||
'principals/groups/group1'
|
'principals/groups/group1',
|
||||||
|
'principals/groups/foo%2Fbar',
|
||||||
];
|
];
|
||||||
$response = $this->connector->getGroupMembership('principals/users/foo');
|
$response = $this->connector->getGroupMembership('principals/users/foo');
|
||||||
$this->assertSame($expectedResponse, $response);
|
$this->assertSame($expectedResponse, $response);
|
||||||
|
@ -259,8 +259,7 @@ class PrincipalTest extends TestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testFindByUri() {
|
public function testFindByUri() {
|
||||||
$fooUser = $this->getMockBuilder('\OC\User\User')
|
$fooUser = $this->createMock(User::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
$fooUser
|
$fooUser
|
||||||
->expects($this->exactly(1))
|
->expects($this->exactly(1))
|
||||||
->method('getUID')
|
->method('getUID')
|
||||||
|
|
|
@ -23,6 +23,7 @@
|
||||||
|
|
||||||
namespace OCA\DAV\Tests\unit\DAV;
|
namespace OCA\DAV\Tests\unit\DAV;
|
||||||
|
|
||||||
|
use OC\Group\Group;
|
||||||
use OCA\DAV\DAV\GroupPrincipalBackend;
|
use OCA\DAV\DAV\GroupPrincipalBackend;
|
||||||
use OCP\IGroupManager;
|
use OCP\IGroupManager;
|
||||||
use PHPUnit_Framework_MockObject_MockObject;
|
use PHPUnit_Framework_MockObject_MockObject;
|
||||||
|
@ -37,8 +38,7 @@ class GroupPrincipalTest extends \Test\TestCase {
|
||||||
private $connector;
|
private $connector;
|
||||||
|
|
||||||
public function setUp() {
|
public function setUp() {
|
||||||
$this->groupManager = $this->getMockBuilder('\OCP\IGroupManager')
|
$this->groupManager = $this->createMock(IGroupManager::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
|
||||||
|
|
||||||
$this->connector = new GroupPrincipalBackend($this->groupManager);
|
$this->connector = new GroupPrincipalBackend($this->groupManager);
|
||||||
parent::setUp();
|
parent::setUp();
|
||||||
|
@ -126,6 +126,22 @@ class GroupPrincipalTest extends \Test\TestCase {
|
||||||
$this->assertSame(null, $response);
|
$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() {
|
public function testGetGroupMemberSet() {
|
||||||
$response = $this->connector->getGroupMemberSet('principals/groups/foo');
|
$response = $this->connector->getGroupMemberSet('principals/groups/foo');
|
||||||
$this->assertSame([], $response);
|
$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) {
|
private function mockGroup($gid) {
|
||||||
$fooUser = $this->getMockBuilder('\OC\Group\Group')
|
$fooGroup = $this->createMock(Group::class);
|
||||||
->disableOriginalConstructor()->getMock();
|
$fooGroup
|
||||||
$fooUser
|
|
||||||
->expects($this->exactly(1))
|
->expects($this->exactly(1))
|
||||||
->method('getGID')
|
->method('getGID')
|
||||||
->will($this->returnValue($gid));
|
->will($this->returnValue($gid));
|
||||||
return $fooUser;
|
return $fooGroup;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue