security(favorite,recent): ensure read permission

src/application/services/favorites_service.rs

@@ -9,10 +9,12 @@ use crate::application::dtos::favorites_dto::{
     BatchFavoritesResult, BatchFavoritesStats, FavoriteItemDto, FavoriteResourceRow,
     FavoritesCursor,
 };
+use crate::application::ports::authorization_ports::AuthorizationEngine;
 use crate::application::ports::favorites_ports::{FavoritesRepositoryPort, FavoritesUseCase};
-use crate::common::errors::{DomainError, ErrorKind, Result};
+use crate::common::errors::Result;
-use crate::domain::services::authorization::ResourceKind;
+use crate::domain::services::authorization::{Permission, Resource, ResourceKind, Subject};
 use crate::infrastructure::repositories::pg::FavoritesPgRepository;
+use crate::infrastructure::services::pg_acl_engine::PgAclEngine;
 
 /// Implementation of the FavoritesUseCase for managing user favorites.
 ///
@@ -20,12 +22,22 @@ use crate::infrastructure::repositories::pg::FavoritesPgRepository;
 /// accessing the database directly, following hexagonal architecture.
 pub struct FavoritesService {
     repo: Arc<FavoritesPgRepository>,
+    /// ReBAC engine — enforces `Permission::Read` on the referenced
+    /// file/folder before enrolling it into a user's favorites.
+    /// Without this gate the write path is an information oracle:
+    /// listing endpoints JOIN back to `storage.files/folders` and
+    /// return name/mime/size/drive_id for any UUID the caller was
+    /// able to enroll. See `docs/plan/authz_audit/rest_storage.md`.
+    authorization: Arc<PgAclEngine>,
 }
 
 impl FavoritesService {
     /// Create a new FavoritesService with the given repository port
-    pub fn new(repo: Arc<FavoritesPgRepository>) -> Self {
+    pub fn new(repo: Arc<FavoritesPgRepository>, authorization: Arc<PgAclEngine>) -> Self {
-        Self { repo }
+        Self {
+            repo,
+            authorization,
+        }
     }
 
     /// Subset of `(item_id, item_type)` pairs the user has favorited — used to
@@ -60,13 +72,15 @@ impl FavoritesUseCase for FavoritesService {
             item_type, item_id, user_id
         );
 
-        if item_type != "file" && item_type != "folder" {
+        // AuthZ pre-write: caller must have Read on the referenced
-            return Err(DomainError::new(
+        // resource. Denial routes through `require` → NotFound
-                ErrorKind::InvalidInput,
+        // (anti-enum, matches the listing shape) + `authz.denied`
-                "Favorites",
+        // audit line. Without this gate the write path was an
-                "Item type must be 'file' or 'folder'",
+        // information oracle over the whole tenant.
-            ));
+        let resource = Resource::parse(item_type, item_id)?;
-        }
+        self.authorization
+            .require(Subject::User(user_id), Permission::Read, resource)
+            .await?;
 
         self.repo.add_favorite(user_id, item_id, item_type).await?;
         info!(
@@ -125,18 +139,17 @@ impl FavoritesUseCase for FavoritesService {
             user_id
         );
 
-        // Validate all item types
+        // AuthZ pre-write: caller must have Read on every referenced
+        // resource. Fail the whole batch on the first denial so the
+        // response shape doesn't tell an attacker which items were
+        // valid (partial success would leak the same oracle we
+        // closed on the single-item path). See
+        // `docs/plan/authz_audit/rest_storage.md`.
         for (item_id, item_type) in items {
-            if item_type != "file" && item_type != "folder" {
+            let resource = Resource::parse(item_type, item_id)?;
-                return Err(DomainError::new(
+            self.authorization
-                    ErrorKind::InvalidInput,
+                .require(Subject::User(user_id), Permission::Read, resource)
-                    "Favorites",
+                .await?;
-                    format!(
-                        "Item type must be 'file' or 'folder' for item '{}'",
-                        item_id
-                    ),
-                ));
-            }
         }
 
         let requested = items.len();

src/application/services/recent_service.rs

@@ -1,10 +1,12 @@
 use crate::application::dtos::cursor::PageCursor;
 use crate::application::dtos::recent_dto::{RecentCursor, RecentItemDto, RecentResourceRow};
+use crate::application::ports::authorization_ports::AuthorizationEngine;
 use crate::application::ports::recent_ports::{RecentItemsRepositoryPort, RecentItemsUseCase};
 use crate::application::ports::resource_access_hook::ResourceAccessHook;
-use crate::common::errors::{DomainError, ErrorKind, Result};
+use crate::common::errors::Result;
-use crate::domain::services::authorization::ResourceKind;
+use crate::domain::services::authorization::{Permission, Resource, ResourceKind, Subject};
 use crate::infrastructure::repositories::pg::RecentItemsPgRepository;
+use crate::infrastructure::services::pg_acl_engine::PgAclEngine;
 use std::sync::{Arc, OnceLock};
 use tracing::info;
 use uuid::Uuid;
@@ -16,6 +18,13 @@ use uuid::Uuid;
 pub struct RecentService {
     repo: Arc<RecentItemsPgRepository>,
     max_recent_items: i32,
+    /// ReBAC engine — enforces `Permission::Read` on the referenced
+    /// file/folder before enrolling it into a user's Recent list.
+    /// The listing side JOINs back to `storage.files/folders` and
+    /// returns name/mime/size/drive_id for any enrolled UUID, so
+    /// the write path is an information oracle without this gate.
+    /// See `docs/plan/authz_audit/rest_storage.md`.
+    authorization: Arc<PgAclEngine>,
     /// Set after construction via [`Self::set_resource_access_hook`].
     /// The hook is built FROM this service (it wraps an `Arc<Self>`), so
     /// we can't take it as a constructor arg without circular ownership;
@@ -28,10 +37,15 @@ pub struct RecentService {
 
 impl RecentService {
     /// Create a new recent items service
-    pub fn new(repo: Arc<RecentItemsPgRepository>, max_recent_items: i32) -> Self {
+    pub fn new(
+        repo: Arc<RecentItemsPgRepository>,
+        authorization: Arc<PgAclEngine>,
+        max_recent_items: i32,
+    ) -> Self {
         Self {
             repo,
             max_recent_items: max_recent_items.clamp(1, 100),
+            authorization,
             resource_access_hook: OnceLock::new(),
         }
     }
@@ -87,13 +101,16 @@ impl RecentItemsUseCase for RecentService {
             item_type, item_id, user_id
         );
 
-        if item_type != "file" && item_type != "folder" {
+        // AuthZ pre-write: caller must have Read on the referenced
-            return Err(DomainError::new(
+        // resource. Denial routes through `require` → NotFound
-                ErrorKind::InvalidInput,
+        // (anti-enum) + `authz.denied` audit line. Without this
-                "RecentItems",
+        // gate the write path was an information oracle over the
-                "Item type must be 'file' or 'folder'",
+        // whole tenant via the listing endpoint's JOIN back to
-            ));
+        // storage.files/folders.
-        }
+        let resource = Resource::parse(item_type, item_id)?;
+        self.authorization
+            .require(Subject::User(user_id), Permission::Read, resource)
+            .await?;
 
         self.repo.upsert_access(user_id, item_id, item_type).await?;
         self.repo.prune(user_id, self.max_recent_items).await?;

src/common/di.rs

@@ -889,23 +889,37 @@ impl AppServiceFactory {
         Some(service)
     }
 
-    /// Creates the favorites service (requires database)
+    /// Creates the favorites service (requires database + authz engine
-    pub fn create_favorites_service(&self, db_pool: &Arc<PgPool>) -> Arc<FavoritesService> {
+    /// for the Read gate on `add_to_favorites` — see the post-Drive
+    /// AuthZ audit).
+    pub fn create_favorites_service(
+        &self,
+        db_pool: &Arc<PgPool>,
+        authorization: &Arc<PgAclEngine>,
+    ) -> Arc<FavoritesService> {
         let repo = Arc::new(
             crate::infrastructure::repositories::pg::FavoritesPgRepository::new(db_pool.clone()),
         );
-        let service = Arc::new(FavoritesService::new(repo));
+        let service = Arc::new(FavoritesService::new(repo, authorization.clone()));
         tracing::info!("Favorites service initialized");
         service
     }
 
-    /// Creates the recent items service (requires database)
+    /// Creates the recent items service (requires database + authz
-    pub fn create_recent_service(&self, db_pool: &Arc<PgPool>) -> Arc<RecentService> {
+    /// engine for the Read gate on `record_item_access` — see the
+    /// post-Drive AuthZ audit).
+    pub fn create_recent_service(
+        &self,
+        db_pool: &Arc<PgPool>,
+        authorization: &Arc<PgAclEngine>,
+    ) -> Arc<RecentService> {
         let repo = Arc::new(
             crate::infrastructure::repositories::pg::RecentItemsPgRepository::new(db_pool.clone()),
         );
         let service = Arc::new(RecentService::new(
-            repo, 50, // Maximum recent items per user
+            repo,
+            authorization.clone(),
+            50, // Maximum recent items per user
         ));
         tracing::info!("Recent items service initialized");
         service
@@ -1161,31 +1175,6 @@ impl AppServiceFactory {
         let pool = Arc::new(pools.primary);
         let maintenance_pool = Arc::new(pools.maintenance);
 
-        // Recent service + recording hook are built up-front so the
-        // hook can be threaded into `create_application_services` below.
-        // The file services hold the hook directly so every authorised
-        // `_with_perms` read/write fires into `auth.user_recent_files`
-        // without per-handler wiring. Reordering vs the legacy in-block
-        // creation (further down) is safe: `create_recent_service` only
-        // needs `pool`, which is already in scope.
-        //
-        // The back-edge `recent_service_eager.set_resource_access_hook`
-        // closes the loop so the clear/remove handlers can drop the
-        // hook's in-memory throttle entries — without it a freshly
-        // cleared Recent list refuses to re-record the same file for a
-        // full TTL window, surfacing as "I cleared, opened the file,
-        // and Recent is still empty" (caught by tests/api/recent.hurl
-        // step 8).
-        let recent_service_eager = self.create_recent_service(&pool);
-        let resource_access_hook: Arc<
-            dyn crate::application::ports::resource_access_hook::ResourceAccessHook,
-        > = Arc::new(
-            crate::infrastructure::services::recent_recording_hook::RecentRecordingHook::new(
-                recent_service_eager.clone(),
-            ),
-        );
-        recent_service_eager.set_resource_access_hook(resource_access_hook.clone());
-
         // 1. Core services (PgPool needed for DedupService index)
         let core = self.create_core_services(&pool, &maintenance_pool).await?;
 
@@ -1196,6 +1185,10 @@ impl AppServiceFactory {
         // because services hold an Arc<PgAclEngine> for ReBAC checks.
         // SubjectGroupPgRepository is constructed here too so the engine can
         // expand a user's transitive group set on cache misses.
+        //
+        // Moved above the eager recent-service build so `create_recent_service`
+        // can receive an `Arc<PgAclEngine>` — the Read gate on
+        // `record_item_access` (post-Drive AuthZ audit fix) needs it.
         let subject_group_repo = Arc::new(
             crate::infrastructure::repositories::pg::SubjectGroupPgRepository::new(pool.clone()),
         );
@@ -1206,6 +1199,29 @@ impl AppServiceFactory {
             subject_group_repo.clone(),
         );
 
+        // Recent service + recording hook are built up-front so the
+        // hook can be threaded into `create_application_services` below.
+        // The file services hold the hook directly so every authorised
+        // `_with_perms` read/write fires into `auth.user_recent_files`
+        // without per-handler wiring.
+        //
+        // The back-edge `recent_service_eager.set_resource_access_hook`
+        // closes the loop so the clear/remove handlers can drop the
+        // hook's in-memory throttle entries — without it a freshly
+        // cleared Recent list refuses to re-record the same file for a
+        // full TTL window, surfacing as "I cleared, opened the file,
+        // and Recent is still empty" (caught by tests/api/recent.hurl
+        // step 8).
+        let recent_service_eager = self.create_recent_service(&pool, &authorization);
+        let resource_access_hook: Arc<
+            dyn crate::application::ports::resource_access_hook::ResourceAccessHook,
+        > = Arc::new(
+            crate::infrastructure::services::recent_recording_hook::RecentRecordingHook::new(
+                recent_service_eager.clone(),
+            ),
+        );
+        recent_service_eager.set_resource_access_hook(resource_access_hook.clone());
+
         // Drive repository — needed both by the lifecycle hook (when auth
         // is enabled) and by `GET /api/drives` on the final `AppState`,
         // so declared at the outer scope.
@@ -1279,7 +1295,7 @@ impl AppServiceFactory {
         > = None;
 
         {
-            let favs = self.create_favorites_service(&pool);
+            let favs = self.create_favorites_service(&pool, &authorization);
             favorites_service = Some(favs.clone());
             apps.favorites_service = Some(favs);
 

src/domain/services/authorization.rs

@@ -118,6 +118,34 @@ impl Resource {
             _ => None,
         }
     }
+
+    /// Parse `(item_type, item_id)` from an API-facing pair of strings
+    /// (favorites, recent, batch endpoints all take this shape).
+    /// Combines UUID parse + type mapping so callers stay one-line and
+    /// error shapes are identical across surfaces. Returns
+    /// `DomainError::new(InvalidInput, …)` on malformed input; callers
+    /// that need the anti-enum 404 shape do that separately by feeding
+    /// the parsed `Resource` into `authz.require(...)`.
+    pub fn parse(
+        item_type: &str,
+        item_id: &str,
+    ) -> Result<Self, crate::common::errors::DomainError> {
+        use crate::common::errors::{DomainError, ErrorKind};
+        let uuid = Uuid::parse_str(item_id).map_err(|_| {
+            DomainError::new(
+                ErrorKind::InvalidInput,
+                "Resource",
+                format!("Invalid item UUID '{item_id}'"),
+            )
+        })?;
+        Self::from_parts(item_type, uuid).ok_or_else(|| {
+            DomainError::new(
+                ErrorKind::InvalidInput,
+                "Resource",
+                format!("Unsupported item type '{item_type}'"),
+            )
+        })
+    }
 }
 
 impl fmt::Display for Resource {