feat(server/m7-352): 403 system_playlist_readonly on playlist edit endpoints
Push kind='system' guard into each of the five Service edit methods (Update, Delete, AppendTracks, RemoveTrack, Reorder) immediately after the ownership check; map the typed error to 403 in writePlaylistErr; add table-driven test covering all five verbs against a seeded system playlist.
This commit is contained in:
@@ -420,6 +420,8 @@ func (h *handlers) writePlaylistErr(w http.ResponseWriter, err error, op string)
|
|||||||
writeErr(w, http.StatusNotFound, "not_found", "playlist not found")
|
writeErr(w, http.StatusNotFound, "not_found", "playlist not found")
|
||||||
case errors.Is(err, playlists.ErrForbidden):
|
case errors.Is(err, playlists.ErrForbidden):
|
||||||
writeErr(w, http.StatusForbidden, "not_authorized", "you don't own this playlist")
|
writeErr(w, http.StatusForbidden, "not_authorized", "you don't own this playlist")
|
||||||
|
case errors.Is(err, playlists.ErrSystemPlaylistReadonly):
|
||||||
|
writeErr(w, http.StatusForbidden, "system_playlist_readonly", "system-generated playlists cannot be edited")
|
||||||
case errors.Is(err, playlists.ErrInvalidInput):
|
case errors.Is(err, playlists.ErrInvalidInput):
|
||||||
writeErr(w, http.StatusBadRequest, "bad_request", err.Error())
|
writeErr(w, http.StatusBadRequest, "bad_request", err.Error())
|
||||||
case errors.Is(err, playlists.ErrTrackNotFound):
|
case errors.Is(err, playlists.ErrTrackNotFound):
|
||||||
|
|||||||
@@ -569,3 +569,49 @@ func TestListPlaylists_BadKindReturns400(t *testing.T) {
|
|||||||
t.Fatalf("expected 400; got %d, body=%s", w.Code, w.Body.String())
|
t.Fatalf("expected 400; got %d, body=%s", w.Code, w.Body.String())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestEditEndpoints_403OnSystemPlaylist verifies that all five edit verbs
|
||||||
|
// return 403 system_playlist_readonly when the target playlist has kind='system'.
|
||||||
|
func TestEditEndpoints_403OnSystemPlaylist(t *testing.T) {
|
||||||
|
h, pool := testHandlers(t)
|
||||||
|
u := seedUser(t, pool, "sysedit", "pw", false)
|
||||||
|
|
||||||
|
// Seed a system playlist directly via SQL (BuildSystemPlaylists requires
|
||||||
|
// play_events seeding which we already exercise in playlists package tests).
|
||||||
|
var sid string
|
||||||
|
if err := pool.QueryRow(context.Background(), `
|
||||||
|
INSERT INTO playlists (user_id, name, description, is_public, kind, system_variant)
|
||||||
|
VALUES ($1, 'For You', '', false, 'system', 'for_you')
|
||||||
|
RETURNING id::text
|
||||||
|
`, u.ID).Scan(&sid); err != nil {
|
||||||
|
t.Fatalf("seed system playlist: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
method string
|
||||||
|
path string
|
||||||
|
body []byte
|
||||||
|
}{
|
||||||
|
{"PATCH", "PATCH", "/api/playlists/" + sid, []byte(`{"name":"new name"}`)},
|
||||||
|
{"DELETE_playlist", "DELETE", "/api/playlists/" + sid, nil},
|
||||||
|
{"POST_tracks", "POST", "/api/playlists/" + sid + "/tracks", []byte(`{"track_ids":["00000000-0000-0000-0000-000000000000"]}`)},
|
||||||
|
{"PUT_tracks_reorder", "PUT", "/api/playlists/" + sid + "/tracks", []byte(`{"ordered_positions":[]}`)},
|
||||||
|
{"DELETE_track_at_position", "DELETE", "/api/playlists/" + sid + "/tracks/0", nil},
|
||||||
|
}
|
||||||
|
for _, c := range cases {
|
||||||
|
t.Run(c.name, func(t *testing.T) {
|
||||||
|
w := doPlaylistsReq(h, u, c.method, c.path, c.body)
|
||||||
|
if w.Code != http.StatusForbidden {
|
||||||
|
t.Fatalf("status: got %d want 403; body=%s", w.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
var env errorBody
|
||||||
|
if err := json.Unmarshal(w.Body.Bytes(), &env); err != nil {
|
||||||
|
t.Fatalf("decode envelope: %v", err)
|
||||||
|
}
|
||||||
|
if env.Error.Code != "system_playlist_readonly" {
|
||||||
|
t.Errorf("code: got %q want system_playlist_readonly; full body=%s", env.Error.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -233,6 +233,9 @@ func (s *Service) Update(ctx context.Context, callerID, playlistID pgtype.UUID,
|
|||||||
if !pgtypeUUIDEqual(existing.UserID, callerID) {
|
if !pgtypeUUIDEqual(existing.UserID, callerID) {
|
||||||
return nil, ErrForbidden
|
return nil, ErrForbidden
|
||||||
}
|
}
|
||||||
|
if existing.Kind == "system" {
|
||||||
|
return nil, ErrSystemPlaylistReadonly
|
||||||
|
}
|
||||||
|
|
||||||
params := dbq.UpdatePlaylistParams{
|
params := dbq.UpdatePlaylistParams{
|
||||||
ID: playlistID,
|
ID: playlistID,
|
||||||
@@ -281,6 +284,9 @@ func (s *Service) Delete(ctx context.Context, callerID, playlistID pgtype.UUID)
|
|||||||
if !pgtypeUUIDEqual(existing.UserID, callerID) {
|
if !pgtypeUUIDEqual(existing.UserID, callerID) {
|
||||||
return ErrForbidden
|
return ErrForbidden
|
||||||
}
|
}
|
||||||
|
if existing.Kind == "system" {
|
||||||
|
return ErrSystemPlaylistReadonly
|
||||||
|
}
|
||||||
|
|
||||||
deleted, err := q.DeletePlaylist(ctx, playlistID)
|
deleted, err := q.DeletePlaylist(ctx, playlistID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -341,6 +347,9 @@ func (s *Service) AppendTracks(ctx context.Context, callerID, playlistID pgtype.
|
|||||||
if !pgtypeUUIDEqual(pl.UserID, callerID) {
|
if !pgtypeUUIDEqual(pl.UserID, callerID) {
|
||||||
return ErrForbidden
|
return ErrForbidden
|
||||||
}
|
}
|
||||||
|
if pl.Kind == "system" {
|
||||||
|
return ErrSystemPlaylistReadonly
|
||||||
|
}
|
||||||
|
|
||||||
tx, err := s.pool.Begin(ctx)
|
tx, err := s.pool.Begin(ctx)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -393,6 +402,9 @@ func (s *Service) RemoveTrack(ctx context.Context, callerID, playlistID pgtype.U
|
|||||||
if !pgtypeUUIDEqual(pl.UserID, callerID) {
|
if !pgtypeUUIDEqual(pl.UserID, callerID) {
|
||||||
return ErrForbidden
|
return ErrForbidden
|
||||||
}
|
}
|
||||||
|
if pl.Kind == "system" {
|
||||||
|
return ErrSystemPlaylistReadonly
|
||||||
|
}
|
||||||
|
|
||||||
tx, err := s.pool.Begin(ctx)
|
tx, err := s.pool.Begin(ctx)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -442,6 +454,9 @@ func (s *Service) Reorder(ctx context.Context, callerID, playlistID pgtype.UUID,
|
|||||||
if !pgtypeUUIDEqual(pl.UserID, callerID) {
|
if !pgtypeUUIDEqual(pl.UserID, callerID) {
|
||||||
return ErrForbidden
|
return ErrForbidden
|
||||||
}
|
}
|
||||||
|
if pl.Kind == "system" {
|
||||||
|
return ErrSystemPlaylistReadonly
|
||||||
|
}
|
||||||
|
|
||||||
if int32(len(orderedPositions)) != pl.TrackCount {
|
if int32(len(orderedPositions)) != pl.TrackCount {
|
||||||
return fmt.Errorf("%w: expected %d positions, got %d", ErrInvalidInput, pl.TrackCount, len(orderedPositions))
|
return fmt.Errorf("%w: expected %d positions, got %d", ErrInvalidInput, pl.TrackCount, len(orderedPositions))
|
||||||
|
|||||||
Reference in New Issue
Block a user