LDAP: Properly handle multi-value attributes

src/main/java/io/kamax/mxisd/backend/ldap/LdapAuthProvider.java

@@ -46,6 +46,7 @@ import org.springframework.stereotype.Component;
 
 import java.io.IOException;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 
@@ -133,14 +134,20 @@ public class LdapAuthProvider extends LdapGenericBackend implements Authenticato
                     // TODO should we canonicalize the MXID?
                     BackendAuthResult result = BackendAuthResult.success(mxid.getId(), UserIdType.MatrixID, name);
                     log.info("Processing 3PIDs for profile");
-                    getAt().getThreepid().forEach((k, v) -> v.forEach(attId -> {
+                    getAt().getThreepid().forEach((k, v) -> {
-                        getAttribute(entry, attId).ifPresent(tpidValue -> {
+                        log.info("Processing 3PID type {}", k);
-                            if (ThreePidMedium.PhoneNumber.is(k)) {
+                        v.forEach(attId -> {
-                                tpidValue = getMsisdn(tpidValue).orElse(tpidValue);
+                            List<String> values = getAttributes(entry, attId);
-                            }
+                            log.info("\tAttribute {} has {} value(s)", attId, values.size());
-                            result.withThreePid(new ThreePid(k, tpidValue));
+                            getAttributes(entry, attId).forEach(tpidValue -> {
+                                if (ThreePidMedium.PhoneNumber.is(k)) {
+                                    tpidValue = getMsisdn(tpidValue).orElse(tpidValue);
+                                }
+                                result.withThreePid(new ThreePid(k, tpidValue));
+                            });
                         });
-                    }));
+                    });
+
                     log.info("Found {} 3PIDs", result.getProfile().getThreePids().size());
                     return result;
                 }

src/main/java/io/kamax/mxisd/backend/ldap/LdapGenericBackend.java

@@ -25,6 +25,7 @@ import io.kamax.mxisd.config.ldap.LdapAttributeConfig;
 import io.kamax.mxisd.config.ldap.LdapConfig;
 import org.apache.commons.lang.StringUtils;
 import org.apache.directory.api.ldap.model.entry.Attribute;
+import org.apache.directory.api.ldap.model.entry.AttributeUtils;
 import org.apache.directory.api.ldap.model.entry.Entry;
 import org.apache.directory.api.ldap.model.exception.LdapException;
 import org.apache.directory.ldap.client.api.LdapConnection;
@@ -32,6 +33,9 @@ import org.apache.directory.ldap.client.api.LdapNetworkConnection;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
@@ -124,7 +128,6 @@ public abstract class LdapGenericBackend {
     public Optional<String> getAttribute(Entry entry, String attName) {
         Attribute attribute = entry.get(attName);
         if (attribute == null) {
-            log.info("DN {}: no attribute {}, skipping", entry.getDn(), attName);
             return Optional.empty();
         }
 
@@ -137,4 +140,22 @@ public abstract class LdapGenericBackend {
         return Optional.of(value);
     }
 
+    public List<String> getAttributes(Entry entry, String attName) {
+        List<String> values = new ArrayList<>();
+        javax.naming.directory.Attribute att = AttributeUtils.toAttributes(entry).get(attName);
+        if (att == null) {
+            return values;
+        }
+
+        try {
+            NamingEnumeration<?> list = att.getAll();
+            while (list.hasMore()) {
+                values.add(list.next().toString());
+            }
+        } catch (NamingException e) {
+            log.warn("Error while processing LDAP attribute {}, result could be incomplete!", attName, e);
+        }
+        return values;
+    }
+
 }