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