security(music): ensure read permission via authz

src/application/services/music_service.rs

@@ -5,17 +5,31 @@ use crate::application::dtos::playlist_dto::{
     AddTracksDto, AudioMetadataDto, CreatePlaylistDto, PlaylistDto, PlaylistItemDto,
     PlaylistQueryDto, PlaylistShareInfoDto, ReorderTracksDto, SharePlaylistDto, UpdatePlaylistDto,
 };
+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::infrastructure::adapters::music_storage_adapter::MusicStorageAdapter;
+use crate::infrastructure::services::pg_acl_engine::PgAclEngine;
 
 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>,
 }
 
 impl MusicService {
-    pub fn new(storage: Arc<MusicStorageAdapter>) -> Self {
-        Self { storage }
+    pub fn new(storage: Arc<MusicStorageAdapter>, authorization: Arc<PgAclEngine>) -> Self {
+        Self {
+            storage,
+            authorization,
+        }
     }
 }
 
@@ -375,10 +389,24 @@ impl MusicUseCase for MusicService {
     async fn get_audio_metadata(
         &self,
         file_id: &str,
-        _user_id: Uuid,
+        caller_id: Uuid,
     ) -> Result<Option<AudioMetadataDto>, DomainError> {
         let file_uuid = Uuid::parse_str(file_id)
             .map_err(|_| DomainError::new(ErrorKind::InvalidInput, "Music", "Invalid file ID"))?;
+        // AuthZ pre-read: caller must have `Read` on the underlying
+        // audio file. Before this check the endpoint returned
+        // 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
+            .require(
+                Subject::User(caller_id),
+                Permission::Read,
+                Resource::File(file_uuid),
+            )
+            .await?;
         self.storage.get_audio_metadata(&file_uuid).await
     }
 }

src/common/di.rs

@@ -1865,7 +1865,7 @@ impl AppServiceFactory {
                     audio_metadata_repo,
                 ),
             );
-            let music_svc = Arc::new(MusicService::new(music_storage));
+            let music_svc = Arc::new(MusicService::new(music_storage, authorization.clone()));
             app_state.music_service = Some(music_svc);
             tracing::info!("Music service initialized");
         }

src/interfaces/api/handlers/favorites_handler.rs

@@ -6,7 +6,7 @@ use axum::{
 };
 use serde::Deserialize;
 use std::sync::Arc;
-use tracing::{error, info};
+use tracing::info;
 use utoipa::ToSchema;
 
 use crate::application::dtos::display_helpers::{
@@ -66,7 +66,8 @@ pub async fn add_favorite(
             Json(serde_json::json!({
                 "error": "Item type must be 'file' or 'folder'"
             })),
-        );
+        )
+            .into_response();
     }
 
     match favorites_service
@@ -81,16 +82,14 @@ pub async fn add_favorite(
                     "message": "Item added to favorites"
                 })),
             )
+                .into_response()
         }
-        Err(err) => {
-            error!("Error adding to favorites: {}", err);
-            (
-                StatusCode::INTERNAL_SERVER_ERROR,
-                Json(serde_json::json!({
-                    "error": "Failed to add to favorites"
-                })),
-            )
-        }
+        // Route through AppError so the `DomainError::kind` maps to the
+        // right status code (NotFound → 404 anti-enum for the pre-write
+        // authz gate, InvalidInput → 400 for a malformed UUID, etc.).
+        // A hardcoded 500 here would mask the 404 the Round 1 AuthZ
+        // fix relies on.
+        Err(err) => AppError::from(err).into_response(),
     }
 }
 
@@ -129,6 +128,7 @@ pub async fn remove_favorite(
                         "message": "Item removed from favorites"
                     })),
                 )
+                    .into_response()
             } else {
                 info!("Item {} '{}' was not in favorites", item_type, item_id);
                 (
@@ -137,17 +137,12 @@ pub async fn remove_favorite(
                         "message": "Item was not in favorites"
                     })),
                 )
