openstack/keystone
Code Issues Proposed changes

Restricting domain_id update

Browse Source 
Restricts the update of a domain_id for a project, (even with the
'domain_id_immutable' property set to False), allowing it only for
root projects that have no children of its own. The update of the
domain_id of a project that has the is_domain field set True is not
allowed either. The update of this property may cause projects hierarchy
inconsistency and security issues.
This patch also sets the 'domain_id_immutable' as deprecated and emits
a WARN in case it is set False, when updating the domain_id of
users, groups or projects.

Closes-bug: 1479452
Related-bug: 1502157

Change-Id: Ib53f2173d4e4694d7ed2ecd330878664f8199371
This commit is contained in:
henriquetruta 2015-07-29 17:49:32 -03:00 committed by Henrique Truta
parent 406fbfaa26
commit 27c4cbc9f7
4 changed files with 136 additions and 4 deletions
keystone
common
config.py
identity
core.py
resource
core.py
tests/unit
test_backend.py

5
keystone/common/config.py

@ -117,7 +117,10 @@ FILE_OPTIONS = {
'domain_id. Allowing such movement is not '
'recommended if the scope of a domain admin is being '
'restricted by use of an appropriate policy file '
'(see policy.v3cloudsample as an example).'),
'(see policy.v3cloudsample as an example). This '
'ability is deprecated and will be removed in a '
'future release.',
deprecated_for_removal=True),
cfg.BoolOpt('strict_password_check', default=False,
help='If set to true, strict password length checking is '
'performed for password manipulation. If a password '

14
keystone/identity/core.py

@ -21,6 +21,7 @@ import uuid
from oslo_config import cfg
from oslo_log import log
from oslo_log import versionutils
import six
from keystone import assignment # TODO(lbragstad): Decouple this dependency
@ -877,6 +878,14 @@ class Manager(manager.Manager):
return self._set_domain_id_and_mapping(
ref_list, domain_scope, driver, mapping.EntityType.USER)
def _check_update_of_domain_id(self, new_domain, old_domain):
if new_domain != old_domain:
versionutils.report_deprecated_feature(
LOG,
_('update of domain_id is deprecated as of Mitaka '
'and will be removed in O.')
)
@domains_configured
@exception_translated('user')
def update_user(self, user_id, user_ref, initiator=None):
@ -887,6 +896,8 @@ class Manager(manager.Manager):
if 'enabled' in user:
user['enabled'] = clean.user_enabled(user['enabled'])
if 'domain_id' in user:
self._check_update_of_domain_id(user['domain_id'],
old_user_ref['domain_id'])
self.resource_api.get_domain(user['domain_id'])
if 'id' in user:
if user_id != user['id']:
@ -980,6 +991,9 @@ class Manager(manager.Manager):
@exception_translated('group')
def update_group(self, group_id, group, initiator=None):
if 'domain_id' in group:
old_group_ref = self.get_group(group_id)
self._check_update_of_domain_id(group['domain_id'],
old_group_ref['domain_id'])
self.resource_api.get_domain(group['domain_id'])
domain_id, driver, entity_id = (
self._get_domain_driver_and_entity_id(group_id))

34
keystone/resource/core.py

@ -304,6 +304,40 @@ class Manager(manager.Manager):
raise exception.ValidationError(
message=_('Update of `is_domain` is not allowed.'))
update_domain = ('domain_id' in project and
project['domain_id'] != original_project['domain_id'])
# NOTE(htruta): Even if we are allowing domain_ids to be
# modified (i.e. 'domain_id_immutable' is set False),
# a project.domain_id can only be updated for root projects
# that have no children. The update of domain_id of a project in
# the middle of the hierarchy creates an inconsistent project
# hierarchy.
if update_domain:
# NOTE(henrynash): As soon as projects start to act as domains,
# this check will no longer be valid, since a regular top level
# project will have a parent, the project that acts as a domain.
# Hence, this check must be changed.
is_root_project = original_project['parent_id'] is None
if not is_root_project:
raise exception.ValidationError(
message=_('Update of domain_id is only allowed for '
'root projects.'))
if original_project['is_domain']:
raise exception.ValidationError(
message=_('Update of domain_id of projects acting as '
'domains is not allowed.'))
subtree_list = self.list_projects_in_subtree(project_id)
if subtree_list:
raise exception.ValidationError(
message=_('Cannot update domain_id of a project that '
'has children.'))
versionutils.report_deprecated_feature(
LOG,
_('update of domain_id is deprecated as of Mitaka '
'and will be removed in O.')
)
if 'enabled' in project:
project['enabled'] = clean.project_enabled(project['enabled'])

87
keystone/tests/unit/test_backend.py

@ -729,7 +729,11 @@ class IdentityTests(AssignmentTestHelperMixin):
user = unit.new_user_ref(domain_id=domain1['id'])
user = self.identity_api.create_user(user)
user['domain_id'] = domain2['id']
self.identity_api.update_user(user['id'], user)
# Update the user asserting that a deprecation warning is emitted
with mock.patch(
'oslo_log.versionutils.report_deprecated_feature') as mock_dep:
self.identity_api.update_user(user['id'], user)
self.assertTrue(mock_dep.called)
updated_user_ref = self.identity_api.get_user(user['id'])
self.assertEqual(domain2['id'], updated_user_ref['domain_id'])
@ -818,7 +822,11 @@ class IdentityTests(AssignmentTestHelperMixin):
project = unit.new_project_ref(domain_id=domain1['id'])
self.resource_api.create_project(project['id'], project)
project['domain_id'] = domain2['id']
self.resource_api.update_project(project['id'], project)
# Update the project asserting that a deprecation warning is emitted
with mock.patch(
'oslo_log.versionutils.report_deprecated_feature') as mock_dep:
self.resource_api.update_project(project['id'], project)
self.assertTrue(mock_dep.called)
updated_project_ref = self.resource_api.get_project(project['id'])
self.assertEqual(domain2['id'], updated_project_ref['domain_id'])
@ -844,6 +852,75 @@ class IdentityTests(AssignmentTestHelperMixin):
project1['id'],
project1)
@unit.skip_if_no_multiple_domains_support
def test_move_project_with_children_between_domains_fails(self):
domain1 = unit.new_domain_ref()
self.resource_api.create_domain(domain1['id'], domain1)
domain2 = unit.new_domain_ref()
self.resource_api.create_domain(domain2['id'], domain2)
project = unit.new_project_ref(domain_id=domain1['id'])
self.resource_api.create_project(project['id'], project)
child_project = unit.new_project_ref(domain_id=domain1['id'],
parent_id=project['id'])
self.resource_api.create_project(child_project['id'], child_project)
project['domain_id'] = domain2['id']
# Update is not allowed, since updating the whole subtree would be
# necessary
self.assertRaises(exception.ValidationError,
self.resource_api.update_project,
project['id'],
project)
@unit.skip_if_no_multiple_domains_support
def test_move_project_not_root_between_domains_fails(self):
domain1 = unit.new_domain_ref()
self.resource_api.create_domain(domain1['id'], domain1)
domain2 = unit.new_domain_ref()
self.resource_api.create_domain(domain2['id'], domain2)
project = unit.new_project_ref(domain_id=domain1['id'])
self.resource_api.create_project(project['id'], project)
child_project = unit.new_project_ref(domain_id=domain1['id'],
parent_id=project['id'])
self.resource_api.create_project(child_project['id'], child_project)
child_project['domain_id'] = domain2['id']
self.assertRaises(exception.ValidationError,
self.resource_api.update_project,
child_project['id'],
child_project)
@unit.skip_if_no_multiple_domains_support
def test_move_root_project_between_domains_succeeds(self):
domain1 = unit.new_domain_ref()
self.resource_api.create_domain(domain1['id'], domain1)
domain2 = unit.new_domain_ref()
self.resource_api.create_domain(domain2['id'], domain2)
root_project = unit.new_project_ref(domain_id=domain1['id'],
parent_id=None)
self.resource_api.create_project(root_project['id'], root_project)
root_project['domain_id'] = domain2['id']
self.resource_api.update_project(root_project['id'], root_project)
project_from_db = self.resource_api.get_project(root_project['id'])
self.assertEqual(domain2['id'], project_from_db['domain_id'])
@unit.skip_if_no_multiple_domains_support
def test_update_domain_id_project_is_domain_fails(self):
other_domain = unit.new_domain_ref()
self.resource_api.create_domain(other_domain['id'], other_domain)
project = unit.new_project_ref(is_domain=True,
domain_id=self.domain_default['id'])
self.resource_api.create_project(project['id'], project)
project['domain_id'] = other_domain['id']
# Update of domain_id of projects acting as domains is not allowed
self.assertRaises(exception.ValidationError,
self.resource_api.update_project,
project['id'],
project)
def test_rename_duplicate_project_name_fails(self):
project1 = unit.new_project_ref(domain_id=DEFAULT_DOMAIN_ID)
project2 = unit.new_project_ref(domain_id=DEFAULT_DOMAIN_ID)
@ -3239,7 +3316,11 @@ class IdentityTests(AssignmentTestHelperMixin):
group = unit.new_group_ref(domain_id=domain1['id'])
group = self.identity_api.create_group(group)
group['domain_id'] = domain2['id']
self.identity_api.update_group(group['id'], group)
# Update the group asserting that a deprecation warning is emitted
with mock.patch(
'oslo_log.versionutils.report_deprecated_feature') as mock_dep:
self.identity_api.update_group(group['id'], group)
self.assertTrue(mock_dep.called)
updated_group_ref = self.identity_api.get_group(group['id'])
self.assertEqual(domain2['id'], updated_group_ref['domain_id'])