Bound credentials, brake password changes, cap bug reports, page the user list, structure every error (B-94, SEC-54, SEC-55, API-09, API-15, API-16, API-19, API-21, API-28, API-33, REG-12, AUD-42)
- login and password-change fields: at most 256 characters
- wrong current password: per-user brake (5 free, 2s doubling to 300s), 429 even for the
right password while locked; password_changed audited in the same transaction
- POST /bug-reports: 10 per IP per hour, consumed only by a stored report
- GET /admin/users: limit/after_id paging, the panel walks the pages
- 401/403/404/405/422/503 all answer {code, message, params}; the 422 omits the input
- static files revalidated on every use (Cache-Control: no-cache)
- error.* keys for the new codes in all seven languages
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c1b192f6d1
commit
24e5fc3d8b
23 files changed
+374
-111
No files matched your search
+17
-8
@@ -1,7 +1,7 @@
|
||||
import json
|
||||
import secrets
|
||||
|
||||
from fastapi import APIRouter, Depends, Header, HTTPException, Query, status
|
||||
from fastapi import APIRouter, Depends, Header, Query, status
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
from sqlalchemy import select, update
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
@@ -28,11 +28,11 @@ async def require_admin(x_admin_token: str = Header(default="")) -> None:
|
||||
# on two empty strings returns True and would otherwise open the panel to
|
||||
# anyone on an instance that never configured a token.
|
||||
if not settings.admin_token:
|
||||
raise HTTPException(status.HTTP_403_FORBIDDEN, "invalid admin token")
|
||||
raise http_error(status.HTTP_403_FORBIDDEN, "invalid_admin_token", "invalid admin token")
|
||||
# compare_digest raises TypeError on a str containing non-ASCII characters
|
||||
# (B-46) -- comparing the UTF-8 bytes instead accepts any input safely.
|
||||
if not secrets.compare_digest(x_admin_token.encode(), settings.admin_token.encode()):
|
||||
raise HTTPException(status.HTTP_403_FORBIDDEN, "invalid admin token")
|
||||
raise http_error(status.HTTP_403_FORBIDDEN, "invalid_admin_token", "invalid admin token")
|
||||
|
||||
|
||||
# `paused` is deliberately NOT here: it has its own audit-logged endpoints
|
||||
@@ -226,8 +226,17 @@ class AdminUserResponse(BaseModel):
|
||||
|
||||
|
||||
@router.get("/users", response_model=list[AdminUserResponse], dependencies=[Depends(require_admin)])
|
||||
async def list_users(session: AsyncSession = Depends(get_session)) -> list[AdminUserResponse]:
|
||||
users = (await session.scalars(select(User).order_by(User.id))).all()
|
||||
async def list_users(
|
||||
limit: int = Query(default=100, ge=1, le=500),
|
||||
after_id: int | None = Query(default=None, ge=0),
|
||||
session: AsyncSession = Depends(get_session),
|
||||
) -> list[AdminUserResponse]:
|
||||
"""B-94 (API-33): by ascending id, a page at a time; the next page asks for `after_id` equal
|
||||
to the last id received, and a list shorter than `limit` is the last one."""
|
||||
query = select(User).order_by(User.id).limit(limit)
|
||||
if after_id is not None:
|
||||
query = query.where(User.id > after_id)
|
||||
users = (await session.scalars(query)).all()
|
||||
return [
|
||||
AdminUserResponse(
|
||||
id=u.id,
|
||||
@@ -254,7 +263,7 @@ async def user_privkey(user_id: int, session: AsyncSession = Depends(get_session
|
||||
the most sensitive data the platform holds."""
|
||||
user = await session.get(User, user_id)
|
||||
if user is None:
|
||||
raise HTTPException(status.HTTP_404_NOT_FOUND, "user not found")
|
||||
raise http_error(status.HTTP_404_NOT_FOUND, "user_not_found", "user not found")
|
||||
|
||||
wif = derive_user_wif(user.derivation_index)
|
||||
await write_audit_log(session, "admin_privkey_accessed", {"user_id": user_id}, user_id=user_id)
|
||||
@@ -282,7 +291,7 @@ async def reset_user_password(
|
||||
reset; only an admin (via /admin, token-gated) can trigger this."""
|
||||
user = await session.get(User, user_id)
|
||||
if user is None:
|
||||
raise HTTPException(status.HTTP_404_NOT_FOUND, "user not found")
|
||||
raise http_error(status.HTTP_404_NOT_FOUND, "user_not_found", "user not found")
|
||||
|
||||
new_password = secrets.token_urlsafe(12)
|
||||
user.password_hash = await hash_password_async(new_password)
|
||||
@@ -484,7 +493,7 @@ async def update_bug_report_status(
|
||||
) -> AdminBugReportResponse:
|
||||
report = await session.get(BugReport, report_id)
|
||||
if report is None:
|
||||
raise HTTPException(status.HTTP_404_NOT_FOUND, "bug report not found")
|
||||
raise http_error(status.HTTP_404_NOT_FOUND, "bug_report_not_found", "bug report not found")
|
||||
|
||||
# B-60: every other admin mutation (config edit, pause/resume, privkey export,
|
||||
# password reset) leaves a trace; this one silently marked a report `resolved`.
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
from fastapi import APIRouter, Depends, status
|
||||
from fastapi import APIRouter, Depends, Request, status
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.api.client_ip import client_ip
|
||||
from app.api.timeutil import isoformat_utc
|
||||
from app.auth.dependencies import get_current_user, get_optional_user
|
||||
from app.auth.routes import _rate_limited_error, _rate_limiters
|
||||
from app.db.models import BugReport, User
|
||||
from app.db.session import get_session
|
||||
|
||||
@@ -39,9 +41,17 @@ class BugReportResponse(BaseModel):
|
||||
@router.post("", response_model=BugReportResponse, status_code=status.HTTP_201_CREATED)
|
||||
async def create_bug_report(
|
||||
body: BugReportCreate,
|
||||
request: Request,
|
||||
user: User | None = Depends(get_optional_user),
|
||||
session: AsyncSession = Depends(get_session),
|
||||
) -> BugReportResponse:
|
||||
# B-94 (SEC-55): a quota per IP, checked before the insert and consumed only by a report
|
||||
# that was stored (validation failures never get here).
|
||||
quota = _rate_limiters(request).bug_reports
|
||||
ip_key = f"ip:{client_ip(request)}"
|
||||
retry_after = quota.retry_after(ip_key)
|
||||
if retry_after > 0:
|
||||
raise _rate_limited_error(retry_after)
|
||||
report = BugReport(
|
||||
description=body.description,
|
||||
contact=body.contact,
|
||||
@@ -49,6 +59,7 @@ async def create_bug_report(
|
||||
)
|
||||
session.add(report)
|
||||
await session.commit()
|
||||
quota.record(ip_key)
|
||||
return BugReportResponse(id=report.id)
|
||||
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import json
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from fastapi import APIRouter, Depends, Request
|
||||
from fastapi.responses import JSONResponse, Response, StreamingResponse
|
||||
from fastapi.responses import Response, StreamingResponse
|
||||
from pydantic import BaseModel
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
@@ -57,7 +57,7 @@ async def round_stream(request: Request) -> Response:
|
||||
try:
|
||||
queue = broadcaster.subscribe(client_ip(request))
|
||||
except RoundEventCapacityError:
|
||||
return JSONResponse(status_code=503, content={"detail": "too many concurrent update streams"})
|
||||
raise http_error(503, "too_many_streams", "too many concurrent update streams")
|
||||
|
||||
async def event_generator():
|
||||
ticks_since_keepalive = 0
|
||||
|
||||
+16
-3
@@ -1,11 +1,13 @@
|
||||
from fastapi import APIRouter, Depends, Request, status
|
||||
from pydantic import BaseModel
|
||||
from pydantic import BaseModel, Field
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.api.errors import http_error
|
||||
from app.audit.log import write_audit_log
|
||||
from app.api.timeutil import isoformat_utc
|
||||
from app.auth.dependencies import get_current_user
|
||||
from app.auth.routes import _rate_limited_error, _rate_limiters
|
||||
from app.auth.security import (
|
||||
MIN_PASSWORD_LENGTH,
|
||||
create_access_token,
|
||||
@@ -52,8 +54,8 @@ async def me(
|
||||
|
||||
|
||||
class ChangePasswordRequest(BaseModel):
|
||||
current_password: str
|
||||
new_password: str
|
||||
current_password: str = Field(max_length=256) # B-94 (REG-12)
|
||||
new_password: str = Field(max_length=256)
|
||||
|
||||
|
||||
class ChangePasswordResponse(BaseModel):
|
||||
@@ -63,13 +65,22 @@ class ChangePasswordResponse(BaseModel):
|
||||
@router.post("/me/change-password", response_model=ChangePasswordResponse)
|
||||
async def change_password(
|
||||
body: ChangePasswordRequest,
|
||||
request: Request,
|
||||
user: User = Depends(get_current_user),
|
||||
session: AsyncSession = Depends(get_session),
|
||||
) -> ChangePasswordResponse:
|
||||
"""Self-service password change — requires the current password, unlike the
|
||||
admin-only /admin/users/{id}/reset-password (which is for a user who's
|
||||
actually locked out and can't provide it)."""
|
||||
# B-94 (SEC-54): a stolen token must not be able to guess the current password, change it
|
||||
# and lock the owner out. Checked before the Argon2 hash and even for the right password.
|
||||
limiter = _rate_limiters(request).change_password
|
||||
key = f"user:{user.id}"
|
||||
retry_after = limiter.retry_after(key)
|
||||
if retry_after > 0:
|
||||
raise _rate_limited_error(retry_after)
|
||||
if not await verify_password_async(body.current_password, user.password_hash):
|
||||
limiter.record_failure(key)
|
||||
raise http_error(
|
||||
status.HTTP_401_UNAUTHORIZED, "current_password_incorrect", "current password is incorrect"
|
||||
)
|
||||
@@ -89,7 +100,9 @@ async def change_password(
|
||||
# every other open session (this user's other devices, or an attacker's)
|
||||
# gets "session_expired" on its next request.
|
||||
user.token_version += 1
|
||||
await write_audit_log(session, "password_changed", {"user_id": user.id}, user_id=user.id)
|
||||
await session.commit()
|
||||
limiter.record_success(key)
|
||||
return ChangePasswordResponse(access_token=create_access_token(user.id, user.token_version))
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user