openstack/keystone
Code Issues Proposed changes

unique role name constraint

Browse Source 
For SQL identity backend, add unique constraint with column definition;
for kvs and ldap backend, use python code to apply this constraint.
Test cases test_create_duplicate_role_name_fails and test_rename_duplicate_role_name_fails are added to guard it.
python run_tests.py test_backend_ldap test_backend_kvs test_backend_sql pass.

bug 932258.

Change-Id: I990f17a270e84d35c078f215c587a81d6784c192
This commit is contained in:
Yong Sheng Gong 2012-03-18 23:56:35 +08:00 committed by Dolph Mathews
parent f98cd4f27d
commit d61aedaf86
5 changed files with 66 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
AUTHORS
View File

@ -100,6 +100,7 @@ vishvananda <vishvananda@gmail.com>
Will Kelly <the.william.kelly@gmail.com>
Yaguang Tang <heut2008@gmail.com>
Yogeshwar Srikrishnan <yoga80@yahoo.com>
Yong Sheng Gong <gongysh@cn.ibm.com>
Yun Mao <yunmao@gmail.com>
Yuriy Taraday <yorik.sar@gmail.com>
Zhongyue Luo <lzyeval@gmail.com>

23
keystone/identity/backends/kvs.py
View File

@ -237,6 +237,15 @@ class Identity(kvs.Base, identity.Driver):
return None
def create_role(self, role_id, role):
role_ref = self.get_role(role_id)
if role_ref:
msg = 'Duplicate ID, %s.' % role_id
raise exception.Conflict(type='role', details=msg)
role_refs = self.list_roles()
for role_ref in role_refs:
if role['name'] == role_ref['name']:
msg = 'Duplicate name, %s.' % role['name']
raise exception.Conflict(type='role', details=msg)
self.db.set('role-%s' % role_id, role)
role_list = set(self.db.get('role_list', []))
role_list.add(role_id)
@ -244,7 +253,19 @@ class Identity(kvs.Base, identity.Driver):
return role
def update_role(self, role_id, role):
self.db.set('role-%s' % role_id, role)
role_refs = self.list_roles()
old_role_ref = None
for role_ref in role_refs:
if role['name'] == role_ref['name'] and role_id != role_ref['id']:
msg = 'Duplicate name, %s.' % role['name']
raise exception.Conflict(type='role', details=msg)
if role_id == role_ref['id']:
old_role_ref = role_ref
if old_role_ref:
role['id'] = role_id
self.db.set('role-%s' % role_id, role)
else:
raise exception.RoleNotFound(role_id=role_id)
return role
def delete_role(self, role_id):

14
keystone/identity/backends/ldap/core.py
View File

@ -176,6 +176,12 @@ class Identity(identity.Driver):
return {}
def create_role(self, role_id, role):
if self.get_role(role_id):
msg = 'Duplicate ID, %s.' % role_id
raise exception.Conflict(type='role', details=msg)
if self.role.get_by_name(role['name']):
msg = 'Duplicate name, %s.' % role['name']
raise exception.Conflict(type='role', details=msg)
return self.role.create(role)
def delete_role(self, role_id):
@ -540,7 +546,13 @@ class RoleApi(common_ldap.BaseLdap, ApiShimMixin):
# pylint: disable=W0221
def get_by_name(self, name, filter=None):
return self.get(name, filter)
roles = self.get_all('(%s=%s)' %
(self.attribute_mapping['name'],
ldap_filter.escape_filter_chars(name)))
try:
return roles[0]
except IndexError:
return None
def add_user(self, role_id, user_id, tenant_id=None):
user = self.user_api.get(user_id)

2
keystone/identity/backends/sql.py
View File

@ -104,7 +104,7 @@ class Tenant(sql.ModelBase, sql.DictBase):
class Role(sql.ModelBase, sql.DictBase):
__tablename__ = 'role'
id = sql.Column(sql.String(64), primary_key=True)
name = sql.Column(sql.String(64))
name = sql.Column(sql.String(64), unique=True)
class Metadata(sql.ModelBase, sql.DictBase):

35
tests/test_backend.py
View File

@ -151,6 +151,29 @@ class IdentityTests(object):
role_id=self.role_keystone_admin['id'])
self.assertDictEquals(role_ref, self.role_keystone_admin)
def test_create_duplicate_role_name_fails(self):
role = {'id': 'fake1',
'name': 'fake1name'}
self.identity_api.create_role('fake1', role)
role['id'] = 'fake2'
self.assertRaises(exception.Conflict,
self.identity_api.create_role,
'fake2',
role)
def test_rename_duplicate_role_name_fails(self):
role1 = {'id': 'fake1',
'name': 'fake1name'}
role2 = {'id': 'fake2',
'name': 'fake2name'}
self.identity_api.create_role('fake1', role1)
self.identity_api.create_role('fake2', role2)
role1['name'] = 'fake2name'
self.assertRaises(exception.Error,
self.identity_api.update_role,
'fake1',
role1)
def test_create_duplicate_user_id_fails(self):
user = {'id': 'fake1',
'name': 'fake1',
@ -158,7 +181,7 @@ class IdentityTests(object):
'tenants': ['bar']}
self.identity_api.create_user('fake1', user)
user['name'] = 'fake2'
self.assertRaises(Exception,
self.assertRaises(exception.Conflict,
self.identity_api.create_user,
'fake1',
user)
@ -170,7 +193,7 @@ class IdentityTests(object):
'tenants': ['bar']}
self.identity_api.create_user('fake1', user)
user['id'] = 'fake2'
self.assertRaises(Exception,
self.assertRaises(exception.Conflict,
self.identity_api.create_user,
'fake2',
user)
@ -187,7 +210,7 @@ class IdentityTests(object):
self.identity_api.create_user('fake1', user1)
self.identity_api.create_user('fake2', user2)
user2['name'] = 'fake1'
self.assertRaises(Exception,
self.assertRaises(exception.Error,
self.identity_api.update_user,
'fake2',
user2)
@ -209,7 +232,7 @@ class IdentityTests(object):
tenant = {'id': 'fake1', 'name': 'fake1'}
self.identity_api.create_tenant('fake1', tenant)
tenant['name'] = 'fake2'
self.assertRaises(Exception,
self.assertRaises(exception.Conflict,
self.identity_api.create_tenant,
'fake1',
tenant)
@ -218,7 +241,7 @@ class IdentityTests(object):
tenant = {'id': 'fake1', 'name': 'fake'}
self.identity_api.create_tenant('fake1', tenant)
tenant['id'] = 'fake2'
self.assertRaises(Exception,
self.assertRaises(exception.Conflict,
self.identity_api.create_tenant,
'fake1',
tenant)
@ -229,7 +252,7 @@ class IdentityTests(object):
self.identity_api.create_tenant('fake1', tenant1)
self.identity_api.create_tenant('fake2', tenant2)
tenant2['name'] = 'fake1'
self.assertRaises(Exception,
self.assertRaises(exception.Error,
self.identity_api.update_tenant,
'fake2',
tenant2)