Provide option to make domain_id immutable
Currently, a user, group or project entity can be moved between domains by updating their domain_id. There are situations where this is not desirable (and in fact could create a potential security hole) - for example when creating a domain admin persona, using an appropriate policy file (such as policy.v3cloudsample). For backward compatibility, the option to make the domain_id immutable is controlled by a config option, with the default being no change to existing functionality. Change-Id: Idd847f471beae7387d6cc59af0a960a923da799f Closes-Bug: 1291393
This commit is contained in:
parent
dc9370cc4f
commit
a2fa6a6f01
etc
keystone
@ -98,6 +98,14 @@
|
||||
# appropriate section (e.g. [assignment]). (integer value)
|
||||
#list_limit=<None>
|
||||
|
||||
# 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
|
||||
|
@ -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)
|
||||
|
||||
|
@ -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 '
|
||||
|
@ -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()
|
||||
|
@ -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()
|
||||
|
@ -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' % {
|
||||
|
Loading…
x
Reference in New Issue
Block a user