diff --git a/internal/api/handlers/progress.go b/internal/api/handlers/progress.go index 1abbb887..382871cf 100644 --- a/internal/api/handlers/progress.go +++ b/internal/api/handlers/progress.go @@ -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. diff --git a/internal/api/handlers/progress_access_test.go b/internal/api/handlers/progress_access_test.go new file mode 100644 index 00000000..a90df003 --- /dev/null +++ b/internal/api/handlers/progress_access_test.go @@ -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)) + } +} diff --git a/internal/catalog/library_repo.go b/internal/catalog/library_repo.go index 602125a6..a3496485 100644 --- a/internal/catalog/library_repo.go +++ b/internal/catalog/library_repo.go @@ -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 diff --git a/internal/catalog/library_repo_access_test.go b/internal/catalog/library_repo_access_test.go new file mode 100644 index 00000000..84a04cb5 --- /dev/null +++ b/internal/catalog/library_repo_access_test.go @@ -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) + } +}