Merge pull request #8566 from nextcloud/provisioning_strict

Make the provisioning api app strict

7 files changed, 59 insertions, 58 deletions

diff --git a/apps/provisioning_api/lib/Controller/AppConfigController.php b/apps/provisioning_api/lib/Controller/AppConfigController.php
index 0ca5e4e20a1..38483d2796c 100644
--- a/apps/provisioning_api/lib/Controller/AppConfigController.php
+++ b/apps/provisioning_api/lib/Controller/AppConfigController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016 Joas Schilling <coding@schilljs.com>
  *
@@ -45,7 +46,7 @@ class AppConfigController extends OCSController {
 	 * @param IConfig $config
 	 * @param IAppConfig $appConfig
 	 */
-	public function __construct($appName,
+	public function __construct(string $appName,
 								IRequest $request,
 								IConfig $config,
 								IAppConfig $appConfig) {
@@ -57,7 +58,7 @@ class AppConfigController extends OCSController {
 	/**
 	 * @return DataResponse
 	 */
-	public function getApps() {
+	public function getApps(): DataResponse {
 		return new DataResponse([
 			'data' => $this->appConfig->getApps(),
 		]);
@@ -67,7 +68,7 @@ class AppConfigController extends OCSController {
 	 * @param string $app
 	 * @return DataResponse
 	 */
-	public function getKeys($app) {
+	public function getKeys(string $app): DataResponse {
 		try {
 			$this->verifyAppId($app);
 		} catch (\InvalidArgumentException $e) {
@@ -84,7 +85,7 @@ class AppConfigController extends OCSController {
 	 * @param string $defaultValue
 	 * @return DataResponse
 	 */
-	public function getValue($app, $key, $defaultValue = '') {
+	public function getValue(string $app, string $key, string $defaultValue = ''): DataResponse {
 		try {
 			$this->verifyAppId($app);
 		} catch (\InvalidArgumentException $e) {
@@ -102,7 +103,7 @@ class AppConfigController extends OCSController {
 	 * @param string $value
 	 * @return DataResponse
 	 */
-	public function setValue($app, $key, $value) {
+	public function setValue(string $app, string $key, string $value): DataResponse {
 		try {
 			$this->verifyAppId($app);
 			$this->verifyConfigKey($app, $key);
@@ -120,7 +121,7 @@ class AppConfigController extends OCSController {
 	 * @param string $key
 	 * @return DataResponse
 	 */
-	public function deleteKey($app, $key) {
+	public function deleteKey(string $app, string $key): DataResponse {
 		try {
 			$this->verifyAppId($app);
 			$this->verifyConfigKey($app, $key);
@@ -136,7 +137,7 @@ class AppConfigController extends OCSController {
 	 * @param string $app
 	 * @throws \InvalidArgumentException
 	 */
-	protected function verifyAppId($app) {
+	protected function verifyAppId(string $app) {
 		if (\OC_App::cleanAppId($app) !== $app) {
 			throw new \InvalidArgumentException('Invalid app id given');
 		}
@@ -147,7 +148,7 @@ class AppConfigController extends OCSController {
 	 * @param string $key
 	 * @throws \InvalidArgumentException
 	 */
-	protected function verifyConfigKey($app, $key) {
+	protected function verifyConfigKey(string $app, string $key) {
 		if (in_array($key, ['installed_version', 'enabled', 'types'])) {
 			throw new \InvalidArgumentException('The given key can not be set');
 		}
diff --git a/apps/provisioning_api/lib/Controller/AppsController.php b/apps/provisioning_api/lib/Controller/AppsController.php
index 30062d94a44..3ceb16c1b0d 100644
--- a/apps/provisioning_api/lib/Controller/AppsController.php
+++ b/apps/provisioning_api/lib/Controller/AppsController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -44,7 +45,7 @@ class AppsController extends OCSController {
 	 * @param IAppManager $appManager
 	 */
 	public function __construct(
-		$appName,
+		string $appName,
 		IRequest $request,
 		IAppManager $appManager
 	) {
@@ -58,7 +59,7 @@ class AppsController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function getApps($filter = null) {
+	public function getApps(string $filter = null): DataResponse {
 		$apps = (new OC_App())->listAllApps();
 		$list = [];
 		foreach($apps as $app) {
@@ -88,7 +89,7 @@ class AppsController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function getAppInfo($app) {
+	public function getAppInfo(string $app): DataResponse {
 		$info = \OCP\App::getAppInfo($app);
 		if(!is_null($info)) {
 			return new DataResponse(OC_App::getAppInfo($app));
@@ -103,7 +104,7 @@ class AppsController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function enable($app) {
+	public function enable(string $app): DataResponse {
 		try {
 			$this->appManager->enableApp($app);
 		} catch (AppPathNotFoundException $e) {
@@ -117,7 +118,7 @@ class AppsController extends OCSController {
 	 * @param string $app
 	 * @return DataResponse
 	 */
-	public function disable($app) {
+	public function disable(string $app): DataResponse {
 		$this->appManager->disableApp($app);
 		return new DataResponse();
 	}
diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php
index cb237d9791f..8aed50bf049 100644
--- a/apps/provisioning_api/lib/Controller/GroupsController.php
+++ b/apps/provisioning_api/lib/Controller/GroupsController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -56,7 +57,7 @@ class GroupsController extends OCSController {
 	 * @param ILogger $logger
 	 */
 	public function __construct(
-			$appName,
+			string $appName,
 			IRequest $request,
 			IGroupManager $groupManager,
 			IUserSession $userSession,
@@ -78,7 +79,7 @@ class GroupsController extends OCSController {
 	 * @param int $offset
 	 * @return DataResponse
 	 */
-	public function getGroups($search = '', $limit = null, $offset = null) {
+	public function getGroups(string $search = '', $limit = null, $offset = null): DataResponse {
 		if ($limit !== null) {
 			$limit = (int)$limit;
 		}
@@ -104,7 +105,7 @@ class GroupsController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function getGroup($groupId) {
+	public function getGroup(string $groupId): DataResponse {
 		$user = $this->userSession->getUser();
 
 		// Check the group exists
@@ -142,7 +143,7 @@ class GroupsController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function addGroup($groupid) {
+	public function addGroup(string $groupid): DataResponse {
 		// Validate name
 		if(empty($groupid)) {
 			$this->logger->error('Group name not supplied', ['app' => 'provisioning_api']);
@@ -163,7 +164,7 @@ class GroupsController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function deleteGroup($groupId) {
+	public function deleteGroup(string $groupId): DataResponse {
 		// Check it exists
 		if(!$this->groupManager->groupExists($groupId)){
 			throw new OCSException('', 101);
@@ -180,7 +181,7 @@ class GroupsController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function getSubAdminsOfGroup($groupId) {
+	public function getSubAdminsOfGroup(string $groupId): DataResponse {
 		// Check group exists
 		$targetGroup = $this->groupManager->get($groupId);
 		if($targetGroup === null) {
diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php
index 402381ab58a..60b8393e1e4 100644
--- a/apps/provisioning_api/lib/Controller/UsersController.php
+++ b/apps/provisioning_api/lib/Controller/UsersController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -87,7 +88,7 @@ class UsersController extends OCSController {
 	 * @param NewUserMailHelper $newUserMailHelper
 	 * @param FederatedFileSharingFactory $federatedFileSharingFactory
 	 */
-	public function __construct($appName,
+	public function __construct(string $appName,
 								IRequest $request,
 								IUserManager $userManager,
 								IConfig $config,
@@ -123,7 +124,7 @@ class UsersController extends OCSController {
 	 * @param int $offset
 	 * @return DataResponse
 	 */
-	public function getUsers($search = '', $limit = null, $offset = null) {
+	public function getUsers(string $search = '', $limit = null, $offset = null): DataResponse {
 		$user = $this->userSession->getUser();
 		$users = [];
 
@@ -167,7 +168,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function addUser($userid, $password, $groups = null) {
+	public function addUser(string $userid, string $password, array $groups = []): DataResponse {
 		$user = $this->userSession->getUser();
 		$isAdmin = $this->groupManager->isAdmin($user->getUID());
 		$subAdminManager = $this->groupManager->getSubAdmin();
@@ -177,7 +178,7 @@ class UsersController extends OCSController {
 			throw new OCSException('User already exists', 102);
 		}
 
-		if(is_array($groups)) {
+		if($groups !== []) {
 			foreach ($groups as $group) {
 				if(!$this->groupManager->groupExists($group)) {
 					throw new OCSException('group '.$group.' does not exist', 104);
@@ -196,12 +197,11 @@ class UsersController extends OCSController {
 			$newUser = $this->userManager->createUser($userid, $password);
 			$this->logger->info('Successful addUser call with userid: ' . $userid, ['app' => 'ocs_api']);
 
-			if (is_array($groups)) {
-				foreach ($groups as $group) {
-					$this->groupManager->get($group)->addUser($newUser);
-					$this->logger->info('Added userid ' . $userid . ' to group ' . $group, ['app' => 'ocs_api']);
-				}
+			foreach ($groups as $group) {
+				$this->groupManager->get($group)->addUser($newUser);
+				$this->logger->info('Added userid ' . $userid . ' to group ' . $group, ['app' => 'ocs_api']);
 			}
+
 			return new DataResponse();
 		} catch (HintException $e ) {
 			$this->logger->logException($e, [
@@ -230,7 +230,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function getUser($userId) {
+	public function getUser(string $userId): DataResponse {
 		$data = $this->getUserData($userId);
 		return new DataResponse($data);
 	}
@@ -244,7 +244,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function getCurrentUser() {
+	public function getCurrentUser(): DataResponse {
 		$user = $this->userSession->getUser();
 		if ($user) {
 			$data =  $this->getUserData($user->getUID());
@@ -266,7 +266,7 @@ class UsersController extends OCSController {
 	 * @return array
 	 * @throws OCSException
 	 */
-	protected function getUserData($userId) {
+	protected function getUserData(string $userId): array {
 		$currentLoggedInUser = $this->userSession->getUser();
 
 		$data = [];
@@ -314,7 +314,7 @@ class UsersController extends OCSController {
 	 * @NoAdminRequired
 	 * @NoSubAdminRequired
 	 */
-	public function getEditableFields() {
+	public function getEditableFields(): DataResponse {
 		$permittedFields = [];
 
 		// Editing self (display, email)
@@ -349,9 +349,8 @@ class UsersController extends OCSController {
 	 * @param string $value
 	 * @return DataResponse
 	 * @throws OCSException
-	 * @throws OCSForbiddenException
 	 */
-	public function editUser($userId, $key, $value) {
+	public function editUser(string $userId, string $key, string $value): DataResponse {
 		$currentLoggedInUser = $this->userSession->getUser();
 
 		$targetUser = $this->userManager->get($userId);
@@ -481,9 +480,8 @@ class UsersController extends OCSController {
 	 * @param string $userId
 	 * @return DataResponse
 	 * @throws OCSException
-	 * @throws OCSForbiddenException
 	 */
-	public function deleteUser($userId) {
+	public function deleteUser(string $userId): DataResponse {
 		$currentLoggedInUser = $this->userSession->getUser();
 
 		$targetUser = $this->userManager->get($userId);
@@ -515,7 +513,7 @@ class UsersController extends OCSController {
 	 * @throws OCSException
 	 * @throws OCSForbiddenException
 	 */
-	public function disableUser($userId) {
+	public function disableUser(string $userId): DataResponse {
 		return $this->setEnabled($userId, false);
 	}
 
@@ -528,7 +526,7 @@ class UsersController extends OCSController {
 	 * @throws OCSException
 	 * @throws OCSForbiddenException
 	 */
-	public function enableUser($userId) {
+	public function enableUser(string $userId): DataResponse {
 		return $this->setEnabled($userId, true);
 	}
 
@@ -537,9 +535,8 @@ class UsersController extends OCSController {
 	 * @param bool $value
 	 * @return DataResponse
 	 * @throws OCSException
-	 * @throws OCSForbiddenException
 	 */
-	private function setEnabled($userId, $value) {
+	private function setEnabled(string $userId, bool $value): DataResponse {
 		$currentLoggedInUser = $this->userSession->getUser();
 
 		$targetUser = $this->userManager->get($userId);
@@ -566,7 +563,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function getUsersGroups($userId) {
+	public function getUsersGroups(string $userId): DataResponse {
 		$loggedInUser = $this->userSession->getUser();
 
 		$targetUser = $this->userManager->get($userId);
@@ -612,7 +609,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function addToGroup($userId, $groupid = '') {
+	public function addToGroup(string $userId, string $groupid = ''): DataResponse {
 		if($groupid === '') {
 			throw new OCSException('', 101);
 		}
@@ -647,7 +644,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function removeFromGroup($userId, $groupid) {
+	public function removeFromGroup(string $userId, string $groupid): DataResponse {
 		$loggedInUser = $this->userSession->getUser();
 
 		if($groupid === null || trim($groupid) === '') {
@@ -711,7 +708,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function addSubAdmin($userId, $groupid) {
+	public function addSubAdmin(string $userId, string $groupid): DataResponse {
 		$group = $this->groupManager->get($groupid);
 		$user = $this->userManager->get($userId);
 
@@ -752,7 +749,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function removeSubAdmin($userId, $groupid) {
+	public function removeSubAdmin(string $userId, string $groupid): DataResponse {
 		$group = $this->groupManager->get($groupid);
 		$user = $this->userManager->get($userId);
 		$subAdminManager = $this->groupManager->getSubAdmin();
@@ -785,7 +782,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function getUserSubAdminGroups($userId) {
+	public function getUserSubAdminGroups(string $userId): DataResponse {
 		$user = $this->userManager->get($userId);
 		// Check if the user exists
 		if($user === null) {
@@ -810,7 +807,7 @@ class UsersController extends OCSController {
 	 * @return array
 	 * @throws \OCP\Files\NotFoundException
 	 */
-	protected function fillStorageInfo($userId) {
+	protected function fillStorageInfo(string $userId): array {
 		try {
 			\OC_Util::tearDownFS();
 			\OC_Util::setupFS($userId);
@@ -838,7 +835,7 @@ class UsersController extends OCSController {
 	 * @return DataResponse
 	 * @throws OCSException
 	 */
-	public function resendWelcomeMessage($userId) {
+	public function resendWelcomeMessage(string $userId): DataResponse {
 		$currentLoggedInUser = $this->userSession->getUser();
 
 		$targetUser = $this->userManager->get($userId);
diff --git a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php
index 4c7f7a654c8..c882ec10279 100644
--- a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php
+++ b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  *
  *
@@ -51,8 +52,8 @@ class ProvisioningApiMiddleware extends Middleware {
 	 */
 	public function __construct(
 		IControllerMethodReflector $reflector,
-		$isAdmin,
-		$isSubAdmin) {
+		bool $isAdmin,
+		bool $isSubAdmin) {
 		$this->reflector = $reflector;
 		$this->isAdmin = $isAdmin;
 		$this->isSubAdmin = $isSubAdmin;
diff --git a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php
index 223f2c371d6..2f299b58586 100644
--- a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php
@@ -138,7 +138,7 @@ class AppConfigControllerTest extends TestCase {
 
 	public function dataGetValue() {
 		return [
-			['app1 ', null, null, null, new \InvalidArgumentException('error'), Http::STATUS_FORBIDDEN],
+			['app1', 'key', 'default', null, new \InvalidArgumentException('error'), Http::STATUS_FORBIDDEN],
 			['app2', 'key', 'default', 'return', null, Http::STATUS_OK],
 		];
 	}
@@ -186,8 +186,8 @@ class AppConfigControllerTest extends TestCase {
 
 	public function dataSetValue() {
 		return [
-			['app1 ', null, null, new \InvalidArgumentException('error1'), null, Http::STATUS_FORBIDDEN],
-			['app2', 'key', null, null, new \InvalidArgumentException('error2'), Http::STATUS_FORBIDDEN],
+			['app1', 'key', 'default', new \InvalidArgumentException('error1'), null, Http::STATUS_FORBIDDEN],
+			['app2', 'key', 'default', null, new \InvalidArgumentException('error2'), Http::STATUS_FORBIDDEN],
 			['app2', 'key', 'default', null, null, Http::STATUS_OK],
 		];
 	}
@@ -252,7 +252,7 @@ class AppConfigControllerTest extends TestCase {
 
 	public function dataDeleteValue() {
 		return [
-			['app1 ', null, new \InvalidArgumentException('error1'), null, Http::STATUS_FORBIDDEN],
+			['app1', 'key', new \InvalidArgumentException('error1'), null, Http::STATUS_FORBIDDEN],
 			['app2', 'key', null, new \InvalidArgumentException('error2'), Http::STATUS_FORBIDDEN],
 			['app2', 'key', null, null, Http::STATUS_OK],
 		];
diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
index a8a5e4a0591..216ca76a0f8 100644
--- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@@ -242,7 +242,7 @@ class UsersControllerTest extends TestCase {
 			->with('adminUser')
 			->willReturn(true);
 
-		$this->api->addUser('AlreadyExistingUser', null, null);
+		$this->api->addUser('AlreadyExistingUser', 'password', []);
 	}
 
 	/**
@@ -490,7 +490,7 @@ class UsersControllerTest extends TestCase {
 			->with()
 			->willReturn($subAdminManager);
 
-		$this->api->addUser('NewUser', 'PasswordOfTheNewUser', null);
+		$this->api->addUser('NewUser', 'PasswordOfTheNewUser', []);
 	}
 
 	/**
@@ -2128,7 +2128,7 @@ class UsersControllerTest extends TestCase {
 			->method('getUser')
 			->will($this->returnValue($loggedInUser));
 
-		$this->api->removeFromGroup('TargetUser', null);
+		$this->api->removeFromGroup('TargetUser', '');
 	}
 
 	/**
@@ -2450,7 +2450,7 @@ class UsersControllerTest extends TestCase {
 			->with('NotExistingUser')
 			->will($this->returnValue(null));
 
-		$this->api->addSubAdmin('NotExistingUser', null);
+		$this->api->addSubAdmin('NotExistingUser', '');
 	}
 
 	/**