Fix tests

owncloud · May 13, 2020 · 739bc51 · 739bc51
1 parent 75fbc49
commit 739bc51
Show file tree

Hide file tree

Showing 6 changed files with 540 additions and 685 deletions.
diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php
@@ -28,7 +28,7 @@
 namespace OCA\Provisioning_API\AppInfo;
 
 $application = new Application();
-/** @phan-suppress-next-line PhanUndeclaredThis */
+/** @phan-suppress-next-line PhanUndeclaredThisenable */
 $application->registerRoutes($this, [
 	'ocs' => [
 		// Users
@@ -54,8 +54,8 @@
 		['root' => '/cloud', 'name' => 'Groups#getSubAdminsOfGroup', 'url' => '/groups/{groupId}/subadmins', 'verb' => 'GET'],
 		// Apps
 		['root' => '/cloud', 'name' => 'Apps#getApps', 'url' => '/apps', 'verb' => 'GET'],
-		['root' => '/cloud', 'name' => 'Apps#getAppInfo', 'url' => '/apps/{appid}', 'verb' => 'GET'],
-		['root' => '/cloud', 'name' => 'Apps#enable', 'url' => '/apps/{appid}', 'verb' => 'POST'],
-		['root' => '/cloud', 'name' => 'Apps#disable', 'url' => '/apps/{appid}', 'verb' => 'DELETE'],
+		['root' => '/cloud', 'name' => 'Apps#getAppInfo', 'url' => '/apps/{appId}', 'verb' => 'GET'],
+		['root' => '/cloud', 'name' => 'Apps#enable', 'url' => '/apps/{appId}', 'verb' => 'POST'],
+		['root' => '/cloud', 'name' => 'Apps#disable', 'url' => '/apps/{appId}', 'verb' => 'DELETE'],
 	]
 ]);
diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php
@@ -100,6 +100,9 @@ public function getGroups() {
 	 */
 	public function getGroup($groupId = '') {
 		$user = $this->userSession->getUser();
+		if ($user === null) {
+			return new Result(null, 102);
+		}
 		if ($this->isSubAdmin($user) === false) {
 			return new Result(null, API::RESPOND_UNAUTHORISED);
 		}
@@ -150,9 +153,12 @@ public function addGroup() {
 			return new Result(null, 102);
 		}
 		$user = $this->userSession->getUser();
-		if ($this->isSubAdmin($user) === false) {
+		if ($user === null) {
 			return new Result(null, 102);
 		}
+		if ($this->isSubAdmin($user) === false) {
+			return new Result(null, API::RESPOND_UNAUTHORISED);
+		}
 		// Only admin has got privilege to create group
 		if ($this->groupManager->isAdmin($user->getUID())) {
 			$this->groupManager->createGroup($groupId);
@@ -205,10 +211,10 @@ public function getSubAdminsOfGroup($groupId) {
 	}
 
 	/**
-	 * @param IUser $user
+	 * @param IUser|null $user
 	 * @return bool
 	 */
-	private function isSubAdmin(IUser $user) {
+	private function isSubAdmin($user) {
 		if ($user === null) {
 			return false;
 		}

diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php
@@ -491,10 +491,10 @@ private function canUserManageGroup($user, $group) {
 	}
 
 	/**
-	 * @param IUser $user
+	 * @param IUser|null $user
 	 * @return bool
 	 */
-	private function isSubAdmin(IUser $user) {
+	private function isSubAdmin($user) {
 		if ($user === null) {
 			return false;
 		}

diff --git a/apps/provisioning_api/tests/Controller/AppsControllerTest.php b/apps/provisioning_api/tests/Controller/AppsControllerTest.php
@@ -6,7 +6,7 @@
  * @author Thomas Müller <[email protected]>
  * @author Tom Needham <[email protected]>
  *
- * @copyright Copyright (c) 2018, ownCloud GmbH
+ * @copyright Copyright (c) 2020, ownCloud GmbH
  * @license AGPL-3.0
  *
  * This code is free software: you can redistribute it and/or modify
@@ -25,6 +25,7 @@
 
 namespace OCA\Provisioning_API\Tests\Controller;
 
+use OC\OCS\Result;
 use OCA\Provisioning_API\Controller\AppsController;
 use OCA\Provisioning_API\Tests\TestCase;
 use OCP\API;
@@ -66,13 +67,13 @@ protected function setUp(): void {
 
 	public function testGetAppInfo() {
 		$result = $this->api->getAppInfo('provisioning_api');
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 	}
 
 	public function testGetAppInfoOnBadAppID() {
 		$result = $this->api->getAppInfo('not_provisioning_api');
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(API::RESPOND_NOT_FOUND, $result->getStatusCode());
 	}

diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php
@@ -7,7 +7,7 @@
  * @author Thomas Müller <[email protected]>
  * @author Tom Needham <[email protected]>
  *
- * @copyright Copyright (c) 2018, ownCloud GmbH
+ * @copyright Copyright (c) 2020, ownCloud GmbH
  * @license AGPL-3.0
  *
  * This code is free software: you can redistribute it and/or modify
@@ -26,6 +26,7 @@
 
 namespace OCA\Provisioning_API\Tests\Controller;
 
+use OC\OCS\Result;
 use OCA\Provisioning_API\Controller\GroupsController;
 use OCP\API;
 use OCP\IGroupManager;
@@ -148,6 +149,11 @@ public function dataGetGroups() {
 	 * @param int|null $offset
 	 */
 	public function testGetGroups($search, $limit, $offset) {
+		$this->asUser();
+		$this->subAdminManager
+			->method('isSubAdmin')
+			->willReturn(true);
+
 		$this->request
 			->expects($this->exactly(3))
 			->method('getParam')
@@ -167,16 +173,17 @@ public function testGetGroups($search, $limit, $offset) {
 			->with($search, $limit, $offset)
 			->willReturn($groups);
 
-		$result = $this->api->getGroups([]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$result = $this->api->getGroups();
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 		$this->assertEquals(['group1', 'group2'], $result->getData()['groups']);
 	}
 
 	public function testGetGroupAsUser() {
-		$result = $this->api->getGroup([]);
+		$this->asUser();
+		$result = $this->api->getGroup();
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(API::RESPOND_UNAUTHORISED, $result->getStatusCode());
 	}
@@ -200,11 +207,13 @@ public function testGetGroupAsSubadmin() {
 				$this->createUser('user2')
 			]);
 
-		$result = $this->api->getGroup([
-			'groupid' => 'group',
-		]);
+		$this->subAdminManager
+			->method('isSubAdmin')
+			->willReturn(true);
+
+		$result = $this->api->getGroup('group');
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 		$this->assertCount(1, $result->getData(), 'Asserting the result data array only has the "users" key');
 		$this->assertArrayHasKey('users', $result->getData());
@@ -225,11 +234,9 @@ public function testGetGroupAsIrrelevantSubadmin() {
 			->with('group')
 			->willReturn(true);
 
-		$result = $this->api->getGroup([
-			'groupid' => 'group',
-		]);
+		$result = $this->api->getGroup('group');
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(API::RESPOND_UNAUTHORISED, $result->getStatusCode());
 	}
@@ -253,36 +260,29 @@ public function testGetGroupAsAdmin() {
 				$this->createUser('user2')
 			]);
 
-		$result = $this->api->getGroup([
-			'groupid' => 'group',
-		]);
+		$result = $this->api->getGroup('group');
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 		$this->assertCount(1, $result->getData(), 'Asserting the result data array only has the "users" key');
 		$this->assertArrayHasKey('users', $result->getData());
 		$this->assertEquals(['user1', 'user2'], $result->getData()['users']);
 	}
 
 	public function testGetGroupNonExisting() {
-		$this->asUser();
-
-		$result = $this->api->getGroup([
-			'groupid' => $this->getUniqueID()
-		]);
+		$this->asAdmin();
+		$result = $this->api->getGroup($this->getUniqueID());
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(API::RESPOND_NOT_FOUND, $result->getStatusCode());
 		$this->assertEquals('The requested group could not be found', $result->getMeta()['message']);
 	}
 
 	public function testGetSubAdminsOfGroupsNotExists() {
-		$result = $this->api->getSubAdminsOfGroup([
-			'groupid' => 'NonExistingGroup',
-		]);
+		$result = $this->api->getSubAdminsOfGroup('NonExistingGroup');
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(101, $result->getStatusCode());
 		$this->assertEquals('Group does not exist', $result->getMeta()['message']);
@@ -304,11 +304,9 @@ public function testGetSubAdminsOfGroup() {
 				$this->createUser('SubAdmin2'),
 			]);
 
-		$result = $this->api->getSubAdminsOfGroup([
-			'groupid' => 'GroupWithSubAdmins',
-		]);
+		$result = $this->api->getSubAdminsOfGroup('GroupWithSubAdmins');
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 		$this->assertEquals(['SubAdmin1', 'SubAdmin2'], $result->getData());
 	}
@@ -327,11 +325,9 @@ public function testGetSubAdminsOfGroupEmptyList() {
 			->willReturn([
 			]);
 
-		$result = $this->api->getSubAdminsOfGroup([
-			'groupid' => 'GroupWithOutSubAdmins',
-		]);
+		$result = $this->api->getSubAdminsOfGroup('GroupWithOutSubAdmins');
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 		$this->assertEquals([], $result->getData());
 	}
@@ -342,9 +338,9 @@ public function testAddGroupEmptyGroup() {
 			->with('groupid')
 			->willReturn('');
 
-		$result = $this->api->addGroup([]);
+		$result = $this->api->addGroup();
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(101, $result->getStatusCode());
 		$this->assertEquals('Invalid group name', $result->getMeta()['message']);
@@ -361,9 +357,9 @@ public function testAddGroupExistingGroup() {
 			->with('ExistingGroup')
 			->willReturn(true);
 
-		$result = $this->api->addGroup([]);
+		$result = $this->api->addGroup();
 
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(102, $result->getStatusCode());
 	}
@@ -384,8 +380,8 @@ public function testAddGroupUserDoesNotExist() {
 			->method('getUser')
 			->willReturn(null);
 
-		$result = $this->api->addGroup([]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$result = $this->api->addGroup();
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(102, $result->getStatusCode());
 	}
@@ -414,8 +410,8 @@ public function testAddGroupUserNotAdmin() {
 			->method('getUser')
 			->willReturn($iUser);
 
-		$result = $this->api->addGroup([]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$result = $this->api->addGroup();
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(997, $result->getStatusCode());
 	}
@@ -436,21 +432,20 @@ public function testAddGroup() {
 			->method('createGroup')
 			->with('NewGroup');
 
-		$this->groupManager->expects($this->once())
+		$this->groupManager
 			->method('isAdmin')
 			->willReturn(true);
 
 		$iUser = $this->createMock(IUser::class);
-		$iUser->expects($this->once())
-			->method('getUID')
+		$iUser->method('getUID')
 			->willReturn('user1');
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
 			->willReturn($iUser);
 
-		$result = $this->api->addGroup([]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$result = $this->api->addGroup();
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 	}
 
@@ -470,29 +465,25 @@ public function testAddGroupWithSpecialChar() {
 			->method('createGroup')
 			->with('Iñtërnâtiônàlizætiøn');
 
-		$this->groupManager->expects($this->once())
-			->method('isAdmin')
+		$this->groupManager->method('isAdmin')
 			->willReturn(true);
 
 		$iUser = $this->createMock(IUser::class);
-		$iUser->expects($this->once())
-			->method('getUID')
+		$iUser->method('getUID')
 			->willReturn('user1');
 		$this->userSession
 			->expects($this->once())
 			->method('getUser')
 			->willReturn($iUser);
 
-		$result = $this->api->addGroup([]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$result = $this->api->addGroup();
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 	}
 
 	public function testDeleteGroupNonExisting() {
-		$result = $this->api->deleteGroup([
-			'groupid' => 'NonExistingGroup'
-		]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$result = $this->api->deleteGroup('NonExistingGroup');
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(101, $result->getStatusCode());
 	}
@@ -503,10 +494,8 @@ public function testDeleteAdminGroup() {
 			->with('admin')
 			->willReturn('true');
 
-		$result = $this->api->deleteGroup([
-			'groupid' => 'admin'
-		]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$result = $this->api->deleteGroup('admin');
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(102, $result->getStatusCode());
 	}
@@ -527,10 +516,8 @@ public function testDeleteGroup() {
 			->method('delete')
 			->willReturn(true);
 
-		$result = $this->api->deleteGroup([
-			'groupid' => 'ExistingGroup',
-		]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$result = $this->api->deleteGroup('ExistingGroup');
+		$this->assertInstanceOf(Result::class, $result);
 		$this->assertTrue($result->succeeded());
 		$this->assertEquals(100, $result->getStatusCode());
 	}