0.2.26: security hardening from audit (P2/P3 fixes)
Address the security-auditor findings the user selected (items 2, 3, 4): Default admin credentials (P2): remove the fixed `Ten31` default. First boot now generates a strong random admin password (secrets.token_urlsafe), records it 0600 at /data/.admin-password, and surfaces it once via a new "Show Initial Admin Password" StartOS action (CLI `show-admin-password`). The stored password is cleared when the admin is reset (CLI reset-password) or self-changes it (change-password endpoint). Login hardening (P2): add a per-IP in-memory sliding-window rate limiter (10 failures / 5 min -> 429 + Retry-After) in ratelimit.py; run a dummy argon2 verify when the user is unknown so timing can't enumerate usernames; keep a single generic 401 for unknown-user and wrong-password. Hardening (P3): server process now runs unprivileged -- Dockerfile adds uid 10001 appuser; start.sh (still root) chowns the mounted /data then drops via `setpriv` before exec'ing uvicorn. Spreadsheet imports are size-capped via storage.read_capped (413 past MAX_UPLOAD_SIZE) in the schedule, capital preview, and batch paths. batch_import no longer returns raw exception text (generic per-file messages). Verified in the packed amd64 container: PID1 uvicorn runs as uid 10001, /data owned 10001 with 0600 secrets; generated admin password retrievable via CLI and logs in (200); 11th bad login -> 429; admin reset clears the stored password. Tests: test_auth_hardening.py (4). Full suite 21 passed; frontend tsc + StartOS bundle clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Jonathan Kirkwood
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
025aff4fac
commit
69f12b0519
@@ -1,10 +1,12 @@
|
||||
"""CLI commands for Ten31Portal (also driven by the StartOS service Actions)."""
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import sys
|
||||
|
||||
from sqlmodel import Session, select
|
||||
|
||||
from ten31portal import config
|
||||
from ten31portal.auth import hash_password
|
||||
from ten31portal.database import engine
|
||||
from ten31portal.db_init import run_migrations
|
||||
@@ -56,6 +58,14 @@ def create_user(args: argparse.Namespace) -> None:
|
||||
print(f"Created user: {user.name} ({user.username}) with role {user.role.value}")
|
||||
|
||||
|
||||
def _clear_admin_password_file() -> None:
|
||||
"""Remove the recorded initial admin password (it is no longer valid once changed)."""
|
||||
try:
|
||||
os.remove(config.ADMIN_PASSWORD_FILE)
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
|
||||
def reset_password(args: argparse.Namespace) -> None:
|
||||
"""Reset a user's password (and re-enable their login)."""
|
||||
run_migrations()
|
||||
@@ -68,11 +78,29 @@ def reset_password(args: argparse.Namespace) -> None:
|
||||
sys.exit(1)
|
||||
user.password_hash = hash_password(args.password)
|
||||
user.login_enabled = True
|
||||
is_admin = user.is_service_admin
|
||||
session.add(user)
|
||||
session.commit()
|
||||
# Resetting the built-in admin invalidates the initial password recorded on first boot.
|
||||
if is_admin:
|
||||
_clear_admin_password_file()
|
||||
print(f"Password reset for {user.name} ({user.username}).")
|
||||
|
||||
|
||||
def show_admin_password(args: argparse.Namespace) -> None:
|
||||
"""Print the randomly-generated initial admin password recorded on first boot."""
|
||||
path = config.ADMIN_PASSWORD_FILE
|
||||
if not os.path.exists(path):
|
||||
print(
|
||||
"No stored initial password. It was either set explicitly at install time, or the "
|
||||
"admin password has already been changed. Use Reset Password to set a new one."
|
||||
)
|
||||
return
|
||||
with open(path) as f:
|
||||
pw = f.read().strip()
|
||||
print(pw if pw else "(the recorded initial password is empty)")
|
||||
|
||||
|
||||
def list_users(args: argparse.Namespace) -> None:
|
||||
"""Print all user accounts."""
|
||||
run_migrations()
|
||||
@@ -240,6 +268,8 @@ def main() -> None:
|
||||
|
||||
sub.add_parser("list-users", help="List all user accounts")
|
||||
|
||||
sub.add_parser("show-admin-password", help="Show the initial admin password from first boot")
|
||||
|
||||
delete = sub.add_parser("delete-user", help="Delete a user (not the Service Admin)")
|
||||
delete.add_argument("--username", required=False, default=None)
|
||||
delete.add_argument("--email", required=False, default=None)
|
||||
@@ -261,6 +291,8 @@ def main() -> None:
|
||||
reset_password(args)
|
||||
elif args.command == "list-users":
|
||||
list_users(args)
|
||||
elif args.command == "show-admin-password":
|
||||
show_admin_password(args)
|
||||
elif args.command == "delete-user":
|
||||
delete_user(args)
|
||||
elif args.command == "dedupe-holdings":
|
||||
|
||||
@@ -7,5 +7,11 @@ DB_PATH: str = os.getenv("TEN31_DB_PATH", "/data/ten31portal/portal.db")
|
||||
SESSION_SECRET: str = os.getenv("TEN31_SESSION_SECRET", "change-me-in-production")
|
||||
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.
|
||||
# unbounded upload could fill the disk and take the portal down. Default 50 MB. The same cap
|
||||
# also bounds spreadsheet imports (which must be read fully into memory to parse).
|
||||
MAX_UPLOAD_SIZE: int = int(os.getenv("TEN31_MAX_UPLOAD_SIZE", str(50 * 1024 * 1024)))
|
||||
|
||||
# Where start.sh records the randomly-generated initial admin password on first boot, so the
|
||||
# operator can retrieve it once (via the "Show Initial Admin Password" service action) and then
|
||||
# change it. Lives next to the DB on the 0600 data volume; removed once the password is reset.
|
||||
ADMIN_PASSWORD_FILE: str = os.path.join(os.path.dirname(DB_PATH) or ".", ".admin-password")
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
"""A tiny in-process sliding-window rate limiter for the login endpoint.
|
||||
|
||||
The portal runs as a single uvicorn process, so an in-memory counter is enough to blunt
|
||||
online password guessing without adding a dependency or a shared store. It is keyed by client
|
||||
IP; only failed attempts are counted, and a successful login clears the key. This deliberately
|
||||
does NOT lock accounts (which would let anyone lock out a user by name) — it throttles the
|
||||
source of the guessing instead.
|
||||
"""
|
||||
|
||||
import time
|
||||
from collections import defaultdict, deque
|
||||
from threading import Lock
|
||||
|
||||
|
||||
class SlidingWindowLimiter:
|
||||
def __init__(self, max_attempts: int, window_seconds: float):
|
||||
self.max_attempts = max_attempts
|
||||
self.window = window_seconds
|
||||
self._hits: dict[str, deque] = defaultdict(deque)
|
||||
self._lock = Lock()
|
||||
|
||||
def _prune(self, key: str, now: float) -> deque:
|
||||
dq = self._hits[key]
|
||||
cutoff = now - self.window
|
||||
while dq and dq[0] <= cutoff:
|
||||
dq.popleft()
|
||||
if not dq:
|
||||
self._hits.pop(key, None)
|
||||
return dq
|
||||
|
||||
def retry_after(self, key: str) -> float:
|
||||
"""Seconds until `key` may try again, or 0.0 if it is under the limit right now."""
|
||||
now = time.monotonic()
|
||||
with self._lock:
|
||||
dq = self._prune(key, now)
|
||||
if len(dq) < self.max_attempts:
|
||||
return 0.0
|
||||
return self.window - (now - dq[0])
|
||||
|
||||
def record_failure(self, key: str) -> None:
|
||||
now = time.monotonic()
|
||||
with self._lock:
|
||||
self._hits[key].append(now)
|
||||
|
||||
def reset(self, key: str) -> None:
|
||||
with self._lock:
|
||||
self._hits.pop(key, None)
|
||||
@@ -1,15 +1,27 @@
|
||||
"""Authentication endpoints."""
|
||||
|
||||
import os
|
||||
import secrets
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
||||
from sqlmodel import Session, select
|
||||
|
||||
from ten31portal import config
|
||||
from ten31portal.auth import get_current_user, hash_password, verify_password
|
||||
from ten31portal.database import get_session
|
||||
from ten31portal.models import User
|
||||
from ten31portal.ratelimit import SlidingWindowLimiter
|
||||
from ten31portal.schemas import ChangePasswordRequest, LoginRequest, UserResponse
|
||||
|
||||
router = APIRouter(prefix="/api/auth", tags=["auth"])
|
||||
|
||||
# Throttle password guessing per client IP: at most 10 failed attempts per 5 minutes.
|
||||
_login_limiter = SlidingWindowLimiter(max_attempts=10, window_seconds=300)
|
||||
|
||||
# A throwaway hash verified when no user matches, so a missing account costs the same argon2
|
||||
# time as a real one — otherwise the timing difference reveals which usernames exist.
|
||||
_DUMMY_HASH = hash_password(secrets.token_urlsafe(16))
|
||||
|
||||
|
||||
@router.post("/login")
|
||||
def login(
|
||||
@@ -17,13 +29,30 @@ def login(
|
||||
request: Request,
|
||||
session: Session = Depends(get_session),
|
||||
) -> UserResponse:
|
||||
client_ip = request.client.host if request.client else "unknown"
|
||||
retry_after = _login_limiter.retry_after(client_ip)
|
||||
if retry_after > 0:
|
||||
raise HTTPException(
|
||||
status_code=429,
|
||||
detail="Too many login attempts. Please wait a moment and try again.",
|
||||
headers={"Retry-After": str(int(retry_after) + 1)},
|
||||
)
|
||||
|
||||
# Accept either a username or an email in the login field.
|
||||
handle = body.login.strip()
|
||||
user = session.exec(select(User).where(User.username == handle)).first()
|
||||
if user is None:
|
||||
user = session.exec(select(User).where(User.email == handle)).first()
|
||||
if user is None or not verify_password(body.password, user.password_hash):
|
||||
# Always run a verify (dummy hash when the user is unknown) so success/failure take the
|
||||
# same time, and give one generic message so neither branch leaks whether the user exists.
|
||||
if user is None:
|
||||
verify_password(body.password, _DUMMY_HASH)
|
||||
_login_limiter.record_failure(client_ip)
|
||||
raise HTTPException(status_code=401, detail="Invalid username or password")
|
||||
if not verify_password(body.password, user.password_hash):
|
||||
_login_limiter.record_failure(client_ip)
|
||||
raise HTTPException(status_code=401, detail="Invalid username or password")
|
||||
# Past this point the password was correct, so these messages don't aid guessing.
|
||||
if user.primary_account_id is not None:
|
||||
raise HTTPException(
|
||||
status_code=401,
|
||||
@@ -33,6 +62,7 @@ def login(
|
||||
raise HTTPException(status_code=401, detail="This account does not have a login yet.")
|
||||
if not user.is_active:
|
||||
raise HTTPException(status_code=401, detail="Account disabled")
|
||||
_login_limiter.reset(client_ip)
|
||||
request.session["user_id"] = user.id
|
||||
return UserResponse.model_validate(user, from_attributes=True)
|
||||
|
||||
@@ -62,4 +92,10 @@ def change_password(
|
||||
user.password_hash = hash_password(body.new_password)
|
||||
session.add(user)
|
||||
session.commit()
|
||||
# Once the built-in admin sets their own password, the initial one from first boot is stale.
|
||||
if user.is_service_admin:
|
||||
try:
|
||||
os.remove(config.ADMIN_PASSWORD_FILE)
|
||||
except OSError:
|
||||
pass
|
||||
return {"status": "ok"}
|
||||
|
||||
@@ -19,6 +19,7 @@ import openpyxl
|
||||
from fastapi import APIRouter, Depends, File, Form, HTTPException, UploadFile
|
||||
from sqlmodel import Session, select
|
||||
|
||||
from ten31portal import storage
|
||||
from ten31portal.audit import record_audit
|
||||
from ten31portal.auth import hash_password, require_internal_admin
|
||||
from ten31portal.database import get_session
|
||||
@@ -195,7 +196,11 @@ 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")
|
||||
|
||||
wb = _open_workbook(file.file.read(), password)
|
||||
try:
|
||||
file_bytes = storage.read_capped(file)
|
||||
except storage.UploadTooLarge as exc:
|
||||
raise HTTPException(status_code=413, detail=str(exc))
|
||||
wb = _open_workbook(file_bytes, password)
|
||||
|
||||
investors = session.exec(select(User).where(User.role == UserRole.investor)).all()
|
||||
by_name = {u.name.strip().lower(): u for u in investors}
|
||||
@@ -388,7 +393,7 @@ def batch_import(
|
||||
fname = upload.filename or "(unnamed)"
|
||||
res = BatchCapitalFileResult(filename=fname)
|
||||
try:
|
||||
wb = _open_workbook(upload.file.read(), password)
|
||||
wb = _open_workbook(storage.read_capped(upload), password)
|
||||
if "ALLOC SI" not in wb.sheetnames:
|
||||
raise HTTPException(status_code=422, detail="No ALLOC SI tab found in this workbook.")
|
||||
as_of, roster = _parse_alloc_si(wb)
|
||||
@@ -452,11 +457,15 @@ def batch_import(
|
||||
session.commit()
|
||||
total_statements += res.statements_written
|
||||
except HTTPException as e:
|
||||
# Our own controlled messages (bad password, no ALLOC SI, no date) are safe to show.
|
||||
session.rollback()
|
||||
res.error = e.detail
|
||||
except Exception as e: # noqa: BLE001 — surface any parse failure per-file, keep going
|
||||
except storage.UploadTooLarge:
|
||||
session.rollback()
|
||||
res.error = str(e) or "Could not process this file."
|
||||
res.error = "File is too large."
|
||||
except Exception: # noqa: BLE001 — keep going on any parse failure; don't leak internals
|
||||
session.rollback()
|
||||
res.error = "Could not process this file (unexpected format or error)."
|
||||
results.append(res)
|
||||
|
||||
return BatchCapitalImportResult(
|
||||
|
||||
@@ -16,6 +16,7 @@ import openpyxl
|
||||
from fastapi import APIRouter, Depends, File, Form, HTTPException, Query, UploadFile
|
||||
from sqlmodel import Session, select, col
|
||||
|
||||
from ten31portal import storage
|
||||
from ten31portal.audit import record_audit
|
||||
from ten31portal.auth import require_role
|
||||
from ten31portal.database import get_session
|
||||
@@ -342,7 +343,10 @@ def import_schedule(
|
||||
user: User = Depends(require_role(UserRole.approver, UserRole.cfo, UserRole.operations)),
|
||||
session: Session = Depends(get_session),
|
||||
) -> dict[str, Any]:
|
||||
file_bytes = file.file.read()
|
||||
try:
|
||||
file_bytes = storage.read_capped(file)
|
||||
except storage.UploadTooLarge as exc:
|
||||
raise HTTPException(status_code=413, detail=str(exc))
|
||||
filename = file.filename or ""
|
||||
|
||||
# Parse the file into holdings/positions previews.
|
||||
|
||||
@@ -14,6 +14,23 @@ class UploadTooLarge(Exception):
|
||||
"""Raised when an upload exceeds MAX_UPLOAD_SIZE. The partial file is removed first."""
|
||||
|
||||
|
||||
def read_capped(file: UploadFile, limit: int = MAX_UPLOAD_SIZE) -> bytes:
|
||||
"""Read an upload fully into memory, but abort past `limit` instead of reading unbounded.
|
||||
|
||||
The spreadsheet importers must parse the whole workbook in memory; without a cap an
|
||||
authenticated writer could POST a huge file and exhaust the process. Reads in 1 MB chunks
|
||||
and raises UploadTooLarge (→ 413) once the total would exceed the limit.
|
||||
"""
|
||||
chunks: list[bytes] = []
|
||||
size = 0
|
||||
while chunk := file.file.read(1024 * 1024):
|
||||
size += len(chunk)
|
||||
if size > limit:
|
||||
raise UploadTooLarge(f"Upload exceeds the {limit}-byte limit.")
|
||||
chunks.append(chunk)
|
||||
return b"".join(chunks)
|
||||
|
||||
|
||||
def ensure_docs_dir() -> Path:
|
||||
path = Path(DOCS_DIR)
|
||||
path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
Reference in New Issue
Block a user