From 1291d051833eac64a5ae5775abb9bee2b77d5884 Mon Sep 17 00:00:00 2001 From: Urtzi Alfaro Date: Sat, 26 Jul 2025 22:03:55 +0200 Subject: [PATCH] REFACTOR API gateway fix 7 --- services/auth/app/api/auth.py | 168 +++++++++++++-------- services/auth/app/core/security.py | 119 +++++++++++---- services/auth/app/services/auth_service.py | 102 +++++++++---- services/auth/requirements.txt | 1 + 4 files changed, 271 insertions(+), 119 deletions(-) diff --git a/services/auth/app/api/auth.py b/services/auth/app/api/auth.py index 997a8f22..55cd7efa 100644 --- a/services/auth/app/api/auth.py +++ b/services/auth/app/api/auth.py @@ -20,6 +20,91 @@ logger = structlog.get_logger() router = APIRouter() security = HTTPBearer() +@router.post("/register", response_model=TokenResponse) +@track_execution_time("registration_duration_seconds", "auth-service") +async def register( + user_data: UserRegistration, + request: Request, + db: AsyncSession = Depends(get_db) +): + """Register new user with enhanced debugging""" + metrics = get_metrics_collector(request) + + # ✅ DEBUG: Log incoming registration data (without password) + logger.info(f"Registration attempt for email: {user_data.email}") + logger.debug(f"Registration data - email: {user_data.email}, full_name: {user_data.full_name}") + + try: + # ✅ DEBUG: Validate input data + if not user_data.email or not user_data.email.strip(): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Email is required" + ) + + if not user_data.password or len(user_data.password) < 8: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Password must be at least 8 characters long" + ) + + if not user_data.full_name or not user_data.full_name.strip(): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Full name is required" + ) + + logger.debug(f"Input validation passed for {user_data.email}") + + # ✅ DEBUG: Call auth service with enhanced error tracking + result = await AuthService.register_user_with_tokens( + email=user_data.email.strip().lower(), # Normalize email + password=user_data.password, + full_name=user_data.full_name.strip(), + db=db + ) + + logger.info(f"Registration successful for {user_data.email}") + + # Record successful registration + if metrics: + metrics.increment_counter("registration_total", labels={"status": "success"}) + + # ✅ DEBUG: Validate response before returning + if not result.get("access_token"): + logger.error(f"Registration succeeded but no access_token in result for {user_data.email}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Registration completed but token generation failed" + ) + + logger.debug(f"Returning token response for {user_data.email}") + return TokenResponse(**result) + + except HTTPException as e: + # Record failed registration with specific error + if metrics: + error_type = "validation_error" if e.status_code == 400 else "conflict" if e.status_code == 409 else "failed" + metrics.increment_counter("registration_total", labels={"status": error_type}) + + logger.warning(f"Registration failed for {user_data.email}: {e.detail}") + raise + + except Exception as e: + # Record registration system error + if metrics: + metrics.increment_counter("registration_total", labels={"status": "error"}) + + logger.error(f"Registration system error for {user_data.email}: {str(e)}", exc_info=True) + + # ✅ DEBUG: Provide more specific error information in development + error_detail = f"Registration failed: {str(e)}" if logger.level == "DEBUG" else "Registration failed" + + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=error_detail + ) + @router.post("/login", response_model=TokenResponse) @track_execution_time("login_duration_seconds", "auth-service") async def login( @@ -27,28 +112,31 @@ async def login( request: Request, db: AsyncSession = Depends(get_db) ): - """ - Login user and return tokens - FIXED: Proper error handling and login attempt tracking - """ + """Login user with enhanced debugging""" metrics = get_metrics_collector(request) + logger.info(f"Login attempt for email: {login_data.email}") + 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"}) + # ✅ DEBUG: Validate login data + if not login_data.email or not login_data.email.strip(): 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." + status_code=status.HTTP_400_BAD_REQUEST, + detail="Email is required" + ) + + if not login_data.password: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Password is required" ) # 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) + result = await AuthService.login( + email=login_data.email.strip().lower(), # Normalize email + password=login_data.password, + db=db + ) # Record successful login if metrics: @@ -58,67 +146,25 @@ async def login( 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 + # Record failed login with specific reason if metrics: - reason = "rate_limited" if e.status_code == 429 else "auth_failed" + reason = "validation_error" if e.status_code == 400 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 + # Record login system error if metrics: metrics.increment_counter("login_failure_total", labels={"reason": "error"}) - logger.error(f"Login error for {login_data.email}: {e}") + logger.error(f"Login system error for {login_data.email}: {str(e)}", exc_info=True) 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") -async def register( - user_data: UserRegistration, - request: Request, - db: AsyncSession = Depends(get_db) -): - """Register new user""" - metrics = get_metrics_collector(request) - - try: - result = await AuthService.register_user_with_tokens(user_data.email, user_data.password, user_data.full_name, db) - - # Record successful registration - if metrics: - metrics.increment_counter("registration_total", labels={"status": "success"}) - - logger.info(f"User registered successfully: {user_data.email}") - return TokenResponse(**result) - - except HTTPException as e: - if metrics: - metrics.increment_counter("registration_total", labels={"status": "failed"}) - logger.warning(f"Registration failed for {user_data.email}: {e.detail}") - raise - except Exception as e: - if metrics: - metrics.increment_counter("registration_total", labels={"status": "error"}) - logger.error(f"Registration error for {user_data.email}: {e}") - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Registration failed" - ) - @router.post("/refresh", response_model=TokenResponse) @track_execution_time("token_refresh_duration_seconds", "auth-service") async def refresh_token( diff --git a/services/auth/app/core/security.py b/services/auth/app/core/security.py index fa5db5d3..5792c112 100644 --- a/services/auth/app/core/security.py +++ b/services/auth/app/core/security.py @@ -66,39 +66,106 @@ class SecurityManager: @staticmethod def create_access_token(user_data: Dict[str, Any], expires_delta: Optional[timedelta] = None) -> str: - """Create JWT access token""" - if expires_delta: - expire = datetime.now(timezone.utc) + expires_delta - else: - expire = datetime.now(timezone.utc) + timedelta(minutes=settings.JWT_ACCESS_TOKEN_EXPIRE_MINUTES) + """Create JWT access token with PROPER validation""" - payload = { - "sub": user_data["user_id"], - "user_id": user_data["user_id"], - "email": user_data["email"], - "type": "access", - "full_name": user_data.get("full_name", ""), - "is_verified": user_data.get("is_verified", False), - "exp": expire, - "iat": datetime.now(timezone.utc) - } + # ✅ FIX 1: Validate required fields BEFORE token creation + required_fields = ["user_id", "email"] + missing_fields = [field for field in required_fields if field not in user_data] - return jwt_handler.create_access_token(payload) + if missing_fields: + error_msg = f"Missing required fields for token creation: {missing_fields}" + logger.error(f"Token creation failed: {error_msg}") + raise ValueError(error_msg) + + # ✅ FIX 2: Validate that required fields are not None/empty + if not user_data.get("user_id"): + raise ValueError("user_id cannot be empty") + if not user_data.get("email"): + raise ValueError("email cannot be empty") + + try: + if expires_delta: + expire = datetime.now(timezone.utc) + expires_delta + else: + expire = datetime.now(timezone.utc) + timedelta(minutes=settings.JWT_ACCESS_TOKEN_EXPIRE_MINUTES) + + # ✅ FIX 3: Build payload with SAFE access to user_data + payload = { + "sub": user_data["user_id"], + "user_id": user_data["user_id"], + "email": user_data["email"], # ✅ Guaranteed to exist now + "type": "access", + "full_name": user_data.get("full_name", ""), # Safe access with default + "is_verified": user_data.get("is_verified", False), # Safe access with default + "is_active": user_data.get("is_active", True), # Safe access with default + "exp": expire, + "iat": datetime.now(timezone.utc), + "iss": "bakery-auth" # Token issuer + } + + logger.debug(f"Creating access token with payload keys: {list(payload.keys())}") + + # ✅ FIX 4: Use jwt_handler with proper error handling + token = jwt_handler.create_access_token(payload) + logger.debug(f"Access token created successfully for user {user_data['email']}") + return token + + except Exception as e: + logger.error(f"Access token creation failed for {user_data.get('email', 'unknown')}: {e}") + raise ValueError(f"Failed to create access token: {str(e)}") @staticmethod def create_refresh_token(user_data: Dict[str, Any]) -> str: - """Create JWT refresh token""" - expire = datetime.now(timezone.utc) + timedelta(days=settings.JWT_REFRESH_TOKEN_EXPIRE_DAYS) + """Create JWT refresh token with FLEXIBLE validation""" - payload = { - "sub": user_data["user_id"], - "user_id": user_data["user_id"], - "type": "refresh", - "exp": expire, - "iat": datetime.now(timezone.utc) - } + # ✅ FIX 1: Validate only essential fields for refresh token + if "user_id" not in user_data: + error_msg = "user_id required for refresh token creation" + logger.error(f"Refresh token creation failed: {error_msg}") + raise ValueError(error_msg) - return jwt_handler.create_access_token(payload) + if not user_data.get("user_id"): + raise ValueError("user_id cannot be empty") + + try: + expire = datetime.now(timezone.utc) + timedelta(days=settings.JWT_REFRESH_TOKEN_EXPIRE_DAYS) + + # ✅ FIX 2: Minimal payload for refresh token (email is optional) + payload = { + "sub": user_data["user_id"], + "user_id": user_data["user_id"], + "type": "refresh", + "exp": expire, + "iat": datetime.now(timezone.utc), + "iss": "bakery-auth" + } + + # ✅ FIX 3: Include email only if available (no longer required) + if "email" in user_data and user_data["email"]: + payload["email"] = user_data["email"] + + logger.debug(f"Creating refresh token with payload keys: {list(payload.keys())}") + + # Use the same JWT handler method (it handles both access and refresh) + token = jwt_handler.create_access_token(payload) + logger.debug(f"Refresh token created successfully for user {user_data['user_id']}") + return token + + except Exception as e: + logger.error(f"Refresh token creation failed for {user_data.get('user_id', 'unknown')}: {e}") + raise ValueError(f"Failed to create refresh token: {str(e)}") + + @staticmethod + def verify_token(token: str) -> Optional[Dict[str, Any]]: + """Verify JWT token with enhanced error handling""" + try: + payload = jwt_handler.verify_token(token) + if payload: + logger.debug(f"Token verified successfully for user: {payload.get('email', 'unknown')}") + return payload + except Exception as e: + logger.warning(f"Token verification failed: {e}") + return None @staticmethod async def track_login_attempt(email: str, ip_address: str, success: bool) -> None: diff --git a/services/auth/app/services/auth_service.py b/services/auth/app/services/auth_service.py index 5a90f411..7e2852c2 100644 --- a/services/auth/app/services/auth_service.py +++ b/services/auth/app/services/auth_service.py @@ -46,25 +46,51 @@ class AuthService: full_name=full_name, is_active=True, is_verified=False, - created_at=datetime.now(timezone.utc) + created_at=datetime.now(timezone.utc), + language='es', # Default language from logs + timezone='Europe/Madrid' # Default timezone from logs ) db.add(new_user) await db.flush() # Get user ID without committing - # ✅ FIX 2: Create complete user_data for token generation - complete_user_data = { + # ✅ FIX 1: Create COMPLETE and CONSISTENT user_data for token generation + token_user_data = { "user_id": str(new_user.id), - "email": new_user.email, + "email": new_user.email, # ✅ Ensure email is included "full_name": new_user.full_name, - "is_verified": new_user.is_verified + "is_verified": new_user.is_verified, + "is_active": new_user.is_active } - # Generate tokens with complete user data - access_token = SecurityManager.create_access_token(user_data=complete_user_data) + logger.debug(f"Creating tokens for user: {email} with data: {token_user_data}") + + # ✅ FIX 2: Generate tokens with VALIDATED user data + try: + access_token = SecurityManager.create_access_token(user_data=token_user_data) + logger.debug(f"Access token created successfully for {email}") + except Exception as token_error: + logger.error(f"Access token creation failed for {email}: {token_error}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Token creation failed: {token_error}" + ) - # ✅ FIX 3: Pass complete user data for refresh token too - refresh_token_value = SecurityManager.create_refresh_token(user_data=complete_user_data) + # ✅ FIX 3: Create refresh token with minimal but complete data + refresh_token_data = { + "user_id": str(new_user.id), + "email": new_user.email # Include email for consistency + } + + try: + refresh_token_value = SecurityManager.create_refresh_token(user_data=refresh_token_data) + logger.debug(f"Refresh token created successfully for {email}") + except Exception as refresh_error: + logger.error(f"Refresh token creation failed for {email}: {refresh_error}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Refresh token creation failed: {refresh_error}" + ) # Store refresh token in database refresh_token = RefreshToken( @@ -75,9 +101,12 @@ class AuthService: ) db.add(refresh_token) - await db.commit() - # Publish registration event + # ✅ FIX 4: Only commit after ALL token creation succeeds + await db.commit() + await db.refresh(new_user) + + # Publish registration event (non-blocking) try: await publish_user_registered({ "user_id": str(new_user.id), @@ -88,7 +117,7 @@ class AuthService: except Exception as e: logger.warning(f"Failed to publish registration event: {e}") - logger.info(f"User registered with tokens: {email}") + logger.info(f"User registered successfully with tokens: {email}") # Return unified token response format return { @@ -112,9 +141,10 @@ class AuthService: except Exception as e: await db.rollback() logger.error(f"Registration with tokens failed for {email}: {e}") + # ✅ FIX 5: Provide more specific error information raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Registration failed" + detail=f"Registration failed: {str(e)}" ) @staticmethod @@ -170,7 +200,7 @@ class AuthService: @staticmethod async def login(email: str, password: str, db: AsyncSession) -> Dict[str, Any]: - """Login user and return tokens (UNCHANGED)""" + """Login user and return tokens - FIXED VERSION""" try: # Get user result = await db.execute(select(User).where(User.email == email)) @@ -182,19 +212,27 @@ class AuthService: detail="Invalid credentials" ) - # Create tokens (standard lifespan for verified login) - access_token = SecurityManager.create_access_token( - user_data={ - "user_id": str(user.id), - "email": user.email, - "full_name": user.full_name, - "is_verified": user.is_verified - } - ) + # ✅ FIX 1: Create COMPLETE user data for access token + access_token_data = { + "user_id": str(user.id), + "email": user.email, # ✅ Include email + "full_name": user.full_name, + "is_verified": user.is_verified, + "is_active": user.is_active + } - refresh_token_value = SecurityManager.create_refresh_token( - user_data={"user_id": str(user.id)} - ) + # ✅ FIX 2: Create COMPLETE user data for refresh token + refresh_token_data = { + "user_id": str(user.id), + "email": user.email # ✅ Include email for consistency + } + + logger.debug(f"Creating access token for login with data: {list(access_token_data.keys())}") + logger.debug(f"Creating refresh token for login with data: {list(refresh_token_data.keys())}") + + # Create tokens with complete data + access_token = SecurityManager.create_access_token(user_data=access_token_data) + refresh_token_value = SecurityManager.create_refresh_token(user_data=refresh_token_data) # Store refresh token in database refresh_token = RefreshToken( @@ -207,15 +245,13 @@ class AuthService: db.add(refresh_token) await db.commit() - # Publish login event + # Publish login event (non-blocking) try: - await publish_user_login( - { + await publish_user_login({ "user_id": str(user.id), "email": user.email, "login_at": datetime.now(timezone.utc).isoformat() - } - ) + }) except Exception as e: logger.warning(f"Failed to publish login event: {e}") @@ -237,9 +273,11 @@ class AuthService: } except HTTPException: + await db.rollback() raise except Exception as e: - logger.error(f"Login error for {email}: {e}") + await db.rollback() + logger.error(f"Login failed for {email}: {e}") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Login failed" diff --git a/services/auth/requirements.txt b/services/auth/requirements.txt index f9360f8a..8f5a668c 100644 --- a/services/auth/requirements.txt +++ b/services/auth/requirements.txt @@ -15,6 +15,7 @@ aiosqlite==0.19.0 python-jose[cryptography]==3.3.0 passlib[bcrypt]==1.7.4 python-multipart==0.0.6 +bcrypt==4.0.1 # HTTP Client httpx==0.25.2