refined code, to be independend from OCP\IUser

rework updateProfile in user_ldap/lib/User/User.php
some cleanup at processAttributes in user_ldap/lib/User/User.php
rearranged Fediverse attribute, to match profile layout

Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Marc Hefter <marchefter@gmail.com>

6 files changed, 51 insertions, 56 deletions

diff --git a/apps/user_ldap/js/wizard/wizardTabAdvanced.js b/apps/user_ldap/js/wizard/wizardTabAdvanced.js
index 170625ce896..5b247294473 100644
--- a/apps/user_ldap/js/wizard/wizardTabAdvanced.js
+++ b/apps/user_ldap/js/wizard/wizardTabAdvanced.js
@@ -143,6 +143,10 @@ OCA = OCA || {};
 					$element: $('#ldap_attr_twitter'),
 					setMethod: 'setTwitterAttribute'
 				},
+				ldap_attr_fediverse: {
+					$element: $('#ldap_attr_fediverse'),
+					setMethod: 'setFediverseAttribute'
+				},
 				ldap_attr_organisation: {
 					$element: $('#ldap_attr_organisation'),
 					setMethod: 'setOrganisationAttribute'
@@ -159,10 +163,6 @@ OCA = OCA || {};
 					$element: $('#ldap_attr_biography'),
 					setMethod: 'setBiographyAttribute'
 				},
