mirror of
https://github.com/MODSetter/SurfSense.git
synced 2026-07-02 22:01:05 +02:00
refactor: update authentication error handling to prevent user enumeration and improve error messages
This commit is contained in:
parent
2add106296
commit
c1016591da
3 changed files with 14 additions and 44 deletions
|
|
@ -62,7 +62,7 @@ def _rate_limit_exceeded_handler(request: Request, exc: RateLimitExceeded):
|
||||||
# ============================================================================
|
# ============================================================================
|
||||||
# Stricter per-IP limits on auth endpoints to prevent:
|
# Stricter per-IP limits on auth endpoints to prevent:
|
||||||
# - Brute force password attacks
|
# - Brute force password attacks
|
||||||
# - User enumeration via LOGIN_USER_NOT_FOUND / REGISTER_USER_ALREADY_EXISTS
|
# - User enumeration via REGISTER_USER_ALREADY_EXISTS
|
||||||
# - Email spam via forgot-password
|
# - Email spam via forgot-password
|
||||||
#
|
#
|
||||||
# These use direct Redis INCR+EXPIRE for simplicity and reliability.
|
# These use direct Redis INCR+EXPIRE for simplicity and reliability.
|
||||||
|
|
|
||||||
|
|
@ -2,7 +2,7 @@ import logging
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
from fastapi import Depends, HTTPException, Request, Response
|
from fastapi import Depends, Request, Response
|
||||||
from fastapi.responses import JSONResponse, RedirectResponse
|
from fastapi.responses import JSONResponse, RedirectResponse
|
||||||
from fastapi_users import BaseUserManager, FastAPIUsers, UUIDIDMixin, models
|
from fastapi_users import BaseUserManager, FastAPIUsers, UUIDIDMixin, models
|
||||||
from fastapi_users.authentication import (
|
from fastapi_users.authentication import (
|
||||||
|
|
@ -47,6 +47,14 @@ if config.AUTH_TYPE == "GOOGLE":
|
||||||
|
|
||||||
|
|
||||||
class UserManager(UUIDIDMixin, BaseUserManager[User, uuid.UUID]):
|
class UserManager(UUIDIDMixin, BaseUserManager[User, uuid.UUID]):
|
||||||
|
"""
|
||||||
|
Custom user manager extending fastapi-users BaseUserManager.
|
||||||
|
|
||||||
|
Authentication returns a generic error for both non-existent accounts
|
||||||
|
and incorrect passwords to comply with OWASP WSTG-IDNT-04 and
|
||||||
|
prevent user enumeration attacks.
|
||||||
|
"""
|
||||||
|
|
||||||
reset_password_token_secret = SECRET
|
reset_password_token_secret = SECRET
|
||||||
verification_token_secret = SECRET
|
verification_token_secret = SECRET
|
||||||
|
|
||||||
|
|
@ -180,36 +188,6 @@ class UserManager(UUIDIDMixin, BaseUserManager[User, uuid.UUID]):
|
||||||
):
|
):
|
||||||
print(f"Verification requested for user {user.id}. Verification token: {token}")
|
print(f"Verification requested for user {user.id}. Verification token: {token}")
|
||||||
|
|
||||||
async def authenticate(self, credentials):
|
|
||||||
"""
|
|
||||||
Override to return a specific error when user account doesn't exist,
|
|
||||||
instead of a generic LOGIN_BAD_CREDENTIALS.
|
|
||||||
"""
|
|
||||||
from fastapi_users.exceptions import UserNotExists
|
|
||||||
|
|
||||||
try:
|
|
||||||
user = await self.get_by_email(credentials.username)
|
|
||||||
except UserNotExists:
|
|
||||||
# Still hash the password to mitigate timing attacks
|
|
||||||
self.password_helper.hash(credentials.password)
|
|
||||||
logger.warning(
|
|
||||||
f"Login attempt for non-existent account: {credentials.username}"
|
|
||||||
)
|
|
||||||
raise HTTPException(
|
|
||||||
status_code=400,
|
|
||||||
detail="LOGIN_USER_NOT_FOUND",
|
|
||||||
) from None
|
|
||||||
|
|
||||||
verified, updated_password_hash = self.password_helper.verify_and_update(
|
|
||||||
credentials.password, user.hashed_password
|
|
||||||
)
|
|
||||||
if not verified:
|
|
||||||
logger.warning(f"Failed login attempt (wrong password) for user: {user.id}")
|
|
||||||
return None
|
|
||||||
if updated_password_hash is not None:
|
|
||||||
await self.user_db.update(user, {"hashed_password": updated_password_hash})
|
|
||||||
return user
|
|
||||||
|
|
||||||
|
|
||||||
async def get_user_manager(user_db: SQLAlchemyUserDatabase = Depends(get_user_db)):
|
async def get_user_manager(user_db: SQLAlchemyUserDatabase = Depends(get_user_db)):
|
||||||
yield UserManager(user_db)
|
yield UserManager(user_db)
|
||||||
|
|
|
||||||
|
|
@ -20,8 +20,8 @@ const AUTH_ERROR_MESSAGES: AuthErrorMapping = {
|
||||||
description: "Your account may be suspended or restricted",
|
description: "Your account may be suspended or restricted",
|
||||||
},
|
},
|
||||||
"404": {
|
"404": {
|
||||||
title: "Account not found",
|
title: "Not found",
|
||||||
description: "No account exists with this email address",
|
description: "The requested resource was not found",
|
||||||
},
|
},
|
||||||
"409": {
|
"409": {
|
||||||
title: "Account conflict",
|
title: "Account conflict",
|
||||||
|
|
@ -46,12 +46,8 @@ const AUTH_ERROR_MESSAGES: AuthErrorMapping = {
|
||||||
|
|
||||||
// FastAPI specific errors
|
// FastAPI specific errors
|
||||||
LOGIN_BAD_CREDENTIALS: {
|
LOGIN_BAD_CREDENTIALS: {
|
||||||
title: "Invalid credentials",
|
title: "Login failed",
|
||||||
description: "The email or password you entered is incorrect",
|
description: "Invalid email or password. If you don't have an account, please sign up.",
|
||||||
},
|
|
||||||
LOGIN_USER_NOT_FOUND: {
|
|
||||||
title: "Account not found",
|
|
||||||
description: "No account exists with this email address. Please sign up first.",
|
|
||||||
},
|
},
|
||||||
LOGIN_USER_NOT_VERIFIED: {
|
LOGIN_USER_NOT_VERIFIED: {
|
||||||
title: "Account not verified",
|
title: "Account not verified",
|
||||||
|
|
@ -147,10 +143,6 @@ export function getAuthErrorMessage(errorCode: string, returnTitle: boolean = fa
|
||||||
if (!errorInfo) {
|
if (!errorInfo) {
|
||||||
const patterns = [
|
const patterns = [
|
||||||
{ pattern: /credential|password|email/i, code: "LOGIN_BAD_CREDENTIALS" },
|
{ pattern: /credential|password|email/i, code: "LOGIN_BAD_CREDENTIALS" },
|
||||||
{
|
|
||||||
pattern: /not found|no account|does not exist|user not found/i,
|
|
||||||
code: "LOGIN_USER_NOT_FOUND",
|
|
||||||
},
|
|
||||||
{ pattern: /verify|verification/i, code: "LOGIN_USER_NOT_VERIFIED" },
|
{ pattern: /verify|verification/i, code: "LOGIN_USER_NOT_VERIFIED" },
|
||||||
{ pattern: /inactive|disabled|suspended/i, code: "USER_INACTIVE" },
|
{ pattern: /inactive|disabled|suspended/i, code: "USER_INACTIVE" },
|
||||||
{ pattern: /exists|duplicate/i, code: "REGISTER_USER_ALREADY_EXISTS" },
|
{ pattern: /exists|duplicate/i, code: "REGISTER_USER_ALREADY_EXISTS" },
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue