Skip to content

Commit

Permalink
Close each LDAP connection after it is used,
Browse files Browse the repository at this point in the history
following python-ldap docs

Fixes bug 1172164

Change-Id: If20e3c1cf3deac5ce4217ce0324f8c91c2e30f92
  • Loading branch information
josecastroleon committed Sep 9, 2013
1 parent dac281a commit eedf321
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 11 deletions.
20 changes: 19 additions & 1 deletion keystone/assignment/backends/ldap.py
Expand Up @@ -240,18 +240,20 @@ def delete_group(self, group_id):
if not self.group.subtree_delete_enabled:
# TODO(spzala): this is only placeholder for group and domain
# role support which will be added under bug 1101287
conn = self.group.get_connection()
query = '(objectClass=%s)' % self.group.object_class
dn = None
dn = self.group._id_to_dn(id)
if dn:
try:
conn = self.group.get_connection()
roles = conn.search_s(dn, ldap.SCOPE_ONELEVEL,
query, ['%s' % '1.1'])
for role_dn, _ in roles:
conn.delete_s(role_dn)
except ldap.NO_SUCH_OBJECT:
pass
finally:
conn.unbind_s()


# TODO(termie): turn this into a data object and move logic to driver
Expand Down Expand Up @@ -309,6 +311,8 @@ def add_user(self, tenant_id, user_dn):
# places, and is not part of the exposed API, it's easier for us to
# just ignore this instead of raising exception.Conflict.
pass
finally:
conn.unbind_s()

def remove_user(self, tenant_id, user_dn, user_id):
conn = self.get_connection()
Expand All @@ -319,6 +323,8 @@ def remove_user(self, tenant_id, user_dn, user_id):
user_dn)])
except ldap.NO_SUCH_ATTRIBUTE:
raise exception.NotFound(user_id)
finally:
conn.unbind_s()

def get_user_dns(self, tenant_id, rolegrants, role_dn=None):
tenant = self._ldap_get(tenant_id)
Expand Down Expand Up @@ -407,6 +413,8 @@ def add_user(self, role_id, role_dn, user_dn, user_id, tenant_id=None):
conn.add_s(role_dn, attrs)
except Exception as inst:
raise inst
finally:
conn.unbind_s()

def delete_user(self, role_dn, user_dn, tenant_dn,
user_id, role_id):
Expand All @@ -428,6 +436,8 @@ def delete_user(self, role_dn, user_dn, tenant_dn,
raise inst
except ldap.NO_SUCH_ATTRIBUTE:
raise exception.UserNotFound(user_id=user_id)
finally:
conn.unbind_s()

def get_role_assignments(self, tenant_dn):
conn = self.get_connection()
Expand All @@ -437,6 +447,8 @@ def get_role_assignments(self, tenant_dn):
roles = conn.search_s(tenant_dn, ldap.SCOPE_ONELEVEL, query)
except ldap.NO_SUCH_OBJECT:
return []
finally:
conn.unbind_s()

res = []
for role_dn, attrs in roles:
Expand Down Expand Up @@ -471,6 +483,8 @@ def list_project_roles_for_user(self, user_dn, project_subtree):
query)
except ldap.NO_SUCH_OBJECT:
return []
finally:
conn.unbind_s()

res = []
for role_dn, _ in roles:
Expand Down Expand Up @@ -499,6 +513,8 @@ def roles_delete_subtree_by_project(self, tenant_dn):
raise inst
except ldap.NO_SUCH_OBJECT:
pass
finally:
conn.unbind_s()

def update(self, role_id, role):
try:
Expand All @@ -519,4 +535,6 @@ def delete(self, id, tenant_dn):
conn.delete_s(role_dn)
except ldap.NO_SUCH_OBJECT:
pass
finally:
conn.unbind_s()
super(RoleApi, self).delete(id)
41 changes: 33 additions & 8 deletions keystone/common/ldap/core.py
Expand Up @@ -243,12 +243,15 @@ def _id_to_dn(self, id):
if self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL:
return self._id_to_dn_string(id)
conn = self.get_connection()
search_result = conn.search_s(
self.tree_dn, self.LDAP_SCOPE,
'(&(%(id_attr)s=%(id)s)(objectclass=%(objclass)s))' %
{'id_attr': self.id_attr,
'id': ldap.filter.escape_filter_chars(str(id)),
'objclass': self.object_class})
try:
search_result = conn.search_s(
self.tree_dn, self.LDAP_SCOPE,
'(&(%(id_attr)s=%(id)s)(objectclass=%(objclass)s))' %
{'id_attr': self.id_attr,
'id': ldap.filter.escape_filter_chars(str(id)),
'objclass': self.object_class})
finally:
conn.unbind_s()
if search_result:
dn, attrs = search_result[0]
return dn
Expand Down Expand Up @@ -321,8 +324,10 @@ def create(self, values):

if 'groupOfNames' in object_classes and self.use_dumb_member:
attrs.append(('member', [self.dumb_member]))

conn.add_s(self._id_to_dn(values['id']), attrs)
try:
conn.add_s(self._id_to_dn(values['id']), attrs)
finally:
conn.unbind_s()
return values

