From 09663a01a4eda4332e55637a120019e1784b967e Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Mon, 10 Dec 2018 22:01:23 +0000 Subject: [PATCH] Implement domain admin functionality for projects This commit add explicit testing to show how users with the admin role on a domain can manage projects within their domain. It also modifies the default policies to account for this functionality. A subsequent patch will do the same for project users. Change-Id: I3e1cc44c4ed09ea0a4123ea13974b963c7335676 Closes-Bug: 1750660 Related-Bug: 1806762 --- keystone/api/projects.py | 5 +- keystone/common/policies/project.py | 28 ++- .../tests/unit/protection/v3/test_projects.py | 176 +++++++++++++++++- .../notes/bug-1750660-e2a360ddd6790fc4.yaml | 39 ++++ 4 files changed, 225 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/bug-1750660-e2a360ddd6790fc4.yaml diff --git a/keystone/api/projects.py b/keystone/api/projects.py index 3c51249431..4eb76b48f7 100644 --- a/keystone/api/projects.py +++ b/keystone/api/projects.py @@ -157,8 +157,11 @@ class ProjectResource(ks_flask.ResourceBase): POST /v3/projects """ - ENFORCER.enforce_call(action='identity:create_project') project = self.request_body_json.get('project', {}) + target = {'project': project} + ENFORCER.enforce_call( + action='identity:create_project', target_attr=target + ) validation.lazy_validate(schema.project_create, project) project = self._assign_unique_id(project) if not project.get('is_domain'): diff --git a/keystone/common/policies/project.py b/keystone/common/policies/project.py index f894bf256d..cde64a93b9 100644 --- a/keystone/common/policies/project.py +++ b/keystone/common/policies/project.py @@ -41,6 +41,11 @@ SYSTEM_READER_OR_DOMAIN_READER = ( '(role:reader and domain_id:%(target.domain_id)s)' ) +SYSTEM_ADMIN_OR_DOMAIN_ADMIN = ( + '(role:admin and system_scope:all) or ' + '(role:admin and domain_id:%(target.project.domain_id)s)' +) + deprecated_list_projects = policy.DeprecatedRule( name=base.IDENTITY % 'list_projects', check_str=base.RULE_ADMIN_REQUIRED @@ -111,15 +116,8 @@ project_policies = [ deprecated_since=versionutils.deprecated.STEIN), policy.DocumentedRuleDefault( name=base.IDENTITY % 'create_project', - check_str=base.SYSTEM_ADMIN, - # FIXME(lbragstad): System administrators should be able to create - # projects anywhere in the deployment. Domain administrators should - # only be able to create projects within their domain. Project - # administrators should only be able to create children projects of the - # project they administer. Until keystone is smart enough to handle - # those checks in code, keep this as a system-level operation for - # backwards compatibility. - scope_types=['system'], + check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN, + scope_types=['system', 'domain'], description='Create project.', operations=[{'path': '/v3/projects', 'method': 'POST'}], @@ -128,10 +126,8 @@ project_policies = [ deprecated_since=versionutils.deprecated.STEIN), policy.DocumentedRuleDefault( name=base.IDENTITY % 'update_project', - check_str=base.SYSTEM_ADMIN, - # FIXME(lbragstad): See the above comment for create_project as to why - # this is limited to only system-scope. - scope_types=['system'], + check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN, + scope_types=['system', 'domain'], description='Update project.', operations=[{'path': '/v3/projects/{project_id}', 'method': 'PATCH'}], @@ -140,10 +136,8 @@ project_policies = [ deprecated_since=versionutils.deprecated.STEIN), policy.DocumentedRuleDefault( name=base.IDENTITY % 'delete_project', - check_str=base.SYSTEM_ADMIN, - # FIXME(lbragstad): See the above comment for create_project as to why - # this is limited to only system-scope. - scope_types=['system'], + check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN, + scope_types=['system', 'domain'], description='Delete project.', operations=[{'path': '/v3/projects/{project_id}', 'method': 'DELETE'}], diff --git a/keystone/tests/unit/protection/v3/test_projects.py b/keystone/tests/unit/protection/v3/test_projects.py index 3b55651b42..1a10bc47cf 100644 --- a/keystone/tests/unit/protection/v3/test_projects.py +++ b/keystone/tests/unit/protection/v3/test_projects.py @@ -580,6 +580,170 @@ class DomainMemberTests(base_classes.TestCaseWithBootstrap, self.headers = {'X-Auth-Token': self.token_id} +class DomainAdminTests(base_classes.TestCaseWithBootstrap, + common_auth.AuthTestMixin, + _DomainUsersTests): + + 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.project have been removed. This is only + # here to make sure we test the new policies instead of the deprecated + # ones. Oslo.policy will OR 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_project': ( + pp.SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER), + 'identity:list_user_projects': ( + pp.SYSTEM_READER_OR_DOMAIN_READER_OR_OWNER), + 'identity:list_projects': ( + pp.SYSTEM_READER_OR_DOMAIN_READER), + 'identity:create_project': pp.SYSTEM_ADMIN_OR_DOMAIN_ADMIN, + 'identity:update_project': pp.SYSTEM_ADMIN_OR_DOMAIN_ADMIN, + 'identity:delete_project': pp.SYSTEM_ADMIN_OR_DOMAIN_ADMIN + } + f.write(jsonutils.dumps(overridden_policies)) + + def test_user_can_create_projects_within_domain(self): + create = {'project': unit.new_project_ref(domain_id=self.domain_id)} + + with self.test_client() as c: + c.post('/v3/projects', json=create, headers=self.headers) + + def test_user_cannot_create_projects_in_other_domains(self): + create = { + 'project': unit.new_project_ref( + domain_id=CONF.identity.default_domain_id + ) + } + + with self.test_client() as c: + c.post( + '/v3/projects', json=create, headers=self.headers, + expected_status_code=http_client.FORBIDDEN + ) + + def test_user_can_update_projects_within_domain(self): + project = PROVIDERS.resource_api.create_project( + uuid.uuid4().hex, + unit.new_project_ref(domain_id=self.domain_id) + ) + + update = {'project': {'description': uuid.uuid4().hex}} + + with self.test_client() as c: + c.patch( + '/v3/projects/%s' % project['id'], json=update, + headers=self.headers + ) + + def test_user_cannot_update_projects_in_other_domain(self): + project = PROVIDERS.resource_api.create_project( + uuid.uuid4().hex, + unit.new_project_ref(domain_id=CONF.identity.default_domain_id) + ) + + update = {'project': {'description': uuid.uuid4().hex}} + + with self.test_client() as c: + c.patch( + '/v3/projects/%s' % project['id'], json=update, + headers=self.headers, + expected_status_code=http_client.FORBIDDEN + ) + + def test_user_cannot_update_non_existent_project_forbidden(self): + # Because domain users operate outside of system scope, we can't + # confidently return a Not Found here because they aren't system users. + # The best we can do is return a Forbidden because we need the + # project's domain in order to resolve the policy check, and the + # project doesn't exist. This errors on the side of opacity and returns + # a 403 instead of a 404. + update = {'project': {'description': uuid.uuid4().hex}} + + with self.test_client() as c: + c.patch( + '/v3/projects/%s' % uuid.uuid4().hex, json=update, + headers=self.headers, + expected_status_code=http_client.FORBIDDEN + ) + + def test_user_can_delete_projects_within_domain(self): + project = PROVIDERS.resource_api.create_project( + uuid.uuid4().hex, + unit.new_project_ref(domain_id=self.domain_id) + ) + + with self.test_client() as c: + c.delete('/v3/projects/%s' % project['id'], headers=self.headers) + + def test_user_cannot_delete_projects_in_other_domain(self): + project = PROVIDERS.resource_api.create_project( + uuid.uuid4().hex, + unit.new_project_ref(domain_id=CONF.identity.default_domain_id) + ) + + with self.test_client() as c: + c.delete( + '/v3/projects/%s' % project['id'], headers=self.headers, + expected_status_code=http_client.FORBIDDEN + ) + + def test_user_cannot_delete_non_existent_projects_forbidden(self): + # Because domain users operate outside of system scope, we can't + # confidently return a Not Found here because they aren't system users. + # The best we can do is return a Forbidden because we need the + # project's domain in order to resolve the policy check, and the + # project doesn't exist. This errors on the side of opacity and returns + # a 403 instead of a 404. + with self.test_client() as c: + c.delete( + '/v3/projects/%s' % uuid.uuid4().hex, headers=self.headers, + expected_status_code=http_client.FORBIDDEN + ) + + class ProjectUserTests(base_classes.TestCaseWithBootstrap, common_auth.AuthTestMixin): @@ -628,12 +792,14 @@ class ProjectUserTests(base_classes.TestCaseWithBootstrap, with open(self.policy_file_name, 'w') as f: overridden_policies = { 'identity:get_project': ( - pp.SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER - ), - 'identity:list_projects': pp.SYSTEM_READER_OR_DOMAIN_READER, + pp.SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER), 'identity:list_user_projects': ( - pp.SYSTEM_READER_OR_DOMAIN_READER_OR_OWNER - ), + pp.SYSTEM_READER_OR_DOMAIN_READER_OR_OWNER), + 'identity:list_projects': ( + pp.SYSTEM_READER_OR_DOMAIN_READER), + 'identity:create_project': pp.SYSTEM_ADMIN_OR_DOMAIN_ADMIN, + 'identity:update_project': pp.SYSTEM_ADMIN_OR_DOMAIN_ADMIN, + 'identity:delete_project': pp.SYSTEM_ADMIN_OR_DOMAIN_ADMIN } f.write(jsonutils.dumps(overridden_policies)) diff --git a/releasenotes/notes/bug-1750660-e2a360ddd6790fc4.yaml b/releasenotes/notes/bug-1750660-e2a360ddd6790fc4.yaml new file mode 100644 index 0000000000..c277c03a67 --- /dev/null +++ b/releasenotes/notes/bug-1750660-e2a360ddd6790fc4.yaml @@ -0,0 +1,39 @@ +--- +features: + - | + [`bug 1750660 `_] + The project API now supports the ``admin``, ``member``, and + ``reader`` default roles across system-scope, domain-scope, and + project-scope. +upgrade: + - | + [`bug 1750660 `_] + The project API uses new default policies that make it more + accessible to end users and administrators in a secure way. Please + consider these new defaults if your deployment overrides + project policies. +deprecations: + - | + [`bug 1750660 `_] + The project policies have been deprecated. The ``identity:get_project`` policy + now uses ``(role:reader and system_scope:all) or (role:reader and + domain_id:%(target.project.domain_id)s) or project_id:%(target.project.id)s`` + instead of ``rule:admin_required or project_id:%(target.project.id)s``. + The ``identity:list_projects`` policy now uses ``(role:reader and + system_scope:all) or (role:reader and domain_id:%(target.domain_id)s`` + instead of ``rule:admin_required``. The ``identity:list_user_projects`` + policy now uses ``(role:reader and system_scope:all) or (role:reader and + domain_id:%(target.user.domain_id)s) or user_id:%(target.user.id)s`` + instead of ``rule:admin_or_owner``. The ``identity:create_project`` + now uses ``(role:admin and system_scope:all) or (role:admin and + domain_id:%(target.project.domain_id)s)`` instead of + ``rule:admin_required``. These new defaults automatically include + support for a read-only role and allow for more granular access + to project APIs, making it easier for system and domain administrators + to delegate authorization, safely. Please consider these new defaults + if your deployment overrides the project policies. +security: + - | + [`bug 1750660 `_] + The project API now uses system-scope, domain-scope, project-scope and default + roles to provide better accessibility to users in a secure way.