Merge pull request RedHatInsights#1459 from astrozzc/user_id

[RHCLOUD-37452] Get user id if not set

rbac/management/tenant_service/__init__.py

@@ -1,12 +1,22 @@
 """Tenant management business logic."""
 
+from typing import Callable, Optional
+
 from django.conf import settings
 from management.relation_replicator.relation_replicator import RelationReplicator
 from management.tenant_service.tenant_service import TenantBootstrapService
 from management.tenant_service.v1 import V1TenantBootstrapService
 from management.tenant_service.v2 import V2TenantBootstrapService
 
+from api.models import User
+
 
-def get_tenant_bootstrap_service(replicator: RelationReplicator) -> "TenantBootstrapService":
+def get_tenant_bootstrap_service(
+    replicator: RelationReplicator, get_user_id: Optional[Callable[[User], str]] = None
+) -> "TenantBootstrapService":
     """Get a TenantBootstrapService instance based on settings."""
-    return V2TenantBootstrapService(replicator) if settings.V2_BOOTSTRAP_TENANT else V1TenantBootstrapService()
+    return (
+        V2TenantBootstrapService(replicator, get_user_id=get_user_id)
+        if settings.V2_BOOTSTRAP_TENANT
+        else V1TenantBootstrapService()
+    )

rbac/management/tenant_service/v2.py

@@ -1,6 +1,6 @@
 """V2 implementation of Tenant bootstrapping."""
 
-from typing import List, Optional
+from typing import Callable, List, Optional
 from uuid import UUID
 
 from django.conf import settings
@@ -24,6 +24,13 @@
 from api.models import Tenant, User
 
 
+def default_get_user_id(user: User):
+    """Get user ID."""
+    if user.user_id is None:
+        raise ValueError(f"Cannot update user without user_id. username={user.username}")
+    return user.user_id
+
+
 class V2TenantBootstrapService:
     """Service for bootstrapping tenants with built-in relationships."""
 
@@ -33,10 +40,16 @@ class V2TenantBootstrapService:
     _platform_default_policy_uuid: Optional[str] = None
     _admin_default_policy_uuid: Optional[str] = None
 
-    def __init__(self, replicator: RelationReplicator, public_tenant: Optional[Tenant] = None):
+    def __init__(
+        self,
+        replicator: RelationReplicator,
+        public_tenant: Optional[Tenant] = None,
+        get_user_id: Optional[Callable[[User], str]] = None,
+    ):
         """Initialize the TenantBootstrapService with a RelationReplicator."""
         self._replicator = replicator
         self._public_tenant = public_tenant
+        self._get_user_id = get_user_id if get_user_id else default_get_user_id
 
     def new_bootstrapped_tenant(self, org_id: str, account_number: Optional[str] = None) -> BootstrappedTenant:
         """Create a new tenant."""
