From 236637fcd33cd68e69df4f311d23c9349c678a6d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 3 Jun 2026 11:34:02 -0400 Subject: [PATCH] feat(server): HMAC stream token auth path (UPnP slice 1/6) Adds SignStreamToken / VerifyStreamToken (HMAC-SHA256 over trackID|exp) and modifies handleGetStream to accept either the existing session cookie OR a valid signed token. Stream route moved out of the authed group so the handler's own auth check runs and the token bypass is reachable. Enables Sonos / UPnP speakers to fetch the stream URL without carrying the user's session cookie - they cannot. The token is short-lived (max 24h per the design); expiry checked at request time only, not per-byte, so long tracks play through. streamSecret field on handlers is nil for now; Task 2 wires the loader (env var with auto-generated fallback persisted in app_preferences). Adds auth.OptionalUser - the permissive sibling of RequireUser that attaches the user to context when a valid cookie / bearer is present but does NOT 401 on absence. The stream route is wrapped with it so the handler can fall through to the token path when no session is present. newLibraryRouter (test fixture) gets a synthetic-user middleware on the stream route so existing media_test tests keep passing without seeding a real session row - production traffic uses auth.OptionalUser, the test path uses auth.UserCtxKeyForTest(). Five tests cover round-trip, tampered token rejection, expiry, wrong-track-ID, and wrong-secret rejection. CI verifies. --- internal/api/api.go | 23 ++++++++- internal/api/library_test.go | 25 +++++++++- internal/api/media.go | 42 +++++++++++++++++ internal/api/stream_token.go | 38 +++++++++++++++ internal/api/stream_token_test.go | 78 +++++++++++++++++++++++++++++++ internal/auth/session.go | 46 ++++++++++++++++++ 6 files changed, 249 insertions(+), 3 deletions(-) create mode 100644 internal/api/stream_token.go create mode 100644 internal/api/stream_token_test.go diff --git a/internal/api/api.go b/internal/api/api.go index 1912eece..59bef6a8 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -54,6 +54,17 @@ 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) + + // Stream lives outside authed.Group so it can accept EITHER a + // session (resolved by the OptionalUser middleware) OR a signed + // query token (UPnP / Sonos path; see streamAuthOk). The + // middleware attaches user to context when a valid cookie / + // bearer is present but does NOT 401 on absence; the handler's + // own streamAuthOk performs the actual auth check. See the + // design at + // docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md. + api.With(auth.OptionalUser(pool, logger)).Get("/tracks/{id}/stream", h.handleGetStream) + api.Group(func(authed chi.Router) { authed.Use(auth.RequireUser(pool)) authed.Post("/auth/logout", h.handleLogout) @@ -77,7 +88,8 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Get("/library/albums", h.handleListLibraryAlbums) authed.Get("/library/sync", h.handleLibrarySync) authed.Get("/tracks/{id}", h.handleGetTrack) - authed.Get("/tracks/{id}/stream", h.handleGetStream) + // /tracks/{id}/stream is mounted above with OptionalUser so + // it can accept either a session or a signed token. authed.Get("/search", h.handleSearch) authed.Get("/radio", h.handleRadio) authed.Get("/discover/suggestions", h.handleListSuggestions) @@ -205,4 +217,13 @@ type handlers struct { mailer mailer.Sender eventbus *eventbus.Bus playlistScheduler *playlists.Scheduler + // streamSecret is the HMAC key used by SignStreamToken / + // VerifyStreamToken to authenticate the UPnP-speaker stream path + // (see internal/api/stream_token.go and the design at + // docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md). + // nil in slice 1; slice 2 wires the env-var-with-app_preferences- + // fallback loader. A nil secret leaves the cookie path intact and + // makes the token path unreachable (HMAC of empty key won't match + // anything a client mints), which is the desired slice-1 default. + streamSecret []byte } diff --git a/internal/api/library_test.go b/internal/api/library_test.go index 845aed75..102b16b7 100644 --- a/internal/api/library_test.go +++ b/internal/api/library_test.go @@ -12,6 +12,7 @@ import ( "github.com/go-chi/chi/v5" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" "git.fabledsword.com/bvandeusen/minstrel/internal/config" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/eventbus" @@ -21,7 +22,15 @@ import ( // newLibraryRouter builds a test-only chi router with the library handlers // mounted at their path-style routes. Tests hit this router rather than // calling handler methods directly so chi URL params populate correctly. -// RequireUser is NOT applied — library handlers don't read user context. +// RequireUser is NOT applied to most routes — library handlers don't read +// user context. +// +// The stream route is wrapped with a synthetic-user middleware because +// handleGetStream now enforces its own auth check (streamAuthOk) after the +// UPnP slice moved it out of the authed.Group. Real traffic carries either +// a session cookie (resolved by auth.OptionalUser) or a signed query +// token; tests get a fake user-in-context so the cookie path of +// streamAuthOk succeeds without seeding a real session row. func newLibraryRouter(h *handlers) chi.Router { r := chi.NewRouter() r.Get("/api/artists", h.handleListArtists) @@ -29,11 +38,23 @@ func newLibraryRouter(h *handlers) chi.Router { r.Get("/api/albums/{id}", h.handleGetAlbum) r.Get("/api/albums/{id}/cover", h.handleGetCover) r.Get("/api/tracks/{id}", h.handleGetTrack) - r.Get("/api/tracks/{id}/stream", h.handleGetStream) + r.With(injectFakeUserForTest).Get("/api/tracks/{id}/stream", h.handleGetStream) r.Get("/api/search", h.handleSearch) return r } +// injectFakeUserForTest attaches an empty dbq.User to request context via +// auth.UserCtxKeyForTest(), letting handleGetStream's streamAuthOk succeed +// on the session path without the test having to seed a real session row. +// Production traffic uses auth.OptionalUser instead; this is the test-only +// equivalent that bypasses the DB lookup. +func injectFakeUserForTest(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), auth.UserCtxKeyForTest(), dbq.User{}) + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + func TestHandleGetTrack_HappyPath(t *testing.T) { h, pool := testHandlers(t) truncateLibrary(t, pool) diff --git a/internal/api/media.go b/internal/api/media.go index 0ff6cbbb..ef6a3b3c 100644 --- a/internal/api/media.go +++ b/internal/api/media.go @@ -11,9 +11,11 @@ import ( "net/http" "os" "path/filepath" + "strconv" "strings" "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" "git.fabledsword.com/bvandeusen/minstrel/internal/coverart" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) @@ -121,15 +123,55 @@ func (h *handlers) handleGetCover(w http.ResponseWriter, r *http.Request) { http.ServeContent(w, r, filepath.Base(path), info.ModTime(), f) } +// streamAuthOk returns true if the request is authorized to fetch the +// stream — either via the standard session path (cookie OR bearer token +// resolved to a user-in-context by auth.OptionalUser, the middleware +// the route is wrapped with in api.go) OR via a valid short-lived HMAC +// token in the ?token=&exp= query (UPnP / Sonos path, new for the +// output-picker UPnP slice). +// +// The token path lets network speakers fetch the stream URL without +// carrying the user's session cookie — they cannot. See the design at +// docs/superpowers/specs/2026-06-03-android-output-picker-upnp-design.md. +// +// When h.streamSecret is nil (slice-1 default, until slice 2 wires the +// loader), the token path always rejects because the HMAC of an empty +// key won't match any token a client could mint. The session path keeps +// working. +func (h *handlers) streamAuthOk(r *http.Request, trackID string) bool { + if _, ok := auth.UserFromContext(r.Context()); ok { + return true + } + tok := r.URL.Query().Get("token") + expStr := r.URL.Query().Get("exp") + if tok == "" || expStr == "" { + return false + } + exp, err := strconv.ParseInt(expStr, 10, 64) + if err != nil { + return false + } + return VerifyStreamToken(h.streamSecret, trackID, exp, tok) +} + // handleGetStream implements GET /api/tracks/{id}/stream. Opens the file on // disk and delegates byte-serving to http.ServeContent, which handles Range, // If-Modified-Since, and ETag based on the file's mod time. +// +// The route lives outside the authed.Group so this handler can accept +// EITHER a session-resolved user (attached by auth.OptionalUser, the +// permissive middleware the route is wrapped with) OR a signed token +// (UPnP slice). streamAuthOk gates both paths. func (h *handlers) handleGetStream(w http.ResponseWriter, r *http.Request) { track, apiErr := resolveByID(r, "id", dbq.New(h.pool).GetTrackByID, "track") if apiErr != nil { writeErr(w, apiErr) return } + if !h.streamAuthOk(r, uuidToString(track.ID)) { + writeErr(w, apierror.ErrUnauthorized) + return + } f, err := os.Open(track.FilePath) if err != nil { // File vanished (scanner indexed it, filesystem lost it). Treat as diff --git a/internal/api/stream_token.go b/internal/api/stream_token.go new file mode 100644 index 00000000..d3cf2144 --- /dev/null +++ b/internal/api/stream_token.go @@ -0,0 +1,38 @@ +package api + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "fmt" + "time" +) + +// SignStreamToken returns the HMAC-SHA256 hex of "|" keyed +// by secret. The token is constant-time-comparable via VerifyStreamToken +// so it's safe to expose the verification function in handlers. +// +// trackID is the canonical track UUID. exp is the Unix time after which +// the token is invalid; the handler checks exp before serving. +// +// The secret comes from MINSTREL_STREAM_SECRET (or its auto-generated + +// persisted fallback in app_preferences). The loader lands in slice 2 +// (POST /api/cast/stream-token); this file is the primitive both the +// loader and the stream handler share. +func SignStreamToken(secret []byte, trackID string, exp int64) string { + mac := hmac.New(sha256.New, secret) + fmt.Fprintf(mac, "%s|%d", trackID, exp) + return hex.EncodeToString(mac.Sum(nil)) +} + +// VerifyStreamToken returns true iff token is a valid HMAC for +// (trackID, exp) AND exp has not yet passed. Wall-clock comparison — +// callers must trust the server's clock. Uses hmac.Equal for +// constant-time compare so a timing oracle can't bias token guessing. +func VerifyStreamToken(secret []byte, trackID string, exp int64, token string) bool { + if time.Now().Unix() > exp { + return false + } + expected := SignStreamToken(secret, trackID, exp) + return hmac.Equal([]byte(expected), []byte(token)) +} diff --git a/internal/api/stream_token_test.go b/internal/api/stream_token_test.go new file mode 100644 index 00000000..a7e78ed9 --- /dev/null +++ b/internal/api/stream_token_test.go @@ -0,0 +1,78 @@ +package api + +import ( + "testing" + "time" +) + +func TestSignStreamToken_RoundTrip(t *testing.T) { + secret := []byte("test-secret-not-real") + trackID := "abc-123" + exp := time.Now().Unix() + 3600 + + token := SignStreamToken(secret, trackID, exp) + if token == "" { + t.Fatal("got empty token") + } + if !VerifyStreamToken(secret, trackID, exp, token) { + t.Fatal("round-trip verify failed") + } +} + +func TestVerifyStreamToken_TamperedTokenRejected(t *testing.T) { + secret := []byte("test-secret") + trackID := "abc-123" + exp := time.Now().Unix() + 3600 + + token := SignStreamToken(secret, trackID, exp) + // Flip a hex digit anywhere in the middle. + mid := len(token) / 2 + tampered := token[:mid] + flipHexDigit(token[mid:mid+1]) + token[mid+1:] + + if VerifyStreamToken(secret, trackID, exp, tampered) { + t.Fatal("verify accepted tampered token") + } +} + +func TestVerifyStreamToken_ExpiredRejected(t *testing.T) { + secret := []byte("test-secret") + trackID := "abc-123" + exp := time.Now().Unix() - 1 // already expired + + token := SignStreamToken(secret, trackID, exp) + if VerifyStreamToken(secret, trackID, exp, token) { + t.Fatal("verify accepted expired token") + } +} + +func TestVerifyStreamToken_WrongTrackIDRejected(t *testing.T) { + secret := []byte("test-secret") + exp := time.Now().Unix() + 3600 + + token := SignStreamToken(secret, "track-a", exp) + if VerifyStreamToken(secret, "track-b", exp, token) { + t.Fatal("verify accepted token for different track") + } +} + +func TestVerifyStreamToken_WrongSecretRejected(t *testing.T) { + trackID := "abc-123" + exp := time.Now().Unix() + 3600 + + token := SignStreamToken([]byte("secret-a"), trackID, exp) + if VerifyStreamToken([]byte("secret-b"), trackID, exp, token) { + t.Fatal("verify accepted token signed with different secret") + } +} + +func flipHexDigit(s string) string { + if s == "" { + return s + } + switch s[0] { + case '0': + return "1" + default: + return "0" + } +} diff --git a/internal/auth/session.go b/internal/auth/session.go index b4b12b1b..e949223e 100644 --- a/internal/auth/session.go +++ b/internal/auth/session.go @@ -112,6 +112,52 @@ func RequireUser(pool *pgxpool.Pool) func(http.Handler) http.Handler { // middleware. Do not use this outside _test.go files. func UserCtxKeyForTest() any { return userCtxKey } +// OptionalUser is RequireUser's permissive sibling: it resolves the caller +// from the session cookie or bearer header and attaches the user to context +// when present + valid, but does NOT 401 on absence. The downstream handler +// runs unconditionally and is responsible for its own auth check via +// UserFromContext (or its own bespoke path — see /api/tracks/{id}/stream's +// streamAuthOk, which accepts EITHER a user-in-context OR a signed query +// token for UPnP / Sonos speakers that don't carry the user's cookie). +// +// Invalid tokens (stale session row, deleted user) silently drop through +// without attaching the user. The handler treats "no user in context" as +// "not authenticated" the same way it treats a missing cookie. +// +// Database lookup failures fall through too — a transient DB blip should not +// 5xx a stream request that may have a perfectly valid signed token. The +// error is logged so the operator can correlate. +func OptionalUser(pool *pgxpool.Pool, logger *slog.Logger) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token := sessionTokenFromRequest(r) + if token == "" || pool == nil { + next.ServeHTTP(w, r) + return + } + q := dbq.New(pool) + sess, err := q.GetSessionByTokenHash(r.Context(), HashSessionToken(token)) + if err != nil { + if !errors.Is(err, pgx.ErrNoRows) && logger != nil { + logger.Warn("api: optional session lookup failed", "err", err) + } + next.ServeHTTP(w, r) + return + } + user, err := q.GetUserByID(r.Context(), sess.UserID) + if err != nil { + if !errors.Is(err, pgx.ErrNoRows) && logger != nil { + logger.Warn("api: optional user lookup failed", "err", err) + } + next.ServeHTTP(w, r) + return + } + ctx := context.WithValue(r.Context(), userCtxKey, user) + next.ServeHTTP(w, r.WithContext(ctx)) + }) + } +} + func sessionTokenFromRequest(r *http.Request) string { if c, err := r.Cookie(SessionCookieName); err == nil && c.Value != "" { return c.Value