Merge pull request RedHatInsights#1243 from RedHatInsights/rhcloud-35…

…742-no-default-group-seed-replication

RHCLOUD-35742: Seeded default group-roles attempt replication in add_role but these shouldn't

rbac/management/group/definer.py

@@ -167,8 +167,9 @@ def add_roles(group, roles_or_role_ids, tenant, user=None):
 
         system_policy.roles.add(role)
 
-        dual_write_handler = RelationApiDualWriteGroupHandler(group, ReplicationEventType.ASSIGN_ROLE, [])
-        dual_write_handler.replicate_added_role(role)
+        if tenant.tenant_name != "public":
+            dual_write_handler = RelationApiDualWriteGroupHandler(group, ReplicationEventType.ASSIGN_ROLE)
+            dual_write_handler.replicate_added_role(role)
         # Send notifications
         group_role_change_notification_handler(user, group, role, "added")
 
@@ -192,8 +193,9 @@ def remove_roles(group, roles_or_role_ids, tenant, user=None):
                 policy.roles.remove(role)
                 logger.info(f"Removing role {role} from group {group.name} for tenant {tenant.org_id}.")
 
-                dual_write_handler = RelationApiDualWriteGroupHandler(group, ReplicationEventType.UNASSIGN_ROLE, [])
-                dual_write_handler.replicate_removed_role(role)
+                if tenant.tenant_name != "public":
+                    dual_write_handler = RelationApiDualWriteGroupHandler(group, ReplicationEventType.UNASSIGN_ROLE)
+                    dual_write_handler.replicate_removed_role(role)
 
                 # Send notifications
                 group_role_change_notification_handler(user, group, role, "removed")

rbac/management/group/relation_api_dual_write_group_handler.py

