Prohibit invalid ids in subtree and parents list

Do not allow passing invalid project_ids (None or non-existent) in
the list_project_parents() and list_projects_in_subtree() methods.

Co-Authored-By: Raildo Mascena <raildo@lsd.ufcg.edu.br>
Co-Authored-By: Erickson Santos <erickson@lsd.ufcg.edu.br>

Closes-Bug: 1425113

Change-Id: Ie0e281d60b358f551828555c3e21e3ccb18deecb

keystone/resource/backends/sql.py

@@ -91,10 +91,9 @@ class Resource(keystone_resource.Driver):
 
     def list_projects_in_subtree(self, project_id):
         with sql.transaction() as session:
-            project = self._get_project(session, project_id).to_dict()
-            children = self._get_children(session, [project['id']])
+            children = self._get_children(session, [project_id])
             subtree = []
-            examined = set([project['id']])
+            examined = set([project_id])
             while children:
                 children_ids = set()
                 for ref in children:

keystone/resource/core.py

@@ -177,7 +177,7 @@ class Manager(manager.Manager):
                              'disabled parents') % project_id)
 
     def _assert_whole_subtree_is_disabled(self, project_id):
-        subtree_list = self.driver.list_projects_in_subtree(project_id)
+        subtree_list = self.list_projects_in_subtree(project_id)
         for ref in subtree_list:
             if ref.get('enabled', True):
                 raise exception.ForbiddenAction(
@@ -244,7 +244,15 @@ class Manager(manager.Manager):
         return [proj for proj in projects_list
                 if proj['id'] in user_projects_ids]
 
+    def _assert_valid_project_id(self, project_id):
+        if project_id is None:
+            msg = _('Project field is required and cannot be empty.')
+            raise exception.ValidationError(message=msg)
+        # Check if project_id exists
+        self.get_project(project_id)
+
     def list_project_parents(self, project_id, user_id=None):
+        self._assert_valid_project_id(project_id)
         parents = self.driver.list_project_parents(project_id)
         # If a user_id was provided, the returned list should be filtered
         # against the projects this user has access to.
@@ -296,6 +304,7 @@ class Manager(manager.Manager):
         return parents_as_ids
 
     def list_projects_in_subtree(self, project_id, user_id=None):
+        self._assert_valid_project_id(project_id)
         subtree = self.driver.list_projects_in_subtree(project_id)
         # If a user_id was provided, the returned list should be filtered
         # against the projects this user has access to.

keystone/tests/unit/test_backend.py

@@ -2250,6 +2250,15 @@ class IdentityTests(object):
         # recursion trap.
         self.assertIsNone(subtree)
 
+    def test_list_projects_in_subtree_invalid_project_id(self):
+        self.assertRaises(exception.ValidationError,
+                          self.resource_api.list_projects_in_subtree,
+                          None)
+
+        self.assertRaises(exception.ProjectNotFound,
+                          self.resource_api.list_projects_in_subtree,
+                          uuid.uuid4().hex)
+
     def test_list_project_parents(self):
         projects_hierarchy = self._create_projects_hierarchy(hierarchy_size=3)
         project1 = projects_hierarchy[0]
@@ -2274,6 +2283,15 @@ class IdentityTests(object):
         parents = self.resource_api.list_project_parents(project1['id'])
         self.assertEqual(0, len(parents))
 
+    def test_list_project_parents_invalid_project_id(self):
+        self.assertRaises(exception.ValidationError,
+                          self.resource_api.list_project_parents,
+                          None)
+
+        self.assertRaises(exception.ProjectNotFound,
+                          self.resource_api.list_project_parents,
+                          uuid.uuid4().hex)
+
     def test_delete_project_with_role_assignments(self):
         tenant = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
                   'domain_id': DEFAULT_DOMAIN_ID}

keystone/tests/unit/test_backend_ldap.py

@@ -1644,7 +1644,7 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase):
         projects = self._assert_create_hierarchy_not_allowed()
         for project in projects:
             subtree_list = self.resource_api.list_projects_in_subtree(
-                project)
+                project['id'])
             self.assertEqual(0, len(subtree_list))
 
     def test_list_projects_in_subtree_with_circular_reference(self):
@@ -1653,7 +1653,8 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase):
     def test_list_project_parents(self):
         projects = self._assert_create_hierarchy_not_allowed()
         for project in projects:
-            parents_list = self.resource_api.list_project_parents(project)
+            parents_list = self.resource_api.list_project_parents(
+                project['id'])
             self.assertEqual(0, len(parents_list))
 
     def test_hierarchical_projects_crud(self):

keystone/tests/unit/test_v3_assignment.py

@@ -701,6 +701,22 @@ class AssignmentTestCase(test_v3.RestfulTestCase):
                 'project_id': self.project_id})
         self.assertValidProjectResponse(r, self.project)
 
+    def test_get_project_with_parents_as_list_with_invalid_id(self):
+        """Call ``GET /projects/{project_id}?parents_as_list``."""
+        self.get('/projects/%(project_id)s?parents_as_list' % {
+                 'project_id': None}, expected_status=404)
+
+        self.get('/projects/%(project_id)s?parents_as_list' % {
+                 'project_id': uuid.uuid4().hex}, expected_status=404)
+
+    def test_get_project_with_subtree_as_list_with_invalid_id(self):
+        """Call ``GET /projects/{project_id}?subtree_as_list``."""
+        self.get('/projects/%(project_id)s?subtree_as_list' % {
+                 'project_id': None}, expected_status=404)
+
+        self.get('/projects/%(project_id)s?subtree_as_list' % {
+                 'project_id': uuid.uuid4().hex}, expected_status=404)
+
     def test_get_project_with_parents_as_ids(self):
         """Call ``GET /projects/{project_id}?parents_as_ids``."""
         projects = self._create_projects_hierarchy(hierarchy_size=2)