From e5b9a4a8db75501d2904dffea250558f111abe71 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Fri, 19 Jun 2026 12:28:30 +0200 Subject: [PATCH] feat(drive): remove create_home_folder remove create_home_folder() & ensure_home_folder() now: on_user_created() and on_user_login both() call provision_if_needed() which calls **create_personal_drive_atomic()** add a helper to find Personal drive for a user and also it's root directorry --- docs/architecture/magic-link-auth.md | 2 +- docs/architecture/user-lifecycle.md | 39 ++++++-- src/application/ports/folder_ports.rs | 10 -- src/application/ports/user_lifecycle.rs | 2 +- .../services/auth_application_service.rs | 18 ++-- src/application/services/folder_service.rs | 94 +------------------ src/application/services/share_service.rs | 9 -- .../services/trash_service_test.rs | 9 -- .../services/user_lifecycle_service.rs | 2 +- src/common/di.rs | 4 +- src/common/stubs.rs | 18 ---- src/domain/repositories/drive_repository.rs | 61 ++++++++++++ src/domain/repositories/folder_repository.rs | 12 --- src/infrastructure/auth_factory.rs | 2 +- .../repositories/pg/folder_db_repository.rs | 94 +------------------ src/interfaces/nextcloud/login_v2_handler.rs | 43 +++++---- 16 files changed, 133 insertions(+), 286 deletions(-) diff --git a/docs/architecture/magic-link-auth.md b/docs/architecture/magic-link-auth.md index ba7c57c7..bfcb5d09 100644 --- a/docs/architecture/magic-link-auth.md +++ b/docs/architecture/magic-link-auth.md @@ -210,7 +210,7 @@ External users have no calendar, no address book, no home folder, and (by design 6. **`POST /api/auth/app-passwords` is closed.** App passwords are persistent credentials; the magic-link-eligibility rule (`has_login_credential`) assumes externals have **no other credential configured**. Letting an external mint an app password would break that invariant and would also be the only way to authenticate them on the NC surface. 403 + audit on rejection. 7. **`GET /api/groups/search` is closed.** Group names aren't strictly secret, but externals have no legitimate use for the share-dialog autocomplete (they can't be added to groups anyway). -Pre-existing safeguards from the user-lifecycle work continue to apply: the DB CHECK constraints `users_external_not_admin` and `users_external_no_storage`, and the `HomeFolderLifecycleHook` short-circuit that skips home-folder provisioning for externals. +Pre-existing safeguards from the user-lifecycle work continue to apply: the DB CHECK constraints `users_external_not_admin` and `users_external_no_storage`, and the `PersonalDriveLifecycleHook` short-circuit that skips drive provisioning for externals (they get no default drive, so `DriveRepository::home_root_folder_id_for(external_user_id)` returns `Ok(None)`). ### Why protocol-level instead of handler-level diff --git a/docs/architecture/user-lifecycle.md b/docs/architecture/user-lifecycle.md index c1d8bd16..ab81c57f 100644 --- a/docs/architecture/user-lifecycle.md +++ b/docs/architecture/user-lifecycle.md @@ -77,8 +77,7 @@ The asymmetry is deliberate. `on_user_created` and `on_user_login` must complete │ ┌────────────────┼────────────────┐ ▼ ▼ ▼ - AuditLifecycleHook HomeFolderHook AuthzCacheHook … - (PR 1 only) (PR 3) (PR 4) + AuditLifecycleHook PersonalDriveHook AuthzCacheHook … ``` ## Owner-located convention @@ -87,7 +86,7 @@ Each concrete hook impl lives **next to the service that owns the work**, not in Examples (PR plan): -- `HomeFolderLifecycleHook` lives in `src/application/services/folder_service.rs` — same module as `FolderService`, owner of home-folder policy. +- `PersonalDriveLifecycleHook` lives in `src/application/services/folder_service.rs` — same module as `FolderService`, owner of home-drive provisioning policy. It calls `DrivePgRepository::create_personal_drive_atomic` to create the drive + root folder + Owner role-grant in one atomic transaction (see [docs/plan/drive.md §3](https://github.com/EdouardVanbelle/OxiCloud/blob/main/docs/plan/drive.md)). - `AuthzCacheLifecycleHook` lives in `src/infrastructure/services/pg_acl_engine.rs` — same module as the Moka cache it invalidates. - `AuditLifecycleHook` lives in `src/application/services/user_lifecycle_service.rs` (with the dispatcher) — cross-cutting, no domain owner. @@ -120,7 +119,7 @@ These are codified in the module-level docstring of `application/ports/user_life | Hook | Lives in | Responsibility | |---|---|---| | `AuditLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with dispatcher) | All four events: emits one `tracing::info!(target: "audit", event = "user.*", ...)` per call, with `is_external` as a field. Co-located because audit is cross-cutting with no domain owner. | -| `HomeFolderLifecycleHook` | `src/application/services/folder_service.rs` (same module as `FolderService`) | `on_user_created` + `on_user_login`: idempotently provision "My Folder - {username}" via `FolderService::ensure_home_folder`. Short-circuits when `user.is_external()`. `on_user_logout`: `Ok(())`. `on_user_deleted`: per-mode `tracing::info!` event so audit distinguishes AdminDelete from GdprPurge — the FK CASCADE on `storage.folders.user_id` handles the actual row removal. Trash-with-retention is documented as future work. | +| `PersonalDriveLifecycleHook` | `src/application/services/folder_service.rs` (same module as `FolderService`) | `on_user_created` + `on_user_login`: idempotently provision the user's **default personal drive** + its root folder + Owner role-grant via `DrivePgRepository::create_personal_drive_atomic` — all four DB writes in one transaction (drive INSERT, folder INSERT, `drives.root_folder_id` UPDATE, `role_grants` INSERT). Idempotency: `find_default_for_user` short-circuits if the user already has a drive. Short-circuits when `user.is_external()`. `on_user_logout`: `Ok(())`. `on_user_deleted`: per-mode `tracing::info!` event so audit distinguishes AdminDelete from GdprPurge — the FK CASCADE on `storage.drives.default_for_user` handles the actual drive row removal, which cascades to folders/files via their `drive_id` FK. Trash-with-retention is documented as future work. | | `AuthzCacheLifecycleHook` | `src/infrastructure/services/pg_acl_engine.rs` (same module as the Moka cache it invalidates) | `on_user_logout` + `on_user_deleted`: `engine.invalidate_user_groups_cache(user.id())` — drops the cached transitive-group expansion immediately so a re-login (or a re-created account with the same id) sees fresh memberships without waiting for the 30 s TTL. `on_user_created` + `on_user_login`: `Ok(())` (no stale entry could exist for these). | | `SessionRevocationLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with dispatcher; no dedicated session service module today) | `on_user_deleted`: explicit `session_storage.revoke_all_user_sessions(user.id())` + aggregate audit event (`event = "user.sessions_revoked_on_delete", count = N`). Replaces the silent FK CASCADE with an observable revocation. All other events: `Ok(())`. | | `DeletionMode` | enum on the trait | Distinguishes admin-initiated delete (`AdminDelete` — currently identical to GDPR but reserved for a future trash-with-retention policy) from GDPR right-to-erasure purge (`GdprPurge` — for a future sweeper). PR 4 ships the variants; future PRs may add per-mode behaviour. | @@ -138,7 +137,7 @@ BEGIN dispatch_deleted(user, AdminDelete, &mut tx) │ ├── AuditLifecycleHook → tracing::info!(event="user.deleted", mode=...) - ├── HomeFolderLifecycleHook → tracing::info!("home folder will be removed via FK CASCADE") + ├── PersonalDriveLifecycleHook → tracing::info!("default drive + its tree will be removed via FK CASCADE on storage.drives.default_for_user") ├── AuthzCacheLifecycleHook → engine.invalidate_user_groups_cache(user_id) └── SessionRevocationLifecycleHook → session_storage.revoke_all_user_sessions(user_id) @@ -163,13 +162,35 @@ If any hook returns `Err`, the dispatcher propagates it; `delete_user_admin` rol 2. `AuthApplicationService::login()` validates the password against the stored Argon2 hash. 3. **Before** `user.register_login()` is called, the dispatcher fires `dispatch_login(&user)`. The user's `last_login_at` is still `None` from creation time. 4. `AuditLifecycleHook::on_user_login` runs first (registration order): emits `event = "user.login", user_id = ..., username = ..., is_external = false, first_login = true`. -5. `HomeFolderLifecycleHook::on_user_login` runs next: sees `!user.is_external()`, calls `FolderService::ensure_home_folder(uid, username)`. The service checks `list_folders_by_owner(None, uid)` — empty → creates `"My Folder - alice"`. Returns `Ok(true)` (newly created). +5. `PersonalDriveLifecycleHook::on_user_login` runs next: sees `!user.is_external()`, calls `provision_if_needed`. The hook asks `DriveRepository::find_default_for_user(uid)` — returns `NotFound`. It then calls `DrivePgRepository::create_personal_drive_atomic(uid, quota)` which runs the four-write transaction: INSERT drive, INSERT folder, UPDATE `drives.root_folder_id`, INSERT `role_grants` row. All four commit together. 6. Dispatcher finishes. `user.register_login()` is now called, stamping `last_login_at` to the current time. 7. The session row is INSERTed; access + refresh tokens generated; response returned to the client. -On the user's **second** login: same flow up through step 5, but `ensure_home_folder` finds the existing folder, returns `Ok(false)`, no-op. The `AuditLifecycleHook` still emits an event, but `first_login = false` this time. +On the user's **second** login: same flow up through step 5, but `find_default_for_user` returns `Ok(drive)` (the drive already exists). The hook re-emits the Owner `role_grant` via `set_role` (UPSERT-safe) as belt-and-suspenders against partial provisioning and returns. The `AuditLifecycleHook` still emits an event, but `first_login = false` this time. -If the home folder gets deleted manually (e.g., SQL `DELETE FROM storage.folders WHERE user_id = $1`), the user's **next** login will re-create it — that's the safety-net behaviour the lifecycle hook contractually owns. +If the drive gets deleted manually (e.g., SQL `DELETE FROM storage.drives WHERE default_for_user = $1`), the user's **next** login will re-create it — that's the safety-net behaviour the lifecycle hook contractually owns. The atomic four-write transaction ensures no partial state can leak through a half-deleted drive. + +### Identifying the home — never by name + +Code that needs to ask "is this the user's home?" must compare ids, not names. Users rename their home folder. Secondary personal drives keep their original sibling-root names. The single source of truth is **drive ownership**: the drive where `default_for_user = user_id` owns the user's home root folder via `root_folder_id`. + +Two helpers in `src/domain/repositories/drive_repository.rs` encapsulate the lookup: + +```rust +// "Give me this user's home root folder id (or None for external)." +drive_repo.home_root_folder_id_for(user_id).await + // → Result, DriveRepositoryError> + +// "Where in this list of items is the user's home?" — generic over +// the item shape; the caller passes an id-extractor closure. +position_of_user_home_root_folder( + drive_repo, user_id, &items, + |item| Uuid::parse_str(&item.id).ok(), +).await + // → Option +``` + +`home_root_folder_id_for` returns `Ok(None)` (not an error) for external users — they have no default drive. The position helper is a free function (not a trait method) so `DriveRepository` stays `dyn`-compatible. Use these everywhere; do not write new code that pattern-matches folder names like `"Personal"` or `"My Folder - "`. ### State of the art: @@ -177,7 +198,7 @@ If the home folder gets deleted manually (e.g., SQL `DELETE FROM storage.folders DI builds: UserLifecycleService ├── AuditLifecycleHook (in user_lifecycle_service.rs) - ├── HomeFolderLifecycleHook (in folder_service.rs) + ├── PersonalDriveLifecycleHook (in folder_service.rs) ├── AuthzCacheLifecycleHook (in pg_acl_engine.rs) ├── SessionRevocationLifecycleHook (in user_lifecycle_service.rs) └── ExternalIdentityLifecycleHook (in external_identity_service.rs, stubbed) diff --git a/src/application/ports/folder_ports.rs b/src/application/ports/folder_ports.rs index 0f7b42f5..ac043caa 100644 --- a/src/application/ports/folder_ports.rs +++ b/src/application/ports/folder_ports.rs @@ -96,16 +96,6 @@ pub trait FolderUseCase: Send + Sync + 'static { /// Deletes a folder (ownership verified against caller_id) async fn delete_folder_with_perms(&self, id: &str, caller_id: Uuid) -> Result<(), DomainError>; - /// Creates a root-level home folder for a user during registration. - /// `drive_id` is the user's personal drive; the wrapper folder stays - /// during the D0 dual-write window (retires in M2b later). - async fn create_home_folder( - &self, - user_id: Uuid, - drive_id: Uuid, - name: String, - ) -> Result; - /// Lists every folder in a subtree rooted at `folder_id` (inclusive), /// ordered by path. Uses ltree `<@` — single GiST-indexed query. /// diff --git a/src/application/ports/user_lifecycle.rs b/src/application/ports/user_lifecycle.rs index d5cf5b89..e927a3cf 100644 --- a/src/application/ports/user_lifecycle.rs +++ b/src/application/ports/user_lifecycle.rs @@ -127,7 +127,7 @@ pub enum LogoutReason { /// How aggressively `on_user_deleted` cleanup should run. Today both /// variants are equivalent (only `AuditLifecycleHook` exists, and it logs -/// regardless). The split exists so PR 4's `HomeFolderLifecycleHook` can +/// regardless). The split exists so PR 4's `PersonalDriveLifecycleHook` can /// trash on `AdminDelete` but hard-delete on `GdprPurge`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum DeletionMode { diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index a77e7125..8f94bfd6 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -124,7 +124,7 @@ pub struct AuthApplicationService { token_service: Arc, /// Dispatcher for user-lifecycle events. `None` only in tests that don't /// exercise the lifecycle path; production DI always wires this. - /// HomeFolderLifecycleHook (registered on this dispatcher) owns the + /// PersonalDriveLifecycleHook (registered on this dispatcher) owns the /// per-user folder provisioning that AuthApplicationService used to do /// inline pre-PR 3. user_lifecycle: Option>, @@ -404,7 +404,7 @@ impl AuthApplicationService { // Save user let created_user = self.user_storage.create_user(user).await?; - // Lifecycle: HomeFolderLifecycleHook handles personal-folder + // Lifecycle: PersonalDriveLifecycleHook handles personal-folder // creation (was inlined here pre-PR 3); audit log + future // provisioning steps land here too. if let Some(lc) = &self.user_lifecycle { @@ -506,8 +506,8 @@ impl AuthApplicationService { let created_user = self.user_storage.create_user(user).await?; // Lifecycle: notify hooks. PR 3 moves home-folder creation into - // HomeFolderLifecycleHook fired here. - // Lifecycle: HomeFolderLifecycleHook provisions the admin's + // PersonalDriveLifecycleHook fired here. + // Lifecycle: PersonalDriveLifecycleHook provisions the admin's // home folder. Audit logs the creation event. if let Some(lc) = &self.user_lifecycle { lc.dispatch_created(&created_user).await; @@ -654,7 +654,7 @@ impl AuthApplicationService { /// A second redemption attempt receives `Ok(false)` and is rejected /// as `AccessDenied`. /// 3. Load the user, verify they're active. - /// 4. Dispatch `on_user_login` (so HomeFolderLifecycleHook can + /// 4. Dispatch `on_user_login` (so PersonalDriveLifecycleHook can /// safety-net any internal user whose first credential happens /// to be a magic link — externals short-circuit by `is_external()`). /// 5. Register login + persist + issue session in the same pipeline @@ -1722,7 +1722,7 @@ impl AuthApplicationService { .await?; } - // Lifecycle: HomeFolderLifecycleHook handles the home-folder + // Lifecycle: PersonalDriveLifecycleHook handles the home-folder // provisioning (idempotent + short-circuits on is_external). // Audit logs the creation event. if let Some(lc) = &self.user_lifecycle { @@ -1785,7 +1785,7 @@ impl AuthApplicationService { /// Runs the whole flow in a single transaction so the lifecycle /// hooks (`SessionRevocationLifecycleHook` revoking sessions with /// audit, `AuthzCacheLifecycleHook` invalidating the Moka cache, - /// `HomeFolderLifecycleHook` for future trash policy, …) can do + /// `PersonalDriveLifecycleHook` for future trash policy, …) can do /// their work atomically with the user DELETE. If any hook returns /// `Err`, the transaction rolls back and the user remains intact. pub async fn delete_user_admin(&self, user_id: Uuid) -> Result<(), DomainError> { @@ -2260,7 +2260,7 @@ impl AuthApplicationService { // Lifecycle: created (audit + home-folder provisioning) + // login (no register_login() for a fresh OIDC user means // `last_login_at` is naturally None → first-login detection - // works). HomeFolderLifecycleHook creates the home folder. + // works). PersonalDriveLifecycleHook creates the home folder. if let Some(lc) = &self.user_lifecycle { lc.dispatch_created(&created_user).await; lc.dispatch_login(&created_user).await; @@ -2362,7 +2362,7 @@ impl AuthApplicationService { // `create_personal_folder` was removed in PR 3 of the // UserLifecycleHook migration — home-folder provisioning is now - // owned by `HomeFolderLifecycleHook` in folder_service.rs and runs + // owned by `PersonalDriveLifecycleHook` in folder_service.rs and runs // via `dispatch_created` / `dispatch_login`. } diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index 4a074a0a..8ec34bf7 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -167,15 +167,6 @@ impl FolderService { ) -> Result<(), DomainError> { Ok(()) } - - async fn create_home_folder( - &self, - _user_id: Uuid, - _drive_id: Uuid, - _name: String, - ) -> Result { - Ok(FolderDto::empty()) - } } FolderServiceStub @@ -239,30 +230,6 @@ impl FolderUseCase for FolderService { Ok(FolderDto::from(folder)) } - /// Creates a root-level home folder for a user during registration. - /// `drive_id` is the user's personal drive — the wrapper folder lives - /// inside it during the D0 dual-write window (M2b retires the wrapper - /// later). - async fn create_home_folder( - &self, - user_id: Uuid, - drive_id: Uuid, - name: String, - ) -> Result { - let folder = self - .folder_storage - .create_home_folder(user_id, drive_id, name) - .await - .map_err(|e| { - DomainError::internal_error( - "FolderStorage", - format!("Failed to create home folder: {}", e), - ) - })?; - - Ok(FolderDto::from(folder)) - } - async fn list_subtree_folders(&self, folder_id: &str) -> Result, DomainError> { let folders = self.folder_storage.list_subtree_folders(folder_id).await?; Ok(folders.into_iter().map(FolderDto::from).collect()) @@ -341,7 +308,7 @@ impl FolderUseCase for FolderService { /// /// **Note (post PR 3):** the self-heal block that auto-created a /// home folder when listing returned empty has been removed. - /// `HomeFolderLifecycleHook` (registered on `UserLifecycleService`) + /// `PersonalDriveLifecycleHook` (registered on `UserLifecycleService`) /// now provisions the folder on `on_user_created` / `on_user_login`, /// idempotently, so the listing path no longer needs to self-heal. async fn list_folders_with_perms( @@ -626,63 +593,6 @@ impl FolderService { Ok((rows, next_cursor)) } - - /// Idempotently provision a home folder for a user. - /// - /// Returns `Ok(true)` if a folder was newly created, `Ok(false)` if the - /// user already had at least one root folder. - /// - /// **System-level operation** — bypasses authz because this runs on - /// the user's own behalf (during creation or login provisioning) at a - /// point where the caller may be the engine itself, not an HTTP user. - /// Callers must be inside trusted code paths (lifecycle hooks). - /// - /// Used by [`HomeFolderLifecycleHook`] on `on_user_created` and - /// `on_user_login`. Replaces the old self-heal at the listing path - /// and the four eager `create_personal_folder` calls in - /// `AuthApplicationService` (removed in the same PR). - pub async fn ensure_home_folder( - &self, - user_id: Uuid, - drive_id: Uuid, - username: Option<&str>, - ) -> Result { - let existing = self - .folder_storage - .list_folders_by_owner(None, user_id) - .await - .map_err(|e| { - DomainError::internal_error( - "FolderStorage", - format!("ensure_home_folder: list root folders: {}", e), - ) - })?; - if !existing.is_empty() { - return Ok(false); - } - - let folder_name = match username { - Some(u) => format!("My Folder - {}", u), - None => format!("My Folder - {}", user_id), - }; - self.folder_storage - .create_home_folder(user_id, drive_id, folder_name.clone()) - .await - .map_err(|e| { - DomainError::internal_error( - "FolderStorage", - format!("ensure_home_folder: create: {}", e), - ) - })?; - tracing::info!( - target: "user_lifecycle", - hook = "home_folder", - user_id = %user_id, - folder_name = %folder_name, - "Home folder provisioned" - ); - Ok(true) - } } /// Build the next-page cursor from the last row of the current page. @@ -738,7 +648,7 @@ fn build_folder_resource_cursor( } // ───────────────────────────────────────────────────────────────────────────── -// HomeFolderLifecycleHook +// PersonalDriveLifecycleHook // // Owns home-folder provisioning policy. Replaces: // - the 4 eager `create_personal_folder` calls in AuthApplicationService diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index c747e70e..06bb64f0 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -1034,15 +1034,6 @@ mod tests { async fn delete_folder_permanently(&self, _folder_id: &str) -> Result<(), DomainError> { unimplemented!() } - - async fn create_home_folder( - &self, - _user_id: Uuid, - _drive_id: Uuid, - _name: String, - ) -> Result { - unimplemented!() - } } struct MockShareRepository { diff --git a/src/application/services/trash_service_test.rs b/src/application/services/trash_service_test.rs index feda0b4e..74621d9b 100644 --- a/src/application/services/trash_service_test.rs +++ b/src/application/services/trash_service_test.rs @@ -848,15 +848,6 @@ impl FolderRepository for MockFolderRepository { )) } } - - async fn create_home_folder( - &self, - _user_id: Uuid, - _drive_id: Uuid, - _name: String, - ) -> std::result::Result { - Ok(Folder::default()) - } } #[cfg(integration_tests)] diff --git a/src/application/services/user_lifecycle_service.rs b/src/application/services/user_lifecycle_service.rs index 0a1793f4..de1db8b4 100644 --- a/src/application/services/user_lifecycle_service.rs +++ b/src/application/services/user_lifecycle_service.rs @@ -132,7 +132,7 @@ impl UserLifecycleService { // // Always-on observer. Emits one structured `tracing::info!(target: "audit", // ...)` line per event. The only hook registered in PR 1; subsequent PRs -// add HomeFolderLifecycleHook, AuthzCacheLifecycleHook, etc., each living +// add PersonalDriveLifecycleHook, AuthzCacheLifecycleHook, etc., each living // next to the service it works for. // ───────────────────────────────────────────────────────────────────────────── diff --git a/src/common/di.rs b/src/common/di.rs index 160c3c2d..9ff71517 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -1157,7 +1157,7 @@ impl AppServiceFactory { // audit event is recorded // even if a later hook // errors out. - // 2. HomeFolderLifecycleHook — provisions the user's + // 2. PersonalDriveLifecycleHook — provisions the user's // home folder on // created/login (no-op // for external users). @@ -1235,7 +1235,7 @@ impl AppServiceFactory { // Auth services. Folder service no longer threaded here — // PR 3 moved home-folder provisioning into - // HomeFolderLifecycleHook, which already holds an Arc to the + // PersonalDriveLifecycleHook, which already holds an Arc to the // folder service via the user_lifecycle dispatcher. if self.config.features.enable_auth { let services = crate::infrastructure::auth_factory::create_auth_services( diff --git a/src/common/stubs.rs b/src/common/stubs.rs index 8698e4a4..b6ae2e93 100644 --- a/src/common/stubs.rs +++ b/src/common/stubs.rs @@ -353,15 +353,6 @@ impl FolderRepository for StubFolderStoragePort { async fn delete_folder_permanently(&self, _folder_id: &str) -> Result<(), DomainError> { Ok(()) } - - async fn create_home_folder( - &self, - _user_id: Uuid, - _drive_id: Uuid, - _name: String, - ) -> Result { - Ok(Folder::default()) - } } // --------------------------------------------------------------------------- @@ -495,15 +486,6 @@ impl FolderUseCase for StubFolderUseCase { ) -> Result<(), DomainError> { Ok(()) } - - async fn create_home_folder( - &self, - _user_id: Uuid, - _drive_id: Uuid, - _name: String, - ) -> Result { - Ok(FolderDto::default()) - } } // --------------------------------------------------------------------------- diff --git a/src/domain/repositories/drive_repository.rs b/src/domain/repositories/drive_repository.rs index a384c9d9..ea2678ad 100644 --- a/src/domain/repositories/drive_repository.rs +++ b/src/domain/repositories/drive_repository.rs @@ -88,6 +88,30 @@ pub trait DriveRepository: Send + Sync + 'static { user_id: Uuid, ) -> Result; + /// Canonical "what is this user's home root folder id?" lookup. + /// + /// Returns `Some(uuid)` for any internal user with a default personal + /// drive (the lifecycle hook provisions one at registration), and + /// `None` for users who have no default drive (external users; users + /// created before the hook existed). The id identifies the user's + /// home **by drive ownership** (`default_for_user == user_id`), + /// never by folder name — users can rename their home, so any code + /// that wants to ask "is this folder the user's home?" must compare + /// folder ids, not names. + /// + /// Storage errors (DB unreachable, etc.) bubble up as `Err`; the + /// "user simply has no home" case is `Ok(None)`, not an error. + async fn home_root_folder_id_for( + &self, + user_id: Uuid, + ) -> Result, DriveRepositoryError> { + match self.find_default_for_user(user_id).await { + Ok(d) => Ok(Some(d.drive.root_folder_id)), + Err(DriveRepositoryError::NotFound(_)) => Ok(None), + Err(e) => Err(e), + } + } + /// List drives the caller can read, resolved via `role_grants` for /// `resource_type='drive'`. The caller's group memberships are /// expanded by the engine's `subject_match_set`; that expanded set @@ -110,3 +134,40 @@ impl DriveKind { DriveKind::parse(s).ok_or_else(|| DriveRepositoryError::InvalidKind(s.to_owned())) } } + +/// Locate the user's home root folder within a generic list of items, +/// identifying it by **drive ownership** (never by folder name — users +/// can rename their home). +/// +/// `id_fn` extracts a candidate `Uuid` from each item. The callsite +/// commonly works with `FolderDto` (whose `id` is a `String`); the +/// closure is `|f| Uuid::parse_str(&f.id).ok()`. Items whose ids can't +/// be parsed are simply skipped — `position` ignores them. +/// +/// Defined as a free function (not a trait method) so the +/// `DriveRepository` trait stays `dyn`-compatible. Generic over both +/// the repo (`R`) and the item shape (`T`); accepts both concrete repo +/// types and `&dyn DriveRepository`. +/// +/// Returns `None` when: +/// * The user has no default drive (external users, pre-hook accounts). +/// * The user's home root folder id isn't present in `items`. +/// * The repo lookup errored (storage error is swallowed to None — +/// callers wanting fail-loud semantics should call +/// `home_root_folder_id_for` directly). +pub async fn position_of_user_home_root_folder( + drive_repo: &R, + user_id: Uuid, + items: &[T], + id_fn: impl Fn(&T) -> Option, +) -> Option +where + R: DriveRepository + ?Sized, +{ + let home_id = drive_repo + .home_root_folder_id_for(user_id) + .await + .ok() + .flatten()?; + items.iter().position(|item| id_fn(item) == Some(home_id)) +} diff --git a/src/domain/repositories/folder_repository.rs b/src/domain/repositories/folder_repository.rs index c9afdb46..aa45286a 100644 --- a/src/domain/repositories/folder_repository.rs +++ b/src/domain/repositories/folder_repository.rs @@ -136,18 +136,6 @@ pub trait FolderRepository: Send + Sync + 'static { /// Permanently deletes a folder (used by the trash) async fn delete_folder_permanently(&self, folder_id: &str) -> Result<(), DomainError>; - /// Creates a root-level home folder for a user inside their personal drive. - /// Called during user registration / first login to maintain the wrapper- - /// folder convention through the D0 dual-write window (the wrapper itself - /// retires in a follow-up migration; for now it stays as a real folder - /// row stamped with `drive_id`). - async fn create_home_folder( - &self, - user_id: Uuid, - drive_id: Uuid, - name: String, - ) -> Result; - /// Lists every folder in a subtree rooted at `folder_id` (inclusive). /// /// Uses ltree `<@` for a single GiST-indexed scan. The result is diff --git a/src/infrastructure/auth_factory.rs b/src/infrastructure/auth_factory.rs index 2003464f..d15790e9 100644 --- a/src/infrastructure/auth_factory.rs +++ b/src/infrastructure/auth_factory.rs @@ -47,7 +47,7 @@ pub async fn create_auth_services( ); // Wire the user-lifecycle dispatcher. Home-folder provisioning is - // now handled by HomeFolderLifecycleHook (registered on the + // now handled by PersonalDriveLifecycleHook (registered on the // dispatcher in DI) — AuthApplicationService no longer needs a // direct FolderService dependency for that path. auth_app_service = auth_app_service.with_user_lifecycle(user_lifecycle); diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index 60994f66..d968f05b 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -198,7 +198,10 @@ impl FolderRepository for FolderDbRepository { } else { return Err(DomainError::internal_error( "FolderDb", - "Cannot create root folder without user_id — use create_home_folder instead", + "Cannot create root folder — root folders are reserved for the \ + atomic drive-creation transaction in DrivePgRepository::\ + create_personal_drive_atomic (docs/plan/drive.md §3). The \ + no-orphan-root-folder trigger enforces this at the DB level.", )); }; @@ -944,95 +947,6 @@ impl FolderRepository for FolderDbRepository { Ok(()) } - async fn create_home_folder( - &self, - user_id: Uuid, - drive_id: Uuid, - name: String, - ) -> Result { - // D0-9 keeps the wrapper-folder convention through the dual-write - // window: the lifecycle hook creates the personal drive AND a - // root folder under it. Wrapper retirement (the `My Folder - - // /` prefix and the wrapper row itself) lands in M2b - // alongside the path rewrite. drive_id is required (M3 NOT NULL); - // created_by/updated_by are stamped from user_id for D0 - // provenance. - let row = sqlx::query_as::<_, (String, String, i64, i64, i64)>( - r#" - INSERT INTO storage.folders - (name, parent_id, user_id, drive_id, created_by, updated_by) - VALUES ($1, NULL, $2, $3, $2, $2) - ON CONFLICT DO NOTHING - RETURNING id::text, - path, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint, - EXTRACT(EPOCH FROM tree_modified_at)::bigint - "#, - ) - .bind(&name) - .bind(user_id) - .bind(drive_id) - .fetch_optional(self.pool()) - .await - .map_err(|e| DomainError::internal_error("FolderDb", format!("home folder: {e}")))?; - - match row { - Some((id, path, ca, ma, tma)) => Self::row_to_folder( - id, - name.clone(), - path, - None, - Some(user_id), - drive_id, - ca, - ma, - tma, - // INSERT stamped both provenance columns from user_id - // (D0 dual-write); D2 will plumb the real caller_id. - Some(user_id), - Some(user_id), - ), - None => { - // Already exists — fetch it. SELECT also pulls the §14 - // provenance columns so the entity layer reflects DB truth. - let existing = sqlx::query_as::< - _, - (String, String, i64, i64, i64, Option, Option), - >( - r#" - SELECT id::text, - path, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint, - EXTRACT(EPOCH FROM tree_modified_at)::bigint, - created_by, updated_by - FROM storage.folders - WHERE name = $1 AND user_id = $2 AND parent_id IS NULL - "#, - ) - .bind(&name) - .bind(user_id) - .fetch_one(self.pool()) - .await - .map_err(|e| DomainError::internal_error("FolderDb", format!("home fetch: {e}")))?; - Self::row_to_folder( - existing.0, - name, - existing.1, - None, - Some(user_id), - drive_id, - existing.2, - existing.3, - existing.4, - existing.5, - existing.6, - ) - } - } - } - /// Lists every folder in a subtree rooted at `folder_id` (inclusive). /// /// Single GiST-indexed query: `fo.lpath <@ (root's lpath)`. diff --git a/src/interfaces/nextcloud/login_v2_handler.rs b/src/interfaces/nextcloud/login_v2_handler.rs index 5f321cae..810fc773 100644 --- a/src/interfaces/nextcloud/login_v2_handler.rs +++ b/src/interfaces/nextcloud/login_v2_handler.rs @@ -29,26 +29,11 @@ struct DrivePickerTemplate { drives: Vec, } -/// Find the index of the user's home folder inside `drives`. -/// -/// Convention: home is the root folder named `"My Folder - {username}"`, -/// set by `FolderService::ensure_home_folder` at registration. Extra -/// root folders (POC drive seeding via direct SQL insert) don't follow -/// this name, so the pattern disambiguates home from sibling drives. -/// Returns `None` if no folder matches — caller decides whether that -/// is fatal or just "treat everything as a non-home drive". -/// -/// Mirrors the same lookup performed in `routes.rs:: -/// verify_url_user_and_resolve_chroot` (legacy no-`~` path); both -/// sites must agree on which row is home or the URL and the auth -/// marker will diverge. -fn find_home_index( - drives: &[crate::application::dtos::folder_dto::FolderDto], - username: &str, -) -> Option { - let expected = format!("My Folder - {}", username); - drives.iter().position(|f| f.name == expected) -} +// Home identification is via `position_of_user_home_root_folder` from +// `domain::repositories::drive_repository` — a generic helper that +// keys off `drives.default_for_user == user_id` rather than folder +// name, so user renames of the home folder don't silently break the +// picker UX. /// Serve an HTML page with a Content-Security-Policy header as defense-in-depth. fn html_with_csp(html: &'static str) -> Response { @@ -242,7 +227,14 @@ pub async fn handle_login_submit( // `loop.first`, so placing home first is the single point // that makes the picker UI line up with the home convention. // Other drives keep their original alphabetical order. - if let Some(idx) = find_home_index(&drives, ¤t_user.username) + if let Some(idx) = + crate::domain::repositories::drive_repository::position_of_user_home_root_folder( + state.drive_repo.as_ref(), + current_user.id, + &drives, + |f| uuid::Uuid::parse_str(&f.id).ok(), + ) + .await && idx != 0 { let home = drives.remove(idx); @@ -478,7 +470,14 @@ pub async fn handle_drive_pick( return StatusCode::INTERNAL_SERVER_ERROR.into_response(); } }; - let home_id = find_home_index(&drives, &user.username).map(|i| drives[i].id.as_str()); + let home_id = crate::domain::repositories::drive_repository::position_of_user_home_root_folder( + state.drive_repo.as_ref(), + user.id, + &drives, + |f| uuid::Uuid::parse_str(&f.id).ok(), + ) + .await + .map(|i| drives[i].id.as_str()); let is_home = home_id == Some(drive_id.as_str()); let drive_marker = if is_home { None