diff --git a/docs/plan/drive.md b/docs/plan/drive.md index 8967a206..8897cc96 100644 --- a/docs/plan/drive.md +++ b/docs/plan/drive.md @@ -637,14 +637,40 @@ accommodates them without schema migration) | URL | Resolves to | |---|---| -| `/webdav/` | Caller's personal drive root + `` (back-compat with today's behaviour) | -| `/webdav/drives//` | Specific drive root + `` | +| `/webdav/` | Caller's default personal drive root + `` (back-compat with today's behaviour) | +| `/webdav/@drive//` | Specific drive root + `` | Today's `/webdav/` handler implicitly looks up the caller's home folder and prepends it. Post-drives, the same handler looks up the caller's personal drive and resolves paths inside it. **Zero breakage** for existing native WebDAV clients. +**Why the `@drive` sigil and NOT `/webdav/drives//...`** +(earlier draft) or top-level `/drives//...` (also +considered): `@` is the established structural-routing sigil +(GitHub `@user/repo`, npm `@scope/pkg`, LDAP `@domain`) — it +reads as "this is not user content, this is a routing token." +Realistic collision risk drops to near-zero: nobody creates a +top-level folder named exactly `@drive` by accident, and the +defensive layer collapses to a single one-liner in MKCOL / PUT / +REST create paths that refuses that literal name at any drive +root. Compared to top-level `/drives//...`, the `@drive` +shape keeps **one URL root for everything WebDAV** — single +`` block in reverse-proxy configs, single mental model +for sysadmins, single dispatcher in `webdav_routes()`. + +**Implementation notes:** +- Route parser accepts both `/webdav/@drive//...` and the + URL-encoded form `/webdav/%40drive//...` — WebDAV clients + percent-encode `@` inconsistently. +- One-liner guard in upload paths refuses creation of a folder + literally named `@drive` at any drive root (case-sensitive). +- `webdav_href()` (today at `webdav_handler.rs:94`) becomes + drive-context-aware: responses for a request under + `/webdav/@drive//...` must reference back to + `/webdav/@drive//...`, otherwise the client follows the + `` and lands on the back-compat surface (wrong drive). + The `drives` path segment is **reserved**: a folder literally named `drives` cannot exist at the top level of any drive. Migration pre-check refuses to start if existing data violates this — operator @@ -1359,9 +1385,13 @@ Pre-flight checks the migration script runs before any writes: operator can sanity-check ("Ed has 4 root folders, expected ≤1; verify those are real and intended before promoting them to drives"). -- Refuse if any sibling root folder is literally named `drives` - (would collide with the reserved URL segment on the native - `/webdav/drives//...` surface). Operator renames first. +- (Obsolete with the `/drives//...` top-level URL — kept + here for historical context.) Originally the migration was + going to refuse any sibling root folder literally named + `drives` because the URL surface was `/webdav/drives//...`. + D1 moved the explicit-drive selector to a separate top-level + `/drives//...` prefix (see §9 "Native WebDAV"), so the + collision no longer exists and the pre-check is unnecessary. - Refuse if any user has `storage_used_bytes > storage_quota_bytes` by an amount that wouldn't fit the destination drive's quota semantics (sanity check). @@ -1382,7 +1412,7 @@ us a real rollback window while the new model bakes in production. |---|---|---| | **D-Prep — role_grants refactor** | `access_grants → role_grants` schema migration with role-bundle semantics. `Manage` Permission added to the enum + role bundle. Engine reads role_grants only; `access_grants` removed (after one dual-write release if compat is needed). API gains `role` parameter on grant endpoints; audit log emits one `role_grant.*` event per role assignment instead of N permission events. **No Drive concept yet.** Sets the foundation that all subsequent PRs build on. **Data shape confirmed**: empirical audit shows >99% of existing `access_grants` rows already cluster into the standard bundles (viewer/editor/owner) — the migration is mechanical for the vast majority of data; the <1% edge cases get absorbed by shipping `commenter` and `contributor` roles on day one or get an explicit per-row migration decision logged. | **Medium** — touches the load-bearing authorisation table, but the data shape removes the main migration risk | | **D0 — foundation** | `storage.drives` schema (no `drive_members` — uses `role_grants` from D-Prep); `Drive` domain entity; migration creating personal drives + backfilling `drive_id` on every resource; read-only `GET /api/drives` listing the caller's drives (single query: `SELECT … FROM role_grants WHERE subject_id=$caller AND resource_type='drive'`). Dual-write `user_id` alongside `drive_id` for safety. **No new UI.** **Every upload path stamps `drive_id` at insert**: classic multipart (`file_handler::upload`), chunked NC (`uploads_handler`), streaming CDC (`upload_ingest`), delta upload (`delta_upload_service`), instant upload by hash. Tantivy reindex (see §11) is part of this PR. **Provenance columns added** (see §14): `created_by` and `updated_by` on both `storage.folders` and `storage.files`, FK to `auth.users` with `ON DELETE SET NULL`; backfilled from `user_id` so pre-Drive content has provenance from day one; every mutation path that touches `updated_at` also sets `updated_by`. | **High** — every storage query touches, all upload paths touched | -| **D1 — UI switcher + URL routing** | Sidebar drive picker, `/files/` reused for cross-drive navigation (existing route — drive context recovered server-side from `folders.drive_id`), `/config/drive/` new route for drive admin. `/` redirects to `/files/` of the caller's default personal drive (internal users) or `/shared-with-me` (external users with no personal drive). WebDAV path dispatcher recognising `drives/` as the drive-explicit prefix on `/webdav/` (NC keeps the credential-side scheme — see §9). `/drive/<...>` reserved for future use. | Medium | +| **D1 — UI switcher + URL routing** | Sidebar drive picker, `/files/` reused for cross-drive navigation (existing route — drive context recovered server-side from `folders.drive_id`), `/config/drive/` new route for drive admin. `/` redirects to `/files/` of the caller's default personal drive (internal users) or `/shared-with-me` (external users with no personal drive). Native WebDAV gets a new `/webdav/@drive//...` route alongside the existing `/webdav/` (which keeps mapping to the caller's default drive — zero back-compat breakage); the `@drive` sigil keeps everything WebDAV under one URL root with near-zero collision risk. NC keeps the credential-side scheme — see §9. `/drive/<...>` (singular, on the SPA) reserved for future use. | Medium | | **D2 — drive membership API + per-drive trash auth** | `POST /api/drives/{id}/members`, `DELETE`, `PUT` for role changes — thin handlers that translate to `role_grants` INSERT/DELETE/UPDATE with `resource_type='drive'`. `Resource::Drive(Uuid)` (added in D-Prep at the enum level) gets its specialised handler surface here. Shared-drive last-owner protection. Group-as-subject support reuses the existing `subject_groups` machinery. **Personal-drive guards** (`add_member`, `remove_member`, `delete_drive` refuse on `kind='personal'` — see §2). **Per-drive trash authorisation** (§12): trash listing filters by drive(s) the caller can read; trash mutations (send/restore/permanent-delete) require `role='owner'` on the drive; `storage.trash_items` VIEW updated to surface `drive_id`; orphan/aborted-upload sweep becomes per-drive. | Medium | | **D3 — group-owned shared drives** | "Create shared drive" flow — admin or group owner triggers, drive created with `kind='shared'`, initial owner row is the group. Group-deletion guard refuses if the group is the last owner of any drive. Drive-rename, drive-delete. | Low | | **D4 — per-drive quota** | Move storage accounting off `auth.users.storage_used_bytes` onto `storage.drives.used_bytes`. **Re-point the existing per-user incremental CTE** (introduced in v0.7.0 — see `b5b80549`, `d6987329`) at drive rows; don't reinvent the counting logic. Upload paths check `drive.quota_bytes` instead of (or in addition to) the user's quota for the dual-write window. **Per-chunk incremental quota check on the NC chunked path** (see §13): MKCOL refuses when the drive is already over quota; each PUT chunk runs an O(1) `used + session_so_far + chunk_size > quota` test and refuses with 507 within one chunk of wasted upload. Closes a pre-existing wart where NC clients could upload GB before learning they were over quota. Reconciliation job runs once per day to fix drift. | Medium | @@ -1656,8 +1686,14 @@ test`), **(c)** `cargo fmt && cargo clippy --all-features ### D1 - **Routing**: `cargo build` clean; WebDAV dispatcher routes - `/webdav/drives//...` correctly; `/webdav/` still - resolves to the caller's default drive (back-compat). + `/webdav/@drive//...` correctly (also accepts the + URL-encoded `/webdav/%40drive//...` form); + `/webdav/` still resolves to the caller's default drive + (back-compat). +- **Collision guard**: MKCOL / PUT / REST refuse creation of a + folder literally named `@drive` at any drive root (case-sensitive, + exact match — sub-folders named `@drive` deeper in the tree are + allowed, the guard is only at root depth). - **NC client back-compat**: a real NC sync client pointed at `/remote.php/dav/files/admin/` continues syncing the user's default personal drive without reconfiguration. The chroot POC's `~` diff --git a/src/application/dtos/grant_dto.rs b/src/application/dtos/grant_dto.rs index 11cd0543..f1e7cf93 100644 --- a/src/application/dtos/grant_dto.rs +++ b/src/application/dtos/grant_dto.rs @@ -9,6 +9,7 @@ use utoipa::{IntoParams, ToSchema}; use uuid::Uuid; use crate::application::dtos::cursor::{CursorListResponse, CursorQuery, PageCursor}; +use crate::application::dtos::drive_dto::DriveDto; use crate::application::dtos::file_dto::FileDto; use crate::application::dtos::folder_dto::FolderDto; use crate::domain::services::authorization::{Grant, Permission, Resource, Role, Subject}; @@ -418,6 +419,7 @@ impl SharedWithMeQuery { pub enum ResourceContentDto { File(FileDto), Folder(FolderDto), + Drive(DriveDto), } /// One item in the shared-with-me list. diff --git a/src/application/services/mod.rs b/src/application/services/mod.rs index e4089ec3..578e3d98 100644 --- a/src/application/services/mod.rs +++ b/src/application/services/mod.rs @@ -7,6 +7,7 @@ pub mod calendar_service; pub mod contact_service; pub mod delta_upload_service; pub mod device_auth_service; +pub mod drive_management_service; pub mod external_identity_service; pub mod favorites_service; pub mod file_lifecycle_service; diff --git a/src/common/di.rs b/src/common/di.rs index 97f1c790..660ecabb 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -1467,8 +1467,14 @@ impl AppServiceFactory { path_resolver: None, webdav_lock_store: crate::infrastructure::services::webdav_lock_service::create_webdav_lock_store(), - authorization, + authorization: authorization.clone(), drive_repo: drive_repo.clone(), + drive_management_service: Arc::new( + crate::application::services::drive_management_service::DriveManagementService::new( + drive_repo.clone(), + authorization.clone(), + ), + ), subject_group_service: Some(Arc::new( crate::application::services::subject_group_service::SubjectGroupService::new( subject_group_repo.clone(), @@ -1935,6 +1941,13 @@ pub struct AppState { /// resolved through `role_grants` not a separate `drive_members` /// table (see `docs/plan/drive.md` §3). pub drive_repo: Arc, + /// D2 — drive membership management service. Translates the membership + /// API (`POST/PATCH/DELETE /api/drives/{id}/members`) into role-grant + /// writes on `resource_type='drive'`, with the personal-drive guard + /// and shared-drive last-owner protection layered in. + pub drive_management_service: Arc< + crate::application::services::drive_management_service::DriveManagementService, + >, /// ReBAC subject-group management (CRUD + membership). `None` when the /// auth subsystem is not configured. pub subject_group_service: diff --git a/src/domain/repositories/drive_repository.rs b/src/domain/repositories/drive_repository.rs index ea2678ad..b047d692 100644 --- a/src/domain/repositories/drive_repository.rs +++ b/src/domain/repositories/drive_repository.rs @@ -78,6 +78,16 @@ pub trait DriveRepository: Send + Sync + 'static { /// when no row matches. async fn get_by_id(&self, id: Uuid) -> Result; + /// Batch fetch — returns one row per existing id. Missing ids are + /// silently dropped (matches the `get_files_by_ids` / `get_folders_by_ids` + /// shape used by `list_shared_with_me`). Caller-side `HashMap` + /// lookup gives `Option` semantics for stale grants whose drive + /// was deleted between listing and resolution. + async fn get_by_ids( + &self, + ids: &[Uuid], + ) -> Result, DriveRepositoryError>; + /// Return the caller's default personal drive paired with its /// display name, or `NotFound` if they don't have one (e.g. /// external users; users created before the lifecycle hook fired). diff --git a/src/infrastructure/repositories/pg/drive_pg_repository.rs b/src/infrastructure/repositories/pg/drive_pg_repository.rs index f02079b4..299e3f55 100644 --- a/src/infrastructure/repositories/pg/drive_pg_repository.rs +++ b/src/infrastructure/repositories/pg/drive_pg_repository.rs @@ -197,6 +197,32 @@ impl DriveRepository for DrivePgRepository { Self::row_to_drive_with_name(&row) } + async fn get_by_ids( + &self, + ids: &[Uuid], + ) -> Result, DriveRepositoryError> { + if ids.is_empty() { + return Ok(Vec::new()); + } + let rows = sqlx::query( + r#" + SELECT d.id, d.kind, d.default_for_user, d.root_folder_id, + d.quota_bytes, d.used_bytes, d.policies, + d.created_at, d.updated_at, + f.name AS root_folder_name + FROM storage.drives d + JOIN storage.folders f ON f.id = d.root_folder_id + WHERE d.id = ANY($1) + "#, + ) + .bind(ids) + .fetch_all(self.pool.as_ref()) + .await + .map_err(|e| Self::map_sqlx_err("get_by_ids", e))?; + + rows.iter().map(Self::row_to_drive_with_name).collect() + } + async fn find_default_for_user( &self, user_id: Uuid, diff --git a/src/interfaces/api/handlers/drive_handler.rs b/src/interfaces/api/handlers/drive_handler.rs index fc2191fb..46666402 100644 --- a/src/interfaces/api/handlers/drive_handler.rs +++ b/src/interfaces/api/handlers/drive_handler.rs @@ -1,22 +1,31 @@ -//! `GET /api/drives` — list every drive the caller can read. +//! Drive endpoints. //! -//! D0 ships the read-only listing; D2 adds shared-drive membership -//! mutations (`POST/DELETE/PUT /api/drives/{id}/members`), D3 adds the -//! create-shared-drive flow, etc. +//! - `GET /api/drives` — list every drive the caller can read (D0) +//! - `GET /api/drives/{id}/members` — list role grants on a drive (D2) +//! - `POST /api/drives/{id}/members` — add a member (D2) +//! - `PATCH /api/drives/{id}/members/{kind}/{sid}` — change a member's role / expiry (D2) +//! - `DELETE /api/drives/{id}/members/{kind}/{sid}` — remove a member (D2) //! -//! The handler resolves the caller's expanded subject set through the -//! engine (so group-mediated drive grants surface — the foundation for -//! D2/D3) and asks the `DriveRepository` for every drive that set can -//! read. Authorization is purely the subject-expansion step: no -//! `require(...)` call here, because "your accessible drives" is a -//! listing query, not a permission decision on a specific drive. +//! D3 adds the create-shared-drive flow under `POST /api/drives`. The +//! membership endpoints are thin wrappers around `DriveManagementService`, +//! which layers the personal-drive guard and shared-drive last-owner +//! protection on top of the generic `role_grants` write path. use std::sync::Arc; -use axum::{Json, extract::State, http::StatusCode, response::IntoResponse}; +use axum::{ + Json, + extract::{Path, State}, + http::StatusCode, + response::IntoResponse, +}; +use serde::Deserialize; use tracing::error; +use utoipa::ToSchema; +use uuid::Uuid; use crate::application::dtos::drive_dto::DriveDto; +use crate::application::dtos::grant_dto::{GrantDto, RoleDto, SubjectDto, SubjectTypeDto}; use crate::common::di::AppState; use crate::domain::repositories::drive_repository::DriveRepository; use crate::domain::services::authorization::Subject; @@ -39,10 +48,6 @@ pub async fn list_drives( ) -> impl IntoResponse { let caller_id = auth_user.id; - // Expand the caller's `Subject::User` into the `(types, ids)` pair - // that includes every group the user transitively belongs to. The - // engine caches this expansion in its Moka cache; if the caller - // just ran a permission check, this is a hit. let (subject_types, subject_ids) = match state .authorization .expand_subject_for_listing(Subject::User(caller_id)) @@ -70,3 +75,173 @@ pub async fn list_drives( } } } + +// ════════════════════════════════════════════════════════════════════════════ +// Membership API (D2) +// ════════════════════════════════════════════════════════════════════════════ + +/// Body for `POST /api/drives/{id}/members`. +#[derive(Debug, Deserialize, ToSchema)] +pub struct AddDriveMemberDto { + pub subject: SubjectDto, + pub role: RoleDto, + #[serde(default)] + pub expires_at: Option>, +} + +/// Body for `PATCH /api/drives/{id}/members/{kind}/{sid}`. +#[derive(Debug, Deserialize, ToSchema)] +pub struct UpdateDriveMemberDto { + pub role: RoleDto, + /// Optional. Pass `null` (or omit) to clear an existing expiry. + #[serde(default)] + pub expires_at: Option>, +} + +fn parse_subject(kind: SubjectTypeDto, id: Uuid) -> Subject { + match kind { + SubjectTypeDto::User => Subject::User(id), + SubjectTypeDto::Group => Subject::Group(id), + SubjectTypeDto::Token => Subject::Token(id), + } +} + +#[utoipa::path( + get, + path = "/api/drives/{id}/members", + params(("id" = Uuid, Path, description = "Drive UUID")), + responses( + (status = 200, description = "Role grants on this drive", body = Vec), + (status = 404, description = "Drive not found or caller lacks Read"), + ), + security(("bearerAuth" = [])), + tag = "drives" +)] +pub async fn list_drive_members( + State(state): State>, + auth_user: AuthUser, + Path(drive_id): Path, +) -> impl IntoResponse { + match state + .drive_management_service + .list_members(auth_user.id, drive_id) + .await + { + Ok(grants) => { + let dtos: Vec = grants.into_iter().map(GrantDto::from).collect(); + (StatusCode::OK, Json(dtos)).into_response() + } + Err(e) => AppError::from(e).into_response(), + } +} + +#[utoipa::path( + post, + path = "/api/drives/{id}/members", + params(("id" = Uuid, Path, description = "Drive UUID")), + request_body = AddDriveMemberDto, + responses( + (status = 201, description = "Member added", body = GrantDto), + (status = 400, description = "Validation error (e.g. last-owner constraint)"), + (status = 404, description = "Drive not found or caller lacks Manage"), + (status = 405, description = "Personal drive — membership is immutable"), + ), + security(("bearerAuth" = [])), + tag = "drives" +)] +pub async fn add_drive_member( + State(state): State>, + auth_user: AuthUser, + Path(drive_id): Path, + Json(dto): Json, +) -> impl IntoResponse { + let subject = parse_subject(dto.subject.kind, dto.subject.id); + match state + .drive_management_service + .set_member_role( + auth_user.id, + drive_id, + subject, + dto.role.into(), + dto.expires_at, + ) + .await + { + Ok(grant) => (StatusCode::CREATED, Json(GrantDto::from(grant))).into_response(), + Err(e) => AppError::from(e).into_response(), + } +} + +#[utoipa::path( + patch, + path = "/api/drives/{id}/members/{kind}/{sid}", + params( + ("id" = Uuid, Path, description = "Drive UUID"), + ("kind" = String, Path, description = "Subject kind: user|group|token"), + ("sid" = Uuid, Path, description = "Subject UUID"), + ), + request_body = UpdateDriveMemberDto, + responses( + (status = 200, description = "Member role updated", body = GrantDto), + (status = 400, description = "Validation error (e.g. last-owner demotion)"), + (status = 404, description = "Drive not found or caller lacks Manage"), + (status = 405, description = "Personal drive — membership is immutable"), + ), + security(("bearerAuth" = [])), + tag = "drives" +)] +pub async fn update_drive_member( + State(state): State>, + auth_user: AuthUser, + Path((drive_id, kind, subject_id)): Path<(Uuid, SubjectTypeDto, Uuid)>, + Json(dto): Json, +) -> impl IntoResponse { + let subject = parse_subject(kind, subject_id); + match state + .drive_management_service + .set_member_role( + auth_user.id, + drive_id, + subject, + dto.role.into(), + dto.expires_at, + ) + .await + { + Ok(grant) => (StatusCode::OK, Json(GrantDto::from(grant))).into_response(), + Err(e) => AppError::from(e).into_response(), + } +} + +#[utoipa::path( + delete, + path = "/api/drives/{id}/members/{kind}/{sid}", + params( + ("id" = Uuid, Path, description = "Drive UUID"), + ("kind" = String, Path, description = "Subject kind: user|group|token"), + ("sid" = Uuid, Path, description = "Subject UUID"), + ), + responses( + (status = 204, description = "Member removed (or was never a member — idempotent)"), + (status = 400, description = "Last-owner protection — promote another member first"), + (status = 404, description = "Drive not found or caller lacks Manage"), + (status = 405, description = "Personal drive — membership is immutable"), + ), + security(("bearerAuth" = [])), + tag = "drives" +)] +pub async fn remove_drive_member( + State(state): State>, + auth_user: AuthUser, + Path((drive_id, kind, subject_id)): Path<(Uuid, SubjectTypeDto, Uuid)>, +) -> impl IntoResponse { + let subject = parse_subject(kind, subject_id); + match state + .drive_management_service + .remove_member(auth_user.id, drive_id, subject) + .await + { + Ok(()) => StatusCode::NO_CONTENT.into_response(), + Err(e) => AppError::from(e).into_response(), + } +} diff --git a/src/interfaces/api/handlers/grant_handler.rs b/src/interfaces/api/handlers/grant_handler.rs index 2b3b0932..de0ea673 100644 --- a/src/interfaces/api/handlers/grant_handler.rs +++ b/src/interfaces/api/handlers/grant_handler.rs @@ -19,6 +19,7 @@ use utoipa::IntoParams; use uuid::Uuid; use crate::application::dtos::cursor::PageCursor; +use crate::application::dtos::drive_dto::DriveDto; use crate::application::dtos::grant_dto::{ CreateGrantDto, CreateGrantResponseDto, GrantDto, MySharesDto, NotifyOutcomeSetDto, OutgoingResourceGrantDto, OutgoingResourceItemDto, ResourceContentDto, ResourceDto, @@ -30,6 +31,7 @@ use crate::application::services::recipient_notification_service::NotifyTrigger; use crate::common::di::AppState; #[allow(unused_imports)] use crate::common::errors::DomainError; +use crate::domain::repositories::drive_repository::DriveRepository; use crate::domain::services::authorization::{ GrantCursor, IncomingGrantSummary, OutgoingResourceSummary, Permission, Resource, ResourceKind, Role, Subject, @@ -43,6 +45,28 @@ type AppStateRef = Arc; // POST /api/grants // ════════════════════════════════════════════════════════════════════════════ +/// Share a resource with someone — kicks off the **social flow** (invite + +/// notification + lazy external-user provisioning if subject is an email). +/// +/// **Use this when:** the recipient may not exist yet, hasn't been told, +/// or this is the initial grant. The handler: +/// - resolves `subject` (User / Group / Token / Email — email lazily creates +/// an external user via `MagicLinkInviteService::resolve_or_create_recipient`), +/// - writes one role-grant row via `authz.set_role` (`ON CONFLICT UPDATE`), +/// - sends a share-notification email (magic-link arm for externals, plain +/// notification for internal users — both honour `notify_on_share` opt-out +/// and per-recipient rate limits). +/// +/// **Compare with `PUT /api/grants/role`:** that endpoint is the *silent* +/// admin-style role change for an already-known subject; it skips the +/// invitation and notification side-effects entirely. Both write through +/// the same idempotent UPSERT, so the only operational difference is the +/// social flow attached here. +/// +/// **Drive resources** (`resource.type == "drive"`) are routed through +/// `DriveManagementService.set_member_role` internally — the +/// personal-drive guard and shared-drive last-owner protection apply +/// no matter which endpoint creates the grant. #[utoipa::path( post, path = "/api/grants", @@ -51,6 +75,7 @@ type AppStateRef = Arc; (status = 201, description = "Grant(s) created", body = CreateGrantResponseDto), (status = 400, description = "Invalid input (both/neither of permissions+role provided)"), (status = 404, description = "Resource not found OR caller lacks Share permission"), + (status = 405, description = "Drive resource: personal drives have immutable membership"), ), security(("bearerAuth" = [])), tag = "grants" @@ -131,14 +156,30 @@ pub async fn create_grant( // Single role row in `storage.role_grants`. `ON CONFLICT UPDATE` in // the engine makes repeated POSTs with the same (subject, resource) // a role refresh, matching the PATCH-style semantics callers expect. - let grant = match authz - .set_role(caller_id, subject, role, resource, expires_at) - .await - { - Ok(g) => g, - Err(err) => { - error!("set_role write failed: {err}"); - return AppError::from(err).into_response(); + // + // Drive resources are routed through `DriveManagementService` so the + // personal-drive guard and shared-drive last-owner protection apply + // — the same guards that gate `/api/drives/{id}/members`. Defense in + // depth: a caller can't bypass them by hitting `/api/grants` directly. + let grant = if let Resource::Drive(drive_id) = resource { + match state + .drive_management_service + .set_member_role(caller_id, drive_id, subject, role, expires_at) + .await + { + Ok(g) => g, + Err(err) => return AppError::from(err).into_response(), + } + } else { + match authz + .set_role(caller_id, subject, role, resource, expires_at) + .await + { + Ok(g) => g, + Err(err) => { + error!("set_role write failed: {err}"); + return AppError::from(err).into_response(); + } } }; let grants = vec![GrantDto::from(grant)]; @@ -267,33 +308,54 @@ pub async fn revoke_grant( Err(e) => return AppError::from(e).into_response(), }; - // Caller is authorized if they are the granter OR have Share on the resource. - if granter != caller_id - && let Err(e) = authz - .require(Subject::User(caller_id), Permission::Share, resource) + // Drive resources are routed through `DriveManagementService` so the + // personal-drive guard and shared-drive last-owner protection apply. + // Note: the "granter can always revoke" shortcut DOES NOT apply for + // drive grants — drive membership mutations always require current + // Manage on the drive, even if you granted the row originally and + // were later demoted. + if let Resource::Drive(drive_id) = resource { + // Also revoke the legacy access_grants row for the dual-write + // window — `remove_member` handles the role_grants side. + if let Err(e) = authz.revoke(grant_id).await { + return AppError::from(e).into_response(); + } + if let Err(e) = state + .drive_management_service + .remove_member(caller_id, drive_id, subject) .await - { - return AppError::from(e).into_response(); - } + { + return AppError::from(e).into_response(); + } + } else { + // Caller is authorized if they are the granter OR have Share on the resource. + if granter != caller_id + && let Err(e) = authz + .require(Subject::User(caller_id), Permission::Share, resource) + .await + { + return AppError::from(e).into_response(); + } - if let Err(e) = authz.revoke(grant_id).await { - return AppError::from(e).into_response(); - } + if let Err(e) = authz.revoke(grant_id).await { + return AppError::from(e).into_response(); + } - // D-Prep dual-write: clear the role_grants row for this (subject, - // resource). Idempotent — succeeds whether or not the row existed. - // - // Today's API revokes one access_grants row by id; the role_grants - // row models the WHOLE (subject, resource) cluster. Calling clear_role - // here effectively revokes the WHOLE role assignment in role_grants, - // even if other per-permission access_grants rows remain. This is the - // correct semantics for the eventual cleanup-PR model (role_grants is - // role-keyed; once access_grants goes away, "revoke" means "drop the - // role"). During the dual-write window the two tables can drift - // briefly if a caller revokes only some permissions of a role, but - // the engine still reads access_grants so behaviour is unchanged. - if let Err(e) = authz.clear_role(subject, resource).await { - return AppError::from(e).into_response(); + // D-Prep dual-write: clear the role_grants row for this (subject, + // resource). Idempotent — succeeds whether or not the row existed. + // + // Today's API revokes one access_grants row by id; the role_grants + // row models the WHOLE (subject, resource) cluster. Calling clear_role + // here effectively revokes the WHOLE role assignment in role_grants, + // even if other per-permission access_grants rows remain. This is the + // correct semantics for the eventual cleanup-PR model (role_grants is + // role-keyed; once access_grants goes away, "revoke" means "drop the + // role"). During the dual-write window the two tables can drift + // briefly if a caller revokes only some permissions of a role, but + // the engine still reads access_grants so behaviour is unchanged. + if let Err(e) = authz.clear_role(subject, resource).await { + return AppError::from(e).into_response(); + } } tracing::info!( @@ -481,13 +543,38 @@ pub async fn notify_grant_recipient( // PUT /api/grants/role // ════════════════════════════════════════════════════════════════════════════ +/// Change an existing member's role on a resource — **silent**, no +/// invitation, no notification. +/// +/// **Use this when:** the subject is already known (user/group/token has +/// an existing grant or you've already informed them out-of-band) and you +/// just want to bump their role. Typical use is the management UI's +/// "Viewer → Editor" dropdown — you don't want a fresh share email +/// firing on every dropdown change. +/// +/// **Compare with `POST /api/grants`:** that endpoint kicks off the +/// invitation + share-notification social flow and accepts an email-shaped +/// subject for lazy external-user provisioning. This endpoint is the +/// admin-style update — concrete subjects only, no side effects beyond +/// the role row itself. +/// +/// Both endpoints write through the same idempotent UPSERT +/// (`authz.set_role`, unique on `(subject, resource)`), so they are +/// indistinguishable in their effect on the role-grants table — the +/// difference is purely the social side-effects attached to `POST`. +/// +/// **Drive resources** (`resource.type == "drive"`) are routed through +/// `DriveManagementService.set_member_role` internally — the +/// personal-drive guard and shared-drive last-owner protection apply. #[utoipa::path( put, path = "/api/grants/role", request_body = UpdateRoleDto, responses( (status = 200, description = "Role applied; returns the new full grant set", body = Vec), + (status = 400, description = "Drive resource: shared-drive last-owner demotion refused"), (status = 404, description = "Resource not found or caller lacks Share"), + (status = 405, description = "Drive resource: personal drives have immutable membership"), ), security(("bearerAuth" = [])), tag = "grants" @@ -515,12 +602,27 @@ pub async fn set_role( // Atomic role refresh. UNIQUE on (subject, resource) + ON CONFLICT // UPDATE in `set_role` turns this into a single UPSERT — no diff, // no race window. Returns the resulting role row. - let grant = match authz - .set_role(caller_id, subject, role, resource, expires_at) - .await - { - Ok(g) => g, - Err(e) => return AppError::from(e).into_response(), + // + // Drive resources are routed through `DriveManagementService` so the + // personal-drive guard and shared-drive last-owner protection apply. + // See `create_grant` for the same delegation rationale. + let grant = if let Resource::Drive(drive_id) = resource { + match state + .drive_management_service + .set_member_role(caller_id, drive_id, subject, role, expires_at) + .await + { + Ok(g) => g, + Err(e) => return AppError::from(e).into_response(), + } + } else { + match authz + .set_role(caller_id, subject, role, resource, expires_at) + .await + { + Ok(g) => g, + Err(e) => return AppError::from(e).into_response(), + } }; tracing::info!( @@ -647,6 +749,10 @@ pub async fn list_shared_with_me( .iter() .filter(|s| matches!(s.resource_type, ResourceKind::Folder)) .collect(); + let drive_summaries: Vec<&IncomingGrantSummary> = summaries + .iter() + .filter(|s| matches!(s.resource_type, ResourceKind::Drive)) + .collect(); let file_service = &state.applications.file_retrieval_service; let folder_service = &state.applications.folder_service_concrete; @@ -660,14 +766,16 @@ pub async fn list_shared_with_me( .iter() .map(|s| s.resource_id.to_string()) .collect(); + let drive_ids: Vec = drive_summaries.iter().map(|s| s.resource_id).collect(); - // Resolve resource details in two batch queries (was one per id via + // Resolve resource details in three batch queries (was one per id via // join_all, which could fan out to ~limit concurrent pooled connections // and starve the primary pool). Missing ids — stale grants whose resource // was deleted before the cascade trigger fired — drop out of the maps. - let (file_list, folder_list) = tokio::join!( + let (file_list, folder_list, drive_list) = tokio::join!( file_service.get_files_by_ids(&file_ids), - folder_service.get_folders_by_ids(&folder_ids) + folder_service.get_folders_by_ids(&folder_ids), + state.drive_repo.get_by_ids(&drive_ids) ); let file_map: HashMap = match file_list { Ok(files) => files.into_iter().map(|f| (f.id.clone(), f)).collect(), @@ -677,6 +785,16 @@ pub async fn list_shared_with_me( Ok(folders) => folders.into_iter().map(|f| (f.id.clone(), f)).collect(), Err(e) => return AppError::from(e).into_response(), }; + let drive_map: HashMap = match drive_list { + Ok(drives) => drives + .into_iter() + .map(|d| (d.drive.id, DriveDto::from(d))) + .collect(), + Err(e) => { + return AppError::internal_error(format!("Failed to batch-resolve drives: {e:?}")) + .into_response(); + } + }; // Build the unified item list in original grant order (newest first), // looking each resolved resource up by id. @@ -719,13 +837,21 @@ pub async fn list_shared_with_me( summary.resource_id ), }, - // Drive grants don't appear in the file/folder "Shared with me" - // listing — they're surfaced through `GET /api/drives` (D0). - // Silently skipping here is the right behaviour: a drive grant - // discovered by `list_incoming_resources_paged` is not a stale - // grant, just a different resource type with a different - // listing surface. - ResourceKind::Drive => continue, + ResourceKind::Drive => match drive_map.get(&summary.resource_id) { + Some(drive_dto) => { + items.push(SharedWithMeItemDto { + resource_type: ResourceTypeDto::Drive, + permissions: summary.permissions.iter().map(|p| (*p).into()).collect(), + granted_at: summary.granted_at, + granted_by: summary.granted_by, + resource: ResourceContentDto::Drive(drive_dto.clone()), + }); + } + None => warn!( + "Skipping stale drive grant for resource_id={}: not found", + summary.resource_id + ), + }, } } @@ -884,6 +1010,10 @@ pub async fn list_my_shares( .iter() .filter(|s| matches!(s.resource_type, ResourceKind::Folder)) .collect(); + let drive_summaries: Vec<&OutgoingResourceSummary> = summaries + .iter() + .filter(|s| matches!(s.resource_type, ResourceKind::Drive)) + .collect(); let file_ids: Vec = file_summaries .iter() @@ -893,11 +1023,13 @@ pub async fn list_my_shares( .iter() .map(|s| s.resource_id.to_string()) .collect(); + let drive_ids: Vec = drive_summaries.iter().map(|s| s.resource_id).collect(); - // Two batch queries instead of one get_* per id (see list_shared_with_me). - let (file_list, folder_list) = tokio::join!( + // Three batch queries instead of one get_* per id (see list_shared_with_me). + let (file_list, folder_list, drive_list) = tokio::join!( file_service.get_files_by_ids(&file_ids), - folder_service.get_folders_by_ids(&folder_ids) + folder_service.get_folders_by_ids(&folder_ids), + state.drive_repo.get_by_ids(&drive_ids) ); let file_map: HashMap = match file_list { Ok(files) => files.into_iter().map(|f| (f.id.clone(), f)).collect(), @@ -907,6 +1039,16 @@ pub async fn list_my_shares( Ok(folders) => folders.into_iter().map(|f| (f.id.clone(), f)).collect(), Err(e) => return AppError::from(e).into_response(), }; + let drive_map: HashMap = match drive_list { + Ok(drives) => drives + .into_iter() + .map(|d| (d.drive.id, DriveDto::from(d))) + .collect(), + Err(e) => { + return AppError::internal_error(format!("Failed to batch-resolve drives: {e:?}")) + .into_response(); + } + }; let mut items: Vec = Vec::with_capacity(summaries.len()); @@ -960,10 +1102,20 @@ pub async fn list_my_shares( summary.resource_id ), }, - // Drive grants are surfaced via `GET /api/drives` (D0), not - // through the My Shares outgoing-resources surface. Silently - // skip — symmetric with the `list_shared_with_me` arm above. - ResourceKind::Drive => continue, + ResourceKind::Drive => match drive_map.get(&summary.resource_id) { + Some(drive_dto) => { + items.push(OutgoingResourceItemDto { + resource_type: ResourceTypeDto::Drive, + first_shared_at: summary.first_shared_at, + resource: ResourceContentDto::Drive(drive_dto.clone()), + grants, + }); + } + None => warn!( + "Skipping stale outgoing drive grant for resource_id={}: not found", + summary.resource_id + ), + }, } } diff --git a/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index 17b53ad7..97ca7e8e 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -416,13 +416,21 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { } // Drives — every drive the caller can read. D0 ships the read-only - // listing; D2 adds the membership API + shared-drive endpoints under - // `/api/drives/{id}/members`. + // listing; D2 adds the membership API; D3 the create-shared-drive flow. { use crate::interfaces::api::handlers::drive_handler; let drives_router = Router::new() .route("/", get(drive_handler::list_drives)) + .route( + "/{id}/members", + get(drive_handler::list_drive_members).post(drive_handler::add_drive_member), + ) + .route( + "/{id}/members/{kind}/{sid}", + axum::routing::patch(drive_handler::update_drive_member) + .delete(drive_handler::remove_drive_member), + ) .with_state(app_state.clone()); router = router.nest("/drives", drives_router);