def _ldap_get(self, id, filter=None):
Expand All @@ -340,6 +345,8 @@ def _ldap_get(self, id, filter=None):
res = conn.search_s(self.tree_dn, self.LDAP_SCOPE, query, attrs)
except ldap.NO_SUCH_OBJECT:
return None
finally:
conn.unbind_s()
try:
return res[0]
except IndexError:
Expand All @@ -356,6 +363,8 @@ def _ldap_get_all(self, filter=None):
self.attribute_mapping.values())
except ldap.NO_SUCH_OBJECT:
return []
finally:
conn.unbind_s()

def get(self, id, filter=None):
res = self._ldap_get(id, filter)
Expand Down Expand Up @@ -411,6 +420,8 @@ def update(self, id, values, old_obj=None):
conn.modify_s(self._id_to_dn(id), modlist)
except ldap.NO_SUCH_OBJECT:
raise self._not_found(id)
finally:
conn.unbind_s()

return self.get(id)

Expand All @@ -424,6 +435,8 @@ def delete(self, id):
conn.delete_s(self._id_to_dn(id))
except ldap.NO_SUCH_OBJECT:
raise self._not_found(id)
finally:
conn.unbind_s()

def deleteTree(self, id):
conn = self.get_connection()
Expand All @@ -435,6 +448,8 @@ def deleteTree(self, id):
serverctrls=[tree_delete_control])
except ldap.NO_SUCH_OBJECT:
raise self._not_found(id)
finally:
conn.unbind_s()


class LdapWrapper(object):
Expand Down Expand Up @@ -511,6 +526,10 @@ def simple_bind_s(self, user, password):
LOG.debug(_("LDAP bind: dn=%s"), user)
return self.conn.simple_bind_s(user, password)

def unbind_s(self):
LOG.debug("LDAP unbind")
return self.conn.unbind_s()

def add_s(self, dn, attrs):
ldap_attrs = [(kind, [py2ldap(x) for x in safe_iter(values)])
for kind, values in attrs]
Expand Down Expand Up @@ -651,6 +670,8 @@ def _get_enabled(self, object_id):
return False
else:
return bool(enabled_value)
finally:
conn.unbind_s()

def _add_enabled(self, object_id):
if not self._get_enabled(object_id):
Expand All @@ -667,6 +688,8 @@ def _add_enabled(self, object_id):
if self.use_dumb_member:
attr_list[1][1].append(self.dumb_member)
conn.add_s(self.enabled_emulation_dn, attr_list)
finally:
conn.unbind_s()

def _remove_enabled(self, object_id):
conn = self.get_connection()
Expand All @@ -677,6 +700,8 @@ def _remove_enabled(self, object_id):
conn.modify_s(self.enabled_emulation_dn, modlist)
except (ldap.NO_SUCH_OBJECT, ldap.NO_SUCH_ATTRIBUTE):
pass
finally:
conn.unbind_s()

def create(self, values):
if self.enabled_emulation:
Expand Down
15 changes: 13 additions & 2 deletions keystone/identity/backends/ldap.py
Expand Up @@ -63,13 +63,17 @@ def authenticate(self, user_id, password):
raise AssertionError('Invalid user / password')
if not user_id or not password:
raise AssertionError('Invalid user / password')
conn = None
try:
conn = self.user.get_connection(self.user._id_to_dn(user_id),
password)
if not conn:
raise AssertionError('Invalid user / password')
except Exception:
raise AssertionError('Invalid user / password')
finally:
if conn:
conn.unbind_s()
return identity.filter_user(user_ref)

def _get_user(self, user_id):
Expand Down Expand Up @@ -286,19 +290,20 @@ def delete(self, id):
# TODO(spzala): this is only placeholder for group and domain
# role support which will be added under bug 1101287

conn = self.get_connection()
query = '(objectClass=%s)' % self.object_class
dn = None
dn = self._id_to_dn(id)
if dn:
try:
conn = self.get_connection()
roles = conn.search_s(dn, ldap.SCOPE_ONELEVEL,
query, ['%s' % '1.1'])
for role_dn, _ in roles:
conn.delete_s(role_dn)
except ldap.NO_SUCH_OBJECT:
pass

finally:
conn.unbind_s()
super(GroupApi, self).delete(id)

def update(self, id, values):
Expand All @@ -317,6 +322,8 @@ def add_user(self, user_dn, group_id, user_id):
raise exception.Conflict(_(
'User %(user_id)s is already a member of group %(group_id)s') %
{'user_id': user_id, 'group_id': group_id})
finally:
conn.unbind_s()

def remove_user(self, user_dn, group_id, user_id):
conn = self.get_connection()
Expand All @@ -328,6 +335,8 @@ def remove_user(self, user_dn, group_id, user_id):
user_dn)])
except ldap.NO_SUCH_ATTRIBUTE:
raise exception.UserNotFound(user_id=user_id)
finally:
conn.unbind_s()

def list_user_groups(self, user_dn):
"""Return a list of groups for which the user is a member."""
Expand All @@ -350,6 +359,8 @@ def list_group_users(self, group_id):
query, ['%s' % self.member_attribute])
except ldap.NO_SUCH_OBJECT:
return []
finally:
conn.unbind_s()
users = []
for dn, member in attrs:
user_dns = member[self.member_attribute]
Expand Down

0 comments on commit eedf321

Please sign in to comment.