From c1e46910b06482b4d393b5b97d281e1c8d328707 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Wed, 8 Jul 2026 00:23:25 +0200 Subject: [PATCH] feat(music): move playlist to authz engine --- .../20260910000000_role_grants_playlist.sql | 63 +++ ...10000001_backfill_playlist_role_grants.sql | 90 ++++ src/application/dtos/grant_dto.rs | 3 + src/application/ports/authorization_ports.rs | 1 + .../services/magic_link_invite_service.rs | 38 +- src/application/services/music_service.rs | 446 +++++++++--------- .../recipient_notification_service.rs | 15 +- src/domain/services/authorization.rs | 20 +- src/infrastructure/services/pg_acl_engine.rs | 30 +- src/interfaces/api/handlers/grant_handler.rs | 10 +- tests/api/playlists.hurl | 408 ++++++++++++++++ tests/api/run.sh | 1 + 12 files changed, 856 insertions(+), 269 deletions(-) create mode 100644 migrations/20260910000000_role_grants_playlist.sql create mode 100644 migrations/20260910000001_backfill_playlist_role_grants.sql create mode 100644 tests/api/playlists.hurl diff --git a/migrations/20260910000000_role_grants_playlist.sql b/migrations/20260910000000_role_grants_playlist.sql new file mode 100644 index 00000000..3b5770b7 --- /dev/null +++ b/migrations/20260910000000_role_grants_playlist.sql @@ -0,0 +1,63 @@ +-- ───────────────────────────────────────────────────────────────────────── +-- Round 3 (Music) — admit 'playlist' into +-- `storage.role_grants.resource_type`. +-- +-- Companion to the domain unblock in +-- `src/domain/services/authorization.rs`: uncomments +-- `Resource::Playlist(Uuid)` and its `type_str` / `id` / `from_parts` +-- arms. Nothing can insert `('playlist', …)` into `role_grants` until +-- the CHECK constraint permits the discriminator. +-- +-- The music surface historically enforced access via a dedicated +-- `audio.playlist_shares` table and bespoke +-- `MusicStorageAdapter::{user_has_access, user_can_write}` helpers. +-- Round 3 folds them into the unified ReBAC engine, giving playlists +-- the same treatment already applied to calendars and address books: +-- +-- * A single ACL source of truth (`storage.role_grants`) covers +-- every OxiCloud resource type — files, folders, drives, +-- calendars, address books, playlists. +-- * Group subjects become a free feature on playlist shares. +-- * The `authz.require` audit line ("👮🏻‍♂️ perms: ⛔ …") fires on +-- denial with no per-domain retrofit. +-- +-- Owner + share backfill from `audio.playlist_shares` happens in the +-- companion migration. The legacy table stays in place through this +-- PR for rollback safety; a follow-up migration one release later +-- drops it. + +-- `resource_type` is a TEXT column with a CHECK constraint (not a PG +-- enum), so extending it is a DROP / ADD pair — no `ALTER TYPE` / +-- non-transactional migration issues. + +ALTER TABLE storage.role_grants + DROP CONSTRAINT IF EXISTS role_grants_resource_type_check; + +ALTER TABLE storage.role_grants + ADD CONSTRAINT role_grants_resource_type_check + CHECK (resource_type IN ('folder', 'file', 'drive', 'calendar', 'address_book', 'playlist')); + +-- Post-flight: introspect the live constraint definition and prove +-- 'playlist' appears. Cheap read-only check with no INSERT. +DO $BODY$ +DECLARE + defn TEXT; +BEGIN + SELECT pg_get_constraintdef(c.oid) INTO defn + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE n.nspname = 'storage' + AND t.relname = 'role_grants' + AND c.conname = 'role_grants_resource_type_check'; + + IF defn IS NULL THEN + RAISE EXCEPTION + 'role_grants_resource_type_check not found on storage.role_grants'; + END IF; + IF position('playlist' IN defn) = 0 THEN + RAISE EXCEPTION + 'CHECK constraint does not admit ''playlist'': %', defn; + END IF; +END; +$BODY$; diff --git a/migrations/20260910000001_backfill_playlist_role_grants.sql b/migrations/20260910000001_backfill_playlist_role_grants.sql new file mode 100644 index 00000000..d1f45107 --- /dev/null +++ b/migrations/20260910000001_backfill_playlist_role_grants.sql @@ -0,0 +1,90 @@ +-- ───────────────────────────────────────────────────────────────────────── +-- Round 3 (Music) Phase 2 — backfill role_grants from the legacy +-- per-domain share table. +-- +-- Companion to `20260910000000_role_grants_playlist.sql` (Phase 1: +-- CHECK constraint extension). This migration seeds +-- `storage.role_grants` with: +-- +-- 1. Owner grants for every existing playlist — replaces the +-- implicit "owner via `audio.playlists.owner_id`" short-circuit +-- that the bespoke `user_has_access` / `user_can_write` helpers +-- used. +-- 2. Non-owner grants translated from `audio.playlist_shares` — +-- existing "shared with me" relationships keep working after the +-- Phase 3 service rewrite starts reading grants from +-- `role_grants` only. +-- +-- The legacy `audio.playlist_shares` table stays in place through +-- this PR for rollback safety. It gets dropped in a follow-up +-- migration one release later, once the new engine path bakes. +-- +-- Idempotent: every INSERT uses `ON CONFLICT DO NOTHING` on the +-- `(subject_type, subject_id, resource_type, resource_id)` unique +-- key so a re-run (or a duplicate row in the legacy table where +-- someone shared with themselves) is a no-op. + +-- ── 1. Owner grants for playlists ─────────────────────────────────────── +-- +-- One row per playlist. `granted_by = owner_id` is the self-seeded +-- creation event — matches the pattern used by the calendar / +-- address-book backfill and by the drive lifecycle hook for personal +-- drives. +INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) +SELECT 'user', p.owner_id, 'playlist', p.id, 'owner'::storage.grant_role, p.owner_id + FROM audio.playlists p +ON CONFLICT (subject_type, subject_id, resource_type, resource_id) + DO NOTHING; + +-- ── 2. Non-owner grants from playlist_shares ──────────────────────────── +-- +-- `audio.playlist_shares.can_write` is a BOOLEAN. Map: +-- - `false` → `viewer` (bundle: Read only) +-- - `true` → `editor` (bundle: Read + Update) +-- +-- `granted_by` = playlist owner, since the legacy share table didn't +-- track the granter. Best available signal — the owner is the only +-- principal who could have created the share via the legacy code path. +INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) +SELECT + 'user', + s.user_id, + 'playlist', + s.playlist_id, + (CASE WHEN s.can_write THEN 'editor' ELSE 'viewer' END)::storage.grant_role, + p.owner_id + FROM audio.playlist_shares s + JOIN audio.playlists p ON p.id = s.playlist_id + WHERE s.user_id <> p.owner_id -- skip self-shares (owner grant already covers them) +ON CONFLICT (subject_type, subject_id, resource_type, resource_id) + DO NOTHING; + +-- ── 3. Post-flight sanity ─────────────────────────────────────────────── +-- +-- Every playlist must now have an owner role_grant. If any row is +-- missing one, the Phase 3 service rewrite would lock owners out of +-- their own resources — refuse to leave the migration in that state. +DO $BODY$ +DECLARE + missing_owners BIGINT; +BEGIN + SELECT COUNT(*) INTO missing_owners + FROM audio.playlists p + WHERE NOT EXISTS ( + SELECT 1 FROM storage.role_grants g + WHERE g.subject_type = 'user' + AND g.subject_id = p.owner_id + AND g.resource_type = 'playlist' + AND g.resource_id = p.id + AND g.role = 'owner'::storage.grant_role + ); + + IF missing_owners > 0 THEN + RAISE EXCEPTION + 'Round 3 (Music) backfill left % playlists without an Owner role_grant', + missing_owners; + END IF; +END; +$BODY$; diff --git a/src/application/dtos/grant_dto.rs b/src/application/dtos/grant_dto.rs index c1941fa5..13c94bf5 100644 --- a/src/application/dtos/grant_dto.rs +++ b/src/application/dtos/grant_dto.rs @@ -62,6 +62,7 @@ pub enum ResourceTypeDto { Drive, Calendar, AddressBook, + Playlist, } #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] @@ -79,6 +80,7 @@ impl From for Resource { ResourceTypeDto::Drive => Resource::Drive(dto.id), ResourceTypeDto::Calendar => Resource::Calendar(dto.id), ResourceTypeDto::AddressBook => Resource::AddressBook(dto.id), + ResourceTypeDto::Playlist => Resource::Playlist(dto.id), } } } @@ -91,6 +93,7 @@ impl From for ResourceDto { Resource::Drive(id) => (ResourceTypeDto::Drive, id), Resource::Calendar(id) => (ResourceTypeDto::Calendar, id), Resource::AddressBook(id) => (ResourceTypeDto::AddressBook, id), + Resource::Playlist(id) => (ResourceTypeDto::Playlist, id), }; ResourceDto { kind, id } } diff --git a/src/application/ports/authorization_ports.rs b/src/application/ports/authorization_ports.rs index 6b10a143..99144483 100644 --- a/src/application/ports/authorization_ports.rs +++ b/src/application/ports/authorization_ports.rs @@ -64,6 +64,7 @@ pub trait AuthorizationEngine: Send + Sync + 'static { Resource::Drive(id) => ("Drive", id), Resource::Calendar(id) => ("Calendar", id), Resource::AddressBook(id) => ("AddressBook", id), + Resource::Playlist(id) => ("Playlist", id), }; // Audit-worthy: denials are the interesting signal. Routed // through the `audit` tracing target so log aggregators can diff --git a/src/application/services/magic_link_invite_service.rs b/src/application/services/magic_link_invite_service.rs index bc858863..d04efdca 100644 --- a/src/application/services/magic_link_invite_service.rs +++ b/src/application/services/magic_link_invite_service.rs @@ -307,17 +307,22 @@ impl MagicLinkInviteService { let (kind, resource_id) = match resource { Resource::Folder(id) => (MagicLinkResourceKind::Folder, id), Resource::File(id) => (MagicLinkResourceKind::File, id), - // Drive / Calendar / AddressBook sharing is out-of-band for - // the magic-link flow. Drive shares land through - // `/api/drives/{id}/members`; Calendar / AddressBook shares - // through the Round-3 `/api/(calendars|address-books)/{id}/shares` - // endpoints. The DTOs accept every `Resource` variant on - // the wire (see `ResourceTypeDto`) but only file/folder - // grants trigger an invitation email. Treating the other - // arms as audit-logged suppressed no-ops keeps the grant - // in place while matching the ineligible-recipient branch + // Drive / Calendar / AddressBook / Playlist sharing is + // out-of-band for the magic-link flow. Drive shares land + // through `/api/drives/{id}/members`; Calendar / + // AddressBook shares through the Round-3 + // `/api/(calendars|address-books)/{id}/shares` endpoints; + // Playlist shares through `/api/playlists/{id}/share`. + // The DTOs accept every `Resource` variant on the wire + // (see `ResourceTypeDto`) but only file/folder grants + // trigger an invitation email. Treating the other arms + // as audit-logged suppressed no-ops keeps the grant in + // place while matching the ineligible-recipient branch // above. - Resource::Drive(_) | Resource::Calendar(_) | Resource::AddressBook(_) => { + Resource::Drive(_) + | Resource::Calendar(_) + | Resource::AddressBook(_) + | Resource::Playlist(_) => { tracing::info!( target: "audit", event = "magic_link.invitation_suppressed", @@ -352,12 +357,13 @@ impl MagicLinkInviteService { Resource::Folder(_) => "server.magic_link.email.kind_folder", Resource::File(_) => "server.magic_link.email.kind_file", // Unreachable — the early-return above exits before we get - // here for Drive / Calendar / AddressBook resources. The - // arms exist only to satisfy exhaustiveness; if you find - // any firing, the early-return was bypassed. - Resource::Drive(_) | Resource::Calendar(_) | Resource::AddressBook(_) => { - "server.magic_link.email.kind_folder" - } + // here for Drive / Calendar / AddressBook / Playlist + // resources. The arms exist only to satisfy exhaustiveness; + // if you find any firing, the early-return was bypassed. + Resource::Drive(_) + | Resource::Calendar(_) + | Resource::AddressBook(_) + | Resource::Playlist(_) => "server.magic_link.email.kind_folder", }; // PR C: render in the recipient's preferred locale (set by UI // switcher, OIDC JIT claim, or inviter inheritance at row diff --git a/src/application/services/music_service.rs b/src/application/services/music_service.rs index d4d4456a..20244251 100644 --- a/src/application/services/music_service.rs +++ b/src/application/services/music_service.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::sync::Arc; use uuid::Uuid; @@ -8,28 +9,77 @@ use crate::application::dtos::playlist_dto::{ use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::application::ports::music_ports::{MusicStoragePort, MusicUseCase}; use crate::common::errors::{DomainError, ErrorKind}; -use crate::domain::services::authorization::{Permission, Resource, Subject}; +use crate::domain::services::authorization::{Permission, Resource, Role, Subject}; use crate::infrastructure::adapters::music_storage_adapter::MusicStorageAdapter; use crate::infrastructure::services::pg_acl_engine::PgAclEngine; +/// Music service — the REST entry point for every playlist or audio +/// metadata operation. Every method routes through +/// `AuthorizationEngine`; the pre-Round-3 `user_has_access` / +/// `user_can_write` bespoke helpers on `MusicStorageAdapter` are no +/// longer consulted for access decisions. +/// +/// Ownership + sharing live entirely in `storage.role_grants` +/// (`resource_type='playlist'`). `audio.playlists.owner_id` stays for +/// provenance and legacy queries; `audio.playlist_shares` is +/// backfilled and slated for removal in a follow-up migration. pub struct MusicService { storage: Arc, - /// ReBAC engine — Round 1 fix from `docs/plan/authz_audit/`. - /// Currently used ONLY by `get_audio_metadata` to close the - /// cross-tenant IDOR (`_user_id: Uuid` was deliberately unused). - /// The full engine rewrite (Round 3 — `Resource::Playlist` + - /// authz.require on every playlist verb) is a separate PR; - /// don't extend the bespoke `user_has_access` / `user_can_write` - /// pattern to new methods, use `require` here instead. - authorization: Arc, + /// ReBAC engine — every user-facing method calls `authz.require` + /// with the appropriate `Permission`. `create_playlist` also uses + /// it to seed an Owner grant for the caller, so the common + /// "owning my own playlist" case takes a single indexed + /// role_grants lookup on subsequent reads. + authz: Arc, } impl MusicService { - pub fn new(storage: Arc, authorization: Arc) -> Self { - Self { - storage, - authorization, - } + pub fn new(storage: Arc, authz: Arc) -> Self { + Self { storage, authz } + } + + /// Parse `playlist_id` and enforce `permission` on + /// `Resource::Playlist(uuid)`. On denial `authz.require` returns + /// `NotFound` (anti-enum — same shape as "no such playlist") and + /// emits the `authz.denied` audit line. Returns the parsed UUID + /// on success so the caller doesn't have to parse it a second + /// time. + async fn require_playlist_perm( + &self, + playlist_id: &str, + caller_id: Uuid, + permission: Permission, + ) -> Result { + let uuid = Uuid::parse_str(playlist_id) + .map_err(|_| DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid ID"))?; + self.authz + .require( + Subject::User(caller_id), + permission, + Resource::Playlist(uuid), + ) + .await?; + Ok(uuid) + } + + /// Check `permission` on a playlist without throwing. Used by the + /// read paths that also allow a public-playlist bypass — they + /// need a bool, not a `Result<(), NotFound>`. + async fn has_playlist_perm( + &self, + playlist_id: &str, + caller_id: Uuid, + permission: Permission, + ) -> Result { + let uuid = Uuid::parse_str(playlist_id) + .map_err(|_| DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid ID"))?; + self.authz + .check( + Subject::User(caller_id), + permission, + Resource::Playlist(uuid), + ) + .await } } @@ -39,7 +89,26 @@ impl MusicUseCase for MusicService { dto: CreatePlaylistDto, user_id: Uuid, ) -> Result { - self.storage.create_playlist(dto, user_id).await + // No pre-write gate: creating a playlist is a personal act. + // Storage stamps `owner_id = user_id`; we then seed an Owner + // role_grant so subsequent reads hit the same + // `storage.role_grants` fast path used everywhere else. + let created = self.storage.create_playlist(dto, user_id).await?; + let playlist_uuid = Uuid::parse_str(&created.id).map_err(|_| { + DomainError::internal_error("Playlist", "storage returned invalid playlist id") + })?; + // `set_role` is idempotent on the `(subject, resource)` unique + // key. `granted_by = user_id` is the self-seeded creation event. + self.authz + .set_role( + user_id, + Subject::User(user_id), + Role::Owner, + Resource::Playlist(playlist_uuid), + None, + ) + .await?; + Ok(created) } async fn update_playlist( @@ -48,45 +117,25 @@ impl MusicUseCase for MusicService { dto: UpdatePlaylistDto, user_id: Uuid, ) -> Result { - let has_access = self.storage.user_has_access(playlist_id, user_id).await?; - if !has_access { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You don't have permission to update this playlist", - )); - } - let can_write = self.storage.user_can_write(playlist_id, user_id).await?; - if !can_write { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You need write access to update this playlist", - )); - } + self.require_playlist_perm(playlist_id, user_id, Permission::Update) + .await?; self.storage.update_playlist(playlist_id, dto).await } async fn delete_playlist(&self, playlist_id: &str, user_id: Uuid) -> Result<(), DomainError> { - let playlist = self.storage.get_playlist(playlist_id).await?; - let playlist = match playlist { - Some(p) => p, - None => { - return Err(DomainError::new( - ErrorKind::NotFound, - "Playlist", - "Playlist not found", - )); - } - }; - if playlist.owner_id != user_id.to_string() { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "Only the owner can delete this playlist", - )); - } - self.storage.delete_playlist(playlist_id).await + let uuid = self + .require_playlist_perm(playlist_id, user_id, Permission::Delete) + .await?; + self.storage.delete_playlist(playlist_id).await?; + // Wipe every grant on this playlist so a re-used UUID + // (impossible today but cheap to defend against) doesn't + // inherit stale ACLs. The storage DELETE won't cascade to + // `storage.role_grants` — it's cross-schema. + let _ = self + .authz + .revoke_all_for_resource(Resource::Playlist(uuid)) + .await; + Ok(()) } async fn get_playlist( @@ -94,23 +143,22 @@ impl MusicUseCase for MusicService { playlist_id: &str, user_id: Uuid, ) -> Result { - let has_access = self.storage.user_has_access(playlist_id, user_id).await?; - if !has_access { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You don't have permission to view this playlist", - )); - } let playlist = self.storage.get_playlist(playlist_id).await?; - match playlist { - Some(p) => Ok(p), - None => Err(DomainError::new( - ErrorKind::NotFound, - "Playlist", - "Playlist not found", - )), + let playlist = match playlist { + Some(p) => p, + None => return Err(DomainError::not_found("Playlist", playlist_id)), + }; + // Public-playlist bypass: anonymous-ish read. `check` returns + // bool (no throw); combine with the public flag before + // deciding. + let allowed = playlist.is_public + || self + .has_playlist_perm(playlist_id, user_id, Permission::Read) + .await?; + if !allowed { + return Err(DomainError::not_found("Playlist", playlist_id)); } + Ok(playlist) } async fn list_playlists( @@ -123,17 +171,38 @@ impl MusicUseCase for MusicService { let limit = query.limit.unwrap_or(100); let offset = query.offset.unwrap_or(0); - let mut playlists = Vec::new(); + // Post-Round-3 semantics: playlists the caller has any grant + // on come from `list_incoming_grants` — one union of owned + + // shared. The pre-Round-3 code fetched them via two separate + // queries (`list_playlists_by_owner` + `list_shared_with_user`) + // that each read a different table. + let grants = self + .authz + .list_incoming_grants(Subject::User(user_id)) + .await?; - let owned = self.storage.list_playlists_by_owner(user_id).await?; - playlists.extend(owned); + // Deduplicate — a user can hold multiple grants on the same + // playlist (direct + group-inherited). We only need one DTO + // per resource. + let mut playlist_ids: HashSet = grants + .into_iter() + .filter_map(|g| match g.resource { + Resource::Playlist(id) => Some(id), + _ => None, + }) + .collect(); - if include_shared { - let shared = self.storage.list_shared_with_user(user_id).await?; - for s in shared { - if !playlists.iter().any(|p: &PlaylistDto| p.id == s.id) { - playlists.push(s); - } + // `include_shared=false` narrows the listing to owned playlists + // only. Owner is a grant like any other in `role_grants`, so we + // filter the aggregated set against the owner_id stamped on + // each row after hydration — cheaper than a second SQL round-trip. + let mut playlists: Vec = Vec::with_capacity(playlist_ids.len()); + let user_str = user_id.to_string(); + for id in playlist_ids.drain() { + if let Ok(Some(p)) = self.storage.get_playlist(&id.to_string()).await + && (include_shared || p.owner_id == user_str) + { + playlists.push(p); } } @@ -155,26 +224,9 @@ impl MusicUseCase for MusicService { dto: AddTracksDto, user_id: Uuid, ) -> Result, DomainError> { - let playlist_uuid = Uuid::parse_str(playlist_id).map_err(|_| { - DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid playlist ID") - })?; - - let has_access = self.storage.user_has_access(playlist_id, user_id).await?; - if !has_access { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You don't have permission to modify this playlist", - )); - } - let can_write = self.storage.user_can_write(playlist_id, user_id).await?; - if !can_write { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You need write access to add tracks", - )); - } + let playlist_uuid = self + .require_playlist_perm(playlist_id, user_id, Permission::Update) + .await?; let file_ids: Result, _> = dto.file_ids.iter().map(|id| Uuid::parse_str(id)).collect(); @@ -191,30 +243,12 @@ impl MusicUseCase for MusicService { file_id: &str, user_id: Uuid, ) -> Result<(), DomainError> { - let playlist_uuid = Uuid::parse_str(playlist_id).map_err(|_| { - DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid playlist ID") - })?; + let playlist_uuid = self + .require_playlist_perm(playlist_id, user_id, Permission::Update) + .await?; let file_uuid = Uuid::parse_str(file_id).map_err(|_| { DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid file ID") })?; - - let has_access = self.storage.user_has_access(playlist_id, user_id).await?; - if !has_access { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You don't have permission to modify this playlist", - )); - } - let can_write = self.storage.user_can_write(playlist_id, user_id).await?; - if !can_write { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You need write access to remove tracks", - )); - } - self.storage.remove_track(&playlist_uuid, &file_uuid).await } @@ -224,26 +258,9 @@ impl MusicUseCase for MusicService { dto: ReorderTracksDto, user_id: Uuid, ) -> Result<(), DomainError> { - let playlist_uuid = Uuid::parse_str(playlist_id).map_err(|_| { - DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid playlist ID") - })?; - - let has_access = self.storage.user_has_access(playlist_id, user_id).await?; - if !has_access { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You don't have permission to modify this playlist", - )); - } - let can_write = self.storage.user_can_write(playlist_id, user_id).await?; - if !can_write { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You need write access to reorder tracks", - )); - } + let playlist_uuid = self + .require_playlist_perm(playlist_id, user_id, Permission::Update) + .await?; let item_ids: Result, _> = dto.item_ids.iter().map(|id| Uuid::parse_str(id)).collect(); @@ -262,16 +279,21 @@ impl MusicUseCase for MusicService { let playlist_uuid = Uuid::parse_str(playlist_id).map_err(|_| { DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid playlist ID") })?; - - let has_access = self.storage.user_has_access(playlist_id, user_id).await?; - if !has_access { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "You don't have permission to view this playlist", - )); + // Public-playlist bypass mirrors `get_playlist`: readers of a + // public playlist can see its tracks. Fetch the playlist row + // to inspect `is_public` before deciding. + let playlist = self + .storage + .get_playlist(playlist_id) + .await? + .ok_or_else(|| DomainError::not_found("Playlist", playlist_id))?; + let allowed = playlist.is_public + || self + .has_playlist_perm(playlist_id, user_id, Permission::Read) + .await?; + if !allowed { + return Err(DomainError::not_found("Playlist", playlist_id)); } - self.storage.list_playlist_tracks(&playlist_uuid).await } @@ -281,36 +303,33 @@ impl MusicUseCase for MusicService { dto: SharePlaylistDto, caller_id: Uuid, ) -> Result<(), DomainError> { - let playlist = self.storage.get_playlist(playlist_id).await?; - let playlist = match playlist { - Some(p) => p, - None => { - return Err(DomainError::new( - ErrorKind::NotFound, - "Playlist", - "Playlist not found", - )); - } - }; - if playlist.owner_id != caller_id.to_string() { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "Only the owner can share this playlist", - )); - } - - let playlist_uuid = Uuid::parse_str(playlist_id).map_err(|_| { - DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid playlist ID") - })?; + let playlist_uuid = self + .require_playlist_perm(playlist_id, caller_id, Permission::Share) + .await?; let target_user_id = Uuid::parse_str(&dto.user_id).map_err(|_| { DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid user ID") })?; - let can_write = dto.can_write.unwrap_or(false); - - self.storage - .share_playlist(&playlist_uuid, target_user_id, can_write) - .await + // Legacy `can_write` boolean maps into the role bundle system: + // - false → Viewer (Read only) + // - true → Editor (Read + Update) + // The endpoint stays boolean-shaped for API back-compat; new + // integrations should switch to the unified `/api/grants` API + // which exposes the full role set. + let role = if dto.can_write.unwrap_or(false) { + Role::Editor + } else { + Role::Viewer + }; + self.authz + .set_role( + caller_id, + Subject::User(target_user_id), + role, + Resource::Playlist(playlist_uuid), + None, + ) + .await?; + Ok(()) } async fn remove_share( @@ -319,33 +338,18 @@ impl MusicUseCase for MusicService { target_user_id: &str, caller_id: Uuid, ) -> Result<(), DomainError> { - let playlist = self.storage.get_playlist(playlist_id).await?; - let playlist = match playlist { - Some(p) => p, - None => { - return Err(DomainError::new( - ErrorKind::NotFound, - "Playlist", - "Playlist not found", - )); - } - }; - if playlist.owner_id != caller_id.to_string() { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "Only the owner can manage sharing", - )); - } - - let playlist_uuid = Uuid::parse_str(playlist_id).map_err(|_| { - DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid playlist ID") - })?; + let playlist_uuid = self + .require_playlist_perm(playlist_id, caller_id, Permission::Share) + .await?; let target_uuid = Uuid::parse_str(target_user_id).map_err(|_| { DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid user ID") })?; - - self.storage.remove_share(&playlist_uuid, target_uuid).await + self.authz + .clear_role( + Subject::User(target_uuid), + Resource::Playlist(playlist_uuid), + ) + .await } async fn get_playlist_shares( @@ -353,35 +357,26 @@ impl MusicUseCase for MusicService { playlist_id: &str, user_id: Uuid, ) -> Result, DomainError> { - let playlist = self.storage.get_playlist(playlist_id).await?; - let playlist = match playlist { - Some(p) => p, - None => { - return Err(DomainError::new( - ErrorKind::NotFound, - "Playlist", - "Playlist not found", - )); - } - }; - if playlist.owner_id != user_id.to_string() { - return Err(DomainError::new( - ErrorKind::AccessDenied, - "Playlist", - "Only the owner can view sharing info", - )); - } - - let playlist_uuid = Uuid::parse_str(playlist_id).map_err(|_| { - DomainError::new(ErrorKind::InvalidInput, "Playlist", "Invalid playlist ID") - })?; - - let shares = self.storage.get_shares(&playlist_uuid).await?; - Ok(shares + let playlist_uuid = self + .require_playlist_perm(playlist_id, user_id, Permission::Share) + .await?; + // `list_grants_on_resource` returns every role_grant row for + // the playlist. Drop the Owner self-grant seeded at creation + // (the caller already knows they own it) and collapse the + // role bundle back to a boolean `can_write` for the legacy + // DTO shape. + let grants = self + .authz + .list_grants_on_resource(Resource::Playlist(playlist_uuid)) + .await?; + Ok(grants .into_iter() - .map(|(uid, can_write)| PlaylistShareInfoDto { - user_id: uid.to_string(), - can_write, + .filter_map(|g| match g.subject { + Subject::User(uid) if g.role != Role::Owner => Some(PlaylistShareInfoDto { + user_id: uid.to_string(), + can_write: g.role.expand().contains(&Permission::Update), + }), + _ => None, }) .collect()) } @@ -398,9 +393,8 @@ impl MusicUseCase for MusicService { // metadata for any known file id (cross-tenant IDOR — the // `_user_id` parameter was deliberately unused). `require` // returns 404 on denial to match the anti-enum shape used - // everywhere else. Post-Drive AuthZ audit fix (Round 1 - // BLOCKER — `docs/plan/authz_audit/rest_storage.md`). - self.authorization + // everywhere else. + self.authz .require( Subject::User(caller_id), Permission::Read, diff --git a/src/application/services/recipient_notification_service.rs b/src/application/services/recipient_notification_service.rs index ab4880ee..a651bf15 100644 --- a/src/application/services/recipient_notification_service.rs +++ b/src/application/services/recipient_notification_service.rs @@ -472,13 +472,14 @@ impl RecipientNotificationService { let kind_key = match resource { Resource::Folder(_) => "server.magic_link.email.kind_folder", Resource::File(_) => "server.magic_link.email.kind_file", - // Drive / Calendar / AddressBook shares don't produce - // email notifications through this path. Fall back to the - // folder label so any code that does reach here still - // produces a readable (if generic) mail body. - Resource::Drive(_) | Resource::Calendar(_) | Resource::AddressBook(_) => { - "server.magic_link.email.kind_folder" - } + // Drive / Calendar / AddressBook / Playlist shares don't + // produce email notifications through this path. Fall + // back to the folder label so any code that does reach + // here still produces a readable (if generic) mail body. + Resource::Drive(_) + | Resource::Calendar(_) + | Resource::AddressBook(_) + | Resource::Playlist(_) => "server.magic_link.email.kind_folder", }; let kind_label = self.i18n_or(kind_key, &locale, &[]).await; // Short form for the subject, long form (with email) for the diff --git a/src/domain/services/authorization.rs b/src/domain/services/authorization.rs index 66ca10da..fb1015b8 100644 --- a/src/domain/services/authorization.rs +++ b/src/domain/services/authorization.rs @@ -90,9 +90,12 @@ pub enum Resource { /// replaces `carddav.address_book_shares` and the /// `check_address_book_access` bespoke helper. AddressBook(Uuid), - // Reserved for future use — same shape but tracked separately - // (music-service rewrite is its own PR): - // Playlist(Uuid), + /// A music playlist. Same shape as `Calendar`/`AddressBook` — + /// `storage.role_grants` with `resource_type='playlist'` replaces + /// the pre-Round-3 dedicated `music.playlist_shares` table and the + /// bespoke `user_has_access` / `user_can_write` helpers on + /// `MusicStorageAdapter`. + Playlist(Uuid), } impl Resource { @@ -103,7 +106,7 @@ impl Resource { Resource::Drive(_) => "drive", Resource::Calendar(_) => "calendar", Resource::AddressBook(_) => "address_book", - //Resource::Playlist(_) => "playlist", + Resource::Playlist(_) => "playlist", } } @@ -113,8 +116,8 @@ impl Resource { | Resource::File(id) | Resource::Drive(id) | Resource::Calendar(id) - | Resource::AddressBook(id) => *id, - //| Resource::Playlist(id) + | Resource::AddressBook(id) + | Resource::Playlist(id) => *id, } } @@ -125,7 +128,7 @@ impl Resource { "drive" => Some(Resource::Drive(id)), "calendar" => Some(Resource::Calendar(id)), "address_book" => Some(Resource::AddressBook(id)), - //"playlist" => Some(Resource::Playlist(id)), + "playlist" => Some(Resource::Playlist(id)), _ => None, } } @@ -552,12 +555,11 @@ mod tests { Resource::File(id), Resource::Calendar(id), Resource::AddressBook(id), + Resource::Playlist(id), ] { let back = Resource::from_parts(r.type_str(), r.id()).unwrap(); assert_eq!(r, back); } - // `playlist` is still pending the Music AuthZ migration. - assert!(Resource::from_parts("playlist", id).is_none()); } #[test] diff --git a/src/infrastructure/services/pg_acl_engine.rs b/src/infrastructure/services/pg_acl_engine.rs index 708f8ba2..4d69e7fe 100644 --- a/src/infrastructure/services/pg_acl_engine.rs +++ b/src/infrastructure/services/pg_acl_engine.rs @@ -480,17 +480,22 @@ impl PgAclEngine { /// drive — this returns `NotFound` for `Resource::Drive` and the caller /// must not invoke it on Drive resources. /// - /// `Resource::Calendar` and `Resource::AddressBook` are top-level per - /// user with no drive ancestor; they also return `NotFound` and the - /// engine short-circuits to a direct `role_grants` lookup (no drive + /// `Resource::Calendar`, `Resource::AddressBook` and + /// `Resource::Playlist` are top-level per user with no drive + /// ancestor; they also return `NotFound` and the engine + /// short-circuits to a direct `role_grants` lookup (no drive /// precheck applies). async fn drive_of(&self, resource: Resource) -> Result { match resource { Resource::Folder(id) => self.folder_repo.get_folder_drive_id(&id.to_string()).await, Resource::File(id) => self.file_repo.get_file_drive_id(&id.to_string()).await, - Resource::Drive(_) | Resource::Calendar(_) | Resource::AddressBook(_) => Err( - DomainError::not_found(resource.type_str(), resource.id().to_string()), - ), + Resource::Drive(_) + | Resource::Calendar(_) + | Resource::AddressBook(_) + | Resource::Playlist(_) => Err(DomainError::not_found( + resource.type_str(), + resource.id().to_string(), + )), } } @@ -902,6 +907,19 @@ impl PgAclEngine { ) .await } + Resource::Playlist(id) => { + let (subject_types, subject_ids) = + self.subject_match_set(subject, counters).await?; + self.direct_grant_exists( + &subject_types, + &subject_ids, + permission, + "playlist", + id, + counters, + ) + .await + } } } } diff --git a/src/interfaces/api/handlers/grant_handler.rs b/src/interfaces/api/handlers/grant_handler.rs index 07d493ca..452164c9 100644 --- a/src/interfaces/api/handlers/grant_handler.rs +++ b/src/interfaces/api/handlers/grant_handler.rs @@ -113,12 +113,12 @@ pub async fn create_grant( .get_by_id(id) .await .map(|d| d.drive.typed_policies()), - // Calendars and address books live outside the drive - // hierarchy (top-level per user), so no drive-level policy - // gates apply. If per-calendar / per-address-book policies - // ever ship, they'll live on the resource itself, not on a + // Calendars, address books and playlists live outside the + // drive hierarchy (top-level per user), so no drive-level + // policy gates apply. If per-resource policies ever ship for + // these kinds, they'll live on the resource itself, not on a // drive; the default-empty bag is the right no-op here. - Resource::Calendar(_) | Resource::AddressBook(_) => { + Resource::Calendar(_) | Resource::AddressBook(_) | Resource::Playlist(_) => { Ok(crate::domain::entities::drive::DrivePolicies::default()) } }; diff --git a/tests/api/playlists.hurl b/tests/api/playlists.hurl new file mode 100644 index 00000000..fd8a8283 --- /dev/null +++ b/tests/api/playlists.hurl @@ -0,0 +1,408 @@ +# ============================================================= +# OxiCloud – Music (playlist) + Round-3 AuthZ end-to-end scenario +# ============================================================= +# Verifies the full playlist REST surface post-Round-3: +# +# * `POST /api/playlists` seeds an Owner grant on +# `Resource::Playlist(uuid)` so the caller can see it via the +# unified engine (list, get) on the very next request. +# * `GET /api/playlists` returns the union of owned + shared +# playlists via `authz.list_incoming_grants`; the pre-Round-3 +# owner-only + separate shared query pair is gone. +# * Cross-user reads (`GET /api/playlists/{id}`) return the 404 +# anti-enum shape (was 403 in the bespoke +# `user_has_access` era). +# * Sharing works through BOTH surfaces post-migration: +# - Generic `POST /api/grants` with `resource.type = "playlist"` +# (first-class ReBAC variant added in this PR) +# - Legacy `POST /api/playlists/{id}/share` (bool `can_write`) +# still routes through the same `role_grants` table via +# `authz.set_role`, so both flows converge on the unified +# engine. +# * `GET /api/playlists/{id}/shares` reads `list_grants_on_resource` +# and hides the Owner self-grant. +# * Revoke through either surface drops the playlist from the +# recipient's listing. +# * Viewer role blocks writes: `Update`/`Delete`/`Share` all 404 for +# a Viewer, matching the anti-enum shape. +# +# The `playlist_id` is captured from the POST response body. Fresh CI +# database via `tests/api/run.sh`, so admin has no prior playlists — +# the JSONPath capture from `GET /api/playlists` is unambiguous. +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 – Alice (admin) logs in. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ + "username": "{{username}}", + "password": "{{password}}" +} + +HTTP 200 +[Captures] +alice_token: jsonpath "$.access_token" +alice_user_id: jsonpath "$.user.id" + + +# ───────────────────────────────────────────────────────────── +# Step 2 – Alice creates a playlist. The response body carries the +# server-assigned UUID and `owner_id == alice_user_id`. The service +# also seeds an Owner role_grant on `Resource::Playlist(uuid)` — +# proven by Step 4 which lists playlists via +# `authz.list_incoming_grants` and expects this one to surface. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/playlists +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ + "name": "round3-playlist", + "description": "Music AuthZ migration coverage" +} + +HTTP 201 +[Captures] +playlist_id: jsonpath "$.id" +[Asserts] +jsonpath "$.name" == "round3-playlist" +jsonpath "$.owner_id" == "{{alice_user_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 3 – Alice GETs the playlist she just created. This is the +# fast-path validation of the Owner grant seeded at create time: +# without it, `authz.require(Read)` would return NotFound and this +# would 404. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$.id" == "{{playlist_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 4 – Alice lists playlists — hers appears exactly once. +# The service reads `list_incoming_grants(Alice)` and filters to +# `Resource::Playlist`, so this exercises the same code path as +# CalDAV's `list_my_calendars`. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$[*].id" contains "{{playlist_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 5 – Provision Bob. Idempotent: `HTTP *` accepts 201 first +# run, 409 subsequent runs. Login is the real precondition. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/admin/users +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ + "username": "music_bob", + "password": "MusicBobPassword1!", + "email": "music_bob@example.com", + "role": "user" +} + +HTTP * + + +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ + "username": "music_bob", + "password": "MusicBobPassword1!" +} + +HTTP 200 +[Captures] +bob_token: jsonpath "$.access_token" +bob_user_id: jsonpath "$.user.id" + + +# ───────────────────────────────────────────────────────────── +# Step 6 – Cross-user GET on Alice's playlist → 404. Before Round 3 +# this was the bespoke `user_has_access` denial which returned 403; +# post-migration `authz.require(Read)` denies with `NotFound` for +# anti-enumeration parity with files/folders/drives. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{bob_token}} + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 7 – Bob's playlist listing does NOT include Alice's. The +# `list_incoming_grants(Bob)` call sees no grant on that playlist, +# so nothing surfaces. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists +Authorization: Bearer {{bob_token}} + +HTTP 200 +[Asserts] +jsonpath "$..id" not contains "{{playlist_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 8 – Alice shares the playlist with Bob as Viewer via the +# generic ReBAC grant endpoint. `resource.type = "playlist"` is a +# first-class variant added by this PR; before Round 3, this +# request would 400 (Unsupported resource type). +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/grants +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{bob_user_id}}" }, + "resource": { "type": "playlist", "id": "{{playlist_id}}" }, + "role": "viewer" +} + +HTTP 201 +[Captures] +share_grant_id: jsonpath "$.grants[0].id" +[Asserts] +jsonpath "$.grants[0].role" == "viewer" +jsonpath "$.grants[0].resource.type" == "playlist" +jsonpath "$.grants[0].resource.id" == "{{playlist_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 9 – Bob GET now succeeds. `authz.require(Read)` sees the +# Viewer role_grant row and grants access. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{bob_token}} + +HTTP 200 +[Asserts] +jsonpath "$.id" == "{{playlist_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 10 – Bob's listing now surfaces Alice's playlist — proving +# the owned + shared union in `list_playlists`. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists?include_shared=true +Authorization: Bearer {{bob_token}} + +HTTP 200 +[Asserts] +jsonpath "$[*].id" contains "{{playlist_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 11 – Bob cannot rename the playlist. Viewer's bundle is +# Read-only (no Update), so `require_playlist_perm(Update)` denies +# with the 404 anti-enum shape. +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{bob_token}} +Content-Type: application/json +{ "name": "hijacked" } + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 12 – Bob cannot delete the playlist. Viewer's bundle +# excludes Delete → 404. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{bob_token}} + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 13 – Bob cannot re-share the playlist. Viewer's bundle +# excludes Share → 404 on the legacy /share endpoint (which now +# routes through `authz.require(Share)`). +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/playlists/{{playlist_id}}/share +Authorization: Bearer {{bob_token}} +Content-Type: application/json +{ "user_id": "{{alice_user_id}}", "can_write": true } + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 14 – Alice lists shares via the legacy endpoint. The +# service reads `list_grants_on_resource` and drops the Owner +# self-grant, so exactly one row surfaces: Bob as Viewer +# (can_write=false). +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists/{{playlist_id}}/shares +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$[*].user_id" contains "{{bob_user_id}}" +jsonpath "$[?(@.user_id == '{{bob_user_id}}')].can_write" == false +jsonpath "$[*].user_id" not contains "{{alice_user_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 14b – Same query, unified endpoint. `GET /api/grants? +# resource_type=playlist&resource_id=…` requires `Share` on the +# resource (same gate as the legacy /shares endpoint) and returns +# the raw `role_grants` rows — including the Owner self-grant that +# the legacy DTO hides. Confirms `ResourceTypeDto::Playlist` is +# admitted at the wire boundary and that both surfaces read the +# same underlying data. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/grants?resource_type=playlist&resource_id={{playlist_id}} +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$[*].subject.id" contains "{{bob_user_id}}" +jsonpath "$[*].subject.id" contains "{{alice_user_id}}" +jsonpath "$[?(@.subject.id == '{{bob_user_id}}')].role" == "viewer" +jsonpath "$[?(@.subject.id == '{{alice_user_id}}')].role" == "owner" +jsonpath "$[?(@.subject.id == '{{bob_user_id}}')].resource.type" == "playlist" + + +# ───────────────────────────────────────────────────────────── +# Step 14c – Bob (Viewer only) is denied on the unified list +# endpoint: `Share` is required, Viewer's bundle excludes it → +# 404 anti-enum shape. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/grants?resource_type=playlist&resource_id={{playlist_id}} +Authorization: Bearer {{bob_token}} + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 15 – Alice revokes the ReBAC grant. `DELETE /api/grants/{id}` +# deletes the single `role_grants` row keyed by grant_id. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/grants/{{share_grant_id}} +Authorization: Bearer {{alice_token}} + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 16 – Bob's GET goes back to 404, and his listing drops the +# playlist. The `role_grants` row is gone → `list_incoming_grants` +# doesn't surface it, `require(Read)` denies. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{bob_token}} + +HTTP 404 + + +GET {{base_url}}/api/playlists?include_shared=true +Authorization: Bearer {{bob_token}} + +HTTP 200 +[Asserts] +jsonpath "$..id" not contains "{{playlist_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 17 – Alice re-shares Bob as Editor via the LEGACY endpoint. +# `can_write=true` maps to `Role::Editor` inside +# `music_service::share_playlist` — proving the legacy surface +# and `/api/grants` now converge on the same `role_grants` table. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/playlists/{{playlist_id}}/share +Authorization: Bearer {{alice_token}} +Content-Type: application/json +{ "user_id": "{{bob_user_id}}", "can_write": true } + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 18 – Editor CAN update (Editor's bundle includes Update). +# Confirms the can_write=true → Editor mapping actually takes +# effect at the engine level. +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{bob_token}} +Content-Type: application/json +{ "description": "renamed by editor bob" } + +HTTP 200 +[Asserts] +jsonpath "$.description" == "renamed by editor bob" + + +# ───────────────────────────────────────────────────────────── +# Step 19 – Editor still cannot Share (Share stays Owner-only). +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/playlists/{{playlist_id}}/share +Authorization: Bearer {{bob_token}} +Content-Type: application/json +{ "user_id": "{{alice_user_id}}", "can_write": false } + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step 20 – `/shares` now reports Bob as Editor (can_write=true). +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists/{{playlist_id}}/shares +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$[*].user_id" contains "{{bob_user_id}}" +jsonpath "$[?(@.user_id == '{{bob_user_id}}')].can_write" == true + + +# ───────────────────────────────────────────────────────────── +# Step 21 – Alice removes the legacy-endpoint share. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/playlists/{{playlist_id}}/share/{{bob_user_id}} +Authorization: Bearer {{alice_token}} + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 22 – Post-remove listing is empty (Owner self-grant is +# still hidden). +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists/{{playlist_id}}/shares +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$..user_id" not contains "{{bob_user_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 23 – Cleanup: Alice deletes the playlist. The service +# runs `authz.require(Delete)` (owner passes via the seeded Owner +# grant), then `revoke_all_for_resource` wipes any stray grants. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{alice_token}} + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 24 – GET returns 404 after delete (nothing to enum). +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/playlists/{{playlist_id}} +Authorization: Bearer {{alice_token}} + +HTTP 404 diff --git a/tests/api/run.sh b/tests/api/run.sh index 86c82567..5f734bb1 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -162,6 +162,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/dedup_blob_cleanup.hurl" \ "$API_DIR/contacts.hurl" \ "$API_DIR/calendar.hurl" \ + "$API_DIR/playlists.hurl" \ "$API_DIR/public_shares.hurl" \ "$API_DIR/permissions.hurl" \ "$API_DIR/grants.hurl" \