Security hardening and first-run/portability improvements
Security (P0): - Remove committed session-secret default; auto-generate and persist a random secret to the config volume when SESSION_SECRET is unset (prevents forgeable session cookies / auth bypass). - Validate playlist names to a safe charset and render sldl configs via literal Python substitution instead of sed (closes a command-injection and path-traversal path through playlist names). - shlex-quote credential values written to shell-sourced env files, and strip newlines from values patched into .conf files. - Render playlist .conf files 0600; warn at startup if the master key is co-located with the config volume; document keeping it separate. Portability: - Configurable timezone via TZ (default UTC) instead of hardcoded Edmonton. - Remove personal defaults (navidrome user "andrew", ephemeral.club URLs). - Ship generic example seeds; move the cross-album dedup keep-list and the legacy playlist import to editable config files; drop the personal _upgrade.csv. - Generic VPN reference in docker-compose.snippet.yml. First-run experience: - Redirect to /setup instead of 500 when OIDC is unconfigured; surface a missing master key inline; entrypoint exits with an actionable message when the config folder is not writable. - Add unauthenticated /health (JSON) and /setup (checklist) diagnostics. Docs: - Write docs/ARCHITECTURE.md and docs/MIGRATION.md (previously referenced but missing); expand README with ownership, backups, advanced settings, and migration guidance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -7,12 +7,18 @@ from sqlalchemy import select
|
||||
from app.db import SessionLocal
|
||||
from app.models import User
|
||||
from app.security.oidc import oauth
|
||||
from app.services import diagnostics
|
||||
|
||||
router = APIRouter(prefix="/auth", tags=["auth"])
|
||||
|
||||
|
||||
@router.get("/login")
|
||||
async def login(request: Request):
|
||||
# If OIDC isn't configured, the client was never registered and calling it
|
||||
# would raise an opaque 500. Send the operator to the setup check instead,
|
||||
# which spells out exactly which variables are missing.
|
||||
if not diagnostics.oidc_configured():
|
||||
return RedirectResponse(url="/setup")
|
||||
redirect_uri = request.url_for("auth_callback")
|
||||
return await oauth.pocketid.authorize_redirect(request, redirect_uri)
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ from fastapi.responses import RedirectResponse
|
||||
from fastapi.templating import Jinja2Templates
|
||||
|
||||
from app.db import get_db
|
||||
from app.security import crypto
|
||||
from app.security.deps import require_auth
|
||||
from app.services import credential_service
|
||||
|
||||
@@ -31,6 +32,7 @@ async def credentials_index(request: Request, user: dict = Depends(require_auth)
|
||||
"core_scopes": credential_service.CORE_SCOPES,
|
||||
"configured": configured,
|
||||
"enabled": enabled,
|
||||
"key_present": crypto.key_present(),
|
||||
},
|
||||
)
|
||||
|
||||
@@ -47,7 +49,13 @@ async def save_credentials(
|
||||
for field, value in form.items()
|
||||
if field in credential_service.SCOPE_FIELDS[scope] and value != ""
|
||||
}
|
||||
credential_service.set_credentials(db, scope, values)
|
||||
try:
|
||||
credential_service.set_credentials(db, scope, values)
|
||||
except RuntimeError:
|
||||
# Almost always the encryption key is missing, so the value can't be
|
||||
# encrypted. Redirect back; the page shows a key-missing banner with
|
||||
# exactly what to do.
|
||||
return RedirectResponse(url="/settings/credentials", status_code=303)
|
||||
return RedirectResponse(url="/settings/credentials", status_code=303)
|
||||
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@ from sqlalchemy import select
|
||||
from app.db import get_db
|
||||
from app.models import JobRun
|
||||
from app.security.deps import require_auth
|
||||
from app.services import beets_service, credential_service, network_service, playlist_service, scheduler_service
|
||||
from app.services import beets_service, credential_service, diagnostics, network_service, playlist_service, scheduler_service
|
||||
|
||||
router = APIRouter(tags=["dashboard"])
|
||||
templates = Jinja2Templates(directory="app/templates")
|
||||
@@ -34,11 +34,13 @@ async def dashboard(request: Request, user: dict = Depends(require_auth), db=Dep
|
||||
)
|
||||
recent_tracks = beets_service.recently_added(time.time() - _RECENT_TRACKS_WINDOW_SECONDS)
|
||||
public_ip = await asyncio.to_thread(network_service.public_ip)
|
||||
setup_warnings = [c for c in diagnostics.checks() if not c["ok"] and (c["critical"] or c.get("warn"))]
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"dashboard.html",
|
||||
{
|
||||
"user": user,
|
||||
"setup_warnings": setup_warnings,
|
||||
"stats": stats,
|
||||
"total_size_human": _human_bytes(stats.get("total_bytes", 0)),
|
||||
"playlist_count": len(playlists),
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
from fastapi import APIRouter, Request
|
||||
from fastapi.responses import JSONResponse
|
||||
from fastapi.templating import Jinja2Templates
|
||||
|
||||
from app.services import diagnostics
|
||||
|
||||
router = APIRouter(tags=["health"])
|
||||
templates = Jinja2Templates(directory="app/templates")
|
||||
|
||||
|
||||
@router.get("/health")
|
||||
async def health():
|
||||
"""Unauthenticated JSON health check. 200 when everything critical is
|
||||
configured, 503 when something critical is missing so an uptime monitor
|
||||
notices. The body always lists every check either way."""
|
||||
result = diagnostics.summary()
|
||||
status_code = 200 if result["status"] == "ok" else 503
|
||||
return JSONResponse(result, status_code=status_code)
|
||||
|
||||
|
||||
@router.get("/setup")
|
||||
async def setup(request: Request):
|
||||
"""Unauthenticated, human-readable version of the same checks, so a fresh
|
||||
operator can see what still needs configuring before login works."""
|
||||
result = diagnostics.summary()
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
"setup.html",
|
||||
{"status": result["status"], "checks": result["checks"]},
|
||||
)
|
||||
@@ -28,13 +28,16 @@ async def create_playlist(
|
||||
user: dict = Depends(require_auth),
|
||||
db=Depends(get_db),
|
||||
):
|
||||
playlist_service.create(
|
||||
db,
|
||||
name=name,
|
||||
spotify_url=spotify_url,
|
||||
cron_expr=playlist_service.time_to_cron(sync_time),
|
||||
no_m3u=no_m3u,
|
||||
)
|
||||
try:
|
||||
playlist_service.create(
|
||||
db,
|
||||
name=name,
|
||||
spotify_url=spotify_url,
|
||||
cron_expr=playlist_service.time_to_cron(sync_time),
|
||||
no_m3u=no_m3u,
|
||||
)
|
||||
except ValueError as exc:
|
||||
raise HTTPException(400, str(exc))
|
||||
return RedirectResponse(url="/playlists", status_code=303)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user