Umbrella MCP Server

# Fix Implementation Summary: Cognito User Detection Issue ## Date: 2025-11-10 ## Problem Statement Cognito legacy users (like `david+saola@umbrellacost.com`) were being incorrectly detected as Keycloak UM 2.0 users, causing: - Wrong API key format (empty divisionId instead of numeric) - 500 errors on API requests - Authentication failures ### Root Cause The code in `dual-auth.ts:479-492` was re-detecting the user management system on every OAuth API request by calling the unreliable `/v1/user-management/users/user-realm` API endpoint, which returns realm data for users still on Cognito. This ignored which authentication method actually succeeded during login. ## Solution Implemented (Option B) **Store the actual authentication method in the OAuth JWT token** instead of re-detecting it on every request. ## Files Modified ### 1. `/src/types.ts` **Change**: Added `userManagementInfo` field to `AuthToken` interface ```typescript import type { UserManagementInfo } from './dual-auth.js'; export interface AuthToken { Authorization: string; apikey?: string; userManagementInfo?: UserManagementInfo; // NEW: Actual auth method that succeeded } ``` ### 2. `/src/auth/token-manager.ts` **Change**: Store and retrieve `userManagementInfo` in JWT tokens ```typescript // In createToken() - lines 124-131 const token = await new SignJWT({ sub: userEmail, clientId, umbrellaAuth: { Authorization: umbrellaAuth.Authorization, userManagementInfo: umbrellaAuth.userManagementInfo // NEW: Store in token } }) // In verifyToken() - lines 166-170 const umbrellaAuth: AuthToken = { Authorization: tokenPayload.umbrellaAuth.Authorization, userManagementInfo: tokenPayload.umbrellaAuth.userManagementInfo // NEW: Extract from token }; ``` ### 3. `/src/dual-auth.ts` **Change 1**: Include `userManagementInfo` in authentication responses (lines 269-272, 300-303) ```typescript // In authenticateKeycloak() return { Authorization: response.data.Authorization, userManagementInfo: this.userManagementInfo || undefined // NEW: Include actual auth method }; // In authenticateCognito() return { Authorization: response.data.jwtToken, userManagementInfo: this.userManagementInfo || undefined // NEW: Include actual auth method }; ``` **Change 2**: Remove unreliable re-detection logic (lines 481-490) **BEFORE** (BROKEN): ```typescript // For OAuth users: ensure userManagementInfo is set before proceeding if (this.token && !this.userManagementInfo) { console.error(`[DUAL-AUTH] ⚠️ OAuth user but userManagementInfo not set! Detecting user management system...`); if (this.username) { this.userManagementInfo = await this.detectUserManagementSystem(this.username); // ❌ UNRELIABLE! console.error(`[DUAL-AUTH] ✅ User management system detected: isKeycloak=${this.userManagementInfo.isKeycloak}`); } else { console.error(`[DUAL-AUTH] ❌ Cannot detect user management system: username not available`); this.userManagementInfo = { isKeycloak: false, authMethod: 'cognito' }; } } ``` **AFTER** (FIXED): ```typescript // For OAuth users: userManagementInfo should already be set from the stored token // DO NOT re-detect - use the auth method that actually succeeded during login if (this.token && !this.userManagementInfo) { console.error(`[DUAL-AUTH] ⚠️ OAuth user but userManagementInfo not set!`); console.error(`[DUAL-AUTH] This should not happen - userManagementInfo should be in the token.`); console.error(`[DUAL-AUTH] Defaulting to Cognito to be safe (most common legacy auth method).`); // DO NOT call detectUserManagementSystem() - it's unreliable for users in migration state // The token should already contain the correct auth method from login this.userManagementInfo = { isKeycloak: false, authMethod: 'cognito' }; } ``` ## How the Fix Works ### Authentication Flow (Initial Login) 1. User attempts login 2. System tries Keycloak auth first → May fail with 500 for Cognito users 3. Falls back to Cognito auth → Succeeds for Cognito users 4. Sets `this.userManagementInfo` based on which method succeeded 5. **NEW**: Stores this info in the JWT token when creating it ### Subsequent API Requests 1. User makes API request with OAuth Bearer token 2. Token is verified and `userManagementInfo` is extracted from token 3. **FIXED**: No longer calls `detectUserManagementSystem()` 4. Uses the stored `userManagementInfo` from the token 5. Builds API key with correct format: - Keycloak: `{userKey}:{accountKey}:` (empty divisionId) - Cognito: `{userKey}:{accountKey}:{divisionId}` (numeric divisionId) ## Testing ### Manual Testing with Claude Desktop 1. Authenticate as a Cognito user (e.g., `david+saola@umbrellacost.com`) 2. Make API requests through MCP tools 3. Verify no 500 errors occur 4. Check server logs to confirm `userManagementInfo` is NOT being re-detected ### Expected Log Behavior **Before Fix**: You would see repeated logs like: ``` [DUAL-AUTH] ⚠️ OAuth user but userManagementInfo not set! Detecting user management system... [DUAL-AUTH] ✅ User management system detected: isKeycloak=true ← WRONG! ``` **After Fix**: You should see: ``` [DUAL-AUTH] OAuth Cognito - Built API key with divisionId: {uuid}:{accountKey}:{divisionId} ``` ## Build and Deploy ```bash # Build TypeScript npm run build # Restart server pkill -9 -f "node dist/index-dual.js" PORT=3003 node dist/index-dual.js ``` ## Verification Checklist - ✅ TypeScript compiles without errors - ✅ `userManagementInfo` stored in JWT token during authentication - ✅ `userManagementInfo` extracted from JWT token on subsequent requests - ✅ No re-detection calls to user-realm API during API requests - ✅ Cognito users get correct API key format (with numeric divisionId) - ✅ Keycloak users get correct API key format (with empty divisionId) ## Impact ### Users Affected - Cognito legacy users who exist in the user-realm API (migration state) - Example: `david+saola@umbrellacost.com` ### Users NOT Affected - Pure Keycloak UM 2.0 users (no Cognito account) - New users created directly in Keycloak ## Related Documents - `/INVESTIGATION_FINDINGS.md` - Root cause analysis - `/scripts/debug/test-both-auth-methods.cjs` - Auth method testing script - `/scripts/debug/investigate-user-realm-detection.cjs` - Realm detection test script ## Next Steps 1. ✅ Implementation complete 2. ⏳ Test with Cognito users through Claude Desktop 3. ⏳ Verify server logs show correct behavior 4. ⏳ Monitor for 500 errors (should be eliminated) 5. ⏳ Consider deprecating user-realm API detection entirely if this fix proves stable