From ab7f8115d45de5eb4ea76eda0996a559f6b4b508 Mon Sep 17 00:00:00 2001
From: Adrian Turjak <adriant@catalystcloud.nz>
Date: Tue, 26 Oct 2021 15:34:14 +1300
Subject: [PATCH] Fix an issue with Adjutant's Invite process not checking
 inherited roles

Adjutant's InviteUser action was not correctly checking inherited roles
which might allow someone to invite/create another user with roles outside
of those they inviting user can manage.

Change-Id: I1f45da4ce5ee6d1295a17767c432875c23106b15
Story: #2009326
---
 adjutant/actions/v1/base.py                   |  3 +-
 .../actions/v1/tests/test_user_actions.py     | 84 +++++++++++++++++++
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/adjutant/actions/v1/base.py b/adjutant/actions/v1/base.py
index 261e00e..c5b2be0 100644
--- a/adjutant/actions/v1/base.py
+++ b/adjutant/actions/v1/base.py
@@ -291,8 +291,9 @@ class UserMixin(ResourceMixin):
     def _validate_role_permissions(self):
         keystone_user = self.action.task.keystone_user
         # Role permissions check
+        requested_roles = set(list(self.roles) + list(self.inherited_roles))
         if not self.are_roles_manageable(
-            user_roles=keystone_user["roles"], requested_roles=self.roles
+            user_roles=keystone_user["roles"], requested_roles=requested_roles
         ):
             self.add_note("User does not have permission to edit role(s).")
             return False
diff --git a/adjutant/actions/v1/tests/test_user_actions.py b/adjutant/actions/v1/tests/test_user_actions.py
index 3f105db..42f83f7 100644
--- a/adjutant/actions/v1/tests/test_user_actions.py
+++ b/adjutant/actions/v1/tests/test_user_actions.py
@@ -374,6 +374,51 @@ class UserActionTests(AdjutantTestCase):
         action.prepare()
         self.assertFalse(action.valid)
 
+    def test_new_user_invalid_roles(self):
+        """
+        Test that you can't add roles outside of managed roles.
+
+        Action should be invalid.
+        """
+
+        project = fake_clients.FakeProject(name="test_project")
+
+        setup_identity_cache(projects=[project])
+
+        task = Task.objects.create(
+            keystone_user={
+                "roles": ["project_admin"],
+                "project_id": project.id,
+                "project_domain_id": "default",
+            }
+        )
+
+        data = {
+            "email": "test@example.com",
+            "project_id": project.id,
+            "roles": ["admin"],
+            "inherited_roles": [],
+            "domain_id": "default",
+        }
+
+        action = NewUserAction(data, task=task, order=1)
+
+        action.prepare()
+        self.assertFalse(action.valid)
+
+        data = {
+            "email": "test@example.com",
+            "project_id": project.id,
+            "roles": [],
+            "inherited_roles": ["admin"],
+            "domain_id": "default",
+        }
+
+        action = NewUserAction(data, task=task, order=1)
+
+        action.prepare()
+        self.assertFalse(action.valid)
+
     def test_new_user_wrong_domain(self):
         """
         Existing user, valid project, invalid domain.
@@ -780,6 +825,18 @@ class UserActionTests(AdjutantTestCase):
                 role_name="project_admin",
                 user={"id": user.id},
             ),
+            fake_clients.FakeRoleAssignment(
+                scope={"project": {"id": project.id}},
+                role_name="member",
+                user={"id": user.id},
+                inherited=True,
+            ),
+            fake_clients.FakeRoleAssignment(
+                scope={"project": {"id": project.id}},
+                role_name="project_admin",
+                user={"id": user.id},
+                inherited=True,
+            ),
         ]
 
         setup_identity_cache(
@@ -813,6 +870,33 @@ class UserActionTests(AdjutantTestCase):
         roles = fake_client._get_roles_as_names(user, project)
         self.assertEqual(roles, ["member", "project_admin"])
 
+        task = Task.objects.create(
+            keystone_user={
+                "roles": ["project_mod"],
+                "project_id": project.id,
+                "project_domain_id": "default",
+            }
+        )
+
+        data = {
+            "domain_id": "default",
+            "user_id": user.id,
+            "project_id": project.id,
+            "roles": [],
+            "inherited_roles": ["project_mod"],
+            "remove": False,
+        }
+
+        action = EditUserRolesAction(data, task=task, order=1)
+
+        action.prepare()
+        self.assertEqual(action.valid, False)
+
+        fake_client = fake_clients.FakeManager()
+
+        roles = fake_client._get_roles_as_names(user, project)
+        self.assertEqual(roles, ["member", "project_admin"])
+
     def test_edit_user_roles_modified_config(self):
         """
         Tests that the role mappings do come from config and that they