From b97af0ea1eb13c1be5f663fb736254866b7d6c76 Mon Sep 17 00:00:00 2001 From: A R R R Associates Date: Thu, 25 Jun 2026 20:33:15 +0530 Subject: [PATCH] Phase 1 fix auth redirect API handling and route issues --- app/core/http_responses.py | 45 ++++++++++++++++ app/core/security/session_auth.py | 4 +- app/main.py | 29 +++++++++- app/modules/billing/ui.py | 14 ++--- app/modules/clients/ui.py | 3 +- app/modules/consultants/ui.py | 3 +- app/modules/core/iam/ui.py | 3 +- app/modules/core/rbac/ui.py | 3 +- app/modules/core/tenancy/api.py | 53 +++++++++++++++++-- app/modules/documents/ui.py | 27 +++++++--- app/modules/employees/ui.py | 5 +- app/modules/managers/ui.py | 3 +- app/modules/marketplace/ui.py | 3 +- app/modules/notice_cases/ui.py | 3 +- app/modules/partners/ui.py | 3 +- app/modules/platform_billing/ui.py | 3 +- .../services/client_services_ui_old.py | 3 +- app/modules/services/engagements_ui.py | 3 +- app/modules/services/execution_ui_old.py | 3 +- app/modules/services/ui.py | 3 +- app/modules/services/work_tracker_ui.py | 3 +- app/modules/system_settings/ui.py | 3 +- 22 files changed, 185 insertions(+), 37 deletions(-) create mode 100644 app/core/http_responses.py diff --git a/app/core/http_responses.py b/app/core/http_responses.py new file mode 100644 index 0000000..3c3699c --- /dev/null +++ b/app/core/http_responses.py @@ -0,0 +1,45 @@ +from __future__ import annotations + +from fastapi import Request +from fastapi.responses import JSONResponse, RedirectResponse, Response + + +def wants_json(request: Request) -> bool: + path = request.url.path or "" + accept = (request.headers.get("accept") or "").lower() + requested_with = (request.headers.get("x-requested-with") or "").lower() + return path.startswith("/api") or "application/json" in accept or requested_with == "xmlhttprequest" + + +def ui_access_denied(message: str = "Access denied") -> Response: + return Response( + content=f"403 Forbidden: {message}", + status_code=403, + media_type="text/plain; charset=utf-8", + ) + + +def ui_not_found(message: str = "Not found") -> Response: + return Response( + content=f"404 Not Found: {message}", + status_code=404, + media_type="text/plain; charset=utf-8", + ) + + +def auth_required_response(request: Request): + if wants_json(request): + return JSONResponse({"detail": "Not authenticated"}, status_code=401) + return RedirectResponse(url="/login", status_code=303) + + +def forbidden_response(request: Request, message: str = "Access denied"): + if wants_json(request): + return JSONResponse({"detail": message}, status_code=403) + return ui_access_denied(message) + + +def not_found_response(request: Request, message: str = "Not found"): + if wants_json(request): + return JSONResponse({"detail": message}, status_code=404) + return ui_not_found(message) diff --git a/app/core/security/session_auth.py b/app/core/security/session_auth.py index 0b943e7..d616ed7 100644 --- a/app/core/security/session_auth.py +++ b/app/core/security/session_auth.py @@ -1,6 +1,6 @@ from __future__ import annotations from datetime import datetime, timedelta, timezone -from fastapi import Request, Depends +from fastapi import Request, Depends, HTTPException from sqlalchemy.orm import Session from sqlalchemy import select @@ -48,5 +48,5 @@ def get_current_user(request: Request, db: Session = Depends(get_common_db)) -> def require_login(user: User | None = Depends(get_current_user)) -> User: if not user: - raise PermissionError("Not authenticated") + raise HTTPException(status_code=401, detail="Not authenticated") return user diff --git a/app/main.py b/app/main.py index bd3a59a..c6085c5 100644 --- a/app/main.py +++ b/app/main.py @@ -1,4 +1,5 @@ -from fastapi import FastAPI +from fastapi import FastAPI, HTTPException, Request +from fastapi.responses import JSONResponse from starlette.middleware.sessions import SessionMiddleware from app.core.settings import get_settings @@ -7,9 +8,34 @@ from app.core.middleware.domain_resolver import DomainResolverMiddleware from app.core.middleware.security_headers import SecurityHeadersMiddleware from app.core.startup import on_startup from app.core.api import api_router +from app.core.http_responses import auth_required_response, forbidden_response, not_found_response, wants_json from app.ui.app import mount_ui +def _register_error_handlers(app: FastAPI) -> None: + @app.exception_handler(PermissionError) + async def permission_error_handler(request: Request, exc: PermissionError): + message = str(exc) or "Access denied" + if "csrf" in message.lower(): + return JSONResponse({"detail": "CSRF validation failed"}, status_code=403) if wants_json(request) else forbidden_response(request, "CSRF validation failed") + if "not authenticated" in message.lower(): + return auth_required_response(request) + return forbidden_response(request, message) + + @app.exception_handler(HTTPException) + async def http_exception_handler(request: Request, exc: HTTPException): + detail = exc.detail if isinstance(exc.detail, str) else "Error" + if request.url.path.startswith("/api"): + return JSONResponse({"detail": exc.detail}, status_code=exc.status_code, headers=exc.headers) + if exc.status_code == 401: + return auth_required_response(request) + if exc.status_code == 403: + return forbidden_response(request, detail or "Access denied") + if exc.status_code == 404: + return not_found_response(request, detail or "Not found") + return JSONResponse({"detail": exc.detail}, status_code=exc.status_code, headers=exc.headers) + + def create_app() -> FastAPI: s = get_settings() app = FastAPI(title=s.APP_NAME, debug=s.DEBUG) @@ -30,6 +56,7 @@ def create_app() -> FastAPI: ) app.add_event_handler("startup", lambda: on_startup(app)) + _register_error_handlers(app) app.include_router(api_router, prefix="/api") mount_ui(app) diff --git a/app/modules/billing/ui.py b/app/modules/billing/ui.py index 059ff29..631c05f 100644 --- a/app/modules/billing/ui.py +++ b/app/modules/billing/ui.py @@ -68,7 +68,8 @@ def _render(request: Request, template: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: @@ -195,7 +196,7 @@ def invoice_list(request: Request, q: str = ""): user, title="Billing - Invoices", q=q, - active_financial_year=financial_year, + active_financial_year=_active_financial_year(request), rows=rows, report_summary=report_summary, can_create=_has_perm(db, user, "billing.create"), @@ -378,6 +379,7 @@ def billing_settings_submit( db.close() +@router.get("/invoices/new") @router.get("/new") def invoice_create_page(request: Request): db = CommonSessionLocal() @@ -397,7 +399,7 @@ def invoice_create_page(request: Request): db, user, title="Create Invoice", - active_financial_year=financial_year, + active_financial_year=_active_financial_year(request), default_billing_period_from=_period_start_for_fy(_active_financial_year(request)).isoformat(), default_billing_period_to=_period_end_for_fy(_active_financial_year(request)).isoformat(), clients=clients, @@ -512,7 +514,7 @@ def fee_structure_list(request: Request, q: str = ""): user, title="Fee Structure", q=q, - active_financial_year=financial_year, + active_financial_year=_active_financial_year(request), rows=rows, can_import=_has_perm(db, user, "billing_fee_structure.import"), ) @@ -627,7 +629,7 @@ def generate_invoices_page( billing_period_to=period_to.isoformat(), auto_generate_only=auto_generate_only, q=q, - active_financial_year=financial_year, + active_financial_year=_active_financial_year(request), result=None, ) finally: @@ -706,7 +708,7 @@ def generate_invoices_submit( billing_period_to=(period_to or date.today()).isoformat(), auto_generate_only=auto_generate_only, q=q, - active_financial_year=financial_year, + active_financial_year=_active_financial_year(request), result=result, ) except ValueError as exc: diff --git a/app/modules/clients/ui.py b/app/modules/clients/ui.py index c440fe7..7109de4 100644 --- a/app/modules/clients/ui.py +++ b/app/modules/clients/ui.py @@ -87,7 +87,8 @@ def _render(request, template, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm_factory(db, user): diff --git a/app/modules/consultants/ui.py b/app/modules/consultants/ui.py index 32d6a98..5d0f107 100644 --- a/app/modules/consultants/ui.py +++ b/app/modules/consultants/ui.py @@ -103,7 +103,8 @@ def _render(request: Request, template_name: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: diff --git a/app/modules/core/iam/ui.py b/app/modules/core/iam/ui.py index 01ca8f2..2973d31 100644 --- a/app/modules/core/iam/ui.py +++ b/app/modules/core/iam/ui.py @@ -61,7 +61,8 @@ def _redirect_login(): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _flash_redirect(url: str) -> RedirectResponse: diff --git a/app/modules/core/rbac/ui.py b/app/modules/core/rbac/ui.py index 17b8ea3..88e62d8 100644 --- a/app/modules/core/rbac/ui.py +++ b/app/modules/core/rbac/ui.py @@ -22,7 +22,8 @@ def _redirect_login(): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _is_system_admin(db, current_user) -> bool: diff --git a/app/modules/core/tenancy/api.py b/app/modules/core/tenancy/api.py index 1b01e21..dbd6c9f 100644 --- a/app/modules/core/tenancy/api.py +++ b/app/modules/core/tenancy/api.py @@ -1,15 +1,58 @@ -from fastapi import APIRouter, Depends +from __future__ import annotations + +from fastapi import APIRouter, Depends, HTTPException from sqlalchemy.orm import Session from sqlalchemy import select + from app.core.db.deps import get_common_db +from app.core.security.session_auth import require_login +from app.modules.core.iam.models import User +from app.modules.core.rbac.deps import get_user_roles from app.modules.core.tenancy.models import Tenant, Branch router = APIRouter(prefix="/tenancy", tags=["tenancy"]) + +def _require_tenancy_api_access(db: Session, user: User) -> None: + roles = set(get_user_roles(db, int(user.id))) + if not roles.intersection({"System Admin", "Firm Admin"}): + raise HTTPException(status_code=403, detail="Tenancy API access denied") + + +def _tenant_payload(t: Tenant) -> dict: + return { + "id": t.id, + "code": getattr(t, "code", None), + "name": getattr(t, "name", None), + "is_active": getattr(t, "is_active", None), + } + + +def _branch_payload(b: Branch) -> dict: + return { + "id": b.id, + "tenant_id": getattr(b, "tenant_id", None), + "code": getattr(b, "code", None), + "name": getattr(b, "name", None), + "is_active": getattr(b, "is_active", None), + } + + @router.get("/tenants") -def list_tenants(db: Session = Depends(get_common_db)): - return db.execute(select(Tenant).order_by(Tenant.id)).scalars().all() +def list_tenants(db: Session = Depends(get_common_db), current_user: User = Depends(require_login)): + _require_tenancy_api_access(db, current_user) + stmt = select(Tenant).order_by(Tenant.id) + roles = set(get_user_roles(db, int(current_user.id))) + if "System Admin" not in roles: + stmt = stmt.where(Tenant.id == current_user.tenant_id) + return [_tenant_payload(t) for t in db.execute(stmt).scalars().all()] + @router.get("/branches") -def list_branches(db: Session = Depends(get_common_db)): - return db.execute(select(Branch).order_by(Branch.id)).scalars().all() +def list_branches(db: Session = Depends(get_common_db), current_user: User = Depends(require_login)): + _require_tenancy_api_access(db, current_user) + stmt = select(Branch).order_by(Branch.id) + roles = set(get_user_roles(db, int(current_user.id))) + if "System Admin" not in roles: + stmt = stmt.where(Branch.tenant_id == current_user.tenant_id) + return [_branch_payload(b) for b in db.execute(stmt).scalars().all()] diff --git a/app/modules/documents/ui.py b/app/modules/documents/ui.py index fd54e8c..588d35b 100644 --- a/app/modules/documents/ui.py +++ b/app/modules/documents/ui.py @@ -106,7 +106,8 @@ def _render(request: Request, template: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _require_user(request: Request, db, permission_code: str): @@ -382,10 +383,13 @@ def download_latest_document(request: Request, document_id: int): return response scope = build_document_scope(request, db, user) document = get_document(db, document_id) + if not document: + from app.core.http_responses import not_found_response + return not_found_response(request, "Document not found") active_fy = _active_financial_year(request) if active_fy and document and getattr(document, "financial_year", None) != active_fy: return RedirectResponse(url=f"/documents?financial_year={active_fy}&error=wrong_year", status_code=303) - if not document or not document.engagement or not user_can_view_engagement(db, user, document.engagement, scope): + if not document.engagement or not user_can_view_engagement(db, user, document.engagement, scope): return _redirect_denied() version = get_latest_version(document) if not version: @@ -404,11 +408,14 @@ def download_document_version(request: Request, version_id: int): return response scope = build_document_scope(request, db, user) version = get_version(db, version_id) - document = version.document if version else None + if not version: + from app.core.http_responses import not_found_response + return not_found_response(request, "Document version not found") + document = version.document active_fy = _active_financial_year(request) if active_fy and document and getattr(document, "financial_year", None) != active_fy: return RedirectResponse(url=f"/documents?financial_year={active_fy}&error=wrong_year", status_code=303) - if not version or not document or not document.engagement or not user_can_view_engagement(db, user, document.engagement, scope): + if not document or not document.engagement or not user_can_view_engagement(db, user, document.engagement, scope): return _redirect_denied() return _download_or_queue_from_local_node(request, db, user, document, version, action="download_version") finally: @@ -578,7 +585,10 @@ def download_latest_permanent_document(request: Request, document_id: int): return response scope = build_document_scope(request, db, user) document = get_permanent_document(db, document_id) - if not document or not document.client or not user_can_view_client_documents(db, user, document.client, scope): + if not document: + from app.core.http_responses import not_found_response + return not_found_response(request, "Permanent document not found") + if not document.client or not user_can_view_client_documents(db, user, document.client, scope): return _redirect_denied() version = get_latest_permanent_version(document) if not version: @@ -597,8 +607,11 @@ def download_permanent_version(request: Request, version_id: int): return response scope = build_document_scope(request, db, user) version = get_permanent_version(db, version_id) - document = version.document if version else None - if not version or not document or not document.client or not user_can_view_client_documents(db, user, document.client, scope): + if not version: + from app.core.http_responses import not_found_response + return not_found_response(request, "Permanent document version not found") + document = version.document + if not document or not document.client or not user_can_view_client_documents(db, user, document.client, scope): return _redirect_denied() return _download_or_queue_permanent_from_local_node(request, db, user, document, version) finally: diff --git a/app/modules/employees/ui.py b/app/modules/employees/ui.py index 7e12d82..8402112 100644 --- a/app/modules/employees/ui.py +++ b/app/modules/employees/ui.py @@ -143,7 +143,8 @@ def _redirect_login(): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _base_ctx(request: Request, db, current_user, **ctx): @@ -687,6 +688,7 @@ def employee_leave_balance_adjust(request: Request, employee_id: int = Form(...) db.close() +@router.get("/leaves") @router.get("/leave") def employee_leave_requests(request: Request, employee_id: int | None = None, status: str = "pending"): db = CommonSessionLocal() @@ -2379,6 +2381,7 @@ def employee_punch_out_submit( db.close() +@portal_router.get("/leaves") @portal_router.get("/leave") def employee_self_leave(request: Request): db = CommonSessionLocal() diff --git a/app/modules/managers/ui.py b/app/modules/managers/ui.py index 0ad7e26..2ca6772 100644 --- a/app/modules/managers/ui.py +++ b/app/modules/managers/ui.py @@ -30,7 +30,8 @@ def _redirect_login(): def _redirect_denied(): - return RedirectResponse(url="/employee/dashboard", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _base_ctx(request: Request, db: Session, current_user, **ctx): diff --git a/app/modules/marketplace/ui.py b/app/modules/marketplace/ui.py index c4c6614..d3feecf 100644 --- a/app/modules/marketplace/ui.py +++ b/app/modules/marketplace/ui.py @@ -161,7 +161,8 @@ def marketplace_public_request_service_submit(request: Request, csrf_token: str def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: diff --git a/app/modules/notice_cases/ui.py b/app/modules/notice_cases/ui.py index 7db5bc1..86ae645 100644 --- a/app/modules/notice_cases/ui.py +++ b/app/modules/notice_cases/ui.py @@ -71,7 +71,8 @@ def _render(request: Request, template_name: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _require_user(request: Request, db, permission: str): diff --git a/app/modules/partners/ui.py b/app/modules/partners/ui.py index 944e64c..0971be5 100644 --- a/app/modules/partners/ui.py +++ b/app/modules/partners/ui.py @@ -38,7 +38,8 @@ def _redirect_login(): def _redirect_denied(): - return RedirectResponse(url="/employee/dashboard", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _is_partner_user(db: Session, current_user) -> bool: diff --git a/app/modules/platform_billing/ui.py b/app/modules/platform_billing/ui.py index 2532709..dce251a 100644 --- a/app/modules/platform_billing/ui.py +++ b/app/modules/platform_billing/ui.py @@ -49,7 +49,8 @@ router = APIRouter(prefix="/platform-billing", tags=["platform-billing-ui"]) def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: diff --git a/app/modules/services/client_services_ui_old.py b/app/modules/services/client_services_ui_old.py index 5cf1613..e9d2cb7 100644 --- a/app/modules/services/client_services_ui_old.py +++ b/app/modules/services/client_services_ui_old.py @@ -43,7 +43,8 @@ def _render(request: Request, template: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: diff --git a/app/modules/services/engagements_ui.py b/app/modules/services/engagements_ui.py index 5dd6c53..d6bdec4 100644 --- a/app/modules/services/engagements_ui.py +++ b/app/modules/services/engagements_ui.py @@ -52,7 +52,8 @@ def _render(request: Request, template: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: diff --git a/app/modules/services/execution_ui_old.py b/app/modules/services/execution_ui_old.py index db47983..98dc3e3 100644 --- a/app/modules/services/execution_ui_old.py +++ b/app/modules/services/execution_ui_old.py @@ -44,7 +44,8 @@ def _render(request: Request, template: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: diff --git a/app/modules/services/ui.py b/app/modules/services/ui.py index a355a90..a5b4d7d 100644 --- a/app/modules/services/ui.py +++ b/app/modules/services/ui.py @@ -83,7 +83,8 @@ def _render(request: Request, template: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: diff --git a/app/modules/services/work_tracker_ui.py b/app/modules/services/work_tracker_ui.py index 92392c2..e095509 100644 --- a/app/modules/services/work_tracker_ui.py +++ b/app/modules/services/work_tracker_ui.py @@ -55,7 +55,8 @@ def _render(request: Request, template: str, db, user, **ctx): def _redirect_denied(): - return RedirectResponse(url="/system-settings", status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _has_perm(db, user, code: str) -> bool: diff --git a/app/modules/system_settings/ui.py b/app/modules/system_settings/ui.py index b39415a..2520c23 100644 --- a/app/modules/system_settings/ui.py +++ b/app/modules/system_settings/ui.py @@ -38,7 +38,8 @@ def _base_ctx(request: Request, user, db, **ctx): def _redirect_denied(default_url: str = "/system-settings"): - return RedirectResponse(url=default_url, status_code=303) + from app.core.http_responses import ui_access_denied + return ui_access_denied() def _render_with_user(request: Request, template: str, ctx: dict, status_code: int = 200):