@@ -90,10 +103,7 @@ def update_user(
         if mapping is None:
             raise ValueError(f"Expected TenantMapping but got None. org_id: {bootstrapped_tenant.tenant.org_id}")
 
-        user_id = user.user_id
-
-        if user_id is None:
-            raise ValueError(f"Cannot update user without user_id. username={user.username}")
+        user_id = self._get_user_id(user)
 
         tuples_to_add = []
         tuples_to_remove = []
@@ -205,13 +215,10 @@ def _disable_user_in_tenant(self, user: User):
 
         # Get tenant mapping if present but no need to create if not
         tuples_to_remove = []
-        user_id = user.user_id
+        user_id = self._get_user_id(user)
         mapping: Optional[TenantMapping] = None
         principal_uuid = ""
 
-        if user_id is None:
-            raise ValueError(f"User {user.username} has no user_id.")
-
         logger.info(
             f"Removing Principal and group membership from RBAC and Relations. user_id={user_id} org_id={user.org_id}"
         )
@@ -409,10 +416,7 @@ def _default_group_tuple_edits(self, user: User, mapping) -> tuple[list[Relation
         """Get the tuples to add and remove for a user."""
         tuples_to_add = []
         tuples_to_remove = []
-        user_id = user.user_id
-
-        if user_id is None:
-            raise ValueError(f"User {user.username} has no user_id.")
+        user_id = self._get_user_id(user)
 
         tuples_to_add.append(Group.relationship_to_user_id_for_group(str(mapping.default_group_uuid), user_id))
 

rbac/rbac/middleware.py

@@ -29,6 +29,7 @@
 from django.utils.deprecation import MiddlewareMixin
 from management.cache import TenantCache
 from management.models import Principal
+from management.principal.proxy import PrincipalProxy
 from management.relation_replicator.outbox_replicator import OutboxReplicator
 from management.tenant_service import get_tenant_bootstrap_service
 from management.tenant_service.tenant_service import TenantBootstrapService
@@ -90,6 +91,24 @@ class HttpResponseUnauthorizedRequest(HttpResponse):
     status_code = 401
 
 
+PROXY = PrincipalProxy()
+
+
+def get_user_id(user: User):
+    """Return the user ID for the given user."""
+    user_id = user.user_id
+    if not user_id:
+        resp = PROXY.request_filtered_principals([user.username], org_id=user.org_id, options={"return_id": True})
+        if isinstance(resp, dict) and "errors" in resp:
+            logging.warning(resp.get("errors"))
+            return
+        if not resp.get("data"):
+            logging.warning(f"No user found of user name {user.username}.")
+            raise Http404()
+        return resp["data"][0]["user_id"]
+    return user.user_id
+
+
 class IdentityHeaderMiddleware(MiddlewareMixin):
     """A subclass of RemoteUserMiddleware.
 
@@ -106,7 +125,7 @@ def __init__(self, get_response):
         # In this case the replicator needs to include a precondition
         # which does not add the tuples if any others already exist for the tenant
         # (the tx will be rolled back in that case)
-        self.bootstrap_service = get_tenant_bootstrap_service(OutboxReplicator())
+        self.bootstrap_service = get_tenant_bootstrap_service(OutboxReplicator(), get_user_id)
 
     def get_tenant(self, model, hostname, request):
         """Override the tenant selection logic."""

tests/management/tenant/test_model.py

@@ -427,6 +427,31 @@ def test_force_bootstrap_replicates_already_bootstrapped_ready_tenants(self):
         self.assertEqual(original_mapping, new_mapping)
         self.assertCountEqual(original_workspaces, new_workspaces)
 
+    def test_inject_get_user_id_method(self):
+        bootstrapped = self.fixture.new_tenant(org_id="o1")
+
+        user = User()
+        user.org_id = "o1"
+        user.admin = False
+        user.is_active = True
+        user.user_name = "user_1"
+
+        get_user_id = lambda user: "u1"
+        service = V2TenantBootstrapService(InMemoryRelationReplicator(self.tuples), get_user_id=get_user_id)
+        bootstrapped = service.update_user(user)
+
+        # But is a regular user
+        self.assertEqual(
+            1,
+            self.tuples.count_tuples(
+                all_of(
+                    resource("rbac", "group", bootstrapped.mapping.default_group_uuid),
+                    relation("member"),
+                    subject("rbac", "principal", "localhost/u1"),
+                )
+            ),
+        )
+
     def assertAddedToDefaultGroup(self, user_id: str, tenant_mapping: TenantMapping, and_admin_group: bool = False):
         self.assertEqual(
             1,

tests/rbac/test_middleware.py

@@ -776,6 +776,45 @@ def test_bootstraps_tenants_if_not_existing(self):
                 ),
             )
 
+    @patch(
+        "management.principal.proxy.PrincipalProxy.request_filtered_principals",
+        return_value={
+            "status_code": 200,
+            "data": [
+                {
+                    "username": "user_1",
+                    "email": "[email protected]",
+                    "first_name": "user",
+                    "last_name": "test",
+                    "user_id": "u1",
+                    "org_id": "12345",
+                }
+            ],
+        },
+    )
+    def test_bootstraps_tenants_if_user_id_is_missing(self, _):
+        with patch("rbac.middleware.OutboxReplicator", new=partial(InMemoryRelationReplicator, self._tuples)):
+            # Change the user's org so we create a new tenant
+            self.request.user.org_id = "12345"
+            self.org_id = "12345"
+            self.request.user.id = None
+            mock_request = self.request
+            tenant_cache = TenantCache()
+            tenant_cache.delete_tenant(self.org_id)
+            middleware = IdentityHeaderMiddleware(get_response=IdentityHeaderMiddleware.get_tenant)
+            result = middleware.get_tenant(Tenant, "localhost", mock_request)
+            self.assertEqual(result.org_id, mock_request.user.org_id)
+            tenant = Tenant.objects.get(org_id=self.org_id)
+            self.assertIsNotNone(tenant)
+            mapping = TenantMapping.objects.get(tenant=tenant)
+            self.assertIsNotNone(mapping)
+            workspaces = list(Workspace.objects.filter(tenant=tenant))
+            self.assertEqual(len(workspaces), 2)
+            default = Workspace.objects.get(type=Workspace.Types.DEFAULT, tenant=tenant)
+            self.assertIsNotNone(default)
+            root = Workspace.objects.get(type=Workspace.Types.ROOT, tenant=tenant)
+            self.assertIsNotNone(root)
+
 
 @override_settings(V2_BOOTSTRAP_TENANT=True)
 class V2IdentityHeaderMiddlewareTest(IdentityHeaderMiddlewareTest):