fix(subsonic): enforce playlist ownership on getPlaylist/deletePlaylist

Author: sentriz

server/ctrlsubsonic/handlers_playlist.go

@@ -49,6 +49,7 @@ func (c *Controller) ServeGetPlaylists(r *http.Request) *spec.Response {
 }
 
 func (c *Controller) ServeGetPlaylist(r *http.Request) *spec.Response {
+	user := r.Context().Value(CtxUser).(*db.User)
 	params := r.Context().Value(CtxParams).(params.Params)
 	playlistID, err := params.GetFirstID("id", "playlistId")
 	if err != nil {
@@ -58,6 +59,9 @@ func (c *Controller) ServeGetPlaylist(r *http.Request) *spec.Response {
 	if err != nil {
 		return spec.NewError(70, "playlist with id %s not found", playlistID)
 	}
+	if playlist.UserID != user.ID && !playlist.IsPublic {
+		return spec.NewError(50, "you aren't allowed to read that user's playlist")
+	}
 	sub := spec.NewResponse()
 	rendered, err := playlistRender(c, params, playlist, playlistID, true)
 	if err != nil {
@@ -175,12 +179,21 @@ func (c *Controller) ServeUpdatePlaylist(r *http.Request) *spec.Response {
 }
 
 func (c *Controller) ServeDeletePlaylist(r *http.Request) *spec.Response {
+	user := r.Context().Value(CtxUser).(*db.User)
 	params := r.Context().Value(CtxParams).(params.Params)
 	playlistID, err := params.GetFirstID("id", "playlistId")
 	if err != nil {
 		return spec.NewError(10, "please provide an `id` or `playlistId` parameter")
 	}
-	if err := c.playlistStore.Delete(playlistIDDecode(playlistID)); err != nil {
+	playlistPath := playlistIDDecode(playlistID)
+	playlist, err := c.playlistStore.Read(playlistPath)
+	if err != nil {
+		return spec.NewError(70, "playlist with id %s not found", playlistID)
+	}
+	if playlist.UserID != 0 && playlist.UserID != user.ID {
+		return spec.NewError(50, "you aren't allowed to delete that user's playlist")
+	}
+	if err := c.playlistStore.Delete(playlistPath); err != nil {
 		return spec.NewError(0, "delete playlist: %v", err)
 	}
 	return spec.NewResponse()

server/ctrlsubsonic/handlers_playlist_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	playlistp "go.senan.xyz/gonic/playlist"
 	"go.senan.xyz/gonic/server/ctrlsubsonic/spec"
 )
 
@@ -115,3 +116,55 @@ func TestDeletePlaylist(t *testing.T) {
 		query{url.Values{}, "after_delete", false},
 	)
 }
+
+func TestGetPlaylistDeniesOtherUsersPrivate(t *testing.T) {
+	t.Parallel()
+	f := newFixture(t)
+
+	privateID := writePrivatePlaylist(t, f)
+
+	body := f.query(t, f.contr.ServeGetPlaylist, f.alt, url.Values{
+		"id": {privateID},
+	})
+	var sub spec.SubsonicResponse
+	if err := json.Unmarshal([]byte(body), &sub); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if sub.Response.Status != "failed" || sub.Response.Error == nil || sub.Response.Error.Code != 50 {
+		t.Fatalf("expected error 50, got: %s", body)
+	}
+}
+
+func TestDeletePlaylistDeniesOtherUsers(t *testing.T) {
+	t.Parallel()
+	f := newFixture(t)
+
+	body := f.query(t, f.contr.ServeDeletePlaylist, f.alt, url.Values{
+		"id": {f.sharedPlaylistID()},
+	})
+	var sub spec.SubsonicResponse
+	if err := json.Unmarshal([]byte(body), &sub); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if sub.Response.Status != "failed" || sub.Response.Error == nil || sub.Response.Error.Code != 50 {
+		t.Fatalf("expected error 50, got: %s", body)
+	}
+	if _, err := f.contr.playlistStore.Read(filepath.Join("1", "shared.m3u")); err != nil {
+		t.Fatalf("playlist was deleted despite auth failure: %v", err)
+	}
+}
+
+func writePrivatePlaylist(t *testing.T, f *fixture) string {
+	t.Helper()
+	relPath := filepath.Join("1", "private.m3u")
+	err := f.contr.playlistStore.Write(relPath, &playlistp.Playlist{
+		UserID:    f.admin.ID,
+		UpdatedAt: time.Date(2020, 5, 1, 12, 0, 0, 0, time.UTC),
+		Name:      "private playlist",
+		IsPublic:  false,
+	})
+	if err != nil {
+		t.Fatalf("write private playlist: %v", err)
+	}
+	return playlistIDEncode(relPath).String()
+}