nice up the code handling AccountManager

merging defaultScopes from DEFAULT_SCOPES and account_manager.default_property_scope
removing unneccessary profileScope setting (using config.php instead)
honoring admin choice 'profile.enabled'=>false in config.php
moved checking for empty array to updateProfile function
corrected some typos and cleaned some comments

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

5 files changed, 66 insertions, 83 deletions

diff --git a/apps/user_ldap/js/wizard/wizardTabAdvanced.js b/apps/user_ldap/js/wizard/wizardTabAdvanced.js
index 5b247294473..a438b847401 100644
--- a/apps/user_ldap/js/wizard/wizardTabAdvanced.js
+++ b/apps/user_ldap/js/wizard/wizardTabAdvanced.js
@@ -163,10 +163,6 @@ OCA = OCA || {};
 					$element: $('#ldap_attr_biography'),
 					setMethod: 'setBiographyAttribute'
 				},
-				ldap_profile_scope: {
-					$element: $('#ldap_profile_scope'),
-					setMethod: 'setProfileScope'
-				},
 			};
 			this.setManagedItems(items);
 		},
@@ -490,15 +486,6 @@ OCA = OCA || {};
 		},
 
 		/**
-		 * sets the visibility scope for the Nextcloud user profile properties
-		 *
-		 * @param {string} scope
-		 */
-		setProfileScope: function(scope) {
-			this.setElementValue(this.managedItems.ldap_profile_scope.$element, scope);
-		},
-
-		/**
 		 * deals with the result of the Test Connection test
 		 *
 		 * @param {WizardTabAdvanced} view
diff --git a/apps/user_ldap/lib/Configuration.php b/apps/user_ldap/lib/Configuration.php
index 3935da8fa89..ef64f75a9ef 100644
--- a/apps/user_ldap/lib/Configuration.php
+++ b/apps/user_ldap/lib/Configuration.php
@@ -133,7 +133,6 @@ class Configuration {
 		'ldapAttributeRole' => null,
 		'ldapAttributeHeadline' => null,
 		'ldapAttributeBiography' => null,
-		'ldapProfileScope' => null,
 	];
 
 	public function __construct(string $configPrefix, bool $autoRead = true) {
@@ -489,7 +488,6 @@ class Configuration {
 			'ldap_attr_role' => '',
 			'ldap_attr_headline' => '',
 			'ldap_attr_biography' => '',
-			'ldap_profile_scope' => '',
 		];
 	}
 
@@ -565,7 +563,6 @@ class Configuration {
 			'ldap_attr_role' => 'ldapAttributeRole',
 			'ldap_attr_headline' => 'ldapAttributeHeadline',
 			'ldap_attr_biography' => 'ldapAttributeBiography',
-			'ldap_profile_scope' => 'ldapProfileScope',
 		];
 		return $array;
 	}
diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php
index 1c365ef2afc..d8d00dd4d27 100644
--- a/apps/user_ldap/lib/Connection.php
+++ b/apps/user_ldap/lib/Connection.php
@@ -82,7 +82,6 @@ use Psr\Log\LoggerInterface;
  * @property string ldapAttributeRole
  * @property string ldapAttributeHeadline
  * @property string ldapAttributeBiography
- * @property string ldapProfileScope
  */
 class Connection extends LDAPUtility {
 	/**
diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index 3dd8f05a07d..915bcae8289 100644
--- a/apps/user_ldap/lib/User/User.php
+++ b/apps/user_ldap/lib/User/User.php
@@ -47,6 +47,7 @@ use OCP\IUserManager;
 use OCP\Accounts\IAccountManager;
 use OCP\Accounts\PropertyDoesNotExistException;
 use OCP\Notification\IManager as INotificationManager;
+use OCP\Server;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -236,62 +237,58 @@ class User {
 		}
 		unset($attr);
 
-		//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])) {
-			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_PHONE] = $ldapEntry[$attr][0];
-		}
-		//User Profile Field - website
-		$attr = strtolower($this->connection->ldapAttributeWebsite);
-		if (isset($ldapEntry[$attr])) {
-			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_WEBSITE] = $ldapEntry[$attr][0];
-		}
-		//User Profile Field - Address
-		$attr = strtolower($this->connection->ldapAttributeAddress);
-		if (isset($ldapEntry[$attr])) {
-			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_ADDRESS] = $ldapEntry[$attr][0];
-		}
-		//User Profile Field - Twitter
-		$attr = strtolower($this->connection->ldapAttributeTwitter);
-		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])) {
-			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_ORGANISATION] = $ldapEntry[$attr][0];
-		}
-		//User Profile Field - role
-		$attr = strtolower($this->connection->ldapAttributeRole);
-		if (isset($ldapEntry[$attr])) {
-			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_ROLE] = $ldapEntry[$attr][0];
-		}
-		//User Profile Field - headline
-		$attr = strtolower($this->connection->ldapAttributeHeadline);
-		if (isset($ldapEntry[$attr])) {
-			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_HEADLINE] = $ldapEntry[$attr][0];
-		}
-		//User Profile Field - biography
-		$attr = strtolower($this->connection->ldapAttributeBiography);
-		if (isset($ldapEntry[$attr])) {
-			$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_BIOGRAPHY] = $ldapEntry[$attr][0];
-		}
-		// Update user profile
-		if(!empty($profileValues)) {
-			$this->updateProfile($profileValues, $profileScope);
+		// honoring profile disabled in config.php
+		if ($this->config->getSystemValueBool('profile.enabled', true)) {
+			$profileValues = array(); // empty array, to prevent unneccessary call to updateProfile
+			//User Profile Field - Phone number
+			$attr = strtolower($this->connection->ldapAttributePhone);
+			if (isset($ldapEntry[$attr])) {
+				$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_PHONE] = $ldapEntry[$attr][0];
+			}
+			//User Profile Field - website
+			$attr = strtolower($this->connection->ldapAttributeWebsite);
+			if (isset($ldapEntry[$attr])) {
+				$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_WEBSITE] = $ldapEntry[$attr][0];
+			}
+			//User Profile Field - Address
+			$attr = strtolower($this->connection->ldapAttributeAddress);
+			if (isset($ldapEntry[$attr])) {
+				$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_ADDRESS] = $ldapEntry[$attr][0];
+			}
+			//User Profile Field - Twitter
+			$attr = strtolower($this->connection->ldapAttributeTwitter);
+			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])) {
+				$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_ORGANISATION] = $ldapEntry[$attr][0];
+			}
+			//User Profile Field - role
+			$attr = strtolower($this->connection->ldapAttributeRole);
+			if (isset($ldapEntry[$attr])) {
+				$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_ROLE] = $ldapEntry[$attr][0];
+			}
+			//User Profile Field - headline
+			$attr = strtolower($this->connection->ldapAttributeHeadline);
+			if (isset($ldapEntry[$attr])) {
+				$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_HEADLINE] = $ldapEntry[$attr][0];
+			}
+			//User Profile Field - biography
+			$attr = strtolower($this->connection->ldapAttributeBiography);
+			if (isset($ldapEntry[$attr])) {
+				$profileValues[\OCP\Accounts\IAccountManager::PROPERTY_BIOGRAPHY] = $ldapEntry[$attr][0];
+			}
+			// Update user profile
+			$this->updateProfile($profileValues);
+			unset($attr);
 		}
-		unset($attr);
 
 		//Avatar
 		/** @var Connection $connection */
@@ -577,25 +574,29 @@ class User {
 	/**
 	 * takes values from LDAP and stores it as Nextcloud user profile value
 	 * 
-	 * @param array $profileValues associaive array of property keys and values from LDAP
-	 * @param string|null $profileScope the scope of visibility to set
+	 * @param array $profileValues associative array of property keys and values from LDAP
 	 */
-	private function updateProfile(array $profileValues, ?string $profileScope=null): void {
+	private function updateProfile(array $profileValues): void {
+		// check if given array is empty
+		if (empty($profileValues)) {
+			return; // okay, nothing to do
+		}
 		// check if user profile was refreshed before
 		if ($this->wasRefreshed('profile')) {
-			return;
+			return; // okay, updated before
 		}
 		// fetch/prepare user
 		$user = $this->userManager->get($this->uid);
 		if (is_null($user)) {
-			return;
+			return; // FIXME: I guess userManager::get would never return null here
 		}
 		// prepare AccountManager and Account
-		$accountManager = \OC::$server->get(IAccountManager::class);
+		$accountManager = Server::get(IAccountManager::class);
 		$account = $accountManager->getAccount($user);	// get Account
 		if (is_null($account)) {
-			return;
+			return; // FIXME: I guess getAccount would never return null here
 		}
+		$defaultScopes = array_merge(AccountManager::DEFAULT_SCOPES, $this->config->getSystemValue('account_manager.default_property_scope', []));
 		// loop through the properties and handle them
 		foreach($profileValues as $property => $valueFromLDAP) {
 			// check and update profile properties
@@ -603,17 +604,17 @@ class User {
 			try {
 				$accountProperty = $account->getProperty($property);
 				$currentValue = $accountProperty->getValue();
-				$scope = ($profileScope ? $profileScope : ($accountProperty->getScope() ? $accountProperty->getScope() : AccountManager::DEFAULT_SCOPES[$property]));
+				$scope = ($accountProperty->getScope() ? $accountProperty->getScope() : $defaultScopes[$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]);
+				$scope = $defaultScopes[$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']);
+				$this->logger->debug('update property: '.$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 805cfce7c23..916ff84b82a 100644
--- a/apps/user_ldap/templates/settings.php
+++ b/apps/user_ldap/templates/settings.php
@@ -131,7 +131,6 @@ style('user_ldap', 'settings');
 				<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_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>
 		<?php print_unescaped($_['settingControls']); ?>