Merge pull request #50162 from nextcloud/fix/improve-ldap-avatar-handling

Improve ldap avatar handling

13 files changed, 44 insertions, 193 deletions

diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php
index 48fe59a9a51..6cecd7fd6a6 100644
--- a/apps/user_ldap/composer/composer/autoload_classmap.php
+++ b/apps/user_ldap/composer/composer/autoload_classmap.php
@@ -38,7 +38,6 @@ return array(
     'OCA\\User_LDAP\\Exceptions\\ConstraintViolationException' => $baseDir . '/../lib/Exceptions/ConstraintViolationException.php',
     'OCA\\User_LDAP\\Exceptions\\NoMoreResults' => $baseDir . '/../lib/Exceptions/NoMoreResults.php',
     'OCA\\User_LDAP\\Exceptions\\NotOnLDAP' => $baseDir . '/../lib/Exceptions/NotOnLDAP.php',
-    'OCA\\User_LDAP\\FilesystemHelper' => $baseDir . '/../lib/FilesystemHelper.php',
     'OCA\\User_LDAP\\GroupPluginManager' => $baseDir . '/../lib/GroupPluginManager.php',
     'OCA\\User_LDAP\\Group_LDAP' => $baseDir . '/../lib/Group_LDAP.php',
     'OCA\\User_LDAP\\Group_Proxy' => $baseDir . '/../lib/Group_Proxy.php',
diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php
index dd5ad0322af..b31ac98834f 100644
--- a/apps/user_ldap/composer/composer/autoload_static.php
+++ b/apps/user_ldap/composer/composer/autoload_static.php
@@ -53,7 +53,6 @@ class ComposerStaticInitUser_LDAP
         'OCA\\User_LDAP\\Exceptions\\ConstraintViolationException' => __DIR__ . '/..' . '/../lib/Exceptions/ConstraintViolationException.php',
         'OCA\\User_LDAP\\Exceptions\\NoMoreResults' => __DIR__ . '/..' . '/../lib/Exceptions/NoMoreResults.php',
         'OCA\\User_LDAP\\Exceptions\\NotOnLDAP' => __DIR__ . '/..' . '/../lib/Exceptions/NotOnLDAP.php',
-        'OCA\\User_LDAP\\FilesystemHelper' => __DIR__ . '/..' . '/../lib/FilesystemHelper.php',
         'OCA\\User_LDAP\\GroupPluginManager' => __DIR__ . '/..' . '/../lib/GroupPluginManager.php',
         'OCA\\User_LDAP\\Group_LDAP' => __DIR__ . '/..' . '/../lib/Group_LDAP.php',
         'OCA\\User_LDAP\\Group_Proxy' => __DIR__ . '/..' . '/../lib/Group_Proxy.php',
diff --git a/apps/user_ldap/lib/AppInfo/Application.php b/apps/user_ldap/lib/AppInfo/Application.php
index 3c1cb5daa12..59c88e06073 100644
--- a/apps/user_ldap/lib/AppInfo/Application.php
+++ b/apps/user_ldap/lib/AppInfo/Application.php
@@ -10,7 +10,6 @@ use OCA\Files_External\Service\BackendService;
 use OCA\User_LDAP\Controller\RenewPasswordController;
 use OCA\User_LDAP\Events\GroupBackendRegistered;
 use OCA\User_LDAP\Events\UserBackendRegistered;
-use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\Group_Proxy;
 use OCA\User_LDAP\GroupPluginManager;
 use OCA\User_LDAP\Handler\ExtStorageConfigHandler;
@@ -85,7 +84,6 @@ class Application extends App implements IBootstrap {
 			function (ContainerInterface $c) {
 				return new Manager(
 					$c->get(IConfig::class),
-					$c->get(FilesystemHelper::class),
 					$c->get(LoggerInterface::class),
 					$c->get(IAvatarManager::class),
 					$c->get(Image::class),
diff --git a/apps/user_ldap/lib/FilesystemHelper.php b/apps/user_ldap/lib/FilesystemHelper.php
deleted file mode 100644
index eea7ec62ad8..00000000000
--- a/apps/user_ldap/lib/FilesystemHelper.php
+++ /dev/null
@@ -1,32 +0,0 @@
-<?php
-
-/**
- * SPDX-FileCopyrightText: 2017-2024 Nextcloud GmbH and Nextcloud contributors
- * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-namespace OCA\User_LDAP;
-
-use OC\Files\Filesystem;
-
-/**
- * @brief wraps around static Nextcloud core methods
- */
-class FilesystemHelper {
-
-	/**
-	 * @brief states whether the filesystem was loaded
-	 * @return bool
-	 */
-	public function isLoaded() {
-		return Filesystem::$loaded;
-	}
-
-	/**
-	 * @brief initializes the filesystem for the given user
-	 * @param string $uid the Nextcloud username of the user
-	 */
-	public function setup($uid) {
-		\OC_Util::setupFS($uid);
-	}
-}
diff --git a/apps/user_ldap/lib/User/Manager.php b/apps/user_ldap/lib/User/Manager.php
index 2b09d425721..125970de1e7 100644
--- a/apps/user_ldap/lib/User/Manager.php
+++ b/apps/user_ldap/lib/User/Manager.php
@@ -8,7 +8,6 @@
 namespace OCA\User_LDAP\User;
 
 use OCA\User_LDAP\Access;
-use OCA\User_LDAP\FilesystemHelper;
 use OCP\Cache\CappedMemoryCache;
 use OCP\IAvatarManager;
 use OCP\IConfig;
@@ -35,7 +34,6 @@ class Manager {
 
 	public function __construct(
 		protected IConfig $ocConfig,
-		protected FilesystemHelper $ocFilesystem,
 		protected LoggerInterface $logger,
 		protected IAvatarManager $avatarManager,
 		protected Image $image,
@@ -66,7 +64,7 @@ class Manager {
 	private function createAndCache($dn, $uid) {
 		$this->checkAccess();
 		$user = new User($uid, $dn, $this->access, $this->ocConfig,
-			$this->ocFilesystem, clone $this->image, $this->logger,
+			clone $this->image, $this->logger,
 			$this->avatarManager, $this->userManager,
 			$this->notificationManager);
 		$this->usersByDN[$dn] = $user;
diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index 824d12c52e1..ec06d1d8636 100644
--- a/apps/user_ldap/lib/User/User.php
+++ b/apps/user_ldap/lib/User/User.php
@@ -12,7 +12,6 @@ use OC\Accounts\AccountManager;
 use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\Exceptions\AttributeNotSet;
-use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\Service\BirthdateParserService;
 use OCP\Accounts\IAccountManager;
 use OCP\Accounts\PropertyDoesNotExistException;
@@ -33,66 +32,40 @@ use Psr\Log\LoggerInterface;
  * represents an LDAP user, gets and holds user-specific information from LDAP
  */
 class User {
+	protected Connection $connection;
 	/**
-	 * @var Connection
+	 * @var array<string,1>
 	 */
-	protected $connection;
-	/**
-	 * @var LoggerInterface
-	 */
-	protected $logger;
-	/**
-	 * @var string
-	 */
-	protected $dn;
-	/**
-	 * @var string
-	 */
-	protected $uid;
-	/**
-	 * @var string[]
-	 */
-	protected $refreshedFeatures = [];
-	/**
-	 * @var string
-	 */
-	protected $avatarImage;
+	protected array $refreshedFeatures = [];
+	protected string|false|null $avatarImage = null;
 
 	protected BirthdateParserService $birthdateParser;
 
 	/**
 	 * DB config keys for user preferences
+	 * @var string
 	 */
 	public const USER_PREFKEY_FIRSTLOGIN = 'firstLoginAccomplished';
 
 	/**
 	 * @brief constructor, make sure the subclasses call this one!
-	 * @param string $username the internal username
-	 * @param string $dn the LDAP DN
 	 */
 	public function __construct(
-		$username,
-		$dn,
+		protected string $uid,
+		protected string $dn,
 		protected Access $access,
 		protected IConfig $config,
-		protected FilesystemHelper $fs,
 		protected Image $image,
-		LoggerInterface $logger,
+		protected LoggerInterface $logger,
 		protected IAvatarManager $avatarManager,
 		protected IUserManager $userManager,
 		protected INotificationManager $notificationManager,
 	) {
-		if ($username === null) {
-			$logger->error("uid for '$dn' must not be null!", ['app' => 'user_ldap']);
-			throw new \InvalidArgumentException('uid must not be null!');
-		} elseif ($username === '') {
+		if ($uid === '') {
 			$logger->error("uid for '$dn' must not be an empty string", ['app' => 'user_ldap']);
 			throw new \InvalidArgumentException('uid must not be an empty string!');
 		}
 		$this->connection = $this->access->getConnection();
-		$this->dn = $dn;
-		$this->uid = $username;
-		$this->logger = $logger;
 		$this->birthdateParser = new BirthdateParserService();
 
 		Util::connectHook('OC_User', 'post_login', $this, 'handlePasswordExpiry');
@@ -103,7 +76,7 @@ class User {
 	 *
 	 * @throws PreConditionNotMetException
 	 */
-	public function markUser() {
+	public function markUser(): void {
 		$curValue = $this->config->getUserValue($this->getUsername(), 'user_ldap', 'isDeleted', '0');
 		if ($curValue === '1') {
 			// the user is already marked, do not write to DB again
@@ -117,7 +90,7 @@ class User {
 	 * processes results from LDAP for attributes as returned by getAttributesToRead()
 	 * @param array $ldapEntry the user entry as retrieved from LDAP
 	 */
-	public function processAttributes($ldapEntry) {
+	public function processAttributes(array $ldapEntry): void {
 		//Quota
 		$attr = strtolower($this->connection->ldapQuotaAttribute);
 		if (isset($ldapEntry[$attr])) {
@@ -331,11 +304,7 @@ class User {
 		foreach ($attributes as $attribute) {
 			if (isset($ldapEntry[$attribute])) {
 				$this->avatarImage = $ldapEntry[$attribute][0];
-				// the call to the method that saves the avatar in the file
-				// system must be postponed after the login. It is to ensure
-				// external mounts are mounted properly (e.g. with login
-				// credentials from the session).
-				Util::connectHook('OC_User', 'post_login', $this, 'updateAvatarPostLogin');
+				$this->updateAvatar();
 				break;
 			}
 		}
@@ -359,11 +328,9 @@ class User {
 
 	/**
 	 * returns the home directory of the user if specified by LDAP settings
-	 * @param ?string $valueFromLDAP
-	 * @return false|string
 	 * @throws \Exception
 	 */
-	public function getHomePath($valueFromLDAP = null) {
+	public function getHomePath(?string $valueFromLDAP = null): string|false {
 		$path = (string)$valueFromLDAP;
 		$attr = null;
 
@@ -371,8 +338,12 @@ class User {
 		   && str_starts_with($this->access->connection->homeFolderNamingRule, 'attr:')
 		   && $this->access->connection->homeFolderNamingRule !== 'attr:') {
 			$attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:'));
-			$homedir = $this->access->readAttribute($this->access->username2dn($this->getUsername()), $attr);
-			if ($homedir && isset($homedir[0])) {
+			$dn = $this->access->username2dn($this->getUsername());
+			if ($dn === false) {
+				return false;
+			}
+			$homedir = $this->access->readAttribute($dn, $attr);
+			if ($homedir !== false && isset($homedir[0])) {
 				$path = $homedir[0];
 			}
 		}
@@ -407,7 +378,7 @@ class User {
 		return false;
 	}
 
-	public function getMemberOfGroups() {
+	public function getMemberOfGroups(): array|false {
 		$cacheKey = 'getMemberOf' . $this->getUsername();
 		$memberOfGroups = $this->connection->getFromCache($cacheKey);
 		if (!is_null($memberOfGroups)) {
@@ -420,9 +391,9 @@ class User {
 
 	/**
 	 * @brief reads the image from LDAP that shall be used as Avatar
-	 * @return string data (provided by LDAP) | false
+	 * @return string|false data (provided by LDAP)
 	 */
-	public function getAvatarImage() {
+	public function getAvatarImage(): string|false {
 		if (!is_null($this->avatarImage)) {
 			return $this->avatarImage;
 		}
@@ -433,7 +404,7 @@ class User {
 		$attributes = $connection->resolveRule('avatar');
 		foreach ($attributes as $attribute) {
 			$result = $this->access->readAttribute($this->dn, $attribute);
-			if ($result !== false && is_array($result) && isset($result[0])) {
+			if ($result !== false && isset($result[0])) {
 				$this->avatarImage = $result[0];
 				break;
 			}
@@ -444,20 +415,16 @@ class User {
 
 	/**
 	 * @brief marks the user as having logged in at least once
-	 * @return null
 	 */
-	public function markLogin() {
+	public function markLogin(): void {
 		$this->config->setUserValue(
 			$this->uid, 'user_ldap', self::USER_PREFKEY_FIRSTLOGIN, '1');
 	}
 
 	/**
 	 * Stores a key-value pair in relation to this user
-	 *
-	 * @param string $key
-	 * @param string $value
 	 */
-	private function store($key, $value) {
+	private function store(string $key, string $value): void {
 		$this->config->setUserValue($this->uid, 'user_ldap', $key, $value);
 	}
 
@@ -465,12 +432,9 @@ class User {
 	 * Composes the display name and stores it in the database. The final
 	 * display name is returned.
 	 *
-	 * @param string $displayName
-	 * @param string $displayName2
 	 * @return string the effective display name
 	 */
-	public function composeAndStoreDisplayName($displayName, $displayName2 = '') {
-		$displayName2 = (string)$displayName2;
+	public function composeAndStoreDisplayName(string $displayName, string $displayName2 = ''): string {
 		if ($displayName2 !== '') {
 			$displayName .= ' (' . $displayName2 . ')';
 		}
@@ -489,9 +453,8 @@ class User {
 
 	/**
 	 * Stores the LDAP Username in the Database
-	 * @param string $userName
 	 */
-	public function storeLDAPUserName($userName) {
+	public function storeLDAPUserName(string $userName): void {
 		$this->store('uid', $userName);
 	}
 
@@ -500,9 +463,8 @@ class User {
 	 * already. If not, it will marked like this, because it is expected that
 	 * the method will be run, when false is returned.
 	 * @param string $feature email | quota | avatar | profile (can be extended)
-	 * @return bool
 	 */
-	private function wasRefreshed($feature) {
+	private function wasRefreshed(string $feature): bool {
 		if (isset($this->refreshedFeatures[$feature])) {
 			return true;
 		}
@@ -512,10 +474,9 @@ class User {
 
 	/**
 	 * fetches the email from LDAP and stores it as Nextcloud user value
-	 * @param string $valueFromLDAP if known, to save an LDAP read request
-	 * @return null
+	 * @param ?string $valueFromLDAP if known, to save an LDAP read request
 	 */
-	public function updateEmail($valueFromLDAP = null) {
+	public function updateEmail(?string $valueFromLDAP = null): void {
 		if ($this->wasRefreshed('email')) {
 			return;
 		}
@@ -534,7 +495,7 @@ class User {
 			if (!is_null($user)) {
 				$currentEmail = (string)$user->getSystemEMailAddress();
 				if ($currentEmail !== $email) {
-					$user->setEMailAddress($email);
+					$user->setSystemEMailAddress($email);
 				}
 			}
 		}
@@ -558,9 +519,8 @@ class User {
 	 * fetches the quota from LDAP and stores it as Nextcloud user value
 	 * @param ?string $valueFromLDAP the quota attribute's value can be passed,
 	 *                               to save the readAttribute request
-	 * @return void
 	 */
-	public function updateQuota($valueFromLDAP = null) {
+	public function updateQuota(?string $valueFromLDAP = null): void {
 		if ($this->wasRefreshed('quota')) {
 			return;
 		}
@@ -574,7 +534,7 @@ class User {
 		$quota = false;
 		if (is_null($valueFromLDAP) && $quotaAttribute !== '') {
 			$aQuota = $this->access->readAttribute($this->dn, $quotaAttribute);
-			if ($aQuota && (count($aQuota) > 0) && $this->verifyQuotaValue($aQuota[0])) {
+			if ($aQuota !== false && isset($aQuota[0]) && $this->verifyQuotaValue($aQuota[0])) {
 				$quota = $aQuota[0];
 			} elseif (is_array($aQuota) && isset($aQuota[0])) {
 				$this->logger->debug('no suitable LDAP quota found for user ' . $this->uid . ': [' . $aQuota[0] . ']', ['app' => 'user_ldap']);
@@ -582,7 +542,7 @@ class User {
 		} elseif (!is_null($valueFromLDAP) && $this->verifyQuotaValue($valueFromLDAP)) {
 			$quota = $valueFromLDAP;
 		} else {
-			$this->logger->debug('no suitable LDAP quota found for user ' . $this->uid . ': [' . $valueFromLDAP . ']', ['app' => 'user_ldap']);
+			$this->logger->debug('no suitable LDAP quota found for user ' . $this->uid . ': [' . ($valueFromLDAP ?? '') . ']', ['app' => 'user_ldap']);
 		}
 
 		if ($quota === false && $this->verifyQuotaValue($defaultQuota)) {
@@ -601,7 +561,7 @@ class User {
 		}
 	}
 
-	private function verifyQuotaValue(string $quotaValue) {
+	private function verifyQuotaValue(string $quotaValue): bool {
 		return $quotaValue === 'none' || $quotaValue === 'default' || \OC_Helper::computerFileSize($quotaValue) !== false;
 	}
 
@@ -656,17 +616,6 @@ class User {
 	}
 
 	/**
-	 * called by a post_login hook to save the avatar picture
-	 *
-	 * @param array $params
-	 */
-	public function updateAvatarPostLogin($params) {
-		if (isset($params['uid']) && $params['uid'] === $this->getUsername()) {
-			$this->updateAvatar();
-		}
-	}
-
-	/**
 	 * @brief attempts to get an image from LDAP and sets it as Nextcloud avatar
 	 * @return bool true when the avatar was set successfully or is up to date
 	 */
@@ -691,7 +640,7 @@ class User {
 			return true;
 		}
 
-		$isSet = $this->setOwnCloudAvatar();
+		$isSet = $this->setNextcloudAvatar();
 
 		if ($isSet) {
 			// save checksum only after successful setting
@@ -712,9 +661,8 @@ class User {
 
 	/**
 	 * @brief sets an image as Nextcloud avatar
-	 * @return bool
 	 */
-	private function setOwnCloudAvatar() {
+	private function setNextcloudAvatar(): bool {
 		if (!$this->image->valid()) {
 			$this->logger->error('avatar image data from LDAP invalid for ' . $this->dn, ['app' => 'user_ldap']);
 			return false;
@@ -728,10 +676,6 @@ class User {
 			return false;
 		}
 
-		if (!$this->fs->isLoaded()) {
-			$this->fs->setup($this->uid);
-		}
-
 		try {
 			$avatar = $this->avatarManager->getAvatar($this->uid);
 			$avatar->set($this->image);
@@ -773,7 +717,7 @@ class User {
 		} else {
 			$extHomeValues = [$valueFromLDAP];
 		}
-		if ($extHomeValues && isset($extHomeValues[0])) {
+		if ($extHomeValues !== false && isset($extHomeValues[0])) {
 			$extHome = $extHomeValues[0];
 			$this->config->setUserValue($this->getUsername(), 'user_ldap', 'extStorageHome', $extHome);
 			return $extHome;
@@ -785,13 +729,12 @@ class User {
 
 	/**
 	 * called by a post_login hook to handle password expiry
-	 *
-	 * @param array $params
 	 */
-	public function handlePasswordExpiry($params) {
+	public function handlePasswordExpiry(array $params): void {
 		$ppolicyDN = $this->connection->ldapDefaultPPolicyDN;
 		if (empty($ppolicyDN) || ((int)$this->connection->turnOnPasswordChange !== 1)) {
-			return;//password expiry handling disabled
+			//password expiry handling disabled
+			return;
 		}
 		$uid = $params['uid'];
 		if (isset($uid) && $uid === $this->getUsername()) {
diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php
index 60d7b7e92c2..6a648b2624c 100644
--- a/apps/user_ldap/lib/User_LDAP.php
+++ b/apps/user_ldap/lib/User_LDAP.php
@@ -450,7 +450,7 @@ class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, I
 
 			$user = $this->access->userManager->get($uid);
 			if ($user instanceof User) {
-				$displayName = $user->composeAndStoreDisplayName($displayName, $displayName2);
+				$displayName = $user->composeAndStoreDisplayName($displayName, (string)$displayName2);
 				$this->access->connection->writeToCache($cacheKey, $displayName);
 			}
 			if ($user instanceof OfflineUser) {
diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php
index 5bb4a4657cb..1ecc5565bed 100644
--- a/apps/user_ldap/tests/AccessTest.php
+++ b/apps/user_ldap/tests/AccessTest.php
@@ -10,7 +10,6 @@ use OC\ServerNotAvailableException;
 use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\Exceptions\ConstraintViolationException;
-use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\Helper;
 use OCA\User_LDAP\ILDAPWrapper;
 use OCA\User_LDAP\LDAP;
@@ -111,7 +110,6 @@ class AccessTest extends TestCase {
 		$um = $this->getMockBuilder(Manager::class)
 			->setConstructorArgs([
 				$this->createMock(IConfig::class),
-				$this->createMock(FilesystemHelper::class),
 				$this->createMock(LoggerInterface::class),
 				$this->createMock(IAvatarManager::class),
 				$this->createMock(Image::class),
diff --git a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php
index 726342a88f7..12f97637618 100644
--- a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php
+++ b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php
@@ -9,7 +9,6 @@ namespace OCA\User_LDAP\Tests\Integration;
 
 use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
-use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\GroupPluginManager;
 use OCA\User_LDAP\Helper;
 use OCA\User_LDAP\LDAP;
@@ -110,7 +109,6 @@ abstract class AbstractIntegrationTest {
 	protected function initUserManager() {
 		$this->userManager = new Manager(
 			\OC::$server->getConfig(),
-			new FilesystemHelper(),
 			\OC::$server->get(LoggerInterface::class),
 			\OC::$server->get(IAvatarManager::class),
 			new Image(),
diff --git a/apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php b/apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php
index 082c8ee46d4..75ce42e299c 100644
--- a/apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php
+++ b/apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php
@@ -7,7 +7,6 @@
  */
 namespace OCA\User_LDAP\Tests\Integration\Lib\User;
 
-use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\Tests\Integration\AbstractIntegrationTest;
 use OCA\User_LDAP\User\DeletedUsersIndex;
@@ -115,7 +114,6 @@ class IntegrationTestUserAvatar extends AbstractIntegrationTest {
 	protected function initUserManager() {
 		$this->userManager = new Manager(
 			\OC::$server->getConfig(),
-			new FilesystemHelper(),
 			\OC::$server->get(LoggerInterface::class),
 			\OC::$server->get(IAvatarManager::class),
 			new Image(),
diff --git a/apps/user_ldap/tests/User/ManagerTest.php b/apps/user_ldap/tests/User/ManagerTest.php
index 449f159ec4d..4f504ff5f7a 100644
--- a/apps/user_ldap/tests/User/ManagerTest.php
+++ b/apps/user_ldap/tests/User/ManagerTest.php
@@ -9,7 +9,6 @@ namespace OCA\User_LDAP\Tests\User;
 
 use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
-use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\ILDAPWrapper;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\User\User;
@@ -36,9 +35,6 @@ class ManagerTest extends \Test\TestCase {
 	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
 	protected $config;
 
-	/** @var FilesystemHelper|\PHPUnit\Framework\MockObject\MockObject */
-	protected $fileSystemHelper;
-
 	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
 	protected $logger;
 
@@ -73,7 +69,6 @@ class ManagerTest extends \Test\TestCase {
 
 		$this->access = $this->createMock(Access::class);
 		$this->config = $this->createMock(IConfig::class);
-		$this->fileSystemHelper = $this->createMock(FilesystemHelper::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->avatarManager = $this->createMock(IAvatarManager::class);
 		$this->image = $this->createMock(Image::class);
@@ -91,7 +86,6 @@ class ManagerTest extends \Test\TestCase {
 		/** @noinspection PhpUnhandledExceptionInspection */
 		$this->manager = new Manager(
 			$this->config,
-			$this->fileSystemHelper,
 			$this->logger,
 			$this->avatarManager,
 			$this->image,
diff --git a/apps/user_ldap/tests/User/UserTest.php b/apps/user_ldap/tests/User/UserTest.php
index 3939c41c4ca..badbca7f476 100644
--- a/apps/user_ldap/tests/User/UserTest.php
+++ b/apps/user_ldap/tests/User/UserTest.php
@@ -9,7 +9,6 @@ namespace OCA\User_LDAP\Tests\User;
 
 use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
-use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\User\User;
 use OCP\IAvatar;
 use OCP\IAvatarManager;
@@ -36,8 +35,6 @@ class UserTest extends \Test\TestCase {
 	protected $connection;
 	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
 	protected $config;
-	/** @var FilesystemHelper|\PHPUnit\Framework\MockObject\MockObject */
-	protected $filesystemhelper;
 	/** @var INotificationManager|\PHPUnit\Framework\MockObject\MockObject */
 	protected $notificationManager;
 	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
@@ -67,7 +64,6 @@ class UserTest extends \Test\TestCase {
 			->willReturn($this->connection);
 
 		$this->config = $this->createMock(IConfig::class);
-		$this->filesystemhelper = $this->createMock(FilesystemHelper::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->avatarManager = $this->createMock(IAvatarManager::class);
 		$this->image = $this->createMock(Image::class);
@@ -79,7 +75,6 @@ class UserTest extends \Test\TestCase {
 			$this->dn,
 			$this->access,
 			$this->config,
-			$this->filesystemhelper,
 			$this->image,
 			$this->logger,
 			$this->avatarManager,
@@ -109,7 +104,7 @@ class UserTest extends \Test\TestCase {
 			->disableOriginalConstructor()
 			->getMock();
 		$coreUser->expects($this->once())
-			->method('setEMailAddress')
+			->method('setSystemEMailAddress')
 			->with('alice@foo.bar');
 
 		$this->userManager->expects($this->any())
@@ -508,10 +503,6 @@ class UserTest extends \Test\TestCase {
 			->method('setUserValue')
 			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo'));
 
-		$this->filesystemhelper->expects($this->once())
-			->method('isLoaded')
-			->willReturn(true);
-
 		$avatar = $this->createMock(IAvatar::class);
 		$avatar->expects($this->once())
 			->method('set')
@@ -559,9 +550,6 @@ class UserTest extends \Test\TestCase {
 		$this->config->expects($this->never())
 			->method('setUserValue');
 
-		$this->filesystemhelper->expects($this->never())
-			->method('isLoaded');
-
 		$avatar = $this->createMock(IAvatar::class);
 		$avatar->expects($this->never())
 			->method('set');
@@ -626,10 +614,6 @@ class UserTest extends \Test\TestCase {
 			->method('setUserValue')
 			->with($this->uid, 'user_ldap', 'lastAvatarChecksum', md5('this is a photo'));
 
-		$this->filesystemhelper->expects($this->once())
-			->method('isLoaded')
-			->willReturn(true);
-
 		$avatar = $this->createMock(IAvatar::class);
 		$avatar->expects($this->once())
 			->method('set')
@@ -681,9 +665,6 @@ class UserTest extends \Test\TestCase {
 		$this->config->expects($this->never())
 			->method('setUserValue');
 
-		$this->filesystemhelper->expects($this->never())
-			->method('isLoaded');
-
 		$avatar = $this->createMock(IAvatar::class);
 		$avatar->expects($this->never())
 			->method('set');
@@ -739,10 +720,6 @@ class UserTest extends \Test\TestCase {
 		$this->config->expects($this->never())
 			->method('setUserValue');
 
-		$this->filesystemhelper->expects($this->once())
-			->method('isLoaded')
-			->willReturn(true);
-
 		$avatar = $this->createMock(IAvatar::class);
 		$avatar->expects($this->once())
 			->method('set')
@@ -792,9 +769,6 @@ class UserTest extends \Test\TestCase {
 		$this->config->expects($this->never())
 			->method('setUserValue');
 
-		$this->filesystemhelper->expects($this->never())
-			->method('isLoaded');
-
 		$this->avatarManager->expects($this->never())
 			->method('getAvatar');
 
@@ -917,7 +891,6 @@ class UserTest extends \Test\TestCase {
 				$this->dn,
 				$this->access,
 				$this->config,
-				$this->filesystemhelper,
 				$this->image,
 				$this->logger,
 				$this->avatarManager,
@@ -1042,7 +1015,6 @@ class UserTest extends \Test\TestCase {
 		return [
 			['Roland Deschain', '', 'Roland Deschain', false],
 			['Roland Deschain', '', 'Roland Deschain', true],
-			['Roland Deschain', null, 'Roland Deschain', false],
 			['Roland Deschain', 'gunslinger@darktower.com', 'Roland Deschain (gunslinger@darktower.com)', false],
 			['Roland Deschain', 'gunslinger@darktower.com', 'Roland Deschain (gunslinger@darktower.com)', true],
 		];
diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index 378429ceda3..11b121045a2 100644
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -1105,20 +1105,6 @@
       <code><![CDATA[public function setLdapAccess(Access $access) {]]></code>
     </InvalidDocblock>
   </file>
-  <file src="apps/user_ldap/lib/User/User.php">
-    <FalsableReturnStatement>
-      <code><![CDATA[$this->avatarImage]]></code>
-    </FalsableReturnStatement>
-    <InvalidPropertyAssignmentValue>
-      <code><![CDATA[$this->refreshedFeatures]]></code>
-    </InvalidPropertyAssignmentValue>
-    <InvalidReturnType>
-      <code><![CDATA[null]]></code>
-    </InvalidReturnType>
-    <RedundantCondition>
-      <code><![CDATA[$aQuota && (count($aQuota) > 0)]]></code>
-    </RedundantCondition>
-  </file>
   <file src="apps/user_ldap/lib/User_LDAP.php">
     <ImplementedReturnTypeMismatch>
       <code><![CDATA[string|false]]></code>