search

jtblin / go-ldap-client

Simple ldap client to authenticate, retrieve basic information and groups for a user.
Other
261 stars 90 forks source link

Fix connections left open and bind issue in GetGroupsOfUser #19

Open gtowey opened 5 years ago

gtowey commented 5 years ago

Two issues I've found while using this library:

  1. If you attempt to use GetGroupsOfUser without first calling Authenticate, you will not have a proper read-only connection to the LDAP server with with to perform a query. In my use-case, I'm not trying to authenticate a user (this is already done by another system). I just need to check the user's group membership to perform RBAC.

  2. The default behavior of this library is to make a connection and then leave it open. When that connection times out or is otherwise broken, all subsequent calls will fail with an error: LDAP Result Code 200 "Network Error": ldap: connection closed . Since the user of this library never calls Connect() it doesn't make sense for them to manage their own reconnection process. This change forces the library to close its connection after each method call. If it's necessary for callers of this library to have persistent connections for performance reasons then it should probably be refactored to remove Connect/Close out of the Authenticate and GetGroupsOfUser methods altogether and let callers handle their own connect/disconnect.

parkr commented 5 years ago

@jtblin Big 👍 to this!