+                    .into_response()
             }
         }
-        Err(err) => {
-            error!("Error removing from favorites: {}", err);
-            (
-                StatusCode::INTERNAL_SERVER_ERROR,
-                Json(serde_json::json!({
-                    "error": "Failed to remove from favorites"
-                })),
-            )
-        }
+        // Same rationale as `add_favorite` — preserve DomainError→HTTP
+        // status mapping instead of collapsing every error to 500.
+        Err(err) => AppError::from(err).into_response(),
     }
 }
 
@@ -347,15 +342,10 @@ pub async fn batch_add_favorites(
             );
             (StatusCode::OK, Json(serde_json::json!(result))).into_response()
         }
-        Err(err) => {
-            error!("Error in batch add favorites: {}", err);
-            (
-                StatusCode::INTERNAL_SERVER_ERROR,
-                Json(serde_json::json!({
-                    "error": "Failed to batch add favorites"
-                })),
-            )
-                .into_response()
-        }
+        // Preserve DomainError→HTTP status mapping — the Round 1
+        // AuthZ fix relies on a per-item NotFound propagating out
+        // of the batch. A hardcoded 500 would mask the 404 that
+        // signals a cross-tenant probe.
+        Err(err) => AppError::from(err).into_response(),
     }
 }

src/interfaces/api/handlers/recent_handler.rs

@@ -5,7 +5,7 @@ use axum::{
     response::IntoResponse,
 };
 use std::sync::Arc;
-use tracing::{error, info};
+use tracing::info;
 
 use crate::application::dtos::display_helpers::{
     category_for, format_file_size, icon_class_for, icon_special_class_for,
@@ -70,16 +70,10 @@ pub async fn record_item_access(
             )
                 .into_response()
         }
-        Err(err) => {
-            error!("Error recording access in recents: {}", err);
-            (
-                StatusCode::INTERNAL_SERVER_ERROR,
-                Json(serde_json::json!({
-                    "error": "Failed to record access"
-                })),
-            )
-                .into_response()
-        }
+        // Preserve DomainError→HTTP status mapping — the Round 1
+        // AuthZ fix relies on the NotFound from `authz.require`
+        // propagating as 404 (anti-enum), not being masked as 500.
+        Err(err) => AppError::from(err).into_response(),
     }
 }
 
@@ -130,16 +124,9 @@ pub async fn remove_from_recent(
                     .into_response()
             }
         }
-        Err(err) => {
-            error!("Error removing from recents: {}", err);
-            (
-                StatusCode::INTERNAL_SERVER_ERROR,
-                Json(serde_json::json!({
-                    "error": "Failed to remove from recents"
-                })),
-            )
-                .into_response()
-        }
+        // Same rationale as `record_item_access` — preserve the
+        // DomainError→HTTP mapping instead of collapsing to 500.
+        Err(err) => AppError::from(err).into_response(),
     }
 }
 
@@ -170,16 +157,9 @@ pub async fn clear_recent_items(
             )
                 .into_response()
         }
-        Err(err) => {
-            error!("Error clearing recent items: {}", err);
-            (
-                StatusCode::INTERNAL_SERVER_ERROR,
-                Json(serde_json::json!({
-                    "error": "Failed to clear recent items"
-                })),
-            )
-                .into_response()
-        }
+        // Same rationale as `record_item_access` — preserve the
+        // DomainError→HTTP mapping instead of collapsing to 500.
+        Err(err) => AppError::from(err).into_response(),
     }
 }
 

tests/api/favorites.hurl

