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..5d7893de 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,79 @@ 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) + // 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") + + // 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() 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),