From 8d378f5b537493cde59e6d1949fb141a88a6c74a Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 17 Jun 2025 15:05:41 +0200 Subject: [PATCH] FEAT: Enhance authentication system with internal API support, Keycloak connectivity diagnostics, and simplified client implementation --- docs/502-error-fixes-implementation.md | 59 ++++++++++++++++++- .../api/debug/test-keycloak-connectivity.ts | 36 +++++++++++ server/utils/auth.ts | 47 +++++++++++++++ server/utils/keycloak-client.ts | 13 +--- 4 files changed, 143 insertions(+), 12 deletions(-) create mode 100644 server/api/debug/test-keycloak-connectivity.ts diff --git a/docs/502-error-fixes-implementation.md b/docs/502-error-fixes-implementation.md index 6ff9965..1181ea1 100644 --- a/docs/502-error-fixes-implementation.md +++ b/docs/502-error-fixes-implementation.md @@ -219,12 +219,67 @@ If issues occur: 3. **Alerts**: Set up monitoring alerts for circuit breaker 4. **Testing**: Add automated integration tests for auth flow +## Post-Implementation Fixes + +After the initial implementation, additional issues were discovered and resolved: + +### Issue: Keycloak Client Compatibility +**Problem**: The enhanced keycloak-client.ts with custom headers was incompatible with Nitro/Nuxt $fetch, causing immediate fetch failures. + +**Solution**: Simplified the client by removing problematic headers: +- Removed `Connection: keep-alive` and `Keep-Alive` headers +- Removed custom timeout implementation +- Kept retry logic and circuit breaker functionality + +### Issue: Background Task Authentication +**Problem**: Background tasks (like `process-sales-emails`) were failing with 401 errors because they don't have user sessions. + +**Solution**: Enhanced `server/utils/auth.ts` to support internal authentication: +- Added support for `x-tag: 094ut234` header for system tasks +- Added localhost detection for internal calls +- Added optional `INTERNAL_API_SECRET` environment variable support + +### Issue: Network Diagnostics +**Problem**: Difficult to diagnose Docker networking issues with Keycloak connectivity. + +**Solution**: Added diagnostic endpoint: +- `/api/debug/test-keycloak-connectivity` - Tests basic connectivity to Keycloak from within container + +## Updated Files Summary + +**New Files**: +- `server/utils/keycloak-client.ts` - Resilient HTTP client (simplified version) +- `server/api/debug/test-keycloak-connectivity.ts` - Connectivity diagnostic tool +- `docs/502-error-fixes-implementation.md` - This documentation + +**Modified Files**: +- `server/api/auth/keycloak/callback.ts` - Uses simplified keycloak client +- `server/api/auth/refresh.ts` - Enhanced with retry logic +- `server/utils/auth.ts` - Added internal authentication support +- `pages/login.vue` - Better error message handling +- `plugins/00.startup-check.server.ts` - Enhanced startup checks +- `server/api/health.ts` - Added circuit breaker monitoring + +## Testing the Fixes + +### 1. Test Keycloak Connectivity +```bash +curl https://client.portnimara.dev/api/debug/test-keycloak-connectivity +``` + +### 2. Test Background Task Authentication +The `process-sales-emails` task should now work without 401 errors due to the `x-tag: 094ut234` header being recognized as internal authentication. + +### 3. Test User Authentication Flow +Normal login should work without 502 errors, with better retry logic handling temporary network issues. + ## Summary These changes provide a robust, resilient authentication system that can handle: - Temporary network issues - Service degradation - High load scenarios -- Monitoring and debugging +- Background task authentication +- Better monitoring and debugging -The 502 errors during login should now be completely eliminated with proper fallback mechanisms and user feedback. +The 502 errors during login should now be completely eliminated with proper fallback mechanisms and user feedback. Background tasks now have proper authentication bypassing user session requirements. diff --git a/server/api/debug/test-keycloak-connectivity.ts b/server/api/debug/test-keycloak-connectivity.ts new file mode 100644 index 0000000..efdd0e3 --- /dev/null +++ b/server/api/debug/test-keycloak-connectivity.ts @@ -0,0 +1,36 @@ +export default defineEventHandler(async (event) => { + console.log('[DEBUG] Testing Keycloak connectivity...') + + try { + // Test basic connectivity to Keycloak + const wellKnownUrl = 'https://auth.portnimara.dev/realms/client-portal/.well-known/openid-configuration' + + const response = await $fetch(wellKnownUrl, { + retry: 0 + }) as any + + return { + success: true, + message: 'Keycloak connectivity successful', + endpoint: wellKnownUrl, + response: { + issuer: response.issuer, + authorization_endpoint: response.authorization_endpoint, + token_endpoint: response.token_endpoint, + userinfo_endpoint: response.userinfo_endpoint + } + } + } catch (error: any) { + console.error('[DEBUG] Keycloak connectivity test failed:', error) + + return { + success: false, + message: 'Keycloak connectivity failed', + error: { + message: error.message, + status: error.status, + cause: error.cause?.message + } + } + } +}) diff --git a/server/utils/auth.ts b/server/utils/auth.ts index 11c3851..31ead07 100644 --- a/server/utils/auth.ts +++ b/server/utils/auth.ts @@ -55,6 +55,14 @@ export const isAuthenticated = async (event: any): Promise => { } export const requireAuth = async (event: any) => { + // First check for internal API authentication + const internalAuth = checkInternalAuth(event); + if (internalAuth) { + console.log('[requireAuth] Internal API authentication successful'); + return; + } + + // Then check user authentication const authenticated = await isAuthenticated(event); if (!authenticated) { console.log('[requireAuth] Authentication failed for:', event.node.req.url); @@ -66,6 +74,45 @@ export const requireAuth = async (event: any) => { } } +/** + * Check if the request is from an internal service/background task + */ +const checkInternalAuth = (event: any): boolean => { + const headers = event.node.req.headers; + + // Check for internal service header with the system tag + const xTag = headers['x-tag']; + const xInternalSecret = headers['x-internal-secret']; + + // System tag authentication (for background tasks) + if (xTag === '094ut234') { + console.log('[auth] Internal system tag authentication successful'); + return true; + } + + // Internal secret authentication (if set in environment) + const internalSecret = process.env.INTERNAL_API_SECRET; + if (internalSecret && xInternalSecret === internalSecret) { + console.log('[auth] Internal secret authentication successful'); + return true; + } + + // Check if request is from localhost (same container) + const xForwardedFor = headers['x-forwarded-for']; + const xRealIp = headers['x-real-ip']; + const remoteAddress = event.node.req.socket?.remoteAddress; + + const isLocalhost = !xForwardedFor && !xRealIp && + (remoteAddress === '127.0.0.1' || remoteAddress === '::1' || remoteAddress === '::ffff:127.0.0.1'); + + if (isLocalhost && xTag) { + console.log('[auth] Localhost with system tag authentication successful'); + return true; + } + + return false; +} + /** * Get the authenticated user from the session */ diff --git a/server/utils/keycloak-client.ts b/server/utils/keycloak-client.ts index fbe4e55..59ccd14 100644 --- a/server/utils/keycloak-client.ts +++ b/server/utils/keycloak-client.ts @@ -61,7 +61,6 @@ class KeycloakClient { async fetch(url: string, options: any = {}, clientOptions: KeycloakClientOptions = {}): Promise { const { - timeout = 30000, retries = 3, retryDelay = 1000 } = clientOptions @@ -81,17 +80,11 @@ class KeycloakClient { try { console.log(`[KEYCLOAK_CLIENT] Attempt ${attempt}/${retries + 1} for ${url}`) + // Use basic $fetch without problematic options const response = await $fetch(url, { ...options, - timeout, - // Add connection reuse headers - headers: { - ...options.headers, - 'Connection': 'keep-alive', - 'Keep-Alive': 'timeout=30, max=100' - }, - // Disable automatic retries from $fetch to handle them ourselves - retry: 0 + // Don't add custom headers that might be incompatible + retry: 0 // Disable automatic retries from $fetch to handle them ourselves }) const duration = Date.now() - startTime