openstack/keystone
Code Issues Proposed changes

Fix assignment to not require user or group existence

Browse Source 
The assignment code was requiring the existence of users and
groups before performing different operations. With federation,
the users and groups may legitimately not be defined in keystone,
so these checks cannot be done.

Implements: bp no-check-id
Change-Id: Ib9d0188ae2a436617bad90bf936e7b3dbf296885
This commit is contained in:
Brant Knudson 2013-12-05 16:56:26 -06:00
parent 4fc6e97097
commit ab1b0c283b
7 changed files with 21 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
keystone/assignment/backends/kvs.py
View File

@ -109,7 +109,6 @@ class Assignment(kvs.Base, assignment.Driver):
return [self.get_project(x) for x in user_ref.get('tenants', [])]
def add_role_to_user_and_project(self, user_id, tenant_id, role_id):
self.identity_api.get_user(user_id)
self.get_project(tenant_id)
self.get_role(role_id)
try:
@ -375,10 +374,6 @@ class Assignment(kvs.Base, assignment.Driver):
def list_grants(self, user_id=None, group_id=None,
domain_id=None, project_id=None,
inherited_to_projects=False):
if user_id:
self.identity_api.get_user(user_id)
if group_id:
self.identity_api.get_group(group_id)
if domain_id:
self.get_domain(domain_id)
if project_id:
@ -398,8 +393,6 @@ class Assignment(kvs.Base, assignment.Driver):
domain_id=None, project_id=None,
inherited_to_projects=False):
self.get_role(role_id)
if user_id:
self.identity_api.get_user(user_id)
if group_id:
self.get_group(group_id)
if domain_id:
@ -424,10 +417,6 @@ class Assignment(kvs.Base, assignment.Driver):
domain_id=None, project_id=None,
inherited_to_projects=False):
self.get_role(role_id)
if user_id:
self.identity_api.get_user(user_id)
if group_id:
self.identity_api.get_group(group_id)
if domain_id:
self.get_domain(domain_id)
if project_id:

12
keystone/assignment/backends/ldap.py
View File

@ -21,7 +21,6 @@ import ldap as ldap
from keystone import assignment
from keystone import clean
from keystone.common import dependency
from keystone.common import driver_hints
from keystone.common import ldap as common_ldap
from keystone.common import models
@ -35,7 +34,6 @@ CONF = config.CONF
LOG = log.getLogger(__name__)
@dependency.requires('identity_api')
class Assignment(assignment.Driver):
def __init__(self):
super(Assignment, self).__init__()
@ -88,7 +86,6 @@ class Assignment(assignment.Driver):
domain_id=None, group_id=None):
def _get_roles_for_just_user_and_project(user_id, tenant_id):
self.identity_api.get_user(user_id)
self.get_project(tenant_id)
return [self.role._dn_to_id(a.role_dn)
for a in self.role.get_role_assignments
@ -96,7 +93,6 @@ class Assignment(assignment.Driver):
if self.user._dn_to_id(a.user_dn) == user_id]
def _get_roles_for_group_and_project(group_id, project_id):
self.identity_api.get_group(group_id)
self.get_project(project_id)
group_dn = self.group._id_to_dn(group_id)
# NOTE(marcos-fermin-lobo): In Active Directory, for functions
@ -138,7 +134,6 @@ class Assignment(assignment.Driver):
# NOTE(henry-nash): The LDAP backend is being deprecated, so no
# support is provided for projects that the user has a role on solely
# by virtue of group membership.
self.identity_api.get_user(user_id)
user_dn = self.user._id_to_dn(user_id)
associations = (self.role.list_project_roles_for_user
(user_dn, self.project.tree_dn))
@ -164,7 +159,6 @@ class Assignment(assignment.Driver):
self.project._id_to_dn(tenant_id))
def add_role_to_user_and_project(self, user_id, tenant_id, role_id):
self.identity_api.get_user(user_id)
self.get_project(tenant_id)
self.get_role(role_id)
user_dn = self.user._id_to_dn(user_id)
@ -176,7 +170,6 @@ class Assignment(assignment.Driver):
tenant_dn=tenant_dn)
def _add_role_to_group_and_project(self, group_id, tenant_id, role_id):
self.identity_api.get_group(group_id)
self.get_project(tenant_id)
self.get_role(role_id)
group_dn = self.group._id_to_dn(group_id)
@ -348,11 +341,6 @@ class Assignment(assignment.Driver):
def delete_grant(self, role_id, user_id=None, group_id=None,
domain_id=None, project_id=None,
inherited_to_projects=False):
if user_id:
self.identity_api.get_user(user_id)
if group_id:
self.identity_api.get_group(group_id)
self.get_role(role_id)
if domain_id:

4
keystone/assignment/backends/sql.py
View File

@ -16,7 +16,6 @@
from keystone import assignment
from keystone import clean
from keystone.common import dependency
from keystone.common import sql
from keystone.common.sql import migration
from keystone import config
@ -27,7 +26,6 @@ from keystone.openstack.common.db.sqlalchemy import session as db_session
CONF = config.CONF
@dependency.requires('identity_api')
class Assignment(sql.Base, assignment.Driver):
# Internal interface to manage the database
@ -303,8 +301,6 @@ class Assignment(sql.Base, assignment.Driver):
return _project_ids_to_dicts(session, project_ids)
def add_role_to_user_and_project(self, user_id, tenant_id, role_id):
self.identity_api.get_user(user_id)
with sql.transaction() as session:
self._get_project(session, tenant_id)
self._get_role(session, role_id)

