Skip to content

Allow sharing playlists across multiple users #525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ FROM alpine:3.19 AS builder-taglib
WORKDIR /tmp
COPY alpine/taglib/APKBUILD .
RUN apk update && \
apk add --no-cache abuild && \
abuild-keygen -a -n && \
apk add --no-cache abuild doas && \
echo "permit nopass root" > /etc/doas.conf && \
abuild-keygen -a -n -i && \
REPODEST=/pkgs abuild -F -r

FROM golang:1.21-alpine AS builder
Expand Down
61 changes: 51 additions & 10 deletions playlist/playlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io/fs"
"os"
"path/filepath"
"slices"
"strconv"
"strings"
"sync"
Expand All @@ -27,12 +28,25 @@ const (
)

type Playlist struct {
UpdatedAt time.Time
UserID int
Name string
Comment string
Items []string
IsPublic bool
UpdatedAt time.Time
UserID int
Name string
Comment string
Items []string
IsPublic bool
SharedWith []int
}

func (p Playlist) CanRead(uid int) bool {
return p.IsPublic || p.CanWrite(uid)
}

func (p Playlist) CanWrite(uid int) bool {
return p.UserID == uid || slices.Contains(p.SharedWith, uid)
}

func (p Playlist) CanDelete(uid int) bool {
return p.UserID == uid
}

