From 154f415f923f6bb0299b2b0308c452f336521fdd Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 10 May 2026 20:28:27 -0400 Subject: [PATCH] fix(server,web): auth-gate client APK + version endpoints + per-user rate limit (#397) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the bandwidth-abuse vector on the in-app update flow. Both endpoints now sit inside the authed.Group; APK additionally gets a 60s/user rate limit to suppress accidental hammering or scripted abuse. ### Server - internal/api/client_assets.go: - clientAPKAllowDownload(): in-memory map[userID]time.Time under a mutex. Returns 0 (allow) or wait duration (block). - handleClientAPK reads user from context, checks the limit, returns 429 + Retry-After header if blocked. - testResetClientAPKRateLimit() lets tests start clean. - internal/api/api.go: routes moved from the root /api group into the authed.Group block (alongside /quarantine, /requests, etc.). - Tests: added TestClientAPK_401WhenUnauthenticated and TestClientAPK_RateLimit_429OnRapidSecondCall (also verifies different user gets a fresh slot). Existing tests updated to use authedRequest() helper. ### Web - MobileAppDownload.svelte: switched from bare fetch (no credentials) to api.get<>() which carries the session cookie. 404 / 401 / network errors all silently hide the download row. - Removed the login-page mount entirely — pre-auth surfaces should never show this. Settings → Mobile app section keeps it for logged-in users. Flutter unaffected: dio's Bearer interceptor already attaches the token, and the polling only fires once the post-login shell mounts the banner widget. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/api/api.go | 11 +-- internal/api/client_assets.go | 69 +++++++++++++++- internal/api/client_assets_test.go | 79 ++++++++++++++++++- .../lib/components/MobileAppDownload.svelte | 20 +++-- web/src/routes/login/+page.svelte | 5 -- 5 files changed, 157 insertions(+), 27 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index b9e2d204..e4cde55f 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -52,11 +52,6 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev api.Post("/auth/register", h.handleRegister) api.Post("/auth/forgot-password", h.handleForgotPassword) api.Post("/auth/reset-password", h.handleResetPassword) - - // Self-hosted in-app update channel (#397). Unauthenticated so - // the install flow doesn't depend on a live session. - api.Get("/client/version", h.handleClientVersion) - api.Get("/client/apk", h.handleClientAPK) api.Group(func(authed chi.Router) { authed.Use(auth.RequireUser(pool)) authed.Post("/auth/logout", h.handleLogout) @@ -106,6 +101,12 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Delete("/quarantine/{track_id}", h.handleUnflag) authed.Get("/quarantine/mine", h.handleListMyQuarantine) + // Self-hosted in-app update channel (#397). Auth-gated to + // prevent anonymous bandwidth abuse on the APK stream; + // /apk additionally per-user rate-limited. + authed.Get("/client/version", h.handleClientVersion) + authed.Get("/client/apk", h.handleClientAPK) + authed.Route("/admin", func(admin chi.Router) { admin.Use(auth.RequireAdmin()) admin.Get("/lidarr/config", h.handleGetLidarrConfig) diff --git a/internal/api/client_assets.go b/internal/api/client_assets.go index 10bfff77..1e25dc69 100644 --- a/internal/api/client_assets.go +++ b/internal/api/client_assets.go @@ -5,25 +5,42 @@ package api // app store. CI sequencing bakes the APK + sidecar version file into // /app/client/ at image build time. // -// Both endpoints are unauthenticated — install flow shouldn't depend -// on a live session, and the URL is hard to guess at scale. +// Both endpoints are authenticated — the bandwidth cost of the APK +// (~30-60 MB) makes anonymous access an abuse vector. The Flutter +// client's polling only fires after login (banner mounts in the post- +// login shell), so this gate is invisible to the actual update flow. +// +// /api/client/apk additionally rate-limits per user to a single +// download every 60s. Real install flows fire one download per +// update; anything tighter is scripted/abusive. // // Returns 404 gracefully when the APK isn't present (dev environments, // pre-CI-wiring); the Flutter client treats 404 as "no update channel -// available" and falls back to the manual download. +// available." import ( "errors" "net/http" "os" "path/filepath" + "strconv" "strings" + "sync" + "time" + + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) const ( defaultClientAPKDir = "/app/client" clientAPKFilename = "minstrel.apk" clientVersionFile = "minstrel.apk.version" + + // clientAPKMinInterval throttles per-user APK downloads. 60s is + // generous for a real install flow (one download) and tight enough + // to suppress accidental hammering or scripted abuse. + clientAPKMinInterval = 60 * time.Second ) // clientAPKDir resolves the directory holding the bundled APK. Env @@ -36,6 +53,37 @@ func clientAPKDir() string { return defaultClientAPKDir } +// clientAPKLastDownload tracks the last APK-download timestamp per +// user id (UUID hex string). Cheap in-memory map under mutex; a single +// household has at most a handful of users so the map never grows. +// Reset on process restart, which is fine — abuse protection, not +// audit. testResetClientAPKRateLimit() lets tests start clean. +var ( + clientAPKLastDownload = map[string]time.Time{} + clientAPKLastDownloadMu sync.Mutex +) + +func testResetClientAPKRateLimit() { + clientAPKLastDownloadMu.Lock() + defer clientAPKLastDownloadMu.Unlock() + clientAPKLastDownload = map[string]time.Time{} +} + +// clientAPKAllowDownload checks + updates the per-user rate-limit +// state. Returns the wait duration if blocked, or 0 if allowed. +func clientAPKAllowDownload(userID string, now time.Time) time.Duration { + clientAPKLastDownloadMu.Lock() + defer clientAPKLastDownloadMu.Unlock() + if last, ok := clientAPKLastDownload[userID]; ok { + elapsed := now.Sub(last) + if elapsed < clientAPKMinInterval { + return clientAPKMinInterval - elapsed + } + } + clientAPKLastDownload[userID] = now + return 0 +} + type clientVersionResponse struct { Version string `json:"version"` APKURL string `json:"apk_url"` @@ -77,8 +125,21 @@ func (h *handlers) handleClientVersion(w http.ResponseWriter, _ *http.Request) { } // handleClientAPK streams the bundled APK with the correct -// Content-Type so Android's PackageInstaller accepts it. +// Content-Type so Android's PackageInstaller accepts it. Per-user +// rate-limited (clientAPKMinInterval). func (h *handlers) handleClientAPK(w http.ResponseWriter, r *http.Request) { + user, ok := auth.UserFromContext(r.Context()) + if !ok { + http.Error(w, `{"error":{"code":"unauthenticated","message":"login required"}}`, http.StatusUnauthorized) + return + } + userID := syncpkg.FormatUUID(user.ID) + if wait := clientAPKAllowDownload(userID, time.Now()); wait > 0 { + w.Header().Set("Retry-After", strconv.Itoa(int(wait.Seconds())+1)) + http.Error(w, `{"error":{"code":"rate_limited","message":"too many downloads; try again shortly"}}`, http.StatusTooManyRequests) + return + } + apkPath := filepath.Join(clientAPKDir(), clientAPKFilename) f, err := os.Open(apkPath) if errors.Is(err, os.ErrNotExist) { diff --git a/internal/api/client_assets_test.go b/internal/api/client_assets_test.go index 3323cc16..f85f49eb 100644 --- a/internal/api/client_assets_test.go +++ b/internal/api/client_assets_test.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "io" "log/slog" @@ -9,8 +10,33 @@ import ( "os" "path/filepath" "testing" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) +// authedRequest builds a request with a fake user in context, mirroring +// what the auth.RequireUser middleware injects in production. +// userIDByte fills every byte of the user's UUID, so two requests with +// different userIDByte values get distinct rate-limit slots. +func authedRequest(method, path string, userIDByte byte) *http.Request { + user := dbq.User{ + ID: pgtype.UUID{ + Bytes: [16]byte{userIDByte, userIDByte, userIDByte, userIDByte, + userIDByte, userIDByte, userIDByte, userIDByte, + userIDByte, userIDByte, userIDByte, userIDByte, + userIDByte, userIDByte, userIDByte, userIDByte}, + Valid: true, + }, + Username: "tester", + } + r := httptest.NewRequest(method, path, nil) + ctx := context.WithValue(r.Context(), auth.UserCtxKeyForTest(), user) + return r.WithContext(ctx) +} + // withClientAPKDir points the handlers at a fresh temp dir for each // test and restores the env var on cleanup. Returns the dir. func withClientAPKDir(t *testing.T) string { @@ -81,11 +107,26 @@ func TestClientVersion_200WithBothFiles(t *testing.T) { } } -func TestClientAPK_404WhenMissing(t *testing.T) { +func TestClientAPK_401WhenUnauthenticated(t *testing.T) { withClientAPKDir(t) + testResetClientAPKRateLimit() h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))} rr := httptest.NewRecorder() + // No user in context — middleware would block in prod; handler also + // rejects defensively in case the route ever lands outside the + // authed group by accident. h.handleClientAPK(rr, httptest.NewRequest(http.MethodGet, "/api/client/apk", nil)) + if rr.Code != http.StatusUnauthorized { + t.Errorf("want 401, got %d", rr.Code) + } +} + +func TestClientAPK_404WhenMissing(t *testing.T) { + withClientAPKDir(t) + testResetClientAPKRateLimit() + h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))} + rr := httptest.NewRecorder() + h.handleClientAPK(rr, authedRequest(http.MethodGet, "/api/client/apk", 0x01)) if rr.Code != http.StatusNotFound { t.Errorf("want 404, got %d", rr.Code) } @@ -93,13 +134,14 @@ func TestClientAPK_404WhenMissing(t *testing.T) { func TestClientAPK_StreamsWithCorrectContentType(t *testing.T) { dir := withClientAPKDir(t) + testResetClientAPKRateLimit() body := []byte("PK\x03\x04 fake apk bytes") if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), body, 0o644); err != nil { t.Fatal(err) } h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))} rr := httptest.NewRecorder() - h.handleClientAPK(rr, httptest.NewRequest(http.MethodGet, "/api/client/apk", nil)) + h.handleClientAPK(rr, authedRequest(http.MethodGet, "/api/client/apk", 0x02)) if rr.Code != http.StatusOK { t.Fatalf("want 200, got %d", rr.Code) } @@ -113,3 +155,36 @@ func TestClientAPK_StreamsWithCorrectContentType(t *testing.T) { t.Errorf("body bytes mismatch") } } + +func TestClientAPK_RateLimit_429OnRapidSecondCall(t *testing.T) { + dir := withClientAPKDir(t) + testResetClientAPKRateLimit() + if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), []byte("apk"), 0o644); err != nil { + t.Fatal(err) + } + h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))} + + // First call from user 0x03 — succeeds. + rr1 := httptest.NewRecorder() + h.handleClientAPK(rr1, authedRequest(http.MethodGet, "/api/client/apk", 0x03)) + if rr1.Code != http.StatusOK { + t.Fatalf("first call: want 200, got %d", rr1.Code) + } + + // Immediate second call from same user — rate-limited. + rr2 := httptest.NewRecorder() + h.handleClientAPK(rr2, authedRequest(http.MethodGet, "/api/client/apk", 0x03)) + if rr2.Code != http.StatusTooManyRequests { + t.Fatalf("second call: want 429, got %d", rr2.Code) + } + if got := rr2.Header().Get("Retry-After"); got == "" { + t.Errorf("expected Retry-After header on 429 response") + } + + // Different user — not rate-limited. + rr3 := httptest.NewRecorder() + h.handleClientAPK(rr3, authedRequest(http.MethodGet, "/api/client/apk", 0x04)) + if rr3.Code != http.StatusOK { + t.Errorf("different user: want 200, got %d", rr3.Code) + } +} diff --git a/web/src/lib/components/MobileAppDownload.svelte b/web/src/lib/components/MobileAppDownload.svelte index 6c4e49d8..67734517 100644 --- a/web/src/lib/components/MobileAppDownload.svelte +++ b/web/src/lib/components/MobileAppDownload.svelte @@ -1,15 +1,15 @@ diff --git a/web/src/routes/login/+page.svelte b/web/src/routes/login/+page.svelte index 3293e968..82a2caf1 100644 --- a/web/src/routes/login/+page.svelte +++ b/web/src/routes/login/+page.svelte @@ -4,7 +4,6 @@ import { goto } from '$app/navigation'; import { login } from '$lib/auth/store.svelte'; import type { ApiError } from '$lib/api/client'; - import MobileAppDownload from '$lib/components/MobileAppDownload.svelte'; let username = $state(''); let password = $state(''); @@ -89,9 +88,5 @@ Don't have an account? Register

- -
- -