0.2.46: security hardening (external-fund scope, role guards, forced password rotation, headers)
Pre-release audit fixes: - External Administrators can only act on investors already in their own funds (grant_access, create_statement, set_partner_exited, import match), closing a cross-fund account-takeover chain. - Role and is_active changes are gated: no self-escalation, non-approvers cannot mint or alter approver accounts, and the Service Admin is protected. - must_change_password is enforced server-side, not just in the client. - Session cookie is Secure with a 12h max-age and SameSite; CSP and standard security headers on every response; the app refuses to boot on a known or short session secret; uvicorn runs with proxy-headers so login throttling keys on the real client IP behind the platform proxy. - Import preview no longer confirms cross-fund investors; no-store on document downloads. - Adds backend/tests/test_security_hardening.py (15 tests); full suite 76 passing. - SECURITY-AUDIT.md records the swarm audit, findings, and remediation status.
This commit is contained in:
@@ -8,7 +8,9 @@ from fastapi import Depends, HTTPException, Request
|
||||
from sqlmodel import Session, select, col
|
||||
|
||||
from ten31portal.database import get_session
|
||||
from ten31portal.models import EntityAccess, User, UserRole, EXTERNAL_ROLES
|
||||
from ten31portal.models import (
|
||||
CapitalAccountStatement, EntityAccess, User, UserRole, EXTERNAL_ROLES,
|
||||
)
|
||||
|
||||
ph = PasswordHasher()
|
||||
|
||||
@@ -24,6 +26,16 @@ def verify_password(password: str, password_hash: str) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
# While an account is on the shared default password it may reach only these endpoints, so the
|
||||
# forced password change cannot be skipped by calling the API directly (the browser modal is not
|
||||
# a security control). Everything else is blocked until the password is rotated.
|
||||
_PASSWORD_CHANGE_ALLOWED = frozenset({
|
||||
"/api/auth/me",
|
||||
"/api/auth/change-password",
|
||||
"/api/auth/logout",
|
||||
})
|
||||
|
||||
|
||||
def get_current_user(request: Request, session: Session = Depends(get_session)) -> User:
|
||||
"""FastAPI dependency: extract user from session cookie."""
|
||||
user_id = request.session.get("user_id")
|
||||
@@ -32,6 +44,8 @@ def get_current_user(request: Request, session: Session = Depends(get_session))
|
||||
user = session.get(User, user_id)
|
||||
if user is None or not user.is_active:
|
||||
raise HTTPException(status_code=401, detail="Not authenticated")
|
||||
if user.must_change_password and request.url.path not in _PASSWORD_CHANGE_ALLOWED:
|
||||
raise HTTPException(status_code=403, detail="password_change_required")
|
||||
return user
|
||||
|
||||
|
||||
@@ -124,3 +138,38 @@ def check_administrator_scope(user: User, entity_id: int, session: Session) -> N
|
||||
"""
|
||||
if user.role in EXTERNAL_ROLES and not can_access_entity(user, entity_id, session):
|
||||
raise HTTPException(status_code=403, detail="No access to this entity")
|
||||
|
||||
|
||||
def visible_investor_ids(user: User, session: Session) -> set[int] | None:
|
||||
"""Investor ids an external account may see or manage. None means unrestricted (internal).
|
||||
|
||||
An investor is in scope when they already have an access grant or a capital-account
|
||||
statement in one of the caller's entities. This is the set an external Administrator is
|
||||
allowed to touch; it is derived from rows that predate the current request, so a write
|
||||
that would CREATE such a row for a new investor must be scope-checked BEFORE it runs.
|
||||
"""
|
||||
scope = accessible_entity_ids(user, session)
|
||||
if scope is None:
|
||||
return None
|
||||
if not scope:
|
||||
return set()
|
||||
ids = set(session.exec(
|
||||
select(EntityAccess.user_id).where(col(EntityAccess.entity_id).in_(scope))
|
||||
).all())
|
||||
ids |= set(session.exec(
|
||||
select(CapitalAccountStatement.investor_user_id).where(
|
||||
col(CapitalAccountStatement.entity_id).in_(scope)
|
||||
)
|
||||
).all())
|
||||
return ids
|
||||
|
||||
|
||||
def check_target_investor_scope(admin: User, target_user_id: int, session: Session) -> None:
|
||||
"""403 unless the caller is internal, or the target is an investor already inside the
|
||||
caller's scope. Blocks an external Administrator from pulling an out-of-scope investor
|
||||
into their funds (which would otherwise widen their scope and unlock further actions)."""
|
||||
allowed = visible_investor_ids(admin, session)
|
||||
if allowed is None:
|
||||
return
|
||||
if target_user_id not in allowed:
|
||||
raise HTTPException(status_code=403, detail="No access to this account")
|
||||
|
||||
@@ -5,6 +5,20 @@ import os
|
||||
|
||||
DB_PATH: str = os.getenv("TEN31_DB_PATH", "/data/ten31portal/portal.db")
|
||||
SESSION_SECRET: str = os.getenv("TEN31_SESSION_SECRET", "change-me-in-production")
|
||||
# Session secrets that must never reach production. The app refuses to start on any of these
|
||||
# (see main.lifespan) so a packaging slip cannot silently boot on a source-visible key that
|
||||
# would let anyone forge a session cookie.
|
||||
INSECURE_SESSION_SECRETS: frozenset[str] = frozenset({
|
||||
"", "change-me", "change-me-in-production",
|
||||
})
|
||||
# Send the session cookie only over HTTPS. Off by default so local dev and the test client
|
||||
# (both plain HTTP) work; the container turns it on via TEN31_SESSION_COOKIE_SECURE=true.
|
||||
SESSION_COOKIE_SECURE: bool = os.getenv(
|
||||
"TEN31_SESSION_COOKIE_SECURE", "false"
|
||||
).lower() in ("1", "true", "yes")
|
||||
# Session lifetime. Default 12 hours (was the Starlette 14-day default), so a captured cookie
|
||||
# ages out in a workday rather than a fortnight.
|
||||
SESSION_MAX_AGE: int = int(os.getenv("TEN31_SESSION_MAX_AGE", str(12 * 60 * 60)))
|
||||
DOCS_DIR: str = os.getenv("TEN31_DOCS_DIR", "/data/ten31portal/documents")
|
||||
# Cap on a single uploaded document. The docs dir shares the data volume with the DB, so an
|
||||
# unbounded upload could fill the disk and take the portal down. Default 50 MB. The same cap
|
||||
|
||||
@@ -4,13 +4,16 @@ import os
|
||||
from contextlib import asynccontextmanager
|
||||
from pathlib import Path
|
||||
|
||||
from fastapi import FastAPI
|
||||
from fastapi import FastAPI, Request
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
from starlette.middleware.gzip import GZipMiddleware
|
||||
from starlette.middleware.sessions import SessionMiddleware
|
||||
from starlette.responses import FileResponse
|
||||
|
||||
from ten31portal.config import SESSION_SECRET
|
||||
from ten31portal import config
|
||||
from ten31portal.config import (
|
||||
INSECURE_SESSION_SECRETS, SESSION_COOKIE_SECURE, SESSION_MAX_AGE, SESSION_SECRET,
|
||||
)
|
||||
from ten31portal.db_init import run_migrations
|
||||
from ten31portal.routers.auth_router import router as auth_router
|
||||
from ten31portal.routers.audit_router import router as audit_router
|
||||
@@ -27,14 +30,53 @@ from ten31portal.routers.capital_import_router import router as capital_import_r
|
||||
|
||||
@asynccontextmanager
|
||||
async def lifespan(app: FastAPI):
|
||||
# Refuse to boot on a publicly-known session secret: with one, anyone can forge a session
|
||||
# cookie for any user id and bypass authentication. start.sh generates a real secret; this
|
||||
# is the backstop for a misconfigured or bypassed entrypoint.
|
||||
if SESSION_SECRET in INSECURE_SESSION_SECRETS or len(SESSION_SECRET) < 32:
|
||||
raise RuntimeError(
|
||||
"TEN31_SESSION_SECRET is unset, a known default, or too short. Set it to a random "
|
||||
"value of at least 32 characters before starting the portal."
|
||||
)
|
||||
run_migrations()
|
||||
from ten31portal.storage import ensure_docs_dir
|
||||
ensure_docs_dir()
|
||||
yield
|
||||
|
||||
|
||||
# Security headers applied to every response. The SPA loads only same-origin hashed assets and
|
||||
# renders the TOTP QR as a data: image, so this CSP does not need to loosen script-src.
|
||||
_SECURITY_HEADERS = {
|
||||
"X-Content-Type-Options": "nosniff",
|
||||
"X-Frame-Options": "DENY",
|
||||
"Referrer-Policy": "no-referrer",
|
||||
"Content-Security-Policy": (
|
||||
"default-src 'self'; base-uri 'self'; object-src 'none'; frame-ancestors 'none'; "
|
||||
"img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self'; "
|
||||
"connect-src 'self'; font-src 'self' data:; form-action 'self'"
|
||||
),
|
||||
"Strict-Transport-Security": "max-age=31536000; includeSubDomains",
|
||||
}
|
||||
|
||||
|
||||
app = FastAPI(title="Ten31Portal", version="0.1.0", lifespan=lifespan)
|
||||
app.add_middleware(SessionMiddleware, secret_key=SESSION_SECRET)
|
||||
|
||||
|
||||
@app.middleware("http")
|
||||
async def add_security_headers(request: Request, call_next):
|
||||
response = await call_next(request)
|
||||
for name, value in _SECURITY_HEADERS.items():
|
||||
response.headers.setdefault(name, value)
|
||||
return response
|
||||
|
||||
|
||||
app.add_middleware(
|
||||
SessionMiddleware,
|
||||
secret_key=SESSION_SECRET,
|
||||
https_only=SESSION_COOKIE_SECURE,
|
||||
same_site="lax",
|
||||
max_age=SESSION_MAX_AGE,
|
||||
)
|
||||
# Compress HTML/JS/CSS/JSON over the wire (the 320KB JS bundle gzips to ~80KB). Added after
|
||||
# SessionMiddleware so it sits outermost and compresses the final response. minimum_size skips
|
||||
# tiny payloads (health checks, small JSON) where compression isn't worth it.
|
||||
|
||||
@@ -7,8 +7,8 @@ from sqlmodel import Session, select, col
|
||||
|
||||
from ten31portal.audit import record_audit
|
||||
from ten31portal.auth import (
|
||||
accessible_entity_ids, check_administrator_scope, get_current_user,
|
||||
household_user_ids, require_admin,
|
||||
accessible_entity_ids, check_administrator_scope, check_target_investor_scope,
|
||||
get_current_user, household_user_ids, require_admin,
|
||||
)
|
||||
from ten31portal.database import get_session
|
||||
from ten31portal.models import (
|
||||
@@ -135,6 +135,9 @@ def create_statement(
|
||||
investor = session.get(User, body.investor_user_id)
|
||||
if investor is None or investor.role != UserRole.investor:
|
||||
raise HTTPException(status_code=400, detail="investor_user_id must be an investor account")
|
||||
# An external Administrator can only write statements for an investor already in their
|
||||
# scope; otherwise this row would itself bring an out-of-scope investor into reach.
|
||||
check_target_investor_scope(admin, body.investor_user_id, session)
|
||||
|
||||
existing = session.exec(
|
||||
select(CapitalAccountStatement).where(
|
||||
|
||||
@@ -20,7 +20,10 @@ from sqlmodel import Session, select
|
||||
|
||||
from ten31portal import config, storage
|
||||
from ten31portal.audit import record_audit
|
||||
from ten31portal.auth import check_administrator_scope, hash_password, require_admin
|
||||
from ten31portal.auth import (
|
||||
check_administrator_scope, check_target_investor_scope, hash_password, require_admin,
|
||||
visible_investor_ids,
|
||||
)
|
||||
from ten31portal.database import get_session
|
||||
from ten31portal.models import (
|
||||
CapitalAccountStatement, Entity, EntityAccess, User, UserRole,
|
||||
@@ -232,6 +235,15 @@ def preview_import(
|
||||
if entity_id is not None and session.get(Entity, entity_id) is None:
|
||||
raise HTTPException(status_code=404, detail="Entity not found")
|
||||
|
||||
# An external Administrator must name one of their funds, and may only be shown matches
|
||||
# against investors already inside their scope. Otherwise the name-matching would confirm
|
||||
# which uploaded names correspond to real accounts (and their usernames) in other funds.
|
||||
scope_ids = visible_investor_ids(admin, session)
|
||||
if scope_ids is not None and entity_id is None:
|
||||
raise HTTPException(
|
||||
status_code=400, detail="Select one of your funds before previewing an import."
|
||||
)
|
||||
|
||||
try:
|
||||
file_bytes = storage.read_capped(file)
|
||||
except storage.UploadTooLarge as exc:
|
||||
@@ -239,6 +251,8 @@ def preview_import(
|
||||
wb = _open_workbook(file_bytes, password)
|
||||
|
||||
investors = session.exec(select(User).where(User.role == UserRole.investor)).all()
|
||||
if scope_ids is not None:
|
||||
investors = [u for u in investors if u.id in scope_ids]
|
||||
by_name = {u.name.strip().lower(): u for u in investors}
|
||||
by_username = {u.username.strip().lower(): u for u in investors}
|
||||
by_extid = {u.external_investor_id: u for u in investors if u.external_investor_id}
|
||||
@@ -340,6 +354,9 @@ def commit_import(
|
||||
user = session.get(User, inv.user_id) if inv.user_id else None
|
||||
if user is None or user.role != UserRole.investor:
|
||||
raise HTTPException(status_code=400, detail="match requires a valid investor user_id.")
|
||||
# An external Administrator can only match an investor already inside their scope;
|
||||
# a new investor for their fund must go through the "create" action instead.
|
||||
check_target_investor_scope(admin, user.id, session)
|
||||
if inv.external_id and not user.external_investor_id:
|
||||
user.external_investor_id = inv.external_id
|
||||
session.add(user)
|
||||
|
||||
@@ -154,6 +154,8 @@ def download_document(
|
||||
path,
|
||||
media_type=doc.content_type,
|
||||
filename=doc.original_filename,
|
||||
# K-1s and capital statements must not linger in a shared-machine browser cache.
|
||||
headers={"Cache-Control": "no-store"},
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -7,8 +7,9 @@ from sqlmodel import Session, col, select
|
||||
|
||||
from ten31portal.audit import record_audit
|
||||
from ten31portal.auth import (
|
||||
accessible_entity_ids, check_administrator_scope, get_current_user, household_user_ids,
|
||||
require_entity_writer, require_internal, require_internal_or_administrator, require_writer,
|
||||
accessible_entity_ids, check_administrator_scope, check_target_investor_scope,
|
||||
get_current_user, household_user_ids, require_entity_writer, require_internal,
|
||||
require_internal_or_administrator, require_writer,
|
||||
)
|
||||
from ten31portal.database import get_session
|
||||
from ten31portal.models import (
|
||||
@@ -178,6 +179,9 @@ def set_partner_exited(
|
||||
member = session.get(User, user_id)
|
||||
if member is None or member.role != UserRole.investor:
|
||||
raise HTTPException(status_code=400, detail="Only investor members can be marked exited")
|
||||
# Marking an exit can mint an access grant (below); an external Administrator may only do
|
||||
# that for an investor already inside their scope, never to pull in an out-of-fund account.
|
||||
check_target_investor_scope(admin, user_id, session)
|
||||
|
||||
access = session.exec(
|
||||
select(EntityAccess).where(
|
||||
|
||||
@@ -5,8 +5,9 @@ from sqlmodel import Session, select, col
|
||||
|
||||
from ten31portal.audit import record_audit
|
||||
from ten31portal.auth import (
|
||||
accessible_entity_ids, can_access_entity, check_administrator_scope, get_current_user,
|
||||
hash_password, household_user_ids, require_admin, require_internal_admin, require_role,
|
||||
accessible_entity_ids, can_access_entity, check_administrator_scope,
|
||||
check_target_investor_scope, get_current_user, hash_password, household_user_ids,
|
||||
require_admin, require_internal_admin, require_role, visible_investor_ids,
|
||||
)
|
||||
from ten31portal.database import get_session
|
||||
from ten31portal.models import (
|
||||
@@ -43,12 +44,15 @@ def _visible_investor_ids(scope: set[int], session: Session) -> set[int]:
|
||||
def _check_target_in_scope(admin: User, target: User, session: Session) -> None:
|
||||
"""403 unless the caller is internal, or the target is an investor tied to one of the
|
||||
Administrator's entities. Keeps an external Administrator away from staff accounts and
|
||||
from other funds' investors entirely."""
|
||||
scope = accessible_entity_ids(admin, session)
|
||||
if scope is None:
|
||||
from other funds' investors entirely. Delegates to the shared helper so the scope
|
||||
definition can never drift between routers."""
|
||||
if target.role != UserRole.investor:
|
||||
# An external Administrator may only ever act on investor accounts.
|
||||
scope = accessible_entity_ids(admin, session)
|
||||
if scope is not None:
|
||||
raise HTTPException(status_code=403, detail="No access to this account")
|
||||
return
|
||||
if target.role != UserRole.investor or target.id not in _visible_investor_ids(scope, session):
|
||||
raise HTTPException(status_code=403, detail="No access to this account")
|
||||
check_target_investor_scope(admin, target.id, session)
|
||||
|
||||
|
||||
def delete_user_cascade(session: Session, user: User) -> list[str]:
|
||||
@@ -106,6 +110,51 @@ def delete_user_cascade(session: Session, user: User) -> list[str]:
|
||||
return warnings
|
||||
|
||||
|
||||
def _enforce_privileged_fields(
|
||||
admin: User,
|
||||
target: User | None,
|
||||
new_role: UserRole | None,
|
||||
new_is_active: bool | None,
|
||||
session: Session,
|
||||
) -> None:
|
||||
"""Guard role/is_active against privilege escalation and admin tampering.
|
||||
|
||||
`role` and `is_active` are not ordinary profile fields: writing them changes who can do
|
||||
what. Without this, any internal admin (operations/cfo) could PATCH their own role to
|
||||
approver, mint new approver accounts, or deactivate the Service Admin. Rules:
|
||||
|
||||
- Only an approver may create or confer the `approver` role, or change an existing
|
||||
approver's role/status (approver holds final valuation sign-off — the one control
|
||||
operations is defined not to have).
|
||||
- The Service Admin's role and status are fixed via the API.
|
||||
- No account may change its own role or active status.
|
||||
|
||||
`target` is None on create. Internal non-approver admins keep managing non-approver
|
||||
accounts (investors, viewers, fund admins, and each other) unchanged.
|
||||
"""
|
||||
changing_role = new_role is not None and (target is None or new_role != target.role)
|
||||
changing_active = (
|
||||
new_is_active is not None and target is not None and new_is_active != target.is_active
|
||||
)
|
||||
if target is not None and (changing_role or changing_active):
|
||||
if target.is_service_admin:
|
||||
raise HTTPException(
|
||||
status_code=403, detail="The Service Admin's role and status are fixed."
|
||||
)
|
||||
if target.id == admin.id:
|
||||
raise HTTPException(
|
||||
status_code=403, detail="You cannot change your own role or status."
|
||||
)
|
||||
touches_approver = (new_role == UserRole.approver) or (
|
||||
target is not None and target.role == UserRole.approver and (changing_role or changing_active)
|
||||
)
|
||||
if touches_approver and admin.role != UserRole.approver:
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail="Only a Managing Partner (approver) can manage approver accounts.",
|
||||
)
|
||||
|
||||
|
||||
@router.get("/access-matrix")
|
||||
def access_matrix(
|
||||
admin: User = Depends(require_admin),
|
||||
@@ -147,6 +196,10 @@ def grant_access(
|
||||
raise HTTPException(status_code=404, detail="User not found")
|
||||
if admin.role == UserRole.fund_administrator and user.role != UserRole.investor:
|
||||
raise HTTPException(status_code=403, detail="Administrators manage investor accounts only")
|
||||
# An external Administrator may only add funds for an investor already inside their scope;
|
||||
# otherwise granting access would pull an out-of-fund investor in and widen the caller's
|
||||
# reach. Internal staff pass through. New investors are onboarded via create_user instead.
|
||||
_check_target_in_scope(admin, user, session)
|
||||
if session.get(Entity, entity_id) is None:
|
||||
raise HTTPException(status_code=404, detail="Entity not found")
|
||||
existing = session.exec(
|
||||
@@ -379,6 +432,8 @@ def create_user(
|
||||
raise HTTPException(
|
||||
status_code=400, detail="Select at least one of your funds for this investor"
|
||||
)
|
||||
# Internal callers may pick any role here; block escalation to approver by non-approvers.
|
||||
_enforce_privileged_fields(admin, None, body.role, None, session)
|
||||
if session.exec(select(User).where(User.username == body.username)).first():
|
||||
raise HTTPException(status_code=409, detail="Username already taken")
|
||||
if body.email and session.exec(select(User).where(User.email == body.email)).first():
|
||||
@@ -413,6 +468,9 @@ def update_user(
|
||||
if user is None:
|
||||
raise HTTPException(status_code=404, detail="User not found")
|
||||
_check_target_in_scope(admin, user, session)
|
||||
# role/is_active are privileged: block self-escalation, approver tampering, and any change
|
||||
# to the Service Admin before applying the field writes below.
|
||||
_enforce_privileged_fields(admin, user, body.role, body.is_active, session)
|
||||
|
||||
changes = body.model_dump(exclude_unset=True)
|
||||
entity_ids = changes.pop("entity_ids", None)
|
||||
|
||||
Reference in New Issue
Block a user