From 73856ac6397031d4587963599253596987a173d5 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 23 Jan 2017 11:23:38 +0100 Subject: [PATCH] Error out when subadmin doesn't select any group Signed-off-by: Joas Schilling --- settings/Controller/UsersController.php | 15 +- .../Controller/UsersControllerTest.php | 209 ++++++------------ 2 files changed, 71 insertions(+), 153 deletions(-) diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php index 28b8d2648d9..43a38432499 100644 --- a/settings/Controller/UsersController.php +++ b/settings/Controller/UsersController.php @@ -346,13 +346,12 @@ class UsersController extends Controller { } if (empty($groups)) { - $groups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($currentUser); - // New class returns IGroup[] so convert back - $gids = []; - foreach ($groups as $group) { - $gids[] = $group->getGID(); - } - $groups = $gids; + return new DataResponse( + array( + 'message' => $this->l10n->t('No valid group selected'), + ), + Http::STATUS_FORBIDDEN + ); } } @@ -380,7 +379,7 @@ class UsersController extends Controller { ); } - if($user instanceof User) { + if($user instanceof IUser) { if($groups !== null) { foreach($groups as $groupName) { $group = $this->groupManager->get($groupName); diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php index b85667740f7..36ca0917524 100644 --- a/tests/Settings/Controller/UsersControllerTest.php +++ b/tests/Settings/Controller/UsersControllerTest.php @@ -19,6 +19,7 @@ use OCP\AppFramework\Http\DataResponse; use OCP\IAvatar; use OCP\IAvatarManager; use OCP\IConfig; +use OCP\IGroup; use OCP\IGroupManager; use OCP\IL10N; use OCP\ILogger; @@ -852,95 +853,6 @@ class UsersControllerTest extends \Test\TestCase { $this->assertEquals($expectedResponse, $response); } - public function testCreateSuccessfulWithoutGroupSubAdmin() { - $controller = $this->getController(false); - $user = $this->getMockBuilder('\OC\User\User') - ->disableOriginalConstructor()->getMock(); - $this->userSession - ->expects($this->once()) - ->method('getUser') - ->will($this->returnValue($user)); - - $newUser = $this->getMockBuilder('\OC\User\User') - ->disableOriginalConstructor()->getMock(); - $newUser - ->method('getUID') - ->will($this->returnValue('foo')); - $newUser - ->method('getHome') - ->will($this->returnValue('/home/user')); - $newUser - ->method('getHome') - ->will($this->returnValue('/home/user')); - $newUser - ->expects($this->once()) - ->method('getBackendClassName') - ->will($this->returnValue('bar')); - $user = $this->getMockBuilder('\OC\User\User') - ->disableOriginalConstructor()->getMock(); - $subGroup1 = $this->getMockBuilder('\OCP\IGroup') - ->disableOriginalConstructor()->getMock(); - $subGroup1 - ->expects($this->once()) - ->method('addUser') - ->with($newUser); - $subGroup2 = $this->getMockBuilder('\OCP\IGroup') - ->disableOriginalConstructor()->getMock(); - $subGroup2 - ->expects($this->once()) - ->method('addUser') - ->with($newUser); - - $this->userManager - ->expects($this->once()) - ->method('createUser') - ->will($this->returnValue($newUser)); - $this->groupManager - ->expects($this->exactly(2)) - ->method('get') - ->will($this->onConsecutiveCalls($subGroup1, $subGroup2)); - $this->groupManager - ->expects($this->once()) - ->method('getUserGroupIds') - ->with($user) - ->will($this->onConsecutiveCalls(['SubGroup1', 'SubGroup2'])); - - $subadmin = $this->getMockBuilder('\OC\SubAdmin') - ->disableOriginalConstructor() - ->getMock(); - $subadmin - ->expects($this->at(0)) - ->method('getSubAdminsGroups') - ->will($this->returnValue([$subGroup1, $subGroup2])); - $subadmin - ->expects($this->at(1)) - ->method('getSubAdminsGroups') - ->will($this->returnValue([])); - $this->groupManager - ->expects($this->any()) - ->method('getSubAdmin') - ->will($this->returnValue($subadmin)); - - $expectedResponse = new DataResponse( - array( - 'name' => 'foo', - 'groups' => ['SubGroup1', 'SubGroup2'], - 'storageLocation' => '/home/user', - 'backend' => 'bar', - 'lastLogin' => 0, - 'displayname' => null, - 'quota' => null, - 'subadmin' => [], - 'email' => null, - 'isRestoreDisabled' => false, - 'isAvatarAvailable' => true, - ), - Http::STATUS_CREATED - ); - $response = $controller->create('foo', 'password'); - $this->assertEquals($expectedResponse, $response); - } - public function testCreateSuccessfulWithGroupAdmin() { $controller = $this->getController(true); @@ -1026,16 +938,12 @@ class UsersControllerTest extends \Test\TestCase { public function testCreateSuccessfulWithGroupSubAdmin() { $controller = $this->getController(false); - $user = $this->getMockBuilder('\OC\User\User') - ->disableOriginalConstructor()->getMock(); + $user = $this->createMock(IUser::class); $this->userSession ->expects($this->once()) ->method('getUser') ->will($this->returnValue($user)); - $user = $this->getMockBuilder('\OC\User\User') - ->disableOriginalConstructor()->getMock(); - $newUser = $this->getMockBuilder('\OC\User\User') - ->disableOriginalConstructor()->getMock(); + $newUser = $this->createMock(IUser::class); $newUser ->method('getHome') ->will($this->returnValue('/home/user')); @@ -1049,8 +957,7 @@ class UsersControllerTest extends \Test\TestCase { ->expects($this->once()) ->method('getBackendClassName') ->will($this->returnValue('bar')); - $subGroup1 = $this->getMockBuilder('\OCP\IGroup') - ->disableOriginalConstructor()->getMock(); + $subGroup1 = $this->createMock(IGroup::class); $subGroup1 ->expects($this->any()) ->method('getGID') @@ -1063,16 +970,6 @@ class UsersControllerTest extends \Test\TestCase { ->expects($this->once()) ->method('createUser') ->will($this->returnValue($newUser)); - $this->groupManager - ->expects($this->at(0)) - ->method('get') - ->with('SubGroup1') - ->will($this->returnValue($subGroup1)); - $this->groupManager - ->expects($this->at(4)) - ->method('get') - ->with('SubGroup1') - ->will($this->returnValue($subGroup1)); $this->groupManager ->expects($this->once()) ->method('getUserGroupIds') @@ -1084,21 +981,28 @@ class UsersControllerTest extends \Test\TestCase { ->with($newUser) ->will($this->onConsecutiveCalls(['SubGroup1'])); - $subadmin = $this->getMockBuilder('\OC\SubAdmin') - ->disableOriginalConstructor() - ->getMock(); - $subadmin->expects($this->at(1)) + $subadmin = $this->createMock(\OC\SubAdmin::class); + $subadmin->expects($this->atLeastOnce()) ->method('getSubAdminsGroups') ->with($user) - ->will($this->returnValue([$subGroup1])); - $subadmin->expects($this->at(2)) - ->method('getSubAdminsGroups') - ->with($newUser) - ->will($this->returnValue([])); + ->willReturnMap([ + [$user, [$subGroup1]], + [$newUser, []], + ]); + $subadmin->expects($this->atLeastOnce()) + ->method('isSubAdminofGroup') + ->willReturnMap([ + [$user, $subGroup1, true], + ]); $this->groupManager ->expects($this->any()) ->method('getSubAdmin') ->will($this->returnValue($subadmin)); + $this->groupManager->expects($this->atLeastOnce()) + ->method('get') + ->willReturnMap([ + ['SubGroup1', $subGroup1], + ]); $expectedResponse = new DataResponse( array( @@ -1137,16 +1041,36 @@ class UsersControllerTest extends \Test\TestCase { $this->assertEquals($expectedResponse, $response); } + public function testCreateUnsuccessfulSubAdminNoGroup() { + $controller = $this->getController(false); + $user = $this->createMock(IUser::class); + $user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('username')); + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + + $this->userManager->expects($this->never()) + ->method('createUser'); + + $expectedResponse = new DataResponse( + [ + 'message' => 'No valid group selected' + ], + Http::STATUS_FORBIDDEN + ); + $response = $controller->create('foo', 'password', []); + $this->assertEquals($expectedResponse, $response); + } + public function testCreateUnsuccessfulSubAdmin() { $controller = $this->getController(false); - $user = $this->getMockBuilder('\OC\User\User') - ->disableOriginalConstructor()->getMock(); - $user - ->expects($this->any()) + $user = $this->createMock(IUser::class); + $user->expects($this->any()) ->method('getUID') ->will($this->returnValue('username')); - $this->userSession - ->expects($this->once()) + $this->userSession->expects($this->once()) ->method('getUser') ->will($this->returnValue($user)); @@ -1154,29 +1078,24 @@ class UsersControllerTest extends \Test\TestCase { ->method('createUser') ->will($this->throwException(new \Exception())); - $subgroup1 = $this->getMockBuilder('\OCP\IGroup') - ->disableOriginalConstructor() - ->getMock(); - $subgroup1->expects($this->once()) - ->method('getGID') - ->will($this->returnValue('SubGroup1')); - $subgroup2 = $this->getMockBuilder('\OCP\IGroup') - ->disableOriginalConstructor() - ->getMock(); - $subgroup2->expects($this->once()) - ->method('getGID') - ->will($this->returnValue('SubGroup2')); - $subadmin = $this->getMockBuilder('\OC\SubAdmin') - ->disableOriginalConstructor() - ->getMock(); - $subadmin->expects($this->once()) - ->method('getSubAdminsGroups') - ->with($user) - ->will($this->returnValue([$subgroup1, $subgroup2])); - $this->groupManager - ->expects($this->any()) + $subgroup1 = $this->createMock(IGroup::class); + $subgroup2 = $this->createMock(IGroup::class); + $subadmin = $this->createMock(\OC\SubAdmin::class); + $subadmin->expects($this->atLeastOnce()) + ->method('isSubAdminofGroup') + ->willReturnMap([ + [$user, $subgroup1, true], + [$user, $subgroup2, true], + ]); + $this->groupManager->expects($this->any()) ->method('getSubAdmin') - ->will($this->returnValue($subadmin)); + ->willReturn($subadmin); + $this->groupManager->expects($this->atLeastOnce()) + ->method('get') + ->willReturnMap([ + ['SubGroup1', $subgroup1], + ['SubGroup2', $subgroup2], + ]); $expectedResponse = new DataResponse( [ @@ -1184,7 +1103,7 @@ class UsersControllerTest extends \Test\TestCase { ], Http::STATUS_FORBIDDEN ); - $response = $controller->create('foo', 'password', array()); + $response = $controller->create('foo', 'password', array('SubGroup1', 'SubGroup2')); $this->assertEquals($expectedResponse, $response); } -- 2.39.5