Implement domain admin functionality for user API
This commit add explicit testing to show how users with the admin role on a domain can manage users within thier domain. It also modifies the default policies to account for this functionality. A subsequent patch will do the same for project users. Change-Id: I3899e07b857e213f85384ed9c9e4add199290a49 Partial-Bug: 1748027 Partial-Bug: 968696
This commit is contained in:
parent
9ca599e506
commit
cf1ce4eb36
|
@ -181,8 +181,11 @@ class UserResource(ks_flask.ResourceBase):
|
|||
|
||||
POST /v3/users
|
||||
"""
|
||||
ENFORCER.enforce_call(action='identity:create_user')
|
||||
user_data = self.request_body_json.get('user', {})
|
||||
target = {'user': user_data}
|
||||
ENFORCER.enforce_call(
|
||||
action='identity:create_user', target_attr=target
|
||||
)
|
||||
validation.lazy_validate(schema.user_create, user_data)
|
||||
user_data = self._normalize_dict(user_data)
|
||||
user_data = self._normalize_domain_id(user_data)
|
||||
|
@ -196,7 +199,11 @@ class UserResource(ks_flask.ResourceBase):
|
|||
|
||||
PATCH /v3/users/{user_id}
|
||||
"""
|
||||
ENFORCER.enforce_call(action='identity:update_user')
|
||||
ENFORCER.enforce_call(
|
||||
action='identity:update_user',
|
||||
build_target=_build_user_target_enforcement
|
||||
)
|
||||
PROVIDERS.identity_api.get_user(user_id)
|
||||
user_data = self.request_body_json.get('user', {})
|
||||
validation.lazy_validate(schema.user_update, user_data)
|
||||
self._require_matching_id(user_data)
|
||||
|
|
|
@ -25,6 +25,11 @@ SYSTEM_READER_OR_DOMAIN_READER = (
|
|||
'(' + base.SYSTEM_READER + ') or (' + base.DOMAIN_READER + ')'
|
||||
)
|
||||
|
||||
SYSTEM_ADMIN_OR_DOMAIN_ADMIN = (
|
||||
'(role:admin and system_scope:all) or '
|
||||
'(role:admin and token.domain.id:%(target.user.domain_id)s)'
|
||||
)
|
||||
|
||||
DEPRECATED_REASON = """
|
||||
As of the Stein release, the user API understands how to handle system-scoped
|
||||
tokens in addition to project and domain tokens, making the API more accessible
|
||||
|
@ -104,15 +109,8 @@ user_policies = [
|
|||
'method': 'GET'}]),
|
||||
policy.DocumentedRuleDefault(
|
||||
name=base.IDENTITY % 'create_user',
|
||||
check_str=base.SYSTEM_ADMIN,
|
||||
# FIXME(lbragstad): This can be considered either a system-level policy
|
||||
# or a project-level policy. System administrator should have the
|
||||
# ability to create users in any domain. Domain (or project)
|
||||
# administrators should have the ability to create users in the domain
|
||||
# they administer. The second case is going to require a policy check
|
||||
# in code. Until that happens, we will leave this as a system-level
|
||||
# policy.
|
||||
scope_types=['system'],
|
||||
check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN,
|
||||
scope_types=['system', 'domain'],
|
||||
description='Create a user.',
|
||||
operations=[{'path': '/v3/users',
|
||||
'method': 'POST'}],
|
||||
|
@ -121,10 +119,8 @@ user_policies = [
|
|||
deprecated_since=versionutils.deprecated.STEIN),
|
||||
policy.DocumentedRuleDefault(
|
||||
name=base.IDENTITY % 'update_user',
|
||||
check_str=base.SYSTEM_ADMIN,
|
||||
# FIXME(lbragstad): See the above comment about adding support for
|
||||
# project scope_types in the future.
|
||||
scope_types=['system'],
|
||||
check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN,
|
||||
scope_types=['system', 'domain'],
|
||||
description='Update a user, including administrative password resets.',
|
||||
operations=[{'path': '/v3/users/{user_id}',
|
||||
'method': 'PATCH'}],
|
||||
|
@ -133,10 +129,8 @@ user_policies = [
|
|||
deprecated_since=versionutils.deprecated.STEIN),
|
||||
policy.DocumentedRuleDefault(
|
||||
name=base.IDENTITY % 'delete_user',
|
||||
check_str=base.SYSTEM_ADMIN,
|
||||
# FIXME(lbragstad): See the above comment about adding support for
|
||||
# project scope_types in the future.
|
||||
scope_types=['system'],
|
||||
check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN,
|
||||
scope_types=['system', 'domain'],
|
||||
description='Delete a user.',
|
||||
operations=[{'path': '/v3/users/{user_id}',
|
||||
'method': 'DELETE'}],
|
||||
|
|
|
@ -12,14 +12,17 @@
|
|||
|
||||
import uuid
|
||||
|
||||
from oslo_serialization import jsonutils
|
||||
from six.moves import http_client
|
||||
|
||||
from keystone.common.policies import user as up
|
||||
from keystone.common import provider_api
|
||||
import keystone.conf
|
||||
from keystone.tests.common import auth as common_auth
|
||||
from keystone.tests import unit
|
||||
from keystone.tests.unit import base_classes
|
||||
from keystone.tests.unit import ksfixtures
|
||||
from keystone.tests.unit.ksfixtures import temporaryfile
|
||||
|
||||
CONF = keystone.conf.CONF
|
||||
PROVIDERS = provider_api.ProviderAPIs
|
||||
|
@ -132,8 +135,8 @@ class _SystemMemberAndReaderUserTests(object):
|
|||
)
|
||||
|
||||
|
||||
class _DomainMemberAndReaderUserTests(object):
|
||||
"""Functionality for all domain members and domain readers."""
|
||||
class _DomainUserTests(object):
|
||||
"""Commont default functionality for all domain users."""
|
||||
|
||||
def test_user_can_get_user_within_domain(self):
|
||||
user = PROVIDERS.identity_api.create_user(
|
||||
|
@ -189,6 +192,10 @@ class _DomainMemberAndReaderUserTests(object):
|
|||
user_ids.append(u['id'])
|
||||
self.assertNotIn(user['id'], user_ids)
|
||||
|
||||
|
||||
class _DomainMemberAndReaderUserTests(object):
|
||||
"""Functionality for all domain members and domain readers."""
|
||||
|
||||
def test_user_cannot_create_users_within_domain(self):
|
||||
create = {
|
||||
'user': {
|
||||
|
@ -438,6 +445,7 @@ class SystemAdminTests(base_classes.TestCaseWithBootstrap,
|
|||
class DomainReaderTests(base_classes.TestCaseWithBootstrap,
|
||||
common_auth.AuthTestMixin,
|
||||
_CommonUserTests,
|
||||
_DomainUserTests,
|
||||
_DomainMemberAndReaderUserTests):
|
||||
|
||||
def setUp(self):
|
||||
|
@ -473,6 +481,7 @@ class DomainReaderTests(base_classes.TestCaseWithBootstrap,
|
|||
class DomainMemberTests(base_classes.TestCaseWithBootstrap,
|
||||
common_auth.AuthTestMixin,
|
||||
_CommonUserTests,
|
||||
_DomainUserTests,
|
||||
_DomainMemberAndReaderUserTests):
|
||||
|
||||
def setUp(self):
|
||||
|
@ -503,3 +512,161 @@ class DomainMemberTests(base_classes.TestCaseWithBootstrap,
|
|||
r = c.post('/v3/auth/tokens', json=auth)
|
||||
self.token_id = r.headers['X-Subject-Token']
|
||||
self.headers = {'X-Auth-Token': self.token_id}
|
||||
|
||||
|
||||
class DomainAdminTests(base_classes.TestCaseWithBootstrap,
|
||||
common_auth.AuthTestMixin,
|
||||
_CommonUserTests,
|
||||
_DomainUserTests):
|
||||
|
||||
def setUp(self):
|
||||
super(DomainAdminTests, self).setUp()
|
||||
self.loadapp()
|
||||
|
||||
self.policy_file = self.useFixture(temporaryfile.SecureTempFile())
|
||||
self.policy_file_name = self.policy_file.file_name
|
||||
self.useFixture(
|
||||
ksfixtures.Policy(
|
||||
self.config_fixture, policy_file=self.policy_file_name
|
||||
)
|
||||
)
|
||||
|
||||
self._override_policy()
|
||||
self.config_fixture.config(group='oslo_policy', enforce_scope=True)
|
||||
|
||||
domain = PROVIDERS.resource_api.create_domain(
|
||||
uuid.uuid4().hex, unit.new_domain_ref()
|
||||
)
|
||||
self.domain_id = domain['id']
|
||||
domain_admin = unit.new_user_ref(domain_id=self.domain_id)
|
||||
self.user_id = PROVIDERS.identity_api.create_user(domain_admin)['id']
|
||||
PROVIDERS.assignment_api.create_grant(
|
||||
self.bootstrapper.admin_role_id, user_id=self.user_id,
|
||||
domain_id=self.domain_id
|
||||
)
|
||||
|
||||
auth = self.build_authentication_request(
|
||||
user_id=self.user_id, password=domain_admin['password'],
|
||||
domain_id=self.domain_id,
|
||||
)
|
||||
|
||||
# Grab a token using the persona we're testing and prepare headers
|
||||
# for requests we'll be making in the tests.
|
||||
with self.test_client() as c:
|
||||
r = c.post('/v3/auth/tokens', json=auth)
|
||||
self.token_id = r.headers['X-Subject-Token']
|
||||
self.headers = {'X-Auth-Token': self.token_id}
|
||||
|
||||
def _override_policy(self):
|
||||
# TODO(lbragstad): Remove this once the deprecated policies in
|
||||
# keystone.common.policies.users have been removed. This is only
|
||||
# here to make sure we test the new policies instead of the deprecated
|
||||
# ones. Oslo.policy will apply a logical OR to deprecated policies with
|
||||
# new policies to maintain compatibility and give operators a chance to
|
||||
# update permissions or update policies without breaking users. This
|
||||
# will cause these specific tests to fail since we're trying to correct
|
||||
# this broken behavior with better scope checking.
|
||||
with open(self.policy_file_name, 'w') as f:
|
||||
overridden_policies = {
|
||||
'identity:get_user': up.SYSTEM_READER_OR_DOMAIN_READER_OR_USER,
|
||||
'identity:list_users': up.SYSTEM_READER_OR_DOMAIN_READER,
|
||||
'identity:create_user': up.SYSTEM_ADMIN_OR_DOMAIN_ADMIN,
|
||||
'identity:update_user': up.SYSTEM_ADMIN_OR_DOMAIN_ADMIN,
|
||||
'identity:delete_user': up.SYSTEM_ADMIN_OR_DOMAIN_ADMIN
|
||||
}
|
||||
f.write(jsonutils.dumps(overridden_policies))
|
||||
|
||||
def test_user_can_create_users_within_domain(self):
|
||||
create = {
|
||||
'user': {
|
||||
'domain_id': self.domain_id,
|
||||
'name': uuid.uuid4().hex
|
||||
}
|
||||
}
|
||||
|
||||
with self.test_client() as c:
|
||||
c.post('/v3/users', json=create, headers=self.headers)
|
||||
|
||||
def test_user_cannot_create_users_in_other_domain(self):
|
||||
domain = PROVIDERS.resource_api.create_domain(
|
||||
uuid.uuid4().hex, unit.new_domain_ref()
|
||||
)
|
||||
|
||||
create = {
|
||||
'user': {
|
||||
'domain_id': domain['id'],
|
||||
'name': uuid.uuid4().hex
|
||||
}
|
||||
}
|
||||
|
||||
with self.test_client() as c:
|
||||
c.post(
|
||||
'/v3/users', json=create, headers=self.headers,
|
||||
expected_status_code=http_client.FORBIDDEN
|
||||
)
|
||||
|
||||
def test_user_can_update_users_within_domain(self):
|
||||
user = PROVIDERS.identity_api.create_user(
|
||||
unit.new_user_ref(domain_id=self.domain_id)
|
||||
)
|
||||
|
||||
update = {'user': {'email': uuid.uuid4().hex}}
|
||||
with self.test_client() as c:
|
||||
c.patch(
|
||||
'/v3/users/%s' % user['id'], json=update, headers=self.headers
|
||||
)
|
||||
|
||||
def test_user_cannot_update_users_in_other_domain(self):
|
||||
domain = PROVIDERS.resource_api.create_domain(
|
||||
uuid.uuid4().hex, unit.new_domain_ref()
|
||||
)
|
||||
user = PROVIDERS.identity_api.create_user(
|
||||
unit.new_user_ref(domain_id=domain['id'])
|
||||
)
|
||||
|
||||
update = {'user': {'email': uuid.uuid4().hex}}
|
||||
with self.test_client() as c:
|
||||
c.patch(
|
||||
'/v3/users/%s' % user['id'], json=update, headers=self.headers,
|
||||
expected_status_code=http_client.FORBIDDEN
|
||||
)
|
||||
|
||||
def test_user_cannot_update_non_existent_user_forbidden(self):
|
||||
update = {'user': {'email': uuid.uuid4().hex}}
|
||||
with self.test_client() as c:
|
||||
c.patch(
|
||||
'/v3/users/%s' % uuid.uuid4().hex, json=update,
|
||||
headers=self.headers,
|
||||
expected_status_code=http_client.FORBIDDEN
|
||||
)
|
||||
|
||||
def test_user_can_delete_users_within_domain(self):
|
||||
user = PROVIDERS.identity_api.create_user(
|
||||
unit.new_user_ref(domain_id=self.domain_id)
|
||||
)
|
||||
|
||||
with self.test_client() as c:
|
||||
c.delete(
|
||||
'/v3/users/%s' % user['id'], headers=self.headers
|
||||
)
|
||||
|
||||
def test_user_cannot_delete_users_in_other_domain(self):
|
||||
domain = PROVIDERS.resource_api.create_domain(
|
||||
uuid.uuid4().hex, unit.new_domain_ref()
|
||||
)
|
||||
user = PROVIDERS.identity_api.create_user(
|
||||
unit.new_user_ref(domain_id=domain['id'])
|
||||
)
|
||||
|
||||
with self.test_client() as c:
|
||||
c.delete(
|
||||
'/v3/users/%s' % user['id'], headers=self.headers,
|
||||
expected_status_code=http_client.FORBIDDEN
|
||||
)
|
||||
|
||||
def test_user_cannot_delete_non_existent_user_forbidden(self):
|
||||
with self.test_client() as c:
|
||||
c.delete(
|
||||
'/v3/users/%s' % uuid.uuid4().hex, headers=self.headers,
|
||||
expected_status_code=http_client.FORBIDDEN
|
||||
)
|
||||
|
|
|
@ -27,12 +27,13 @@ deprecations:
|
|||
and domain_id:%(target.domain_id)s)`` instead of
|
||||
``rule:admin_required``. The ``identity:create_user``,
|
||||
``identity:update_user``, and ``identity:delete_user`` policies now use
|
||||
``role:admin and system_scope:all`` instead of ``rule:admin_required``.
|
||||
These new defaults automatically account
|
||||
for system-scope, domain-scope, and support a read-only role, making it easier
|
||||
for system and domain administrators to delegate subsets of responsibility
|
||||
without compromising security. Please consider these new defaults
|
||||
if your deployment overrides the user policies.
|
||||
``(role:admin and system_scope:all) or (role:admin and
|
||||
token.domain.id:%(target.user.domain_id)s)`` instead of ``rule:admin_required``.
|
||||
These new defaults automatically account for system-scope, domain-scope,
|
||||
and support a read-only role, making it easier for system and domain
|
||||
administrators to delegate subsets of responsibility without compromising
|
||||
security. Please consider these new defaults if your deployment overrides
|
||||
the user policies.
|
||||
security:
|
||||
- |
|
||||
[`bug 1805406 <https://bugs.launchpad.net/keystone/+bug/1805406>`_]
|
||||
|
|
Loading…
Reference in New Issue