diff --git a/REUSE.toml b/REUSE.toml index 3cd1fd26..25a072d3 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -18,7 +18,7 @@ SPDX-FileCopyrightText = "2019 Nextcloud translators" SPDX-License-Identifier = "AGPL-3.0-or-later" [[annotations]] -path = [".tx/config", ".eslintrc.json", "vendor-bin/cs-fixer/composer.json", "vendor-bin/cs-fixer/composer.lock", "vendor-bin/psalm/composer.json", "vendor-bin/psalm/composer.lock", "vendor-bin/rector/composer.json", "vendor-bin/rector/composer.lock"] +path = [".tx/config", ".eslintrc.json", "vendor-bin/cs-fixer/composer.json", "vendor-bin/cs-fixer/composer.lock", "vendor-bin/psalm/composer.json", "vendor-bin/psalm/composer.lock", "vendor-bin/rector/composer.json", "vendor-bin/rector/composer.lock", "tests/psalm-baseline.xml"] precedence = "aggregate" SPDX-FileCopyrightText = "2024 Nextcloud GmbH and Nextcloud contributors" SPDX-License-Identifier = "AGPL-3.0-or-later" diff --git a/composer.json b/composer.json index 9b6219b9..397f0db2 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,8 @@ "psalm": "psalm", "psalm:fix": "psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType", "psalm:update-baseline": "psalm --threads=1 --update-baseline", - "test:integration": "cd tests/integration && ./run.sh" + "test:integration": "cd tests/integration && ./run.sh", + "rector": "rector && composer cs:fix" }, "license": "AGPLv3", "authors": [ diff --git a/composer.lock b/composer.lock index 1de089b3..0287966d 100644 --- a/composer.lock +++ b/composer.lock @@ -70,12 +70,12 @@ "source": { "type": "git", "url": "https://github.com/nextcloud-deps/ocp.git", - "reference": "07722b9013ea9e57f79d3a75ccc68d4278bd7fd6" + "reference": "e282f342ed616a619237169bdab757d2df1058a3" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/07722b9013ea9e57f79d3a75ccc68d4278bd7fd6", - "reference": "07722b9013ea9e57f79d3a75ccc68d4278bd7fd6", + "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/e282f342ed616a619237169bdab757d2df1058a3", + "reference": "e282f342ed616a619237169bdab757d2df1058a3", "shasum": "" }, "require": { @@ -112,7 +112,7 @@ "issues": "https://github.com/nextcloud-deps/ocp/issues", "source": "https://github.com/nextcloud-deps/ocp/tree/master" }, - "time": "2026-05-15T08:42:57+00:00" + "time": "2026-05-12T01:58:54+00:00" }, { "name": "psr/clock", diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 1a9c427c..0e3152cc 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -1,10 +1,12 @@ @@ -22,37 +24,23 @@ class GroupAdminsToLdap extends Command { /** * This adds/removes group subadmins as ldap group owners */ - private $simulate = false; - private $verbose = false; - - /** @var SubAdmin */ - private $subAdmin; - /** @var IConfig */ - private $ocConfig; - /** @var Helper */ - private $helper; - /** @var Group_Proxy */ - private $groupProxy; + private bool $simulate = false; + private bool $verbose = false; /** * GroupAdminsToLdap constructor. */ public function __construct( - SubAdmin $subAdmin, - IConfig $ocConfig, - Helper $helper, - Group_Proxy $groupProxy, + private readonly SubAdmin $subAdmin, + private readonly IConfig $ocConfig, + private readonly Helper $helper, + private readonly Group_Proxy $groupProxy, ) { parent::__construct(); - - $this->subAdmin = $subAdmin; - $this->ocConfig = $ocConfig; - $this->helper = $helper; - $this->groupProxy = $groupProxy; } #[\Override] - protected function configure() { + protected function configure(): void { $this ->setName('ldap-ext:sync-group-admins') ->setDescription('syncs group admin informations to ldap') @@ -147,6 +135,9 @@ function diff_user_arrays($array1, $array2) { foreach ($onlyInNC as $gid => $users) { $groupDN = $access->getGroupMapper()->getDNByName($gid); + if ($groupDN === false) { + throw new Exception('Failed to find group ' . $gid); + } foreach ($users as $uid) { $userDN = $access->getUserMapper()->getDNByName($uid); $entry = [ @@ -163,6 +154,9 @@ function diff_user_arrays($array1, $array2) { foreach ($onlyInLDAP as $gid => $users) { $groupDN = $access->getGroupMapper()->getDNByName($gid); + if ($groupDN === false) { + throw new Exception('Failed to find group ' . $gid); + } foreach ($users as $uid) { $userDN = $access->getUserMapper()->getDNByName($uid); $entry = [ diff --git a/lib/LDAPConnect.php b/lib/LDAPConnect.php index 3da3cebf..467db545 100644 --- a/lib/LDAPConnect.php +++ b/lib/LDAPConnect.php @@ -1,5 +1,7 @@ @@ -9,6 +11,7 @@ namespace OCA\LdapWriteSupport; use LDAP\Connection; +use LDAP\Result; use OC\ServerNotAvailableException; use OCA\LdapWriteSupport\AppInfo\Application; use OCA\User_LDAP\Configuration; @@ -16,26 +19,26 @@ use Psr\Log\LoggerInterface; class LDAPConnect { - /** @var Configuration */ - private $ldapConfig; - /** @var bool|null */ - private $passwdSupport; + private readonly Configuration $ldapConfig; + private ?bool $passwdSupport; public function __construct( Helper $ldapBackendHelper, - private LoggerInterface $logger, + private readonly LoggerInterface $logger, ) { $this->passwdSupport = null; $ldapConfigPrefixes = $ldapBackendHelper->getServerConfigurationPrefixes(true); $prefix = array_shift($ldapConfigPrefixes); + if ($prefix === null) { + throw new \LogicException('This should never get called when no LDAP configurations were saved'); + } $this->ldapConfig = new Configuration($prefix); } /** - * @return resource|Connection * @throws ServerNotAvailableException */ - public function connect() { + public function connect(): Connection { $ldapHost = $this->ldapConfig->ldapHost; $ldapPort = $this->ldapConfig->ldapPort; @@ -51,7 +54,7 @@ public function connect() { // Connecting to LDAP - TODO: connect directly via LDAP plugin $cr = ldap_connect($ldapHost); - if (!is_resource($cr) && !is_object($cr)) { + if (!is_object($cr)) { $this->logger->error('Unable to connect to LDAP host {ldapHost}:{ldapPort}', [ 'app' => Application::APP_ID, @@ -72,10 +75,9 @@ public function connect() { } /** - * @return false|resource|Connection * @throws ServerNotAvailableException */ - public function bind() { + public function bind(): Connection|false { $ds = $this->connect(); $dn = $this->ldapConfig->ldapAgentName; $secret = $this->ldapConfig->ldapAgentPassword; @@ -95,10 +97,9 @@ public function bind() { } /** - * @return false|resource|Connection * @throws ServerNotAvailableException */ - public function getLDAPConnection() { + public function getLDAPConnection(): Connection|false { return $this->bind(); } @@ -142,11 +143,12 @@ public function hasPasswordPolicy(): bool { * checks whether the LDAP server supports the passwd exop * * @param Connection $connection LDAP connection to check - * @return boolean either the user can or cannot + * @return bool either the user can or cannot */ - public function hasPasswdExopSupport($connection): bool { + public function hasPasswdExopSupport(Connection $connection): bool { // TODO: We should cache this by ldap prefix, but currently we have no access to it. if (is_null($this->passwdSupport)) { + /** @var Result|false */ $ret = ldap_read($connection, '', '(objectclass=*)', ['supportedExtension']); if ($ret === false) { $this->passwdSupport = false; diff --git a/lib/LDAPGroupManager.php b/lib/LDAPGroupManager.php index fcf01056..8d2984f1 100644 --- a/lib/LDAPGroupManager.php +++ b/lib/LDAPGroupManager.php @@ -1,5 +1,7 @@ @@ -18,10 +20,10 @@ class LDAPGroupManager implements ILDAPGroupPlugin { public function __construct( - private IGroupManager $groupManager, - private LDAPConnect $ldapConnect, - private LoggerInterface $logger, - private ILDAPProvider $ldapProvider, + private readonly IGroupManager $groupManager, + private readonly LDAPConnect $ldapConnect, + private readonly LoggerInterface $logger, + private readonly ILDAPProvider $ldapProvider, ) { if ($this->ldapConnect->groupsEnabled()) { $this->makeLdapBackendFirst(); diff --git a/lib/LDAPUserManager.php b/lib/LDAPUserManager.php index d81d0047..05d640a7 100644 --- a/lib/LDAPUserManager.php +++ b/lib/LDAPUserManager.php @@ -1,5 +1,7 @@ @@ -29,15 +31,15 @@ class LDAPUserManager implements ILDAPUserPlugin { public function __construct( - private IUserManager $userManager, - private IUserSession $userSession, - private LDAPConnect $ldapConnect, - private ILDAPProvider $ldapProvider, - private Configuration $configuration, - private IL10N $l10n, - private LoggerInterface $logger, + private readonly IUserManager $userManager, + private readonly IUserSession $userSession, + private readonly LDAPConnect $ldapConnect, + private readonly ILDAPProvider $ldapProvider, + private readonly Configuration $configuration, + private readonly IL10N $l10n, + private readonly LoggerInterface $logger, ) { - $this->userManager->listen('\OC\User', 'changeUser', [$this, 'changeUserHook']); + $this->userManager->listen('\OC\User', 'changeUser', $this->changeUserHook(...)); $this->makeLdapBackendFirst(); } @@ -48,7 +50,7 @@ public function __construct( * @return int bitwise-or'ed actions */ #[\Override] - public function respondToActions() { + public function respondToActions(): int { $setPassword = $this->canSetPassword() && !$this->ldapConnect->hasPasswordPolicy() ? Backend::SET_PASSWORD : 0; @@ -63,12 +65,11 @@ public function respondToActions() { * * @param string $uid user ID of the user * @param string $displayName new user's display name - * @return string * @throws HintException * @throws ServerNotAvailableException */ #[\Override] - public function setDisplayName($uid, $displayName) { + public function setDisplayName($uid, $displayName): string { $userDN = $this->getUserDN($uid); $connection = $this->ldapProvider->getLDAPConnection($uid); @@ -84,10 +85,6 @@ public function setDisplayName($uid, $displayName) { ); } - if (!is_resource($connection) && !is_object($connection)) { - $this->logger->debug('LDAP resource not available', ['app' => 'ldap_write_support']); - throw new ServerNotAvailableException('LDAP server is not available'); - } try { if (ldap_mod_replace($connection, $userDN, [$displayNameField => $displayName])) { return $displayName; @@ -106,10 +103,9 @@ public function setDisplayName($uid, $displayName) { * checks whether the user is allowed to change his avatar in Nextcloud * * @param string $uid the Nextcloud user name - * @return bool either the user can or cannot */ #[\Override] - public function canChangeAvatar($uid) { + public function canChangeAvatar($uid): bool { return $this->configuration->hasAvatarPermission(); } @@ -158,11 +154,10 @@ public function changeEmail(IUser $user, string $newEmail): void { * * @param string $uid The username of the user to create * @param string $password The password of the new user - * @return bool|string the created user of false * @throws Exception */ #[\Override] - public function createUser($uid, $password) { + public function createUser($uid, $password): string|false { $adminUser = $this->userSession->getUser(); $requireActorFromLDAP = $this->configuration->isLdapActorRequired(); if ($requireActorFromLDAP && !$adminUser instanceof IUser) { @@ -230,7 +225,7 @@ public function ensureAttribute(array &$ldif, string $attribute, string $fallbac } } - public function buildNewEntry($username, $password, $base): array { + private function buildNewEntry(string $username, string $password, string $base): array { // Make sure the parameters don't fool the following algorithm if (str_contains($username, PHP_EOL)) { throw new Exception('Username contains a new line'); @@ -253,6 +248,9 @@ public function buildNewEntry($username, $password, $base): array { foreach ($lines as $line) { $split = explode(':', $line, 2); $key = trim($split[0]); + if (!isset($split[1])) { + throw new Exception('Invalid LDIF format'); + } $value = trim($split[1]); if (!isset($entry[$key])) { $entry[$key] = $value; @@ -268,10 +266,6 @@ public function buildNewEntry($username, $password, $base): array { return [$dn, $entry]; } - /** - * @param $uid - * @return bool - */ public function deleteUser($uid): bool { $connection = $this->ldapProvider->getLDAPConnection($uid); $userDN = $this->getUserDN($uid); @@ -318,12 +312,11 @@ public function canSetPassword(): bool { * * @param string $uid The username * @param string $password The new password - * @return bool * * Change the password of a user */ #[\Override] - public function setPassword($uid, $password) { + public function setPassword($uid, $password): bool { $connection = $this->ldapProvider->getLDAPConnection($uid); $userDN = $this->getUserDN($uid); @@ -334,10 +327,9 @@ public function setPassword($uid, $password) { * get the user's home directory * * @param string $uid the username - * @return bool */ #[\Override] - public function getHome($uid) { + public function getHome($uid): bool { // Not implemented return false; } @@ -346,21 +338,18 @@ public function getHome($uid) { * get display name of the user * * @param string $uid user ID of the user - * @return string display name */ #[\Override] - public function getDisplayName($uid) { + public function getDisplayName($uid): string { // Not implemented return $uid; } /** * Count the number of users - * - * @return int|bool */ #[\Override] - public function countUsers() { + public function countUsers(): false { // Not implemented return false; } @@ -398,7 +387,7 @@ public function changeUserHook(IUser $user, string $feature, $attr1, $attr2): vo } } - private function getUserDN($uid): string { + private function getUserDN(string $uid): string { return $this->ldapProvider->getUserDN($uid); } diff --git a/lib/Listener/GroupBackendRegisteredListener.php b/lib/Listener/GroupBackendRegisteredListener.php index 1ab8abbf..34151739 100644 --- a/lib/Listener/GroupBackendRegisteredListener.php +++ b/lib/Listener/GroupBackendRegisteredListener.php @@ -1,6 +1,7 @@ */ class GroupBackendRegisteredListener implements IEventListener { - /** @var IAppManager */ - private $appManager; - public function __construct( - IAppManager $appManager, - private LDAPGroupManager $ldapGroupManager, + private readonly IAppManager $appManager, + private readonly LDAPGroupManager $ldapGroupManager, ) { - $this->appManager = $appManager; } /** diff --git a/lib/Listener/UserBackendRegisteredListener.php b/lib/Listener/UserBackendRegisteredListener.php index eb816746..7f80ee27 100644 --- a/lib/Listener/UserBackendRegisteredListener.php +++ b/lib/Listener/UserBackendRegisteredListener.php @@ -1,6 +1,7 @@ */ class UserBackendRegisteredListener implements IEventListener { - /** @var IAppManager */ - private $appManager; - public function __construct( - IAppManager $appManager, - private LDAPUserManager $ldapUserManager, + private readonly IAppManager $appManager, + private readonly LDAPUserManager $ldapUserManager, ) { - $this->appManager = $appManager; } /** diff --git a/lib/Service/Configuration.php b/lib/Service/Configuration.php index 4737607b..fa09e5b0 100644 --- a/lib/Service/Configuration.php +++ b/lib/Service/Configuration.php @@ -1,6 +1,7 @@ config = $config; + public function __construct( + private readonly IConfig $config, + ) { } public function isLdapActorRequired(): bool { @@ -39,7 +38,7 @@ public function useUnicodePassword(): bool { return $this->config->getAppValue('ldap_write_support', 'useUnicodePassword', '0') === '1'; } - public function getUserTemplate() { + public function getUserTemplate(): string { return $this->config->getAppValue( Application::APP_ID, 'template.user', @@ -47,7 +46,7 @@ public function getUserTemplate() { ); } - public function getUserTemplateDefault() { + public function getUserTemplateDefault(): string { return 'dn: uid={UID},{BASE}' . PHP_EOL . 'objectClass: inetOrgPerson' . PHP_EOL diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index ca06a9de..0ef3325f 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -1,6 +1,7 @@ initialStateService = $initialStateService; } /** @@ -31,7 +28,7 @@ public function __construct( * @since 9.1 */ #[\Override] - public function getForm() { + public function getForm(): TemplateResponse { $this->initialStateService->provideInitialState( 'templates', [ @@ -58,25 +55,13 @@ public function getForm() { return new TemplateResponse(Application::APP_ID, 'settings-admin'); } - /** - * @return string the section ID, e.g. 'sharing' - * @since 9.1 - */ #[\Override] - public function getSection() { + public function getSection(): string { return 'ldap'; } - /** - * @return int whether the form should be rather on the top or bottom of - * the admin section. The forms are arranged in ascending order of the - * priority values. It is required to return a value between 0 and 100. - * - * E.g.: 70 - * @since 9.1 - */ #[\Override] - public function getPriority() { + public function getPriority(): int { return 35; } } diff --git a/psalm.xml b/psalm.xml index c34a9ec4..a203a6b6 100644 --- a/psalm.xml +++ b/psalm.xml @@ -4,7 +4,7 @@ - SPDX-License-Identifier: AGPL-3.0-or-later --> - - + ldapConfig->ldapGroupFilter]]> ldapConfig->ldapGroupMemberAssocAttr]]> - - !is_resource($cr) - - listen + diff --git a/tests/stubs/stub.phpstub b/tests/stubs/stub.phpstub index 5dd7b27c..aaf12f26 100644 --- a/tests/stubs/stub.phpstub +++ b/tests/stubs/stub.phpstub @@ -176,7 +176,7 @@ namespace OCA\User_LDAP { public string $ldapGroupFilter; public string $ldapGroupDisplayName; /** - * @return resource|\LDAP\Connection The LDAP resource + * @return \LDAP\Connection The LDAP resource */ public function getConnectionResource(); }