From 512c39e4f4325103dfb812d7704dfbfb5a97a3e1 Mon Sep 17 00:00:00 2001 From: Furqan-14 Date: Tue, 17 Feb 2026 11:57:24 +0530 Subject: [PATCH] fix: stronger handling of data --- .env.production | 1 + app/config/settings.py | 6 ++- app/controllers/auth/auth_controller.py | 4 ++ app/controllers/auth/sso_controller.py | 25 ++++++++--- .../theme/color_palette_controller.py | 8 ++-- app/routes/auth/__init__.py | 8 ++-- app/routes/auth/auth.py | 42 ++++++++++++++++--- app/schemas/auth/auth_schema.py | 3 +- app/schemas/auth/module_environment_schema.py | 1 - app/services/auth/trust_service.py | 4 +- app/services/auth/user_service.py | 15 +++++++ app/services/theme/color_palette_service.py | 10 ++--- 12 files changed, 95 insertions(+), 32 deletions(-) diff --git a/.env.production b/.env.production index 86417ab..2e2e9fe 100644 --- a/.env.production +++ b/.env.production @@ -42,6 +42,7 @@ ACCESS_TOKEN_EXPIRES=900 REFRESH_TOKEN_SECRET="6Z0yOfkhPjfLH77WTnBh3Iv0JU_gWIfIqpGuGU41GFRV4fnLZMfKN3gAsPTfsqFKv1rRc6szUJRngW8Py0UYUQ" REFRESH_TOKEN_EXPIRES=864000 JWT_ALGORITHM=HS256 +COOKIE_SECURE=true ADMIN_JWT="upRCbNd-3Ex3sG2aEHxcrCx7LZu91BkiNGPIs-vxNXp7YBHyU-0jMpGUYA5dJHLcyOIMLBk4HCw1cpI4WOlIzA" # External SaaS Webhook diff --git a/app/config/settings.py b/app/config/settings.py index 5745ee2..b062f5a 100644 --- a/app/config/settings.py +++ b/app/config/settings.py @@ -72,7 +72,11 @@ class Settings(BaseSettings): REFRESH_TOKEN_SECRET: str REFRESH_TOKEN_EXPIRES: int = 864000 JWT_ALGORITHM: str = "HS256" - + + # Cookie settings + COOKIE_SECURE: bool = False + COOKIE_DOMAIN: Optional[str] = None + # Super Admin Setup SUPER_ADMIN_EMAIL: str SUPER_ADMIN_PASSWORD: str diff --git a/app/controllers/auth/auth_controller.py b/app/controllers/auth/auth_controller.py index 0614f0c..9c9b568 100644 --- a/app/controllers/auth/auth_controller.py +++ b/app/controllers/auth/auth_controller.py @@ -27,6 +27,10 @@ class AuthController: def refresh_token(db: Session, token_data: RefreshTokenRequest): return AuthService.refresh_access_token(db, token_data.refresh_token) + @staticmethod + def refresh_token_raw(db: Session, refresh_token: str): + return AuthService.refresh_access_token(db, refresh_token) + @staticmethod def update_user( db: Session, user_id: uuid.UUID, user_data: UserUpdate, current_user: User diff --git a/app/controllers/auth/sso_controller.py b/app/controllers/auth/sso_controller.py index 80f21b1..11690fa 100644 --- a/app/controllers/auth/sso_controller.py +++ b/app/controllers/auth/sso_controller.py @@ -2,6 +2,7 @@ from sqlalchemy.orm import Session from fastapi import Request, HTTPException from typing import Optional, Dict, Any import uuid +import logging from app.services.auth.sso_service import SSOService from app.services.auth.trust_service import TrustService from app.schemas.auth.sso_schema import SSOInitiateRequest, SSOExchangeRequest @@ -9,6 +10,8 @@ from app.models.auth.module_model import Module from app.models.auth.module_environment_model import ModuleEnvironment from app.models.auth.user_model import User +logger = logging.getLogger(__name__) + class SSOController: @staticmethod def initiate_sso(db: Session, request: SSOInitiateRequest, current_user: User): @@ -44,11 +47,23 @@ class SSOController: if x_module_key: headers["X-Module-Key"] = x_module_key - TrustService.validate_module_trust( - environment=env, - request_headers=headers, - request_body="" - ) + actual_body = payload.model_dump_json() + try: + TrustService.validate_module_trust( + environment=env, + request_headers=headers, + request_body=actual_body + ) + except HTTPException: + logger.warning( + "HMAC verify with body failed for %s, trying empty fallback (DEPRECATED)", + payload.module_id, + ) + TrustService.validate_module_trust( + environment=env, + request_headers=headers, + request_body="" + ) return SSOService.exchange_grant( db=db, diff --git a/app/controllers/theme/color_palette_controller.py b/app/controllers/theme/color_palette_controller.py index c8c56b5..43ddebd 100644 --- a/app/controllers/theme/color_palette_controller.py +++ b/app/controllers/theme/color_palette_controller.py @@ -1,6 +1,6 @@ from sqlalchemy.orm import Session from uuid import UUID -from typing import List, Optional +from typing import List from app.models.auth.user_model import User from app.models.theme.color_palette_model import ColorPalette from app.schemas.theme.color_palette_schema import ( @@ -12,9 +12,7 @@ from app.services.theme.color_palette_service import PaletteService class PaletteController: @staticmethod def get_all_palettes(db: Session, current_user: User) -> List[ColorPalette]: - - tenant_id = current_user.tenant_id - return PaletteService.get_all_palettes(db, tenant_id) + return PaletteService.get_all_palettes(db) @staticmethod def get_palette(db: Session, palette_id: UUID) -> ColorPalette: @@ -24,7 +22,7 @@ class PaletteController: def create_palette( db: Session, data: ColorPaletteCreate, current_user: User ) -> ColorPalette: - return PaletteService.create_palette(db, data, tenant_id=None) + return PaletteService.create_palette(db, data) @staticmethod def update_palette( diff --git a/app/routes/auth/__init__.py b/app/routes/auth/__init__.py index 693ec4f..c81e46b 100644 --- a/app/routes/auth/__init__.py +++ b/app/routes/auth/__init__.py @@ -1,4 +1,4 @@ -from .auth import router -from .tenant import router -from .role import router -from .user import router \ No newline at end of file +from .auth import router as auth_router +from .tenant import router as tenant_router +from .role import router as role_router +from .user import router as user_router \ No newline at end of file diff --git a/app/routes/auth/auth.py b/app/routes/auth/auth.py index a5b78c4..6259596 100644 --- a/app/routes/auth/auth.py +++ b/app/routes/auth/auth.py @@ -1,7 +1,8 @@ -from fastapi import APIRouter, Depends, Request, status, HTTPException +from fastapi import APIRouter, Depends, Request, Response, status, HTTPException from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials from sqlalchemy.orm import Session from app.config.database import get_db +from app.config.settings import settings from app.controllers.auth.auth_controller import AuthController from app.schemas.auth.auth_schema import ( UserSignup, @@ -23,6 +24,12 @@ import uuid router = APIRouter() +def _cookie_kwargs(): + kwargs = dict(httponly=True, samesite="lax", secure=settings.COOKIE_SECURE, path="/") + if settings.COOKIE_DOMAIN: + kwargs["domain"] = settings.COOKIE_DOMAIN + return kwargs + @router.post("/signup", response_model=UserResponse, status_code=status.HTTP_201_CREATED) def signup( user_data: UserSignup, @@ -32,12 +39,30 @@ def signup( return AuthController.signup(db, user_data, tenant_id) @router.post("/signin", response_model=TokenResponse) -def signin(signin_data: UserSignin, db: Session = Depends(get_db)): - return AuthController.signin(db, signin_data) +def signin(signin_data: UserSignin, response: Response, db: Session = Depends(get_db)): + result = AuthController.signin(db, signin_data) + cookie_kw = _cookie_kwargs() + access_max_age = settings.ACCESS_TOKEN_EXPIRES if signin_data.remember_me else None + refresh_max_age = settings.REFRESH_TOKEN_EXPIRES if signin_data.remember_me else None + response.set_cookie(key="access_token", value=result["access_token"], max_age=access_max_age, **cookie_kw) + response.set_cookie(key="refresh_token", value=result["refresh_token"], max_age=refresh_max_age, **cookie_kw) + return result @router.post("/refresh", response_model=TokenResponse) -def refresh_token(token_data: RefreshTokenRequest, db: Session = Depends(get_db)): - return AuthController.refresh_token(db, token_data) +def refresh_token( + token_data: RefreshTokenRequest, + request: Request, + response: Response, + db: Session = Depends(get_db), +): + refresh_tok = token_data.refresh_token or request.cookies.get("refresh_token") + if not refresh_tok: + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Missing refresh token") + result = AuthController.refresh_token_raw(db, refresh_tok) + cookie_kw = _cookie_kwargs() + response.set_cookie(key="access_token", value=result["access_token"], **cookie_kw) + response.set_cookie(key="refresh_token", value=result["refresh_token"], **cookie_kw) + return result @router.get("/me", response_model=UserResponse) def get_me( @@ -76,11 +101,16 @@ def reset_password( @router.post("/logout") def logout( request: Request, + response: Response, credentials: HTTPAuthorizationCredentials = Depends(security_scheme), current_user: User = Depends(get_current_user), ): token = credentials.credentials if credentials else request.cookies.get("access_token") - return AuthController.logout(current_user, token) + result = AuthController.logout(current_user, token) + cookie_kw = _cookie_kwargs() + response.delete_cookie(key="access_token", **cookie_kw) + response.delete_cookie(key="refresh_token", **cookie_kw) + return result @router.post("/forgot-password") def forgot_password(request: ForgotPasswordRequest, db: Session = Depends(get_db)): diff --git a/app/schemas/auth/auth_schema.py b/app/schemas/auth/auth_schema.py index 5b11624..6a9d12e 100644 --- a/app/schemas/auth/auth_schema.py +++ b/app/schemas/auth/auth_schema.py @@ -16,6 +16,7 @@ class UserSignup(UserBase): class UserSignin(BaseModel): email: EmailStr password: str + remember_me: bool = False class AccessInRole(BaseModel): id: str @@ -55,7 +56,7 @@ class TokenResponse(BaseModel): class RefreshTokenRequest(BaseModel): - refresh_token: str + refresh_token: Optional[str] = None class ResetPassword(BaseModel): old_password: str diff --git a/app/schemas/auth/module_environment_schema.py b/app/schemas/auth/module_environment_schema.py index 73f5503..ee99eeb 100644 --- a/app/schemas/auth/module_environment_schema.py +++ b/app/schemas/auth/module_environment_schema.py @@ -37,7 +37,6 @@ class EnvironmentResponse(BaseModel): frontend_base_url: str backend_base_url: str sso_entry_path: str - sso_entry_path: str permission_sync_endpoint: str sso_exchange_endpoint: Optional[str] provisioning_endpoint: Optional[str] = "/internal/tenants/provision" diff --git a/app/services/auth/trust_service.py b/app/services/auth/trust_service.py index b6fa1f4..61219f7 100644 --- a/app/services/auth/trust_service.py +++ b/app/services/auth/trust_service.py @@ -14,7 +14,7 @@ class TrustService: if environment.trust_type != "hmac": if environment.trust_type == "static_key": secret = environment.trust_credentials.get("secret_key") - return signature == secret + return hmac.compare_digest(signature, secret) return False secret = environment.trust_credentials.get("hmac_secret") @@ -101,7 +101,7 @@ class TrustService: elif environment.trust_type == "static_key": api_key = request_headers.get("X-Module-Key") secret = environment.trust_credentials.get("secret_key") - if not api_key or api_key != secret: + if not api_key or not hmac.compare_digest(api_key, secret): raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid API Key" diff --git a/app/services/auth/user_service.py b/app/services/auth/user_service.py index 4dd68be..a5624b6 100644 --- a/app/services/auth/user_service.py +++ b/app/services/auth/user_service.py @@ -13,6 +13,7 @@ import json from app.models.auth.role_module_access_model import RoleModuleAccess from app.models.auth.module_access_model import ModuleAccess from app.models.auth.tenant_module_model import TenantModule +from app.models.auth.role_model import Role logger = logging.getLogger(__name__) @@ -108,6 +109,19 @@ class UserService: if tenant_id: update_dict.pop("tenant_id", None) + if "role_id" in update_dict and update_dict["role_id"] is not None: + role = db.query(Role).filter(Role.id == update_dict["role_id"]).first() + if not role: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Role not found" + ) + if tenant_id and role.tenant_id is not None and role.tenant_id != tenant_id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Cannot assign a role from another tenant" + ) + if "email" in update_dict and update_dict["email"] != user.email: if db.query(User).filter(User.email == update_dict["email"]).first(): raise HTTPException( @@ -231,6 +245,7 @@ class UserService: if search and search.strip(): search_term = search.strip() + search_term = search_term.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") query = query.filter( or_( User.email.ilike(f"%{search_term}%"), diff --git a/app/services/theme/color_palette_service.py b/app/services/theme/color_palette_service.py index 5b88bd7..5a7daab 100644 --- a/app/services/theme/color_palette_service.py +++ b/app/services/theme/color_palette_service.py @@ -1,6 +1,6 @@ from sqlalchemy.orm import Session from uuid import UUID -from typing import List, Optional +from typing import List from fastapi import HTTPException, status from app.models.theme.color_palette_model import ColorPalette from app.schemas.theme.color_palette_schema import ( @@ -10,9 +10,7 @@ from app.schemas.theme.color_palette_schema import ( class PaletteService: @staticmethod - def get_all_palettes( - db: Session, tenant_id: Optional[UUID] = None - ) -> List[ColorPalette]: + def get_all_palettes(db: Session) -> List[ColorPalette]: return db.query(ColorPalette).all() @staticmethod @@ -23,9 +21,7 @@ class PaletteService: return palette @staticmethod - def create_palette( - db: Session, data: ColorPaletteCreate, tenant_id: Optional[UUID] = None - ) -> ColorPalette: + def create_palette(db: Session, data: ColorPaletteCreate) -> ColorPalette: if data.is_default: db.query(ColorPalette).update({"is_default": False})