feat(user): add is_external flag, will permit integration of external users (without any storage) but able to be invited

This commit is contained in:
Edouard Vanbelle
2026-06-01 15:51:05 +02:00
parent bb6429a620
commit e130842bfc
8 changed files with 226 additions and 28 deletions
+7 -1
View File
@@ -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.
@@ -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).';
+6
View File
@@ -91,9 +91,15 @@ pub struct AdminCreateUserDto {
/// "admin" or "user"; defaults to "user"
pub role: Option<String>,
/// 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<i64>,
/// Whether the account is active; defaults to true
pub active: Option<bool>,
/// `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<bool>,
}
/// Request body for admin password reset
+6
View File
@@ -19,6 +19,11 @@ pub struct UserDto {
pub auth_provider: String,
pub image: Option<String>,
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<User> for UserDto {
@@ -37,6 +42,7 @@ impl From<User> 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(),
}
}
}
@@ -858,15 +858,54 @@ 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| {
// 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",
@@ -890,11 +929,20 @@ impl AuthApplicationService {
lc.dispatch_created(&created).await;
}
// Create personal folder
// 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))
}
@@ -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(())
+67
View File
@@ -36,6 +36,13 @@ pub struct User {
oidc_provider: Option<String>,
oidc_subject: Option<String>,
image: Option<String>,
/// 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> {
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<String>,
oidc_subject: Option<String>,
image: Option<String>,
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<String>) {
self.image = image;
self.updated_at = Utc::now();
@@ -87,10 +87,10 @@ impl UserRepository for UserPgRepository {
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"),
))
}