From a3aa93534d4aa2f55aa8cd4491845ee6cf4181ee Mon Sep 17 00:00:00 2001 From: Puks The Pirate <120460627+Pukabyte@users.noreply.github.com> Date: Sat, 27 Jun 2026 04:27:37 +1200 Subject: [PATCH] fix(jellycompat): authenticate Android TV token-less direct play (#200) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(jellycompat): authenticate Android TV token-less direct play Stock Jellyfin Android TV ignores the api_key-bearing DirectStreamUrl returned from PlaybackInfo and builds its own direct-play URL with no auth header, no api_key/ApiKey query param, and no PlaySessionId. The media request arrives via the player's HTTP stack (okhttp) with auth_kind=none, so PlaybackSessionAuth 401s it — the client retries, falls back to a transcode that stalls, and surfaces "player error". Add a third fallback in PlaybackSessionAuth, scoped strictly to the direct-play video stream routes (/Videos/{id}/stream[.{container}]) via the chi route pattern so /Items/{id}/Download stays protected: anchor auth on the PlaybackSession negotiated during the preceding (already authenticated) PlaybackInfo, looked up by mediaSourceId when present (else the route item id), and resolve its CompatToken. Covered by tests: token-less direct play with a matching session succeeds, no matching session 401s, and Download is not loosened. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(jellycompat): require session item match + expand direct-play tests Address CodeRabbit review on PR #200: - Require the matched PlaybackSession's RouteItemID to equal the requested route item before authorizing, so a mediaSourceId cannot authorize a stream for a different item. - Seed the compat session in the 401 tests so they fail on route/session scoping rather than a missing session. - Table-drive the positive test across /Videos/{id}/stream and /Videos/{id}/stream.{container}, plus the route-item lookup branch when mediaSourceId is absent; add a cross-item denial test. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- internal/jellycompat/auth.go | 32 ++++ internal/jellycompat/auth_directplay_test.go | 171 +++++++++++++++++++ 2 files changed, 203 insertions(+) create mode 100644 internal/jellycompat/auth_directplay_test.go diff --git a/internal/jellycompat/auth.go b/internal/jellycompat/auth.go index 5dc34e28..2cdbc5e4 100644 --- a/internal/jellycompat/auth.go +++ b/internal/jellycompat/auth.go @@ -10,6 +10,7 @@ import ( "github.com/Silo-Server/silo-server/internal/auth" "github.com/Silo-Server/silo-server/internal/playback" + "github.com/go-chi/chi/v5" ) type sessionContextKey string @@ -249,6 +250,37 @@ func PlaybackSessionAuth(sessions *SessionStore, playbackStore *PlaybackSessionS return } } + + // Stock Jellyfin Android TV ignores the api_key-bearing DirectStreamUrl + // we return from PlaybackInfo and builds its own direct-play URL with no + // auth header, no api_key/ApiKey, and no PlaySessionId. Anchor auth on + // the PlaybackSession negotiated for this item: a successful PlaybackInfo + // already authenticated the user and registered a session holding the + // CompatToken. Scope this strictly to the direct-play video stream routes + // (NOT /Items/{id}/Download) via the chi route pattern, prefer matching + // on mediaSourceId when present, and require the matched session's + // RouteItemID to equal the requested item so a source id can't + // authorize a stream for a different item. + if playbackStore != nil { + switch chi.RouteContext(r.Context()).RoutePattern() { + case "/Videos/{id}/stream", "/Videos/{id}/stream.{container}": + routeItemID := chi.URLParam(r, "id") + if routeItemID != "" { + mediaSourceID := newCaseInsensitiveQuery(r.URL.Query()).Get("mediaSourceId") + lookupID := routeItemID + if mediaSourceID != "" { + lookupID = mediaSourceID + } + if playSession, _, found := playbackStore.FindByRoute("", lookupID); found && playSession.RouteItemID == routeItemID { + if session, ok := resolveCompatToken(r.Context(), sessions, keyAuth, playSession.CompatToken); ok { + serveWithSession(next, w, r, session) + return + } + } + } + } + } + writeError(w, http.StatusUnauthorized, "Unauthorized", "Missing authentication token") }) } diff --git a/internal/jellycompat/auth_directplay_test.go b/internal/jellycompat/auth_directplay_test.go new file mode 100644 index 00000000..9b2fa40f --- /dev/null +++ b/internal/jellycompat/auth_directplay_test.go @@ -0,0 +1,171 @@ +package jellycompat + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-chi/chi/v5" +) + +// directPlayRouter wraps the probe handler in PlaybackSessionAuth and mounts it +// on a chi router so RoutePattern() and URLParam("id") are populated exactly as +// they are in production. The same handler also fronts /Items/{id}/Download to +// prove the new token-less fallback does NOT leak into that route. +func directPlayRouter(t *testing.T, sessions *SessionStore, playback *PlaybackSessionStore, keyAuth *AdminAPIKeyAuthenticator, reached *bool) *chi.Mux { + t.Helper() + probe := PlaybackSessionAuth(sessions, playback, keyAuth)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *reached = true + w.WriteHeader(http.StatusOK) + })) + r := chi.NewRouter() + r.Handle("/Videos/{id}/stream", probe) + r.Handle("/Videos/{id}/stream.{container}", probe) + r.Handle("/Items/{id}/Download", probe) + return r +} + +// seedDirectPlaySession registers a resolvable compat session plus the +// PlaybackSession that PlaybackInfo would have negotiated for the item. +func seedDirectPlaySession(t *testing.T, clock func() time.Time) (*SessionStore, *PlaybackSessionStore) { + t.Helper() + sessions := NewSessionStore(time.Hour, clock) + if err := sessions.Put(Session{Token: "compat-tok", StreamAppUserID: 7}); err != nil { + t.Fatalf("put session: %v", err) + } + playback := NewPlaybackSessionStore(time.Hour, clock) + playback.Put(PlaybackSession{ + ID: "ps1", + CompatToken: "compat-tok", + RouteItemID: "item123", + MediaSources: []PlaybackMediaSource{{ID: "src9"}}, + }) + return sessions, playback +} + +// TestPlaybackSessionAuth_DirectPlayNoToken: stock Jellyfin Android TV requests +// the stream with no auth header, no api_key/ApiKey, and no PlaySessionId. The +// negotiated PlaybackSession for the item authorizes it — across both stream +// route patterns and both lookup branches (by mediaSourceId, and by route item +// id when mediaSourceId is absent). +func TestPlaybackSessionAuth_DirectPlayNoToken(t *testing.T) { + now := fixedNow() + clock := func() time.Time { return now } + + cases := []struct { + name string + url string + }{ + {"stream + mediaSourceId", "/Videos/item123/stream?static=true&mediaSourceId=src9&container=mkv"}, + {"stream.{container} + mediaSourceId", "/Videos/item123/stream.mkv?static=true&mediaSourceId=src9"}, + {"stream, route-item lookup (no mediaSourceId)", "/Videos/item123/stream?static=true&container=mkv"}, + {"stream.{container}, route-item lookup", "/Videos/item123/stream.mkv?static=true"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + sessions, playback := seedDirectPlaySession(t, clock) + var reached bool + router := directPlayRouter(t, sessions, playback, nil, &reached) + + req := httptest.NewRequest(http.MethodGet, tc.url, nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + if !reached { + t.Fatal("expected inner handler to be reached via PlaybackSession fallback") + } + }) + } +} + +// TestPlaybackSessionAuth_DirectPlayNoMatchingSession: with a resolvable compat +// session present but NO PlaybackSession negotiated for the requested item, the +// token-less request stays a 401 (proves the playback-session match gates it, +// not a missing compat session). +func TestPlaybackSessionAuth_DirectPlayNoMatchingSession(t *testing.T) { + now := fixedNow() + clock := func() time.Time { return now } + sessions := NewSessionStore(time.Hour, clock) + if err := sessions.Put(Session{Token: "compat-tok", StreamAppUserID: 7}); err != nil { + t.Fatalf("put session: %v", err) + } + playback := NewPlaybackSessionStore(time.Hour, clock) + // Session exists, but for a different item id and source id. + playback.Put(PlaybackSession{ID: "ps1", CompatToken: "compat-tok", RouteItemID: "other"}) + + var reached bool + router := directPlayRouter(t, sessions, playback, nil, &reached) + + req := httptest.NewRequest(http.MethodGet, "/Videos/item123/stream?static=true&mediaSourceId=src9", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401; body=%s", rec.Code, rec.Body.String()) + } + if reached { + t.Fatal("inner handler must not run without a matching PlaybackSession") + } +} + +// TestPlaybackSessionAuth_DirectPlayCrossItemDenied: a mediaSourceId that resolves +// to a session for a DIFFERENT route item must not authorize the request — the +// session's RouteItemID must match the requested item. +func TestPlaybackSessionAuth_DirectPlayCrossItemDenied(t *testing.T) { + now := fixedNow() + clock := func() time.Time { return now } + sessions := NewSessionStore(time.Hour, clock) + if err := sessions.Put(Session{Token: "compat-tok", StreamAppUserID: 7}); err != nil { + t.Fatalf("put session: %v", err) + } + playback := NewPlaybackSessionStore(time.Hour, clock) + // src9 belongs to itemB's session; the request targets itemA. + playback.Put(PlaybackSession{ + ID: "psB", + CompatToken: "compat-tok", + RouteItemID: "itemB", + MediaSources: []PlaybackMediaSource{{ID: "src9"}}, + }) + + var reached bool + router := directPlayRouter(t, sessions, playback, nil, &reached) + + req := httptest.NewRequest(http.MethodGet, "/Videos/itemA/stream?static=true&mediaSourceId=src9", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401; body=%s", rec.Code, rec.Body.String()) + } + if reached { + t.Fatal("inner handler must not run when the matched session belongs to another item") + } +} + +// TestPlaybackSessionAuth_DownloadNotLoosened: the new direct-play fallback must +// not apply to /Items/{id}/Download — a token-less download stays 401 even when a +// resolvable compat session AND a PlaybackSession exist for the same item id, so +// the 401 proves route scoping rather than a missing session. +func TestPlaybackSessionAuth_DownloadNotLoosened(t *testing.T) { + now := fixedNow() + clock := func() time.Time { return now } + sessions, playback := seedDirectPlaySession(t, clock) + + var reached bool + router := directPlayRouter(t, sessions, playback, nil, &reached) + + req := httptest.NewRequest(http.MethodGet, "/Items/item123/Download", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401; body=%s", rec.Code, rec.Body.String()) + } + if reached { + t.Fatal("download route must not be served via the direct-play fallback") + } +}