From f10c28e5cdcef52cc44e953a8494b4bbe35b77fe Mon Sep 17 00:00:00 2001 From: Lars Date: Tue, 13 Oct 2026 16:47:57 +0200 Subject: [PATCH] Close medium exposure on reports, registration, uploads, and exercise HTML. A superadmin can also turn off another account''s two-factor when the authenticator is lost. Co-authored-by: Cursor --- .../working/ACCESS_LAYER_ENDPOINT_AUDIT.md | 1 + backend/exercise_rich_text.py | 123 +++++++++++++- backend/models.py | 5 +- backend/password_reset_mail.py | 1 + backend/routers/admin_users.py | 2 +- backend/routers/auth.py | 138 ++++++++-------- backend/routers/content_reports.py | 13 +- backend/routers/exercises.py | 58 +++---- backend/routers/profiles.py | 33 ++++ backend/tests/test_auth_hardening.py | 35 ++++ backend/tests/test_exercise_rich_text.py | 22 ++- backend/version.py | 30 +++- frontend/package.json | 2 + frontend/src/pages/AdminUsersPage.jsx | 33 +++- frontend/src/pages/ClubsPage.jsx | 2 +- frontend/src/utils/api.js | 6 + .../src/utils/exerciseRichTextSanitize.js | 150 +++++++----------- .../utils/exerciseRichTextSanitize.test.js | 21 +++ frontend/src/utils/htmlUtils.js | 16 +- frontend/src/utils/sanitizeHtml.js | 36 +---- 20 files changed, 480 insertions(+), 247 deletions(-) create mode 100644 frontend/src/utils/exerciseRichTextSanitize.test.js diff --git a/.claude/docs/working/ACCESS_LAYER_ENDPOINT_AUDIT.md b/.claude/docs/working/ACCESS_LAYER_ENDPOINT_AUDIT.md index 5f75854..b46e857 100644 --- a/.claude/docs/working/ACCESS_LAYER_ENDPOINT_AUDIT.md +++ b/.claude/docs/working/ACCESS_LAYER_ENDPOINT_AUDIT.md @@ -7,6 +7,7 @@ Fortlaufend gemäß `ACCESS_LAYER_AND_GOVERNANCE_PLAN.md` Stufe A–C. | profiles | `GET /api/profiles/me` | ja | `resolve_tenant_context` inline (`invalid_header_policy=ignore`) | teils | + `effective_club_id`; veralteter Header bricht Refresh nicht | | profiles | `GET /api/profiles`, `GET /profiles/{pid}`, `POST /profiles`, `DELETE /profiles/{pid}` | ja/teils | `require_auth` | ja | Liste nur Plattform-Admin; GET nach ID eigenes Profil oder Admin; POST/DELETE nur Admin | | profiles | `PUT /api/profiles/{id}`, `PUT /api/profile` | ja | `get_tenant_context` | `active_club_id` Mitgliedschaft | Validiert `X-Active-Club-Id` konsistent zu Mitgliedschaft | +| profiles | `POST /api/profiles/{id}/totp-reset` | Plattform | `get_tenant_context` | nur `superadmin`, nicht das eigene Konto | Schaltet TOTP ab, löscht Wiederherstellungscodes, beendet Sitzungen | | clubs | geschützte `/api/clubs*`, `/divisions*`, `/groups*` | ja | `get_tenant_context` | Mitgliedschaft / `can_manage_*` | Öffentlich: `/clubs/public-directory` ohne Auth | | club_memberships | `/clubs/{id}/members*` | ja | `get_tenant_context` | ja | | | club_join_requests | `/me/club-join-requests`, `/clubs/{id}/join-requests*` | ja | `get_tenant_context` | ja | | diff --git a/backend/exercise_rich_text.py b/backend/exercise_rich_text.py index 0e996a9..80a71cc 100644 --- a/backend/exercise_rich_text.py +++ b/backend/exercise_rich_text.py @@ -8,7 +8,9 @@ from __future__ import annotations import re +from html.parser import HTMLParser from typing import FrozenSet, Optional, Set +from html import escape as html_escape from fastapi import HTTPException @@ -40,7 +42,126 @@ def normalize_inline_exercise_media_markup(html: Optional[str]) -> Optional[str] f'class="shinkan-inline-media">' ) - return _BRACE_PATTERN.sub(_repl, html) + return sanitize_exercise_rich_html(_BRACE_PATTERN.sub(_repl, html)) + + +_ALLOWED_RICH_TAGS = frozenset( + {"p", "div", "br", "ul", "ol", "li", "h3", "b", "strong", "i", "em", "u", "span", "a"} +) +_DROP_WITH_CHILDREN = frozenset( + {"script", "iframe", "object", "embed", "style", "link", "meta", "svg", "math"} +) +_VOID_TAGS = frozenset({"br"}) +_MEDIA_SIZES = frozenset({"small", "medium", "full"}) +_CAPTION_MAX = 120 + + +def _clean_caption(raw: str) -> str: + text = re.sub(r"[\u0000-\u001f\u007f]", " ", raw or "") + text = re.sub(r"[\"'`<>]", " ", text) + text = re.sub(r"\s+", " ", text).strip() + return text[:_CAPTION_MAX] + + +def _safe_rich_attrs(tag: str, attrs: list[tuple[str, Optional[str]]]) -> list[tuple[str, str]]: + values = {} + for name, value in attrs: + if not name: + continue + values[name.lower()] = "" if value is None else str(value) + if tag == "a": + href = values.get("href", "").strip() + if href.lower().startswith(("http://", "https://")) and not re.search(r"[\u0000-\u001f\u007f]", href): + return [("href", href)] + return [] + if tag != "span": + return [] + media_id = values.get("data-shinkan-exercise-media", "").strip() + if not re.fullmatch(r"\d+", media_id): + return [] + out = [("data-shinkan-exercise-media", media_id)] + size = values.get("data-shinkan-exercise-media-size", "").strip().lower() + if size in _MEDIA_SIZES: + out.append(("data-shinkan-exercise-media-size", size)) + caption = _clean_caption(values.get("data-shinkan-exercise-media-caption", "")) + if caption: + out.append(("data-shinkan-exercise-media-caption", caption)) + classes = [c for c in values.get("class", "").split() if c == "shinkan-inline-media"] + if classes: + out.append(("class", "shinkan-inline-media")) + return out + + +class _RichHtmlSanitizer(HTMLParser): + """Baut erlaubtes Übungs-HTML neu auf. Unbekannte Tags fallen weg, ihr Text bleibt.""" + + def __init__(self) -> None: + super().__init__(convert_charrefs=True) + self.parts: list[str] = [] + self._drop_depth = 0 + self._stack: list[str] = [] + + def handle_starttag(self, tag: str, attrs: list[tuple[str, Optional[str]]]) -> None: + self._start(tag.lower(), attrs, void=False) + + def handle_startendtag(self, tag: str, attrs: list[tuple[str, Optional[str]]]) -> None: + self._start(tag.lower(), attrs, void=True) + + def _start(self, tag: str, attrs: list[tuple[str, Optional[str]]], *, void: bool) -> None: + if self._drop_depth: + if not void and tag not in _VOID_TAGS: + self._drop_depth += 1 + return + if tag in _DROP_WITH_CHILDREN: + if not void: + self._drop_depth = 1 + return + if tag not in _ALLOWED_RICH_TAGS: + return + safe = _safe_rich_attrs(tag, attrs) + rendered = tag + if safe: + rendered += "".join(f' {name}="{html_escape(value, quote=True)}"' for name, value in safe) + if void or tag in _VOID_TAGS: + self.parts.append(f"<{rendered}>") + return + self._stack.append(tag) + self.parts.append(f"<{rendered}>") + + def handle_endtag(self, tag: str) -> None: + tag = tag.lower() + if self._drop_depth: + if tag in _DROP_WITH_CHILDREN or self._drop_depth > 1: + self._drop_depth = max(0, self._drop_depth - 1) + return + if self._stack and self._stack[-1] == tag: + self._stack.pop() + self.parts.append(f"") + + def handle_data(self, data: str) -> None: + if self._drop_depth or not data: + return + self.parts.append(html_escape(data)) + + def handle_entityref(self, name: str) -> None: + self.handle_data(f"&{name};") + + def handle_charref(self, name: str) -> None: + self.handle_data(f"&#{name};") + + +def sanitize_exercise_rich_html(html: Optional[str]) -> Optional[str]: + """Allowlist für gespeichertes Übungs-HTML. Skripte, Handler und fremde Attribute fallen weg.""" + if html is None: + return None + if not isinstance(html, str): + html = str(html) + if not html.strip(): + return html + parser = _RichHtmlSanitizer() + parser.feed(html) + parser.close() + return "".join(parser.parts) def collect_inline_exercise_media_ids(html: Optional[str]) -> Set[int]: diff --git a/backend/models.py b/backend/models.py index d00b0dd..9f454f5 100644 --- a/backend/models.py +++ b/backend/models.py @@ -41,7 +41,10 @@ class RegisterRequest(BaseModel): requested_club_id: Optional[int] = Field(default=None, ge=1) class PasswordResetRequest(BaseModel): - email: str + email: EmailStr + +class ResendVerificationRequest(BaseModel): + email: EmailStr class PasswordResetConfirm(BaseModel): token: str diff --git a/backend/password_reset_mail.py b/backend/password_reset_mail.py index 5cb0e26..a748fe5 100644 --- a/backend/password_reset_mail.py +++ b/backend/password_reset_mail.py @@ -53,6 +53,7 @@ Neues Passwort setzen: Der Link ist 1 Stunde gültig. Erst wenn du ihn nutzt und ein neues Passwort wählst, wird dein bestehendes Passwort ersetzt — bis dahin kannst du dich wie gewohnt anmelden. +Der Link meldet dich nicht an und schaltet die Zwei-Faktor-Anmeldung nicht ab. Falls du diese Anfrage nicht erwartest, ignoriere diese E-Mail; dein Zugang bleibt unverändert. diff --git a/backend/routers/admin_users.py b/backend/routers/admin_users.py index 2d32f41..ac22c20 100644 --- a/backend/routers/admin_users.py +++ b/backend/routers/admin_users.py @@ -13,7 +13,7 @@ router = APIRouter(prefix="/api/admin", tags=["admin_users"]) _SAFE_PROFILE_COLS = """ id, name, email, role, tier, email_verified, active_club_id, - created_at, updated_at, auth_type + created_at, updated_at, auth_type, totp_enabled """ diff --git a/backend/routers/auth.py b/backend/routers/auth.py index ce97898..4c5ac0c 100644 --- a/backend/routers/auth.py +++ b/backend/routers/auth.py @@ -38,6 +38,7 @@ from models import ( PasswordResetRequest, PasswordResetConfirm, RegisterRequest, + ResendVerificationRequest, TotpCodeRequest, TotpDisableRequest, ) @@ -643,55 +644,76 @@ async def register(req: RegisterRequest, request: Request): with get_db() as conn: cur = get_cursor(conn) - # Check if email already exists - cur.execute("SELECT id FROM profiles WHERE email=%s", (email,)) - if cur.fetchone(): - raise HTTPException(400, "E-Mail-Adresse bereits registriert") + cur.execute("SELECT id, name FROM profiles WHERE email=%s", (email,)) + existing = cur.fetchone() + if existing: + existing_name = existing.get("name") + else: + existing_name = None - # Generate verification token - verification_token = secrets.token_urlsafe(32) - verification_expires = datetime.now(timezone.utc) + timedelta(hours=24) + if existing is None: + # Generate verification token + verification_token = secrets.token_urlsafe(32) + verification_expires = datetime.now(timezone.utc) + timedelta(hours=24) - # Rolle: erster Nutzer oder ADMIN_BOOTSTRAP_EMAILS → superadmin - role = registration_role(cur, email) + # Rolle: erster Nutzer oder ADMIN_BOOTSTRAP_EMAILS → superadmin + role = registration_role(cur, email) - # Create profile (inactive until verified) — profiles.id ist SERIAL (INT), keine String-IDs einfügen. - pin_hash = hash_pin(password) - trial_ends = datetime.now(timezone.utc) + timedelta(days=14) # 14-day trial + # Create profile (inactive until verified) — profiles.id ist SERIAL (INT), keine String-IDs einfügen. + pin_hash = hash_pin(password) + trial_ends = datetime.now(timezone.utc) + timedelta(days=14) # 14-day trial - cur.execute(""" - INSERT INTO profiles ( - name, email, pin_hash, auth_type, role, tier, - email_verified, verification_token, verification_expires, - trial_ends_at, created_at - ) VALUES (%s, %s, %s, 'email', %s, 'free', FALSE, %s, %s, %s, CURRENT_TIMESTAMP) - RETURNING id - """, (name, email, pin_hash, role, verification_token, verification_expires, trial_ends)) - new_profile_id = cur.fetchone()["id"] + cur.execute(""" + INSERT INTO profiles ( + name, email, pin_hash, auth_type, role, tier, + email_verified, verification_token, verification_expires, + trial_ends_at, created_at + ) VALUES (%s, %s, %s, 'email', %s, 'free', FALSE, %s, %s, %s, CURRENT_TIMESTAMP) + RETURNING id + """, (name, email, pin_hash, role, verification_token, verification_expires, trial_ends)) + new_profile_id = cur.fetchone()["id"] - req_club = req.requested_club_id - if req_club is not None: - cur.execute( - "SELECT id FROM clubs WHERE id = %s AND status = 'active'", - (int(req_club),), - ) - if cur.fetchone(): + req_club = req.requested_club_id + if req_club is not None: cur.execute( - """ - SELECT id FROM club_membership_requests - WHERE profile_id = %s AND club_id = %s AND status = 'pending' - LIMIT 1 - """, - (new_profile_id, int(req_club)), + "SELECT id FROM clubs WHERE id = %s AND status = 'active'", + (int(req_club),), ) - if not cur.fetchone(): + if cur.fetchone(): cur.execute( """ - INSERT INTO club_membership_requests (profile_id, club_id, status, message) - VALUES (%s, %s, 'pending', NULL) + SELECT id FROM club_membership_requests + WHERE profile_id = %s AND club_id = %s AND status = 'pending' + LIMIT 1 """, (new_profile_id, int(req_club)), ) + if not cur.fetchone(): + cur.execute( + """ + INSERT INTO club_membership_requests (profile_id, club_id, status, message) + VALUES (%s, %s, 'pending', NULL) + """, + (new_profile_id, int(req_club)), + ) + + generic = { + "ok": True, + "message": "Registrierung erfolgreich! Bitte prüfe dein E-Mail-Postfach und bestätige deine E-Mail-Adresse.", + } + if existing is not None: + send_email( + email, + "Shinkan Jinkendo – Registrierung", + f"""Hallo {existing_name or name}, + +für diese E-Mail-Adresse gibt es bereits ein Konto. Es wurde kein neues Passwort gesetzt. +Wenn du den Zugang brauchst, melde dich an oder fordere ein neues Passwort an. + +Dein Shinkan Jinkendo Team +""", + ) + return generic verify_url = verification_link(verification_token) @@ -711,10 +733,7 @@ Dein Shinkan Jinkendo Team if not send_email(email, "Shinkan Jinkendo – E-Mail bestätigen", email_body): print("[SMTP] Verifizierungs-Mail konnte nicht gesendet werden — Logs prüfen, SMTP_* in der Laufzeitumgebung.") - return { - "ok": True, - "message": "Registrierung erfolgreich! Bitte prüfe dein E-Mail-Postfach und bestätige deine E-Mail-Adresse." - } + return generic @router.get("/verify/{token}") @@ -773,44 +792,34 @@ async def verify_email(token: str): @router.post("/resend-verification") @limiter.limit("3/hour") -async def resend_verification(req: dict, request: Request): - """Resend verification email for unverified account.""" - email = req.get('email', '').strip().lower() - - if not email: - raise HTTPException(400, "E-Mail-Adresse erforderlich") +async def resend_verification(req: ResendVerificationRequest, request: Request): + """Bestätigung erneut senden. Bekannte und unbekannte Adressen antworten gleich.""" + email = str(req.email).lower().strip() + generic = { + "ok": True, + "message": "Falls ein Account mit dieser E-Mail existiert, wurde eine Bestätigungs-E-Mail versendet.", + } with get_db() as conn: cur = get_cursor(conn) - - # Find profile by email cur.execute(""" SELECT id, name, email, email_verified, verification_token, verification_expires FROM profiles WHERE email=%s """, (email,)) - prof = cur.fetchone() + if not prof or prof["email_verified"]: + return generic - if not prof: - # Don't leak info about existing emails - return {"ok": True, "message": "Falls ein Account mit dieser E-Mail existiert, wurde eine Bestätigungs-E-Mail versendet."} - - if prof['email_verified']: - raise HTTPException(400, "E-Mail-Adresse bereits bestätigt") - - # Generate new verification token verification_token = secrets.token_urlsafe(32) verification_expires = datetime.now(timezone.utc) + timedelta(hours=24) - cur.execute(""" UPDATE profiles SET verification_token=%s, verification_expires=%s WHERE id=%s - """, (verification_token, verification_expires, prof['id'])) + """, (verification_token, verification_expires, prof["id"])) verify_url = verification_link(verification_token) - email_body = f"""Hallo {prof['name']}, du hast eine neue Bestätigungs-E-Mail angefordert. @@ -823,8 +832,5 @@ Dieser Link ist 24 Stunden gültig. Dein Shinkan Jinkendo Team """ - - if not send_email(email, "Shinkan Jinkendo – E-Mail bestätigen", email_body): - raise HTTPException(502, "E-Mail konnte nicht versendet werden. SMTP-Einstellungen und Container-Logs prüfen.") - - return {"ok": True, "message": "Bestätigungs-E-Mail wurde erneut versendet."} + send_email(email, "Shinkan Jinkendo – E-Mail bestätigen", email_body) + return generic diff --git a/backend/routers/content_reports.py b/backend/routers/content_reports.py index 525090e..30c9362 100644 --- a/backend/routers/content_reports.py +++ b/backend/routers/content_reports.py @@ -28,11 +28,12 @@ import os import re from typing import Optional -from fastapi import APIRouter, Depends, Header, HTTPException +from fastapi import APIRouter, Depends, Header, HTTPException, Request from pydantic import BaseModel, Field, field_validator from club_tenancy import is_platform_admin, is_superadmin from db import get_db, get_cursor, r2d +from routers.auth import limiter from media_legal_hold import ( LEGAL_HOLD_REASON_CODES, assert_superadmin_for_legal_hold, @@ -432,8 +433,17 @@ def _report_row_to_dict(row) -> dict: # ─── Endpoints ────────────────────────────────────────────────────────────── +def _content_report_rate_marker(request: Request) -> None: + """SlowAPI-Ziel für 10 Meldungen pro Stunde. Nicht als Route: der Decorator bricht die Annotationen.""" + return None + + +limiter.limit("10/hour")(_content_report_rate_marker) + + @router.post("/content-reports", status_code=201) def submit_content_report( + request: Request, body: ContentReportCreate, x_auth_token: Optional[str] = Header(default=None), ): @@ -446,6 +456,7 @@ def submit_content_report( - Journaleintrag im Medien-Audit-Log (wenn target_type=media_asset) - E-Mail-Bestaetigung an Melder + Benachrichtigung an Plattform-Admins (best-effort) """ + limiter._check_request_limit(request, _content_report_rate_marker, False) if not body.good_faith_confirmed: raise HTTPException( status_code=400, diff --git a/backend/routers/exercises.py b/backend/routers/exercises.py index cbb917e..0df30ee 100644 --- a/backend/routers/exercises.py +++ b/backend/routers/exercises.py @@ -163,24 +163,9 @@ ALLOWED_UPLOAD_MIMES = frozenset( } ) -# Dateiendung → MIME wenn der Client keinen sinnvollen Content-Type sendet (häufig mobil / iOS). -_UPLOAD_FILENAME_MIME_FALLBACK = { - ".jpg": "image/jpeg", - ".jpeg": "image/jpeg", - ".png": "image/png", - ".gif": "image/gif", - ".heic": "image/heic", - ".heif": "image/heif", - ".mp4": "video/mp4", - ".mov": "video/quicktime", - ".pdf": "application/pdf", -} - def _sniff_allowed_upload_mime(raw: bytes) -> Optional[str]: """Erkennt erlaubte MIME-Typen anhand weniger Magic Bytes (ohne Pillow/python-magic).""" - if len(raw) < 12: - return None if raw[:3] == b"\xff\xd8\xff": return "image/jpeg" if raw[:8] == b"\x89PNG\r\n\x1a\n": @@ -189,7 +174,7 @@ def _sniff_allowed_upload_mime(raw: bytes) -> Optional[str]: return "image/gif" if raw[:4] == b"%PDF": return "application/pdf" - if raw[4:8] != b"ftyp": + if len(raw) < 12 or raw[4:8] != b"ftyp": return None brand = raw[8:12] # HEIC / HEIF (u. a. iPhone „高效率“) @@ -208,17 +193,11 @@ def resolve_upload_mime_type( content_type: Optional[str], filename: Optional[str], ) -> str: - """Ermittelt ein erlaubtes MIME (Client-Header, Magic Bytes oder Dateiendung).""" - ct = (content_type or "").split(";")[0].strip().lower() - if ct in ALLOWED_UPLOAD_MIMES: - return ct + """MIME nur aus dem Dateiinhalt. Client-Header und Dateiendung entscheiden nicht.""" guessed = _sniff_allowed_upload_mime(raw) if guessed in ALLOWED_UPLOAD_MIMES: return guessed - ext = Path(filename or "").suffix.lower() - fb = _UPLOAD_FILENAME_MIME_FALLBACK.get(ext) - if fb in ALLOWED_UPLOAD_MIMES: - return fb + ct = (content_type or "").split(";")[0].strip().lower() raise ValueError(f"Dateityp nicht erlaubt: {ct or 'unbekannt'}") @@ -3083,16 +3062,28 @@ def delete_exercise_variant( return {"ok": True} -def _content_disposition_inline(filename: Optional[str]) -> str: - """Inline-Darstellung im Browser (