diff --git a/docs/architecture/user-lifecycle.md b/docs/architecture/user-lifecycle.md index fda08ec4..371cedab 100644 --- a/docs/architecture/user-lifecycle.md +++ b/docs/architecture/user-lifecycle.md @@ -99,7 +99,13 @@ These are codified in the module-level docstring of `application/ports/user_life 1. **First-ever login detection.** `on_user_login` fires *before* `user.register_login()` is called, so `user.last_login_at().is_none()` is a reliable "this is the user's first login since account creation" signal. Use it for welcome emails, one-shot default-resource seeding, "complete your profile" prompts. -2. **Idempotency is mandatory.** `on_user_login` fires on every successful authentication, not just the first. A hook that creates a resource must check whether the resource already exists before creating it. Cache invalidation, audit deduplication, etc., must all tolerate redundant calls. +2. **External-user short-circuit.** Hooks that provision per-user resources (home folder, default calendar, address book, GPG keys, …) must start with `if user.is_external() { return Ok(()); }`. External users (`is_external = TRUE`) are grant-only recipients — they have no home folder and don't consume storage quota. The DB `CHECK (NOT is_external OR storage_used_bytes = 0)` constraint catches code paths that bypass this short-circuit. + + **Subtle but important rule**: external users can **never** be admins. The DB enforces this via `CHECK (NOT (is_external AND role = 'admin'))`. `User::new_external(...)` doesn't accept a role parameter — it always sets `UserRole::User`. To make an existing external user an admin, an admin must first convert them to internal (`UPDATE auth.users SET is_external = FALSE`) and *then* update the role. The two-step process is intentional friction: granting admin to a federated principal would let external identity providers indirectly manage the local instance. + +3. **Idempotency is mandatory.** `on_user_login` fires on every successful authentication, not just the first. A hook that creates a resource must check whether the resource already exists before creating it. Cache invalidation, audit deduplication, etc., must all tolerate redundant calls. + +4. **External → internal conversion needs no special event.** When an admin flips `is_external = FALSE`, the user's next login fires `on_user_login` with the new flag value. Idempotent hooks see `!is_external` and missing resources → provision. No `on_user_converted` method needed; the safety-net pattern carries the load. 3. **Failure swallowing on create/login.** If your hook returns `Err`, the user is still created/logged in; only your hook's effect is delayed. Log enough detail via `tracing::error!` that subsequent investigation can identify the user. The next successful login's `on_user_login` will retry idempotently. diff --git a/migrations/20260612000002_auth_users_is_external.sql b/migrations/20260612000002_auth_users_is_external.sql new file mode 100644 index 00000000..14902f2f --- /dev/null +++ b/migrations/20260612000002_auth_users_is_external.sql @@ -0,0 +1,53 @@ +-- ════════════════════════════════════════════════════════════════════════════ +-- Add `is_external` flag to auth.users +-- ════════════════════════════════════════════════════════════════════════════ +-- Distinguishes storage-owning internal users (default — `is_external = FALSE`) +-- from grant-only external users (`is_external = TRUE`). External users are +-- recipients of share-grants who do not have a home folder, do not consume +-- storage quota, and authenticate via magic-link / OIDC / OCM (future). +-- +-- This migration is additive — all existing rows default to internal. The +-- backend code that consumes the flag lands in PR 3 (HomeFolderLifecycleHook +-- short-circuits when `is_external = TRUE`) and the magic-link flow ships +-- later still. +-- +-- Subject::External(uuid) in the domain becomes redundant after this — every +-- principal is now Subject::User(uuid) with `is_external` as a property, not +-- a variant. The cleanup happens in a small follow-up after the flag has +-- been observed in production. + +ALTER TABLE auth.users + ADD COLUMN IF NOT EXISTS is_external BOOLEAN NOT NULL DEFAULT FALSE; + +-- Partial index for the two query patterns that scan by this flag: +-- - admin "list external users" surface +-- - GDPR / cleanup sweepers that filter on `is_external = TRUE` and +-- last_login_at older than a threshold. +-- Internal-user queries don't go through this index — they ignore the +-- column entirely. +CREATE INDEX IF NOT EXISTS idx_users_is_external_login + ON auth.users (is_external, last_login_at) + WHERE is_external = TRUE; + +-- Schema-level safety net: external users must not be charged for storage. +-- HomeFolderLifecycleHook (PR 3) short-circuits before creating a home +-- folder for them, so storage_used_bytes should stay at 0. This CHECK +-- catches any code path that bypasses the hook and tries to attribute +-- storage to an external user. +ALTER TABLE auth.users + ADD CONSTRAINT users_external_no_storage + CHECK (NOT is_external OR storage_used_bytes = 0); + +-- Forbid external + admin combination. External users are grant-only +-- recipients authenticating via federated identity (magic-link, OIDC, +-- future OCM). Granting them the admin role would let a federated +-- principal manage the local instance — undesirable. To promote an +-- external user to admin: first flip is_external to FALSE (converting +-- them to internal), then update the role separately. The two steps +-- are intentional friction. +ALTER TABLE auth.users + ADD CONSTRAINT users_external_not_admin + CHECK (NOT (is_external AND role = 'admin')); + +COMMENT ON COLUMN auth.users.is_external IS + 'TRUE for grant-only external recipients (magic-link, OIDC-only, OCM federated). FALSE for storage-owning internal users. Set at creation; can be flipped to FALSE by admin to convert external → internal (next login provisions the home folder via HomeFolderLifecycleHook).'; diff --git a/src/application/dtos/settings_dto.rs b/src/application/dtos/settings_dto.rs index 37649b94..0154cc07 100644 --- a/src/application/dtos/settings_dto.rs +++ b/src/application/dtos/settings_dto.rs @@ -91,9 +91,15 @@ pub struct AdminCreateUserDto { /// "admin" or "user"; defaults to "user" pub role: Option, /// Storage quota in bytes; 0 = unlimited. If omitted, uses role default. + /// Ignored when `is_external = true` (external users have no storage). pub quota_bytes: Option, /// Whether the account is active; defaults to true pub active: Option, + /// `true` to create a grant-only external user (no home folder, no + /// storage quota). Defaults to `false` (internal user). External + /// users authenticate via magic-link / OIDC / OCM federation — + /// password is set but never used. + pub is_external: Option, } /// Request body for admin password reset diff --git a/src/application/dtos/user_dto.rs b/src/application/dtos/user_dto.rs index 3188b03b..54d47f3b 100644 --- a/src/application/dtos/user_dto.rs +++ b/src/application/dtos/user_dto.rs @@ -19,6 +19,11 @@ pub struct UserDto { pub auth_provider: String, pub image: Option, pub can_edit_image: bool, + /// `true` for grant-only external recipients (magic-link, OIDC-only, + /// future OCM federated). External users have no home folder and + /// can't own storage; their quota is always 0. Internal users + /// default to `false`. + pub is_external: bool, } impl From for UserDto { @@ -37,6 +42,7 @@ impl From for UserDto { auth_provider: user.oidc_provider().unwrap_or("local").to_string(), image: user.image().map(|s| s.to_string()), can_edit_image: !user.is_oidc_user(), + is_external: user.is_external(), } } } diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index b334d29e..008c2b8c 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -858,21 +858,60 @@ impl AuthApplicationService { _ => UserRole::User, }; - // Determine quota, capped to available disk space - let quota = dto.quota_bytes.unwrap_or_else(|| self.capped_quota(&role)); + let is_external = dto.is_external.unwrap_or(false); - // Hash password + // Forbid external + admin combo. The DB `users_external_not_admin` + // CHECK constraint would catch this too, but a 400 with an + // explanatory message is friendlier than a generic 500 from a + // constraint violation. See the CHECK definition in + // migrations/20260612000002_auth_users_is_external.sql for the + // rationale. + if is_external && matches!(role, UserRole::Admin) { + return Err(DomainError::new( + ErrorKind::InvalidInput, + "User", + "External users cannot be admins. To promote an external user to admin, \ + first convert them to internal (set is_external = false), then update \ + the role separately." + .to_string(), + )); + } + + // External users never own storage. The DB `users_external_no_storage` + // CHECK constraint enforces this; setting quota=0 here keeps the + // domain consistent and matches `User::new_external`. + let quota = if is_external { + 0 + } else { + dto.quota_bytes.unwrap_or_else(|| self.capped_quota(&role)) + }; + + // Hash password (kept for both internal and external users — for + // external users it's currently unused since they authenticate via + // magic-link / OIDC, but the DB column is NOT NULL). let password_hash = self.password_hasher.hash_password(&dto.password).await?; - // Create domain entity - let user = - User::new(dto.username.clone(), email, password_hash, role, quota).map_err(|e| { - DomainError::new( - ErrorKind::InvalidInput, - "User", - format!("Error creating user: {}", e), - ) - })?; + // Create domain entity. External path uses `new_external` so the + // is_external flag is set + the EXTERNAL placeholder password + // marker is applied for clarity in DB inspection. `new_external` + // forces role=User (the admin+external combo was rejected above). + let user = if is_external { + User::new_external(dto.username.clone(), email).map(|mut u| { + // The hashed password from the request is unused for auth + // but is persisted so audit-trail integrity is preserved. + u.update_password_hash(password_hash); + u + }) + } else { + User::new(dto.username.clone(), email, password_hash, role, quota) + } + .map_err(|e| { + DomainError::new( + ErrorKind::InvalidInput, + "User", + format!("Error creating user: {}", e), + ) + })?; // Persist let created = self.user_storage.create_user(user).await?; @@ -890,11 +929,20 @@ impl AuthApplicationService { lc.dispatch_created(&created).await; } - // Create personal folder - self.create_personal_folder(&dto.username, created.id()) - .await; + // External users have no home folder by design. Internal users + // get one — PR 3 will move this provisioning into the lifecycle + // hook (which short-circuits on `is_external` itself). + if !created.is_external() { + self.create_personal_folder(&dto.username, created.id()) + .await; + } - tracing::info!("Admin created user: {} ({})", dto.username, created.id()); + tracing::info!( + "Admin created user: {} ({}, is_external={})", + dto.username, + created.id(), + created.is_external() + ); Ok(UserDto::from(created)) } diff --git a/src/application/services/user_lifecycle_service.rs b/src/application/services/user_lifecycle_service.rs index 12e91db1..16047c76 100644 --- a/src/application/services/user_lifecycle_service.rs +++ b/src/application/services/user_lifecycle_service.rs @@ -145,6 +145,7 @@ impl UserLifecycleHook for AuditLifecycleHook { event = "user.created", user_id = %user.id(), username = %user.username(), + is_external = user.is_external(), ); Ok(()) } @@ -155,6 +156,7 @@ impl UserLifecycleHook for AuditLifecycleHook { event = "user.login", user_id = %user.id(), username = %user.username(), + is_external = user.is_external(), first_login = user.last_login_at().is_none(), ); Ok(()) @@ -166,6 +168,7 @@ impl UserLifecycleHook for AuditLifecycleHook { event = "user.logout", user_id = %user.id(), username = %user.username(), + is_external = user.is_external(), reason = ?reason, ); Ok(()) @@ -177,6 +180,7 @@ impl UserLifecycleHook for AuditLifecycleHook { event = "user.deleted", user_id = %user.id(), username = %user.username(), + is_external = user.is_external(), mode = ?mode, ); Ok(()) diff --git a/src/domain/entities/user.rs b/src/domain/entities/user.rs index 1d4572c9..ad1b795c 100644 --- a/src/domain/entities/user.rs +++ b/src/domain/entities/user.rs @@ -36,6 +36,13 @@ pub struct User { oidc_provider: Option, oidc_subject: Option, image: Option, + /// TRUE = grant-only external recipient (magic-link, OIDC-only, OCM + /// federated). FALSE = storage-owning internal user. Hooks that + /// provision per-user resources (home folder, default calendar, …) + /// must short-circuit when `is_external` is TRUE — see tip #2 in + /// `application/ports/user_lifecycle.rs`. The DB CHECK constraint + /// `users_external_no_storage` is the schema-level safety net. + is_external: bool, } impl User { @@ -85,6 +92,7 @@ impl User { oidc_provider: None, oidc_subject: None, image: None, + is_external: false, }) } @@ -115,6 +123,48 @@ impl User { oidc_provider: Some(oidc_provider), oidc_subject: Some(oidc_subject), image: None, + is_external: false, + }) + } + + /// Create a new external user — magic-link / OIDC-only / OCM-federated + /// recipient who does NOT own storage. The `CHECK (NOT is_external OR + /// storage_used_bytes = 0)` DB constraint enforces the no-storage rule + /// at the schema level. + /// + /// **External users are always `UserRole::User`** — there is no role + /// parameter because admin + external is an explicitly forbidden + /// combination enforced by the `users_external_not_admin` DB CHECK + /// constraint. Granting admin to a federated principal would let + /// external identity providers indirectly manage the local instance. + /// To make an external user an admin: first convert them to internal + /// (`UPDATE auth.users SET is_external = FALSE`), then update role. + /// The two-step process is intentional friction. + /// + /// Quota is set to 0 because external users can't upload content + /// into any folder they own (they have no folder). They can only + /// act on grants the resource owner provides — which counts against + /// the owner's quota, not theirs. + pub fn new_external(username: String, email: String) -> UserResult { + Self::validate_username(&username)?; + Self::validate_email(&email)?; + let now = Utc::now(); + Ok(Self { + id: Uuid::new_v4(), + username, + email, + password_hash: "__EXTERNAL_NO_PASSWORD__".to_string(), + role: UserRole::User, + storage_quota_bytes: 0, + storage_used_bytes: 0, + created_at: now, + updated_at: now, + last_login_at: None, + active: true, + oidc_provider: None, + oidc_subject: None, + image: None, + is_external: true, }) } @@ -147,6 +197,13 @@ impl User { oidc_provider: None, oidc_subject: None, image: None, + // `from_data` is the minimal-args reconstruction path used by + // tests and JWT-claim-based principal hydration (which doesn't + // carry `is_external`). Default to FALSE — JWT-validated + // principals are existing internal users; magic-link external + // sessions take a different path that hydrates from DB via + // `from_data_full`. + is_external: false, } } @@ -166,6 +223,7 @@ impl User { oidc_provider: Option, oidc_subject: Option, image: Option, + is_external: bool, ) -> Self { Self { id, @@ -182,6 +240,7 @@ impl User { oidc_provider, oidc_subject, image, + is_external, } } @@ -242,6 +301,14 @@ impl User { self.image.as_deref() } + /// `TRUE` for grant-only external recipients (magic-link, OIDC-only, + /// OCM federated). Hooks provisioning per-user resources must + /// short-circuit when this returns `true` — see tip #2 in + /// `application/ports/user_lifecycle.rs`. + pub fn is_external(&self) -> bool { + self.is_external + } + pub fn set_image(&mut self, image: Option) { self.image = image; self.updated_at = Utc::now(); diff --git a/src/infrastructure/repositories/pg/user_pg_repository.rs b/src/infrastructure/repositories/pg/user_pg_repository.rs index 3a8436c4..8d2669c8 100644 --- a/src/infrastructure/repositories/pg/user_pg_repository.rs +++ b/src/infrastructure/repositories/pg/user_pg_repository.rs @@ -84,13 +84,13 @@ impl UserRepository for UserPgRepository { let _result = sqlx::query( r#" INSERT INTO auth.users ( - id, username, email, password_hash, role, - storage_quota_bytes, storage_used_bytes, + id, username, email, password_hash, role, + storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject + oidc_provider, oidc_subject, is_external ) VALUES ( $1, $2, $3, $4, $5::auth.userrole, $6, $7, $8, $9, $10, $11, - $12, $13 + $12, $13, $14 ) RETURNING * "#, @@ -108,6 +108,7 @@ impl UserRepository for UserPgRepository { .bind(user_clone.is_active()) .bind(user_clone.oidc_provider()) .bind(user_clone.oidc_subject()) + .bind(user_clone.is_external()) .execute(&mut **tx) .await .map_err(Self::map_sqlx_error)?; @@ -131,7 +132,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE id = $1 "#, @@ -163,6 +164,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), )) } @@ -174,7 +176,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE username = $1 "#, @@ -206,6 +208,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), )) } @@ -217,7 +220,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE email = $1 "#, @@ -249,6 +252,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), )) } @@ -354,7 +358,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users ORDER BY created_at DESC LIMIT $1 OFFSET $2 @@ -391,6 +395,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), ) }) .collect(); @@ -406,7 +411,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE username ILIKE $1 OR email ILIKE $1 ORDER BY username @@ -443,6 +448,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), ) }) .collect(); @@ -529,7 +535,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE role::text = $1 ORDER BY created_at DESC @@ -565,6 +571,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), ) }) .collect(); @@ -600,7 +607,7 @@ impl UserRepository for UserPgRepository { id, username, email, password_hash, role::text as role_text, storage_quota_bytes, storage_used_bytes, created_at, updated_at, last_login_at, active, - oidc_provider, oidc_subject, image + oidc_provider, oidc_subject, image, is_external FROM auth.users WHERE oidc_provider = $1 AND oidc_subject = $2 "#, @@ -632,6 +639,7 @@ impl UserRepository for UserPgRepository { row.get("oidc_provider"), row.get("oidc_subject"), row.get("image"), + row.get("is_external"), )) }