@@ -159,3 +159,77 @@ Authorization: Bearer {{token}}
 HTTP 200
 [Asserts]
 jsonpath "$.items" count == 0
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 12 — Cross-tenant regression (post-Drive AuthZ audit,
+#           Round 1 HIGH). Before this fix, `POST /api/favorites/…`
+#           accepted any UUID and enrolled it; the listing endpoint
+#           then JOINed back to storage.files/folders and returned
+#           name/mime/size/drive_id for anything the caller had
+#           managed to add — an information oracle over the whole
+#           tenant. Now the write path calls `authz.require(Read, …)`
+#           per item; a caller with no grant gets 404 (anti-enum)
+#           + `authz.denied` audit line. See
+#           `docs/plan/authz_audit/rest_storage.md`.
+# ─────────────────────────────────────────────────────────────
+
+# Create a second, unprivileged user. Idempotent: `HTTP *` accepts
+# either 201 (first run) or 409 (subsequent runs). The login below
+# is the actual precondition — if it succeeds we know the user
+# exists with the expected password.
+POST {{base_url}}/api/admin/users
+Authorization: Bearer {{token}}
+Content-Type: application/json
+{ "username": "fav_mallory", "password": "FavMalloryPassword1!", "email": "fav_mallory@example.com", "role": "user" }
+
+HTTP *
+
+
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "fav_mallory", "password": "FavMalloryPassword1!" }
+
+HTTP 200
+[Captures]
+mallory_token: jsonpath "$.access_token"
+
+
+# Step 12a — Single-add on admin's file: 404 (anti-enum shape).
+POST {{base_url}}/api/favorites/file/{{file_id}}
+Authorization: Bearer {{mallory_token}}
+
+HTTP 404
+
+
+# Step 12b — Single-add on admin's folder: 404.
+POST {{base_url}}/api/favorites/folder/{{test1_id}}
+Authorization: Bearer {{mallory_token}}
+
+HTTP 404
+
+
+# Step 12c — Batch: must fail wholesale on the first denial. A partial
+#           success would still leak "which items are valid" — the same
+#           oracle we're closing.
+POST {{base_url}}/api/favorites/batch
+Authorization: Bearer {{mallory_token}}
+Content-Type: application/json
+{
+  "items": [
+    { "item_id": "{{file_id}}",  "item_type": "file"   },
+    { "item_id": "{{test1_id}}", "item_type": "folder" }
+  ]
+}
+
+HTTP 404
+
+
+# Step 12d — Mallory's favorites list is EMPTY — no partial success
+#           slipped through.
+GET {{base_url}}/api/favorites/resources
+Authorization: Bearer {{mallory_token}}
+
+HTTP 200
+[Asserts]
+jsonpath "$.items" count == 0

tests/api/public_shares.hurl

@@ -274,6 +274,85 @@ status >= 400
 status <  500
 
 
