updates
This commit is contained in:
Binary file not shown.
@@ -149,11 +149,39 @@ async def login(
|
||||
request_id = getattr(request.state, 'request_id', None)
|
||||
|
||||
try:
|
||||
result = await auth_service.login(db=db, email=login_request.email, password=login_request.password, remember_me=login_request.rememberMe or False, mfa_token=login_request.mfaToken)
|
||||
result = await auth_service.login(
|
||||
db=db,
|
||||
email=login_request.email,
|
||||
password=login_request.password,
|
||||
remember_me=login_request.rememberMe or False,
|
||||
mfa_token=login_request.mfaToken,
|
||||
expected_role=login_request.expectedRole
|
||||
)
|
||||
if result.get('requires_mfa'):
|
||||
# Log MFA required
|
||||
user = db.query(User).filter(User.email == login_request.email.lower().strip()).first()
|
||||
if user:
|
||||
# Validate role even for MFA flow if expected_role is provided
|
||||
if login_request.expectedRole and user.role:
|
||||
user_role = user.role.name.lower() if hasattr(user.role, 'name') else None
|
||||
expected_role = login_request.expectedRole.lower()
|
||||
if user_role != expected_role:
|
||||
await audit_service.log_action(
|
||||
db=db,
|
||||
action='login_role_mismatch',
|
||||
resource_type='authentication',
|
||||
user_id=user.id,
|
||||
ip_address=client_ip,
|
||||
user_agent=user_agent,
|
||||
request_id=request_id,
|
||||
details={'email': login_request.email, 'expected_role': expected_role, 'actual_role': user_role},
|
||||
status='failed'
|
||||
)
|
||||
return JSONResponse(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
content={'status': 'error', 'message': f'This login endpoint is only for {expected_role} users'}
|
||||
)
|
||||
|
||||
await audit_service.log_action(
|
||||
db=db,
|
||||
action='login_mfa_required',
|
||||
@@ -212,6 +240,31 @@ async def login(
|
||||
logger = get_logger(__name__)
|
||||
logger.warning(f'Failed to create session during login: {str(e)}')
|
||||
|
||||
# Validate role if expected_role is provided (for role-specific login endpoints)
|
||||
if login_request.expectedRole:
|
||||
user_role = result['user'].get('role', '').lower()
|
||||
expected_role = login_request.expectedRole.lower()
|
||||
if user_role != expected_role:
|
||||
# Log security event
|
||||
await audit_service.log_action(
|
||||
db=db,
|
||||
action='login_role_mismatch',
|
||||
resource_type='authentication',
|
||||
user_id=result['user']['id'],
|
||||
ip_address=client_ip,
|
||||
user_agent=user_agent,
|
||||
request_id=request_id,
|
||||
details={'email': login_request.email, 'expected_role': expected_role, 'actual_role': user_role},
|
||||
status='failed'
|
||||
)
|
||||
# Clear cookies on role mismatch
|
||||
response.delete_cookie(key='refreshToken', path='/', secure=settings.is_production, samesite=samesite_value)
|
||||
response.delete_cookie(key='accessToken', path='/', secure=settings.is_production, samesite=samesite_value)
|
||||
return JSONResponse(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
content={'status': 'error', 'message': f'This login endpoint is only for {expected_role} users'}
|
||||
)
|
||||
|
||||
# Log successful login
|
||||
await audit_service.log_action(
|
||||
db=db,
|
||||
@@ -229,9 +282,25 @@ async def login(
|
||||
return {'status': 'success', 'data': {'user': result['user']}}
|
||||
except ValueError as e:
|
||||
error_message = str(e)
|
||||
status_code = status.HTTP_401_UNAUTHORIZED if 'Invalid email or password' in error_message or 'Invalid MFA token' in error_message else status.HTTP_400_BAD_REQUEST
|
||||
# SECURITY: Sanitize error messages to prevent information disclosure
|
||||
# Don't reveal specific details about account status, remaining attempts, etc. to unauthenticated users
|
||||
sanitized_message = error_message
|
||||
|
||||
# Log failed login attempt
|
||||
# Generic error messages for authentication failures
|
||||
if 'Invalid email or password' in error_message or 'Invalid MFA token' in error_message:
|
||||
sanitized_message = 'Invalid email or password'
|
||||
status_code = status.HTTP_401_UNAUTHORIZED
|
||||
elif 'Account is disabled' in error_message:
|
||||
sanitized_message = 'Account is disabled. Please contact support.'
|
||||
status_code = status.HTTP_403_FORBIDDEN
|
||||
elif 'Account is temporarily locked' in error_message or 'Account has been temporarily locked' in error_message:
|
||||
# Keep lockout message but remove specific timing details for security
|
||||
sanitized_message = 'Account is temporarily locked due to multiple failed login attempts. Please try again later.'
|
||||
status_code = status.HTTP_403_FORBIDDEN
|
||||
else:
|
||||
status_code = status.HTTP_400_BAD_REQUEST
|
||||
|
||||
# Log failed login attempt with full details (for internal logging)
|
||||
await audit_service.log_action(
|
||||
db=db,
|
||||
action='login_failed',
|
||||
@@ -241,10 +310,10 @@ async def login(
|
||||
request_id=request_id,
|
||||
details={'email': login_request.email},
|
||||
status='failed',
|
||||
error_message=error_message
|
||||
error_message=error_message # Log full error internally
|
||||
)
|
||||
|
||||
return JSONResponse(status_code=status_code, content={'status': 'error', 'message': error_message})
|
||||
return JSONResponse(status_code=status_code, content={'status': 'error', 'message': sanitized_message})
|
||||
|
||||
@router.post('/refresh-token', response_model=TokenResponse)
|
||||
async def refresh_token(
|
||||
|
||||
Binary file not shown.
@@ -32,6 +32,7 @@ class LoginRequest(BaseModel):
|
||||
password: str
|
||||
rememberMe: Optional[bool] = False
|
||||
mfaToken: Optional[str] = None
|
||||
expectedRole: Optional[str] = None # Optional role validation for role-specific login endpoints
|
||||
|
||||
class RefreshTokenRequest(BaseModel):
|
||||
refreshToken: Optional[str] = None
|
||||
|
||||
Binary file not shown.
@@ -46,6 +46,16 @@ class AuthService:
|
||||
else:
|
||||
logger.warning(error_msg)
|
||||
|
||||
# SECURITY: Validate JWT secret entropy (check for predictable patterns)
|
||||
# Check if secret appears to be randomly generated (not a simple pattern)
|
||||
if len(set(self.jwt_secret)) < len(self.jwt_secret) * 0.3: # Less than 30% unique characters suggests low entropy
|
||||
error_msg = 'JWT_SECRET appears to have low entropy. Please use a randomly generated secret.'
|
||||
logger.error(error_msg)
|
||||
if settings.is_production:
|
||||
raise ValueError(error_msg)
|
||||
else:
|
||||
logger.warning(error_msg)
|
||||
|
||||
# Refresh secret should be different from access secret
|
||||
self.jwt_refresh_secret = os.getenv("JWT_REFRESH_SECRET")
|
||||
if not self.jwt_refresh_secret:
|
||||
@@ -187,7 +197,7 @@ class AuthService:
|
||||
"refreshToken": tokens["refreshToken"]
|
||||
}
|
||||
|
||||
async def login(self, db: Session, email: str, password: str, remember_me: bool = False, mfa_token: str = None) -> dict:
|
||||
async def login(self, db: Session, email: str, password: str, remember_me: bool = False, mfa_token: str = None, expected_role: str = None) -> dict:
|
||||
|
||||
email = email.lower().strip() if email else ""
|
||||
if not email:
|
||||
@@ -206,9 +216,9 @@ class AuthService:
|
||||
# Check if account is locked (reset if lockout expired)
|
||||
if user.locked_until:
|
||||
if user.locked_until > datetime.utcnow():
|
||||
remaining_minutes = int((user.locked_until - datetime.utcnow()).total_seconds() / 60)
|
||||
logger.warning(f"Login attempt for locked account: {email} (locked until {user.locked_until})")
|
||||
raise ValueError(f"Account is temporarily locked due to multiple failed login attempts. Please try again in {remaining_minutes} minute(s).")
|
||||
# SECURITY: Don't reveal specific lockout duration to prevent timing attacks
|
||||
raise ValueError("Account is temporarily locked due to multiple failed login attempts. Please try again later.")
|
||||
else:
|
||||
# Lockout expired, reset it
|
||||
user.locked_until = None
|
||||
@@ -230,12 +240,13 @@ class AuthService:
|
||||
user.locked_until = datetime.utcnow() + timedelta(minutes=lockout_duration)
|
||||
logger.warning(f"Account locked due to {user.failed_login_attempts} failed login attempts: {email}")
|
||||
db.commit()
|
||||
raise ValueError(f"Account has been temporarily locked due to {max_attempts} failed login attempts. Please try again in {lockout_duration} minute(s).")
|
||||
# SECURITY: Don't reveal specific lockout duration
|
||||
raise ValueError("Account has been temporarily locked due to multiple failed login attempts. Please try again later.")
|
||||
else:
|
||||
remaining_attempts = max_attempts - user.failed_login_attempts
|
||||
logger.warning(f"Login attempt with invalid password for user: {email} ({user.failed_login_attempts}/{max_attempts} failed attempts)")
|
||||
db.commit()
|
||||
raise ValueError(f"Invalid email or password. {remaining_attempts} attempt(s) remaining before account lockout.")
|
||||
# SECURITY: Don't reveal remaining attempts to prevent enumeration
|
||||
raise ValueError("Invalid email or password")
|
||||
|
||||
if user.mfa_enabled:
|
||||
if not mfa_token:
|
||||
@@ -258,12 +269,21 @@ class AuthService:
|
||||
user.locked_until = datetime.utcnow() + timedelta(minutes=lockout_duration)
|
||||
logger.warning(f"Account locked due to {user.failed_login_attempts} failed attempts (MFA failure): {email}")
|
||||
db.commit()
|
||||
raise ValueError(f"Account has been temporarily locked due to {max_attempts} failed login attempts. Please try again in {lockout_duration} minute(s).")
|
||||
# SECURITY: Don't reveal specific lockout duration
|
||||
raise ValueError("Account has been temporarily locked due to multiple failed login attempts. Please try again later.")
|
||||
else:
|
||||
remaining_attempts = max_attempts - user.failed_login_attempts
|
||||
db.commit()
|
||||
raise ValueError(f"Invalid MFA token. {remaining_attempts} attempt(s) remaining before account lockout.")
|
||||
# SECURITY: Don't reveal remaining attempts
|
||||
raise ValueError("Invalid MFA token")
|
||||
|
||||
# Validate role if expected_role is provided (for role-specific login endpoints)
|
||||
if expected_role:
|
||||
user_role = user.role.name.lower() if user.role and hasattr(user.role, 'name') else None
|
||||
expected_role_lower = expected_role.lower()
|
||||
if user_role != expected_role_lower:
|
||||
logger.warning(f"Role mismatch: user {email} has role {user_role} but expected {expected_role_lower}")
|
||||
raise ValueError(f"This login endpoint is only for {expected_role_lower} users")
|
||||
|
||||
# Reset failed login attempts and unlock account on successful login
|
||||
if user.failed_login_attempts > 0 or user.locked_until:
|
||||
user.failed_login_attempts = 0
|
||||
@@ -447,7 +467,8 @@ class AuthService:
|
||||
|
||||
db.query(PasswordResetToken).filter(PasswordResetToken.user_id == user.id).delete()
|
||||
|
||||
expires_at = datetime.utcnow() + timedelta(hours=1)
|
||||
# SECURITY: Shorter expiration for password reset tokens (15 minutes instead of 1 hour)
|
||||
expires_at = datetime.utcnow() + timedelta(minutes=15)
|
||||
reset_token_obj = PasswordResetToken(
|
||||
user_id=user.id,
|
||||
token=hashed_token,
|
||||
|
||||
Binary file not shown.
@@ -50,6 +50,18 @@ def get_jwt_secret() -> str:
|
||||
else:
|
||||
logger.warning(error_msg)
|
||||
|
||||
# SECURITY: Validate JWT secret entropy (check for predictable patterns)
|
||||
# Check if secret appears to be randomly generated (not a simple pattern)
|
||||
if len(set(jwt_secret)) < len(jwt_secret) * 0.3: # Less than 30% unique characters suggests low entropy
|
||||
error_msg = 'JWT_SECRET appears to have low entropy. Please use a randomly generated secret.'
|
||||
import logging
|
||||
logger = logging.getLogger(__name__)
|
||||
logger.error(error_msg)
|
||||
if settings.is_production:
|
||||
raise ValueError(error_msg)
|
||||
else:
|
||||
logger.warning(error_msg)
|
||||
|
||||
return jwt_secret
|
||||
|
||||
def get_current_user(
|
||||
|
||||
Reference in New Issue
Block a user