-				ldap_attr_fediverse: {
-					$element: $('#ldap_attr_fediverse'),
-					setMethod: 'setFediverseAttribute'
-				},
 				ldap_profile_scope: {
 					$element: $('#ldap_profile_scope'),
 					setMethod: 'setProfileScope'
@@ -445,6 +445,15 @@ OCA = OCA || {};
 		},
 
 		/**
+		 * sets the attribute for the Nextcloud user profile fediverse
+		 *
+		 * @param {string} attribute
+		 */
+		setFediverseAttribute: function(attribute) {
+			this.setElementValue(this.managedItems.ldap_attr_fediverse.$element, attribute);
+		},
+
+		/**
 		 * sets the attribute for the Nextcloud user profile organisation
 		 *
 		 * @param {string} attribute
@@ -481,15 +490,6 @@ OCA = OCA || {};
 		},
 
 		/**
-		 * sets the attribute for the Nextcloud user profile fediverse
-		 *
-		 * @param {string} attribute
-		 */
-		setFediverseAttribute: function(attribute) {
-			this.setElementValue(this.managedItems.ldap_attr_fediverse.$element, attribute);
-		},
-
-		/**
 		 * sets the visibility scope for the Nextcloud user profile properties
 		 *
 		 * @param {string} scope
diff --git a/apps/user_ldap/lib/Configuration.php b/apps/user_ldap/lib/Configuration.php
index 1eb6c7986e5..3935da8fa89 100644
--- a/apps/user_ldap/lib/Configuration.php
+++ b/apps/user_ldap/lib/Configuration.php
@@ -128,11 +128,11 @@ class Configuration {
 		'ldapAttributeWebsite' => null,
 		'ldapAttributeAddress' => null,
 		'ldapAttributeTwitter' => null,
+		'ldapAttributeFediverse' => null,
 		'ldapAttributeOrganisation' => null,
 		'ldapAttributeRole' => null,
 		'ldapAttributeHeadline' => null,
 		'ldapAttributeBiography' => null,
-		'ldapAttributeFediverse' => null,
 		'ldapProfileScope' => null,
 	];
 
@@ -484,11 +484,11 @@ class Configuration {
 			'ldap_attr_website' => '',
 			'ldap_attr_address' => '',
 			'ldap_attr_twitter' => '',
+			'ldap_attr_fediverse' => '',
 			'ldap_attr_organisation' => '',
 			'ldap_attr_role' => '',
 			'ldap_attr_headline' => '',
 			'ldap_attr_biography' => '',
-			'ldap_attr_fediverse' => '',
 			'ldap_profile_scope' => '',
 		];
 	}
@@ -560,11 +560,11 @@ class Configuration {
 			'ldap_attr_website' => 'ldapAttributeWebsite',
 			'ldap_attr_address' => 'ldapAttributeAddress',
 			'ldap_attr_twitter' => 'ldapAttributeTwitter',
+			'ldap_attr_fediverse' => 'ldapAttributeFediverse',
 			'ldap_attr_organisation' => 'ldapAttributeOrganisation',
 			'ldap_attr_role' => 'ldapAttributeRole',
 			'ldap_attr_headline' => 'ldapAttributeHeadline',
 			'ldap_attr_biography' => 'ldapAttributeBiography',
-			'ldap_attr_fediverse' => 'ldapAttributeFediverse',
 			'ldap_profile_scope' => 'ldapProfileScope',
 		];
 		return $array;
diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php
index 8bf2904179a..1c365ef2afc 100644
--- a/apps/user_ldap/lib/Connection.php
+++ b/apps/user_ldap/lib/Connection.php
@@ -77,11 +77,11 @@ use Psr\Log\LoggerInterface;
  * @property string ldapAttributeWebsite
  * @property string ldapAttributeAddress
  * @property string ldapAttributeTwitter
+ * @property string ldapAttributeFediverse
  * @property string ldapAttributeOrganisation
  * @property string ldapAttributeRole
  * @property string ldapAttributeHeadline
  * @property string ldapAttributeBiography
- * @property string ldapAttributeFediverse
  * @property string ldapProfileScope
  */
 class Connection extends LDAPUtility {
diff --git a/apps/user_ldap/lib/User/Manager.php b/apps/user_ldap/lib/User/Manager.php
index d356a272f8c..04c67a537b8 100644
--- a/apps/user_ldap/lib/User/Manager.php
+++ b/apps/user_ldap/lib/User/Manager.php
@@ -157,11 +157,11 @@ class Manager {
 			$this->access->getConnection()->ldapAttributeWebsite,
 			$this->access->getConnection()->ldapAttributeAddress,
 			$this->access->getConnection()->ldapAttributeTwitter,
+			$this->access->getConnection()->ldapAttributeFediverse,
 			$this->access->getConnection()->ldapAttributeOrganisation,
 			$this->access->getConnection()->ldapAttributeRole,
 			$this->access->getConnection()->ldapAttributeHeadline,
 			$this->access->getConnection()->ldapAttributeBiography,
-			$this->access->getConnection()->ldapAttributeFediverse,
 		];
 
 		$homeRule = (string)$this->access->getConnection()->homeFolderNamingRule;
diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index f327ec44e77..3dd8f05a07d 100644
--- a/apps/user_ldap/lib/User/User.php
+++ b/apps/user_ldap/lib/User/User.php
@@ -32,6 +32,8 @@
  */
 namespace OCA\User_LDAP\User;
 
+use Exception;
+use OC\Accounts\AccountManager;
 use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\Exceptions\AttributeNotSet;
@@ -42,6 +44,7 @@ use OCP\ILogger;
 use OCP\Image;
 use OCP\IUser;
 use OCP\IUserManager;
+use OCP\Accounts\IAccountManager;
 use OCP\Accounts\PropertyDoesNotExistException;
 use OCP\Notification\IManager as INotificationManager;
 use Psr\Log\LoggerInterface;
@@ -233,17 +236,12 @@ class User {
 		}
 		unset($attr);
 
-		/**
-		 * Additions to User_LDAP, for writing the User Profile
-		 *
-		 * @var string|null $profileScope the configured scope of visibility
-		 * @var array<string, string> $profileValues array of the LDAP data
-		 */
 		//User profile visibility
 		$profileScope = $this->connection->ldapProfileScope;
 		if (empty($profileScope) || $profileScope === 'unset') {
 			$profileScope = null;
 		}
+		$profileValues = array(); // empty array, to prevent unneccessary call to updateProfile
 		//User Profile Field - Phone number
 		$attr = strtolower($this->connection->ldapAttributePhone);
 		if (isset($ldapEntry[$attr])) {
@@ -264,6 +262,11 @@ class User {
 		if (isset($ldapEntry[$attr])) {
 			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_TWITTER] = $ldapEntry[$attr][0];
 		}
+		//User Profile Field - fediverse
+		$attr = strtolower($this->connection->ldapAttributeFediverse);
+		if (isset($ldapEntry[$attr])) {
+			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_FEDIVERSE] = $ldapEntry[$attr][0];
+		}
 		//User Profile Field - organisation
 		$attr = strtolower($this->connection->ldapAttributeOrganisation);
 		if (isset($ldapEntry[$attr])) {
@@ -284,17 +287,10 @@ class User {
 		if (isset($ldapEntry[$attr])) {
 			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_BIOGRAPHY] = $ldapEntry[$attr][0];
 		}
-		//User Profile Field - fediverse
-		$attr = strtolower($this->connection->ldapAttributeFediverse);
-		if (isset($ldapEntry[$attr])) {
-			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_FEDIVERSE] = $ldapEntry[$attr][0];
-		}
 		// Update user profile
-		if(0 < count($profileValues)) {
+		if(!empty($profileValues)) {
 			$this->updateProfile($profileValues, $profileScope);
-			unset($profileValues);
 		}
-		unset($profileScope);
 		unset($attr);
 
 		//Avatar
@@ -589,39 +585,38 @@ class User {
 		if ($this->wasRefreshed('profile')) {
 			return;
 		}
-		// check if parameter array is empty
-		if(0 == count($profileValues)) {
-			return;
-		}
 		// fetch/prepare user
 		$user = $this->userManager->get($this->uid);
 		if (is_null($user)) {
 			return;
 		}
+		// prepare AccountManager and Account
+		$accountManager = \OC::$server->get(IAccountManager::class);
+		$account = $accountManager->getAccount($user);	// get Account
+		if (is_null($account)) {
+			return;
+		}
 		// loop through the properties and handle them
-		/** @var string $property the array key (property name from AccountManager class) */
-		/** @var string $valueFromLDAP the value as read from LDAP */
 		foreach($profileValues as $property => $valueFromLDAP) {
-			$this->logger->debug('user profile data ('.$property.') from LDAP '.$this->dn, ['app' => 'user_ldap']);
 			// check and update profile properties
-			/** @var string $propertyValue */
-			$propertyValue = [$valueFromLDAP];
-			if (isset($propertyValue[0])) {
-				/** @var string $value */
-				$value = $propertyValue[0];
-				try {
-					/** @var string $currentValue */
-						$currentValue = (string)$user->getProfilePropertyValue($property);
-						if ($currentValue !== $value) {
-							$user->setProfileProperty($property,$value,$profileScope,null);
-							$this->logger->debug('property updated: '.$property.'='.$value.' for user '.$this->getUsername().'', ['app' => 'user_ldap']);
-						}
-				} catch (PropertyDoesNotExistException $e) {
-					$this->logger->error('property does not exist: '.$property.' for user '.$this->getUsername().'', ['app' => 'user_ldap']);
-					return;
-				}
+			$value = (is_array($valueFromLDAP) ? $valueFromLDAP[0] : $valueFromLDAP); // take ONLY the first value, if multiple values specified
+			try {
+				$accountProperty = $account->getProperty($property);
+				$currentValue = $accountProperty->getValue();
+				$scope = ($profileScope ? $profileScope : ($accountProperty->getScope() ? $accountProperty->getScope() : AccountManager::DEFAULT_SCOPES[$property]));
+			}
+			catch (PropertyDoesNotExistException $e) { // thrown at getProperty
+				$this->logger->error('property does not exist: '.$property.' for uid='.$this->uid.'', ['app' => 'user_ldap', 'exception' => $e]);
+				$currentValue = '';
+				$scope = ($profileScope ? $profileScope : AccountManager::DEFAULT_SCOPES[$property]);
+			}
+			$verified = IAccountManager::VERIFIED; // trust the LDAP admin knew what he put there
+			if ($currentValue !== $value) {
+				$account->setProperty($property,$value,$scope,$verified);
+				$this->logger->debug('property updated: '.$property.'='.$value.' for uid='.$this->uid.'', ['app' => 'user_ldap']);
 			}
 		}
+		$accountManager->updateAccount($account);
 	}
 
 	/**
diff --git a/apps/user_ldap/templates/settings.php b/apps/user_ldap/templates/settings.php
index c0ff2b6a866..805cfce7c23 100644
--- a/apps/user_ldap/templates/settings.php
+++ b/apps/user_ldap/templates/settings.php
@@ -126,11 +126,11 @@ style('user_ldap', 'settings');
 				<p><label for="ldap_attr_website"> <?php p($l->t('Website Field')); ?></label><input type="text" id="ldap_attr_website" name="ldap_attr_website" title="<?php p($l->t('User profile Website will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_website_default']); ?>"></p>
 				<p><label for="ldap_attr_address"> <?php p($l->t('Address Field')); ?></label><input type="text" id="ldap_attr_address" name="ldap_attr_address" title="<?php p($l->t('User profile Address will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_address_default']); ?>"></p>
 				<p><label for="ldap_attr_twitter"> <?php p($l->t('Twitter Field')); ?></label><input type="text" id="ldap_attr_twitter" name="ldap_attr_twitter" title="<?php p($l->t('User profile Twitter will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_twitter_default']); ?>"></p>
+				<p><label for="ldap_attr_fediverse"> <?php p($l->t('Fediverse Field')); ?></label><input type="text" id="ldap_attr_fediverse" name="ldap_attr_fediverse" title="<?php p($l->t('User profile Fediverse will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_fediverse_default']); ?>"></p>
 				<p><label for="ldap_attr_organisation"> <?php p($l->t('Organisation Field')); ?></label><input type="text" id="ldap_attr_organisation" name="ldap_attr_organisation" title="<?php p($l->t('User profile Organisation will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_organisation_default']); ?>"></p>
 				<p><label for="ldap_attr_role"> <?php p($l->t('Role Field')); ?></label><input type="text" id="ldap_attr_role" name="ldap_attr_role" title="<?php p($l->t('User profile Role will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_role_default']); ?>"></p>
 				<p><label for="ldap_attr_headline"> <?php p($l->t('Headline Field')); ?></label><input type="text" id="ldap_attr_headline" name="ldap_attr_headline" title="<?php p($l->t('User profile Headline will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_headline_default']); ?>"></p>
 				<p><label for="ldap_attr_biography"> <?php p($l->t('Biography Field')); ?></label><input type="text" id="ldap_attr_biography" name="ldap_attr_biography" title="<?php p($l->t('User profile Biography will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_biography_default']); ?>"></p>
-				<p><label for="ldap_attr_fediverse"> <?php p($l->t('Fediverse Field')); ?></label><input type="text" id="ldap_attr_fediverse" name="ldap_attr_fediverse" title="<?php p($l->t('User profile Fediverse will be set from the specified attribute')); ?>" data-default="<?php p($_['ldap_attr_fediverse_default']); ?>"></p>
 				<p><label for="ldap_profile_scope"><?php p($l->t('Profile Scope'));?></label><select id="ldap_profile_scope" name="ldap_profile_scope" data-default="<?php p($_['ldap_profile_scope_default']); ?>" ><option value="unset"<?php if (! isset($_['ldap_profile_scope']) || ($_['ldap_profile_scope'] === 'unset')) p(' selected'); ?>>must be set by user</option><option value="v2-private"<?php if (isset($_['ldap_profile_scope']) && ($_['ldap_profile_scope'] === 'v2-private')) p(' selected'); ?>>private</option><option value="v2-local"<?php if (isset($_['ldap_profile_scope']) && ($_['ldap_profile_scope'] === 'v2-local')) p(' selected'); ?>>local</option><option value="v2-federated"<?php if (isset($_['ldap_profile_scope']) && ($_['ldap_profile_scope'] === 'v2-federated')) p(' selected'); ?>>federated</option><option value="v2-published"<?php if (isset($_['ldap_profile_scope']) && ($_['ldap_profile_scope'] === 'v2-published')) p(' selected'); ?>>published</option></select></p>
 			</div>
 		</div>