1

2025-11-16 20:02:45 +00:00
parent 9f3c4a6cdd
commit b2e60b749a
3 changed files with 283 additions and 23 deletions
--- a/docs/DIAGNOSIS-AUTHENTICATION-ISSUE.md
+++ b/docs/DIAGNOSIS-AUTHENTICATION-ISSUE.md
@@ -0,0 +1,273 @@
+# Authentication & Account Context Diagnosis
+
+## Issue Summary
+**Problem**: Wrong user showing without proper rights - authentication/account context mismatch
+
+## Architecture Overview
+
+### Request Flow
+```
+1. Request arrives
+   ↓
+2. Django Middleware Stack (settings.py:74-88)
+   - SecurityMiddleware
+   - WhiteNoiseMiddleware
+   - CorsMiddleware
+   - SessionMiddleware
+   - CommonMiddleware
+   - CsrfViewMiddleware
+   - AuthenticationMiddleware (sets request.user from session)
+   ↓
+3. AccountContextMiddleware (line 83)
+   - Extracts account from JWT token OR session
+   - Sets request.account
+   ↓
+4. DRF Authentication Classes (settings.py:210-214)
+   - JWTAuthentication (runs first)
+   - CSRFExemptSessionAuthentication
+   - BasicAuthentication
+   ↓
+5. View/ViewSet
+   - Uses request.user (from DRF auth)
+   - Uses request.account (from middleware OR JWTAuthentication)
+```
+
+## Critical Issues Found
+
+### Issue #1: Duplicate Account Setting Logic
+**Location**: Two places set `request.account` with different logic
+
+1. **AccountContextMiddleware** (`backend/igny8_core/auth/middleware.py:99-106`)
+   ```python
+   if account_id:
+       account = Account.objects.get(id=account_id)
+       # If user's account changed, use the new one from user object
+       if user.account and user.account.id != account_id:
+           request.account = user.account  # Prioritizes user's current account
+       else:
+           request.account = account  # Uses token's account
+   ```
+
+2. **JWTAuthentication** (`backend/igny8_core/api/authentication.py:64-80`)
+   ```python
+   account_id = payload.get('account_id')
+   account = None
+   if account_id:
+       account = Account.objects.get(id=account_id)  # Always uses token's account
+   
+   if not account:
+       account = getattr(user, 'account', None)  # Fallback only if no account_id
+   
+   request.account = account  # OVERWRITES middleware's account
+   ```
+
+**Problem**: 
+- Middleware validates if user's account changed and prioritizes `user.account`
+- JWTAuthentication runs AFTER middleware and OVERWRITES `request.account` without validation
+- This means middleware's validation is ignored
+
+### Issue #2: User Object Loading Mismatch
+**Location**: Different user loading strategies
+
+1. **AccountContextMiddleware** (line 98)
+   ```python
+   user = User.objects.select_related('account', 'account__plan').get(id=user_id)
+   ```
+   - Loads user WITH account relationship (efficient, has account data)
+
+2. **JWTAuthentication** (line 58)
+   ```python
+   user = User.objects.get(id=user_id)
+   ```
+   - Does NOT load account relationship
+   - When checking `user.account`, it triggers a separate DB query
+   - If account relationship is stale or missing, this can fail
+
+**Problem**:
+- JWTAuthentication's user object doesn't have account relationship loaded
+- When `/me` endpoint uses `request.user.id` and then serializes with `UserSerializer`, it tries to access `user.account`
+- This might trigger lazy loading which could return wrong/stale data
+
+### Issue #3: Middleware Updates request.user (Session Auth)
+**Location**: `backend/igny8_core/auth/middleware.py:32-46`
+
+```python
+if hasattr(request, 'user') and request.user and request.user.is_authenticated:
+    user = UserModel.objects.select_related('account', 'account__plan').get(id=request.user.id)
+    request.user = user  # OVERWRITES request.user
+    request.account = user_account
+```
+
+**Problem**:
+- Middleware is setting `request.user` for session authentication
+- But then JWTAuthentication runs and might set a DIFFERENT user (from JWT token)
+- This creates a conflict where middleware's user is overwritten
+
+### Issue #4: Token Account vs User Account Mismatch
+**Location**: Token generation vs user's current account
+
+**Token Generation** (`backend/igny8_core/auth/utils.py:30-57`):
+```python
+def generate_access_token(user, account=None):
+    if account is None:
+        account = getattr(user, 'account', None)
+    
+    payload = {
+        'user_id': user.id,
+        'account_id': account.id if account else None,  # Token stores account_id at login time
+        ...
+    }
+```
+
+**Problem**:
+- Token is generated at login with `user.account` at that moment
+- If user's account changes AFTER login (e.g., admin moves user to different account), token still has old `account_id`
+- Middleware tries to handle this (line 103-104), but JWTAuthentication overwrites it
+
+### Issue #5: /me Endpoint Uses request.user Without Account Relationship
+**Location**: `backend/igny8_core/auth/urls.py:188-197`
+
+```python
+def get(self, request):
+    user = UserModel.objects.select_related('account', 'account__plan').get(id=request.user.id)
+    serializer = UserSerializer(user)
+    return success_response(data={'user': serializer.data}, request=request)
+```
+
+**Problem**:
+- `/me` endpoint correctly loads user with account relationship
+- BUT `request.user` (from JWTAuthentication) doesn't have account relationship loaded
+- If other code uses `request.user.account` directly, it might get wrong/stale data
+
+## Root Cause Analysis
+
+### Primary Root Cause
+**JWTAuthentication overwrites `request.account` set by middleware without validating if user's account changed**
+
+### Secondary Issues
+1. JWTAuthentication doesn't load user with account relationship (inefficient + potential stale data)
+2. Middleware sets `request.user` for session auth, but JWTAuthentication might overwrite it
+3. Token's `account_id` can become stale if user's account changes after login
+
+## Data Flow Problem
+
+### Current Flow (BROKEN)
+```
+1. Request with JWT token arrives
+   ↓
+2. AccountContextMiddleware runs:
+   - Decodes JWT token
+   - Gets user_id=5, account_id=10
+   - Loads User(id=5) with account relationship
+   - Checks: user.account.id = 12 (user moved to account 12)
+   - Sets: request.account = Account(id=12) ✅ CORRECT
+   ↓
+3. JWTAuthentication runs:
+   - Decodes JWT token (again)
+   - Gets user_id=5, account_id=10
+   - Loads User(id=5) WITHOUT account relationship
+   - Gets Account(id=10) from token
+   - Sets: request.account = Account(id=10) ❌ WRONG (overwrites middleware)
+   - Sets: request.user = User(id=5) (without account relationship)
+   ↓
+4. View uses request.account (WRONG - account 10 instead of 12)
+5. View uses request.user.account (might trigger lazy load, could be stale)
+```
+
+### Expected Flow (CORRECT)
+```
+1. Request with JWT token arrives
+   ↓
+2. AccountContextMiddleware runs:
+   - Sets request.account based on token with validation
+   ↓
+3. JWTAuthentication runs:
+   - Sets request.user with account relationship loaded
+   - Does NOT overwrite request.account (respects middleware)
+   ↓
+4. View uses request.account (CORRECT - from middleware)
+5. View uses request.user.account (CORRECT - loaded with relationship)
+```
+
+## Database Schema Check
+
+### User Model
+- `User.account` = ForeignKey to Account, `db_column='tenant_id'`, nullable
+- Relationship: User → Account (many-to-one)
+
+### Account Model
+- `Account.owner` = ForeignKey to User
+- Relationship: Account → User (many-to-one, owner)
+
+### Potential Database Issues
+- If `User.account_id` (tenant_id column) doesn't match token's `account_id`, there's a mismatch
+- If user's account was changed in DB but token wasn't refreshed, token has stale account_id
+
+## Permission System Check
+
+### HasTenantAccess Permission
+**Location**: `backend/igny8_core/api/permissions.py:25-67`
+
+```python
+def has_permission(self, request, view):
+    account = getattr(request, 'account', None)
+    
+    # If no account in request, try to get from user
+    if not account and hasattr(request.user, 'account'):
+        account = request.user.account
+    
+    # Check if user belongs to this account
+    if account:
+        user_account = request.user.account
+        return user_account == account or user_account.id == account.id
+```
+
+**Problem**:
+- Permission checks `request.account` vs `request.user.account`
+- If `request.account` is wrong (from JWTAuthentication overwrite), permission check fails
+- User gets 403 Forbidden even though they should have access
+
+## Recommendations (Diagnosis Only - No Code Changes)
+
+### Fix Priority
+
+1. **CRITICAL**: Make JWTAuthentication respect middleware's `request.account` OR remove duplicate logic
+   - Option A: JWTAuthentication should check if `request.account` already exists and not overwrite it
+   - Option B: Remove account setting from JWTAuthentication, let middleware handle it
+
+2. **HIGH**: Load user with account relationship in JWTAuthentication
+   - Change `User.objects.get(id=user_id)` to `User.objects.select_related('account', 'account__plan').get(id=user_id)`
+
+3. **MEDIUM**: Don't set `request.user` in middleware for JWT auth
+   - Middleware should only set `request.user` for session auth
+   - For JWT auth, let JWTAuthentication handle `request.user`
+
+4. **LOW**: Add validation in token generation to ensure account_id matches user.account
+   - Or add token refresh mechanism when user's account changes
+
+### Architecture Decision Needed
+
+**Question**: Should `request.account` be set by:
+- A) Middleware only (current middleware logic with validation)
+- B) JWTAuthentication only (simpler, but loses validation)
+- C) Both, but JWTAuthentication checks if middleware already set it
+
+**Recommendation**: Option C - Middleware sets it with validation, JWTAuthentication only sets if not already set
+
+## Files Involved
+
+1. `backend/igny8_core/auth/middleware.py` - AccountContextMiddleware
+2. `backend/igny8_core/api/authentication.py` - JWTAuthentication
+3. `backend/igny8_core/auth/urls.py` - MeView endpoint
+4. `backend/igny8_core/auth/utils.py` - Token generation
+5. `backend/igny8_core/api/permissions.py` - HasTenantAccess permission
+6. `backend/igny8_core/settings.py` - Middleware and authentication class order
+
+## Testing Scenarios to Verify
+
+1. **User with account_id in token matches user.account** → Should work
+2. **User's account changed after login (token has old account_id)** → Currently broken
+3. **User with no account in token** → Should fallback to user.account
+4. **Developer/admin user** → Should bypass account checks
+5. **Session auth vs JWT auth** → Both should work consistently
+