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

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

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,
+        check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN,
-        # FIXME(lbragstad): System administrators should be able to create
+        scope_types=['system', 'domain'],
-        # 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'],
         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,
+        check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN,
-        # FIXME(lbragstad): See the above comment for create_project as to why
+        scope_types=['system', 'domain'],
-        # this is limited to only system-scope.
-        scope_types=['system'],
         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,
+        check_str=SYSTEM_ADMIN_OR_DOMAIN_ADMIN,
-        # FIXME(lbragstad): See the above comment for create_project as to why
+        scope_types=['system', 'domain'],
-        # this is limited to only system-scope.
-        scope_types=['system'],
         description='Delete project.',
         operations=[{'path': '/v3/projects/{project_id}',
                      'method': 'DELETE'}],

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
+                    pp.SYSTEM_READER_OR_DOMAIN_READER_OR_PROJECT_USER),
-                ),
-                'identity:list_projects': pp.SYSTEM_READER_OR_DOMAIN_READER,
                 '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))
 

releasenotes/notes/bug-1750660-e2a360ddd6790fc4.yaml

@@ -0,0 +1,39 @@
+---
+features:
+  - |
+    [`bug 1750660 <https://bugs.launchpad.net/keystone/+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 <https://bugs.launchpad.net/keystone/+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 <https://bugs.launchpad.net/keystone/+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 <https://bugs.launchpad.net/keystone/+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.