Make UserController strict

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

1 files changed, 48 insertions, 72 deletions

diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php
index d311b9dcb80..df89c2ec1a7 100644
--- a/settings/Controller/UsersController.php
+++ b/settings/Controller/UsersController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -39,6 +40,7 @@ namespace OC\Settings\Controller;
 use OC\Accounts\AccountManager;
 use OC\AppFramework\Http;
 use OC\ForbiddenException;
+use OC\Group\Manager as GroupManager;
 use OC\HintException;
 use OC\Settings\Mailer\NewUserMailHelper;
 use OC\Security\IdentityProof\Manager;
@@ -50,7 +52,6 @@ use OCP\Files\Config\IUserMountCache;
 use OCP\Encryption\IEncryptionModule;
 use OCP\Encryption\IManager;
 use OCP\IConfig;
-use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
@@ -76,7 +77,7 @@ class UsersController extends Controller {
 	private $isAdmin;
 	/** @var IUserManager */
 	private $userManager;
-	/** @var IGroupManager */
+	/** @var GroupManager */
 	private $groupManager;
 	/** @var IConfig */
 	private $config;
@@ -109,36 +110,13 @@ class UsersController extends Controller {
 	/** @var IManager */
 	private $encryptionManager;
 
-
-	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param IUserManager $userManager
-	 * @param IGroupManager $groupManager
-	 * @param IUserSession $userSession
-	 * @param IConfig $config
-	 * @param bool $isAdmin
-	 * @param IL10N $l10n
-	 * @param ILogger $log
-	 * @param IMailer $mailer
-	 * @param IURLGenerator $urlGenerator
-	 * @param IAppManager $appManager
-	 * @param IAvatarManager $avatarManager
-	 * @param AccountManager $accountManager
-	 * @param ISecureRandom $secureRandom
-	 * @param NewUserMailHelper $newUserMailHelper
-	 * @param Manager $keyManager
-	 * @param IJobList $jobList
-	 * @param IUserMountCache $userMountCache
-	 * @param IManager $encryptionManager
-	 */
-	public function __construct($appName,
+	public function __construct(string $appName,
 								IRequest $request,
 								IUserManager $userManager,
-								IGroupManager $groupManager,
+								GroupManager $groupManager,
 								IUserSession $userSession,
 								IConfig $config,
-								$isAdmin,
+								bool $isAdmin,
 								IL10N $l10n,
 								ILogger $log,
 								IMailer $mailer,
@@ -185,7 +163,7 @@ class UsersController extends Controller {
 	 * @param array|null $userGroups
 	 * @return array
 	 */
-	private function formatUserForIndex(IUser $user, array $userGroups = null) {
+	private function formatUserForIndex(IUser $user, array $userGroups = null): array {
 
 		// TODO: eliminate this encryption specific code below and somehow
 		// hook in additional user info from other apps
@@ -261,7 +239,7 @@ class UsersController extends Controller {
 	 * @param array $userIDs Array with schema [$uid => $displayName]
 	 * @return IUser[]
 	 */
-	private function getUsersForUID(array $userIDs) {
+	private function getUsersForUID(array $userIDs): array {
 		$users = [];
 		foreach ($userIDs as $uid => $displayName) {
 			$users[$uid] = $this->userManager->get($uid);
@@ -281,7 +259,7 @@ class UsersController extends Controller {
 	 *
 	 * TODO: Tidy up and write unit tests - code is mainly static method calls
 	 */
-	public function index($offset = 0, $limit = 10, $gid = '', $pattern = '', $backend = '') {
+	public function index(int $offset = 0, int $limit = 10, string $gid = '', string $pattern = '', string $backend = ''): DataResponse {
 		// Remove backends
 		if (!empty($backend)) {
 			$activeBackends = $this->userManager->getBackends();
@@ -375,11 +353,11 @@ class UsersController extends Controller {
 	 * @param string $email
 	 * @return DataResponse
 	 */
-	public function create($username, $password, array $groups = [], $email = '') {
+	public function create(string $username, string $password, array $groups = [], $email = ''): DataResponse {
 		if ($email !== '' && !$this->mailer->validateMailAddress($email)) {
 			return new DataResponse(
 				[
-					'message' => (string)$this->l10n->t('Invalid mail address')
+					'message' => $this->l10n->t('Invalid mail address')
 				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
@@ -396,7 +374,7 @@ class UsersController extends Controller {
 						continue;
 					}
 
-					if (!$this->groupManager->getSubAdmin()->isSubAdminofGroup($currentUser, $groupObject)) {
+					if (!$this->groupManager->getSubAdmin()->isSubAdminOfGroup($currentUser, $groupObject)) {
 						unset($groups[$key]);
 					}
 				}
@@ -415,7 +393,7 @@ class UsersController extends Controller {
 		if ($this->userManager->userExists($username)) {
 			return new DataResponse(
 				[
-					'message' => (string)$this->l10n->t('A user with that name already exists.')
+					'message' => $this->l10n->t('A user with that name already exists.')
 				],
 				Http::STATUS_CONFLICT
 			);
@@ -426,7 +404,7 @@ class UsersController extends Controller {
 			if ($email === '') {
 				return new DataResponse(
 					[
-						'message' => (string)$this->l10n->t('To send a password link to the user an email address is required.')
+						'message' => $this->l10n->t('To send a password link to the user an email address is required.')
 					],
 					Http::STATUS_UNPROCESSABLE_ENTITY
 				);
@@ -494,7 +472,7 @@ class UsersController extends Controller {
 
 		return new DataResponse(
 			[
-				'message' => (string)$this->l10n->t('Unable to create user.')
+				'message' => $this->l10n->t('Unable to create user.')
 			],
 			Http::STATUS_FORBIDDEN
 		);
@@ -508,7 +486,7 @@ class UsersController extends Controller {
 	 * @param string $id
 	 * @return DataResponse
 	 */
-	public function destroy($id) {
+	public function destroy(string $id): DataResponse {
 		$userId = $this->userSession->getUser()->getUID();
 		$user = $this->userManager->get($id);
 
@@ -517,7 +495,7 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string)$this->l10n->t('Unable to delete user.')
+						'message' => $this->l10n->t('Unable to delete user.')
 					]
 				],
 				Http::STATUS_FORBIDDEN
@@ -529,32 +507,30 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string)$this->l10n->t('Authentication error')
+						'message' => $this->l10n->t('Authentication error')
 					]
 				],
 				Http::STATUS_FORBIDDEN
 			);
 		}
 
-		if ($user) {
-			if ($user->delete()) {
-				return new DataResponse(
-					[
-						'status' => 'success',
-						'data' => [
-							'username' => $id
-						]
-					],
-					Http::STATUS_NO_CONTENT
-				);
-			}
+		if ($user && $user->delete()) {
+			return new DataResponse(
+				[
+					'status' => 'success',
+					'data' => [
+						'username' => $id
+					]
+				],
+				Http::STATUS_NO_CONTENT
+			);
 		}
 
 		return new DataResponse(
 			[
 				'status' => 'error',
 				'data' => [
-					'message' => (string)$this->l10n->t('Unable to delete user.')
+					'message' => $this->l10n->t('Unable to delete user.')
 				]
 			],
 			Http::STATUS_FORBIDDEN
@@ -568,12 +544,12 @@ class UsersController extends Controller {
 	 * @param int $enabled
 	 * @return DataResponse
 	 */
-	public function setEnabled($id, $enabled) {
+	public function setEnabled(string $id, int $enabled): DataResponse {
 		$enabled = (bool)$enabled;
 		if ($enabled) {
-			$errorMsgGeneral = (string)$this->l10n->t('Error while enabling user.');
+			$errorMsgGeneral = $this->l10n->t('Error while enabling user.');
 		} else {
-			$errorMsgGeneral = (string)$this->l10n->t('Error while disabling user.');
+			$errorMsgGeneral = $this->l10n->t('Error while disabling user.');
 		}
 
 		$userId = $this->userSession->getUser()->getUID();
@@ -596,7 +572,7 @@ class UsersController extends Controller {
 					[
 						'status' => 'error',
 						'data' => [
-							'message' => (string)$this->l10n->t('Authentication error')
+							'message' => $this->l10n->t('Authentication error')
 						]
 					],
 					Http::STATUS_FORBIDDEN
@@ -638,7 +614,7 @@ class UsersController extends Controller {
 	 * @param bool $onlyVerificationCode only return verification code without updating the data
 	 * @return DataResponse
 	 */
-	public function getVerificationCode($account, $onlyVerificationCode) {
+	public function getVerificationCode(string $account, bool $onlyVerificationCode): DataResponse {
 
 		$user = $this->userSession->getUser();
 
@@ -648,7 +624,7 @@ class UsersController extends Controller {
 
 		$accountData = $this->accountManager->getUser($user);
 		$cloudId = $user->getCloudId();
-		$message = "Use my Federated Cloud ID to share with me: " . $cloudId;
+		$message = 'Use my Federated Cloud ID to share with me: ' . $cloudId;
 		$signature = $this->signMessage($user, $message);
 
 		$code = $message . ' ' . $signature;
@@ -697,7 +673,7 @@ class UsersController extends Controller {
 	 *
 	 * @return int
 	 */
-	protected function getCurrentTime() {
+	protected function getCurrentTime(): int {
 		return time();
 	}
 
@@ -709,7 +685,7 @@ class UsersController extends Controller {
 	 *
 	 * @return string base64 encoded signature
 	 */
-	protected function signMessage(IUser $user, $message) {
+	protected function signMessage(IUser $user, string $message): string {
 		$privateKey = $this->keyManager->getKey($user)->getPrivate();
 		openssl_sign(json_encode($message), $signature, $privateKey, OPENSSL_ALGO_SHA512);
 		return base64_encode($signature);
@@ -755,7 +731,7 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string)$this->l10n->t('Invalid mail address')
+						'message' => $this->l10n->t('Invalid mail address')
 					]
 				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
@@ -799,7 +775,7 @@ class UsersController extends Controller {
 						'websiteScope' => $data[AccountManager::PROPERTY_WEBSITE]['scope'],
 						'address' => $data[AccountManager::PROPERTY_ADDRESS]['value'],
 						'addressScope' => $data[AccountManager::PROPERTY_ADDRESS]['scope'],
-						'message' => (string)$this->l10n->t('Settings saved')
+						'message' => $this->l10n->t('Settings saved')
 					]
 				],
 				Http::STATUS_OK
@@ -823,7 +799,7 @@ class UsersController extends Controller {
 	 * @param array $data
 	 * @throws ForbiddenException
 	 */
-	protected function saveUserSettings(IUser $user, $data) {
+	protected function saveUserSettings(IUser $user, array $data) {
 
 		// keep the user back-end up-to-date with the latest display name and email
 		// address
@@ -861,7 +837,7 @@ class UsersController extends Controller {
 	 *
 	 * @return DataResponse
 	 */
-	public function stats() {
+	public function stats(): DataResponse {
 		$userCount = 0;
 		if ($this->isAdmin) {
 			$countByBackend = $this->userManager->countUsers();
@@ -904,7 +880,7 @@ class UsersController extends Controller {
 	 * @param string $displayName
 	 * @return DataResponse
 	 */
-	public function setDisplayName($username, $displayName) {
+	public function setDisplayName(string $username, string $displayName) {
 		$currentUser = $this->userSession->getUser();
 		$user = $this->userManager->get($username);
 
@@ -961,7 +937,7 @@ class UsersController extends Controller {
 	 * @param string $mailAddress
 	 * @return DataResponse
 	 */
-	public function setEMailAddress($id, $mailAddress) {
+	public function setEMailAddress(string $id, string $mailAddress) {
 		$user = $this->userManager->get($id);
 		if (!$this->isAdmin
 			&& !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)
@@ -970,7 +946,7 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string)$this->l10n->t('Forbidden')
+						'message' => $this->l10n->t('Forbidden')
 					]
 				],
 				Http::STATUS_FORBIDDEN
@@ -982,7 +958,7 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string)$this->l10n->t('Invalid mail address')
+						'message' => $this->l10n->t('Invalid mail address')
 					]
 				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
@@ -994,7 +970,7 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string)$this->l10n->t('Invalid user')
+						'message' => $this->l10n->t('Invalid user')
 					]
 				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
@@ -1007,7 +983,7 @@ class UsersController extends Controller {
 				[
 					'status' => 'error',
 					'data' => [
-						'message' => (string)$this->l10n->t('Unable to change mail address')
+						'message' => $this->l10n->t('Unable to change mail address')
 					]
 				],
 				Http::STATUS_FORBIDDEN
@@ -1025,7 +1001,7 @@ class UsersController extends Controller {
 					'data' => [
 						'username' => $id,
 						'mailAddress' => $mailAddress,
-						'message' => (string)$this->l10n->t('Email saved')
+						'message' => $this->l10n->t('Email saved')
 					]
 				],
 				Http::STATUS_OK