@@ -59,8 +59,9 @@ def __init__(
             self.group_relations_to_remove = []
             self.principals = []
             self.group = group
-            self.tenant = group.tenant
-            self.default_workspace = Workspace.objects.get(tenant=self.tenant, type=Workspace.Types.DEFAULT)
+            self.default_workspace = Workspace.objects.get(
+                tenant_id=self.group.tenant_id, type=Workspace.Types.DEFAULT
+            )
             self.event_type = event_type
             self.user_domain = settings.PRINCIPAL_USER_DOMAIN
             self._replicator = replicator if replicator else OutboxReplicator()
@@ -126,9 +127,6 @@ def replicate_added_role(self, role: Role):
         """Replicate added role."""
         if not self.replication_enabled():
             return
-        # TODO - This needs to be removed to seed the default groups.
-        if self.group.tenant.tenant_name == "public":
-            return
 
         def add_group_to_binding(mapping: BindingMapping):
             self.group_relations_to_add.append(mapping.add_group_to_bindings(str(self.group.uuid)))
@@ -155,9 +153,6 @@ def replicate_removed_role(self, role: Role):
         """Replicate removed role."""
         if not self.replication_enabled():
             return
-        # TODO - This needs to be removed to seed the default groups.
-        if self.group.tenant.tenant_name == "public":
-            return
 
         self._update_mapping_for_role_removal(role)
         self._replicate()
@@ -184,9 +179,6 @@ def _update_mapping_for_role(
         """
         if not self.replication_enabled():
             return
-        # TODO - This needs to be removed to seed the default groups.
-        if self.group.tenant.tenant_name == "public":
-            return
 
         if role.system:
             try:

rbac/management/tenant_service/v2.py

@@ -245,6 +245,9 @@ def _get_or_bootstrap_tenant(self, org_id: str, account_number: Optional[str] =
             return bootstrap
 
     def _bootstrap_tenant(self, tenant: Tenant) -> BootstrappedTenant:
+        if tenant.tenant_name == "public":
+            raise ValueError("Cannot bootstrap public tenant.")
+
         # Set up workspace hierarchy for Tenant
         root_workspace = Workspace.objects.create(
             tenant=tenant,

tests/management/group/test_definer.py

@@ -33,24 +33,9 @@ def setUp(self):
         """Set up the group definer tests."""
         super().setUp()
         self.public_tenant = Tenant.objects.get(tenant_name="public")
-        self.root_workspace = Workspace.objects.create(
-            type=Workspace.Types.ROOT,
-            name="Root",
-            tenant=self.public_tenant,
-        )
-        self.default_workspace = Workspace.objects.create(
-            type=Workspace.Types.DEFAULT,
-            name="Default",
-            tenant=self.public_tenant,
-            parent=self.root_workspace,
-        )
         seed_roles()
         seed_group()
 
-    def tearDown(self):
-        Workspace.objects.filter(parent__isnull=False).delete()
-        Workspace.objects.filter(parent__isnull=True).delete()
-
     def test_default_group_seeding_properly(self):
         """Test that default group are seeded properly."""
         group = Group.objects.get(platform_default=True)

tests/management/role/test_dual_write.py

@@ -16,7 +16,6 @@
 #
 """Test tuple changes for RBAC operations."""
 
-import unittest
 from typing import Optional, Tuple
 from django.test import TestCase, override_settings
 from django.db.models import Q
@@ -28,7 +27,7 @@
 from management.policy.model import Policy
 from management.principal.model import Principal
 from management.relation_replicator.noop_replicator import NoopReplicator
-from management.relation_replicator.relation_replicator import ReplicationEventType
+from management.relation_replicator.relation_replicator import DualWriteException, ReplicationEventType
 from management.role.model import Access, ResourceDefinition, Role, BindingMapping
 from management.role.relation_api_dual_write_handler import (
     RelationApiDualWriteHandler,
@@ -158,7 +157,7 @@ def given_additional_group_members(
         principals = self.fixture.add_members_to_group(group, users, service_accounts, group.tenant)
         dual_write = RelationApiDualWriteGroupHandler(
             group,
-            ReplicationEventType.CREATE_GROUP,
+            ReplicationEventType.ADD_PRINCIPALS_TO_GROUP,
             replicator=InMemoryRelationReplicator(self.tuples),
         )
         dual_write.replicate_new_principals(principals)
@@ -171,7 +170,7 @@ def given_removed_group_members(
         principals = self.fixture.remove_members_from_group(group, users, service_accounts, group.tenant)
         dual_write = RelationApiDualWriteGroupHandler(
             group,
-            ReplicationEventType.CREATE_GROUP,
+            ReplicationEventType.REMOVE_PRINCIPALS_FROM_GROUP,
             replicator=InMemoryRelationReplicator(self.tuples),
         )
         dual_write.replicate_removed_principals(principals)
@@ -311,6 +310,16 @@ def expect_role_bindings_to_workspace(
 class DualWriteGroupTestCase(DualWriteTestCase):
     """Test dual write logic for group modifications."""
 
+    def test_cannot_replicate_group_for_public_tenant(self):
+        """Do not replicate group changes for the public tenant groups (system groups)."""
+        platform_default, admin_default = seed_group()
+
+        with self.assertRaises(DualWriteException):
+            RelationApiDualWriteGroupHandler(platform_default, ReplicationEventType.CREATE_GROUP)
+
+        with self.assertRaises(DualWriteException):
+            RelationApiDualWriteGroupHandler(admin_default, ReplicationEventType.CREATE_GROUP)
+
     def test_create_group_tuples(self):
         """Create a group and add users to it."""
         group, principals = self.given_group("g1", ["u1", "u2"])

tests/management/tenant/test_model.py

@@ -49,6 +49,10 @@ def setUp(self):
         self.fixture = RbacFixture(self.service)
         self.default_group, self.admin_group = seed_group()
 
+    def test_prevents_bootstrapping_public_tenant(self):
+        with self.assertRaises(ValueError):
+            self.service.bootstrap_tenant(self.fixture.public_tenant)
+
     def test_relates_workspace_tenant_platform_hierarchy(self):
         bootstrapped = self.fixture.new_tenant(org_id="o1")
         root = self.fixture.root_workspace(bootstrapped.tenant)