owncloud / user_ldap

đź“’
GNU Affero General Public License v3.0
17 stars 17 forks source link

Exposed attributes #801

Closed jvillafanez closed 11 months ago

jvillafanez commented 1 year ago

Expose user attributes from LDAP to other apps.

The admin will expose some attributes for other apps to consume. For example, the admin could expose the businessCategory, employeeType and employeeNumber (assuming those attributes are available) so other apps can use that information.

By default, no attribute will be exposed. This means that other apps will have to use the information provided by core (check the core's IUser interface). The admin can add any attribute (one per line) in the user_ldap's wizard -> advanced tab -> special attributes -> exposed user attributes (at the bottom of the advanced tab). Note that the information will be available for any app, so choose wisely what information you want to expose.

Other apps can access to those attributes via the getExtendedAttributes method of the IUser class (available since 10.11). The recommended procedure to get the exposed LDAP attributes is:

  1. Check that the current IUser instance is from LDAP ($user->getBackendClassName() === 'LDAP' should be enough)
  2. Get the user's extended attributes: $eattrs = $user->getExtendedAttributes()
  3. Check that the state of the LDAP attributes is OK. This is needed because we'll have to connect to the LDAP server and the connection could be down. if (isset($eattrs['user_ldap_state']) && $eattrs['user_ldap_state'] === 'OK') ... The user_ldap_state can be either OK if we get the attributes, or Error if something wrong happens. If it's Error, there won't be any LDAP attribute available. In addition, note that the user_ldap_state will appear only for LDAP users.
  4. Once we know the state is OK, we can check whether the attribute we want is present. The attribute will have the following format: user_ldap_attr_<lowercaseAttr>, for example user_ldap_attr_employeetype or user_ldap_attr_homephonenumber
$user = $this->userManager->get('myUser');
if ($user->getBackendClassName() !== 'LDAP') {
  return; // abort, not LDAP user
}

$extendedAttrs = $user->getExtendedAttributes();
if (!isset($extendedAttrs['user_ldap_state']) || $extendedAttrs['user_ldap_state'] !== 'OK') {
  return; // abort because we couldn't get the LDAP attributes reliably
}

if (isset($extendedAttrs['user_ldap_attr_homephone'])) {
  printf("myUser's phone number is %s", $extendedAttrs['user_ldap_attr_homephone']);
} else {
  printf("myUser doesn't have a phone number");
}

Note that the information exposed isn't being used for now. It's expected that other apps document additional requirements if they need the admin to modify the user_ldap's configuration in order to expose specific attributes

jvillafanez commented 1 year ago

Ugly patch for core in order to help testing this (don't use it in production)

diff --git a/settings/Panels/Personal/Profile.php b/settings/Panels/Personal/Profile.php
index bd7caeb062..3b626771da 100644
--- a/settings/Panels/Personal/Profile.php
+++ b/settings/Panels/Personal/Profile.php
@@ -108,6 +108,7 @@ class Profile implements ISettings {
                $tmpl->assign('languageSelector', $selector->fetchPage());
                $tmpl->assign('legal_privacy_policy', $this->config->getAppValue('core', 'legal.privacy_policy_url'));
                $tmpl->assign('legal_imprint', $this->config->getAppValue('core', 'legal.imprint_url'));
+               $tmpl->assign('user_extended_attributes', $this->userSession->getUser()->getExtendedAttributes());
                return $tmpl;
        }

diff --git a/settings/templates/panels/personal/profile.php b/settings/templates/panels/personal/profile.php
index 52ae176183..8aecfafaf0 100644
--- a/settings/templates/panels/personal/profile.php
+++ b/settings/templates/panels/personal/profile.php
@@ -138,6 +138,13 @@ if ($_['passwordChangeSupported']) {
 <?php
 }
 ?>
+
+       <ul>
+       <?php foreach ($_['user_extended_attributes'] as $attr => $value): ?>
+       <li><?php p($attr); ?> : <?php p($value); ?></li>
+       <?php endforeach; ?>
+       </ul>
+
 <form id="language" class="section">
        <h2>
                <label><?php p($l->t('Language'));?></label>

Screenshot from 2023-08-09 11-39-46

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

52.0% 52.0% Coverage
0.0% 0.0% Duplication

jvillafanez commented 1 year ago

Looks OK. If the admin puts "rubbish" text into the textarea and saves, I wonder how a "reasonableness" check can be done to prevent saving obviously invalid crud?

I don't think we can do anything. I don't know if we could access to the ldap schema, but it sounds painful. Without the schema, we can't know what attributes are available. We could gather a list of all possible attributes, but we can't guarantee that there is no additional attribute outside of our list. Moreover, AD and openldap (for example) have a different set of attributes. In the end, providing something that works reasonable well seems too problematic.

jvillafanez commented 1 year ago

Minimum OC version for the app is already 10.11, which is the one we need for this feature to work, otherwise the event we need to listen to is missing and it could crash the app.

sonarcloud[bot] commented 11 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

52.0% 52.0% Coverage
0.0% 0.0% Duplication

pako81 commented 11 months ago

Looks ok to me.