From 90a47d59b5bab59e6f9697568f067130391edafe Mon Sep 17 00:00:00 2001 From: Phan An Date: Tue, 4 Jun 2024 11:44:33 +0200 Subject: [PATCH] fix: only consider episodes accessible if subscribed to podcasts --- app/Builders/SongBuilder.php | 21 +++++++++---- app/Enums/SmartPlaylistModel.php | 4 +++ .../API/PlaylistSongController.php | 4 +-- .../API/AddSongsToPlaylistRequest.php | 7 ++--- .../Requests/API/PlaylistStoreRequest.php | 5 ++-- app/Models/Song.php | 1 + app/Rules/AllPlayablesAreAccessibleBy.php | 30 +++++++++++++++++++ 7 files changed, 58 insertions(+), 14 deletions(-) create mode 100644 app/Rules/AllPlayablesAreAccessibleBy.php diff --git a/app/Builders/SongBuilder.php b/app/Builders/SongBuilder.php index 66028ec6..ce173047 100644 --- a/app/Builders/SongBuilder.php +++ b/app/Builders/SongBuilder.php @@ -69,17 +69,28 @@ class SongBuilder extends Builder ); } - public function accessibleBy(User $user, bool $withTableName = true): self + public function accessibleBy(User $user): self { if (License::isCommunity()) { // In the Community Edition, all songs are accessible by all users. return $this; } - return $this->where(static function (Builder $query) use ($user, $withTableName): void { - $query->where(($withTableName ? 'songs.' : '') . 'is_public', true) - ->orWhere(($withTableName ? 'songs.' : '') . 'owner_id', $user->id); - }); + // We want to alias both podcasts and podcast_user tables to avoid possible conflicts with other joins. + return $this->leftJoin('podcasts as podcasts_a11y', 'songs.podcast_id', 'podcasts_a11y.id') + ->leftJoin('podcast_user as podcast_user_a11y', static function (JoinClause $join) use ($user): void { + $join->on('podcasts_a11y.id', 'podcast_user_a11y.podcast_id') + ->where('podcast_user_a11y.user_id', $user->id); + }) + ->where(static function (Builder $query) use ($user): void { + // Songs must be public or owned by the user. + $query->where('songs.is_public', true) + ->orWhere('songs.owner_id', $user->id); + })->whereNot(static function (Builder $query): void { + // Episodes must belong to a podcast that the user is not subscribed to. + $query->whereNotNull('songs.podcast_id') + ->whereNull('podcast_user_a11y.podcast_id'); + }); } private function sortByOneColumn(string $column, string $direction): self diff --git a/app/Enums/SmartPlaylistModel.php b/app/Enums/SmartPlaylistModel.php index dc73d036..5c0beb6b 100644 --- a/app/Enums/SmartPlaylistModel.php +++ b/app/Enums/SmartPlaylistModel.php @@ -19,6 +19,10 @@ enum SmartPlaylistModel: string public function toColumnName(): string { return match ($this) { + self::TITLE => 'songs.title', + self::LENGTH => 'songs.length', + self::GENRE => 'songs.genre', + self::YEAR => 'songs.year', self::ALBUM_NAME => 'albums.name', self::ARTIST_NAME => 'artists.name', self::DATE_ADDED => 'songs.created_at', diff --git a/app/Http/Controllers/API/PlaylistSongController.php b/app/Http/Controllers/API/PlaylistSongController.php index 9eb53fee..c2db4a26 100644 --- a/app/Http/Controllers/API/PlaylistSongController.php +++ b/app/Http/Controllers/API/PlaylistSongController.php @@ -47,10 +47,10 @@ class PlaylistSongController extends Controller $this->authorize('collaborate', $playlist); - $songs = $this->songRepository->getMany(ids: $request->songs, scopedUser: $this->user); + $playables = $this->songRepository->getMany(ids: $request->songs, scopedUser: $this->user); return self::createResourceCollection( - $this->playlistService->addPlayablesToPlaylist($playlist, $songs, $this->user) + $this->playlistService->addPlayablesToPlaylist($playlist, $playables, $this->user) ); } diff --git a/app/Http/Requests/API/AddSongsToPlaylistRequest.php b/app/Http/Requests/API/AddSongsToPlaylistRequest.php index 37a185e7..3df73f9e 100644 --- a/app/Http/Requests/API/AddSongsToPlaylistRequest.php +++ b/app/Http/Requests/API/AddSongsToPlaylistRequest.php @@ -2,19 +2,18 @@ namespace App\Http\Requests\API; -use App\Models\Song; -use Illuminate\Validation\Rule; +use App\Rules\AllPlayablesAreAccessibleBy; /** * @property-read array $songs */ class AddSongsToPlaylistRequest extends Request { - /** @return array */ + /** @inheritdoc */ public function rules(): array { return [ - 'songs' => ['required', 'array', Rule::exists(Song::class, 'id')], + 'songs' => ['required', 'array', new AllPlayablesAreAccessibleBy($this->user())], ]; } } diff --git a/app/Http/Requests/API/PlaylistStoreRequest.php b/app/Http/Requests/API/PlaylistStoreRequest.php index 0b8df052..41eafa8f 100644 --- a/app/Http/Requests/API/PlaylistStoreRequest.php +++ b/app/Http/Requests/API/PlaylistStoreRequest.php @@ -3,7 +3,7 @@ namespace App\Http\Requests\API; use App\Models\PlaylistFolder; -use App\Models\Song; +use App\Rules\AllPlayablesAreAccessibleBy; use App\Rules\ValidSmartPlaylistRulePayload; use Illuminate\Validation\Rule; @@ -21,8 +21,7 @@ class PlaylistStoreRequest extends Request { return [ 'name' => 'required', - 'songs' => 'array', - 'songs.*' => [Rule::exists(Song::class, 'id')], + 'songs' => ['array', new AllPlayablesAreAccessibleBy($this->user())], 'rules' => ['array', 'nullable', new ValidSmartPlaylistRulePayload()], 'folder_id' => ['nullable', 'sometimes', Rule::exists(PlaylistFolder::class, 'id')], 'own_songs_only' => 'sometimes', diff --git a/app/Models/Song.php b/app/Models/Song.php index 2ca988a5..27f60717 100644 --- a/app/Models/Song.php +++ b/app/Models/Song.php @@ -54,6 +54,7 @@ use Throwable; * // The following are only available for collaborative playlists * @property-read ?string $collaborator_email The email of the user who added the song to the playlist * @property-read ?string $collaborator_name The name of the user who added the song to the playlist + * @property-read ?string $collaborator_avatar The avatar of the user who added the song to the playlist * @property-read ?int $collaborator_id The ID of the user who added the song to the playlist * @property-read ?string $added_at The date the song was added to the playlist * @property-read PlayableType $type diff --git a/app/Rules/AllPlayablesAreAccessibleBy.php b/app/Rules/AllPlayablesAreAccessibleBy.php new file mode 100644 index 00000000..4bf7f725 --- /dev/null +++ b/app/Rules/AllPlayablesAreAccessibleBy.php @@ -0,0 +1,30 @@ + $value + */ + public function validate(string $attribute, mixed $value, Closure $fail): void + { + $ids = array_unique(Arr::wrap($value)); + + if ($ids && app(SongRepository::class)->getMany(ids: $ids, scopedUser: $this->user)->count() !== count($ids)) { + $fail('Not all songs or episodes exist in the database or are accessible by the user.'); + } + } +}