Skip to content

Commit

Permalink
Fixes for user response with LDAP user_enabled_mask
Browse files Browse the repository at this point in the history
This fixes a couple of problems with the returned user object
when the Keystone server when the LDAP Identity backend is
configured and the user_enabled_mask setting is not zero.

First, the returned user had the masked value (e.g., 512) when
a user was created rather than the actual enabled value (true
or false).

Second, the Keystone server was returning the enabled_nomask
attribute in the user entry when users were listed or updated.

This change makes it so that the server returns the enabled
value as a Boolean when a user is created. Also, the
enabled_nomask attribute is not returned when users are
listed or updated.

Change-Id: I8a22aec3377ffd335095e645c1b3d65008dd0cf2
Closes-bug: #1220945
  • Loading branch information
Brant Knudson committed Sep 9, 2013
1 parent 573ba45 commit 507dfd4
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
5 changes: 3 additions & 2 deletions keystone/identity/backends/ldap.py
Expand Up @@ -114,7 +114,6 @@ def update_user(self, user_id, user):

user = utils.hash_ldap_user_password(user)
if self.user.enabled_mask:
user['enabled_nomask'] = old_obj['enabled_nomask']
self.user.mask_enabled_attribute(user)
self.user.update(user_id, user, old_obj)
return self.user.get_filtered(user_id)
Expand Down Expand Up @@ -219,7 +218,6 @@ def _ldap_res_to_model(self, res):
obj = super(UserApi, self)._ldap_res_to_model(res)
if self.enabled_mask != 0:
enabled = int(obj.get('enabled', self.enabled_default))
obj['enabled_nomask'] = enabled
obj['enabled'] = ((enabled & self.enabled_mask) !=
self.enabled_mask)
return obj
Expand All @@ -236,8 +234,11 @@ def mask_enabled_attribute(self, values):
def create(self, values):
values = utils.hash_ldap_user_password(values)
if self.enabled_mask:
orig_enabled = values['enabled']
self.mask_enabled_attribute(values)
values = super(UserApi, self).create(values)
if self.enabled_mask:
values['enabled'] = orig_enabled
return values

def check_password(self, user_id, password):
Expand Down
11 changes: 9 additions & 2 deletions keystone/tests/test_backend_ldap.py
Expand Up @@ -571,34 +571,41 @@ def get_enabled_vals():

user_ref = self.identity_api.create_user('fake1', user)

self.assertEqual(user_ref['enabled'], 512)
# TODO(blk-u): 512 seems wrong, should it be True?
# Use assertIs rather than assertTrue because assertIs will assert the
# value is a Boolean as expected.
self.assertIs(user_ref['enabled'], True)
self.assertNotIn('enabled_nomask', user_ref)

enabled_vals = get_enabled_vals()
self.assertEqual(enabled_vals, [512])

user_ref = self.identity_api.get_user('fake1')
self.assertIs(user_ref['enabled'], True)
self.assertNotIn('enabled_nomask', user_ref)

user['enabled'] = False
user_ref = self.identity_api.update_user('fake1', user)
self.assertIs(user_ref['enabled'], False)
self.assertNotIn('enabled_nomask', user_ref)

enabled_vals = get_enabled_vals()
self.assertEqual(enabled_vals, [514])

user_ref = self.identity_api.get_user('fake1')
self.assertIs(user_ref['enabled'], False)
self.assertNotIn('enabled_nomask', user_ref)

user['enabled'] = True
user_ref = self.identity_api.update_user('fake1', user)
self.assertIs(user_ref['enabled'], True)
self.assertNotIn('enabled_nomask', user_ref)

enabled_vals = get_enabled_vals()
self.assertEqual(enabled_vals, [512])

user_ref = self.identity_api.get_user('fake1')
self.assertIs(user_ref['enabled'], True)
self.assertNotIn('enabled_nomask', user_ref)

def test_user_api_get_connection_no_user_password(self):
"""Don't bind in case the user and password are blank."""
Expand Down

0 comments on commit 507dfd4

Please sign in to comment.