sismics / docs

Lightweight document management system packed with all the features you can expect from big expensive solutions
https://teedy.io
GNU General Public License v2.0
1.93k stars 486 forks source link

add explicit binding #735

Closed Enrice closed 10 months ago

Enrice commented 11 months ago

needed for Active Directory, fixes #734

jendib commented 11 months ago

@Enrice Answering your question, I don't think it breaks the "normal" LDAP auth either but it would be great if this connector is covered by a unit test. A quick search gives me this https://github.com/zapodot/embedded-ldap-junit or we could use testcontainers, it's worth digging into it if you have time.

Enrice commented 11 months ago

I don't see how your above mentioned library could help here. I played around a bit with it, but this library is based on UnboundID LDAP SDK for Java, whereas the teedy implementation is based on Apache Directory LDAP API.

Enrice commented 11 months ago

Let me clarify: According to https://stackoverflow.com/questions/18613622/why-does-ldap-requires-a-two-step-login-connect-and-then-bind for example, LDAP connections NEED binding, if unauthenticated connections are not authorized for searching (which is the first operation in the case of the current teedy implementation). This is actually the case with my Active Directory LDAPS connection, anonymous searches are not allowed.

The connection configuration actually sets the configured credentials (called LDAP_ADMIN_DN and LDAP_ADMIN_PASSWORD; they actually don't have to have admin role however, so these names may be misleading) in the connection (which is good), but they will never be used when no binding is done!

So my fix IS necessary.

The actual binding of the found user (if so) is actually a rebind then.

Enrice commented 11 months ago

I will try to write a unit test.