refactor: Apply code best practices PHP8+

Signed-off-by: Git'Fellow <[email protected]>

lib/Command/ConfigCreate.php

@@ -13,11 +13,11 @@
 use Symfony\Component\Console\Output\OutputInterface;
 
 class ConfigCreate extends Base {
-	private SAMLSettings $samlSettings;
 
-	public function __construct(SAMLSettings $samlSettings) {
+	public function __construct(
+		private SAMLSettings $samlSettings,
+	) {
 		parent::__construct();
-		$this->samlSettings = $samlSettings;
 	}
 
 	protected function configure(): void {

lib/Command/ConfigDelete.php

@@ -15,11 +15,11 @@
 use Symfony\Component\Console\Output\OutputInterface;
 
 class ConfigDelete extends Base {
-	private SAMLSettings $samlSettings;
 
-	public function __construct(SAMLSettings $samlSettings) {
+	public function __construct(
+		private SAMLSettings $samlSettings,
+	) {
 		parent::__construct();
-		$this->samlSettings = $samlSettings;
 	}
 
 	protected function configure(): void {

lib/Command/ConfigGet.php

@@ -14,11 +14,11 @@
 use Symfony\Component\Console\Output\OutputInterface;
 
 class ConfigGet extends Base {
-	private SAMLSettings $samlSettings;
 
-	public function __construct(SAMLSettings $samlSettings) {
+	public function __construct(
+		private SAMLSettings $samlSettings,
+	) {
 		parent::__construct();
-		$this->samlSettings = $samlSettings;
 	}
 
 	protected function configure(): void {

lib/Command/ConfigSet.php

@@ -16,11 +16,11 @@
 use Symfony\Component\Console\Output\OutputInterface;
 
 class ConfigSet extends Base {
-	private SAMLSettings $samlSettings;
 
-	public function __construct(SAMLSettings $samlSettings) {
+	public function __construct(
+		private SAMLSettings $samlSettings,
+	) {
 		parent::__construct();
-		$this->samlSettings = $samlSettings;
 	}
 
 	protected function configure(): void {

lib/Command/GetMetadata.php

@@ -17,13 +17,10 @@
 class GetMetadata extends Command {
 	use TXmlHelper;
 
-	private SAMLSettings $SAMLSettings;
-
 	public function __construct(
-		SAMLSettings $SAMLSettings,
+		private SAMLSettings $samlSettings,
 	) {
 		parent::__construct();
-		$this->SAMLSettings = $SAMLSettings;
 	}
 
 	protected function configure(): void {
@@ -49,7 +46,11 @@ protected function configure(): void {
 
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$idp = (int)$input->getArgument('idp');
-		$settings = new Settings($this->SAMLSettings->getOneLoginSettingsArray($idp));
+		$settingsArray = $this->samlSettings->getOneLoginSettingsArray($idp);
+		if ($settingsArray === null) {
+			throw new \InvalidArgumentException('Settings cannot be null');
+		}
+		$settings = new Settings($settingsArray);
 		$metadata = $settings->getSPMetadata();
 		$errors = $this->callWithXmlEntityLoader(function () use ($settings, $metadata) {
 			return $settings->validateMetadata($metadata);

lib/Controller/SettingsController.php

@@ -17,24 +17,14 @@
 
 class SettingsController extends Controller {
 
-	/** @var IConfig */
-	private $config;
-	/** @var Admin */
-	private $admin;
-	/** @var SAMLSettings */
-	private $samlSettings;
-
 	public function __construct(
 		$appName,
 		IRequest $request,
-		IConfig $config,
-		Admin $admin,
-		SAMLSettings $samlSettings,
+		private IConfig $config,
+		private Admin $admin,
+		private SAMLSettings $samlSettings,
 	) {
 		parent::__construct($appName, $request);
-		$this->config = $config;
-		$this->admin = $admin;
-		$this->samlSettings = $samlSettings;
 	}
 
 	public function getSamlProviderIds(): DataResponse {

lib/Controller/TimezoneController.php

@@ -16,22 +16,14 @@
 
 class TimezoneController extends Controller {
 
-	/** @var IConfig */
-	private $config;
-	/** @var string */
-	private $userId;
-	/** @var ISession */
-	private $session;
-
-	public function __construct($appName,
+	public function __construct(
+		$appName,
 		IRequest $request,
-		IConfig $config,
-		$userId,
-		ISession $session) {
+		private IConfig $config,
+		private $userId,
+		private ISession $session,
+	) {
 		parent::__construct($appName, $request);
-		$this->config = $config;
-		$this->userId = $userId;
-		$this->session = $session;
 	}
 
 	/**

lib/DavPlugin.php

@@ -15,22 +15,17 @@
 use Sabre\HTTP\ResponseInterface;
 
 class DavPlugin extends ServerPlugin {
-	private $session;
-	private $config;
-	private $auth;
 	/** @var Server */
 	private $server;
-	/** @var SAMLSettings */
-	private $samlSettings;
 
-	public function __construct(ISession $session, IConfig $config, array $auth, SAMLSettings $samlSettings) {
-		$this->session = $session;
-		$this->config = $config;
-		$this->auth = $auth;
-		$this->samlSettings = $samlSettings;
+	public function __construct(
+		private ISession $session,
+		private IConfig $config,
+		private array $auth,
+		private SAMLSettings $samlSettings,
+	) {
 	}
 
-
 	public function initialize(Server $server) {
 		// before auth
 		$server->on('beforeMethod:*', [$this, 'beforeMethod'], 9);

lib/GroupManager.php

@@ -20,46 +20,22 @@
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IUser;
+use OCP\Server;
+use Psr\Log\LoggerInterface;
 
 class GroupManager {
 	public const LOCAL_GROUPS_CHECK_FOR_MIGRATION = 'localGroupsCheckForMigration';
 	public const STATE_MIGRATION_PHASE_EXPIRED = 'EXPIRED';
 
-	/**
-	 * @var IDBConnection $db
-	 */
-	protected $db;
-
-	/** @var IGroupManager */
-	private $groupManager;
-	/** @var GroupBackend */
-	private $ownGroupBackend;
-	/** @var IConfig */
-	private $config;
-	/** @var IEventDispatcher */
-	private $dispatcher;
-	/** @var IJobList */
-	private $jobList;
-	/** @var SAMLSettings */
-	private $settings;
-
-
 	public function __construct(
-		IDBConnection $db,
-		IGroupManager $groupManager,
-		GroupBackend $ownGroupBackend,
-		IConfig $config,
-		IEventDispatcher $dispatcher,
-		IJobList $jobList,
-		SAMLSettings $settings,
+		protected IDBConnection $db,
+		private IGroupManager $groupManager,
+		private GroupBackend $ownGroupBackend,
+		private IConfig $config,
+		private IEventDispatcher $dispatcher,
+		private IJobList $jobList,
+		private SAMLSettings $settings,
 	) {
-		$this->db = $db;
-		$this->groupManager = $groupManager;
-		$this->ownGroupBackend = $ownGroupBackend;
-		$this->config = $config;
-		$this->dispatcher = $dispatcher;
-		$this->jobList = $jobList;
-		$this->settings = $settings;
 	}
 
 	/**
@@ -70,7 +46,7 @@ public function __construct(
 	private function getGroupsToRemove(array $samlGroupNames, array $assignedGroups): array {
 		$groupsToRemove = [];
 		foreach ($assignedGroups as $group) {
-			\OCP\Log\logger('user_saml')->debug('Checking group {group} for removal', ['group' => $group->getGID()]);
+			Server::get(LoggerInterface::class)->debug('Checking group {group} for removal', ['app' => 'user_saml', 'group' => $group->getGID()]);
 			if (in_array($group->getGID(), $samlGroupNames, true)) {
 				continue;
 			}
@@ -92,7 +68,7 @@ private function getGroupsToRemove(array $samlGroupNames, array $assignedGroups)
 	private function getGroupsToAdd(array $samlGroupNames, array $assignedGroupIds): array {
 		$groupsToAdd = [];
 		foreach ($samlGroupNames as $groupName) {
-			\OCP\Log\logger('user_saml')->debug('Checking group {group} for addition', ['group' => $groupName]);
+			Server::get(LoggerInterface::class)->debug('Checking group {group} for addition', ['app' => 'user_saml', 'group' => $groupName]);
 			$group = $this->groupManager->get($groupName);
 			// if user is not assigned to the group or the provided group has a non SAML backend
 			if (!in_array($groupName, $assignedGroupIds) || !$this->hasSamlBackend($group)) {
@@ -298,9 +274,9 @@ protected function mayModifyGroup(?IGroup $group): bool {
 			&& $this->isGroupInTransitionList($group->getGID());
 
 		if ($isInTransition) {
-			\OCP\Log\logger('user_saml')->debug('Checking group {group} for foreign members', ['group' => $group->getGID()]);
+			Server::get(LoggerInterface::class)->debug('Checking group {group} for foreign members', ['app' => 'user_saml', 'group' => $group->getGID()]);
 			$hasOnlySamlUsers = !$this->hasGroupForeignMembers($group);
-			\OCP\Log\logger('user_saml')->debug('Completed checking group {group} for foreign members', ['group' => $group->getGID()]);
+			Server::get(LoggerInterface::class)->debug('Completed checking group {group} for foreign members', ['app' => 'user_saml', 'group' => $group->getGID()]);
 			if (!$hasOnlySamlUsers) {
 				$this->updateCandidatePool([$group->getGID()]);
 			}

lib/Jobs/MigrateGroups.php

@@ -32,33 +32,16 @@ class MigrateGroups extends QueuedJob {
 
 	protected const BATCH_SIZE = 1000;
 
-	/** @var IConfig */
-	private $config;
-	/** @var IGroupManager */
-	private $groupManager;
-	/** @var IDBConnection */
-	private $dbc;
-	/** @var GroupBackend */
-	private $ownGroupBackend;
-	/** @var LoggerInterface */
-	private $logger;
-
 	public function __construct(
 		protected GroupMigration $groupMigration,
 		protected GroupManager $ownGroupManager,
-		IConfig $config,
-		IGroupManager $groupManager,
-		IDBConnection $dbc,
-		GroupBackend $ownGroupBackend,
-		LoggerInterface $logger,
+		private IConfig $config,
+		private IGroupManager $groupManager,
+		private IDBConnection $dbc,
+		private GroupBackend $ownGroupBackend,
+		private LoggerInterface $logger,
 		ITimeFactory $timeFactory,
 	) {
-		parent::__construct($timeFactory);
-		$this->config = $config;
-		$this->groupManager = $groupManager;
-		$this->dbc = $dbc;
-		$this->ownGroupBackend = $ownGroupBackend;
-		$this->logger = $logger;
 	}
 
 	protected function run($argument) {

lib/Middleware/OnlyLoggedInMiddleware.php

@@ -19,21 +19,12 @@
  * @package OCA\User_SAML\MiddleWare
  */
 class OnlyLoggedInMiddleware extends Middleware {
-	/** @var IControllerMethodReflector */
-	private $reflector;
-	/** @var IUserSession */
-	private $userSession;
-	/** @var IURLGenerator */
-	private $urlGenerator;
 
 	public function __construct(
-		IControllerMethodReflector $reflector,
-		IUserSession $userSession,
-		IURLGenerator $urlGenerator,
+		private IControllerMethodReflector $reflector,
+		private IUserSession $userSession,
+		private IURLGenerator $urlGenerator,
 	) {
-		$this->reflector = $reflector;
-		$this->userSession = $userSession;
-		$this->urlGenerator = $urlGenerator;
 	}
 
 	/**

lib/Migration/RememberLocalGroupsForPotentialMigrations.php

@@ -18,17 +18,10 @@
 
 class RememberLocalGroupsForPotentialMigrations implements IRepairStep {
 
-	/** @var IGroupManager */
-	private $groupManager;
-	/** @var IConfig */
-	private $config;
-
 	public function __construct(
-		IGroupManager $groupManager,
-		IConfig $config,
+		private IGroupManager $groupManager,
+		private IConfig $config,
 	) {
-		$this->groupManager = $groupManager;
-		$this->config = $config;
 	}
 
 	public function getName(): string {

lib/SAMLSettings.php

@@ -65,29 +65,17 @@ class SAMLSettings {
 
 	public const DEFAULT_GROUP_PREFIX = 'SAML_';
 
-	/** @var IURLGenerator */
-	private $urlGenerator;
-	/** @var IConfig */
-	private $config;
-	/** @var ISession */
-	private $session;
 	/** @var array<int, array<string, string>> */
 	private $configurations = [];
 	/** @var int */
 	private $configurationsLoadedState = self::LOADED_NONE;
-	/** @var ConfigurationsMapper */
-	private $mapper;
 
 	public function __construct(
-		IURLGenerator $urlGenerator,
-		IConfig $config,
-		ISession $session,
-		ConfigurationsMapper $mapper,
+		private IURLGenerator $urlGenerator,
+		private IConfig $config,
+		private ISession $session,
+		private ConfigurationsMapper $mapper,
 	) {
-		$this->urlGenerator = $urlGenerator;
-		$this->config = $config;
-		$this->session = $session;
-		$this->mapper = $mapper;
 	}
 
 	/**
@@ -132,7 +120,7 @@ public function getOneLoginSettingsArray(int $idp): array {
 
 		$settings = [
 			'strict' => true,
-			'debug' => $this->config->getSystemValue('debug', false),
+			'debug' => $this->config->getSystemValueBool('debug', false),
 			'baseurl' => $this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.base'),
 			'security' => [
 				'nameIdEncrypted' => ($this->configurations[$idp]['security-nameIdEncrypted'] ?? '0') === '1',