diff --git a/src/metabase/integrations/ldap/default_implementation.clj b/src/metabase/integrations/ldap/default_implementation.clj
index 0e43088938f7c5451b73a1a009ac0b0ec2a007e1..0adf998168c9140382df1d5cb395a28eaf090b98 100644
--- a/src/metabase/integrations/ldap/default_implementation.clj
+++ b/src/metabase/integrations/ldap/default_implementation.clj
@@ -67,7 +67,7 @@
:groups (when sync-groups?
;; Active Directory and others (like FreeIPA) will supply a `memberOf` overlay attribute for
;; groups. Otherwise we have to make the inverse query to get them.
- (or (:memberof result)
+ (or (u/one-or-many (:memberof result))
(user-groups ldap-connection dn settings)
[]))})))
diff --git a/test/metabase/integrations/ldap_test.clj b/test/metabase/integrations/ldap_test.clj
index 104ec65229551b76819f4d4621effe9963f6b4bf..9e6b58adedccabf53e39c55fdf16661c3fab5bc3 100644
--- a/test/metabase/integrations/ldap_test.clj
+++ b/test/metabase/integrations/ldap_test.clj
@@ -96,7 +96,26 @@
:last-name "Taylor"
:email "fred.taylor@metabase.com"
:groups []}
- (ldap/find-user "fred.taylor@metabase.com")))))))
+ (ldap/find-user "fred.taylor@metabase.com")))))
+
+ ;; Test group lookups for directory servers that use the memberOf attribute overlay, such as Active Directory
+ (ldap.test/with-active-directory-ldap-server
+ (testing "find user with one group using memberOf attribute"
+ (is (= {:dn "cn=John Smith,ou=People,dc=metabase,dc=com"
+ :first-name "John"
+ :last-name "Smith"
+ :email "John.Smith@metabase.com"
+ :groups ["cn=Accounting,ou=Groups,dc=metabase,dc=com"]}
+ (ldap/find-user "jsmith1"))))
+
+ (testing "find user with two groups using memberOf attribute"
+ (is (= {:dn "cn=Sally Brown,ou=People,dc=metabase,dc=com"
+ :first-name "Sally"
+ :last-name "Brown"
+ :email "sally.brown@metabase.com"
+ :groups ["cn=Accounting,ou=Groups,dc=metabase,dc=com",
+ "cn=Engineering,ou=Groups,dc=metabase,dc=com"]}
+ (ldap/find-user "sbrown20")))))))
(deftest group-matching-test
(testing "LDAP group matching should identify Metabase groups using DN equality rules"
diff --git a/test/metabase/test/integrations/ldap.clj b/test/metabase/test/integrations/ldap.clj
index bacbc0ff273d215538a706f19fa7f3db946f225d..2340e5f2044228508eca3ed130f62dfff1023f1a 100644
--- a/test/metabase/test/integrations/ldap.clj
+++ b/test/metabase/test/integrations/ldap.clj
@@ -10,18 +10,19 @@
"An in-memory LDAP testing server."
nil)
-(defn- get-server-config []
+(defn- get-server-config
+ [schema]
(doto (InMemoryDirectoryServerConfig. (into-array String ["dc=metabase,dc=com"]))
(.addAdditionalBindCredentials "cn=Directory Manager" "password")
- (.setSchema (Schema/getDefaultStandardSchema))
+ (.setSchema schema)
(.setListenerConfigs (into-array InMemoryListenerConfig [(InMemoryListenerConfig/createLDAPConfig "LDAP" 0)]))))
-(defn- start-ldap-server! []
- (with-open [ldif (LDIFReader. (or (io/file (io/resource "ldap.ldif"))
- (io/file "test_resources/ldap.ldif")
+(defn- start-ldap-server!
+ [{:keys [ldif-resource schema]}]
+ (with-open [ldif (LDIFReader. (or (io/file (str "test_resources/" ldif-resource))
(throw
- (FileNotFoundException. "ldap.ldif does not exist!"))))]
- (doto (InMemoryDirectoryServer. (get-server-config))
+ (FileNotFoundException. (str ldif-resource " does not exist!")))))]
+ (doto (InMemoryDirectoryServer. (get-server-config schema))
(.importFromLDIF true ldif)
(.startListening))))
@@ -30,10 +31,15 @@
[]
(.getListenPort *ldap-server*))
+(defn get-default-schema
+ "Get the default schema for the directory server."
+ []
+ (Schema/getDefaultStandardSchema))
+
(defn do-with-ldap-server
"Bind `*ldap-server*` and the relevant settings to an in-memory LDAP testing server and executes `f`."
- [f]
- (binding [*ldap-server* (start-ldap-server!)]
+ [f options]
+ (binding [*ldap-server* (start-ldap-server! options)]
(try
(tu/with-temporary-setting-values [ldap-enabled true
ldap-host "localhost"
@@ -49,4 +55,14 @@
(defmacro with-ldap-server
"Bind `*ldap-server*` and the relevant settings to an in-memory LDAP testing server and executes `body`."
[& body]
- `(do-with-ldap-server (fn [] ~@body)))
+ `(do-with-ldap-server (fn [] ~@body)
+ {:ldif-resource "ldap.ldif"
+ :schema (get-default-schema)}))
+
+(defmacro with-active-directory-ldap-server
+ "Bind `*ldap-server*` and the relevant settings to an in-memory LDAP testing server and executes `body`.
+ This version of the macro uses options that simulate an Active Directory server with memberOf attributes."
+ [& body]
+ `(do-with-ldap-server (fn [] ~@body)
+ {:ldif-resource "active_directory.ldif"
+ :schema nil}))
diff --git a/test_resources/active_directory.ldif b/test_resources/active_directory.ldif
new file mode 100644
index 0000000000000000000000000000000000000000..2d02c3233cf9199948a813792b6d1874f2d832c9
--- /dev/null
+++ b/test_resources/active_directory.ldif
@@ -0,0 +1,56 @@
+dn: dc=metabase,dc=com
+objectClass: top
+objectClass: organization
+objectClass: dcObject
+dc: metabase
+o: Metabase Corporation
+
+dn: ou=People,dc=metabase,dc=com
+objectClass: top
+objectClass: organizationalUnit
+ou: People
+
+dn: cn=John Smith,ou=People,dc=metabase,dc=com
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: John Smith
+sn: Smith
+givenName: John
+uid: jsmith1
+mail: John.Smith@metabase.com
+userPassword: strongpassword
+memberOf: cn=Accounting,ou=Groups,dc=metabase,dc=com
+
+dn: cn=Sally Brown,ou=People,dc=metabase,dc=com
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Sally Brown
+sn: Brown
+givenName: Sally
+uid: sbrown20
+mail: sally.brown@metabase.com
+userPassword: 1234
+memberOf: cn=Accounting,ou=Groups,dc=metabase,dc=com
+memberOf: cn=Engineering,ou=Groups,dc=metabase,dc=com
+
+dn: ou=Groups,dc=metabase,dc=com
+objectClass: top
+objectClass: organizationalUnit
+ou: Groups
+
+dn: cn=Accounting,ou=Groups,dc=metabase,dc=com
+objectClass: top
+objectClass: groupOfNames
+cn: Accounting
+member: cn=John Smith,ou=People,dc=metabase,dc=com
+member: cn=Sally Brown,ou=People,dc=metabase,dc=com
+
+dn: cn=Engineering,ou=Groups,dc=metabase,dc=com
+objectClass: top
+objectClass: groupOfNames
+cn: Accounting
+member: cn=Sally Brown,ou=People,dc=metabase,dc=com