From 7f3502ed347d4080ff17a1929beafb1fee82a84c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 1 Apr 2026 14:56:22 +0200 Subject: [PATCH 1/6] fix: Use stricter typing and increase psalm level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/Command/GroupAdminsToLdap.php | 32 ++++++++++++------------------- lib/LDAPConnect.php | 24 ++++++++++------------- lib/LDAPUserManager.php | 4 ++-- psalm.xml | 2 +- tests/stubs/stub.phpstub | 2 +- 5 files changed, 26 insertions(+), 38 deletions(-) diff --git a/lib/Command/GroupAdminsToLdap.php b/lib/Command/GroupAdminsToLdap.php index 426e7926..0b1e875e 100644 --- a/lib/Command/GroupAdminsToLdap.php +++ b/lib/Command/GroupAdminsToLdap.php @@ -22,33 +22,19 @@ 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 SubAdmin $subAdmin, + private IConfig $ocConfig, + private Helper $helper, + private Group_Proxy $groupProxy, ) { parent::__construct(); - - $this->subAdmin = $subAdmin; - $this->ocConfig = $ocConfig; - $this->helper = $helper; - $this->groupProxy = $groupProxy; } #[\Override] @@ -147,6 +133,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 +152,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..0a194afa 100644 --- a/lib/LDAPConnect.php +++ b/lib/LDAPConnect.php @@ -16,10 +16,8 @@ use Psr\Log\LoggerInterface; class LDAPConnect { - /** @var Configuration */ - private $ldapConfig; - /** @var bool|null */ - private $passwdSupport; + private Configuration $ldapConfig; + private ?bool $passwdSupport; public function __construct( Helper $ldapBackendHelper, @@ -27,15 +25,14 @@ public function __construct( ) { $this->passwdSupport = null; $ldapConfigPrefixes = $ldapBackendHelper->getServerConfigurationPrefixes(true); - $prefix = array_shift($ldapConfigPrefixes); + $prefix = array_shift($ldapConfigPrefixes) ?? ''; $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 +48,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 +69,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 +91,9 @@ public function bind() { } /** - * @return false|resource|Connection * @throws ServerNotAvailableException */ - public function getLDAPConnection() { + public function getLDAPConnection(): Connection|false { return $this->bind(); } @@ -142,11 +137,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 \LDAP\Result|false */ $ret = ldap_read($connection, '', '(objectclass=*)', ['supportedExtension']); if ($ret === false) { $this->passwdSupport = false; diff --git a/lib/LDAPUserManager.php b/lib/LDAPUserManager.php index d81d0047..c7c92319 100644 --- a/lib/LDAPUserManager.php +++ b/lib/LDAPUserManager.php @@ -84,7 +84,7 @@ public function setDisplayName($uid, $displayName) { ); } - if (!is_resource($connection) && !is_object($connection)) { + if (!is_object($connection)) { $this->logger->debug('LDAP resource not available', ['app' => 'ldap_write_support']); throw new ServerNotAvailableException('LDAP server is not available'); } @@ -253,7 +253,7 @@ public function buildNewEntry($username, $password, $base): array { foreach ($lines as $line) { $split = explode(':', $line, 2); $key = trim($split[0]); - $value = trim($split[1]); + $value = trim($split[1] ?? ''); if (!isset($entry[$key])) { $entry[$key] = $value; } elseif (is_array($entry[$key])) { 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 --> Date: Wed, 1 Apr 2026 15:19:57 +0200 Subject: [PATCH 2/6] fix: As much strong type as possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/AppInfo/Application.php | 2 ++ lib/Command/GroupAdminsToLdap.php | 4 ++- lib/LDAPConnect.php | 2 ++ lib/LDAPGroupManager.php | 2 ++ lib/LDAPUserManager.php | 34 +++++++------------ .../GroupBackendRegisteredListener.php | 7 ++-- .../UserBackendRegisteredListener.php | 7 ++-- lib/Service/Configuration.php | 13 ++++--- lib/Settings/Admin.php | 25 +++----------- 9 files changed, 36 insertions(+), 60 deletions(-) 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 @@ @@ -38,7 +40,7 @@ public function __construct( } #[\Override] - protected function configure() { + protected function configure(): void { $this ->setName('ldap-ext:sync-group-admins') ->setDescription('syncs group admin informations to ldap') diff --git a/lib/LDAPConnect.php b/lib/LDAPConnect.php index 0a194afa..30ae9232 100644 --- a/lib/LDAPConnect.php +++ b/lib/LDAPConnect.php @@ -1,5 +1,7 @@ diff --git a/lib/LDAPGroupManager.php b/lib/LDAPGroupManager.php index fcf01056..33f93186 100644 --- a/lib/LDAPGroupManager.php +++ b/lib/LDAPGroupManager.php @@ -1,5 +1,7 @@ diff --git a/lib/LDAPUserManager.php b/lib/LDAPUserManager.php index c7c92319..2536b6bd 100644 --- a/lib/LDAPUserManager.php +++ b/lib/LDAPUserManager.php @@ -1,5 +1,7 @@ @@ -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); @@ -106,10 +107,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 +158,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 +229,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'); @@ -268,10 +267,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 +313,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 +328,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 +339,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 +388,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..10cdc34f 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 IAppManager $appManager, private LDAPGroupManager $ldapGroupManager, ) { - $this->appManager = $appManager; } /** diff --git a/lib/Listener/UserBackendRegisteredListener.php b/lib/Listener/UserBackendRegisteredListener.php index eb816746..18cb238e 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 IAppManager $appManager, private LDAPUserManager $ldapUserManager, ) { - $this->appManager = $appManager; } /** diff --git a/lib/Service/Configuration.php b/lib/Service/Configuration.php index 4737607b..b57467f8 100644 --- a/lib/Service/Configuration.php +++ b/lib/Service/Configuration.php @@ -1,6 +1,7 @@ config = $config; + public function __construct( + private 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..2225358b 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; } } From fa3f7fe87121765e572b0606c30977d872745884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 12 May 2026 17:18:22 +0200 Subject: [PATCH 3/6] chore: Update nextcloud/ocp, update baseline, remove dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- composer.lock | 8 ++++---- lib/LDAPUserManager.php | 4 ---- tests/psalm-baseline.xml | 11 ++--------- 3 files changed, 6 insertions(+), 17 deletions(-) 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/LDAPUserManager.php b/lib/LDAPUserManager.php index 2536b6bd..9cda7afb 100644 --- a/lib/LDAPUserManager.php +++ b/lib/LDAPUserManager.php @@ -85,10 +85,6 @@ public function setDisplayName($uid, $displayName): string { ); } - if (!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; diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 809bdfa4..4771c8b5 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,21 +1,14 @@ - - + ldapConfig->ldapGroupFilter]]> ldapConfig->ldapGroupMemberAssocAttr]]> - - !is_resource($cr) - - listen + From 306ef02654bf12e7f5c8f01eccee030bfc9a43cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 12 May 2026 17:27:37 +0200 Subject: [PATCH 4/6] chore: Run rector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- composer.json | 3 ++- lib/Command/GroupAdminsToLdap.php | 12 ++++++------ lib/LDAPConnect.php | 7 ++++--- lib/LDAPGroupManager.php | 8 ++++---- lib/LDAPUserManager.php | 16 ++++++++-------- lib/Listener/GroupBackendRegisteredListener.php | 4 ++-- lib/Listener/UserBackendRegisteredListener.php | 4 ++-- lib/Service/Configuration.php | 2 +- lib/Settings/Admin.php | 4 ++-- 9 files changed, 31 insertions(+), 29 deletions(-) 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/lib/Command/GroupAdminsToLdap.php b/lib/Command/GroupAdminsToLdap.php index 10a1b210..d26abfe9 100644 --- a/lib/Command/GroupAdminsToLdap.php +++ b/lib/Command/GroupAdminsToLdap.php @@ -31,10 +31,10 @@ class GroupAdminsToLdap extends Command { * GroupAdminsToLdap constructor. */ public function __construct( - private SubAdmin $subAdmin, - private IConfig $ocConfig, - private Helper $helper, - private Group_Proxy $groupProxy, + private readonly SubAdmin $subAdmin, + private readonly IConfig $ocConfig, + private readonly Helper $helper, + private readonly Group_Proxy $groupProxy, ) { parent::__construct(); } @@ -136,7 +136,7 @@ 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); + throw new Exception('Failed to find group ' . $gid); } foreach ($users as $uid) { $userDN = $access->getUserMapper()->getDNByName($uid); @@ -155,7 +155,7 @@ 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); + throw new Exception('Failed to find group ' . $gid); } foreach ($users as $uid) { $userDN = $access->getUserMapper()->getDNByName($uid); diff --git a/lib/LDAPConnect.php b/lib/LDAPConnect.php index 30ae9232..159d1ba6 100644 --- a/lib/LDAPConnect.php +++ b/lib/LDAPConnect.php @@ -11,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; @@ -18,12 +19,12 @@ use Psr\Log\LoggerInterface; class LDAPConnect { - private Configuration $ldapConfig; + 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); @@ -144,7 +145,7 @@ public function hasPasswordPolicy(): 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 \LDAP\Result|false */ + /** @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 33f93186..8d2984f1 100644 --- a/lib/LDAPGroupManager.php +++ b/lib/LDAPGroupManager.php @@ -20,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 9cda7afb..4f413d4b 100644 --- a/lib/LDAPUserManager.php +++ b/lib/LDAPUserManager.php @@ -31,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(); } diff --git a/lib/Listener/GroupBackendRegisteredListener.php b/lib/Listener/GroupBackendRegisteredListener.php index 10cdc34f..34151739 100644 --- a/lib/Listener/GroupBackendRegisteredListener.php +++ b/lib/Listener/GroupBackendRegisteredListener.php @@ -20,8 +20,8 @@ */ class GroupBackendRegisteredListener implements IEventListener { public function __construct( - private IAppManager $appManager, - private LDAPGroupManager $ldapGroupManager, + private readonly IAppManager $appManager, + private readonly LDAPGroupManager $ldapGroupManager, ) { } diff --git a/lib/Listener/UserBackendRegisteredListener.php b/lib/Listener/UserBackendRegisteredListener.php index 18cb238e..7f80ee27 100644 --- a/lib/Listener/UserBackendRegisteredListener.php +++ b/lib/Listener/UserBackendRegisteredListener.php @@ -20,8 +20,8 @@ */ class UserBackendRegisteredListener implements IEventListener { public function __construct( - private IAppManager $appManager, - private LDAPUserManager $ldapUserManager, + private readonly IAppManager $appManager, + private readonly LDAPUserManager $ldapUserManager, ) { } diff --git a/lib/Service/Configuration.php b/lib/Service/Configuration.php index b57467f8..fa09e5b0 100644 --- a/lib/Service/Configuration.php +++ b/lib/Service/Configuration.php @@ -14,7 +14,7 @@ class Configuration { public function __construct( - private IConfig $config, + private readonly IConfig $config, ) { } diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index 2225358b..0ef3325f 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -18,8 +18,8 @@ class Admin implements ISettings { public function __construct( - private IInitialState $initialStateService, - private Configuration $config, + private readonly IInitialState $initialStateService, + private readonly Configuration $config, ) { } From 55b5fa58ec9b826a60e08f2741ff728e94a9f95c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 12 May 2026 17:30:38 +0200 Subject: [PATCH 5/6] chore: Add baseline to REUSE.toml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- REUSE.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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" From a7bbcae46a92b70c32d843156dac9e50d4f3106e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 18 May 2026 11:55:21 +0200 Subject: [PATCH 6/6] fix: Throw Exceptions instead of continuing on error cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes from code review Signed-off-by: Côme Chilliet --- lib/LDAPConnect.php | 5 ++++- lib/LDAPUserManager.php | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/LDAPConnect.php b/lib/LDAPConnect.php index 159d1ba6..467db545 100644 --- a/lib/LDAPConnect.php +++ b/lib/LDAPConnect.php @@ -28,7 +28,10 @@ public function __construct( ) { $this->passwdSupport = null; $ldapConfigPrefixes = $ldapBackendHelper->getServerConfigurationPrefixes(true); - $prefix = array_shift($ldapConfigPrefixes) ?? ''; + $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); } diff --git a/lib/LDAPUserManager.php b/lib/LDAPUserManager.php index 4f413d4b..05d640a7 100644 --- a/lib/LDAPUserManager.php +++ b/lib/LDAPUserManager.php @@ -248,7 +248,10 @@ private function buildNewEntry(string $username, string $password, string $base) foreach ($lines as $line) { $split = explode(':', $line, 2); $key = trim($split[0]); - $value = trim($split[1] ?? ''); + if (!isset($split[1])) { + throw new Exception('Invalid LDIF format'); + } + $value = trim($split[1]); if (!isset($entry[$key])) { $entry[$key] = $value; } elseif (is_array($entry[$key])) {