REFACTOR API gateway fix 8
This commit is contained in:
Urtzi Alfaro
@@ -3,16 +3,21 @@
|
||||
Authentication Service - Updated to support registration with direct token issuance
|
||||
"""
|
||||
|
||||
from datetime import datetime, timezone, timedelta
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy import select
|
||||
from fastapi import HTTPException, status
|
||||
import hashlib
|
||||
import uuid
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from typing import Dict, Any, Optional
|
||||
|
||||
from fastapi import HTTPException, status
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy import select, update
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
import structlog
|
||||
|
||||
from app.models.users import User, RefreshToken
|
||||
from app.schemas.auth import UserRegistration, UserLogin
|
||||
from app.core.security import SecurityManager
|
||||
from app.services.messaging import publish_user_registered, publish_user_login, publish_user_logout
|
||||
from app.services.messaging import publish_user_registered, publish_user_login
|
||||
|
||||
logger = structlog.get_logger()
|
||||
|
||||
@@ -20,91 +25,71 @@ class AuthService:
|
||||
"""Enhanced Authentication service with unified token response"""
|
||||
|
||||
@staticmethod
|
||||
async def register_user_with_tokens(
|
||||
email: str,
|
||||
password: str,
|
||||
full_name: str,
|
||||
db: AsyncSession
|
||||
) -> Dict[str, Any]:
|
||||
"""Register new user and return tokens directly - COMPLETELY FIXED"""
|
||||
async def register_user(user_data: UserRegistration, db: AsyncSession) -> Dict[str, Any]:
|
||||
"""Register a new user with FIXED token generation"""
|
||||
try:
|
||||
# Check if user already exists
|
||||
result = await db.execute(select(User).where(User.email == email))
|
||||
existing_user = result.scalar_one_or_none()
|
||||
|
||||
if existing_user:
|
||||
existing_user = await db.execute(
|
||||
select(User).where(User.email == user_data.email)
|
||||
)
|
||||
if existing_user.scalar_one_or_none():
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="User with this email already exists"
|
||||
)
|
||||
|
||||
# Create new user
|
||||
hashed_password = SecurityManager.hash_password(password)
|
||||
hashed_password = SecurityManager.hash_password(user_data.password)
|
||||
new_user = User(
|
||||
email=email,
|
||||
id=uuid.uuid4(),
|
||||
email=user_data.email,
|
||||
full_name=user_data.full_name,
|
||||
hashed_password=hashed_password,
|
||||
full_name=full_name,
|
||||
is_active=True,
|
||||
is_verified=False,
|
||||
created_at=datetime.now(timezone.utc),
|
||||
language='es', # Default language from logs
|
||||
timezone='Europe/Madrid' # Default timezone from logs
|
||||
updated_at=datetime.now(timezone.utc)
|
||||
)
|
||||
|
||||
db.add(new_user)
|
||||
await db.flush() # Get user ID without committing
|
||||
|
||||
# ✅ FIX 1: Create COMPLETE and CONSISTENT user_data for token generation
|
||||
token_user_data = {
|
||||
# ✅ FIX 1: Create SEPARATE access and refresh tokens with different payloads
|
||||
access_token_data = {
|
||||
"user_id": str(new_user.id),
|
||||
"email": new_user.email, # ✅ Ensure email is included
|
||||
"email": new_user.email,
|
||||
"full_name": new_user.full_name,
|
||||
"is_verified": new_user.is_verified,
|
||||
"is_active": new_user.is_active
|
||||
"is_active": new_user.is_active,
|
||||
"type": "access" # ✅ Explicitly mark as access token
|
||||
}
|
||||
|
||||
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: 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
|
||||
"email": new_user.email,
|
||||
"type": "refresh" # ✅ Explicitly mark as refresh token
|
||||
}
|
||||
|
||||
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}"
|
||||
)
|
||||
logger.debug(f"Creating tokens for registration: {user_data.email}")
|
||||
|
||||
# Store refresh token in database
|
||||
# ✅ FIX 2: Generate tokens with different payloads
|
||||
access_token = SecurityManager.create_access_token(user_data=access_token_data)
|
||||
refresh_token_value = SecurityManager.create_refresh_token(user_data=refresh_token_data)
|
||||
|
||||
logger.debug(f"Tokens created successfully for {user_data.email}")
|
||||
|
||||
# ✅ FIX 3: Store ONLY the refresh token in database (not access token)
|
||||
refresh_token = RefreshToken(
|
||||
id=uuid.uuid4(),
|
||||
user_id=new_user.id,
|
||||
token=refresh_token_value,
|
||||
expires_at=datetime.now(timezone.utc) + timedelta(days=7),
|
||||
is_revoked=False
|
||||
token=refresh_token_value, # Store the actual refresh token
|
||||
expires_at=datetime.now(timezone.utc) + timedelta(days=30),
|
||||
is_revoked=False,
|
||||
created_at=datetime.now(timezone.utc)
|
||||
)
|
||||
|
||||
db.add(refresh_token)
|
||||
|
||||
# ✅ FIX 4: Only commit after ALL token creation succeeds
|
||||
await db.commit()
|
||||
await db.refresh(new_user)
|
||||
|
||||
# Publish registration event (non-blocking)
|
||||
try:
|
||||
@@ -112,14 +97,13 @@ class AuthService:
|
||||
"user_id": str(new_user.id),
|
||||
"email": new_user.email,
|
||||
"full_name": new_user.full_name,
|
||||
"registered_at": new_user.created_at.isoformat()
|
||||
"registered_at": datetime.now(timezone.utc).isoformat()
|
||||
})
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to publish registration event: {e}")
|
||||
|
||||
logger.info(f"User registered successfully with tokens: {email}")
|
||||
logger.info(f"User registered successfully: {user_data.email}")
|
||||
|
||||
# Return unified token response format
|
||||
return {
|
||||
"access_token": access_token,
|
||||
"refresh_token": refresh_token_value,
|
||||
@@ -138,111 +122,101 @@ class AuthService:
|
||||
except HTTPException:
|
||||
await db.rollback()
|
||||
raise
|
||||
except Exception as e:
|
||||
except IntegrityError as e:
|
||||
await db.rollback()
|
||||
logger.error(f"Registration with tokens failed for {email}: {e}")
|
||||
# ✅ FIX 5: Provide more specific error information
|
||||
logger.error(f"Registration failed for {user_data.email}: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail=f"Registration failed: {str(e)}"
|
||||
detail="Registration failed"
|
||||
)
|
||||
except Exception as e:
|
||||
await db.rollback()
|
||||
logger.error(f"Registration failed for {user_data.email}: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Registration failed"
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
async def create_user(
|
||||
email: str,
|
||||
password: str,
|
||||
full_name: str,
|
||||
db: AsyncSession
|
||||
) -> User:
|
||||
"""
|
||||
Create user without tokens (LEGACY METHOD - kept for compatibility)
|
||||
Use register_user_with_tokens() for new implementations
|
||||
"""
|
||||
async def login_user(login_data: UserLogin, db: AsyncSession) -> Dict[str, Any]:
|
||||
"""Login user with FIXED token generation and SQLAlchemy syntax"""
|
||||
try:
|
||||
# Check if user already exists
|
||||
result = await db.execute(select(User).where(User.email == email))
|
||||
existing_user = result.scalar_one_or_none()
|
||||
|
||||
if existing_user:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail="User with this email already exists"
|
||||
)
|
||||
|
||||
# Create new user
|
||||
hashed_password = SecurityManager.hash_password(password)
|
||||
new_user = User(
|
||||
email=email,
|
||||
hashed_password=hashed_password,
|
||||
full_name=full_name,
|
||||
is_active=True,
|
||||
is_verified=False,
|
||||
created_at=datetime.now(timezone.utc)
|
||||
# Find user
|
||||
result = await db.execute(
|
||||
select(User).where(User.email == login_data.email)
|
||||
)
|
||||
|
||||
db.add(new_user)
|
||||
await db.commit()
|
||||
await db.refresh(new_user)
|
||||
|
||||
logger.info(f"User created (legacy): {email}")
|
||||
return new_user
|
||||
|
||||
except HTTPException:
|
||||
await db.rollback()
|
||||
raise
|
||||
except Exception as e:
|
||||
await db.rollback()
|
||||
logger.error(f"User creation failed for {email}: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="User creation failed"
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
async def login(email: str, password: str, db: AsyncSession) -> Dict[str, Any]:
|
||||
"""Login user and return tokens - FIXED VERSION"""
|
||||
try:
|
||||
# Get user
|
||||
result = await db.execute(select(User).where(User.email == email))
|
||||
user = result.scalar_one_or_none()
|
||||
|
||||
if not user or not SecurityManager.verify_password(password, user.hashed_password):
|
||||
if not user or not SecurityManager.verify_password(login_data.password, user.hashed_password):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Invalid credentials"
|
||||
detail="Invalid email or password"
|
||||
)
|
||||
|
||||
# ✅ FIX 1: Create COMPLETE user data for access token
|
||||
if not user.is_active:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail="Account is deactivated"
|
||||
)
|
||||
|
||||
# ✅ FIX 4: Revoke existing refresh tokens using proper SQLAlchemy ORM syntax
|
||||
logger.debug(f"Revoking existing refresh tokens for user: {user.id}")
|
||||
|
||||
# Using SQLAlchemy ORM update (more reliable than raw SQL)
|
||||
stmt = update(RefreshToken).where(
|
||||
RefreshToken.user_id == user.id,
|
||||
RefreshToken.is_revoked == False
|
||||
).values(
|
||||
is_revoked=True,
|
||||
revoked_at=datetime.now(timezone.utc)
|
||||
)
|
||||
|
||||
result = await db.execute(stmt)
|
||||
revoked_count = result.rowcount
|
||||
logger.debug(f"Revoked {revoked_count} existing refresh tokens for user: {user.id}")
|
||||
|
||||
# ✅ FIX 5: Create DIFFERENT token payloads
|
||||
access_token_data = {
|
||||
"user_id": str(user.id),
|
||||
"email": user.email, # ✅ Include email
|
||||
"email": user.email,
|
||||
"full_name": user.full_name,
|
||||
"is_verified": user.is_verified,
|
||||
"is_active": user.is_active
|
||||
"is_active": user.is_active,
|
||||
"type": "access" # ✅ Explicitly mark as access token
|
||||
}
|
||||
|
||||
# ✅ FIX 2: Create COMPLETE user data for refresh token
|
||||
refresh_token_data = {
|
||||
"user_id": str(user.id),
|
||||
"email": user.email # ✅ Include email for consistency
|
||||
"email": user.email,
|
||||
"type": "refresh", # ✅ Explicitly mark as refresh token
|
||||
"jti": str(uuid.uuid4()) # ✅ Add unique identifier for each refresh token
|
||||
}
|
||||
|
||||
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
|
||||
# ✅ FIX 6: Generate tokens with different payloads and expiration
|
||||
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
|
||||
logger.debug(f"Access token created successfully for user {login_data.email}")
|
||||
logger.debug(f"Refresh token created successfully for user {str(user.id)}")
|
||||
|
||||
# ✅ FIX 7: Store ONLY refresh token in database with unique constraint handling
|
||||
refresh_token = RefreshToken(
|
||||
id=uuid.uuid4(),
|
||||
user_id=user.id,
|
||||
token=refresh_token_value,
|
||||
token=refresh_token_value, # This should be the refresh token, not access token
|
||||
expires_at=datetime.now(timezone.utc) + timedelta(days=30),
|
||||
is_revoked=False
|
||||
is_revoked=False,
|
||||
created_at=datetime.now(timezone.utc)
|
||||
)
|
||||
|
||||
db.add(refresh_token)
|
||||
|
||||
# Update last login
|
||||
user.last_login = datetime.now(timezone.utc)
|
||||
|
||||
await db.commit()
|
||||
|
||||
# Publish login event (non-blocking)
|
||||
@@ -255,13 +229,13 @@ class AuthService:
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to publish login event: {e}")
|
||||
|
||||
logger.info(f"User logged in successfully: {email}")
|
||||
logger.info(f"User logged in successfully: {login_data.email}")
|
||||
|
||||
return {
|
||||
"access_token": access_token,
|
||||
"refresh_token": refresh_token_value,
|
||||
"token_type": "bearer",
|
||||
"expires_in": 3600, # 1 hour
|
||||
"expires_in": 1800, # 30 minutes
|
||||
"user": {
|
||||
"id": str(user.id),
|
||||
"email": user.email,
|
||||
@@ -275,108 +249,119 @@ class AuthService:
|
||||
except HTTPException:
|
||||
await db.rollback()
|
||||
raise
|
||||
except IntegrityError as e:
|
||||
await db.rollback()
|
||||
logger.error(f"Login failed for {login_data.email}: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Login failed"
|
||||
)
|
||||
except Exception as e:
|
||||
await db.rollback()
|
||||
logger.error(f"Login failed for {email}: {e}")
|
||||
logger.error(f"Login failed for {login_data.email}: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Login failed"
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
async def logout_user(user_id: str, refresh_token: str, db: AsyncSession) -> bool:
|
||||
"""Logout user by revoking refresh token"""
|
||||
try:
|
||||
# Revoke the specific refresh token using ORM
|
||||
stmt = update(RefreshToken).where(
|
||||
RefreshToken.user_id == user_id,
|
||||
RefreshToken.token == refresh_token,
|
||||
RefreshToken.is_revoked == False
|
||||
).values(
|
||||
is_revoked=True,
|
||||
revoked_at=datetime.now(timezone.utc)
|
||||
)
|
||||
|
||||
result = await db.execute(stmt)
|
||||
|
||||
if result.rowcount > 0:
|
||||
await db.commit()
|
||||
logger.info(f"User logged out successfully: {user_id}")
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
except Exception as e:
|
||||
await db.rollback()
|
||||
logger.error(f"Logout failed for user {user_id}: {e}")
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
async def refresh_access_token(refresh_token: str, db: AsyncSession) -> Dict[str, Any]:
|
||||
"""Refresh access token using refresh token (UNCHANGED)"""
|
||||
"""Refresh access token using refresh token"""
|
||||
try:
|
||||
# Verify refresh token
|
||||
payload = SecurityManager.verify_token(refresh_token)
|
||||
if not payload:
|
||||
payload = SecurityManager.decode_token(refresh_token)
|
||||
user_id = payload.get("user_id")
|
||||
|
||||
if not user_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Invalid refresh token"
|
||||
)
|
||||
|
||||
user_id = payload.get("user_id")
|
||||
if not user_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Invalid token payload"
|
||||
)
|
||||
|
||||
# Check if refresh token exists and is not revoked
|
||||
# Check if refresh token exists and is valid using ORM
|
||||
result = await db.execute(
|
||||
select(RefreshToken).where(
|
||||
RefreshToken.user_id == user_id,
|
||||
RefreshToken.token == refresh_token,
|
||||
RefreshToken.is_revoked == False,
|
||||
RefreshToken.expires_at > datetime.now(timezone.utc)
|
||||
)
|
||||
)
|
||||
|
||||
stored_token = result.scalar_one_or_none()
|
||||
|
||||
if not stored_token:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Invalid or expired refresh token"
|
||||
)
|
||||
|
||||
# Get user info
|
||||
result = await db.execute(select(User).where(User.id == user_id))
|
||||
user = result.scalar_one_or_none()
|
||||
# Get user
|
||||
user_result = await db.execute(
|
||||
select(User).where(User.id == user_id)
|
||||
)
|
||||
user = user_result.scalar_one_or_none()
|
||||
|
||||
if not user:
|
||||
if not user or not user.is_active:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="User not found"
|
||||
detail="User not found or inactive"
|
||||
)
|
||||
|
||||
# Create new access token
|
||||
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
|
||||
}
|
||||
)
|
||||
access_token_data = {
|
||||
"user_id": str(user.id),
|
||||
"email": user.email,
|
||||
"full_name": user.full_name,
|
||||
"is_verified": user.is_verified,
|
||||
"is_active": user.is_active,
|
||||
"type": "access"
|
||||
}
|
||||
|
||||
logger.info(f"Token refreshed successfully for user {user_id}")
|
||||
new_access_token = SecurityManager.create_access_token(user_data=access_token_data)
|
||||
|
||||
return {
|
||||
"access_token": access_token,
|
||||
"access_token": new_access_token,
|
||||
"token_type": "bearer",
|
||||
"expires_in": 3600
|
||||
"expires_in": 1800
|
||||
}
|
||||
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.error(f"Token refresh error: {e}")
|
||||
logger.error(f"Token refresh failed: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Token refresh failed"
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
async def logout(refresh_token: str, db: AsyncSession) -> bool:
|
||||
"""Logout user by revoking refresh token (UNCHANGED)"""
|
||||
try:
|
||||
# Revoke refresh token
|
||||
result = await db.execute(
|
||||
select(RefreshToken).where(RefreshToken.token == refresh_token)
|
||||
)
|
||||
|
||||
token = result.scalar_one_or_none()
|
||||
if token:
|
||||
token.is_revoked = True
|
||||
token.revoked_at = datetime.now(timezone.utc)
|
||||
await db.commit()
|
||||
|
||||
return True
|
||||
|
||||
except Exception as e:
|
||||
logger.error(f"Logout error: {e}")
|
||||
await db.rollback()
|
||||
return False
|
||||
|
||||
|
||||
@staticmethod
|
||||
async def verify_user_token(token: str) -> Dict[str, Any]:
|
||||
"""Verify access token and return user info (UNCHANGED)"""
|
||||
|
||||
Reference in New Issue
Block a user