REFACTOR API gateway fix 7
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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 3: Pass complete user data for refresh token too
|
||||
refresh_token_value = SecurityManager.create_refresh_token(user_data=complete_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: 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"
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user