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
(cherry picked from commit cf1ce4eb36)
This commit is contained in:
Lance Bragstad 2018-12-06 20:50:28 +00:00
parent dd6da4cd45
commit a5fbec6a09
4 changed files with 196 additions and 27 deletions

View File

@ -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)

View File

@ -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'}],

View File

@ -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
)

View File

@ -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>`_]