From 1ce01ec348abb40358af164928635413abae3e1c Mon Sep 17 00:00:00 2001 From: Rephl3x Date: Sat, 23 May 2026 21:12:45 +1200 Subject: [PATCH] Harden auth and outbound admin surfaces --- backend/app/auth.py | 127 +++++++++++++++++++------ backend/app/config.py | 29 +++++- backend/app/db.py | 17 +++- backend/app/main.py | 24 ++++- backend/app/network_security.py | 132 ++++++++++++++++++++++++++ backend/app/routers/admin.py | 13 +++ backend/app/routers/auth.py | 126 +++++++++++++++--------- backend/app/routers/feedback.py | 5 + backend/app/services/diagnostics.py | 37 +++++++- backend/app/services/notifications.py | 4 + backend/tests/test_backend_quality.py | 22 +++++ frontend/app/lib/auth.ts | 50 +++++++--- frontend/app/login/page.tsx | 5 +- frontend/app/signup/page.tsx | 7 +- frontend/app/ui/HeaderIdentity.tsx | 7 +- 15 files changed, 495 insertions(+), 110 deletions(-) create mode 100644 backend/app/network_security.py diff --git a/backend/app/auth.py b/backend/app/auth.py index 129d3f4..945bc84 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -1,13 +1,15 @@ from datetime import datetime, timezone -from typing import Dict, Any, Optional +from typing import Any, Dict, Optional -from fastapi import Depends, HTTPException, status, Request +from fastapi import Depends, HTTPException, Request, Response, status from fastapi.security import OAuth2PasswordBearer +from .config import settings from .db import get_user_by_username, set_user_auth_provider, upsert_user_activity -from .security import safe_decode_token, TokenError, verify_password +from .network_security import request_trusts_forwarded_headers +from .security import TokenError, safe_decode_token, verify_password -oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/auth/login") +oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/auth/login", auto_error=False) def _is_expired(expires_at: str | None) -> bool: @@ -24,20 +26,79 @@ def _is_expired(expires_at: str | None) -> bool: parsed = parsed.replace(tzinfo=timezone.utc) return parsed <= datetime.now(timezone.utc) + def _extract_client_ip(request: Request) -> str: - forwarded = request.headers.get("x-forwarded-for") - if forwarded: - parts = [part.strip() for part in forwarded.split(",") if part.strip()] - if parts: - return parts[0] - real_ip = request.headers.get("x-real-ip") - if real_ip: - return real_ip.strip() - if request.client and request.client.host: - return request.client.host + direct_host = request.client.host if request.client else None + if request_trusts_forwarded_headers(direct_host): + forwarded = request.headers.get("x-forwarded-for") + if forwarded: + parts = [part.strip() for part in forwarded.split(",") if part.strip()] + if parts: + return parts[0] + real_ip = request.headers.get("x-real-ip") + if real_ip: + return real_ip.strip() + if direct_host: + return direct_host return "unknown" +def _cookie_settings() -> dict[str, Any]: + samesite = str(settings.auth_cookie_samesite or "lax").strip().lower() + if samesite not in {"lax", "strict", "none"}: + samesite = "lax" + return { + "secure": bool(settings.auth_cookie_secure), + "httponly": True, + "samesite": samesite, + "domain": settings.auth_cookie_domain or None, + "path": "/", + } + + +def _state_cookie_settings() -> dict[str, Any]: + cookie = _cookie_settings() + cookie["httponly"] = False + return cookie + + +def set_auth_cookies(response: Response, token: str) -> None: + max_age = max(60, int(settings.jwt_exp_minutes or 720) * 60) + response.set_cookie( + settings.auth_cookie_name, + token, + max_age=max_age, + **_cookie_settings(), + ) + response.set_cookie( + settings.auth_state_cookie_name, + "1", + max_age=max_age, + **_state_cookie_settings(), + ) + + +def clear_auth_cookies(response: Response) -> None: + response.delete_cookie(settings.auth_cookie_name, path="/", domain=settings.auth_cookie_domain or None) + response.delete_cookie( + settings.auth_state_cookie_name, + path="/", + domain=settings.auth_cookie_domain or None, + ) + + +def _extract_access_token(request: Request, oauth_token: Optional[str]) -> Optional[str]: + auth_header = request.headers.get("authorization", "") + if auth_header.lower().startswith("bearer "): + return auth_header.split(" ", 1)[1].strip() + if oauth_token: + return oauth_token + cookie_token = request.cookies.get(settings.auth_cookie_name) + if isinstance(cookie_token, str) and cookie_token.strip(): + return cookie_token.strip() + return None + + def resolve_user_auth_provider(user: Optional[Dict[str, Any]]) -> str: if not isinstance(user, dict): return "local" @@ -122,24 +183,28 @@ def _load_current_user_from_token( } -def get_current_user(token: str = Depends(oauth2_scheme), request: Request = None) -> Dict[str, Any]: - return _load_current_user_from_token(token, request) - - -def get_current_user_event_stream(request: Request) -> Dict[str, Any]: - """EventSource cannot send Authorization headers, so allow a short-lived stream token via query.""" - token = None - stream_query_token = None - auth_header = request.headers.get("authorization", "") - if auth_header.lower().startswith("bearer "): - token = auth_header.split(" ", 1)[1].strip() - if not token: - stream_query_token = request.query_params.get("stream_token") - if not token and not stream_query_token: +def get_current_user( + request: Request, + token: Optional[str] = Depends(oauth2_scheme), +) -> Dict[str, Any]: + resolved_token = _extract_access_token(request, token) + if not resolved_token: + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Missing token") + return _load_current_user_from_token(resolved_token, request) + + +def get_current_user_event_stream( + request: Request, + token: Optional[str] = Depends(oauth2_scheme), +) -> Dict[str, Any]: + """EventSource cannot send Authorization headers, so allow a short-lived stream token via query.""" + resolved_token = _extract_access_token(request, token) + stream_query_token = request.query_params.get("stream_token") + if resolved_token: + # Allow standard bearer tokens for non-browser EventSource clients. + return _load_current_user_from_token(resolved_token, None) + if not stream_query_token: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Missing token") - if token: - # Allow standard bearer tokens in Authorization for non-browser EventSource clients. - return _load_current_user_from_token(token, None) return _load_current_user_from_token( str(stream_query_token), None, diff --git a/backend/app/config.py b/backend/app/config.py index 65a42e6..ddea219 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -12,7 +12,7 @@ class Settings(BaseSettings): sqlite_journal_mode: str = Field( default="DELETE", validation_alias=AliasChoices("SQLITE_JOURNAL_MODE") ) - jwt_secret: str = Field(default="change-me", validation_alias=AliasChoices("JWT_SECRET")) + jwt_secret: str = Field(default="", validation_alias=AliasChoices("JWT_SECRET")) jwt_exp_minutes: int = Field(default=720, validation_alias=AliasChoices("JWT_EXP_MINUTES")) api_docs_enabled: bool = Field(default=False, validation_alias=AliasChoices("API_DOCS_ENABLED")) auth_rate_limit_window_seconds: int = Field( @@ -34,7 +34,22 @@ class Settings(BaseSettings): default=3, validation_alias=AliasChoices("PASSWORD_RESET_RATE_LIMIT_MAX_ATTEMPTS_IDENTIFIER") ) admin_username: str = Field(default="admin", validation_alias=AliasChoices("ADMIN_USERNAME")) - admin_password: str = Field(default="adminadmin", validation_alias=AliasChoices("ADMIN_PASSWORD")) + admin_password: str = Field(default="", validation_alias=AliasChoices("ADMIN_PASSWORD")) + auth_cookie_name: str = Field( + default="magent_auth", validation_alias=AliasChoices("AUTH_COOKIE_NAME") + ) + auth_cookie_secure: bool = Field( + default=False, validation_alias=AliasChoices("AUTH_COOKIE_SECURE") + ) + auth_cookie_samesite: str = Field( + default="lax", validation_alias=AliasChoices("AUTH_COOKIE_SAMESITE") + ) + auth_cookie_domain: Optional[str] = Field( + default=None, validation_alias=AliasChoices("AUTH_COOKIE_DOMAIN") + ) + auth_state_cookie_name: str = Field( + default="magent_logged_in", validation_alias=AliasChoices("AUTH_STATE_COOKIE_NAME") + ) log_level: str = Field(default="INFO", validation_alias=AliasChoices("LOG_LEVEL")) log_file: str = Field(default="data/magent.log", validation_alias=AliasChoices("LOG_FILE")) log_file_max_bytes: int = Field( @@ -121,6 +136,10 @@ class Settings(BaseSettings): magent_proxy_trust_forwarded_headers: bool = Field( default=True, validation_alias=AliasChoices("MAGENT_PROXY_TRUST_FORWARDED_HEADERS") ) + magent_proxy_trusted_proxies: str = Field( + default="127.0.0.1,::1", + validation_alias=AliasChoices("MAGENT_PROXY_TRUSTED_PROXIES"), + ) magent_proxy_forwarded_prefix: Optional[str] = Field( default=None, validation_alias=AliasChoices("MAGENT_PROXY_FORWARDED_PREFIX") ) @@ -216,6 +235,10 @@ class Settings(BaseSettings): magent_notify_webhook_url: Optional[str] = Field( default=None, validation_alias=AliasChoices("MAGENT_NOTIFY_WEBHOOK_URL") ) + magent_allow_private_notification_targets: bool = Field( + default=False, + validation_alias=AliasChoices("MAGENT_ALLOW_PRIVATE_NOTIFICATION_TARGETS"), + ) jellyseerr_base_url: Optional[str] = Field( default=None, validation_alias=AliasChoices("JELLYSEERR_URL", "JELLYSEERR_BASE_URL") @@ -288,7 +311,7 @@ class Settings(BaseSettings): ) discord_webhook_url: Optional[str] = Field( - default="https://discord.com/api/webhooks/1464141924775629033/O_rvCAmIKowR04tyAN54IuMPcQFEiT-ustU3udDaMTlF62PmoI6w4-52H3ZQcjgHQOgt", + default=None, validation_alias=AliasChoices("DISCORD_WEBHOOK_URL"), ) diff --git a/backend/app/db.py b/backend/app/db.py index 2bc3892..9d2c156 100644 --- a/backend/app/db.py +++ b/backend/app/db.py @@ -21,6 +21,8 @@ SQLITE_BUSY_TIMEOUT_MS = 5_000 SQLITE_CACHE_SIZE_KIB = 32_768 SQLITE_MMAP_SIZE_BYTES = 256 * 1024 * 1024 _DB_UNSET = object() +_DEFAULT_JWT_SECRET = "change-me" +_DEFAULT_ADMIN_PASSWORD = "adminadmin" def _db_path() -> str: @@ -178,6 +180,11 @@ def _normalize_stored_email(value: Optional[Any]) -> Optional[str]: return candidate +def _has_secure_bootstrap_admin_credentials() -> bool: + password = str(settings.admin_password or "") + return bool(password and password != _DEFAULT_ADMIN_PASSWORD) + + def init_db() -> None: with _connect() as conn: conn.execute( @@ -767,7 +774,7 @@ def get_recent_actions(request_id: str, limit: int = 10) -> list[dict[str, Any]] def ensure_admin_user() -> None: - if not settings.admin_username or not settings.admin_password: + if not settings.admin_username or not _has_secure_bootstrap_admin_credentials(): return existing = get_user_by_username(settings.admin_username) if existing: @@ -775,6 +782,14 @@ def ensure_admin_user() -> None: create_user(settings.admin_username, settings.admin_password, role="admin") +def has_admin_user() -> bool: + with _connect() as conn: + row = conn.execute( + "SELECT 1 FROM users WHERE LOWER(role) = 'admin' LIMIT 1" + ).fetchone() + return bool(row) + + def create_user( username: str, password: str, diff --git a/backend/app/main.py b/backend/app/main.py index a07105e..5db75d8 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -8,7 +8,7 @@ from fastapi import FastAPI, Request from fastapi.middleware.cors import CORSMiddleware from .config import settings -from .db import init_db +from .db import has_admin_user, init_db from .routers.requests import ( router as requests_router, startup_warmup_requests_cache, @@ -165,13 +165,15 @@ def _launch_background_task(name: str, coroutine_factory: Callable[[], Awaitable def _log_security_configuration_warnings() -> None: - if str(settings.jwt_secret or "").strip() == "change-me": + jwt_secret = str(settings.jwt_secret or "").strip() + if not jwt_secret or jwt_secret == "change-me": logger.warning( - "security configuration warning: JWT_SECRET is still set to the default value" + "security configuration warning: JWT_SECRET is unset or still set to the default value" ) - if str(settings.admin_password or "") == "adminadmin": + admin_password = str(settings.admin_password or "") + if not admin_password or admin_password == "adminadmin": logger.warning( - "security configuration warning: ADMIN_PASSWORD is still set to the bootstrap default" + "security configuration warning: ADMIN_PASSWORD is unset or still set to the bootstrap default" ) if bool(settings.api_docs_enabled): logger.warning( @@ -179,6 +181,17 @@ def _log_security_configuration_warnings() -> None: ) +def _enforce_secure_startup_configuration() -> None: + jwt_secret = str(settings.jwt_secret or "").strip() + if not jwt_secret or jwt_secret == "change-me": + raise RuntimeError("JWT_SECRET must be set to a strong, non-default value before startup.") + admin_password = str(settings.admin_password or "") + if not has_admin_user() and (not admin_password or admin_password == "adminadmin"): + raise RuntimeError( + "A secure ADMIN_PASSWORD is required on first startup until an admin account exists." + ) + + @app.on_event("startup") async def startup() -> None: configure_logging( @@ -192,6 +205,7 @@ async def startup() -> None: logger.info("startup begin app=%s build=%s", settings.app_name, settings.site_build_number) _log_security_configuration_warnings() init_db() + _enforce_secure_startup_configuration() runtime = get_runtime_settings() configure_logging( runtime.log_level, diff --git a/backend/app/network_security.py b/backend/app/network_security.py new file mode 100644 index 0000000..68219ed --- /dev/null +++ b/backend/app/network_security.py @@ -0,0 +1,132 @@ +from __future__ import annotations + +import ipaddress +import socket +from functools import lru_cache +from typing import Iterable +from urllib.parse import urlparse + +from .config import settings + +_METADATA_HOSTS = { + "169.254.169.254", + "metadata.google.internal", + "metadata.azure.internal", +} + + +def _normalize_text(value: object) -> str: + if value is None: + return "" + return str(value).strip() + + +def _split_csv(value: object) -> list[str]: + raw = _normalize_text(value) + if not raw: + return [] + return [part.strip() for part in raw.split(",") if part.strip()] + + +def _ip_is_sensitive(ip_obj: ipaddress._BaseAddress) -> bool: + return bool( + ip_obj.is_loopback + or ip_obj.is_link_local + or ip_obj.is_multicast + or ip_obj.is_unspecified + or ip_obj.is_reserved + or ip_obj.is_private + ) + + +@lru_cache(maxsize=256) +def _resolve_host_ips(host: str) -> tuple[ipaddress._BaseAddress, ...]: + resolved: list[ipaddress._BaseAddress] = [] + for family, _, _, _, sockaddr in socket.getaddrinfo(host, None): + if family == socket.AF_INET: + resolved.append(ipaddress.ip_address(sockaddr[0])) + elif family == socket.AF_INET6: + resolved.append(ipaddress.ip_address(sockaddr[0])) + return tuple(resolved) + + +def _is_trusted_proxy_host(host: str, trusted_proxies: Iterable[str]) -> bool: + candidate = _normalize_text(host) + if not candidate: + return False + try: + host_ip = ipaddress.ip_address(candidate) + except ValueError: + return candidate.lower() in {entry.lower() for entry in trusted_proxies} + + for entry in trusted_proxies: + raw = _normalize_text(entry) + if not raw: + continue + try: + if "/" in raw: + if host_ip in ipaddress.ip_network(raw, strict=False): + return True + elif host_ip == ipaddress.ip_address(raw): + return True + except ValueError: + continue + return False + + +def request_trusts_forwarded_headers(client_host: str | None) -> bool: + if not settings.magent_proxy_enabled or not settings.magent_proxy_trust_forwarded_headers: + return False + trusted = _split_csv(settings.magent_proxy_trusted_proxies) + if not trusted: + return False + return _is_trusted_proxy_host(client_host or "", trusted) + + +def validate_notification_target_url( + url: str, + *, + allow_private: bool | None = None, +) -> str: + raw = _normalize_text(url) + if not raw: + raise ValueError("URL cannot be empty.") + + parsed = urlparse(raw) + if parsed.scheme not in {"http", "https"}: + raise ValueError("URL must use http:// or https://.") + if parsed.username or parsed.password: + raise ValueError("URL must not embed credentials.") + hostname = _normalize_text(parsed.hostname).lower() + if not hostname: + raise ValueError("URL must include a valid host.") + + allow_private_targets = ( + settings.magent_allow_private_notification_targets + if allow_private is None + else bool(allow_private) + ) + if hostname in _METADATA_HOSTS: + raise ValueError("Metadata service targets are not allowed.") + if hostname == "localhost" and not allow_private_targets: + raise ValueError("Local notification targets are not allowed.") + + try: + host_ip = ipaddress.ip_address(hostname) + except ValueError: + host_ip = None + + if host_ip is not None: + if _ip_is_sensitive(host_ip) and not allow_private_targets: + raise ValueError("Private or local notification targets are not allowed.") + return raw + + try: + resolved_ips = _resolve_host_ips(hostname) + except socket.gaierror as exc: + raise ValueError("Host could not be resolved.") from exc + if not resolved_ips: + raise ValueError("Host could not be resolved.") + if not allow_private_targets and any(_ip_is_sensitive(ip_obj) for ip_obj in resolved_ips): + raise ValueError("Private or local notification targets are not allowed.") + return raw diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py index 3d1844a..232351a 100644 --- a/backend/app/routers/admin.py +++ b/backend/app/routers/admin.py @@ -20,6 +20,7 @@ from ..auth import ( resolve_user_auth_provider, ) from ..config import settings as env_settings +from ..network_security import validate_notification_target_url from ..db import ( delete_setting, get_all_users, @@ -153,6 +154,12 @@ URL_SETTING_KEYS = { "qbittorrent_base_url", } +NOTIFICATION_URL_SETTING_KEYS = { + "magent_notify_discord_webhook_url", + "magent_notify_push_base_url", + "magent_notify_webhook_url", +} + SETTING_KEYS: List[str] = [ "magent_application_url", "magent_application_port", @@ -659,6 +666,12 @@ async def update_settings(payload: Dict[str, Any]) -> Dict[str, Any]: except ValueError as exc: friendly_key = key.replace("_", " ") raise HTTPException(status_code=400, detail=f"{friendly_key}: {exc}") from exc + if key in NOTIFICATION_URL_SETTING_KEYS and value_to_store: + try: + value_to_store = validate_notification_target_url(value_to_store) + except ValueError as exc: + friendly_key = key.replace("_", " ") + raise HTTPException(status_code=400, detail=f"{friendly_key}: {exc}") from exc set_setting(key, value_to_store) updates += 1 changed_keys.append(key) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 46d93bb..018636e 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -7,7 +7,7 @@ import time from threading import Lock import httpx -from fastapi import APIRouter, HTTPException, status, Depends, Request +from fastapi import APIRouter, HTTPException, status, Depends, Request, Response from fastapi.security import OAuth2PasswordRequestForm from ..db import ( @@ -47,8 +47,15 @@ from ..security import ( verify_password, ) from ..security import create_stream_token -from ..auth import get_current_user, normalize_user_auth_provider, resolve_user_auth_provider +from ..auth import ( + clear_auth_cookies, + get_current_user, + normalize_user_auth_provider, + resolve_user_auth_provider, + set_auth_cookies, +) from ..config import settings +from ..network_security import request_trusts_forwarded_headers from ..services.user_cache import ( build_jellyseerr_candidate_map, extract_jellyseerr_user_email, @@ -96,12 +103,14 @@ def _require_recipient_email(value: object) -> str: def _auth_client_ip(request: Request) -> str: - forwarded = request.headers.get("x-forwarded-for") - if isinstance(forwarded, str) and forwarded.strip(): - return forwarded.split(",", 1)[0].strip() - real = request.headers.get("x-real-ip") - if isinstance(real, str) and real.strip(): - return real.strip() + direct_host = request.client.host if request.client else None + if request_trusts_forwarded_headers(direct_host): + forwarded = request.headers.get("x-forwarded-for") + if isinstance(forwarded, str) and forwarded.strip(): + return forwarded.split(",", 1)[0].strip() + real = request.headers.get("x-real-ip") + if isinstance(real, str) and real.strip(): + return real.strip() if request.client and request.client.host: return str(request.client.host) return "unknown" @@ -358,6 +367,15 @@ def _assert_user_can_login(user: dict | None) -> None: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="User access has expired") +def _auth_success_response(response: Response, token: str, user_payload: dict) -> dict: + set_auth_cookies(response, token) + return { + "authenticated": True, + "token_type": "cookie", + "user": user_payload, + } + + def _public_invite_payload(invite: dict, profile: dict | None = None) -> dict: return { "code": invite.get("code"), @@ -580,7 +598,11 @@ def _master_invite_controlled_values(master_invite: dict) -> tuple[int | None, s @router.post("/login") -async def login(request: Request, form_data: OAuth2PasswordRequestForm = Depends()) -> dict: +async def login( + request: Request, + response: Response, + form_data: OAuth2PasswordRequestForm = Depends(), +) -> dict: _enforce_login_rate_limit(request, form_data.username) logger.info( "login attempt provider=local username=%s client=%s", @@ -629,15 +651,19 @@ async def login(request: Request, form_data: OAuth2PasswordRequestForm = Depends user["role"], _auth_client_ip(request), ) - return { - "access_token": token, - "token_type": "bearer", - "user": {"username": user["username"], "role": user["role"]}, - } + return _auth_success_response( + response, + token, + {"username": user["username"], "role": user["role"]}, + ) @router.post("/jellyfin/login") -async def jellyfin_login(request: Request, form_data: OAuth2PasswordRequestForm = Depends()) -> dict: +async def jellyfin_login( + request: Request, + response: Response, + form_data: OAuth2PasswordRequestForm = Depends(), +) -> dict: _enforce_login_rate_limit(request, form_data.username) logger.info( "login attempt provider=jellyfin username=%s client=%s", @@ -668,13 +694,13 @@ async def jellyfin_login(request: Request, form_data: OAuth2PasswordRequestForm canonical_username, _auth_client_ip(request), ) - return { - "access_token": token, - "token_type": "bearer", - "user": {"username": canonical_username, "role": "user"}, - } + return _auth_success_response( + response, + token, + {"username": canonical_username, "role": "user"}, + ) try: - response = await client.authenticate_by_name(username, password) + auth_response = await client.authenticate_by_name(username, password) except Exception as exc: logger.exception( "login upstream error provider=jellyfin username=%s client=%s", @@ -682,7 +708,7 @@ async def jellyfin_login(request: Request, form_data: OAuth2PasswordRequestForm _auth_client_ip(request), ) raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail=str(exc)) from exc - if not isinstance(response, dict) or not response.get("User"): + if not isinstance(auth_response, dict) or not auth_response.get("User"): _record_login_failure(request, username) raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid Jellyfin credentials") if not preferred_match: @@ -724,16 +750,20 @@ async def jellyfin_login(request: Request, form_data: OAuth2PasswordRequestForm get_user_by_username(canonical_username).get("jellyseerr_user_id") if get_user_by_username(canonical_username) else None, _auth_client_ip(request), ) - return { - "access_token": token, - "token_type": "bearer", - "user": {"username": canonical_username, "role": "user"}, - } + return _auth_success_response( + response, + token, + {"username": canonical_username, "role": "user"}, + ) @router.post("/seerr/login") @router.post("/jellyseerr/login") -async def jellyseerr_login(request: Request, form_data: OAuth2PasswordRequestForm = Depends()) -> dict: +async def jellyseerr_login( + request: Request, + response: Response, + form_data: OAuth2PasswordRequestForm = Depends(), +) -> dict: _enforce_login_rate_limit(request, form_data.username) logger.info( "login attempt provider=seerr username=%s client=%s", @@ -745,7 +775,7 @@ async def jellyseerr_login(request: Request, form_data: OAuth2PasswordRequestFor if not client.configured(): raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Seerr not configured") try: - response = await client.login_local(form_data.username, form_data.password) + auth_response = await client.login_local(form_data.username, form_data.password) except Exception as exc: logger.exception( "login upstream error provider=seerr username=%s client=%s", @@ -753,11 +783,11 @@ async def jellyseerr_login(request: Request, form_data: OAuth2PasswordRequestFor _auth_client_ip(request), ) raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail=str(exc)) from exc - if not isinstance(response, dict): + if not isinstance(auth_response, dict): _record_login_failure(request, form_data.username) raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid Seerr credentials") - jellyseerr_user_id = _extract_jellyseerr_user_id(response) - jellyseerr_email = _extract_jellyseerr_response_email(response) + jellyseerr_user_id = _extract_jellyseerr_user_id(auth_response) + jellyseerr_email = _extract_jellyseerr_response_email(auth_response) ci_matches = get_users_by_username_ci(form_data.username) preferred_match = _pick_preferred_ci_user_match(ci_matches, form_data.username) canonical_username = str(preferred_match.get("username") or form_data.username) if preferred_match else form_data.username @@ -791,11 +821,11 @@ async def jellyseerr_login(request: Request, form_data: OAuth2PasswordRequestFor jellyseerr_user_id, _auth_client_ip(request), ) - return { - "access_token": token, - "token_type": "bearer", - "user": {"username": canonical_username, "role": "user"}, - } + return _auth_success_response( + response, + token, + {"username": canonical_username, "role": "user"}, + ) @router.get("/me") @@ -803,6 +833,12 @@ async def me(current_user: dict = Depends(get_current_user)) -> dict: return current_user +@router.post("/logout") +async def logout(response: Response) -> dict: + clear_auth_cookies(response) + return {"status": "ok"} + + @router.get("/stream-token") async def stream_token(current_user: dict = Depends(get_current_user)) -> dict: token = create_stream_token( @@ -832,7 +868,7 @@ async def invite_details(code: str) -> dict: @router.post("/signup") -async def signup(payload: dict) -> dict: +async def signup(payload: dict, response: Response) -> dict: if not isinstance(payload, dict): raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid payload") invite_code = str(payload.get("invite_code") or "").strip() @@ -908,14 +944,14 @@ async def signup(payload: dict) -> dict: duplicate_like = status_code in {400, 409} if duplicate_like: try: - response = await jellyfin_client.authenticate_by_name(username, password_value) + auth_response = await jellyfin_client.authenticate_by_name(username, password_value) except Exception as auth_exc: detail = _extract_http_error_detail(auth_exc) or _extract_http_error_detail(exc) raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail=f"Jellyfin account already exists and could not be authenticated: {detail}", ) from exc - if not isinstance(response, dict) or not response.get("User"): + if not isinstance(auth_response, dict) or not auth_response.get("User"): raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail="Jellyfin account already exists for that username.", @@ -987,17 +1023,17 @@ async def signup(payload: dict) -> dict: created_user.get("profile_id") if created_user else None, invite.get("code"), ) - return { - "access_token": token, - "token_type": "bearer", - "user": { + return _auth_success_response( + response, + token, + { "username": username, "role": role, "auth_provider": created_user.get("auth_provider") if created_user else auth_provider, "profile_id": created_user.get("profile_id") if created_user else None, "expires_at": created_user.get("expires_at") if created_user else None, }, - } + ) @router.post("/password/forgot") diff --git a/backend/app/routers/feedback.py b/backend/app/routers/feedback.py index d523e43..b42f066 100644 --- a/backend/app/routers/feedback.py +++ b/backend/app/routers/feedback.py @@ -3,6 +3,7 @@ import httpx from fastapi import APIRouter, Depends, HTTPException from ..auth import get_current_user +from ..network_security import validate_notification_target_url from ..runtime import get_runtime_settings router = APIRouter(prefix="/feedback", tags=["feedback"], dependencies=[Depends(get_current_user)]) @@ -17,6 +18,10 @@ async def send_feedback(payload: Dict[str, Any], user: Dict[str, str] = Depends( ) if not webhook_url: raise HTTPException(status_code=400, detail="Discord webhook not configured") + try: + webhook_url = validate_notification_target_url(webhook_url) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc feedback_type = str(payload.get("type") or "").strip().lower() if feedback_type not in {"bug", "feature"}: diff --git a/backend/app/services/diagnostics.py b/backend/app/services/diagnostics.py index 9184045..394c527 100644 --- a/backend/app/services/diagnostics.py +++ b/backend/app/services/diagnostics.py @@ -17,6 +17,7 @@ from ..clients.radarr import RadarrClient from ..clients.sonarr import SonarrClient from ..config import settings as env_settings from ..db import get_database_diagnostics +from ..network_security import validate_notification_target_url from ..runtime import get_runtime_settings from .invite_email import send_test_email, smtp_email_config_ready, smtp_email_delivery_warning @@ -97,7 +98,12 @@ def _config_status(detail: str) -> str: def _discord_config_ready(runtime) -> tuple[bool, str]: if not runtime.magent_notify_enabled or not runtime.magent_notify_discord_enabled: return False, "Discord notifications are disabled." - if _clean_text(runtime.magent_notify_discord_webhook_url) or _clean_text(runtime.discord_webhook_url): + webhook_url = _clean_text(runtime.magent_notify_discord_webhook_url) or _clean_text(runtime.discord_webhook_url) + if webhook_url: + try: + validate_notification_target_url(webhook_url) + except ValueError as exc: + return False, str(exc) return True, "ok" return False, "Discord webhook URL is required." @@ -113,7 +119,12 @@ def _telegram_config_ready(runtime) -> tuple[bool, str]: def _webhook_config_ready(runtime) -> tuple[bool, str]: if not runtime.magent_notify_enabled or not runtime.magent_notify_webhook_enabled: return False, "Generic webhook notifications are disabled." - if _clean_text(runtime.magent_notify_webhook_url): + webhook_url = _clean_text(runtime.magent_notify_webhook_url) + if webhook_url: + try: + validate_notification_target_url(webhook_url) + except ValueError as exc: + return False, str(exc) return True, "ok" return False, "Generic webhook URL is required." @@ -123,11 +134,21 @@ def _push_config_ready(runtime) -> tuple[bool, str]: return False, "Push notifications are disabled." provider = _clean_text(runtime.magent_notify_push_provider, "ntfy").lower() if provider == "ntfy": - if _clean_text(runtime.magent_notify_push_base_url) and _clean_text(runtime.magent_notify_push_topic): + push_url = _clean_text(runtime.magent_notify_push_base_url) + if push_url and _clean_text(runtime.magent_notify_push_topic): + try: + validate_notification_target_url(push_url) + except ValueError as exc: + return False, str(exc) return True, "ok" return False, "ntfy requires a base URL and topic." if provider == "gotify": - if _clean_text(runtime.magent_notify_push_base_url) and _clean_text(runtime.magent_notify_push_token): + push_url = _clean_text(runtime.magent_notify_push_base_url) + if push_url and _clean_text(runtime.magent_notify_push_token): + try: + validate_notification_target_url(push_url) + except ValueError as exc: + return False, str(exc) return True, "ok" return False, "Gotify requires a base URL and app token." if provider == "pushover": @@ -135,7 +156,12 @@ def _push_config_ready(runtime) -> tuple[bool, str]: return True, "ok" return False, "Pushover requires an application token and user key." if provider == "webhook": - if _clean_text(runtime.magent_notify_push_base_url): + push_url = _clean_text(runtime.magent_notify_push_base_url) + if push_url: + try: + validate_notification_target_url(push_url) + except ValueError as exc: + return False, str(exc) return True, "ok" return False, "Webhook relay requires a target URL." if provider == "telegram": @@ -190,6 +216,7 @@ async def _run_http_post( params: Optional[Dict[str, Any]] = None, headers: Optional[Dict[str, str]] = None, ) -> Dict[str, Any]: + validate_notification_target_url(url) async with httpx.AsyncClient(timeout=15.0, follow_redirects=True) as client: response = await client.post(url, json=json_payload, data=data_payload, params=params, headers=headers) response.raise_for_status() diff --git a/backend/app/services/notifications.py b/backend/app/services/notifications.py index 2b47319..5816031 100644 --- a/backend/app/services/notifications.py +++ b/backend/app/services/notifications.py @@ -8,6 +8,7 @@ import httpx from ..config import settings as env_settings from ..db import get_setting +from ..network_security import validate_notification_target_url from ..runtime import get_runtime_settings from .invite_email import send_generic_email @@ -49,6 +50,7 @@ def _portal_item_url(item_id: int) -> str: async def _http_post_json(url: str, payload: Dict[str, Any]) -> Dict[str, Any]: + validate_notification_target_url(url) async with httpx.AsyncClient(timeout=12.0) as client: response = await client.post(url, json=payload) response.raise_for_status() @@ -115,6 +117,7 @@ async def _send_push(title: str, message: str, payload: Dict[str, Any]) -> Dict[ if provider == "ntfy": if not base_url or not topic: return {"status": "skipped", "detail": "ntfy needs base URL and topic."} + validate_notification_target_url(base_url) url = f"{base_url.rstrip('/')}/{quote(topic)}" headers = {"Title": title, "Tags": "magent,portal"} async with httpx.AsyncClient(timeout=12.0) as client: @@ -124,6 +127,7 @@ async def _send_push(title: str, message: str, payload: Dict[str, Any]) -> Dict[ if provider == "gotify": if not base_url or not token: return {"status": "skipped", "detail": "Gotify needs base URL and token."} + validate_notification_target_url(base_url) url = f"{base_url.rstrip('/')}/message?token={quote(token)}" body = {"title": title, "message": message, "priority": 5, "extras": {"client::display": {"contentType": "text/plain"}}} result = await _http_post_json(url, body) diff --git a/backend/tests/test_backend_quality.py b/backend/tests/test_backend_quality.py index a5e36e2..4bcd3ef 100644 --- a/backend/tests/test_backend_quality.py +++ b/backend/tests/test_backend_quality.py @@ -8,6 +8,7 @@ from starlette.requests import Request from backend.app import db from backend.app.config import settings +from backend.app.network_security import request_trusts_forwarded_headers, validate_notification_target_url from backend.app.routers import auth as auth_router from backend.app.routers import portal as portal_router from backend.app.security import PASSWORD_POLICY_MESSAGE, validate_password_policy @@ -72,6 +73,27 @@ class PasswordPolicyTests(unittest.TestCase): self.assertEqual(validate_password_policy(" password123 "), "password123") +class NetworkSecurityTests(unittest.TestCase): + def test_notification_targets_reject_loopback(self) -> None: + with self.assertRaisesRegex(ValueError, "Private or local notification targets are not allowed."): + validate_notification_target_url("http://127.0.0.1:8080/webhook") + + def test_forwarded_headers_require_trusted_proxy(self) -> None: + original_enabled = settings.magent_proxy_enabled + original_trust = settings.magent_proxy_trust_forwarded_headers + original_proxies = settings.magent_proxy_trusted_proxies + settings.magent_proxy_enabled = True + settings.magent_proxy_trust_forwarded_headers = True + settings.magent_proxy_trusted_proxies = "127.0.0.1,::1" + try: + self.assertTrue(request_trusts_forwarded_headers("127.0.0.1")) + self.assertFalse(request_trusts_forwarded_headers("203.0.113.10")) + finally: + settings.magent_proxy_enabled = original_enabled + settings.magent_proxy_trust_forwarded_headers = original_trust + settings.magent_proxy_trusted_proxies = original_proxies + + class DatabaseEmailTests(TempDatabaseMixin, unittest.TestCase): def test_set_user_email_is_case_insensitive(self) -> None: created = db.create_user_if_missing( diff --git a/frontend/app/lib/auth.ts b/frontend/app/lib/auth.ts index 99612bf..c89bfbb 100644 --- a/frontend/app/lib/auth.ts +++ b/frontend/app/lib/auth.ts @@ -1,27 +1,53 @@ +const AUTH_STATE_COOKIE = 'magent_logged_in' + export const getApiBase = () => process.env.NEXT_PUBLIC_API_BASE ?? '/api' -export const getToken = () => { - if (typeof window === 'undefined') return null - return window.localStorage.getItem('magent_token') +const setCookie = (name: string, value: string, maxAgeSeconds: number) => { + if (typeof document === 'undefined') return + document.cookie = `${name}=${value}; Max-Age=${maxAgeSeconds}; Path=/; SameSite=Lax` } -export const setToken = (token: string) => { - if (typeof window === 'undefined') return - window.localStorage.setItem('magent_token', token) +const clearCookie = (name: string) => { + if (typeof document === 'undefined') return + document.cookie = `${name}=; Max-Age=0; Path=/; SameSite=Lax` +} + +export const getToken = () => { + if (typeof document === 'undefined') return null + const cookies = document.cookie.split(';').map((entry) => entry.trim()) + const marker = cookies.find((entry) => entry.startsWith(`${AUTH_STATE_COOKIE}=`)) + if (!marker) return null + const [, value] = marker.split('=', 2) + return value || null +} + +export const setToken = (_token: string) => { + setCookie(AUTH_STATE_COOKIE, '1', 60 * 60 * 12) } export const clearToken = () => { + clearCookie(AUTH_STATE_COOKIE) if (typeof window === 'undefined') return - window.localStorage.removeItem('magent_token') + const baseUrl = getApiBase() + void fetch(`${baseUrl}/auth/logout`, { + method: 'POST', + credentials: 'include', + keepalive: true, + }).catch(() => undefined) +} + +export const logout = async () => { + const baseUrl = getApiBase() + clearCookie(AUTH_STATE_COOKIE) + await fetch(`${baseUrl}/auth/logout`, { + method: 'POST', + credentials: 'include', + }) } export const authFetch = (input: RequestInfo | URL, init?: RequestInit) => { - const token = getToken() const headers = new Headers(init?.headers || {}) - if (token) { - headers.set('Authorization', `Bearer ${token}`) - } - return fetch(input, { ...init, headers }) + return fetch(input, { ...init, headers, credentials: 'include' }) } export const getEventStreamToken = async () => { diff --git a/frontend/app/login/page.tsx b/frontend/app/login/page.tsx index 95acf27..7cae285 100644 --- a/frontend/app/login/page.tsx +++ b/frontend/app/login/page.tsx @@ -42,13 +42,14 @@ export default function LoginPage() { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, body, + credentials: 'include', }) if (!response.ok) { throw new Error('Login failed') } const data = await response.json() - if (data?.access_token) { - setToken(data.access_token) + if (data?.authenticated) { + setToken('cookie') if (typeof window !== 'undefined') { window.location.href = '/' return diff --git a/frontend/app/signup/page.tsx b/frontend/app/signup/page.tsx index fdc9c8e..8ced8f6 100644 --- a/frontend/app/signup/page.tsx +++ b/frontend/app/signup/page.tsx @@ -106,6 +106,7 @@ function SignupPageContent() { const response = await fetch(`${baseUrl}/auth/signup`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, + credentials: 'include', body: JSON.stringify({ invite_code: inviteCode, username: username.trim(), @@ -117,12 +118,12 @@ function SignupPageContent() { throw new Error(text || 'Sign-up failed') } const data = await response.json() - if (data?.access_token) { - setToken(data.access_token) + if (data?.authenticated) { + setToken('cookie') window.location.href = '/' return } - throw new Error('Sign-up did not return a token') + throw new Error('Sign-up did not complete') } catch (err) { console.error(err) setError(err instanceof Error ? err.message : 'Unable to create account.') diff --git a/frontend/app/ui/HeaderIdentity.tsx b/frontend/app/ui/HeaderIdentity.tsx index 5901a88..0ba8025 100644 --- a/frontend/app/ui/HeaderIdentity.tsx +++ b/frontend/app/ui/HeaderIdentity.tsx @@ -1,7 +1,7 @@ 'use client' import { useEffect, useState } from 'react' -import { authFetch, clearToken, getApiBase, getToken } from '../lib/auth' +import { authFetch, clearToken, getApiBase, getToken, logout } from '../lib/auth' export default function HeaderIdentity() { const [identity, setIdentity] = useState<{ username: string; role?: string } | null>(null) @@ -49,7 +49,8 @@ export default function HeaderIdentity() { const label = `${identity.username}${identity.role ? ` (${identity.role})` : ''}` const initial = identity.username.slice(0, 1).toUpperCase() - const signOut = () => { + const signOut = async () => { + await logout().catch(() => undefined) clearToken() if (typeof window !== 'undefined') { window.location.href = '/login' @@ -83,7 +84,7 @@ export default function HeaderIdentity() { setOpen(false)}> Changelog -