Fix user member sync on django

Author: filippomc

infrastructure/common-images/cloudharness-django/libraries/cloudharness-django/cloudharness_django/middleware.py

@@ -15,35 +15,82 @@
 
 
 def _get_user(kc_user_id: str) -> User:
+    """
+    Get or create a Django user for the given Keycloak user ID.
+    
+    CRITICAL SAFETY GUARANTEE: This function will NEVER return a User without a valid Member.
+    If we cannot ensure a Member exists, we return None (which triggers anonymous user behavior).
+    
+    Returns:
+        User: A Django User with a guaranteed Member relationship, or None for anonymous
+    """
     user = None
     if kc_user_id is None:
         return None
-        # found bearer token get the Django user
+    
     try:
+        # Try to get existing user by member relationship
         try:
             user = User.objects.get(member__kc_id=kc_user_id)
+            
+            # SAFETY CHECK: Verify member relationship is intact
+            try:
+                _ = user.member  # Access to verify it exists
+            except Member.DoesNotExist:
+                # Member was deleted between the query and now - return None for safety
+                log.error("User %s found but Member missing. Returning anonymous.", user.id)
+                return None
+                
         except User.DoesNotExist:
+            # User doesn't exist - create it via sync_kc_user
             user_svc = get_user_service()
             kc_user = user_svc.auth_client.get_current_user()
             try:
+                # sync_kc_user is atomic and guarantees Member creation
                 user = user_svc.sync_kc_user(kc_user)
                 user_svc.sync_kc_user_groups(kc_user)
+                
+                # SAFETY CHECK: Final verification that Member exists
+                try:
+                    _ = user.member
+                except Member.DoesNotExist:
+                    # This should NEVER happen due to sync_kc_user safety, but be defensive
+                    log.error("sync_kc_user returned user %s without Member! Returning anonymous.", user.id)
+                    return None
+                    
             except UniqueViolation as e:
-                # this can happen as a race condition while creating the Member object
+                # Race condition while creating the Member object
                 log.warning("UniqueViolation error for kc_id %s. Probably a race condition. %s", kc_user_id, str(e))
-                return User.objects.get(member__kc_id=kc_user_id)  # If it still fails, we are missing something serious
-
+                # Try to get the user again
+                try:
+                    user = User.objects.get(member__kc_id=kc_user_id)
+                    # Verify member exists
+                    _ = user.member
+                except (User.DoesNotExist, Member.DoesNotExist):
+                    log.error("Failed to retrieve user after UniqueViolation. Returning anonymous.")
+                    return None
+                    
         except User.MultipleObjectsReturned:
             # Race condition, multiple users created for the same kc_id
             log.warning("Multiple users found for kc_id %s, cleaning up...", kc_user_id)
             user = User.objects.filter(member__kc_id=kc_user_id).order_by('id').first()
             User.objects.filter(member__kc_id=kc_user_id).exclude(id=user.id).delete()
+            
+            # SAFETY CHECK: Verify the kept user has a Member
+            try:
+                _ = user.member
+            except:
+                log.error("Cleaned up user %s has no Member. Returning anonymous.", user.id)
+                return None
+            
             return user
 
         except Exception as e:
             log.exception("User sync error, %s", kc_user.email)
             return None
+            
         return user
+        
     except KeycloakGetError:
         # KC user not found
         return None
@@ -68,9 +115,19 @@ def __call__(self, request):
             if not user or user.is_anonymous or getattr(user, "member", None) is None or user.member.kc_id != kc_user.id:
                 user = _get_user(kc_user)
                 if user:
-                    # auto login, set the user
-                    request.user = user
-                    request._cached_user = user
+                    # CRITICAL SAFETY CHECK: Never assign user without Member
+                    # This is the final defense to ensure request.user always has a Member
+                    try:
+                        _ = user.member  # Verify member exists
+                        # Safe to assign - user has a valid Member
+                        request.user = user
+                        request._cached_user = user
+                    except:
+                        # This should NEVER happen due to _get_user safety checks,
+                        # but if it does, DO NOT assign the user - keep anonymous
+                        log.error("CRITICAL: _get_user returned user %s without Member! Keeping anonymous.", user.id)
+                        logout(request)
+                        # Don't assign user - request will remain anonymous
         # elif not request.path.startswith('/admin/'):
         #     logout(request)
 
@@ -85,5 +142,23 @@ def authenticate(self, request):
             return None
         user: User = getattr(request._request, 'user', None)
         if user and user.is_authenticated:
-            return (user, None)
-        return (_get_user(kc_user), None)
+            # SAFETY CHECK: Verify authenticated user has Member
+            try:
+                _ = user.member
+                return (user, None)
+            except Member.DoesNotExist:
+                log.error("Authenticated user %s has no Member! Falling back to _get_user.", user.id)
+                # Fall through to _get_user which will handle this safely
+        
+        # Get or create user with guaranteed Member
+        user = _get_user(kc_user)
+        if user:
+            # FINAL SAFETY CHECK: Ensure Member exists before returning
+            try:
+                _ = user.member
+                return (user, None)
+            except Member.DoesNotExist:
+                log.error("CRITICAL: _get_user returned user %s without Member! Returning None.", user.id)
+                return None
+        
+        return None

infrastructure/common-images/cloudharness-django/libraries/cloudharness-django/cloudharness_django/services/user.py

@@ -1,4 +1,5 @@
 from django.contrib.auth.models import User, Group
+from django.db import transaction
 
 from cloudharness_django.models import Team, Member
 from cloudharness_django.services.auth import AuthService, AuthorizationLevel
@@ -96,28 +97,47 @@ def sync_kc_groups(self, kc_groups=None):
         for kc_group in kc_groups:
             self.sync_kc_group(kc_group)
 
+    @transaction.atomic
     def sync_kc_user(self, kc_user: ch_models.User, is_superuser=False, delete=False):
-        # sync the kc user with the django user using kc_id for reliable lookups
+        """
+        Sync the kc user with the django user using kc_id for reliable lookups.
+        ALWAYS creates both User and Member atomically - never returns a User without a Member.
+        """
         user = get_user_by_kc_id(kc_user.id)
 
         if user is None:
-            # User doesn't exist, create new one
+            # User doesn't exist, create new one WITH Member atomically
             username = kc_user.username or kc_user.email
             if not username:
                 raise ValueError(f"Keycloak user {kc_user.id} has no username or email")
 
-            user, _ = User.objects.get_or_create(username=username)
+            # Create user and member atomically
+            user, user_created = User.objects.get_or_create(username=username)
+            
+            # CRITICAL: Ensure Member exists before proceeding
             try:
                 member = Member.objects.get(user=user)
-                member.kc_id = kc_user.id
-                member.save()
+                # Member exists but might have wrong kc_id
+                if member.kc_id != kc_user.id:
+                    member.kc_id = kc_user.id
+                    member.save()
             except Member.DoesNotExist:
                 # Create the member relationship
                 Member.objects.create(kc_id=kc_user.id, user=user)
 
+        # Update user attributes
         user = self._map_kc_user(user, kc_user, is_superuser, delete)
         self.sync_kc_user_groups(kc_user)
         user.save()
+        
+        # FINAL SAFETY CHECK: Verify Member exists before returning
+        # This ensures we never return a user without a Member
+        try:
+            _ = user.member  # Access member to trigger DoesNotExist if missing
+        except:
+            # This should never happen, but if it does, create Member immediately
+            Member.objects.create(kc_id=kc_user.id, user=user)
+        
         return user
 
     def sync_kc_user_groups(self, kc_user: ch_models.User):