type Store struct {
Expand Down Expand Up @@ -94,6 +108,10 @@ func (s *Store) Read(relPath string) (*Playlist, error) {
return nil, fmt.Errorf("stat m3u: %w", err)
}

if stat.IsDir() {
return nil, errors.New("path is a directory")
}

var playlist Playlist
playlist.UpdatedAt = stat.ModTime()

Expand Down Expand Up @@ -122,6 +140,20 @@ func (s *Store) Read(relPath string) (*Playlist, error) {
playlist.Comment = value
case attrIsPublic:
playlist.IsPublic, _ = strconv.ParseBool(value)
case attrSharedWith:
sharedWith := strings.Split(value, ",")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently thinking whether it would be better for this to be a comma-separated list (as it is now), or if we should use the comment multiple times 🤔

#GONIC-NAME:"Roobre"
#GONIC-COMMENT:""
#GONIC-IS-PUBLIC:"false"
#GONIC-SHARED-WITH:"2"
#GONIC-SHARED-WITH:"3"

The latter sounds easier to parse.

if len(sharedWith) == 0 {
continue
}

for _, idStr := range sharedWith {
id, err := strconv.ParseInt(idStr, 10, 64)
if err != nil {
continue
}

playlist.SharedWith = append(playlist.SharedWith, int(id))
}
}
if strings.HasPrefix(line, "#") {
continue
Expand Down Expand Up @@ -177,6 +209,14 @@ func (s *Store) Write(relPath string, playlist *Playlist) error {
fmt.Fprintln(file, encodeAttr(attrName, playlist.Name))
fmt.Fprintln(file, encodeAttr(attrCommment, playlist.Comment))
fmt.Fprintln(file, encodeAttr(attrIsPublic, fmt.Sprint(playlist.IsPublic)))
if len(playlist.SharedWith) != 0 {
sharedWithStr := make([]string, 0, len(playlist.SharedWith))
for _, id := range playlist.SharedWith {
sharedWithStr = append(sharedWithStr, strconv.Itoa(id))
}
fmt.Fprintln(file, encodeAttr(attrSharedWith, strings.Join(sharedWithStr, ",")))
}

for _, line := range playlist.Items {
fmt.Fprintln(file, line)
}
Expand Down Expand Up @@ -233,10 +273,11 @@ func sanityCheck(basePath string) error {
}

const (
attrPrefix = "#GONIC-"
attrName = "NAME"
attrCommment = "COMMENT"
attrIsPublic = "IS-PUBLIC"
attrPrefix = "#GONIC-"
attrName = "NAME"
attrCommment = "COMMENT"
attrIsPublic = "IS-PUBLIC"
attrSharedWith = "SHARED-WITH"
)

func encodeAttr(name, value string) string {
Expand Down
71 changes: 70 additions & 1 deletion playlist/playlist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ It has multiple lines 👍
"item 2.flac",
"item 3.flac",
},
IsPublic: true,
IsPublic: true,
SharedWith: []int{2, 3},
}

newPath := playlist.NewPath(before.UserID, before.Name)
Expand All @@ -50,8 +51,76 @@ It has multiple lines 👍
require.Equal(t, after.Comment, before.Comment)
require.Equal(t, after.Items, before.Items)
require.Equal(t, after.IsPublic, before.IsPublic)
require.Equal(t, after.SharedWith, before.SharedWith)

playlistIDs, err = store.List()
require.NoError(t, err)
require.True(t, len(playlistIDs) == 1)
}

func TestAccess(t *testing.T) {
t.Parallel()

for _, tc := range []struct {
name string
playlist playlist.Playlist
userID int
expectRead bool
expectWrite bool
expectDelete bool
}{
{
name: "owner can do anything",
playlist: playlist.Playlist{
UserID: 7,
},
userID: 7,
expectRead: true,
expectWrite: true,
expectDelete: true,
},
{
name: "third party cannot do anything",
playlist: playlist.Playlist{
UserID: 7,
},
userID: 99,
expectRead: false,
expectWrite: false,
expectDelete: false,
},
{
name: "third party can read if public",
playlist: playlist.Playlist{
IsPublic: true,
UserID: 7,
},
userID: 99,
expectRead: true,
expectWrite: false,
expectDelete: false,
},
{
name: "shared user can read and write",
playlist: playlist.Playlist{
IsPublic: true,
UserID: 7,
SharedWith: []int{99},
},
userID: 99,
expectRead: true,
expectWrite: true,
expectDelete: false,
},
} {
tc := tc

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

require.Equal(t, tc.expectRead, tc.playlist.CanRead(tc.userID))
require.Equal(t, tc.expectWrite, tc.playlist.CanWrite(tc.userID))
require.Equal(t, tc.expectDelete, tc.playlist.CanDelete(tc.userID))
})
}
}
29 changes: 22 additions & 7 deletions server/ctrlsubsonic/handlers_playlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (c *Controller) ServeGetPlaylists(r *http.Request) *spec.Response {
if err != nil {
return spec.NewError(0, "error reading playlist %q: %v", path, err)
}
if playlist.UserID != user.ID && !playlist.IsPublic {
if !playlist.CanRead(user.ID) {
continue
}
playlistID := playlistIDEncode(path)
Expand All @@ -49,13 +49,14 @@ 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.GetFirst("id", "playlistId")
if err != nil {
return spec.NewError(10, "please provide an `id` parameter")
}
playlist, err := c.playlistStore.Read(playlistIDDecode(playlistID))
if err != nil {
if err != nil || !playlist.CanRead(user.ID) {
return spec.NewError(70, "playlist with id %s not found", playlistID)
}
sub := spec.NewResponse()
Expand All @@ -75,11 +76,13 @@ func (c *Controller) ServeCreateOrUpdatePlaylist(r *http.Request) *spec.Response
playlistPath := playlistIDDecode(playlistID)

var playlist playlistp.Playlist
if pl, _ := c.playlistStore.Read(playlistPath); pl != nil {
playlist = *pl
if playlistPath != "" {
if pl, err := c.playlistStore.Read(playlistPath); err != nil && pl != nil {
playlist = *pl
}
}

if playlist.UserID != 0 && playlist.UserID != user.ID {
if playlist.UserID != 0 && !playlist.CanWrite(user.ID) {
return spec.NewError(50, "you aren't allowed update that user's playlist")
}

Expand Down Expand Up @@ -130,7 +133,7 @@ func (c *Controller) ServeUpdatePlaylist(r *http.Request) *spec.Response {
}

// update meta info
if playlist.UserID != 0 && playlist.UserID != user.ID {
if !playlist.CanWrite(user.ID) {
return spec.NewResponse()
}

Expand Down Expand Up @@ -170,9 +173,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 := params.GetFirstOr( /* default */ "", "id", "playlistId")
if err := c.playlistStore.Delete(playlistIDDecode(playlistID)); err != nil {
playlistPath := playlistIDDecode(playlistID)
playlist, err := c.playlistStore.Read(playlistPath)
if err != nil {
return spec.NewError(0, "find playlist: %v", err)
}

if !playlist.CanDelete(user.ID) {
return spec.NewError(0, "you cannot delete playlists you do not own")
}

if err := c.playlistStore.Delete(playlistPath); err != nil {
return spec.NewError(0, "delete playlist: %v", err)
}
return spec.NewResponse()
Expand Down
Loading