fix(progress): filter continue-watching by viewer access scope (#167)

* fix(progress): filter continue-watching by viewer access scope

The continue-watching list (GET /progress) only filtered by library when an
explicit library_id query param was passed. The global call passes none, so a
restricted profile received progress rows for items outside its scope (e.g. an
XXX title, or a title above the profile's content-rating cap). The web client
then fans out a per-item GET /catalog/items/{id} detail fetch for each row, and
the inaccessible ones return 404 — surfacing as a dead Continue Watching tile
and stray 404s.

Always apply the viewer's access scope to the progress list. Adds
LibraryItemRepository.FilterAccessibleContentIDs, a batched, episode-aware
mirror of the detail endpoint's access predicate (library membership +
content-rating ceiling), and wires it into HandleListProgress for restricted
profiles only (unrestricted viewers are unaffected). ExcludedMediaTypes is
omitted intentionally: the viewer access.Scope does not carry it and the
request path never sets it.

* fix(progress): gate continue-watching episode access on the parent series

FilterAccessibleContentIDs keyed episode access off episode_libraries and
required a media_item_libraries membership even for rating-only viewers, both
of which diverge from the detail/watch path that masks inaccessible items
(DetailService.GetItemDetail → EnsureAccessible(episode.SeriesID)). For shows
whose episodes span multiple library folders this reintroduced the dead tile /
out-of-scope leak this filter exists to prevent, and rating-only profiles
could lose membership-less items the detail endpoint still serves.

Rewrite the predicate to mirror EnsureAccessible exactly: base the lookup on
media_items, join media_item_libraries only when the viewer is
library-restricted, and resolve episodes through their parent series. Extract a
pure buildFilterAccessibleContentIDsSQL so the query shape (placeholder
numbering, the parent-series join, the optional rating predicate) is
unit-tested without a database, and de-duplicate the two progress filter
helpers via progressContentIDs/keepAccessibleEntries.

AI-use disclosure: implemented with AI assistance (Claude).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
d3v1l1989
2026-06-16 18:32:52 -04:00
committed by GitHub
co-authored by Quick Claude Opus 4.8
parent 9d00ce2089
commit 63d608f6fc
4 changed files with 451 additions and 12 deletions
+65 -12
View File
@@ -6,6 +6,7 @@ import (
"net/http"
"strconv"
"github.com/Silo-Server/silo-server/internal/access"
apimw "github.com/Silo-Server/silo-server/internal/api/middleware"
evt "github.com/Silo-Server/silo-server/internal/events"
"github.com/Silo-Server/silo-server/internal/userstore"
@@ -14,6 +15,9 @@ import (
// ProgressLibraryLookup resolves which progress items belong to a library.
type ProgressLibraryLookup interface {
GetItemsInFolder(ctx context.Context, contentIDs []string, folderID int) (map[string]bool, error)
// FilterAccessibleContentIDs returns the subset of contentIDs the viewer
// may access given their library scope and content-rating ceiling.
FilterAccessibleContentIDs(ctx context.Context, contentIDs []string, allowedFolderIDs, disabledFolderIDs []int, maxContentRating string) (map[string]bool, error)
}
// ProgressHandler handles watch progress and sync endpoints.
@@ -102,6 +106,25 @@ func (h *ProgressHandler) HandleListProgress(w http.ResponseWriter, r *http.Requ
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list progress")
return
}
// Drop entries the viewer can't access before they reach the client.
// Without this, a library-restricted profile receives progress rows for
// items outside its scope (e.g. an XXX title) and the client then fans out
// per-item detail fetches that 404 — a dead Continue Watching tile. Only
// runs for restricted profiles; unrestricted viewers are unaffected.
if scope, ok := access.GetScope(r.Context()); ok &&
(scope.AllowedLibraryIDs != nil || len(scope.DisabledLibraryIDs) > 0 || scope.MaxContentRating != "") {
if h.LibraryLookup == nil {
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to apply access filter")
return
}
entries, err = filterProgressEntriesByAccess(r.Context(), entries, scope, h.LibraryLookup)
if err != nil {
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to apply access filter")
return
}
}
if libraryID > 0 {
if h.LibraryLookup == nil {
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to apply library filter")
@@ -144,6 +167,27 @@ func parseLibraryIDParam(r *http.Request) (int, error) {
return libraryID, nil
}
// progressContentIDs collects the media item IDs from a progress slice.
func progressContentIDs(entries []userstore.WatchProgress) []string {
contentIDs := make([]string, 0, len(entries))
for _, entry := range entries {
contentIDs = append(contentIDs, entry.MediaItemID)
}
return contentIDs
}
// keepAccessibleEntries returns, in order, the entries whose media item ID maps
// to true in accessible.
func keepAccessibleEntries(entries []userstore.WatchProgress, accessible map[string]bool) []userstore.WatchProgress {
filtered := make([]userstore.WatchProgress, 0, len(entries))
for _, entry := range entries {
if accessible[entry.MediaItemID] {
filtered = append(filtered, entry)
}
}
return filtered
}
func filterProgressEntriesByLibrary(
ctx context.Context,
entries []userstore.WatchProgress,
@@ -154,24 +198,33 @@ func filterProgressEntriesByLibrary(
return entries, nil
}
contentIDs := make([]string, 0, len(entries))
for _, entry := range entries {
contentIDs = append(contentIDs, entry.MediaItemID)
}
allowed, err := lookup.GetItemsInFolder(ctx, contentIDs, libraryID)
allowed, err := lookup.GetItemsInFolder(ctx, progressContentIDs(entries), libraryID)
if err != nil {
return nil, err
}
filtered := make([]userstore.WatchProgress, 0, len(entries))
for _, entry := range entries {
if allowed[entry.MediaItemID] {
filtered = append(filtered, entry)
}
return keepAccessibleEntries(entries, allowed), nil
}
// filterProgressEntriesByAccess removes progress entries whose item falls
// outside the viewer's access scope (allowed/disabled libraries and the
// content-rating ceiling).
func filterProgressEntriesByAccess(
ctx context.Context,
entries []userstore.WatchProgress,
scope access.Scope,
lookup ProgressLibraryLookup,
) ([]userstore.WatchProgress, error) {
if len(entries) == 0 {
return entries, nil
}
return filtered, nil
accessible, err := lookup.FilterAccessibleContentIDs(ctx, progressContentIDs(entries), scope.AllowedLibraryIDs, scope.DisabledLibraryIDs, scope.MaxContentRating)
if err != nil {
return nil, err
}
return keepAccessibleEntries(entries, accessible), nil
}
// HandleSyncProgress handles POST /sync/progress.
@@ -0,0 +1,111 @@
package handlers
import (
"context"
"slices"
"testing"
"github.com/Silo-Server/silo-server/internal/access"
"github.com/Silo-Server/silo-server/internal/userstore"
)
// fakeProgressLookup records the arguments it was called with and returns a
// fixed accessibility map.
type fakeProgressLookup struct {
accessible map[string]bool
gotContentIDs []string
gotAllowed []int
gotDisabled []int
gotRating string
}
func (f *fakeProgressLookup) GetItemsInFolder(context.Context, []string, int) (map[string]bool, error) {
return nil, nil
}
func (f *fakeProgressLookup) FilterAccessibleContentIDs(
_ context.Context, contentIDs []string, allowedFolderIDs, disabledFolderIDs []int, maxContentRating string,
) (map[string]bool, error) {
f.gotContentIDs = contentIDs
f.gotAllowed = allowedFolderIDs
f.gotDisabled = disabledFolderIDs
f.gotRating = maxContentRating
return f.accessible, nil
}
func entries(ids ...string) []userstore.WatchProgress {
out := make([]userstore.WatchProgress, 0, len(ids))
for _, id := range ids {
out = append(out, userstore.WatchProgress{MediaItemID: id})
}
return out
}
func ids(entries []userstore.WatchProgress) []string {
out := make([]string, 0, len(entries))
for _, e := range entries {
out = append(out, e.MediaItemID)
}
return out
}
func TestFilterProgressEntriesByAccess(t *testing.T) {
// access.Scope sets DisabledLibraryIDs only when AllowedLibraryIDs is nil
// (see access.Scope docs), so the two restriction shapes are tested
// separately rather than as one impossible combined scope.
cases := []struct {
name string
scope access.Scope
}{
{
name: "allowed libraries + rating",
scope: access.Scope{AllowedLibraryIDs: []int{1, 2}, MaxContentRating: "PG-13"},
},
{
name: "disabled libraries + rating",
scope: access.Scope{DisabledLibraryIDs: []int{9}, MaxContentRating: "PG-13"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
lookup := &fakeProgressLookup{accessible: map[string]bool{"a": true, "c": true}}
got, err := filterProgressEntriesByAccess(context.Background(), entries("a", "b", "c"), tc.scope, lookup)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
want := []string{"a", "c"}
if g := ids(got); len(g) != len(want) || g[0] != want[0] || g[1] != want[1] {
t.Fatalf("filtered entries = %v, want %v", g, want)
}
// Scope must be forwarded verbatim to the lookup.
if !slices.Equal(lookup.gotAllowed, tc.scope.AllowedLibraryIDs) {
t.Errorf("allowed folders = %v, want %v", lookup.gotAllowed, tc.scope.AllowedLibraryIDs)
}
if !slices.Equal(lookup.gotDisabled, tc.scope.DisabledLibraryIDs) {
t.Errorf("disabled folders = %v, want %v", lookup.gotDisabled, tc.scope.DisabledLibraryIDs)
}
if len(lookup.gotContentIDs) != 3 {
t.Errorf("content ids = %v, want 3 entries", lookup.gotContentIDs)
}
if lookup.gotRating != tc.scope.MaxContentRating {
t.Errorf("max content rating = %q, want %q", lookup.gotRating, tc.scope.MaxContentRating)
}
})
}
}
func TestFilterProgressEntriesByAccessEmpty(t *testing.T) {
lookup := &fakeProgressLookup{accessible: map[string]bool{}}
got, err := filterProgressEntriesByAccess(context.Background(), nil, access.Scope{}, lookup)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(got) != 0 {
t.Fatalf("expected no entries, got %v", ids(got))
}
}
+157
View File
@@ -4,11 +4,13 @@ import (
"context"
"fmt"
"slices"
"strings"
"time"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgxpool"
"github.com/Silo-Server/silo-server/internal/access"
"github.com/Silo-Server/silo-server/internal/models"
)
@@ -201,6 +203,161 @@ func (r *LibraryItemRepository) GetItemsInFolders(ctx context.Context, contentID
return result, nil
}
// FilterAccessibleContentIDs returns the subset of contentIDs that pass the
// viewer's access scope, checking library membership and the content-rating
// ceiling. It is the batched, set-oriented mirror of the per-item predicate the
// detail/watch path enforces (ItemRepository.EnsureAccessible +
// applyAccessFilter), so list endpoints (e.g. continue-watching) can drop
// out-of-scope items in one query instead of letting the client discover them
// via per-item 404s.
//
// Semantics match EnsureAccessible exactly:
// - A media item (movie/series) is accessible when its media_items row
// satisfies the rating ceiling and — when the viewer is library-restricted
// — it has a media_item_libraries membership in a permitted folder (within
// allowedFolderIDs when that slice is non-nil, and not in disabledFolderIDs).
// - An episode is gated on its PARENT SERIES, mirroring how the detail/watch
// path resolves an episode id to episode.SeriesID and calls
// EnsureAccessible(series_id) (catalog.DetailService.GetItemDetail): series
// media_item_libraries membership + series content_rating. It deliberately
// does NOT key off episode_libraries — that membership can diverge from the
// series for multi-folder shows, which would re-introduce the dead-tile /
// leak this filter exists to prevent.
// - When the viewer has no library restriction, membership is not required
// (matching EnsureAccessible, which only joins media_item_libraries when a
// library restriction is set, so a rating-only viewer is gated on rating
// alone).
//
// A non-nil but empty allowedFolderIDs, or a maxContentRating that permits no
// ratings, means nothing is accessible. ExcludedMediaTypes is intentionally
// omitted: the viewer access.Scope does not carry it and the native request
// path never sets it (only the jellycompat layer populates it), so it is a
// no-op here.
//
// The emitted SQL is built by buildFilterAccessibleContentIDsSQL so its shape
// (placeholder numbering, the parent-series join for episodes, the optional
// rating predicate) is unit-testable without a database.
func (r *LibraryItemRepository) FilterAccessibleContentIDs(ctx context.Context, contentIDs []string, allowedFolderIDs, disabledFolderIDs []int, maxContentRating string) (map[string]bool, error) {
result := make(map[string]bool, len(contentIDs))
if len(contentIDs) == 0 {
return result, nil
}
if allowedFolderIDs != nil && len(allowedFolderIDs) == 0 {
// Library-restricted to nothing → nothing is accessible.
return result, nil
}
var allowedRatings []string
if maxContentRating != "" {
allowedRatings = access.AllowedRatingsUpTo(maxContentRating)
if len(allowedRatings) == 0 {
// Ceiling permits no ratings → nothing is accessible.
return result, nil
}
}
query, args := buildFilterAccessibleContentIDsSQL(contentIDs, allowedFolderIDs, disabledFolderIDs, allowedRatings)
rows, err := r.pool.Query(ctx, query, args...)
if err != nil {
return nil, fmt.Errorf("filtering accessible content ids: %w", err)
}
defer rows.Close()
for rows.Next() {
var contentID string
if err := rows.Scan(&contentID); err != nil {
return nil, fmt.Errorf("scanning accessible content id row: %w", err)
}
result[contentID] = true
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("iterating accessible content id rows: %w", err)
}
return result, nil
}
// buildFilterAccessibleContentIDsSQL builds the membership/rating query used by
// FilterAccessibleContentIDs. It is a pure function (no DB access) so the query
// shape can be unit-tested. allowedRatings must already be resolved via
// access.AllowedRatingsUpTo (nil/empty means no rating ceiling); the caller
// handles the "permits nothing" early-outs.
//
// The structure mirrors ItemRepository.EnsureAccessible: select FROM the owning
// media_items row, join media_item_libraries only when the viewer is
// library-restricted, and resolve episodes through their parent series so an
// episode is gated on EnsureAccessible(series_id)-equivalent membership.
func buildFilterAccessibleContentIDsSQL(contentIDs []string, allowedFolderIDs, disabledFolderIDs []int, allowedRatings []string) (string, []any) {
args := []any{contentIDs}
var allowedIdx, disabledIdx, ratingIdx int
if allowedFolderIDs != nil {
args = append(args, allowedFolderIDs)
allowedIdx = len(args)
}
if len(disabledFolderIDs) > 0 {
args = append(args, disabledFolderIDs)
disabledIdx = len(args)
}
if len(allowedRatings) > 0 {
args = append(args, allowedRatings)
ratingIdx = len(args)
}
needsLibJoin := allowedIdx > 0 || disabledIdx > 0
// folderConds builds the membership predicate against the media_item_libraries
// alias (mil), shared verbatim by the item and episode branches. Only called
// when needsLibJoin, so it always yields at least one condition.
folderConds := func() string {
conds := make([]string, 0, 2)
if allowedIdx > 0 {
conds = append(conds, fmt.Sprintf("mil.media_folder_id = ANY($%d)", allowedIdx))
}
if disabledIdx > 0 {
conds = append(conds, fmt.Sprintf("NOT (mil.media_folder_id = ANY($%d))", disabledIdx))
}
return strings.Join(conds, " AND ")
}
// Item branch gates the media item directly; episode branch resolves the
// parent series and gates on it (mirroring EnsureAccessible(series_id)).
itemFrom := "media_items mi"
episodeFrom := "episodes e JOIN media_items mi ON mi.content_id = e.series_id"
itemConds := []string{"mi.content_id = req.content_id"}
episodeConds := []string{"e.content_id = req.content_id"}
if needsLibJoin {
itemFrom += " JOIN media_item_libraries mil ON mil.content_id = mi.content_id"
episodeFrom += " JOIN media_item_libraries mil ON mil.content_id = e.series_id"
fc := folderConds()
itemConds = append(itemConds, fc)
episodeConds = append(episodeConds, fc)
}
if ratingIdx > 0 {
rc := fmt.Sprintf("mi.content_rating = ANY($%d)", ratingIdx)
itemConds = append(itemConds, rc)
episodeConds = append(episodeConds, rc)
}
query := fmt.Sprintf(`
SELECT req.content_id
FROM unnest($1::text[]) AS req(content_id)
WHERE EXISTS (
SELECT 1
FROM %s
WHERE %s
)
OR EXISTS (
SELECT 1
FROM %s
WHERE %s
)`,
itemFrom, strings.Join(itemConds, " AND "),
episodeFrom, strings.Join(episodeConds, " AND "),
)
return query, args
}
func (r *LibraryItemRepository) GetFolderIDsForItem(ctx context.Context, contentID string) ([]int, error) {
rows, err := r.pool.Query(ctx, `
SELECT media_folder_id
@@ -0,0 +1,118 @@
package catalog
import (
"strings"
"testing"
"github.com/Silo-Server/silo-server/internal/access"
)
// These tests pin the SQL emitted by buildFilterAccessibleContentIDsSQL — the
// query builder behind LibraryItemRepository.FilterAccessibleContentIDs — for
// each viewer scope shape, without needing a database. They guard the
// properties that make the batch filter agree with the per-item access
// predicate the detail/watch path enforces (ItemRepository.EnsureAccessible):
// - episodes are gated on their parent SERIES (media_item_libraries via
// series_id), never on episode_libraries;
// - a rating-only viewer is gated on rating alone, with no membership join;
// - placeholder numbering tracks the bound args.
func TestBuildFilterAccessibleContentIDsSQL_AllowedLibrariesOnly(t *testing.T) {
sql, args := buildFilterAccessibleContentIDsSQL(
[]string{"a", "b"}, []int{1, 2}, nil, nil,
)
if len(args) != 2 {
t.Fatalf("expected 2 args (ids, allowed libs); got %d (%v)", len(args), args)
}
if !strings.Contains(sql, "unnest($1::text[])") {
t.Errorf("expected content ids bound at $1; got %s", sql)
}
if !strings.Contains(sql, "mil.media_folder_id = ANY($2)") {
t.Errorf("expected allowed libraries bound at $2; got %s", sql)
}
// Item branch joins membership on the item's own content_id; episode branch
// resolves the parent series and joins membership on series_id.
if !strings.Contains(sql, "JOIN media_item_libraries mil ON mil.content_id = mi.content_id") {
t.Errorf("expected item membership join on mi.content_id; got %s", sql)
}
if !strings.Contains(sql, "episodes e JOIN media_items mi ON mi.content_id = e.series_id") {
t.Errorf("expected episode branch to resolve parent series; got %s", sql)
}
if !strings.Contains(sql, "JOIN media_item_libraries mil ON mil.content_id = e.series_id") {
t.Errorf("expected episode membership join on series_id; got %s", sql)
}
// Episodes must NOT be gated on episode_libraries — that diverges from the
// detail endpoint's EnsureAccessible(series_id).
if strings.Contains(sql, "episode_libraries") {
t.Errorf("episode access must gate on the series, not episode_libraries; got %s", sql)
}
if strings.Contains(sql, "content_rating") {
t.Errorf("no rating ceiling set, expected no content_rating predicate; got %s", sql)
}
}
func TestBuildFilterAccessibleContentIDsSQL_RatingOnlyRequiresNoMembership(t *testing.T) {
ratings := access.AllowedRatingsUpTo("PG-13")
sql, args := buildFilterAccessibleContentIDsSQL(
[]string{"a"}, nil, nil, ratings,
)
if len(args) != 2 {
t.Fatalf("expected 2 args (ids, ratings); got %d (%v)", len(args), args)
}
// EnsureAccessible only joins media_item_libraries when a library
// restriction is set; a rating-only viewer is gated on rating alone.
if strings.Contains(sql, "media_item_libraries") {
t.Errorf("rating-only scope must not require a membership join; got %s", sql)
}
if !strings.Contains(sql, "mi.content_rating = ANY($2)") {
t.Errorf("expected rating predicate bound at $2; got %s", sql)
}
// The episode branch still resolves the rating from the parent series.
if !strings.Contains(sql, "episodes e JOIN media_items mi ON mi.content_id = e.series_id") {
t.Errorf("expected episode branch to resolve parent series for rating; got %s", sql)
}
}
func TestBuildFilterAccessibleContentIDsSQL_DisabledLibrariesOnly(t *testing.T) {
sql, args := buildFilterAccessibleContentIDsSQL(
[]string{"a"}, nil, []int{9, 10}, nil,
)
if len(args) != 2 {
t.Fatalf("expected 2 args (ids, disabled libs); got %d (%v)", len(args), args)
}
if !strings.Contains(sql, "NOT (mil.media_folder_id = ANY($2))") {
t.Errorf("expected disabled libraries as NOT ANY($2); got %s", sql)
}
if !strings.Contains(sql, "JOIN media_item_libraries mil") {
t.Errorf("expected membership join for disabled-library restriction; got %s", sql)
}
}
func TestBuildFilterAccessibleContentIDsSQL_AllowedDisabledAndRatingPlaceholders(t *testing.T) {
ratings := access.AllowedRatingsUpTo("R")
sql, args := buildFilterAccessibleContentIDsSQL(
[]string{"a"}, []int{1}, []int{9}, ratings,
)
// $1 ids, $2 allowed, $3 disabled, $4 ratings — in append order.
if len(args) != 4 {
t.Fatalf("expected 4 args (ids, allowed, disabled, ratings); got %d (%v)", len(args), args)
}
for _, want := range []string{
"mil.media_folder_id = ANY($2)",
"NOT (mil.media_folder_id = ANY($3))",
"mi.content_rating = ANY($4)",
} {
if !strings.Contains(sql, want) {
t.Errorf("expected SQL to contain %q; got %s", want, sql)
}
}
// Both the allowed and disabled predicates apply to both branches, so each
// folder placeholder appears once per branch (item + episode).
if got := strings.Count(sql, "mil.media_folder_id = ANY($2)"); got != 2 {
t.Errorf("expected allowed predicate in both branches; found %d occurrences", got)
}
}