cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

feat(music): move playlist to authz engine

Browse Source
This commit is contained in:
Edouard Vanbelle
2026-07-08 00:23:25 +02:00
parent 57bea52125
commit c1e46910b0
12 changed files with 856 additions and 269 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/application/services/magic_link_invite_service.rs
+22 -16
View File
@@ -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
src/application/services/music_service.rs
+220 -226
View File
@@ -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<MusicStorageAdapter>,
/// 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<PgAclEngine>,
/// 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<PgAclEngine>,
}
impl MusicService {
pub fn new(storage: Arc<MusicStorageAdapter>, authorization: Arc<PgAclEngine>) -> Self {
Self {
storage,
authorization,
}
pub fn new(storage: Arc<MusicStorageAdapter>, authz: Arc<PgAclEngine>) -> 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<Uuid, DomainError> {
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<bool, DomainError> {
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<PlaylistDto, DomainError> {
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<PlaylistDto, DomainError> {
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<PlaylistDto, DomainError> {
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<Uuid> = 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<PlaylistDto> = 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<Vec<PlaylistItemDto>, 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<Vec<Uuid>, _> =
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<Vec<Uuid>, _> =
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<Vec<PlaylistShareInfoDto>, 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,
src/application/services/recipient_notification_service.rs
+8 -7
View File
@@ -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