refactor: Setup and run rector

REUSE.toml

@@ -12,7 +12,7 @@ SPDX-FileCopyrightText = "2017 Nextcloud GmbH and Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"
 
 [[annotations]]
-path = ["vendor-bin/csfixer/composer.json", "vendor-bin/csfixer/composer.lock", "vendor-bin/mozart/composer.json", "vendor-bin/mozart/composer.lock", "vendor-bin/phpunit/composer.json", "vendor-bin/phpunit/composer.lock", "vendor-bin/psalm/composer.json", "vendor-bin/psalm/composer.lock", "composer.lock"]
+path = ["vendor-bin/csfixer/composer.json", "vendor-bin/csfixer/composer.lock", "vendor-bin/mozart/composer.json", "vendor-bin/mozart/composer.lock", "vendor-bin/phpunit/composer.json", "vendor-bin/phpunit/composer.lock", "vendor-bin/psalm/composer.json", "vendor-bin/psalm/composer.lock", "composer.lock", "vendor-bin/rector/composer.json", "vendor-bin/rector/composer.lock"]
 precedence = "aggregate"
 SPDX-FileCopyrightText = "2023 Nextcloud GmbH and Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"

composer.json

@@ -27,13 +27,13 @@
         }
     },
     "scripts": {
-        "cs:fix": "./vendor-bin/csfixer/vendor/bin/php-cs-fixer fix",
-        "cs:check": "./vendor-bin/csfixer/vendor/bin/php-cs-fixer fix --dry-run --diff",
+        "cs:fix": "php-cs-fixer fix",
+        "cs:check": "php-cs-fixer fix --dry-run --diff",
         "lint": "find . -name \\*.php -not -path './vendor/*' -not -path './tests/stubs/*' -print0 | xargs -0 -n1 php -l",
-        "psalm": "./vendor-bin/psalm/vendor/bin/psalm --threads=1",
-        "psalm:clear": "./vendor-bin/psalm/vendor/bin/psalm --clear-cache && ./vendor-bin/psalm/vendor/bin/psalm --clear-global-cache",
-        "psalm:fix": "./vendor-bin/psalm/vendor/bin/psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
-        "test:unit": "./vendor-bin/phpunit/vendor/bin/phpunit -c tests/phpunit.xml --color --fail-on-warning --fail-on-risky",
+        "psalm": "psalm --threads=1",
+        "psalm:clear": "psalm --clear-cache && psalm --clear-global-cache",
+        "psalm:fix": ".psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
+        "test:unit": "phpunit -c tests/phpunit.xml --color --fail-on-warning --fail-on-risky",
         "rector": "rector && composer cs:fix",
         "post-install-cmd": [
             "@composer bin all install --ansi",
@@ -50,7 +50,7 @@
     },
     "extra": {
         "bamarni-bin": {
-            "bin-links": false,
+            "bin-links": true,
             "target-directory": "vendor-bin",
             "forward-command": true
         },

lib/AppInfo/Application.php

@@ -9,7 +9,6 @@
 
 namespace OCA\GlobalSiteSelector\AppInfo;
 
-use Closure;
 use Exception;
 use OC;
 use OCA\GlobalSiteSelector\GlobalSiteSelector;
@@ -24,13 +23,12 @@
 use OCA\GlobalSiteSelector\SetupChecks\LongJwtKeySetupCheck;
 use OCA\GlobalSiteSelector\Slave;
 use OCA\GlobalSiteSelector\UserBackend;
+use OCP\Accounts\UserUpdatedEvent;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IBootContext;
 use OCP\AppFramework\Bootstrap\IBootstrap;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
-use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IRequest;
-use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\Security\CSP\AddContentSecurityPolicyEvent;
@@ -44,7 +42,6 @@
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\NotFoundExceptionInterface;
 use Psr\Log\LoggerInterface;
-use Symfony\Component\EventDispatcher\GenericEvent;
 use Throwable;
 
 /**
@@ -63,9 +60,6 @@ public function __construct(array $urlParams = []) {
 		parent::__construct(self::APP_ID, $urlParams);
 	}
 
-	/**
-	 * @param IRegistrationContext $context
-	 */
 	#[\Override]
 	public function register(IRegistrationContext $context): void {
 		$context->registerCapability(PublicCapabilities::class);
@@ -83,41 +77,27 @@ public function register(IRegistrationContext $context): void {
 		$context->registerEventListener(UserDeletedEvent::class, UserDeleted::class);
 		$context->registerEventListener(UserLoggedOutEvent::class, UserLoggedOut::class);
 		$context->registerEventListener(UserChangedEvent::class, UserChanged::class);
+		$context->registerEventListener(UserUpdatedEvent::class, UserChanged::class);
 
 		$context->registerSetupCheck(LongJwtKeySetupCheck::class);
-
-		// It seems that AccountManager use deprecated dispatcher, let's use a deprecated listener
-		/** @var IEventDispatcher $eventDispatcher */
-		$dispatcher = Server::get(IEventDispatcher::class);
-		$dispatcher->addListener(
-			'OC\AccountManager::userUpdated',
-			function (GenericEvent $event) {
-				/** @var IUser $user */
-				$user = $event->getSubject();
-				$slave = OC::$server->get(Slave::class);
-				$slave->updateUser($user);
-			}
-		);
 	}
 
 	/**
-	 * @param IBootContext $context
-	 *
 	 * @throws Throwable
 	 */
 	#[\Override]
 	public function boot(IBootContext $context): void {
 		$this->globalSiteSelector = $context->getAppContainer()->get(GlobalSiteSelector::class);
 		$this->logger = $context->getServerContainer()->get(LoggerInterface::class);
 
-		$context->injectFn(Closure::fromCallable([$this, 'registerUserBackendForSlave']));
-		$context->injectFn(Closure::fromCallable([$this, 'redirectToMasterLogin']));
+		$context->injectFn(\Closure::fromCallable($this->registerUserBackendForSlave(...)));
+		$context->injectFn(\Closure::fromCallable($this->redirectToMasterLogin(...)));
 	}
 
 	/**
 	 * Register the Global Scale User Backend if we run in slave mode
 	 */
-	private function registerUserBackendForSlave() {
+	private function registerUserBackendForSlave(): void {
 		if (!$this->globalSiteSelector->isSlave()) {
 			return;
 		}
@@ -144,7 +124,7 @@ private function registerUserBackendForSlave() {
 	/**
 	 * Register the Global Scale User Backend if we run in slave mode
 	 */
-	private function redirectToMasterLogin() {
+	private function redirectToMasterLogin(): void {
 		if (OC::$CLI) {
 			return;
 		}

lib/Command/UsersUpdate.php

@@ -15,12 +15,10 @@
 use Symfony\Component\Console\Output\OutputInterface;
 
 class UsersUpdate extends Base {
-	private Slave $slave;
-
-	public function __construct(Slave $slave) {
+	public function __construct(
+		private readonly Slave $slave,
+	) {
 		parent::__construct();
-
-		$this->slave = $slave;
 	}
 
 	/**
@@ -33,12 +31,6 @@ protected function configure() {
 			->setDescription('update known users data to Lookup Server');
 	}
 
-	/**
-	 * @param InputInterface $input
-	 * @param OutputInterface $output
-	 *
-	 * @return int
-	 */
 	#[\Override]
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$this->slave->batchUpdate();

lib/Controller/MasterController.php

@@ -12,10 +12,12 @@
 use OCA\GlobalSiteSelector\Master;
 use OCA\GlobalSiteSelector\Vendor\Firebase\JWT\JWT;
 use OCA\GlobalSiteSelector\Vendor\Firebase\JWT\Key;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
+use OCP\AppFramework\Http\Attribute\PublicPage;
+use OCP\AppFramework\Http\Attribute\UseSession;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\OCSController;
 use OCP\IRequest;
-use OCP\ISession;
 use OCP\IURLGenerator;
 use Psr\Log\LoggerInterface;
 
@@ -27,40 +29,20 @@
  * @package OCA\GlobalSiteSelector\Controller
  */
 class MasterController extends OCSController {
-	private IURLGenerator $urlGenerator;
-	private ISession $session;
-	private GlobalSiteSelector $gss;
-	private Master $master;
-	private LoggerInterface $logger;
-
 	public function __construct(
 		$appName,
 		IRequest $request,
-		IURLGenerator $urlGenerator,
-		ISession $session,
-		GlobalSiteSelector $globalSiteSelector,
-		Master $master,
-		LoggerInterface $logger,
+		private readonly IURLGenerator $urlGenerator,
+		private readonly GlobalSiteSelector $gss,
+		private readonly LoggerInterface $logger,
 	) {
 		parent::__construct($appName, $request);
-
-		$this->urlGenerator = $urlGenerator;
-		$this->session = $session;
-		$this->gss = $globalSiteSelector;
-		$this->master = $master;
-		$this->logger = $logger;
 	}
 
-	/**
-	 * @PublicPage
-	 * @NoCSRFRequired
-	 * @UseSession
-	 *
-	 * @param string|null $jwt
-	 *
-	 * @return RedirectResponse
-	 */
-	public function autoLogout(?string $jwt) {
+	#[PublicPage]
+	#[NoCSRFRequired]
+	#[UseSession]
+	public function autoLogout(?string $jwt): RedirectResponse {
 		try {
 			if ($jwt !== null) {
 				$key = $this->gss->getJwtKey();

lib/Controller/SlaveController.php

@@ -189,12 +189,12 @@ public function autoLogin(string $jwt): RedirectResponse {
 			if ($result === false) {
 				throw new \Exception('wrong username or password given for: ' . $uid);
 			}
-		} catch (ExpiredException $e) {
+		} catch (ExpiredException) {
 			$this->logger->info('token expired');
 			$response = new RedirectResponse($masterUrl);
 			$response->throttle();
 			return $response;
-		} catch (DisabledUserException $e) {
+		} catch (DisabledUserException) {
 			// user is disabled, remove from lookup server
 			$params = ['uid' => $uid];
 			$this->slave->preDeleteUser($params);
@@ -208,6 +208,7 @@ public function autoLogin(string $jwt): RedirectResponse {
 		}
 
 		$this->logger->debug('all good. creating session');
+		/** @psalm-suppress UndefinedInterfaceMethod defined in the private implementation */
 		$this->userSession->createSessionToken($this->request, $uid, $uid, null, IToken::REMEMBER);
 
 		// ignore the need of password validation on slaves
@@ -220,7 +221,7 @@ public function autoLogin(string $jwt): RedirectResponse {
 		$this->slaveService->updateUserById($uid);
 		$this->logger->debug('userdata updated on lus');
 
-		if (str_starts_with($target, 'http://') || str_starts_with($target, 'https://')) {
+		if (str_starts_with((string)$target, 'http://') || str_starts_with((string)$target, 'https://')) {
 			$home = $target;
 		} else {
 			$home = $this->urlGenerator->getAbsoluteURL($target);
@@ -260,7 +261,7 @@ public function createAppToken($jwt): DataResponse {
 					return new DataResponse($token);
 				}
 			}
-		} catch (ExpiredException $e) {
+		} catch (ExpiredException) {
 			$this->logger->info('Create app password: JWT token expired');
 		} catch (\Exception $e) {
 			$this->logger->info('issue while token creation', ['exception' => $e]);
@@ -274,12 +275,10 @@ public function createAppToken($jwt): DataResponse {
 	/**
 	 * decode jwt and return the uid and the password
 	 *
-	 * @param string $jwt
 	 *
-	 * @return array
 	 * @throws \Exception
 	 */
-	protected function decodeJwt($jwt) {
+	protected function decodeJwt(string $jwt): array {
 		$key = $this->gss->getJwtKey();
 		$decoded = (array)JWT::decode($jwt, new Key($key, Application::JWT_ALGORITHM));
 
@@ -299,14 +298,11 @@ protected function decodeJwt($jwt) {
 	}
 
 	/**
-	 * create new user if the user doesn't exist yet on the client node
-	 *
-	 * @param string $uid
-	 * @param array $options
+	 * Create new user if the user doesn't exist yet on the client node
 	 */
-	protected function autoprovisionIfNeeded($uid, $options) {
+	protected function autoprovisionIfNeeded(string $uid, array $options) {
 		// make sure that a valid UID is given
-		if (empty($uid)) {
+		if ($uid === '') {
 			$this->logger->error('Uid "{uid}" is not valid.', ['app' => $this->appName, 'uid' => $uid]);
 			throw new \InvalidArgumentException('No valid uid given. Given uid: ' . $uid);
 		}

lib/Db/FileRequest.php

@@ -191,10 +191,11 @@ public function getTeamStorages(FederatedShare $federatedShare, string $instance
 	 */
 	private function getCachedMountInfoFromNodeId(int $nodeId): ?ICachedMountFileInfo {
 		$mounts = $this->userMountCache->getMountsForFileId($nodeId);
-		if (empty($mounts ?? [])) {
+		if ($mounts === []) {
 			$this->logger->warning('mount not found for node id ' . $nodeId);
+			return null;
 		}
 
-		return reset($mounts);
+		return current($mounts);
 	}
 }

lib/Db/ShareRequest.php

@@ -57,8 +57,8 @@ public function getFederatedSharesRelatedToRemoteInstance(array $files, string $
 		$shares = [];
 		while ($row = $result->fetch()) {
 			$shareWith = $row['share_with'];
-			if (str_ends_with(strtolower($shareWith), '@' . strtolower($instance))) {
-				$shareWith = substr($shareWith, 0, -strlen('@' . $instance));
+			if (str_ends_with(strtolower((string)$shareWith), '@' . strtolower($instance))) {
+				$shareWith = substr((string)$shareWith, 0, -strlen('@' . $instance));
 			}
 
 			$federatedShare = new FederatedShare();
@@ -78,7 +78,7 @@ public function getFederatedSharesRelatedToRemoteInstance(array $files, string $
 	/**
 	 * return id and owner about a file.
 	 *
-	 * @return array{int, string} [fileId, fileOwner]
+	 * @return array{0?: int, 1?: string} [fileId, fileOwner]
 	 */
 	public function getFileOwnerFromShareId(int $shareId): array {
 		$qb = $this->connection->getQueryBuilder();
@@ -92,7 +92,7 @@ public function getFileOwnerFromShareId(int $shareId): array {
 			return [];
 		}
 		$fileId = (int)$row['file_source'];
-		$owner = $row['uid_owner'];
+		$owner = (string)$row['uid_owner'];
 		$result->closeCursor();
 
 		return [$fileId, $owner];

lib/Exceptions/LocalFederatedShareException.php

@@ -22,7 +22,7 @@ public function __construct(
 		parent::__construct($message, $code, $previous);
 	}
 
-	public function getFederatedShare(): FederatedShare {
+	public function getFederatedShare(): ?FederatedShare {
 		return $this->federatedShare;
 	}
 }