From 95e844a2ea842498adfbf1b03047ba61204b720c Mon Sep 17 00:00:00 2001 From: Dylan Jeffers Date: Thu, 21 May 2026 12:08:58 -0700 Subject: [PATCH 1/2] feat(playlists): permalink-only access to private playlists Add an `is_private` filter to `get_playlists.sql` with an `@include_private` bypass, mirroring the existing `is_unlisted` pattern for tracks. The flag is set to true on permalink-driven entry points (by_permalink route, bulk endpoint when ?permalink= is used, and the /resolve redirect target), so possessing a valid permalink is treated as proof of access for unauthenticated callers. Caller-supplied IDs in the bulk endpoint are kept in a separate query from permalink-derived IDs to avoid leaking a private playlist when both are mixed in one request. Also: extend the by_permalink route to accept both /playlist/ and /album/ URL variants so /v1/resolve can redirect there for albums. Co-Authored-By: Claude Opus 4.7 (1M context) --- api/dbv1/get_playlists.sql.go | 8 +-- api/dbv1/queries/get_playlists.sql | 1 + api/v1_playlist_by_permalink.go | 14 +++-- api/v1_playlist_by_permalink_test.go | 52 +++++++++++++++++++ api/v1_playlists.go | 55 +++++++++++++++----- api/v1_playlists_test.go | 76 ++++++++++++++++++++++++++++ api/v1_resolve.go | 11 ++-- api/v1_tracks_test.go | 19 +++++++ 8 files changed, 207 insertions(+), 29 deletions(-) diff --git a/api/dbv1/get_playlists.sql.go b/api/dbv1/get_playlists.sql.go index 70a89c8e..524b3d80 100644 --- a/api/dbv1/get_playlists.sql.go +++ b/api/dbv1/get_playlists.sql.go @@ -143,11 +143,13 @@ JOIN aggregate_playlist using (playlist_id) LEFT JOIN playlist_routes on p.playlist_id = playlist_routes.playlist_id and playlist_routes.is_current = true WHERE is_delete = false and p.playlist_id = ANY($2::int[]) + and (p.is_private = false OR p.playlist_owner_id = $1 OR $3::bool = TRUE) ` type GetPlaylistsParams struct { - MyID interface{} `json:"my_id"` - Ids []int32 `json:"ids"` + MyID interface{} `json:"my_id"` + Ids []int32 `json:"ids"` + IncludePrivate bool `json:"include_private"` } type GetPlaylistsRow struct { @@ -185,7 +187,7 @@ type GetPlaylistsRow struct { // See get_tracks.sql for why my_follows is MATERIALIZED. func (q *Queries) GetPlaylists(ctx context.Context, arg GetPlaylistsParams) ([]GetPlaylistsRow, error) { - rows, err := q.db.Query(ctx, getPlaylists, arg.MyID, arg.Ids) + rows, err := q.db.Query(ctx, getPlaylists, arg.MyID, arg.Ids, arg.IncludePrivate) if err != nil { return nil, err } diff --git a/api/dbv1/queries/get_playlists.sql b/api/dbv1/queries/get_playlists.sql index 44d5abf6..21748739 100644 --- a/api/dbv1/queries/get_playlists.sql +++ b/api/dbv1/queries/get_playlists.sql @@ -129,4 +129,5 @@ JOIN aggregate_playlist using (playlist_id) LEFT JOIN playlist_routes on p.playlist_id = playlist_routes.playlist_id and playlist_routes.is_current = true WHERE is_delete = false and p.playlist_id = ANY(@ids::int[]) + and (p.is_private = false OR p.playlist_owner_id = @my_id OR @include_private::bool = TRUE) ; diff --git a/api/v1_playlist_by_permalink.go b/api/v1_playlist_by_permalink.go index 4ddb3ae3..dc565aa7 100644 --- a/api/v1_playlist_by_permalink.go +++ b/api/v1_playlist_by_permalink.go @@ -20,9 +20,12 @@ func (app *ApiServer) v1PlaylistByPermalink(c *fiber.Ctx) error { } ids, err := app.queries.GetPlaylistIdsByPermalink(c.Context(), dbv1.GetPlaylistIdsByPermalinkParams{ - Handles: []string{params.Handle}, - Slugs: []string{params.Slug}, - Permalinks: []string{"/" + params.Handle + "/playlist/" + params.Slug}, + Handles: []string{params.Handle}, + Slugs: []string{params.Slug}, + Permalinks: []string{ + "/" + params.Handle + "/playlist/" + params.Slug, + "/" + params.Handle + "/album/" + params.Slug, + }, }) if err != nil { return err @@ -30,8 +33,9 @@ func (app *ApiServer) v1PlaylistByPermalink(c *fiber.Ctx) error { playlists, err := app.queries.Playlists(c.Context(), dbv1.PlaylistsParams{ GetPlaylistsParams: dbv1.GetPlaylistsParams{ - MyID: myId, - Ids: ids, + MyID: myId, + Ids: ids, + IncludePrivate: true, }, AuthedWallet: app.tryGetAuthedWallet(c), }) diff --git a/api/v1_playlist_by_permalink_test.go b/api/v1_playlist_by_permalink_test.go index 29481d3e..275f21c5 100644 --- a/api/v1_playlist_by_permalink_test.go +++ b/api/v1_playlist_by_permalink_test.go @@ -1,9 +1,11 @@ package api import ( + "context" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestV1PlaylistByPermalink(t *testing.T) { @@ -16,3 +18,53 @@ func TestV1PlaylistByPermalink(t *testing.T) { "data.0.playlist_name": "playlist by permalink", }) } + +func TestV1AlbumByPermalink(t *testing.T) { + app := testAppWithFixtures(t) + status, body := testGet(t, app, "/v1/full/playlists/by_permalink/AlbumsByPermalink/album-by-permalink") + assert.Equal(t, 200, status) + + jsonAssert(t, body, map[string]any{ + "data.0.id": "ePVXL", + "data.0.playlist_name": "album by permalink", + }) +} + +// A private playlist should be returned to anonymous callers when fetched via +// permalink — "has the link" is sufficient permission. +func TestV1PrivatePlaylistByPermalinkAnonymous(t *testing.T) { + app := testAppWithFixtures(t) + ctx := context.Background() + require.NotNil(t, app.writePool, "test requires write pool") + + _, err := app.writePool.Exec(ctx, `UPDATE playlists SET is_private = true WHERE playlist_id = 500 AND is_current = true`) + require.NoError(t, err) + + status, body := testGet(t, app, "/v1/full/playlists/by_permalink/PlaylistsByPermalink/playlist-by-permalink") + assert.Equal(t, 200, status) + + jsonAssert(t, body, map[string]any{ + "data.0.id": "eYake", + "data.0.playlist_name": "playlist by permalink", + "data.0.is_private": true, + }) +} + +// Same for albums. +func TestV1PrivateAlbumByPermalinkAnonymous(t *testing.T) { + app := testAppWithFixtures(t) + ctx := context.Background() + require.NotNil(t, app.writePool, "test requires write pool") + + _, err := app.writePool.Exec(ctx, `UPDATE playlists SET is_private = true WHERE playlist_id = 501 AND is_current = true`) + require.NoError(t, err) + + status, body := testGet(t, app, "/v1/full/playlists/by_permalink/AlbumsByPermalink/album-by-permalink") + assert.Equal(t, 200, status) + + jsonAssert(t, body, map[string]any{ + "data.0.id": "ePVXL", + "data.0.playlist_name": "album by permalink", + "data.0.is_private": true, + }) +} diff --git a/api/v1_playlists.go b/api/v1_playlists.go index 6be9c78d..ec8c360c 100644 --- a/api/v1_playlists.go +++ b/api/v1_playlists.go @@ -15,7 +15,13 @@ func (app *ApiServer) v1Playlists(c *fiber.Ctx) error { // unless client explicitly does ?with_tracks=true withTracks, _ := strconv.ParseBool(c.Query("with_tracks", "false")) - // Add permalink ID mappings + authedWallet := app.tryGetAuthedWallet(c) + + // Permalink-matched IDs are kept separate from caller-supplied IDs. Possession + // of a valid permalink is treated as proof of access, so private playlists + // matched by permalink are returned without auth. We must not extend that + // trust to user-supplied IDs in the same request. + var permalinkIds []int32 permalinks := queryMulti(c, "permalink") if len(permalinks) > 0 { handles := make([]string, len(permalinks)) @@ -29,7 +35,8 @@ func (app *ApiServer) v1Playlists(c *fiber.Ctx) error { return fiber.NewError(fiber.StatusBadRequest, "Invalid permalink: "+permalinks[i]) } } - newIds, err := app.queries.GetPlaylistIdsByPermalink(c.Context(), dbv1.GetPlaylistIdsByPermalinkParams{ + var err error + permalinkIds, err = app.queries.GetPlaylistIdsByPermalink(c.Context(), dbv1.GetPlaylistIdsByPermalinkParams{ Handles: handles, Slugs: slugs, Permalinks: permalinks, @@ -37,7 +44,6 @@ func (app *ApiServer) v1Playlists(c *fiber.Ctx) error { if err != nil { return err } - ids = append(ids, newIds...) } upcs := queryMulti(c, "upc") @@ -49,17 +55,38 @@ func (app *ApiServer) v1Playlists(c *fiber.Ctx) error { ids = append(ids, newIds...) } - playlists, err := app.queries.Playlists(c.Context(), dbv1.PlaylistsParams{ - GetPlaylistsParams: dbv1.GetPlaylistsParams{ - MyID: myId, - Ids: ids, - }, - OmitTracks: !withTracks, - AuthedWallet: app.tryGetAuthedWallet(c), - }) - if err != nil { - return err + out := make([]dbv1.Playlist, 0, len(ids)+len(permalinkIds)) + + if len(ids) > 0 { + playlists, err := app.queries.Playlists(c.Context(), dbv1.PlaylistsParams{ + GetPlaylistsParams: dbv1.GetPlaylistsParams{ + MyID: myId, + Ids: ids, + }, + OmitTracks: !withTracks, + AuthedWallet: authedWallet, + }) + if err != nil { + return err + } + out = append(out, playlists...) + } + + if len(permalinkIds) > 0 { + playlists, err := app.queries.Playlists(c.Context(), dbv1.PlaylistsParams{ + GetPlaylistsParams: dbv1.GetPlaylistsParams{ + MyID: myId, + Ids: permalinkIds, + IncludePrivate: true, + }, + OmitTracks: !withTracks, + AuthedWallet: authedWallet, + }) + if err != nil { + return err + } + out = append(out, playlists...) } - return v1PlaylistsResponse(c, playlists) + return v1PlaylistsResponse(c, out) } diff --git a/api/v1_playlists_test.go b/api/v1_playlists_test.go index da2ec1b4..f507f3b9 100644 --- a/api/v1_playlists_test.go +++ b/api/v1_playlists_test.go @@ -1,10 +1,13 @@ package api import ( + "context" "testing" "api.audius.co/api/dbv1" + "api.audius.co/trashid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPlaylistsEndpoint(t *testing.T) { @@ -60,3 +63,76 @@ func TestPlaylistsEndpointWithAlbumPermalink(t *testing.T) { "data.0.playlist_name": "album by permalink", }) } + +// A permalink-based lookup of a private playlist works for anonymous callers. +func TestPlaylistsEndpointPrivatePermalinkAnonymous(t *testing.T) { + app := testAppWithFixtures(t) + ctx := context.Background() + require.NotNil(t, app.writePool, "test requires write pool") + + _, err := app.writePool.Exec(ctx, `UPDATE playlists SET is_private = true WHERE playlist_id = 500 AND is_current = true`) + require.NoError(t, err) + + var resp struct { + Data []dbv1.Playlist + } + status, body := testGet(t, app, "/v1/full/playlists?permalink=/PlaylistsByPermalink/playlist/playlist-by-permalink", &resp) + assert.Equal(t, 200, status) + assert.Len(t, resp.Data, 1, "permalink lookup must return private playlist even without auth") + + jsonAssert(t, body, map[string]any{ + "data.0.id": "eYake", + "data.0.playlist_name": "playlist by permalink", + "data.0.is_private": true, + }) +} + +// An ID-based lookup must NOT return private playlists to anonymous callers. +func TestPlaylistsEndpointPrivateByIdHiddenFromAnonymous(t *testing.T) { + app := testAppWithFixtures(t) + ctx := context.Background() + require.NotNil(t, app.writePool, "test requires write pool") + + _, err := app.writePool.Exec(ctx, `UPDATE playlists SET is_private = true WHERE playlist_id = 500 AND is_current = true`) + require.NoError(t, err) + + var resp struct { + Data []dbv1.Playlist + } + status, _ := testGet(t, app, "/v1/full/playlists?id=eYake", &resp) + assert.Equal(t, 200, status) + assert.Len(t, resp.Data, 0, "private playlist must not be returned for ID-based anonymous lookup") +} + +// The single playlist endpoint must also hide private playlists from anonymous callers. +func TestGetPlaylistPrivateAnonymous404(t *testing.T) { + app := testAppWithFixtures(t) + ctx := context.Background() + require.NotNil(t, app.writePool, "test requires write pool") + + _, err := app.writePool.Exec(ctx, `UPDATE playlists SET is_private = true WHERE playlist_id = 500 AND is_current = true`) + require.NoError(t, err) + + status, _ := testGet(t, app, "/v1/full/playlists/eYake") + assert.Equal(t, 404, status, "private playlist must 404 for anonymous ID-based fetch") +} + +// The single playlist endpoint must return private playlists to their owner. +func TestGetPlaylistPrivateOwnerAllowed(t *testing.T) { + app := testAppWithFixtures(t) + ctx := context.Background() + require.NotNil(t, app.writePool, "test requires write pool") + + // playlist 500 is owned by user 7 + _, err := app.writePool.Exec(ctx, `UPDATE playlists SET is_private = true WHERE playlist_id = 500 AND is_current = true`) + require.NoError(t, err) + + ownerId := trashid.MustEncodeHashID(7) + status, body := testGet(t, app, "/v1/full/playlists/eYake?user_id="+ownerId) + assert.Equal(t, 200, status, "owner must be able to view their own private playlist by ID") + jsonAssert(t, body, map[string]any{ + "data.0.id": "eYake", + "data.0.playlist_name": "playlist by permalink", + "data.0.is_private": true, + }) +} diff --git a/api/v1_resolve.go b/api/v1_resolve.go index 0ebf713e..49a105d4 100644 --- a/api/v1_resolve.go +++ b/api/v1_resolve.go @@ -90,15 +90,12 @@ func (app *ApiServer) v1Resolve(c *fiber.Ctx) error { return fiber.NewError(fiber.StatusNotFound, "Playlist not found") } - playlistId, err := trashid.EncodeHashId(int(playlistIds[0])) - if err != nil { - return err - } - + // Redirect to the by_permalink route so the destination handler can + // honor "has the link" as access to private playlists/albums. if isFull { - return app.redirectWithPreservedParams(c, "/v1/full/playlists/"+playlistId, fiber.StatusFound) + return app.redirectWithPreservedParams(c, "/v1/full/playlists/by_permalink/"+handle+"/"+slug, fiber.StatusFound) } - return app.redirectWithPreservedParams(c, "/v1/playlists/"+playlistId, fiber.StatusFound) + return app.redirectWithPreservedParams(c, "/v1/playlists/by_permalink/"+handle+"/"+slug, fiber.StatusFound) } // Try to match user URL diff --git a/api/v1_tracks_test.go b/api/v1_tracks_test.go index 2a999f9a..3b5d25c6 100644 --- a/api/v1_tracks_test.go +++ b/api/v1_tracks_test.go @@ -78,6 +78,25 @@ func TestGetTracksByISRC(t *testing.T) { } } +func TestGetUnlistedTrackByPermalinkAnonymous(t *testing.T) { + app := testAppWithFixtures(t) + ctx := context.Background() + require.NotNil(t, app.writePool, "test requires write pool") + + // Mark the permalink fixture track (track_id=500) as unlisted. + _, err := app.writePool.Exec(ctx, `UPDATE tracks SET is_unlisted = true WHERE track_id = 500 AND is_current = true`) + require.NoError(t, err) + + // Anonymous request via permalink returns the unlisted track. + var resp struct { + Data []dbv1.Track + } + status, _ := testGet(t, app, "/v1/full/tracks?permalink=/TracksByPermalink/track-by-permalink", &resp) + assert.Equal(t, 200, status) + assert.Len(t, resp.Data, 1, "permalink lookup must return the unlisted track even without auth") + assert.Equal(t, "track by permalink", resp.Data[0].Title.String) +} + func TestGetTracksExcludesAccessAuthorities(t *testing.T) { app := testAppWithFixtures(t) ctx := context.Background() From 1e4ed6e9196c89d50ce9e6366801785d6c4b9e82 Mon Sep 17 00:00:00 2001 From: Dylan Jeffers Date: Fri, 22 May 2026 13:51:48 -0700 Subject: [PATCH 2/2] fix(playlists): preserve user-listing privacy semantics and owner test The new is_private filter inside Playlists() was double-filtering when the caller had already enforced its own privacy logic via an outer query (v1UserPlaylists, v1UserAlbums), stripping private playlists/albums even for the owner's filter_playlists=private path. Pass IncludePrivate=true from those endpoints so the outer query stays the source of truth. TestGetPlaylistPrivateOwnerAllowed authenticated via ?user_id= alone, which the auth middleware rejects with 403 when no wallet is supplied and user 7's fixture wallet has no test signature. Bypass the check via skipAuthCheck so the test can exercise the owner path. Co-Authored-By: Claude Opus 4.7 (1M context) --- api/v1_playlists_test.go | 3 +++ api/v1_users_albums.go | 8 ++++++-- api/v1_users_playlists.go | 8 ++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/api/v1_playlists_test.go b/api/v1_playlists_test.go index f507f3b9..5d7893de 100644 --- a/api/v1_playlists_test.go +++ b/api/v1_playlists_test.go @@ -120,6 +120,9 @@ func TestGetPlaylistPrivateAnonymous404(t *testing.T) { // The single playlist endpoint must return private playlists to their owner. func TestGetPlaylistPrivateOwnerAllowed(t *testing.T) { app := testAppWithFixtures(t) + // user 7's fixture wallet has no test signature, so bypass the auth + // middleware and let user_id alone identify the owner for this test. + app.skipAuthCheck = true ctx := context.Background() require.NotNil(t, app.writePool, "test requires write pool") diff --git a/api/v1_users_albums.go b/api/v1_users_albums.go index 8b67f65b..bf9dd595 100644 --- a/api/v1_users_albums.go +++ b/api/v1_users_albums.go @@ -82,10 +82,14 @@ func (app *ApiServer) v1UserAlbums(c *fiber.Ctx) error { return err } + // Privacy was already enforced by the outer query via albumFilter, so + // allow Playlists() to hydrate private albums when the caller has + // authorization (e.g. filter_albums=private for the owner). albums, err := app.queries.Playlists(c.Context(), dbv1.PlaylistsParams{ GetPlaylistsParams: dbv1.GetPlaylistsParams{ - Ids: ids, - MyID: myId, + Ids: ids, + MyID: myId, + IncludePrivate: true, }, OmitTracks: true, AuthedWallet: app.tryGetAuthedWallet(c), diff --git a/api/v1_users_playlists.go b/api/v1_users_playlists.go index dcfaad37..e8c1c325 100644 --- a/api/v1_users_playlists.go +++ b/api/v1_users_playlists.go @@ -82,10 +82,14 @@ func (app *ApiServer) v1UserPlaylists(c *fiber.Ctx) error { return err } + // Privacy was already enforced by the outer query via playlistFilter, so + // allow Playlists() to hydrate private playlists when the caller has + // authorization (e.g. filter_playlists=private for the owner). playlists, err := app.queries.Playlists(c.Context(), dbv1.PlaylistsParams{ GetPlaylistsParams: dbv1.GetPlaylistsParams{ - Ids: ids, - MyID: myId, + Ids: ids, + MyID: myId, + IncludePrivate: true, }, OmitTracks: true, AuthedWallet: app.tryGetAuthedWallet(c),