From 6176d5c4d824527927bb6b01e0a8ee4c565bfcff Mon Sep 17 00:00:00 2001 From: Urtzi Alfaro Date: Sat, 26 Jul 2025 20:04:24 +0200 Subject: [PATCH] REFACTOR API gateway fix 3 --- check.sh | 149 +++++++++++++++++++ gateway/app/middleware/auth.py | 171 +++++++++++++-------- jwt_debug_script.sh | 230 +++++++++++++++++++++++++++++ services/auth/app/api/auth.py | 196 ++++++++++++------------ services/auth/app/core/security.py | 150 +++++++++++++++---- shared/auth/jwt_handler.py | 187 +++++++++++++++++++---- test_new.sh | 2 +- 7 files changed, 861 insertions(+), 224 deletions(-) create mode 100755 check.sh create mode 100755 jwt_debug_script.sh diff --git a/check.sh b/check.sh new file mode 100755 index 00000000..f14121e0 --- /dev/null +++ b/check.sh @@ -0,0 +1,149 @@ +#!/bin/bash +# Environment Variable Diagnostic Script +# This script helps identify where environment variables are getting lost + +set -e + +echo "🔍 Environment Variable Diagnostic Script" +echo "==========================================" + +# Check if .env file exists and contains JWT settings +echo "📋 Step 1: Checking .env file..." +if [ -f ".env" ]; then + echo "✅ .env file exists" + if grep -q "JWT_SECRET_KEY" .env; then + JWT_VALUE=$(grep "JWT_SECRET_KEY" .env | cut -d'=' -f2-) + echo "✅ JWT_SECRET_KEY found in .env: ${JWT_VALUE:0:30}..." + else + echo "❌ JWT_SECRET_KEY not found in .env" + fi +else + echo "❌ .env file does not exist" +fi + +echo "" + +# Check docker-compose.yml configuration +echo "📋 Step 2: Checking docker-compose.yml..." +if grep -A 20 "auth-service:" docker-compose.yml | grep -q "JWT_SECRET_KEY"; then + echo "✅ auth-service has JWT_SECRET_KEY in docker-compose.yml" +else + echo "❌ auth-service missing JWT_SECRET_KEY in docker-compose.yml" +fi + +echo "" + +# Check if services are running +echo "📋 Step 3: Checking running services..." +AUTH_RUNNING=$(docker-compose ps auth-service | grep -c "Up" || echo "0") +GATEWAY_RUNNING=$(docker-compose ps gateway | grep -c "Up" || echo "0") + +echo "Auth Service Running: $AUTH_RUNNING" +echo "Gateway Running: $GATEWAY_RUNNING" + +echo "" + +# Check environment at container startup +echo "📋 Step 4: Checking container environment at startup..." +echo "Starting fresh auth-service container..." + +# Stop and start auth service +docker-compose stop auth-service +sleep 2 + +# Start auth service and immediately check environment +echo "Starting auth-service..." +docker-compose up -d auth-service + +# Wait a moment for startup +sleep 3 + +echo "Checking environment immediately after startup..." +STARTUP_ENV=$(docker-compose exec -T auth-service env | grep JWT_SECRET_KEY || echo "NOT_SET") +echo "Startup Environment: $STARTUP_ENV" + +# Wait a bit more and check again +sleep 5 +echo "Checking environment after 5 seconds..." +AFTER_STARTUP=$(docker-compose exec -T auth-service env | grep JWT_SECRET_KEY || echo "NOT_SET") +echo "After Startup: $AFTER_STARTUP" + +echo "" + +# Check if the application is overriding environment variables +echo "📋 Step 5: Checking application configuration loading..." + +# Create a temporary script to check Python environment loading +cat > /tmp/check_env.py << 'EOF' +import os +import sys +sys.path.insert(0, '/app') + +print("=== Python Environment Check ===") +print(f"JWT_SECRET_KEY from os.getenv: {os.getenv('JWT_SECRET_KEY', 'NOT_SET')}") + +try: + from app.core.config import settings + print(f"JWT_SECRET_KEY from settings: {getattr(settings, 'JWT_SECRET_KEY', 'NOT_SET')}") + + # Check if settings inherits from base + try: + from shared.config.base import BaseServiceSettings + base_settings = BaseServiceSettings() + print(f"JWT_SECRET_KEY from base settings: {getattr(base_settings, 'JWT_SECRET_KEY', 'NOT_SET')}") + except Exception as e: + print(f"Error loading base settings: {e}") + +except Exception as e: + print(f"Error loading auth settings: {e}") + +print("=== Raw Environment Variables ===") +for key, value in sorted(os.environ.items()): + if 'JWT' in key or 'SECRET' in key: + print(f"{key}={value[:30]}..." if len(value) > 30 else f"{key}={value}") +EOF + +# Copy the script into the container and run it +docker cp /tmp/check_env.py "$(docker-compose ps -q auth-service):/tmp/check_env.py" +echo "Running Python environment check..." +docker-compose exec -T auth-service python /tmp/check_env.py + +echo "" + +# Check if there are any startup scripts or entrypoints modifying environment +echo "📋 Step 6: Checking container startup process..." +echo "Container entrypoint:" +docker-compose exec -T auth-service cat /usr/local/bin/docker-entrypoint.sh 2>/dev/null || echo "No entrypoint script found" + +echo "" +echo "Docker image environment:" +docker-compose exec -T auth-service printenv | grep JWT || echo "No JWT variables in container environment" + +echo "" + +# Check application logs for any environment variable issues +echo "📋 Step 7: Checking application logs for environment issues..." +echo "Recent auth-service logs:" +docker-compose logs --tail=20 auth-service | grep -i -E "(jwt|secret|env|config)" || echo "No relevant logs found" + +echo "" + +# Final diagnosis +echo "📋 Step 8: Diagnosis Summary" +echo "==============================" + +if [ "$STARTUP_ENV" = "NOT_SET" ]; then + echo "❌ ISSUE: JWT_SECRET_KEY is not being passed to the container" + echo "🔧 SOLUTION: Fix docker-compose.yml environment variables" +elif [ "$STARTUP_ENV" != "NOT_SET" ] && [ "$AFTER_STARTUP" = "NOT_SET" ]; then + echo "❌ ISSUE: JWT_SECRET_KEY is present at startup but disappears" + echo "🔧 SOLUTION: Check application configuration loading" +else + echo "✅ Environment variables appear to be stable" +fi + +echo "" +echo "🏁 Diagnostic complete!" + +# Cleanup +rm -f /tmp/check_env.py \ No newline at end of file diff --git a/gateway/app/middleware/auth.py b/gateway/app/middleware/auth.py index a018a40b..a9850550 100644 --- a/gateway/app/middleware/auth.py +++ b/gateway/app/middleware/auth.py @@ -1,6 +1,7 @@ # gateway/app/middleware/auth.py """ Enhanced Authentication Middleware for API Gateway with Tenant Access Control +FIXED VERSION - Proper JWT verification and token structure handling """ import structlog @@ -9,6 +10,8 @@ from fastapi.responses import JSONResponse from starlette.middleware.base import BaseHTTPMiddleware from starlette.responses import Response from typing import Optional, Dict, Any +import httpx +import json from app.core.config import settings from shared.auth.jwt_handler import JWTHandler @@ -16,7 +19,7 @@ from shared.auth.tenant_access import tenant_access_manager, extract_tenant_id_f logger = structlog.get_logger() -# JWT handler for local token validation +# JWT handler for local token validation - using SAME configuration as auth service jwt_handler = JWTHandler(settings.JWT_SECRET_KEY, settings.JWT_ALGORITHM) # Routes that don't require authentication @@ -63,13 +66,12 @@ class AuthMiddleware(BaseHTTPMiddleware): ) # ✅ STEP 2: Verify token and get user context - # Pass self.redis_client to _verify_token to enable caching user_context = await self._verify_token(token) if not user_context: logger.warning(f"Invalid token for route: {request.url.path}") return JSONResponse( status_code=401, - content={"detail": "Invalid or expired token"} + content={"detail": "User not authenticated"} ) # ✅ STEP 3: Extract tenant context from URL using shared utility @@ -78,11 +80,10 @@ class AuthMiddleware(BaseHTTPMiddleware): # ✅ STEP 4: Verify tenant access if this is a tenant-scoped route if tenant_id and is_tenant_scoped_path(request.url.path): # Use TenantAccessManager for gateway-level verification with caching - # Ensure tenant_access_manager uses the redis_client from the middleware if self.redis_client and tenant_access_manager.redis_client is None: tenant_access_manager.redis_client = self.redis_client - has_access = await tenant_access_manager.verify_basic_tenant_access( # Corrected method call + has_access = await tenant_access_manager.verify_basic_tenant_access( user_context["user_id"], tenant_id ) @@ -129,18 +130,22 @@ class AuthMiddleware(BaseHTTPMiddleware): return None async def _verify_token(self, token: str) -> Optional[Dict[str, Any]]: - """Verify JWT token with fallback strategy""" + """ + Verify JWT token with improved fallback strategy + FIXED: Better error handling and token structure validation + """ - # Try local JWT validation first (fast) + # Strategy 1: Try local JWT validation first (fast) try: payload = jwt_handler.verify_token(token) if payload and self._validate_token_payload(payload): logger.debug("Token validated locally") - return payload + # Convert JWT payload to user context format + return self._jwt_payload_to_user_context(payload) except Exception as e: logger.debug(f"Local token validation failed: {e}") - # Check cache for recently validated tokens + # Strategy 2: Check cache for recently validated tokens if self.redis_client: try: cached_user = await self._get_cached_user(token) @@ -150,7 +155,7 @@ class AuthMiddleware(BaseHTTPMiddleware): except Exception as e: logger.warning(f"Cache lookup failed: {e}") - # Verify with auth service (authoritative) + # Strategy 3: Verify with auth service (authoritative) try: user_context = await self._verify_with_auth_service(token) if user_context: @@ -165,92 +170,134 @@ class AuthMiddleware(BaseHTTPMiddleware): return None def _validate_token_payload(self, payload: Dict[str, Any]) -> bool: - """Validate JWT payload has required fields""" - required_fields = ["user_id", "email", "exp"] - return all(field in payload for field in required_fields) + """ + Validate JWT payload has required fields + FIXED: Updated to match actual token structure from auth service + """ + required_fields = ["user_id", "email", "exp", "type"] + missing_fields = [field for field in required_fields if field not in payload] + + if missing_fields: + logger.warning(f"Token payload missing fields: {missing_fields}") + return False + + # Validate token type + if payload.get("type") != "access": + logger.warning(f"Invalid token type: {payload.get('type')}") + return False + + return True + + def _jwt_payload_to_user_context(self, payload: Dict[str, Any]) -> Dict[str, Any]: + """ + Convert JWT payload to user context format + FIXED: Proper mapping between JWT structure and user context + """ + return { + "user_id": payload["user_id"], + "email": payload["email"], + "exp": payload["exp"], + "valid": True + } async def _verify_with_auth_service(self, token: str) -> Optional[Dict[str, Any]]: - """Verify token with auth service""" + """ + Verify token with auth service + FIXED: Improved error handling and response parsing + """ try: - import httpx - async with httpx.AsyncClient(timeout=3.0) as client: + async with httpx.AsyncClient(timeout=5.0) as client: response = await client.post( f"{settings.AUTH_SERVICE_URL}/api/v1/auth/verify", headers={"Authorization": f"Bearer {token}"} ) if response.status_code == 200: - return response.json() + auth_response = response.json() + + # Validate auth service response structure + if auth_response.get("valid") and auth_response.get("user_id"): + return { + "user_id": auth_response["user_id"], + "email": auth_response["email"], + "exp": auth_response.get("exp"), + "valid": True + } + else: + logger.warning(f"Auth service returned invalid response: {auth_response}") + return None else: - logger.warning(f"Auth service returned {response.status_code}") + logger.warning(f"Auth service returned {response.status_code}: {response.text}") return None + except httpx.TimeoutException: + logger.error("Auth service timeout during token verification") + return None except Exception as e: logger.error(f"Auth service error: {e}") return None async def _get_cached_user(self, token: str) -> Optional[Dict[str, Any]]: - """Get user context from cache""" + """ + Get user context from cache + FIXED: Better error handling and JSON parsing + """ if not self.redis_client: return None - cache_key = f"auth:token:{hash(token)}" + cache_key = f"auth:token:{hash(token) % 1000000}" # Use modulo for shorter keys try: cached_data = await self.redis_client.get(cache_key) if cached_data: - import json if isinstance(cached_data, bytes): cached_data = cached_data.decode() return json.loads(cached_data) + except json.JSONDecodeError as e: + logger.warning(f"Failed to parse cached user data: {e}") except Exception as e: - logger.warning(f"Cache get failed: {e}") + logger.warning(f"Cache lookup error: {e}") + return None - async def _cache_user(self, token: str, user_context: Dict[str, Any], ttl: int = 300): - """Cache user context for 5 minutes""" + async def _cache_user(self, token: str, user_context: Dict[str, Any]) -> None: + """ + Cache user context + FIXED: Better error handling and expiration + """ if not self.redis_client: return - cache_key = f"auth:token:{hash(token)}" + cache_key = f"auth:token:{hash(token) % 1000000}" try: - import json - await self.redis_client.setex(cache_key, ttl, json.dumps(user_context)) + # Cache for 5 minutes (shorter than token expiry) + await self.redis_client.setex( + cache_key, + 300, # 5 minutes + json.dumps(user_context) + ) except Exception as e: - logger.warning(f"Cache set failed: {e}") + logger.warning(f"Failed to cache user context: {e}") - def _inject_context_headers(self, request: Request, user_context: Dict[str, Any], tenant_id: Optional[str]): - """Inject authentication and tenant headers for downstream services""" + def _inject_context_headers(self, request: Request, user_context: Dict[str, Any], tenant_id: Optional[str] = None): + """ + Inject user and tenant context headers for downstream services + FIXED: Proper header injection + """ + # Add user context headers + request.headers.__dict__["_list"].append(( + b"x-user-id", user_context["user_id"].encode() + )) + request.headers.__dict__["_list"].append(( + b"x-user-email", user_context["email"].encode() + )) - # Remove any existing auth headers to prevent spoofing - headers_to_remove = [ - "x-user-id", "x-user-email", "x-user-role", - "x-tenant-id", "x-tenant-verified", "x-authenticated" - ] - - for header in headers_to_remove: - request.headers.__dict__["_list"] = [ - (k, v) for k, v in request.headers.raw - if k.lower() != header.lower() - ] - - # Inject new headers - new_headers = [ - (b"x-authenticated", b"true"), - (b"x-user-id", str(user_context.get("user_id", "")).encode()), - (b"x-user-email", str(user_context.get("email", "")).encode()), - (b"x-user-role", str(user_context.get("role", "user")).encode()), - ] - - # Add tenant context if verified + # Add tenant context if available if tenant_id: - new_headers.extend([ - (b"x-tenant-id", tenant_id.encode()), - (b"x-tenant-verified", b"true") - ]) - - # Add headers to request - request.headers.__dict__["_list"].extend(new_headers) - - logger.debug(f"Injected context headers", - user_id=user_context.get("user_id"), - tenant_id=tenant_id) \ No newline at end of file + request.headers.__dict__["_list"].append(( + b"x-tenant-id", tenant_id.encode() + )) + + # Add gateway identification + request.headers.__dict__["_list"].append(( + b"x-forwarded-by", b"bakery-gateway" + )) \ No newline at end of file diff --git a/jwt_debug_script.sh b/jwt_debug_script.sh new file mode 100755 index 00000000..b0e66986 --- /dev/null +++ b/jwt_debug_script.sh @@ -0,0 +1,230 @@ +#!/bin/bash +# JWT Debug and Verification Script +# This script helps debug JWT token issues between gateway and auth service + +set -e + +# Configuration +API_BASE="http://localhost:8000" +AUTH_BASE="http://localhost:8001" +EMAIL="test@bakery.com" +PASSWORD="TestPassword123!" + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +BLUE='\033[0;34m' +NC='\033[0m' # No Color + +log_info() { + echo -e "${BLUE}ℹī¸ $1${NC}" +} + +log_success() { + echo -e "${GREEN}✅ $1${NC}" +} + +log_warning() { + echo -e "${YELLOW}⚠ī¸ $1${NC}" +} + +log_error() { + echo -e "${RED}❌ $1${NC}" +} + +log_step() { + echo -e "${BLUE}🔄 $1${NC}" +} + +echo "🔍 JWT Token Debug and Verification Script" +echo "===========================================" + +# Step 1: Check JWT Configuration +log_step "Step 1: Checking JWT Configuration Consistency" + +log_info "Checking Gateway JWT Settings..." +GATEWAY_JWT_SECRET=$(docker-compose exec -T gateway env 2>/dev/null | grep JWT_SECRET_KEY || echo "NOT_SET") +GATEWAY_JWT_ALGO=$(docker-compose exec -T gateway env 2>/dev/null | grep JWT_ALGORITHM || echo "NOT_SET") + +log_info "Checking Auth Service JWT Settings..." +AUTH_JWT_SECRET=$(docker-compose exec -T auth-service env 2>/dev/null | grep JWT_SECRET_KEY || echo "NOT_SET") +AUTH_JWT_ALGO=$(docker-compose exec -T auth-service env 2>/dev/null | grep JWT_ALGORITHM || echo "NOT_SET") + +echo "Gateway JWT Secret: $GATEWAY_JWT_SECRET" +echo "Auth Service JWT Secret: $AUTH_JWT_SECRET" +echo "Gateway JWT Algorithm: $GATEWAY_JWT_ALGO" +echo "Auth Service JWT Algorithm: $AUTH_JWT_ALGO" + +if [ "$GATEWAY_JWT_SECRET" = "$AUTH_JWT_SECRET" ]; then + log_success "JWT Secret Keys match!" +else + log_error "JWT Secret Keys DO NOT match!" + log_warning "This is likely the cause of the authentication issue" +fi + +echo "" + +# Step 2: Test Direct Auth Service +log_step "Step 2: Testing Direct Auth Service" + +# Login directly to auth service +log_info "Logging in directly to auth service..." +AUTH_LOGIN_RESPONSE=$(curl -s -X POST "$AUTH_BASE/api/v1/auth/login" \ + -H "Content-Type: application/json" \ + -d "{ + \"email\": \"$EMAIL\", + \"password\": \"$PASSWORD\" + }") + +echo "Direct Auth Login Response:" +echo "$AUTH_LOGIN_RESPONSE" | jq '.' 2>/dev/null || echo "$AUTH_LOGIN_RESPONSE" + +# Extract token +DIRECT_TOKEN=$(echo "$AUTH_LOGIN_RESPONSE" | jq -r '.access_token' 2>/dev/null) + +if [ "$DIRECT_TOKEN" != "null" ] && [ "$DIRECT_TOKEN" != "" ]; then + log_success "Direct auth login successful!" + echo "Token: ${DIRECT_TOKEN:0:50}..." + + # Test direct auth verification + log_info "Testing direct auth token verification..." + AUTH_VERIFY_RESPONSE=$(curl -s -X POST "$AUTH_BASE/api/v1/auth/verify" \ + -H "Authorization: Bearer $DIRECT_TOKEN") + + echo "Direct Auth Verify Response:" + echo "$AUTH_VERIFY_RESPONSE" | jq '.' 2>/dev/null || echo "$AUTH_VERIFY_RESPONSE" + + # Test direct auth /users/me + log_info "Testing direct auth /users/me endpoint..." + AUTH_ME_RESPONSE=$(curl -s -X GET "$AUTH_BASE/api/v1/users/me" \ + -H "Authorization: Bearer $DIRECT_TOKEN") + + echo "Direct Auth /users/me Response:" + echo "$AUTH_ME_RESPONSE" | jq '.' 2>/dev/null || echo "$AUTH_ME_RESPONSE" + +else + log_error "Direct auth login failed!" + exit 1 +fi + +echo "" + +# Step 3: Test Gateway Login +log_step "Step 3: Testing Gateway Login" + +GATEWAY_LOGIN_RESPONSE=$(curl -s -X POST "$API_BASE/api/v1/auth/login" \ + -H "Content-Type: application/json" \ + -d "{ + \"email\": \"$EMAIL\", + \"password\": \"$PASSWORD\" + }") + +echo "Gateway Login Response:" +echo "$GATEWAY_LOGIN_RESPONSE" | jq '.' 2>/dev/null || echo "$GATEWAY_LOGIN_RESPONSE" + +GATEWAY_TOKEN=$(echo "$GATEWAY_LOGIN_RESPONSE" | jq -r '.access_token' 2>/dev/null) + +if [ "$GATEWAY_TOKEN" != "null" ] && [ "$GATEWAY_TOKEN" != "" ]; then + log_success "Gateway login successful!" + echo "Token: ${GATEWAY_TOKEN:0:50}..." +else + log_error "Gateway login failed!" + exit 1 +fi + +echo "" + +# Step 4: Compare Tokens +log_step "Step 4: Comparing Tokens" + +if [ "$DIRECT_TOKEN" = "$GATEWAY_TOKEN" ]; then + log_success "Tokens are identical (expected)" +else + log_warning "Tokens are different (unexpected if same login)" +fi + +# Decode tokens for comparison +log_info "Decoding direct auth token payload..." +DIRECT_PAYLOAD=$(echo "$DIRECT_TOKEN" | cut -d'.' -f2) +# Add padding if needed +while [ $((${#DIRECT_PAYLOAD} % 4)) -ne 0 ]; do + DIRECT_PAYLOAD="${DIRECT_PAYLOAD}=" +done +echo "$DIRECT_PAYLOAD" | base64 -d 2>/dev/null | jq '.' || echo "Failed to decode" + +log_info "Decoding gateway token payload..." +GATEWAY_PAYLOAD=$(echo "$GATEWAY_TOKEN" | cut -d'.' -f2) +# Add padding if needed +while [ $((${#GATEWAY_PAYLOAD} % 4)) -ne 0 ]; do + GATEWAY_PAYLOAD="${GATEWAY_PAYLOAD}=" +done +echo "$GATEWAY_PAYLOAD" | base64 -d 2>/dev/null | jq '.' || echo "Failed to decode" + +echo "" + +# Step 5: Test Gateway Authentication +log_step "Step 5: Testing Gateway Authentication Middleware" + +# Test gateway token verification +log_info "Testing gateway token verification..." +GATEWAY_VERIFY_RESPONSE=$(curl -s -X POST "$API_BASE/api/v1/auth/verify" \ + -H "Authorization: Bearer $GATEWAY_TOKEN") + +echo "Gateway Verify Response:" +echo "$GATEWAY_VERIFY_RESPONSE" | jq '.' 2>/dev/null || echo "$GATEWAY_VERIFY_RESPONSE" + +# Test gateway /users/me (this is where the issue occurs) +log_info "Testing gateway /users/me endpoint (THE FAILING ENDPOINT)..." +GATEWAY_ME_RESPONSE=$(curl -s -w "\nHTTP_CODE:%{http_code}\n" -X GET "$API_BASE/api/v1/users/me" \ + -H "Authorization: Bearer $GATEWAY_TOKEN") + +echo "Gateway /users/me Response:" +echo "$GATEWAY_ME_RESPONSE" + +# Check if successful +if echo "$GATEWAY_ME_RESPONSE" | grep -q "HTTP_CODE:200"; then + log_success "Gateway /users/me endpoint working!" +elif echo "$GATEWAY_ME_RESPONSE" | grep -q "HTTP_CODE:401"; then + log_error "Gateway /users/me endpoint returning 401 Unauthorized" + log_warning "This confirms the JWT middleware issue" +else + log_warning "Gateway /users/me endpoint returned unexpected response" +fi + +echo "" + +# Step 6: Test with verbose curl to see middleware behavior +log_step "Step 6: Verbose Gateway Request Analysis" + +log_info "Making verbose request to gateway /users/me..." +curl -v -X GET "$API_BASE/api/v1/users/me" \ + -H "Authorization: Bearer $GATEWAY_TOKEN" 2>&1 | head -20 + +echo "" + +# Step 7: Recommendations +log_step "Step 7: Recommendations" + +if [ "$GATEWAY_JWT_SECRET" != "$AUTH_JWT_SECRET" ]; then + log_error "CRITICAL: JWT secrets don't match between services" + echo "Fix: Update your .env file and restart services:" + echo "export JWT_SECRET_KEY='your-super-secret-jwt-key-change-in-production-min-32-characters-long'" + echo "docker-compose down && docker-compose up -d" +elif echo "$GATEWAY_ME_RESPONSE" | grep -q "HTTP_CODE:401"; then + log_warning "JWT secrets match but gateway middleware is still failing" + echo "Possible causes:" + echo "1. Gateway middleware token validation logic issue" + echo "2. Token payload structure mismatch" + echo "3. Gateway not using updated shared JWT handler" + echo "" + echo "Recommended fixes:" + echo "1. Apply the fixed gateway auth middleware" + echo "2. Apply the fixed shared JWT handler" + echo "3. Restart gateway service: docker-compose restart gateway" +else + log_success "All tests passed! JWT authentication is working correctly." +fi + +echo "" +echo "🏁 Debug script completed!" \ No newline at end of file diff --git a/services/auth/app/api/auth.py b/services/auth/app/api/auth.py index d676fc1c..014e9a44 100644 --- a/services/auth/app/api/auth.py +++ b/services/auth/app/api/auth.py @@ -1,36 +1,89 @@ -# services/auth/app/api/auth.py - UPDATED TO RETURN TOKENS FROM REGISTRATION +# services/auth/app/api/auth.py - Fixed Login Method """ -Authentication API routes - Updated to return tokens directly from registration -Following industry best practices with unified token response format +Authentication API endpoints - FIXED VERSION """ from fastapi import APIRouter, Depends, HTTPException, status, Request from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials from sqlalchemy.ext.asyncio import AsyncSession -from typing import Optional import structlog from app.core.database import get_db -from app.schemas.auth import ( - UserRegistration, UserLogin, TokenResponse, - RefreshTokenRequest, UserResponse, PasswordChange, - PasswordReset, TokenVerification -) -from app.services.auth_service import AuthService from app.core.security import SecurityManager +from app.services.auth_service import AuthService +from app.schemas.auth import PasswordReset, UserRegistration, UserLogin, TokenResponse, RefreshTokenRequest, PasswordChange from shared.monitoring.decorators import track_execution_time +from shared.monitoring.metrics import get_metrics_collector logger = structlog.get_logger() -router = APIRouter(tags=["authentication"]) -security = HTTPBearer(auto_error=False) -def get_metrics_collector(request: Request): - """Get metrics collector from app state""" - return getattr(request.app.state, 'metrics_collector', None) +router = APIRouter() +security = HTTPBearer() -# ================================================================ -# AUTHENTICATION ENDPOINTS -# ================================================================ +@router.post("/login", response_model=TokenResponse) +@track_execution_time("login_duration_seconds", "auth-service") +async def login( + login_data: UserLogin, + request: Request, + db: AsyncSession = Depends(get_db) +): + """ + Login user and return tokens + FIXED: Proper error handling and login attempt tracking + """ + metrics = get_metrics_collector(request) + + try: + # Check if account is locked due to too many failed attempts + can_attempt = await SecurityManager.check_login_attempts(login_data.email) + if not can_attempt: + if metrics: + metrics.increment_counter("login_failure_total", labels={"reason": "rate_limited"}) + raise HTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + detail=f"Too many login attempts. Please try again in {SecurityManager.settings.LOCKOUT_DURATION_MINUTES} minutes." + ) + + # Attempt login through AuthService + result = await AuthService.login(login_data.email, login_data.password, db) + + # Clear login attempts on successful login + await SecurityManager.clear_login_attempts(login_data.email) + + # Record successful login + if metrics: + metrics.increment_counter("login_success_total") + + logger.info(f"Login successful for {login_data.email}") + return TokenResponse(**result) + + except HTTPException as e: + # Don't increment attempts for rate limiting errors (already handled above) + if e.status_code != status.HTTP_429_TOO_MANY_REQUESTS: + # Increment login attempts on authentication failure + await SecurityManager.increment_login_attempts(login_data.email) + + # Record failed login + if metrics: + reason = "rate_limited" if e.status_code == 429 else "auth_failed" + metrics.increment_counter("login_failure_total", labels={"reason": reason}) + + logger.warning(f"Login failed for {login_data.email}: {e.detail}") + raise + + except Exception as e: + # Increment login attempts on any other error + await SecurityManager.increment_login_attempts(login_data.email) + + # Record login error + if metrics: + metrics.increment_counter("login_failure_total", labels={"reason": "error"}) + + logger.error(f"Login error for {login_data.email}: {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Login failed" + ) @router.post("/register", response_model=TokenResponse) @track_execution_time("registration_duration_seconds", "auth-service") @@ -39,31 +92,17 @@ async def register( request: Request, db: AsyncSession = Depends(get_db) ): - """Register a new user and return tokens directly""" + """Register new user""" metrics = get_metrics_collector(request) try: - # Validate password strength - if not SecurityManager.validate_password(user_data.password): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Password does not meet security requirements" - ) - - # Create user and generate tokens in one operation - result = await AuthService.register_user_with_tokens( - email=user_data.email, - password=user_data.password, - full_name=user_data.full_name, - db=db - ) + result = await AuthService.register(user_data, db) # Record successful registration if metrics: metrics.increment_counter("registration_total", labels={"status": "success"}) - logger.info(f"User registration with tokens successful: {user_data.email}") - + logger.info(f"User registered successfully: {user_data.email}") return TokenResponse(**result) except HTTPException as e: @@ -80,58 +119,6 @@ async def register( detail="Registration failed" ) -@router.post("/login", response_model=TokenResponse) -@track_execution_time("login_duration_seconds", "auth-service") -async def login( - login_data: UserLogin, - request: Request, - db: AsyncSession = Depends(get_db) -): - """Login user and return tokens""" - metrics = get_metrics_collector(request) - - try: - # Check login attempts (rate limiting) - #attempts = await SecurityManager.get_login_attempts(login_data.email) - #if attempts >= 5: - # raise HTTPException( - # status_code=status.HTTP_429_TOO_MANY_REQUESTS, - # detail="Too many login attempts. Please try again later." - # ) - - # Attempt login - result = await AuthService.login(login_data.email, login_data.password, db) - - # Clear login attempts on success - await SecurityManager.clear_login_attempts(login_data.email) - - # Record successful login - if metrics: - metrics.increment_counter("login_success_total") - - logger.info(f"Login successful for {login_data.email}") - return TokenResponse(**result) - - except HTTPException as e: - # Increment login attempts on failure - await SecurityManager.increment_login_attempts(login_data.email) - - # Record failed login - if metrics: - metrics.increment_counter("login_failure_total", labels={"reason": "auth_failed"}) - logger.warning(f"Login failed for {login_data.email}: {e.detail}") - raise - - except Exception as e: - # Record login error - if metrics: - metrics.increment_counter("login_failure_total", labels={"reason": "error"}) - logger.error(f"Login error for {login_data.email}: {e}") - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Login failed" - ) - @router.post("/refresh", response_model=TokenResponse) @track_execution_time("token_refresh_duration_seconds", "auth-service") async def refresh_token( @@ -149,7 +136,6 @@ async def refresh_token( if metrics: metrics.increment_counter("token_refresh_success_total") - logger.info("Token refresh successful") return TokenResponse(**result) except HTTPException as e: @@ -157,7 +143,6 @@ async def refresh_token( metrics.increment_counter("token_refresh_failure_total") logger.warning(f"Token refresh failed: {e.detail}") raise - except Exception as e: if metrics: metrics.increment_counter("token_refresh_failure_total") @@ -167,37 +152,40 @@ async def refresh_token( detail="Token refresh failed" ) -@router.post("/verify", response_model=TokenVerification) -@track_execution_time("token_verification_duration_seconds", "auth-service") +@router.post("/verify") +@track_execution_time("token_verify_duration_seconds", "auth-service") async def verify_token( credentials: HTTPAuthorizationCredentials = Depends(security), request: Request = None ): - """Verify access token""" + """Verify access token and return user info""" metrics = get_metrics_collector(request) if request else None - if not credentials: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Authorization header required" - ) - try: + if not credentials: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Authentication required" + ) + result = await AuthService.verify_user_token(credentials.credentials) + # Record successful verification if metrics: metrics.increment_counter("token_verify_success_total") - return TokenVerification( - valid=True, - user_id=result.get("user_id"), - email=result.get("email"), - exp=result.get("exp") - ) + return { + "valid": True, + "user_id": result.get("user_id"), + "email": result.get("email"), + "exp": result.get("exp"), + "message": None + } except HTTPException as e: if metrics: metrics.increment_counter("token_verify_failure_total") + logger.warning(f"Token verification failed: {e.detail}") raise except Exception as e: if metrics: diff --git a/services/auth/app/core/security.py b/services/auth/app/core/security.py index 67ec803a..1735793f 100644 --- a/services/auth/app/core/security.py +++ b/services/auth/app/core/security.py @@ -1,14 +1,13 @@ -# ================================================================ -# services/auth/app/core/security.py (COMPLETE VERSION) -# ================================================================ +# services/auth/app/core/security.py """ Security utilities for authentication service +FIXED VERSION - Consistent JWT token structure """ import bcrypt import re import hashlib -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from typing import Optional, Dict, Any import redis.asyncio as redis from fastapi import HTTPException, status @@ -19,14 +18,14 @@ from shared.auth.jwt_handler import JWTHandler logger = structlog.get_logger() -# Initialize JWT handler +# Initialize JWT handler with SAME configuration as gateway jwt_handler = JWTHandler(settings.JWT_SECRET_KEY, settings.JWT_ALGORITHM) # Redis client for session management redis_client = redis.from_url(settings.REDIS_URL) class SecurityManager: - """Security utilities for authentication""" + """Security utilities for authentication - FIXED VERSION""" @staticmethod def hash_password(password: str) -> str: @@ -61,25 +60,109 @@ class SecurityManager: @staticmethod def create_access_token(user_data: Dict[str, Any]) -> str: - """Create JWT access token""" - expires_delta = timedelta(minutes=settings.JWT_ACCESS_TOKEN_EXPIRE_MINUTES) - return jwt_handler.create_access_token(user_data, expires_delta) + """ + Create JWT access token with CONSISTENT structure + FIXED: Uses shared JWT handler for consistency + """ + return jwt_handler.create_access_token( + user_data=user_data, + expires_delta=timedelta(minutes=settings.JWT_ACCESS_TOKEN_EXPIRE_MINUTES) + ) @staticmethod def create_refresh_token(user_data: Dict[str, Any]) -> str: - """Create JWT refresh token""" - expires_delta = timedelta(days=settings.JWT_REFRESH_TOKEN_EXPIRE_DAYS) - return jwt_handler.create_refresh_token(user_data, expires_delta) + """ + Create JWT refresh token with CONSISTENT structure + FIXED: Uses shared JWT handler for consistency + """ + return jwt_handler.create_refresh_token( + user_data=user_data, + expires_delta=timedelta(days=settings.JWT_REFRESH_TOKEN_EXPIRE_DAYS) + ) @staticmethod def verify_token(token: str) -> Optional[Dict[str, Any]]: - """Verify JWT token""" - return jwt_handler.verify_token(token) + """ + Verify JWT token with CONSISTENT validation + FIXED: Uses shared JWT handler for consistency + """ + try: + return jwt_handler.verify_token(token) + except Exception as e: + logger.warning(f"Token verification failed: {e}") + return None @staticmethod - def hash_token(token: str) -> str: - """Hash token for storage""" - return hashlib.sha256(token.encode()).hexdigest() + def decode_token(token: str) -> Optional[Dict[str, Any]]: + """ + Decode JWT token without verification (for debugging) + """ + try: + return jwt_handler.decode_token_unsafe(token) + except Exception as e: + logger.warning(f"Token decode failed: {e}") + return None + + @staticmethod + async def track_login_attempt(email: str, ip_address: str, success: bool) -> None: + """Track login attempts for security monitoring""" + try: + key = f"login_attempts:{email}:{ip_address}" + + if success: + # Clear failed attempts on successful login + await redis_client.delete(key) + else: + # Increment failed attempts + attempts = await redis_client.incr(key) + if attempts == 1: + # Set expiration on first failed attempt + await redis_client.expire(key, settings.LOCKOUT_DURATION_MINUTES * 60) + + if attempts >= settings.MAX_LOGIN_ATTEMPTS: + logger.warning(f"Account locked for {email} from {ip_address}") + raise HTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + detail=f"Too many failed login attempts. Try again in {settings.LOCKOUT_DURATION_MINUTES} minutes." + ) + + except Exception as e: + logger.error(f"Failed to track login attempt: {e}") + + @staticmethod + async def is_account_locked(email: str, ip_address: str) -> bool: + """Check if account is locked due to failed login attempts""" + try: + key = f"login_attempts:{email}:{ip_address}" + attempts = await redis_client.get(key) + + if attempts: + attempts = int(attempts) + return attempts >= settings.MAX_LOGIN_ATTEMPTS + + except Exception as e: + logger.error(f"Failed to check account lock status: {e}") + + return False + + @staticmethod + def hash_api_key(api_key: str) -> str: + """Hash API key for storage""" + return hashlib.sha256(api_key.encode()).hexdigest() + + @staticmethod + def generate_secure_token(length: int = 32) -> str: + """Generate secure random token""" + import secrets + return secrets.token_urlsafe(length) + + @staticmethod + def mask_sensitive_data(data: str, visible_chars: int = 4) -> str: + """Mask sensitive data for logging""" + if not data or len(data) <= visible_chars: + return "*" * len(data) if data else "" + + return data[:visible_chars] + "*" * (len(data) - visible_chars) @staticmethod async def check_login_attempts(email: str) -> bool: @@ -108,10 +191,11 @@ class SecurityManager: @staticmethod async def clear_login_attempts(email: str) -> None: - """Clear login attempts for email""" + """Clear login attempts for email after successful login""" try: key = f"login_attempts:{email}" await redis_client.delete(key) + logger.debug(f"Cleared login attempts for {email}") except Exception as e: logger.error(f"Error clearing login attempts: {e}") @@ -119,37 +203,39 @@ class SecurityManager: async def store_refresh_token(user_id: str, token: str) -> None: """Store refresh token in Redis""" try: - token_hash = SecurityManager.hash_token(token) + token_hash = SecurityManager.hash_api_key(token) # Reuse hash method key = f"refresh_token:{user_id}:{token_hash}" - # Store for the duration of the refresh token + # Store with expiration matching JWT refresh token expiry expire_seconds = settings.JWT_REFRESH_TOKEN_EXPIRE_DAYS * 24 * 60 * 60 await redis_client.setex(key, expire_seconds, "valid") + except Exception as e: logger.error(f"Error storing refresh token: {e}") @staticmethod - async def verify_refresh_token(user_id: str, token: str) -> bool: - """Verify refresh token exists in Redis""" + async def is_refresh_token_valid(user_id: str, token: str) -> bool: + """Check if refresh token is still valid in Redis""" try: - token_hash = SecurityManager.hash_token(token) + token_hash = SecurityManager.hash_api_key(token) key = f"refresh_token:{user_id}:{token_hash}" - result = await redis_client.get(key) - return result is not None + exists = await redis_client.exists(key) + return bool(exists) + except Exception as e: - logger.error(f"Error verifying refresh token: {e}") + logger.error(f"Error checking refresh token validity: {e}") return False @staticmethod async def revoke_refresh_token(user_id: str, token: str) -> None: - """Revoke refresh token""" + """Revoke refresh token by removing from Redis""" try: - token_hash = SecurityManager.hash_token(token) + token_hash = SecurityManager.hash_api_key(token) key = f"refresh_token:{user_id}:{token_hash}" + await redis_client.delete(key) + logger.debug(f"Revoked refresh token for user {user_id}") + except Exception as e: - logger.error(f"Error revoking refresh token: {e}") - -# Create singleton instance -security_manager = SecurityManager() + logger.error(f"Error revoking refresh token: {e}") \ No newline at end of file diff --git a/shared/auth/jwt_handler.py b/shared/auth/jwt_handler.py index 64045830..7122fb99 100644 --- a/shared/auth/jwt_handler.py +++ b/shared/auth/jwt_handler.py @@ -1,6 +1,7 @@ -# shared/auth/jwt_handler.py - IMPROVED VERSION +# shared/auth/jwt_handler.py """ -Enhanced JWT Handler with proper token structure +Enhanced JWT Handler with proper token structure and validation +FIXED VERSION - Consistent token format between all services """ from jose import jwt, JWTError @@ -11,7 +12,7 @@ import structlog logger = structlog.get_logger() class JWTHandler: - """Enhanced JWT token handling""" + """Enhanced JWT token handling with consistent format""" def __init__(self, secret_key: str, algorithm: str = "HS256"): self.secret_key = secret_key @@ -19,16 +20,23 @@ class JWTHandler: def create_access_token(self, user_data: Dict[str, Any], expires_delta: Optional[timedelta] = None) -> str: """ - Create JWT access token WITHOUT tenant_id - Tenant context is determined per request, not stored in token + Create JWT access token with STANDARD structure + FIXED: Consistent payload format across all services """ to_encode = { - "sub": user_data["user_id"], # Standard JWT subject - "user_id": user_data["user_id"], - "email": user_data["email"], - "type": "access" + "sub": user_data["user_id"], # Standard JWT subject claim + "user_id": user_data["user_id"], # Explicit user ID + "email": user_data["email"], # User email + "type": "access" # Token type } + # Add optional fields if present + if "full_name" in user_data: + to_encode["full_name"] = user_data["full_name"] + if "is_verified" in user_data: + to_encode["is_verified"] = user_data["is_verified"] + + # Set expiration if expires_delta: expire = datetime.now(timezone.utc) + expires_delta else: @@ -44,7 +52,10 @@ class JWTHandler: return encoded_jwt def create_refresh_token(self, user_data: Dict[str, Any], expires_delta: Optional[timedelta] = None) -> str: - """Create JWT refresh token""" + """ + Create JWT refresh token with MINIMAL payload + FIXED: Consistent refresh token structure + """ to_encode = { "sub": user_data["user_id"], "user_id": user_data["user_id"], @@ -62,35 +73,161 @@ class JWTHandler: }) encoded_jwt = jwt.encode(to_encode, self.secret_key, algorithm=self.algorithm) + logger.debug(f"Created refresh token for user {user_data['user_id']}") return encoded_jwt def verify_token(self, token: str) -> Optional[Dict[str, Any]]: - """Verify and decode JWT token with comprehensive validation""" + """ + Verify and decode JWT token with comprehensive validation + FIXED: Better error handling and validation + """ try: - payload = jwt.decode(token, self.secret_key, algorithms=[self.algorithm]) + # Decode token + payload = jwt.decode( + token, + self.secret_key, + algorithms=[self.algorithm], + options={"verify_exp": True} # Verify expiration + ) # Validate required fields - required_fields = ["user_id", "email", "exp", "type"] - if not all(field in payload for field in required_fields): - logger.warning(f"Token missing required fields: {required_fields}") + if not self._validate_payload(payload): + logger.warning("Token payload validation failed") return None - # Validate token type - if payload.get("type") not in ["access", "refresh"]: - logger.warning(f"Invalid token type: {payload.get('type')}") - return None - - # Check expiration (jose handles this, but double-check) + # Check if token is expired (additional check) exp = payload.get("exp") if exp and datetime.fromtimestamp(exp, tz=timezone.utc) < datetime.now(timezone.utc): logger.warning("Token has expired") return None + logger.debug(f"Token verified successfully for user {payload.get('user_id')}") return payload - except JWTError as e: - logger.warning(f"JWT validation failed: {e}") + except jwt.ExpiredSignatureError: + logger.warning("Token has expired") + return None + except jwt.JWTClaimsError as e: + logger.warning(f"Token claims validation failed: {e}") + return None + except jwt.JWTError as e: + logger.warning(f"Token validation failed: {e}") return None except Exception as e: - logger.error(f"Unexpected error validating token: {e}") - return None \ No newline at end of file + logger.error(f"Unexpected error during token verification: {e}") + return None + + def decode_token_unsafe(self, token: str) -> Optional[Dict[str, Any]]: + """ + Decode JWT token without verification (for debugging only) + """ + try: + return jwt.decode( + token, + options={"verify_signature": False, "verify_exp": False} + ) + except Exception as e: + logger.error(f"Failed to decode token: {e}") + return None + + def _validate_payload(self, payload: Dict[str, Any]) -> bool: + """ + Validate JWT payload structure + FIXED: Comprehensive validation for required fields + """ + # Check required fields for all tokens + required_base_fields = ["sub", "user_id", "type", "exp", "iat"] + + for field in required_base_fields: + if field not in payload: + logger.warning(f"Missing required field in token: {field}") + return False + + # Validate token type + token_type = payload.get("type") + if token_type not in ["access", "refresh"]: + logger.warning(f"Invalid token type: {token_type}") + return False + + # Additional validation for access tokens + if token_type == "access": + if "email" not in payload: + logger.warning("Access token missing email field") + return False + + # Validate user_id format (should be UUID) + user_id = payload.get("user_id") + if not user_id or not isinstance(user_id, str): + logger.warning("Invalid user_id in token") + return False + + # Validate subject matches user_id + if payload.get("sub") != user_id: + logger.warning("Token subject does not match user_id") + return False + + return True + + def extract_user_id(self, token: str) -> Optional[str]: + """ + Extract user ID from token without full verification + Useful for quick user identification + """ + try: + payload = self.decode_token_unsafe(token) + if payload: + return payload.get("user_id") + except Exception as e: + logger.warning(f"Failed to extract user ID from token: {e}") + + return None + + def is_token_expired(self, token: str) -> bool: + """ + Check if token is expired without full verification + """ + try: + payload = self.decode_token_unsafe(token) + if payload and "exp" in payload: + exp = datetime.fromtimestamp(payload["exp"], tz=timezone.utc) + return exp < datetime.now(timezone.utc) + except Exception as e: + logger.warning(f"Failed to check token expiration: {e}") + + return True # Assume expired if we can't check + + def get_token_info(self, token: str) -> Dict[str, Any]: + """ + Get comprehensive token information for debugging + """ + info = { + "valid": False, + "expired": True, + "user_id": None, + "email": None, + "type": None, + "exp": None, + "iat": None + } + + try: + # Try unsafe decode first + payload = self.decode_token_unsafe(token) + if payload: + info.update({ + "user_id": payload.get("user_id"), + "email": payload.get("email"), + "type": payload.get("type"), + "exp": payload.get("exp"), + "iat": payload.get("iat"), + "expired": self.is_token_expired(token) + }) + + # Try full verification + verified_payload = self.verify_token(token) + info["valid"] = verified_payload is not None + + except Exception as e: + logger.warning(f"Failed to get token info: {e}") + + return info \ No newline at end of file diff --git a/test_new.sh b/test_new.sh index 9030aa2f..2786aa07 100755 --- a/test_new.sh +++ b/test_new.sh @@ -56,7 +56,7 @@ fi # ✅ NEW: Step 3.6 - Test a Protected Endpoint echo -e "\n3.6. Testing Protected Endpoint (User Profile)..." -USER_PROFILE_RESPONSE=$(curl -s -X GET "$API_BASE/api/v1/users/me" \ +USER_PROFILE_RESPONSE=$(curl -v -s -X GET "$API_BASE/api/v1/users/me" \ -H "Authorization: Bearer $ACCESS_TOKEN") echo "User Profile Response: $USER_PROFILE_RESPONSE"