+# ─────────────────────────────────────────────────────────────
+# 14b — Viewer-laundering regression (post-Drive AuthZ audit,
+#       Round 1 HIGH). Before the fix, `POST /api/shares` checked
+#       only "does the item exist" — any authenticated user who
+#       could name the UUID could mint a public Viewer link,
+#       laundering read access into a permanent anonymous URL
+#       that survived their own grant revocation. Now the
+#       service calls `authz.require(Share, resource)` before
+#       minting the token; a caller without `Share`
+#       (Viewer/Commenter/Contributor/no-grant-at-all) gets 404
+#       (anti-enum) + `authz.denied` audit line. See
+#       `docs/plan/authz_audit/admin_membership.md`.
+#
+#       We test the strongest form: an unrelated user with no
+#       grant at all. The intermediate case (Viewer with Read
+#       but not Share) is covered by the same code path — Share
+#       is bundled only with owner/editor role_grants.
+# ─────────────────────────────────────────────────────────────
+
+# Create/lookup the attacker. Idempotent: `HTTP *` accepts either
+# 201 (first run) or 409 (subsequent runs). Login below is the real
+# precondition.
+POST {{base_url}}/api/admin/users
+Authorization: Bearer {{admin_token}}
+Content-Type: application/json
+{ "username": "sh_mallory", "password": "ShMalloryPassword1!", "email": "sh_mallory@example.com", "role": "user" }
+
+HTTP *
+
+
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "sh_mallory", "password": "ShMalloryPassword1!" }
+
+HTTP 200
+[Captures]
+mallory_token: jsonpath "$.access_token"
+
+
+# Step 14b.i — Mallory tries to mint a public share on admin's
+#              folder: 404 (anti-enum). No token appears in the
+#              response body.
+POST {{base_url}}/api/shares
+Authorization: Bearer {{mallory_token}}
+Content-Type: application/json
+{
+  "item_id":   "{{share_folder_id}}",
+  "item_name": "public-share-test",
+  "item_type": "folder"
+}
+
+HTTP 404
+
+
+# Step 14b.ii — Same attempt on admin's file: 404.
+POST {{base_url}}/api/shares
+Authorization: Bearer {{mallory_token}}
+Content-Type: application/json
+{
+  "item_id":   "{{shared_file_id}}",
+  "item_name": "hello.txt",
+  "item_type": "file"
+}
+
+HTTP 404
+
+
+# Step 14b.iii — Mallory has no shares — no partial success slipped
+#                through. (`GET /api/shares` returns only shares the
+#                caller created; response is paginated.)
+GET {{base_url}}/api/shares
+Authorization: Bearer {{mallory_token}}
+
+HTTP 200
+[Asserts]
+jsonpath "$.items" isCollection
+jsonpath "$.items" count == 0
+
+
 # ─────────────────────────────────────────────────────────────
 # 15 — Teardown: revoke the password share + the direct
 #      file-share, then delete the folder.

tests/api/recent.hurl

@@ -187,3 +187,67 @@ DELETE {{base_url}}/api/recent/clear
 Authorization: Bearer {{token}}
 
 HTTP 200
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 10 — Cross-tenant regression (post-Drive AuthZ audit,
+#           Round 1 HIGH). Before this fix, `POST /api/recent/…`
+#           accepted any UUID and the listing endpoint JOINed back
+#           to storage.files/folders (name/mime/size/drive_id) — a
+#           metadata oracle over the whole tenant. Now the write
+#           path calls `authz.require(Read, …)`; unauthorised
+#           callers get 404 (anti-enum) + `authz.denied` audit line.
+#           See `docs/plan/authz_audit/rest_storage.md`.
+# ─────────────────────────────────────────────────────────────
+
+# Re-discover a folder id so the attacker has TWO targets to probe
+# (file + folder). Same test1 folder as favorites.hurl.
+GET {{base_url}}/api/folders/{{home_folder_id}}/resources?resource_types=folder
+Authorization: Bearer {{token}}
+
+HTTP 200
+[Captures]
+test1_id: jsonpath "$.items[0].resource.id"
+
+
+# Create/lookup the attacker. Idempotent: `HTTP *` accepts either
+# 201 (first run) or 409 (subsequent runs). Login below is the real
+# precondition.
+POST {{base_url}}/api/admin/users
+Authorization: Bearer {{token}}
+Content-Type: application/json
+{ "username": "rec_mallory", "password": "RecMalloryPassword1!", "email": "rec_mallory@example.com", "role": "user" }
+
+HTTP *
+
+
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "rec_mallory", "password": "RecMalloryPassword1!" }
+
+HTTP 200
+[Captures]
+mallory_token: jsonpath "$.access_token"
+
+
+# Step 10a — Record admin's file into mallory's recent: 404.
+POST {{base_url}}/api/recent/file/{{file_id}}
+Authorization: Bearer {{mallory_token}}
+
+HTTP 404
+
+
+# Step 10b — Same for admin's folder: 404.
+POST {{base_url}}/api/recent/folder/{{test1_id}}
+Authorization: Bearer {{mallory_token}}
+
+HTTP 404
+
+
+# Step 10c — Mallory's recent list stays empty.
+GET {{base_url}}/api/recent/resources
+Authorization: Bearer {{mallory_token}}
+
+HTTP 200
+[Asserts]
+jsonpath "$.items" count == 0