14
keystone/assignment/controllers.py
View File

@ -174,7 +174,7 @@ class Tenant(controller.V2Controller):
return o
@dependency.requires('assignment_api', 'identity_api')
@dependency.requires('assignment_api')
class Role(controller.V2Controller):
# COMPAT(essex-3)
@ -275,8 +275,6 @@ class Role(controller.V2Controller):
"""
self.assert_admin(context)
# Ensure user exists by getting it first.
self.identity_api.get_user(user_id)
tenants = self.assignment_api.list_projects_for_user(user_id)
o = []
for tenant in tenants:
@ -509,11 +507,6 @@ class RoleV3(controller.V3Controller):
self._require_domain_xor_project(domain_id, project_id)
self._require_user_xor_group(user_id, group_id)
if user_id:
self.identity_api.get_user(user_id)
if group_id:
self.identity_api.get_group(group_id)
self.assignment_api.create_grant(
role_id, user_id, group_id, domain_id, project_id,
self._check_if_inherited(context))
@ -537,11 +530,6 @@ class RoleV3(controller.V3Controller):
self._require_domain_xor_project(domain_id, project_id)
self._require_user_xor_group(user_id, group_id)
if user_id:
self.identity_api.get_user(user_id)
if group_id:
self.identity_api.get_group(group_id)
self.assignment_api.get_grant(
role_id, user_id, group_id, domain_id, project_id,
self._check_if_inherited(context))

23
keystone/tests/test_backend.py
View File

@ -723,12 +723,6 @@ class IdentityTests(object):
uuid.uuid4().hex)
def test_add_role_to_user_and_project_404(self):
self.assertRaises(exception.UserNotFound,
self.assignment_api.add_role_to_user_and_project,
uuid.uuid4().hex,
self.tenant_bar['id'],
self.role_admin['id'])
self.assertRaises(exception.ProjectNotFound,
self.assignment_api.add_role_to_user_and_project,
self.user_foo['id'],
@ -741,6 +735,13 @@ class IdentityTests(object):
self.tenant_bar['id'],
uuid.uuid4().hex)
def test_add_role_to_user_and_project_no_user(self):
# If add_role_to_user_and_project and the user doesn't exist, then
# no error.
user_id_not_exist = uuid.uuid4().hex
self.assignment_api.add_role_to_user_and_project(
user_id_not_exist, self.tenant_bar['id'], self.role_admin['id'])
def test_remove_role_from_user_and_project(self):
self.assignment_api.add_role_to_user_and_project(
self.user_foo['id'], self.tenant_bar['id'], 'member')
@ -1567,10 +1568,12 @@ class IdentityTests(object):
uuid.uuid4().hex,
self.user_foo['id'])
self.assertRaises(exception.UserNotFound,
self.assignment_api.add_user_to_project,
self.tenant_bar['id'],
uuid.uuid4().hex)
def test_add_user_to_project_no_user(self):
# If add_user_to_project and the user doesn't exist, then
# no error.
user_id_not_exist = uuid.uuid4().hex
self.assignment_api.add_user_to_project(self.tenant_bar['id'],
user_id_not_exist)
def test_remove_user_from_project(self):
self.assignment_api.add_user_to_project(self.tenant_baz['id'],

12
keystone/tests/test_backend_kvs.py
View File

@ -67,18 +67,6 @@ class KvsIdentity(tests.TestCase, test_backend.IdentityTests):
def test_move_project_between_domains_with_clashing_names_fails(self):
self.skipTest('Blocked by bug 1119770')
def test_delete_user_grant_no_user(self):
# See bug 1239476, kvs checks if user exists and sql does not.
self.assertRaises(
exception.UserNotFound,
super(KvsIdentity, self).test_delete_user_grant_no_user)
def test_delete_group_grant_no_group(self):
# See bug 1239476, kvs checks if group exists and sql does not.
self.assertRaises(
exception.GroupNotFound,
super(KvsIdentity, self).test_delete_group_grant_no_group)
class KvsToken(tests.TestCase, test_backend.TokenTests):
def setUp(self):

12
keystone/tests/test_keystoneclient.py
View File

@ -895,17 +895,19 @@ class KeystoneClientTests(object):
tenant=uuid.uuid4().hex,
user=self.user_foo['id'],
role=self.role_member['id'])
self.assertRaises(client_exceptions.NotFound,
client.roles.add_user_role,
tenant=self.tenant_baz['id'],
user=uuid.uuid4().hex,
role=self.role_member['id'])
self.assertRaises(client_exceptions.NotFound,
client.roles.add_user_role,
tenant=self.tenant_baz['id'],
user=self.user_foo['id'],
role=uuid.uuid4().hex)
def test_user_role_add_no_user(self):
# If add_user_role and user doesn't exist, doesn't fail.
client = self.get_client(admin=True)
client.roles.add_user_role(tenant=self.tenant_baz['id'],
user=uuid.uuid4().hex,
role=self.role_member['id'])
def test_user_role_remove_404(self):
from keystoneclient import exceptions as client_exceptions
client = self.get_client(admin=True)