diff --git a/etc/keystone.conf.sample b/etc/keystone.conf.sample index f7f5afcec5..7e0d4dbc55 100644 --- a/etc/keystone.conf.sample +++ b/etc/keystone.conf.sample @@ -98,6 +98,14 @@ # appropriate section (e.g. [assignment]). (integer value) #list_limit= +# Set this to true if you want to disable the ability for +# user, group and project entities to be moved between domains +# by updating their domain_id. This can be used to further +# restrict scope of a domain admin, in conjunction with an +# appropriate policy file (see policy.v3cloudsample as an +# example). (boolean value) +#domain_id_immutable=false + # # Options defined in oslo.messaging diff --git a/keystone/assignment/controllers.py b/keystone/assignment/controllers.py index 41fa6295fb..fa01644ab9 100644 --- a/keystone/assignment/controllers.py +++ b/keystone/assignment/controllers.py @@ -419,7 +419,8 @@ class ProjectV3(controller.V3Controller): @controller.protected() def update_project(self, context, project_id, project): self._require_matching_id(project_id, project) - + self._require_matching_domain_id( + project_id, project, self.assignment_api.get_project) ref = self.assignment_api.update_project(project_id, project) return ProjectV3.wrap_member(context, ref) diff --git a/keystone/common/config.py b/keystone/common/config.py index b483aead97..fdeefb6af1 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -108,7 +108,15 @@ FILE_OPTIONS = { 'list_limit, with no limit set by default. This ' 'global limit may be then overridden for a specific ' 'driver, by specifying a list_limit in the ' - 'appropriate section (e.g. [assignment]).')], + 'appropriate section (e.g. [assignment]).'), + cfg.BoolOpt('domain_id_immutable', default=False, + help='Set this to true if you want to disable the ' + 'ability for user, group and project entities ' + 'to be moved between domains by updating their ' + 'domain_id. This can be used to further restrict ' + 'scope of a domain admin, in conjunction with an ' + 'appropriate policy file (see policy.v3cloudsample ' + 'as an example).')], 'identity': [ cfg.StrOpt('default_domain_id', default='default', help='This references the domain to use for all ' diff --git a/keystone/common/controller.py b/keystone/common/controller.py index b624530942..fbb2691920 100644 --- a/keystone/common/controller.py +++ b/keystone/common/controller.py @@ -515,6 +515,28 @@ class V3Controller(wsgi.Application): if 'id' in ref and ref['id'] != value: raise exception.ValidationError('Cannot change ID') + def _require_matching_domain_id(self, ref_id, ref, get_member): + """Ensure the current domain ID matches the reference one, if any. + + Provided we want domain IDs to be immutable, check whether any + domain_id specified in the ref dictionary matches the existing + domain_id for this entity. + + :param ref_id: the ID of the entity + :param ref: the dictionary of new values proposed for this entity + :param get_member: The member function to call to get the current + entity + :raises: :class:`keystone.exception.ValidationError` + + """ + # TODO(henry-nash): It might be safer and more efficient to do this + # check in the managers affected, so look to migrate this check to + # there in the future. + if CONF.domain_id_immutable and 'domain_id' in ref: + existing_ref = get_member(ref_id) + if ref['domain_id'] != existing_ref['domain_id']: + raise exception.ValidationError(_('Cannot change Domain ID')) + def _assign_unique_id(self, ref): """Generates and assigns a unique identifer to a reference.""" ref = ref.copy() diff --git a/keystone/identity/controllers.py b/keystone/identity/controllers.py index b61c34fba2..0224d73074 100644 --- a/keystone/identity/controllers.py +++ b/keystone/identity/controllers.py @@ -14,6 +14,7 @@ """Workflow Logic the Identity service.""" +import functools import inspect import six import uuid @@ -300,6 +301,10 @@ class UserV3(controller.V3Controller): def _update_user(self, context, user_id, user, domain_scope): self._require_matching_id(user_id, user) + self._require_matching_domain_id( + user_id, user, + functools.partial(self.identity_api.get_user, + domain_scope=domain_scope)) ref = self.identity_api.update_user( user_id, user, domain_scope=domain_scope) return UserV3.wrap_member(context, ref) @@ -401,10 +406,14 @@ class GroupV3(controller.V3Controller): @controller.protected() def update_group(self, context, group_id, group): self._require_matching_id(group_id, group) - + domain_scope = self._get_domain_id_for_request(context) + self._require_matching_domain_id( + group_id, group, + functools.partial(self.identity_api.get_group, + domain_scope=domain_scope)) ref = self.identity_api.update_group( group_id, group, - domain_scope=self._get_domain_id_for_request(context)) + domain_scope=domain_scope) return GroupV3.wrap_member(context, ref) @controller.protected() diff --git a/keystone/tests/test_v3_identity.py b/keystone/tests/test_v3_identity.py index d1fa872357..e82d8e2599 100644 --- a/keystone/tests/test_v3_identity.py +++ b/keystone/tests/test_v3_identity.py @@ -421,6 +421,22 @@ class IdentityTestCase(test_v3.RestfulTestCase): body={'project': ref}) self.assertValidProjectResponse(r, ref) + def test_update_project_domain_id(self): + """Call ``PATCH /projects/{project_id}`` with domain_id.""" + project = self.new_project_ref(domain_id=self.domain['id']) + self.assignment_api.create_project(project['id'], project) + project['domain_id'] = CONF.identity.default_domain_id + r = self.patch('/projects/%(project_id)s' % { + 'project_id': project['id']}, + body={'project': project}) + self.assertValidProjectResponse(r, project) + self.config_fixture.config(domain_id_immutable=True) + project['domain_id'] = self.domain['id'] + r = self.patch('/projects/%(project_id)s' % { + 'project_id': project['id']}, + body={'project': project}, + expected_status=exception.ValidationError.code) + def test_delete_project(self): """Call ``DELETE /projects/{project_id}`` @@ -577,6 +593,22 @@ class IdentityTestCase(test_v3.RestfulTestCase): body={'user': user}) self.assertValidUserResponse(r, user) + def test_update_user_domain_id(self): + """Call ``PATCH /users/{user_id}`` with domain_id.""" + user = self.new_user_ref(domain_id=self.domain['id']) + self.identity_api.create_user(user['id'], user) + user['domain_id'] = CONF.identity.default_domain_id + r = self.patch('/users/%(user_id)s' % { + 'user_id': user['id']}, + body={'user': user}) + self.assertValidUserResponse(r, user) + self.config_fixture.config(domain_id_immutable=True) + user['domain_id'] = self.domain['id'] + r = self.patch('/users/%(user_id)s' % { + 'user_id': user['id']}, + body={'user': user}, + expected_status=exception.ValidationError.code) + def test_delete_user(self): """Call ``DELETE /users/{user_id}``. @@ -668,6 +700,22 @@ class IdentityTestCase(test_v3.RestfulTestCase): body={'group': group}) self.assertValidGroupResponse(r, group) + def test_update_group_domain_id(self): + """Call ``PATCH /groups/{group_id}`` with domain_id.""" + group = self.new_group_ref(domain_id=self.domain['id']) + self.identity_api.create_group(group['id'], group) + group['domain_id'] = CONF.identity.default_domain_id + r = self.patch('/groups/%(group_id)s' % { + 'group_id': group['id']}, + body={'group': group}) + self.assertValidGroupResponse(r, group) + self.config_fixture.config(domain_id_immutable=True) + group['domain_id'] = self.domain['id'] + r = self.patch('/groups/%(group_id)s' % { + 'group_id': group['id']}, + body={'group': group}, + expected_status=exception.ValidationError.code) + def test_delete_group(self): """Call ``DELETE /groups/{group_id}``.""" self.delete('/groups/%(group_id)s' % {