Skip to content

Commit

Permalink
Update get_metadata to return {}
Browse files Browse the repository at this point in the history
Fixes bug 951093

While the actual issue was encountered in keystone/service.py,
the underlying issue is that all identity backends seems to be
returning None when no metadata is found for a user. I would argue
that returning {} makes it easier on clients.

Change-Id: I06faf755cc0dbe45b5d0a0f86c6235b27c856047
  • Loading branch information
Brian Lamar committed Mar 9, 2012
1 parent e05bc6a commit 6f2c858
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 9 deletions.
2 changes: 1 addition & 1 deletion keystone/identity/backends/kvs.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def get_user_by_name(self, user_name):
return _filter_user(self._get_user_by_name(user_name))

def get_metadata(self, user_id, tenant_id):
return self.db.get('metadata-%s-%s' % (tenant_id, user_id))
return self.db.get('metadata-%s-%s' % (tenant_id, user_id)) or {}

def get_role(self, role_id):
role_ref = self.db.get('role-%s' % role_id)
Expand Down
8 changes: 3 additions & 5 deletions keystone/identity/backends/ldap/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,11 @@ def get_user_by_name(self, user_name):
return _filter_user(user_ref)

def get_metadata(self, user_id, tenant_id):
if not self.get_tenant(tenant_id):
return None
if not self.get_user(user_id):
return None
if not self.get_tenant(tenant_id) or not self.get_user(user_id):
return {}

metadata_ref = self.get_roles_for_user_and_tenant(user_id, tenant_id)
return metadata_ref
return metadata_ref or {}

def get_role(self, role_id):
return self.role.get(role_id)
Expand Down
2 changes: 1 addition & 1 deletion keystone/identity/backends/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def get_metadata(self, user_id, tenant_id):
.filter_by(user_id=user_id)\
.filter_by(tenant_id=tenant_id)\
.first()
return getattr(metadata_ref, 'data', None)
return getattr(metadata_ref, 'data', {})

def get_role(self, role_id):
session = self.get_session()
Expand Down
4 changes: 4 additions & 0 deletions tests/default_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
USERS = [
{'id': 'foo', 'name': 'FOO', 'password': 'foo2', 'tenants': ['bar',]},
{'id': 'two', 'name': 'TWO', 'password': 'two2', 'tenants': ['baz',]},
{'id': 'no_meta',
'name': 'NO_META',
'password': 'no_meta2',
'tenants': ['baz',]},
]

METADATA = [
Expand Down
19 changes: 17 additions & 2 deletions tests/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,21 @@ def test_authenticate(self):
self.assertDictEquals(tenant_ref, self.tenant_bar)
self.assertDictEquals(metadata_ref, self.metadata_foobar)

def test_authenticate_no_metadata(self):
user = self.user_no_meta
tenant = self.tenant_baz
user_ref, tenant_ref, metadata_ref = self.identity_api.authenticate(
user_id=user['id'],
tenant_id=tenant['id'],
password=user['password'])
# NOTE(termie): the password field is left in user_foo to make it easier
# to authenticate in tests, but should not be returned by
# the api
user.pop('password')
self.assertEquals(metadata_ref, {})
self.assertDictEquals(user_ref, user)
self.assertDictEquals(tenant_ref, tenant)

def test_password_hashed(self):
user_ref = self.identity_api._get_user(self.user_foo['id'])
self.assertNotEqual(user_ref['password'], self.user_foo['password'])
Expand Down Expand Up @@ -117,13 +132,13 @@ def test_get_metadata_bad_user(self):
metadata_ref = self.identity_api.get_metadata(
user_id=self.user_foo['id'] + 'WRONG',
tenant_id=self.tenant_bar['id'])
self.assert_(metadata_ref is None)
self.assert_(metadata_ref == {})

def test_get_metadata_bad_tenant(self):
metadata_ref = self.identity_api.get_metadata(
user_id=self.user_foo['id'],
tenant_id=self.tenant_bar['id'] + 'WRONG')
self.assert_(metadata_ref is None)
self.assert_(metadata_ref == {})

def test_get_metadata(self):
metadata_ref = self.identity_api.get_metadata(
Expand Down

0 comments on commit 6f2c858

Please sign in to comment.