chore(frontend): remove the legacy vanilla-JS frontend and its tooling
The SvelteKit app under /frontend has fully superseded the legacy
vanilla-JS/CSS frontend in /static, which was only ever served by a
debug `cargo run` / `PROFILE=dev` and never shipped to production.
Remove it together with the whole subsystem that existed only to
support it (~54k lines).
Frontend & assets:
- Delete /static (js/, css/, *.html, sw.js, basemaps/, locales symlink).
- Relocate the brand/PWA assets (logo/, favicon.ico, manifest.webmanifest)
to frontend/static/ so they ship with the SPA. This also fixes the
favicon, which app.html referenced but was missing from the prod bundle.
- Migrate the Nextcloud login-flow redirects from /nextcloud-error.html
to the SvelteKit /nextcloud/error route.
Web layer:
- Simplify resolve_static_path: drop the PROFILE=dev branch; always prefer
the Vite static-dist/ build, fall back to the configured path.
- Resolve i18n locales from the served SPA dir with a frontend/static
fallback so `just dev` works without a prior build.
Build:
- Prune build.rs from 1262 to ~70 lines (git metadata only); the Rust asset
pipeline and the OXICLOUD_RUST_ASSETS rollback flag are gone.
- Drop the now-unused build-dependencies (oxc_*, lightningcss).
- Remove the COPY static lines from the Dockerfile (cacher + builder).
Tooling & docs:
- Delete biome.json, jsconfig.json, tools/check-*.py, identifier.sh.
- Remove the legacy front-* justfile recipes; repoint the design-system
scripts (locales, dead-tokens, brand-drift, token-docs) at the frontend,
and drop check-contrast/check-headings (coupled to the old token
taxonomy / multi-page HTML).
- Repoint docs/DESIGN-SYSTEM.md links; remove 5 superseded docs/plan/*.
Backend dead code:
- Remove the dead `folder_repo` field from FileBlobWriteRepository.
- Remove the deprecated GET /api/folders/{id}/listing endpoint
(superseded by /resources).
Verified: cargo clippy (all-features/all-targets) clean, cargo test
--workspace 448 passed, cargo fmt clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,315 +0,0 @@
|
||||
# Plan — Magic-link external authentication
|
||||
|
||||
## Context
|
||||
|
||||
The UserLifecycleHook plan (PRs 1-5) shipped: `is_external` flag, `User::new_external`, lifecycle dispatcher with five hooks, `ExternalIdentityLifecycleHook` registered as a no-op stub awaiting this work. The DB CHECK `users_external_no_storage` and `users_external_not_admin` are in place. The `auth.users` table can already hold external recipients; nothing addresses them yet.
|
||||
|
||||
This plan implements the recipient-side flow: an internal user shares a resource by email; the server resolves the email to an existing user OR creates an external user on the fly; an invitation email is sent; the recipient clicks the magic link and lands on the resource (deep link) or on `/shared-with-me` (generic email login). External users have no password and authenticate exclusively via magic link until they later set a credential (password / OIDC / future webauthn), at which point magic-link silently becomes unavailable for that account.
|
||||
|
||||
The end state: OxiCloud can share with people who don't have accounts yet, with the same authz semantics as any other grant; the sharer cannot enumerate who already has an account (uniform API response shape); admin holds a kill switch (`OXICLOUD_ALLOW_EXTERNAL_USERS=false`).
|
||||
|
||||
## Design decisions (locked in)
|
||||
|
||||
### Security model — "Option A, nuanced"
|
||||
|
||||
A user is **magic-link-eligible** iff they have no other authentication method configured. Encapsulated in:
|
||||
|
||||
```rust
|
||||
impl User {
|
||||
pub fn has_login_credential(&self) -> bool {
|
||||
self.password_hash != "__EXTERNAL_NO_PASSWORD__"
|
||||
&& self.password_hash != "__OIDC_NO_PASSWORD__"
|
||||
|| self.oidc_subject.is_some()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The placeholder-string approach is a known smell — a proper `auth.user_auth_methods` side-table is the future evolution path, listed in "Future work" below. Today every magic-link-eligibility check goes through `has_login_credential()`, so the migration to the side-table only touches that method's body.
|
||||
|
||||
State graph (verified by `has_login_credential()`):
|
||||
|
||||
| State | password_hash | oidc_subject | Magic-link eligible |
|
||||
|---|---|---|---|
|
||||
| External, freshly invited | `__EXTERNAL_NO_PASSWORD__` | NULL | yes |
|
||||
| External who set password | real argon2 hash | NULL | no |
|
||||
| External who linked OIDC | `__OIDC_NO_PASSWORD__` | set | no |
|
||||
| Internal, password | real argon2 hash | NULL | no |
|
||||
| Internal, OIDC-only | `__OIDC_NO_PASSWORD__` | set | no |
|
||||
|
||||
Internal users who receive a "Bob shared FILE with you" mail get a notification-only link that deep-links to the OxiCloud login page with a return URL — no auto-auth, no mailbox-as-2FA-bypass.
|
||||
|
||||
### Identity: username = email for external users
|
||||
|
||||
- External users get `username = normalized_email`.
|
||||
- `auth.users.username` length cap widened from 32 to 254 (RFC 5321 maximum).
|
||||
- Login form accepts username OR email; lookup tries `username` first, falls back to `email`.
|
||||
- Username becomes mutable (post-create), via a new endpoint. The home folder name (`"My Folder - alice"`) is **not** renamed when username changes — it was display text at creation; semantically the folder is owned by `user_id`.
|
||||
- New columns `auth.users.given_name` and `auth.users.family_name`, both `TEXT NULL` — populated from OIDC standard claims at JIT provisioning; external users get NULL initially; users can set them later via a profile-edit endpoint.
|
||||
|
||||
### Email normalization
|
||||
|
||||
```rust
|
||||
fn normalize_email(input: &str) -> Result<String, ValidationError> {
|
||||
let trimmed = input.trim();
|
||||
let (local, domain) = trimmed.rsplit_once('@').ok_or(Malformed)?;
|
||||
let local_lower = local.to_lowercase();
|
||||
let domain_ascii = idna::domain_to_ascii(&domain.to_lowercase())
|
||||
.map_err(|_| InvalidDomain)?;
|
||||
Ok(format!("{}@{}", local_lower, domain_ascii))
|
||||
}
|
||||
```
|
||||
|
||||
Stored form is always ASCII (punycode for IDN domains). UI can reverse for display via `idna::domain_to_unicode`. Local-part case-folding to lower; Gmail `+tag` and `.` insensitivity are NOT special-cased (treat strings as opaque post-normalization).
|
||||
|
||||
### Internal virtual group finally narrowed
|
||||
|
||||
`pg_acl_engine.rs::expand_user` today inserts `INTERNAL_GROUP_ID` unconditionally with a TODO: *"Once the external-users work lands this will narrow to `if !user.is_external { ... }`."* Now's the time. External users do NOT belong to the Internal virtual group. The group's name finally honours its semantics.
|
||||
|
||||
### Magic-link tokens — mirror `auth.device_codes`
|
||||
|
||||
The closest existing pattern is `auth.device_codes` (entity at `src/domain/entities/device_code.rs`, repo at `src/infrastructure/repositories/pg/device_code_pg_repository.rs`). Status enum with PostgreSQL custom type, plain-text token, indexed on `expires_at WHERE pending`, `delete_expired()` cleanup helper. Copy verbatim.
|
||||
|
||||
New table:
|
||||
|
||||
```sql
|
||||
CREATE TYPE auth.magic_link_status AS ENUM ('pending', 'used', 'expired');
|
||||
|
||||
CREATE TABLE auth.magic_link_tokens (
|
||||
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||
token TEXT NOT NULL UNIQUE, -- 32 random bytes, base64url
|
||||
user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE,
|
||||
status auth.magic_link_status NOT NULL DEFAULT 'pending',
|
||||
issued_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||
expires_at TIMESTAMPTZ NOT NULL,
|
||||
used_at TIMESTAMPTZ,
|
||||
-- Optional deep-link target. NULL → generic "login via email" flow,
|
||||
-- lands on /shared-with-me. NOT NULL → invitation, lands directly.
|
||||
resource_type TEXT CHECK (resource_type IN ('file', 'folder')),
|
||||
resource_id UUID,
|
||||
CHECK ((resource_type IS NULL) = (resource_id IS NULL))
|
||||
);
|
||||
|
||||
CREATE INDEX ON auth.magic_link_tokens (expires_at) WHERE status = 'pending';
|
||||
CREATE INDEX ON auth.magic_link_tokens (user_id, status);
|
||||
```
|
||||
|
||||
Token lifetime: env-driven `OXICLOUD_MAGIC_LINK_TTL_HOURS` (default 24).
|
||||
|
||||
### Sharing flow extends `POST /api/grants`
|
||||
|
||||
New shape for the request body's `subject`:
|
||||
|
||||
```json
|
||||
{
|
||||
"subject": { "type": "email", "email": "Alice@Example.COM" },
|
||||
"resource": { "type": "folder", "id": "..." },
|
||||
"role": "viewer",
|
||||
"expires_at": "...",
|
||||
"notify": true,
|
||||
"message": "Hi Alice, here's the report."
|
||||
}
|
||||
```
|
||||
|
||||
Server flow (uniform response shape to defeat enumeration):
|
||||
|
||||
1. Validate `email` regex.
|
||||
2. Normalize (lowercase + punycode).
|
||||
3. Look up by normalized email (case-insensitive query against `auth.users.email`).
|
||||
4. If found: use existing `user_id`. If not: respect `OXICLOUD_ALLOW_EXTERNAL_USERS`. If false: 403. Else: `User::new_external(email_as_username, email)` and `dispatch_created`.
|
||||
5. Build grant with `subject = User(uuid)`.
|
||||
6. If `notify` (required `true` in v1): issue magic-link token targeting the resource, send email via `EmailSender` port.
|
||||
7. Return standard `GrantDto` with the resolved `user_id`.
|
||||
|
||||
Latency is the enumeration risk: existing user is a single SELECT; new user is SELECT + INSERT + INSERT + SMTP. The SMTP send goes through `tokio::spawn` (fire-and-forget) so the API response timing doesn't differ meaningfully between the two paths. Server logs the SMTP failure (if any) but the response stays uniform.
|
||||
|
||||
`notify = false` is rejected with 400 in v1 (no way for the recipient to access otherwise). Reserved for future "I'll send the URL myself via Slack" flow.
|
||||
|
||||
### Landing UX
|
||||
|
||||
```
|
||||
Magic link in invitation mail (resource_type/id NOT NULL)
|
||||
↓
|
||||
/magic/v1/{token}
|
||||
↓ (validate, mark used, emit session)
|
||||
↓
|
||||
Redirect to /folders/{id} or /files/{id} — direct to the resource
|
||||
```
|
||||
|
||||
```
|
||||
"Login via email" form on /login (user types their email)
|
||||
↓
|
||||
POST /api/auth/magic-link/send (uniform response)
|
||||
↓ (if user has no credential, issue token with NULL resource, send mail)
|
||||
↓
|
||||
User clicks /magic/v1/{token}
|
||||
↓ (validate, mark used, emit session)
|
||||
↓
|
||||
Redirect to /shared-with-me — their home for incoming grants
|
||||
```
|
||||
|
||||
Same redemption endpoint, different landing logic keyed on whether the token has a resource target.
|
||||
|
||||
### Configuration
|
||||
|
||||
```
|
||||
OXICLOUD_SMTP_HOST=smtp.example.com
|
||||
OXICLOUD_SMTP_PORT=587
|
||||
OXICLOUD_SMTP_USER=oxicloud@example.com
|
||||
OXICLOUD_SMTP_PASS=...
|
||||
OXICLOUD_SMTP_FROM="OxiCloud <noreply@example.com>"
|
||||
OXICLOUD_SMTP_TLS=starttls # starttls | tls | none
|
||||
OXICLOUD_MAGIC_LINK_TTL_HOURS=24
|
||||
OXICLOUD_ALLOW_EXTERNAL_USERS=true # set false to disable the whole feature
|
||||
OXICLOUD_PUBLIC_URL=https://oxicloud.example.com # for building link URLs
|
||||
```
|
||||
|
||||
`EmailSender` is `Option<Arc<dyn EmailSender>>` in DI — `None` when SMTP isn't configured. Endpoints that require email return 503 in that state with a clear "SMTP not configured" message.
|
||||
|
||||
### Rate limits
|
||||
|
||||
Reusing the existing `RateLimiter` at `src/interfaces/middleware/rate_limit.rs` (moka cache + counter, sliding window). Two new limiters:
|
||||
|
||||
- **Per-sharer email invitation**: 50 / hour, keyed by `caller_id`. Defends against an admin or compromised account spamming invites.
|
||||
- **Per-target-email resend**: 5 / hour, keyed by the normalized email being resent to. Defends against the resend endpoint being used as an email-bombing primitive.
|
||||
|
||||
### Defense in depth — boundary protections for external users
|
||||
|
||||
External users are a new principal kind. Several existing surfaces implicitly assume "all users are internal employees of this instance" and would leak / over-share once externals show up. **PR 6 closes all of these gaps** (alongside the schema groundwork) so subsequent PRs in this sequence don't accidentally surface external users where they don't belong.
|
||||
|
||||
**Already protected (by PR 2 of the lifecycle work — verified)**:
|
||||
|
||||
- DB CHECK `users_external_not_admin`: an external user cannot hold admin role. Three-layer enforcement (DB + entity factory + handler).
|
||||
- DB CHECK `users_external_no_storage`: an external user's `storage_used_bytes` must always be 0.
|
||||
- `HomeFolderLifecycleHook::provision_if_needed` short-circuits on `user.is_external()` — no home folder for externals.
|
||||
- `INTERNAL_GROUP_ID` is immutable (membership is implicit, additions/removals rejected as `VirtualImmutable` at the service layer).
|
||||
|
||||
**Already-existing gaps this work must close (PR 6)**:
|
||||
|
||||
1. **Subject groups admit external users today.** `subject_group_service.rs::add_member` (line 238) protects the `Internal` virtual group but does **not** reject `GroupMember::User(uuid)` where the candidate has `is_external = TRUE`. Concrete attack: admin adds `alice@example.com` (external) to the "Engineering" group; "Engineering" later gets a grant on internal-only resources; alice silently gains access. **Fix**: in `add_member`, after the `INTERNAL_GROUP_ID` guard, fetch the candidate user and reject with `DomainError::AccessDenied` if `user.is_external()` is true. Error message: "External users cannot be members of subject groups; share resources with them directly." Mirrors the no-external-admins enforcement style.
|
||||
|
||||
2. **System-contacts endpoint surfaces every user.** `contacts_handler::list_contacts(book_id=SYSTEM_BOOK_ID)` (line 447) calls `auth_service.list_users` which returns all users including externals. The share modal autocomplete (via `addressBook.searchContacts(q, [SYSTEM_BOOK_ID])`) would then suggest external users as recipients — wrong UX, and also leaks external identities to other internal users. **Fix**: `auth_service.list_users` and `auth_service.search_users` accept an `include_external: bool` parameter, defaulting to `false`. SQL adds `WHERE is_external = FALSE` when the flag is off. Existing call sites pass `false`. A new admin-list-users endpoint can pass `true` if the admin UI ever needs to show externals (handled in a future PR; not in scope here).
|
||||
|
||||
3. **`expand_user` adds external users to `INTERNAL_GROUP_ID`.** The TODO in `pg_acl_engine.rs:141` (*"narrow to if !user.is_external"*). **Fix**: include the conditional. External users get an expansion of `{their_uid}` only, no implicit Internal membership. This protects every Internal-group grant from inadvertent leakage to externals.
|
||||
|
||||
**Considered and intentionally deferred to a future hardening PR** (documented in "Out of scope"):
|
||||
|
||||
- **External users with `Permission::Share` resharing to create more externals.** Today nothing stops an external `Share`-grantee from invoking the email-grant flow and minting new external users. Policy question: should we forbid externals from being a `granted_by` value? Possible env flag: `OXICLOUD_EXTERNAL_USERS_CAN_RESHARE=false`. Not in this work.
|
||||
- **Shorter session/refresh-token TTL for external users.** Today refresh-token expiry is global. The plan keeps it that way for v1; future env `OXICLOUD_EXTERNAL_REFRESH_TOKEN_EXPIRY_DAYS` for differentiated lifetimes.
|
||||
- **`session_kind` tagging on sessions emitted from magic-link.** Could enable scoped sessions later (Option B from the security discussion). Not in v1.
|
||||
|
||||
**Magic-link-specific protections built into PR 8/9**:
|
||||
|
||||
- Tokens are 32-byte random base64url, generated via the OS CSPRNG (same pattern as `device_codes`).
|
||||
- Single-use via `status = 'used'` + `used_at` stamp; second redemption attempt rejected with 400 ("link already used").
|
||||
- TTL-enforced at the redemption endpoint (`expires_at < NOW()` → 400 "link expired").
|
||||
- Redemption endpoint is `GET /magic/v1/{token}` (token in URL path, not query string, to keep it out of `Referer` headers). On successful redemption the server immediately 302s to the resource — the magic-link URL is replaced in the address bar before the user can navigate further.
|
||||
- Uniform response on `POST /api/auth/magic-link/send` (`"If we have an account, a link will be sent"`) regardless of whether the email exists. Per-target-email rate limit prevents using the endpoint as an enumeration oracle by latency.
|
||||
|
||||
## PR sequence
|
||||
|
||||
| PR | Subject | Why land separately |
|
||||
|---|---|---|
|
||||
| **6** | Prelude: `has_login_credential()`, narrow `INTERNAL_GROUP_ID`, widen username, add `given_name`/`family_name`, make username mutable, **+ defense-in-depth: subject groups reject external members, `list_users`/`search_users` filter externals, `expand_user` excludes externals from Internal** | Entity + schema groundwork **plus the three boundary protections enumerated in "Defense in depth"**. Verifiable in isolation by running the existing Hurl suite — no new behaviour for internal users, just protections that activate once externals exist. |
|
||||
| **7** | SMTP infrastructure: `EmailSender` port + `lettre`-backed impl + env config | Pure infrastructure. Mocked in tests. No user-visible feature yet. |
|
||||
| **8** | `auth.magic_link_tokens` table + repo + redemption endpoint `/magic/v1/{token}` + `ExternalIdentityLifecycleHook` populated | The magic-link plumbing. Tokens can be manually fabricated for unit tests; sharer flow still pending. |
|
||||
| **9** | Extend `POST /api/grants` for `subject.type = "email"` + email normalization + lazy external-user creation + invitation email + Hurl coverage of the invite path | The sharer side, end-to-end. The Hurl test creates an unknown email, claims the resulting magic link, lands on the resource. |
|
||||
| **10** | Login-via-email endpoint (`POST /api/auth/magic-link/send`) with uniform response + landing on `/shared-with-me` for NULL-resource tokens | The recovery / no-password-yet path. Lands the existing user back into their incoming-grants view. |
|
||||
| **11** | Frontend: share-modal accepts arbitrary email + login page "Login with email link" section | UI changes alone. Pure frontend PR for clean review. |
|
||||
| **12** | Rate limits + comprehensive Hurl coverage + architecture doc + sidebar | Hardening + acceptance gate. `docs/architecture/magic-link-auth.md` + sidebar entry. Updated `share-integration.md`. |
|
||||
|
||||
## Critical files
|
||||
|
||||
**New files**:
|
||||
|
||||
- PR 6: `migrations/20260612000003_users_username_email_login.sql` (widen username, add given_name/family_name, mutable username)
|
||||
- PR 7: `Cargo.toml` (+lettre), `src/application/ports/email_sender.rs`, `src/infrastructure/services/smtp_email_sender.rs`
|
||||
- PR 8: `migrations/20260612000004_magic_link_tokens.sql`, `src/domain/entities/magic_link_token.rs`, `src/infrastructure/repositories/pg/magic_link_token_pg_repository.rs`, `src/interfaces/api/handlers/magic_link_handler.rs`
|
||||
- PR 9: `src/domain/services/email_normalize.rs` (small utility), invitation email template inline in `external_identity_service.rs`
|
||||
- PR 12: `docs/architecture/magic-link-auth.md`
|
||||
|
||||
**Modified files**:
|
||||
|
||||
- PR 6: `src/domain/entities/user.rs` (`has_login_credential`, username mutability getter/setter), `src/infrastructure/services/pg_acl_engine.rs` (drop the unconditional `INTERNAL_GROUP_ID` insert when `user.is_external()` — closes protection gap #3), `src/application/services/auth_application_service.rs` (login lookup tries email fallback; `list_users` / `search_users` gain `include_external: bool` defaulting to false — closes protection gap #2), `src/application/services/subject_group_service.rs` (`add_member` rejects external user members — closes protection gap #1), `src/application/dtos/user_dto.rs` (given_name/family_name fields), `src/infrastructure/repositories/pg/user_pg_repository.rs` (`list_users` / `search_users` SQL gains `WHERE is_external = FALSE` when filter is on)
|
||||
- PR 7: `src/common/di.rs` (wire `EmailSender`), `src/common/config.rs` (parse SMTP env vars)
|
||||
- PR 8: `src/application/services/external_identity_service.rs` (populate the PR-5 stub), `src/common/di.rs` (wire magic_link_repo into external_identity hook)
|
||||
- PR 9: `src/interfaces/api/handlers/grant_handler.rs` (extend `POST /api/grants` request parsing), `src/application/dtos/grant_dto.rs` (new SubjectTypeDto variant; or accept email-as-string in existing SubjectDto), `src/interfaces/api/routes.rs`
|
||||
- PR 10: `src/interfaces/api/routes.rs` (register `/api/auth/magic-link/send`), `src/application/services/auth_application_service.rs` (login-via-email use case)
|
||||
- PR 11: `static/js/components/shareModal.js` (free-text email input), `static/login.html` (new section), `static/js/features/auth/auth.js` (POST flow + success UI), i18n keys in 16 locales
|
||||
- PR 12: `src/interfaces/middleware/rate_limit.rs` (two new limiter constructors), `tests/api/magic_link.hurl` (new test file), `docs/.vitepress/config.mts` (sidebar entry), `docs/architecture/share-integration.md` (cross-reference)
|
||||
|
||||
## Existing patterns to reuse (with paths)
|
||||
|
||||
- **Rate limiter**: `src/interfaces/middleware/rate_limit.rs` — `RateLimiter::new(max_requests, window_secs, max_entries)` + `check_and_increment(&key)`. Two new factory functions (`rate_limit_email_invite`, `rate_limit_magic_link_send`).
|
||||
- **Token storage pattern**: `src/domain/entities/device_code.rs` + `src/infrastructure/repositories/pg/device_code_pg_repository.rs`. Status enum (pending/used/expired) with PostgreSQL custom type; `delete_expired()` cleanup helper.
|
||||
- **Lifecycle hook**: `ExternalIdentityLifecycleHook` already registered in DI (PR 5). Body filled in here.
|
||||
- **Audit pattern**: `tracing::info!(target: "audit", event = "...")` — same convention as `subject_group_service.rs` and `user_lifecycle_service.rs`.
|
||||
- **Email-input UX in share modal**: Today autocomplete-only (lines 350-391 of `shareModal.js`). Add a third "external email" suggestion type alongside `ContactItem` and `GroupSuggestion` — uses the same staging/chip rendering machinery.
|
||||
- **Login page extensibility**: `static/login.html` lines 59-121 + `static/js/features/auth/auth.js::initLoginElements` lines 758-810. New section mirrors the OIDC button pattern.
|
||||
- **Idna for punycode**: add `idna` crate to Cargo.toml; standard Rust crate for IDN handling.
|
||||
|
||||
## Verification
|
||||
|
||||
Per-PR (all PRs):
|
||||
|
||||
```bash
|
||||
cargo fmt --all
|
||||
cargo clippy --all-features --all-targets -- -D warnings
|
||||
cargo test --workspace
|
||||
bash tests/api/run.sh # 13 existing Hurl files must still pass
|
||||
```
|
||||
|
||||
End-to-end gate after PR 9:
|
||||
|
||||
1. Login as admin in browser. Share a folder with `newly-invited@example.com`. Confirm:
|
||||
- HTTP 201 + grant_id returned.
|
||||
- `auth.users` has a new row, `is_external = TRUE`, `username = 'newly-invited@example.com'`, no password_hash (placeholder).
|
||||
- `auth.magic_link_tokens` has a new row pointing at that user and the folder.
|
||||
- SMTP relay receives one mail (use MailHog or `OXICLOUD_SMTP_HOST=localhost` + a netcat trap).
|
||||
2. Open the magic-link URL from the captured mail. Confirm:
|
||||
- Session cookie issued; redirected to the folder URL.
|
||||
- Token row's `status = 'used'`, `used_at` set.
|
||||
3. Reload the URL. Confirm 400 "link already used".
|
||||
4. Wait past TTL on a fresh token; confirm 400 "link expired" + "Resend" UI.
|
||||
|
||||
End-to-end gate after PR 10:
|
||||
|
||||
5. Log out. Go to `/login`. Click "Login with email link". Enter the same email. Confirm:
|
||||
- HTTP 200 with uniform "If we have an account, a link will be sent" body.
|
||||
- Fresh magic-link token in DB (no resource target this time).
|
||||
- Mail received. Click → land on `/shared-with-me`. Confirm the previously shared folder is in the list.
|
||||
|
||||
End-to-end gate after PR 12:
|
||||
|
||||
6. Issue 60 invitations from one admin in a minute → confirm 50 succeed and 10 are rate-limited with 429.
|
||||
7. POST `/api/auth/magic-link/send` 10× for the same email in 10 minutes → confirm 5 succeed and 5 are rate-limited with 429.
|
||||
8. Hurl suite `tests/api/magic_link.hurl` covers: invite-new-email, invite-existing-email (no duplicate user), token redemption, expired token, resend uniform response, rate-limit triggers.
|
||||
|
||||
## Out of scope (do NOT bundle)
|
||||
|
||||
- **Auth-method side-table refactor.** Acknowledged smell with the placeholder strings (`__EXTERNAL_NO_PASSWORD__` etc.). Future PR introduces `auth.user_auth_methods` with rows per `(user_id, method_type, credentials)`. The `has_login_credential()` method is the single migration point; refactor changes its body without rippling.
|
||||
- **Email template engine + i18n localization of emails.** v1 ships English-only hardcoded templates. Template engine (handlebars / askama) + recipient-locale detection is a future PR.
|
||||
- **MX-record validation at share time.** Regex only; bad domains discover themselves via SMTP bounce.
|
||||
- **Periodic cleanup of dormant external users.** A sweeper that purges users with no `last_login_at` for 13+ months. Future PR; the GDPR-sweeper variant `DeletionMode::GdprPurge` (already in the trait) is its hook entry point.
|
||||
- **Per-instance allowlist of external email domains** (e.g. only `*@my-company.com`). Future env var `OXICLOUD_EXTERNAL_EMAIL_DOMAINS`. Kill switch (`OXICLOUD_ALLOW_EXTERNAL_USERS=false`) ships in PR 6 as a coarser tool.
|
||||
- **WebAuthn / passkey enrolment for external users after first login.** Distinct future feature; the magic-link bootstrap is the prerequisite.
|
||||
- **`OXICLOUD_EXTERNAL_USERS_CAN_RESHARE=false`** env flag forbidding externals from being a grant's `granted_by`. Today an external user with `Permission::Share` can mint more external users via the email-grant flow. Soft policy; deferred.
|
||||
- **Differentiated session lifetime for externals** (`OXICLOUD_EXTERNAL_REFRESH_TOKEN_EXPIRY_DAYS`). Today refresh-token TTL is uniform across all users. Deferred until operational data shows it matters.
|
||||
- **`session_kind` discriminator on sessions emitted from magic-link.** Enables Option B-style scoped sessions (magic-link sessions only access granted resources, not the user's own home folder). Today every authenticated session is full-tier; magic-link only happens for users without home folders (externals), so the practical exposure is small. Deferred.
|
||||
- **Admin-list-users surface that includes externals.** PR 6 makes `list_users` filter externals by default. The admin endpoint at `GET /api/admin/users` will eventually want an `include_external` query param so admins can manage externals (rename, deactivate, see grants). Not in scope of this work — the admin UI for externals is its own future PR.
|
||||
- **Open Cloud Mesh (OCM) federation.** External users via OCM (federated partner servers) are a separate path; magic-link is one of several future external-identity providers. `ExternalIdentityLifecycleHook::on_user_created` design accommodates the `source` discriminator (`magic_link` / `oidc` / `ocm`).
|
||||
|
||||
## Recommended future event triggers (DON'T ship in this work)
|
||||
|
||||
Same convention as the lifecycle plan: a future event ships only when there's a concrete consumer.
|
||||
|
||||
| Future event | What would force it |
|
||||
|---|---|
|
||||
| `on_external_user_credential_set` | When an external user sets a password OR links OIDC — useful for an audit event ("alice@example.com is no longer magic-link-eligible") and for invalidating any outstanding magic-link tokens she has. Today the new tokens are simply unused; reaping them via this event would be cleaner. |
|
||||
| `on_magic_link_resent` | If audit consumers want to see resend traffic distinguishable from invite traffic. Today the resend goes through the same code path as the initial issuance; an audit-distinguishable event isn't worth the trait surface yet. |
|
||||
| `on_email_bounce` | When SMTP delivery fails permanently. Useful for surfacing "this user's email is dead" in admin UI. Requires a bounce-tracking infrastructure (SES-style webhook, custom bounce-mailbox monitoring) — out of scope. |
|
||||
|
||||
These are doc-only; their absence doesn't block anything.
|
||||
|
||||
## Two open questions I want to confirm via AskUserQuestion
|
||||
|
||||
None at this point — the conversation pinned every design decision. Proceeding straight to ExitPlanMode.
|
||||
@@ -1,213 +0,0 @@
|
||||
# Plan: Unified lifecycle hooks (file + blob)
|
||||
|
||||
## Context
|
||||
|
||||
The file and blob lifecycle hook systems are partially built but inconsistently wired:
|
||||
- `FileLifecycleService` only fans out `on_file_deleted`; created/updated hooks are wired directly on `FileUploadService`.
|
||||
- `AudioMetadataService` implements no hook traits — called raw from 4 handler files.
|
||||
- `ThumbnailRefreshHook` (file created/updated) and `ThumbnailService` (file deleted, blob deleted) are separate registrations for the same concern.
|
||||
- `copy_file()` fires no hooks — copied files never get audio metadata (confirmed gap: `audio.file_metadata` is keyed by `file_id`, not `blob_hash`).
|
||||
- Blob lifecycle has the same structural problem: two separate traits and two separate vecs in `DedupService`.
|
||||
|
||||
Goal: one `FileLifecycleHook` + one `BlobLifecycleHook` trait, each with a composite dispatcher, all side-effects wired through them, handlers reduced to protocol translators.
|
||||
|
||||
---
|
||||
|
||||
## Design decisions
|
||||
|
||||
### Synchronous trait methods
|
||||
|
||||
Hooks are fire-and-notify: every implementation either spawns a `tokio::spawn` internally or does nothing. Sync trait = no `Box::pin`, no `async_trait`, genuine one-liner noops.
|
||||
|
||||
```rust
|
||||
// application/ports/file_lifecycle.rs
|
||||
pub trait FileLifecycleHook: Send + Sync {
|
||||
fn on_file_created(&self, file_id: &str, blob_hash: &str, content_type: &str, is_new_blob: bool);
|
||||
fn on_file_updated(&self, file_id: &str, blob_hash: &str, content_type: &str);
|
||||
fn on_file_copied(&self, file_id: &str, blob_hash: &str, content_type: &str, source_id: &str)
|
||||
// not information if the blob still exists, up to implementor to use BlobLifecycleHook if needed
|
||||
fn on_file_deleted(&self, file_id: &str);
|
||||
}
|
||||
|
||||
// application/ports/blob_lifecycle.rs
|
||||
pub trait BlobLifecycleHook: Send + Sync {
|
||||
fn on_blob_created(&self, blob_hash: &str, content_type: Option<&str>);
|
||||
fn on_blob_deleted(&self, blob_hash: &str);
|
||||
}
|
||||
```
|
||||
|
||||
**No default methods** — explicit noops required (forces developer acknowledgement of all events).
|
||||
|
||||
### `is_new_blob: bool` on `on_file_created`
|
||||
|
||||
Tells the implementor whether the underlying blob is genuinely new (fresh upload, no dedup hit) or already existed (copy, dedup hit on re-upload). This prevents implementors from re-scanning/re-generating work that can be shared or cloned from an existing record:
|
||||
|
||||
- `ThumbnailRefreshHook`: if `!is_new_blob`, the `blob_hash` thumbnail already exists on disk — skip scheduling generation entirely.
|
||||
- `AudioMetadataService`: if `!is_new_blob`, clone the existing metadata row for the `blob_hash` (fast DB copy) instead of re-parsing the blob.
|
||||
|
||||
**Where `is_new_blob` comes from**: `FileUploadService` gets the dedup result from `FileBlobWriteRepository.save_file_from_temp()` (already computed during upload). For `copy_file()`, always `false` — same blob by definition.
|
||||
|
||||
### Old traits removed entirely
|
||||
|
||||
Six old traits (`FileCreatedHook`, `FileUpdatedHook`, `FileDeletedHook`, `BlobCreationHook`, `BlobDeletionHook`) removed. All implementors migrate to the two new traits.
|
||||
|
||||
### Why `on_file_deleted` can be sync
|
||||
|
||||
`ThumbnailService.delete_thumbnails` is currently awaited by the caller. It moves to `tokio::spawn` internally — thumbnail cleanup is best-effort, callers don't depend on it completing.
|
||||
|
||||
---
|
||||
|
||||
## Thumbnail storage model (context)
|
||||
|
||||
- **Disk**: keyed by `blob_hash` → `thumbnails_root/{size}/{blob_hash}.jpg` — shared between all files with the same content.
|
||||
- **Moka cache**: keyed by `(file_id, size)` — cold-misses on first request for a new `file_id`, then reads from disk.
|
||||
- **External thumbnails** (video frames): keyed by `file_id` → `ext-{file_id}.jpg`.
|
||||
|
||||
Image copy is safe: disk thumbnail exists for the `blob_hash`, no regeneration needed (`is_new_blob = false` will skip it).
|
||||
|
||||
---
|
||||
|
||||
## Files to change
|
||||
|
||||
### 1. `src/application/ports/file_lifecycle.rs`
|
||||
Replace three separate async traits with one sync `FileLifecycleHook` trait (3 methods + `is_new_blob` on created, no defaults).
|
||||
|
||||
### 2. `src/application/ports/blob_lifecycle.rs`
|
||||
Replace two separate async traits with one sync `BlobLifecycleHook` trait (2 methods, no defaults).
|
||||
|
||||
### 3. `src/application/services/file_lifecycle_service.rs`
|
||||
- One `Vec<Arc<dyn FileLifecycleHook>>`.
|
||||
- One builder: `.with_hook(hook)`.
|
||||
- `impl FileLifecycleHook`: plain `for` loops, no async, forwards `is_new_blob`.
|
||||
|
||||
### 4. New: `src/application/services/blob_lifecycle_service.rs`
|
||||
Mirror of `FileLifecycleService` for blob events:
|
||||
- `Vec<Arc<dyn BlobLifecycleHook>>`, `.with_hook()` builder, `impl BlobLifecycleHook` fan-out.
|
||||
|
||||
### 5. `src/infrastructure/services/thumbnail_service.rs`
|
||||
Consolidate all thumbnail hook logic into `ThumbnailRefreshHook`, implementing **both** new traits:
|
||||
|
||||
**`impl FileLifecycleHook for ThumbnailRefreshHook`**:
|
||||
- `on_file_created`: if `!is_new_blob` or unsupported content type → return early (blob thumbnail already on disk). Otherwise spawn generation.
|
||||
- `on_file_updated`: spawn thumbnail invalidation + regeneration (existing logic).
|
||||
- `on_file_deleted`: `tokio::spawn({ thumbnail.delete_thumbnails(file_id).await })`.
|
||||
|
||||
**`impl BlobLifecycleHook for ThumbnailRefreshHook`**:
|
||||
- `on_blob_created`: explicit noop — thumbnail gen is handled at file level via `on_file_created`.
|
||||
- `on_blob_deleted`: `tokio::spawn({ thumbnail.delete_blob_thumbnails(blob_hash).await })`.
|
||||
|
||||
Remove: `impl FileDeletedHook for ThumbnailService`, `impl BlobDeletionHook for ThumbnailService`.
|
||||
|
||||
### 6. `src/infrastructure/services/audio_metadata_service.rs`
|
||||
|
||||
**New method**: `clone_or_extract_background(service: Arc<Self>, new_file_id: Uuid, blob_hash: String)`
|
||||
- Spawns a task that runs:
|
||||
```sql
|
||||
INSERT INTO audio.file_metadata (file_id, title, artist, album, album_artist,
|
||||
genre, track_number, disc_number, year, duration_secs, format)
|
||||
SELECT $new_file_id, title, artist, album, album_artist, genre,
|
||||
track_number, disc_number, year, duration_secs, format
|
||||
FROM audio.file_metadata am
|
||||
JOIN storage.files sf ON sf.id = am.file_id
|
||||
WHERE sf.blob_hash = $blob_hash
|
||||
LIMIT 1
|
||||
ON CONFLICT (file_id) DO NOTHING
|
||||
```
|
||||
- If 0 rows inserted (original not yet processed), falls back to `extract_and_save`.
|
||||
|
||||
**`impl FileLifecycleHook for AudioMetadataService`**:
|
||||
- `on_file_created`: if `is_audio_file(content_type)` → parse UUID, then:
|
||||
- `is_new_blob = true` → `spawn_extraction_background(file_id, blob_path(blob_hash))`
|
||||
- `is_new_blob = false` → `clone_or_extract_background(file_id, blob_hash)`
|
||||
- `on_file_updated`: if audio → `spawn_extraction_with_delete_background`.
|
||||
- `on_file_deleted`: explicit one-liner noop + comment: `audio.file_metadata` has `ON DELETE CASCADE`, DB handles cleanup.
|
||||
|
||||
### 7. `src/application/services/file_upload_service.rs`
|
||||
- Replace `file_created_hooks: Vec<Arc<dyn FileCreatedHook>>` + `file_updated_hooks` with `file_lifecycle_hook: Option<Arc<dyn FileLifecycleHook>>`.
|
||||
- Builder: `.with_file_lifecycle_hook(hook)`.
|
||||
- Sync calls replacing async fan-out loops. Pass `is_new_blob` from the dedup result already available at this layer.
|
||||
|
||||
### 8. `src/application/services/file_management_service.rs`
|
||||
- Replace `Arc<dyn FileDeletedHook>` with `Arc<dyn FileLifecycleHook>`.
|
||||
- `on_file_deleted` call becomes sync.
|
||||
- **Fix copy gap**: after `file_repository.copy_file()` returns the new file DTO, call `self.file_lifecycle.on_file_created(new_id, blob_hash, mime_type, false)`.
|
||||
|
||||
### 9. `src/application/services/trash_service.rs`
|
||||
- Replace `Arc<dyn FileDeletedHook>` with `Arc<dyn FileLifecycleHook>`.
|
||||
- `on_file_deleted` calls become sync.
|
||||
|
||||
### 10. `src/infrastructure/services/dedup_service.rs`
|
||||
- Replace `blob_creation_hooks: Vec<Arc<dyn BlobCreationHook>>` + `blob_hooks: Vec<Arc<dyn BlobDeletionHook>>` with `blob_lifecycle: Option<Arc<BlobLifecycleService>>`.
|
||||
- Builder: `.with_blob_lifecycle(hook)`.
|
||||
- Sync calls replacing async fan-outs.
|
||||
|
||||
### 11. `src/common/di.rs`
|
||||
|
||||
```rust
|
||||
let thumbnail_hook = Arc::new(ThumbnailRefreshHook::new(
|
||||
core.thumbnail_service.clone(),
|
||||
dedup.clone(),
|
||||
));
|
||||
|
||||
let file_lifecycle = Arc::new(
|
||||
FileLifecycleService::new()
|
||||
.with_hook(thumbnail_hook.clone())
|
||||
.with_hook(audio_metadata_service.clone()) // if Some
|
||||
);
|
||||
|
||||
let blob_lifecycle = Arc::new(
|
||||
BlobLifecycleService::new()
|
||||
.with_hook(thumbnail_hook.clone())
|
||||
);
|
||||
|
||||
dedup_service.with_blob_lifecycle(blob_lifecycle)
|
||||
file_upload_service.with_file_lifecycle_hook(file_lifecycle.clone())
|
||||
file_management_service.with_file_lifecycle_hook(file_lifecycle.clone())
|
||||
trash_service.with_file_lifecycle_hook(file_lifecycle.clone())
|
||||
```
|
||||
|
||||
### 12. Handler cleanup — 4 files (deletes only)
|
||||
|
||||
| File | Remove |
|
||||
|---|---|
|
||||
| `src/interfaces/api/handlers/file_handler.rs` | direct `thumbnail_service.generate_all_sizes_background_from_bytes(...)` + `AudioMetadataService::spawn_extraction_background(...)` |
|
||||
| `src/interfaces/nextcloud/webdav_handler.rs` | `AudioMetadataService::spawn_extraction_background(...)` (create) + `AudioMetadataService::spawn_extraction_with_delete_background(...)` (update) |
|
||||
| `src/interfaces/nextcloud/uploads_handler.rs` | `AudioMetadataService::spawn_extraction_background(...)` |
|
||||
| `src/interfaces/api/handlers/webdav_handler.rs` | `AudioMetadataService::spawn_extraction_background(...)` |
|
||||
|
||||
---
|
||||
|
||||
## Execution order
|
||||
|
||||
1. `file_lifecycle.rs` — new trait
|
||||
2. `blob_lifecycle.rs` — new trait
|
||||
3. `file_lifecycle_service.rs` — updated composite
|
||||
4. New `blob_lifecycle_service.rs`
|
||||
5. `thumbnail_service.rs` — merged impl of both traits
|
||||
6. `audio_metadata_service.rs` — new method + `FileLifecycleHook` impl
|
||||
7. `file_upload_service.rs` — unified hook field + `is_new_blob` plumbing
|
||||
8. `file_management_service.rs` — type update + copy hook
|
||||
9. `trash_service.rs` — type update
|
||||
10. `dedup_service.rs` — unified blob hook field
|
||||
11. `di.rs` — rewire
|
||||
12. Handler cleanups (4 files, independent)
|
||||
|
||||
---
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cargo fmt --all
|
||||
cargo clippy --all-features --all-targets -- -D warnings # zero warnings
|
||||
cargo test --workspace # all ~208 tests green
|
||||
```
|
||||
|
||||
Smoke-test manually:
|
||||
- Upload an image → thumbnail appears.
|
||||
- Upload same image again (dedup hit) → no thumbnail re-generation.
|
||||
- Upload an audio file via Nextcloud WebDAV → audio metadata present.
|
||||
- Copy an image file → copy has thumbnail served instantly from blob_hash path.
|
||||
- Copy a music file → copy has audio metadata (cloned row, no blob re-parse).
|
||||
- Delete a file → thumbnails cleared; audio metadata gone (DB cascade).
|
||||
- Overwrite file via WebDAV PUT → thumbnail refreshes.
|
||||
- Delete last copy of a blob → blob-hash thumbnail file removed from disk.
|
||||
@@ -1,671 +0,0 @@
|
||||
# Plan — ReBAC Subject Groups (v1)
|
||||
|
||||
## Context
|
||||
|
||||
OxiCloud's `storage.access_grants` table already declares `subject_type IN
|
||||
('user', 'group', 'token', 'external')` and `expires_at TIMESTAMPTZ`, but no
|
||||
schema, code, or UI exists today for the `'group'` subject. This plan
|
||||
implements that: a nested, root-owned group subject with cascading
|
||||
authorization, cycle protection, and a global namespace.
|
||||
|
||||
After this lands:
|
||||
- `Subject::Group(Id)` can be granted permissions on files/folders, with
|
||||
membership cascading through nested groups.
|
||||
- A `Subject::User(Id)` is reached via direct grant **OR** via membership in
|
||||
any group (transitively) that holds a grant.
|
||||
- One predefined immutable virtual group `Internal` represents *all internal
|
||||
users* (`is_external = false`), the way "Everyone in your org" works in
|
||||
Google Workspace.
|
||||
- Groups are admin-managed (creation, naming, membership).
|
||||
- Group names are RFC 5321 local-part compliant so the door to a future
|
||||
mailing-list / email-addressable feature stays open.
|
||||
- Performance: recursive CTE for transitive expansion, fronted by a 30s Moka
|
||||
cache keyed by user_id. Designed so a future closure-table migration is a
|
||||
swap-in behind one function.
|
||||
|
||||
**Decisions accepted earlier in the conversation (encoded in this plan):**
|
||||
- Max nesting depth: **8**.
|
||||
- Cycle detection: **at write time** (rejects mutations).
|
||||
- Cascade-delete grants when a group is deleted.
|
||||
- No `Everyone` virtual group; external users are only reached via explicit
|
||||
per-grant action.
|
||||
- `UseAsSubject` permission acknowledged as future work — v1 ships
|
||||
admin-only group management (anyone can target any group in a grant).
|
||||
- Audit events emit via `tracing::info!(target = "audit", ...)`; a syslog
|
||||
subscriber hook is documented but its concrete wiring is a follow-up.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope (v1)
|
||||
|
||||
1. New tables `auth.subject_groups` + `auth.subject_group_members`.
|
||||
2. Predefined `Internal` virtual group (well-known UUID, immutable).
|
||||
3. CRUD REST API at `/api/groups/...` (admin-only).
|
||||
4. Membership add/remove with cycle + depth checks.
|
||||
5. Transitive-expansion function in `AuthorizationEngine`, Moka-cached.
|
||||
6. Cascade queries in `pg_acl_engine.rs` updated to use `subject_id = ANY(...)`.
|
||||
7. Share-dialog autocomplete: extend to also return groups (via the new
|
||||
authenticated `/api/groups/search` endpoint).
|
||||
8. Audit logging via structured `tracing::info!(target = "audit", ...)`.
|
||||
9. Minimal i18n: API-returned error message keys only.
|
||||
|
||||
### Out of scope (v2 / later)
|
||||
|
||||
- **Admin UI for group management.** v1 is API-only — `POST /api/groups`,
|
||||
member add/remove, etc. are reachable via curl / Hurl until a dedicated
|
||||
admin tab is added in a follow-up. The autocomplete extension in the
|
||||
*share dialog* (file/folder sharing UX) is the only UI change in v1.
|
||||
- `Manage` and `UseAsSubject` permissions on groups themselves (delegated
|
||||
group admin requires adding `subject_group` to the `access_grants`
|
||||
resource_type CHECK and per-group authz).
|
||||
- Mailing-list dispatcher (the RFC-compliant naming preserves the door).
|
||||
- Concrete syslog appender wiring (env-var driven `tracing-syslog` or
|
||||
`tracing-journald` subscriber — code emits structured events today,
|
||||
operators choose a sink).
|
||||
- Closure table for transitive membership (Moka cache is enough; future
|
||||
swap behind `expand_subject()`).
|
||||
|
||||
## Schema migration
|
||||
|
||||
New file: `migrations/20260612000000_subject_groups.sql`.
|
||||
|
||||
```sql
|
||||
-- ── auth.subject_groups: root-owned authorization principals ─────────────
|
||||
CREATE TABLE IF NOT EXISTS auth.subject_groups (
|
||||
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||
name CITEXT NOT NULL,
|
||||
description TEXT,
|
||||
is_virtual BOOLEAN NOT NULL DEFAULT FALSE,
|
||||
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||
updated_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||
|
||||
-- RFC 5321 local-part: starts alnum, then alnum/dot/dash/underscore,
|
||||
-- max 64 chars. Future-proofs `group@instance` mailing-list addressing.
|
||||
CONSTRAINT subject_groups_name_rfc5321
|
||||
CHECK (name ~ '^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$'),
|
||||
CONSTRAINT subject_groups_name_uq UNIQUE (name)
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_subject_groups_is_virtual
|
||||
ON auth.subject_groups (is_virtual) WHERE is_virtual = TRUE;
|
||||
|
||||
-- ── auth.subject_group_members: edges (user→group or group→group) ────────
|
||||
CREATE TABLE IF NOT EXISTS auth.subject_group_members (
|
||||
group_id UUID NOT NULL REFERENCES auth.subject_groups(id) ON DELETE CASCADE,
|
||||
member_user_id UUID REFERENCES auth.users(id) ON DELETE CASCADE,
|
||||
member_group_id UUID REFERENCES auth.subject_groups(id) ON DELETE CASCADE,
|
||||
added_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||
added_by UUID NOT NULL REFERENCES auth.users(id),
|
||||
|
||||
-- Exactly one of the two member columns is set.
|
||||
CONSTRAINT subject_group_members_xor CHECK (
|
||||
(member_user_id IS NOT NULL)::int + (member_group_id IS NOT NULL)::int = 1
|
||||
),
|
||||
-- A group can't contain itself directly.
|
||||
CONSTRAINT subject_group_members_no_self CHECK (
|
||||
member_group_id IS NULL OR member_group_id <> group_id
|
||||
)
|
||||
);
|
||||
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_subject_group_members_user
|
||||
ON auth.subject_group_members (group_id, member_user_id)
|
||||
WHERE member_user_id IS NOT NULL;
|
||||
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_subject_group_members_group
|
||||
ON auth.subject_group_members (group_id, member_group_id)
|
||||
WHERE member_group_id IS NOT NULL;
|
||||
|
||||
-- For transitive expansion: "all groups a user belongs to directly"
|
||||
CREATE INDEX IF NOT EXISTS idx_subject_group_members_by_user
|
||||
ON auth.subject_group_members (member_user_id)
|
||||
WHERE member_user_id IS NOT NULL;
|
||||
|
||||
-- For cycle check: "what groups does group X contain (immediate children)"
|
||||
CREATE INDEX IF NOT EXISTS idx_subject_group_members_by_child_group
|
||||
ON auth.subject_group_members (member_group_id, group_id)
|
||||
WHERE member_group_id IS NOT NULL;
|
||||
|
||||
-- ── Seed the predefined `Internal` virtual group ─────────────────────────
|
||||
-- Well-known UUID hard-coded in Rust so application code can reference it
|
||||
-- without a runtime lookup: 00000000-0000-0000-0000-000000000001.
|
||||
INSERT INTO auth.subject_groups (id, name, description, is_virtual)
|
||||
VALUES (
|
||||
'00000000-0000-0000-0000-000000000001',
|
||||
'Internal',
|
||||
'All internal users (is_external = false). Membership is implicit; no rows in subject_group_members.',
|
||||
TRUE
|
||||
)
|
||||
ON CONFLICT (id) DO NOTHING;
|
||||
```
|
||||
|
||||
**Notes on the schema:**
|
||||
- `CITEXT` extension is already enabled elsewhere — confirm via
|
||||
`\dx` against a dev DB; if not, the migration must `CREATE EXTENSION IF NOT
|
||||
EXISTS citext;` first.
|
||||
- The two member columns (`member_user_id`, `member_group_id`) form a tagged
|
||||
union enforced by the XOR check. Simpler than polymorphic FKs.
|
||||
- The `is_virtual = TRUE` flag distinguishes the immutable `Internal` group;
|
||||
the service layer rejects member mutations on it.
|
||||
- No `created_by` column — groups have no owner by design. `added_by` on
|
||||
*memberships* still records who performed the edit, for audit.
|
||||
|
||||
## Domain layer
|
||||
|
||||
New file: `src/domain/entities/subject_group.rs`.
|
||||
|
||||
```rust
|
||||
pub struct SubjectGroup {
|
||||
pub id: Uuid,
|
||||
pub name: String,
|
||||
pub description: Option<String>,
|
||||
pub is_virtual: bool,
|
||||
pub created_at: DateTime<Utc>,
|
||||
pub updated_at: DateTime<Utc>,
|
||||
}
|
||||
|
||||
pub enum GroupMember {
|
||||
User(Uuid),
|
||||
Group(Uuid),
|
||||
}
|
||||
|
||||
impl SubjectGroup {
|
||||
pub fn new(name: &str, description: Option<String>) -> Result<Self, DomainError> {
|
||||
Self::validate_name(name)?;
|
||||
// ...
|
||||
}
|
||||
|
||||
/// Enforce RFC 5321 local-part shape at the domain layer too (defence in
|
||||
/// depth — the DB CHECK constraint is the authority).
|
||||
fn validate_name(name: &str) -> Result<(), DomainError> {
|
||||
static RE: Lazy<Regex> = Lazy::new(|| {
|
||||
Regex::new(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$").unwrap()
|
||||
});
|
||||
if !RE.is_match(name) { return Err(DomainError::invalid("group.name.rfc5321")); }
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
pub const INTERNAL_GROUP_ID: Uuid = uuid!("00000000-0000-0000-0000-000000000001");
|
||||
pub const MAX_GROUP_DEPTH: u8 = 8;
|
||||
```
|
||||
|
||||
New file: `src/domain/repositories/subject_group_repository.rs` (trait).
|
||||
|
||||
Methods:
|
||||
- `create(group: SubjectGroup) -> Result<SubjectGroup, DomainError>`
|
||||
- `get_by_id(id: Uuid) -> Result<Option<SubjectGroup>, DomainError>`
|
||||
- `get_by_name(name: &str) -> Result<Option<SubjectGroup>, DomainError>` (case-insensitive via CITEXT)
|
||||
- `list(limit, offset, name_query: Option<&str>) -> Result<(Vec<SubjectGroup>, u64 /*total*/), DomainError>`
|
||||
- `rename(id: Uuid, new_name: &str) -> Result<SubjectGroup, DomainError>`
|
||||
- `delete(id: Uuid) -> Result<(), DomainError>` (cascade-deletes grants via FK from access_grants — TODO confirm; if no FK exists, also delete grants in the same transaction)
|
||||
- `add_member(group_id: Uuid, member: GroupMember, added_by: Uuid) -> Result<(), DomainError>`
|
||||
- `remove_member(group_id: Uuid, member: GroupMember) -> Result<(), DomainError>`
|
||||
- `list_direct_members(group_id: Uuid) -> Result<Vec<GroupMember>, DomainError>`
|
||||
- `list_transitive_users(group_id: Uuid) -> Result<Vec<Uuid>, DomainError>` (debug/audit)
|
||||
- `groups_for_user(user_id: Uuid) -> Result<HashSet<Uuid>, DomainError>` (the hot path — recursive CTE)
|
||||
- `would_introduce_cycle(parent: Uuid, candidate_child_group: Uuid) -> Result<bool, DomainError>`
|
||||
- `current_depth(group_id: Uuid) -> Result<u8, DomainError>` (longest path from this group to any leaf)
|
||||
|
||||
## Infrastructure layer
|
||||
|
||||
New file:
|
||||
`src/infrastructure/repositories/pg/subject_group_pg_repository.rs`.
|
||||
|
||||
Two queries deserve attention because the rest are straight CRUD.
|
||||
|
||||
### Cycle detection (write-time)
|
||||
|
||||
```sql
|
||||
-- Adding member_group_id=$candidate to group_id=$parent introduces a cycle
|
||||
-- iff $parent is reachable from $candidate by walking child-edges.
|
||||
WITH RECURSIVE descendants AS (
|
||||
SELECT member_group_id AS g
|
||||
FROM auth.subject_group_members
|
||||
WHERE group_id = $candidate AND member_group_id IS NOT NULL
|
||||
|
||||
UNION -- de-dup; the union of disjoint paths is OK
|
||||
|
||||
SELECT m.member_group_id
|
||||
FROM auth.subject_group_members m
|
||||
JOIN descendants d ON m.group_id = d.g
|
||||
WHERE m.member_group_id IS NOT NULL
|
||||
)
|
||||
SELECT 1 FROM descendants WHERE g = $parent LIMIT 1;
|
||||
```
|
||||
|
||||
If this returns a row → reject with `DomainError::invalid("group.cycle")`.
|
||||
|
||||
### Transitive expansion: groups_for_user
|
||||
|
||||
```sql
|
||||
WITH RECURSIVE user_groups AS (
|
||||
-- Base: direct memberships
|
||||
SELECT group_id FROM auth.subject_group_members
|
||||
WHERE member_user_id = $1
|
||||
|
||||
UNION
|
||||
|
||||
-- Recursive: groups containing those groups
|
||||
SELECT m.group_id
|
||||
FROM auth.subject_group_members m
|
||||
JOIN user_groups ug ON m.member_group_id = ug.group_id
|
||||
)
|
||||
SELECT group_id FROM user_groups;
|
||||
```
|
||||
|
||||
The depth cap (`MAX_GROUP_DEPTH = 8`) is enforced at *write* time, so this
|
||||
recursion is bounded by the data — Postgres has no depth limit on the CTE
|
||||
itself.
|
||||
|
||||
### Depth check (write-time)
|
||||
|
||||
```sql
|
||||
-- The depth of group $parent after adding $child as a member group =
|
||||
-- depth-of($parent before mutation) + (1 + depth-of-subtree-rooted-at($child))
|
||||
-- Simpler: compute longest path from $parent across the proposed graph and
|
||||
-- reject if it would exceed 8.
|
||||
WITH RECURSIVE path AS (
|
||||
SELECT group_id AS g, 1 AS depth
|
||||
FROM auth.subject_group_members WHERE group_id = $parent
|
||||
UNION
|
||||
SELECT m.group_id, p.depth + 1
|
||||
FROM auth.subject_group_members m
|
||||
JOIN path p ON m.member_group_id = p.g
|
||||
)
|
||||
SELECT COALESCE(MAX(depth), 0) FROM path;
|
||||
```
|
||||
|
||||
If the post-mutation projection of this would exceed `MAX_GROUP_DEPTH = 8`,
|
||||
reject with `DomainError::invalid("group.depth_exceeded")`.
|
||||
|
||||
In practice both checks can be combined in the same transaction, run with
|
||||
`FOR UPDATE` on the parent group row to prevent concurrent racing mutations
|
||||
from each squeezing under the limit individually.
|
||||
|
||||
## Application services
|
||||
|
||||
New file: `src/application/services/subject_group_service.rs`.
|
||||
|
||||
Methods mirror the repository trait, plus:
|
||||
- Each mutator emits one structured audit event (`tracing::info!(target = "audit", ...)`).
|
||||
- `add_member` runs cycle + depth checks in the same transaction as the insert.
|
||||
- `delete` is guarded against removing `is_virtual = TRUE` groups.
|
||||
- Service exposes one fast path: `is_user_in_group(user_id, group_id) -> bool`,
|
||||
used by the `Internal` group check (special-cased as
|
||||
`!user.is_external` — no DB hit).
|
||||
|
||||
Wire the service into `AppState::services` in `src/common/di.rs` alongside
|
||||
the other application services.
|
||||
|
||||
## AuthorizationEngine extension
|
||||
|
||||
Modify `src/application/ports/authorization_ports.rs`:
|
||||
|
||||
Add a new helper on the trait (default impl can be provided in the trait,
|
||||
overridden by `PgAclEngine`):
|
||||
|
||||
```rust
|
||||
/// Returns the caller plus the IDs of every group they belong to transitively,
|
||||
/// plus the predefined `INTERNAL_GROUP_ID` when the caller is not external.
|
||||
/// This is the single place transitive membership is walked — all v1
|
||||
/// callers, and the future closure-table swap-in, go through this function.
|
||||
async fn expand_subject(&self, user_id: Uuid) -> Result<Arc<HashSet<Uuid>>, DomainError>;
|
||||
```
|
||||
|
||||
Modify `src/infrastructure/services/pg_acl_engine.rs`:
|
||||
|
||||
1. Add a Moka cache field on the struct:
|
||||
|
||||
```rust
|
||||
user_groups_cache: moka::future::Cache<Uuid, Arc<HashSet<Uuid>>>,
|
||||
```
|
||||
|
||||
constructed with:
|
||||
|
||||
```rust
|
||||
Cache::builder()
|
||||
.max_capacity(50_000)
|
||||
.time_to_live(Duration::from_secs(30))
|
||||
.build();
|
||||
```
|
||||
|
||||
2. Implement `expand_subject`:
|
||||
|
||||
```rust
|
||||
async fn expand_subject(&self, user_id: Uuid) -> Result<Arc<HashSet<Uuid>>, DomainError> {
|
||||
if let Some(cached) = self.user_groups_cache.get(&user_id).await {
|
||||
return Ok(cached);
|
||||
}
|
||||
let direct = self.repo.groups_for_user(user_id).await?; // recursive CTE
|
||||
let mut set = HashSet::with_capacity(direct.len() + 2);
|
||||
set.insert(user_id);
|
||||
set.extend(direct);
|
||||
// Internal virtual group: implicit for every non-external user.
|
||||
if !self.users.is_external(user_id).await? {
|
||||
set.insert(INTERNAL_GROUP_ID);
|
||||
}
|
||||
let arc = Arc::new(set);
|
||||
self.user_groups_cache.insert(user_id, arc.clone()).await;
|
||||
Ok(arc)
|
||||
}
|
||||
```
|
||||
|
||||
3. Modify the existing cascade queries (`folder_cascade_grant_exists` at
|
||||
lines 92–121 and `file_cascade_grant_exists` at lines 125–168 of
|
||||
`pg_acl_engine.rs`):
|
||||
|
||||
Replace `g.subject_id = $2` with `g.subject_id = ANY($2)` and bind a
|
||||
`Vec<Uuid>` produced by `expand_subject(user_id).await?.iter().copied().collect()`.
|
||||
|
||||
Subject_type must also be relaxed: today the query passes
|
||||
`subject_type = 'user'`. With groups, the helper should match against
|
||||
`subject_type IN ('user', 'group')`. (Tokens and externals are not part
|
||||
of this path; they have their own auth flows.)
|
||||
|
||||
The shape of the rest of the query — and the folder/file ltree cascade — is
|
||||
unchanged. The closure-table migration (future) will only re-implement
|
||||
`groups_for_user` against a precomputed table; callers stay the same.
|
||||
|
||||
## REST API
|
||||
|
||||
New file: `src/interfaces/api/handlers/subject_group_handler.rs`.
|
||||
Wire into `src/interfaces/api/routes.rs` alongside `admin_handler`.
|
||||
|
||||
| Method | Route | Body / Query | Guard |
|
||||
|---|---|---|---|
|
||||
| POST | `/api/groups` | `{ name, description? }` | admin only |
|
||||
| GET | `/api/groups` | `?limit&offset&q` | admin only |
|
||||
| GET | `/api/groups/{id}` | — | admin only |
|
||||
| PATCH | `/api/groups/{id}` | `{ name?, description? }` | admin only |
|
||||
| DELETE | `/api/groups/{id}` | — | admin only |
|
||||
| POST | `/api/groups/{id}/members` | `{ user_id?, group_id? }` (XOR) | admin only |
|
||||
| GET | `/api/groups/{id}/members` | direct members | admin only |
|
||||
| GET | `/api/groups/{id}/effective-members` | transitive resolved users | admin only |
|
||||
| DELETE | `/api/groups/{id}/members/user/{uid}` | — | admin only |
|
||||
| DELETE | `/api/groups/{id}/members/group/{gid}` | — | admin only |
|
||||
| GET | `/api/groups/{id}/grants` | grants where this group is subject | admin only |
|
||||
| GET | `/api/groups/{id}/path-to-user/{uid}` | audit: explain membership | admin only |
|
||||
|
||||
Plus the share-dialog endpoint extension:
|
||||
|
||||
| Method | Route | Body / Query | Guard |
|
||||
|---|---|---|---|
|
||||
| GET | `/api/groups/search` | `?q=` returns non-virtual groups whose name matches | authenticated |
|
||||
|
||||
This new search endpoint is **authenticated, not admin-gated** — any user can
|
||||
discover groups to share with. Returns name + id only (no membership list).
|
||||
|
||||
Admin guard implementation: mirror `admin_handler.rs:64-100` exactly (extract
|
||||
JWT, check `claims.role == "admin"`, 403 otherwise). Extract into a shared
|
||||
helper `require_admin(state, headers) -> Result<(Uuid, String), AppError>`
|
||||
in `interfaces/middleware/` so the new handler and `admin_handler` both use
|
||||
the same code path.
|
||||
|
||||
## Audit logging
|
||||
|
||||
Convention: every mutating service-layer action emits one
|
||||
`tracing::info!(target = "audit", ...)` event with structured fields.
|
||||
Example:
|
||||
|
||||
```rust
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "group.member_added",
|
||||
group_id = %group_id,
|
||||
member_user_id = ?member_user_id,
|
||||
member_group_id = ?member_group_id,
|
||||
added_by = %caller_id,
|
||||
);
|
||||
```
|
||||
|
||||
Events to emit:
|
||||
- `group.created` { group_id, name, created_by }
|
||||
- `group.renamed` { group_id, old_name, new_name, by }
|
||||
- `group.deleted` { group_id, name, by }
|
||||
- `group.member_added` { group_id, member, by }
|
||||
- `group.member_removed` { group_id, member, by }
|
||||
- `group.cycle_rejected` { parent, candidate_child, by } (security-relevant)
|
||||
- `group.depth_exceeded` { parent, by, attempted_depth }
|
||||
|
||||
The plan does *not* wire a syslog appender; downstream operators add a
|
||||
`tracing-syslog` or `tracing-journald` subscriber via env-var-driven config.
|
||||
A follow-up issue should be opened for that.
|
||||
|
||||
## Debug instrumentation (perf observability)
|
||||
|
||||
Distinct from the audit log: every authorization check emits one structured
|
||||
`tracing::debug!` line with timing and cache-hit telemetry so the closure-
|
||||
table-vs-cache decision (Option 2 → Option 3 in the design doc) can be made
|
||||
on real data rather than speculation.
|
||||
|
||||
Implementation: wrap each call to `AuthorizationEngine::check` /
|
||||
`AuthorizationEngine::expand_subject` in a tracing span and increment
|
||||
per-call counters. Suggested shape:
|
||||
|
||||
```rust
|
||||
impl PgAclEngine {
|
||||
async fn check(&self, subject: Subject, perm: Permission, resource: Resource)
|
||||
-> Result<bool, DomainError>
|
||||
{
|
||||
let start = std::time::Instant::now();
|
||||
let counters = QueryCounters::default();
|
||||
|
||||
let result = self.check_inner(subject, perm, resource, &counters).await;
|
||||
|
||||
tracing::debug!(
|
||||
event = "authz.check",
|
||||
subject = ?subject,
|
||||
permission = ?perm,
|
||||
resource = ?resource,
|
||||
allowed = result.as_ref().ok().copied().unwrap_or(false),
|
||||
duration_us = start.elapsed().as_micros() as u64,
|
||||
cache_hit = counters.cache_hit.load(Ordering::Relaxed),
|
||||
sql_queries = counters.sql_queries.load(Ordering::Relaxed),
|
||||
expanded_groups = counters.expanded_group_count.load(Ordering::Relaxed),
|
||||
);
|
||||
|
||||
result
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Where `QueryCounters` is a tiny struct of `AtomicU32`s passed through the
|
||||
call chain, incremented at each `sqlx::query*` call site inside the authz
|
||||
path. `cache_hit` is set by `expand_subject` based on whether the Moka
|
||||
`Cache::get` returned `Some`.
|
||||
|
||||
**What this gives you:**
|
||||
|
||||
| Field | Use |
|
||||
|---|---|
|
||||
| `duration_us` | latency histogram per check; alert on p99 regression |
|
||||
| `cache_hit` | hit-rate metric → decide when to extend TTL or switch to closure table |
|
||||
| `sql_queries` | 0 on cache hit; 1 on cache miss + grant lookup; 2 if cache miss + transitive expansion + grant lookup — confirms the query plan in production |
|
||||
| `expanded_groups` | size of the user's transitive group set; if this stays small in practice, the recursive CTE is more than enough |
|
||||
|
||||
**Cost:** sub-microsecond per check (atomic increments + a single
|
||||
`tracing::debug!` emission, which becomes a no-op when the subscriber is at
|
||||
INFO or higher). No runtime cost in production unless debug logging is
|
||||
explicitly enabled.
|
||||
|
||||
**Recommended deployment hook:** an env var `OXICLOUD_AUTHZ_DEBUG=true` that
|
||||
flips the subscriber filter to allow `target="oxicloud::authz" level=debug`
|
||||
events through. Operators turn it on temporarily when investigating
|
||||
performance issues; default is INFO and emits nothing from this path.
|
||||
|
||||
## Share dialog extension
|
||||
|
||||
Modify `static/js/components/shareModal.js` around line 310.
|
||||
|
||||
Currently it calls `addressBook.searchContacts(q, [SYSTEM_BOOK_ID])`. Add a
|
||||
parallel call to `fetch('/api/groups/search?q=' + encodeURIComponent(q))`.
|
||||
Merge the two result lists, tag each item by source (`user` vs `group`
|
||||
vs `contact`), and render with the appropriate icon (user avatar /
|
||||
`fa-layer-group` / contact card).
|
||||
|
||||
On selection, dispatch to the existing grant-creation flow with the
|
||||
correct `subject_type`:
|
||||
- `user` → `subject_type = 'user'`, `subject_id = user.id`
|
||||
- `group` → `subject_type = 'group'`, `subject_id = group.id`
|
||||
- `contact` → resolved through the existing address-book mapper to the
|
||||
matching user_id (no change from today)
|
||||
|
||||
## i18n
|
||||
|
||||
Add to `static/locales/en.json` (errors surfaced by the API + share-dialog):
|
||||
|
||||
```json
|
||||
"errors": {
|
||||
"group_name_invalid": "Group name must match the email-prefix format (letters, digits, dot, dash, underscore; 1–64 chars).",
|
||||
"group_cycle": "This member would create a circular group reference.",
|
||||
"group_depth_exceeded": "This nesting depth exceeds the maximum allowed (8).",
|
||||
"group_virtual_immutable": "The 'Internal' group is system-managed and cannot be modified.",
|
||||
"group_not_found": "Group not found."
|
||||
}
|
||||
```
|
||||
|
||||
These keys are referenced by `ApiError` payloads from the new handler and
|
||||
by the share-dialog UI when a target group is invalid. Sync the 15 locale
|
||||
files using the Python script pattern from the earlier i18n turn.
|
||||
|
||||
The full set of admin-table labels (`admin.tab_groups`, `admin.col_*`, etc.)
|
||||
is **deferred to the v2 admin UI work** along with the rest of the admin
|
||||
surface for groups.
|
||||
|
||||
## Tests
|
||||
|
||||
### Unit tests
|
||||
|
||||
Module: `src/infrastructure/repositories/pg/subject_group_pg_repository.rs#tests`
|
||||
|
||||
1. `test_create_group_validates_name_rfc5321` — names with spaces / emojis
|
||||
rejected; valid names accepted.
|
||||
2. `test_group_name_unique_case_insensitive` — "Engineering" and
|
||||
"engineering" collide (CITEXT).
|
||||
3. `test_cycle_check_rejects_direct_loop` — adding A to A rejected by the
|
||||
`no_self` CHECK or by the cycle CTE.
|
||||
4. `test_cycle_check_rejects_two_step_loop` — A∋B, B∋C, attempting C∋A
|
||||
rejected.
|
||||
5. `test_cycle_check_rejects_eight_step_loop` — same with longer chain.
|
||||
6. `test_depth_cap_at_8` — adding a 9th level rejected.
|
||||
7. `test_transitive_expansion_includes_indirect_groups` — A∋B, B∋C, U∈A
|
||||
returns {A, B, C} (plus U and Internal).
|
||||
8. `test_internal_group_implicit_for_internal_users` — non-external user's
|
||||
expansion contains `INTERNAL_GROUP_ID`; external user's doesn't.
|
||||
9. `test_virtual_group_cannot_be_deleted` — service rejects delete on the
|
||||
Internal group.
|
||||
10. `test_member_can_be_user_or_group_but_not_both` — XOR check.
|
||||
|
||||
### Integration tests
|
||||
|
||||
11. `test_authz_cascades_through_group` — Alice in group G; G has read grant
|
||||
on file F; AuthorizationEngine::check returns Allow.
|
||||
12. `test_authz_cascades_through_nested_group` — Alice in B, B in A, A has
|
||||
grant. Expect Allow.
|
||||
13. `test_grant_revoked_when_group_deleted` — delete G; previous
|
||||
G-mediated grants no longer apply (FK CASCADE).
|
||||
14. `test_user_removed_from_group_loses_access_after_cache_ttl` — remove
|
||||
Alice from G; within 30s old answer may persist; after TTL, denied.
|
||||
15. `test_internal_group_grant_visible_to_all_internal` — grant `read` on
|
||||
file F to `INTERNAL_GROUP_ID`; every internal user can read F; no
|
||||
external user can.
|
||||
|
||||
### API tests (Hurl)
|
||||
|
||||
16. `tests/api/groups_admin_only.hurl` — non-admin POST /api/groups → 403.
|
||||
17. `tests/api/groups_crud_happy_path.hurl` — create, list, get, rename,
|
||||
delete.
|
||||
18. `tests/api/groups_member_lifecycle.hurl` — add user, add nested
|
||||
group, remove user, remove nested group.
|
||||
19. `tests/api/groups_invalid_name.hurl` — name with space → 400.
|
||||
|
||||
## Verification
|
||||
|
||||
Pre-commit:
|
||||
|
||||
```bash
|
||||
cargo fmt --all
|
||||
cargo clippy --all-features --all-targets -- -D warnings
|
||||
cargo test --workspace
|
||||
biome check --fix static/js/
|
||||
stylelint static/css/
|
||||
tsc -p jsconfig.json --noEmit
|
||||
```
|
||||
|
||||
End-to-end smoke test (manual; v1 is API-driven):
|
||||
|
||||
1. `docker compose up -d postgres`.
|
||||
2. `cargo run`.
|
||||
3. Obtain an admin JWT (log in via the existing login flow, copy the access
|
||||
token).
|
||||
4. Create a group:
|
||||
```
|
||||
curl -X POST /api/groups -H 'Authorization: Bearer …' \
|
||||
-d '{"name":"engineering"}'
|
||||
```
|
||||
5. Confirm name validation rejects `Engineering Team` (returns 400 with
|
||||
`error_code: group_name_invalid`).
|
||||
6. Add yourself as a member:
|
||||
```
|
||||
curl -X POST /api/groups/<gid>/members -d '{"user_id":"<you>"}'
|
||||
```
|
||||
7. Create a second group `qa`, then add `engineering` as a nested member of
|
||||
`qa`. Confirm with `GET /api/groups/<qa_id>/members`.
|
||||
8. Attempt to add `qa` as a member of `engineering` — expect 400 with
|
||||
`error_code: group_cycle`.
|
||||
9. In the browser, open a file → share dialog → type `eng`. The
|
||||
`engineering` group should appear with the layer-group icon. Pick it
|
||||
and grant `read`.
|
||||
10. Log in as a user who is a member of `engineering` (directly or via
|
||||
`qa` cascading) — confirm the file is accessible.
|
||||
11. `DELETE /api/groups/<gid>/members/user/<uid>`. After 30 seconds (cache
|
||||
TTL), confirm access is denied.
|
||||
12. Grant `read` on a file to the `Internal` virtual group (`subject_id`
|
||||
= `00000000-0000-0000-0000-000000000001`). Confirm every internal user
|
||||
has access. Confirm an external user (if available) does not.
|
||||
13. `journalctl -t oxicloud | grep audit` (or equivalent log inspection) —
|
||||
confirm one structured log line per group mutation, with the
|
||||
`target="audit"` and `event="group.*"` fields.
|
||||
|
||||
## Critical files to be modified
|
||||
|
||||
**New files:**
|
||||
- `migrations/20260612000000_subject_groups.sql`
|
||||
- `src/domain/entities/subject_group.rs`
|
||||
- `src/domain/repositories/subject_group_repository.rs`
|
||||
- `src/infrastructure/repositories/pg/subject_group_pg_repository.rs`
|
||||
- `src/application/services/subject_group_service.rs`
|
||||
- `src/interfaces/api/handlers/subject_group_handler.rs`
|
||||
- `tests/api/groups_*.hurl`
|
||||
|
||||
**Modified files:**
|
||||
- `src/application/ports/authorization_ports.rs` — add `expand_subject`.
|
||||
- `src/infrastructure/services/pg_acl_engine.rs` — add Moka cache field,
|
||||
implement `expand_subject`, modify `folder_cascade_grant_exists` (lines
|
||||
92–121) and `file_cascade_grant_exists` (lines 125–168) to use
|
||||
`subject_id = ANY($caller_plus_groups)`.
|
||||
- `src/common/di.rs` — wire `SubjectGroupService` into `AppState`, pass
|
||||
user-repo into `PgAclEngine` constructor.
|
||||
- `src/interfaces/api/routes.rs` — register the new handler.
|
||||
- `src/interfaces/middleware/` — extract `require_admin` shared helper
|
||||
from `admin_handler.rs:64-100`.
|
||||
- `static/js/components/shareModal.js` — parallel `/api/groups/search`
|
||||
call around line 310 to surface groups in the recipient autocomplete.
|
||||
- `static/locales/en.json` + 15 locale files — new `errors.group_*` keys
|
||||
for API error rendering.
|
||||
|
||||
(No changes to `static/admin.html` or `static/js/views/admin/admin.js` in
|
||||
v1 — admin UI is v2 work.)
|
||||
|
||||
## Reused utilities
|
||||
|
||||
- `AppState.authorization` (Arc<PgAclEngine>) — existing DI wiring.
|
||||
- `admin_handler::admin_guard` pattern (`admin_handler.rs:64-100`) — extract
|
||||
shared.
|
||||
- `moka` 0.12.15 (`Cargo.toml:38`) — already present.
|
||||
- `tracing::info!` — existing observability pipeline; just add the
|
||||
`target: "audit"` convention.
|
||||
- `ResourceListComponent` and `userVignette` — already used by other
|
||||
admin tables; reuse for the Groups admin table.
|
||||
- Recursive CTE pattern — new to OxiCloud but standard Postgres.
|
||||
- `auth.users.role = 'admin'` ENUM check — admin guard.
|
||||
@@ -1,298 +0,0 @@
|
||||
# Plan: User Avatar / Image Support
|
||||
|
||||
## Context
|
||||
|
||||
Users need to be able to set a profile photo (avatar). The image must:
|
||||
- Be stored as a URL (`https://…`, `http://…`) or data URI (`data:image/(png|webp|jpeg);base64,…`)
|
||||
- Match the CardDAV `PHOTO` format so the system address book exports it correctly
|
||||
- Be editable **only** for local (username+password) accounts
|
||||
- Be **synced automatically from OIDC** `picture` claim on every login for OIDC accounts
|
||||
- Surface in `userVignette` components (owner column, ShareModal member rows)
|
||||
|
||||
Currently: no `image` column on `auth.users`, no `picture` claim extraction in OIDC, profile page shows initials only, `user_to_contact()` hardcodes `photo_url: None`.
|
||||
|
||||
---
|
||||
|
||||
## Execution order
|
||||
|
||||
### 1. DB Migration
|
||||
**New file:** `migrations/20260526000000_add_user_image.sql`
|
||||
```sql
|
||||
ALTER TABLE auth.users ADD COLUMN IF NOT EXISTS image TEXT;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 2. Domain Entity
|
||||
**`src/domain/entities/user.rs`**
|
||||
- Add `image: Option<String>` field
|
||||
- `User::new()` and `User::new_oidc()` — initialise to `None`
|
||||
- Add getter `pub fn image(&self) -> Option<&str>`
|
||||
- Add setter `pub fn set_image(&mut self, image: Option<String>)`
|
||||
- Add owned getter for persistence `pub fn image_owned(&self) -> Option<String>`
|
||||
|
||||
---
|
||||
|
||||
### 3. User Repository
|
||||
**`src/infrastructure/repositories/pg/user_pg_repository.rs`**
|
||||
- Add `image` to every `SELECT` that builds a `User` (row-mapper)
|
||||
- Extend the `UPDATE` SQL in `update_user()` to include `image = $11`
|
||||
- Add dedicated: `async fn update_image(&self, user_id: Uuid, image: Option<String>) -> Result<(), DomainError>`
|
||||
|
||||
---
|
||||
|
||||
### 4. OIDC: extract `picture` claim
|
||||
**`src/application/ports/auth_ports.rs`**
|
||||
- Add `pub picture: Option<String>` to `OidcIdClaims`
|
||||
|
||||
**`src/infrastructure/services/oidc_service.rs`**
|
||||
- Add `picture: Option<String>` to both `IdTokenClaims` and `UserInfoResponse` structs
|
||||
- Pass `picture` into the returned `OidcIdClaims`
|
||||
|
||||
**`src/application/services/auth_application_service.rs`** — in `oidc_callback()`:
|
||||
- **Create path**: pass `claims.picture` to `User::new_oidc()`
|
||||
(or call `user.set_image(claims.picture.clone())` before persisting)
|
||||
- **Update path**: always call `user.set_image(claims.picture.clone())` then persist
|
||||
(OIDC image is always authoritative — overwrite even if user had set one before)
|
||||
|
||||
---
|
||||
|
||||
### 5. User DTO
|
||||
**`src/application/dtos/user_dto.rs`**
|
||||
Add two fields to `UserDto`:
|
||||
```rust
|
||||
pub image: Option<String>,
|
||||
pub can_edit_image: bool, // true iff !user.is_oidc_user()
|
||||
```
|
||||
Populate in `UserDto::from(user)`.
|
||||
|
||||
---
|
||||
|
||||
### 6. Validation helper (shared)
|
||||
In the auth application service (or a small `validation.rs` module in `src/common/`):
|
||||
```rust
|
||||
fn validate_image_url(image: &str) -> bool {
|
||||
image.starts_with("https://")
|
||||
|| image.starts_with("http://")
|
||||
|| image.starts_with("data:image/png;base64,")
|
||||
|| image.starts_with("data:image/webp;base64,")
|
||||
|| image.starts_with("data:image/jpeg;base64,")
|
||||
}
|
||||
```
|
||||
Max length for data URIs: **10 KB** (10 608 bytes) to prevent DB abuse — a 1à4×104 WebP at quality 0.85 is well under this; a raw PNG could exceed it so the client must resize/compress first.
|
||||
|
||||
---
|
||||
|
||||
### 7. Auth Application Service — new method
|
||||
**`src/application/services/auth_application_service.rs`**
|
||||
```rust
|
||||
pub async fn update_user_image(
|
||||
&self,
|
||||
caller_id: Uuid,
|
||||
image: Option<String>,
|
||||
) -> Result<(), AppError>
|
||||
```
|
||||
Logic:
|
||||
1. Load user from repository
|
||||
2. If `user.is_oidc_user()` → return `AppError::Forbidden`
|
||||
3. If `image.is_some()` → validate format + length; return `AppError::Validation` if invalid
|
||||
4. Call `user_repository.update_image(caller_id, image).await`
|
||||
|
||||
---
|
||||
|
||||
### 8. Auth Handler + Route
|
||||
**`src/interfaces/api/handlers/auth_handler.rs`**
|
||||
|
||||
New DTO (inline or in a dto file):
|
||||
```rust
|
||||
#[derive(Deserialize)]
|
||||
pub struct UpdateUserImageDto {
|
||||
pub image: Option<String>, // None = clear the image
|
||||
}
|
||||
```
|
||||
|
||||
New handler `update_user_image` — pattern mirrors `change_password`:
|
||||
- Extract `CurrentUserId`, JSON body
|
||||
- Call service method
|
||||
- Map `AppError::Forbidden` → 403, `AppError::Validation` → 422, else 200
|
||||
|
||||
**`src/interfaces/api/routes.rs`** — in `auth_protected_routes()`:
|
||||
```rust
|
||||
.route("/me/image", put(update_user_image))
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 9. System Address Book
|
||||
**`src/interfaces/api/handlers/contacts_handler.rs`** — `user_to_contact()`:
|
||||
```rust
|
||||
photo_url: user.image.clone(), // was: None
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 10. Frontend — `systemUsers.js`
|
||||
**`static/js/model/systemUsers.js`**
|
||||
- Add `let _photoIndex = null;` (`Map<string, string|null>`)
|
||||
- In `_ensureIndex()`: build `_photoIndex` from `c.photo_url` alongside the name map
|
||||
- Inject current user's photo from `localStorage.getItem('oxicloud_user')?.image`
|
||||
- Add `async function getPhoto(userId): Promise<string|null>`
|
||||
- Export `{ prefetch, getDisplayName, getPhoto, isAvailable }`
|
||||
|
||||
---
|
||||
|
||||
### 11. Frontend — `userVignette.js`
|
||||
**`static/js/components/userVignette.js`**
|
||||
|
||||
In `createUserVignette(userId, size)`:
|
||||
- After async name resolves, also await `systemUsers.getPhoto(userId)`
|
||||
- If photo URL is truthy: replace the initials text with `<img src="…" alt="…">` inside `user-vignette__avatar`
|
||||
- Wire `onerror` on the img to fall back to initials (guard against broken URLs)
|
||||
|
||||
CSS addition in `userVignette.css`:
|
||||
```css
|
||||
.user-vignette__avatar img {
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
object-fit: cover;
|
||||
border-radius: 50%;
|
||||
display: block;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 12. Frontend — User Menu (top-right)
|
||||
**`static/js/app/userMenu.js`** — `updateUserMenuData()`:
|
||||
- Read `user.image` from the stored `oxicloud_user` in localStorage
|
||||
- `#user-avatar` (38 px circle): if `user.image` is set, replace inner HTML with `<img src="…" alt="…">` instead of initials text; wire `onerror` fallback to initials
|
||||
- `#user-menu-avatar` (48 px circle in dropdown): same treatment
|
||||
- When `profile.js` saves a new image successfully, it must also refresh the stored `oxicloud_user` in localStorage (re-fetch `/api/auth/me` and update) then call `updateUserMenuData()`
|
||||
|
||||
**`static/css/components/userMenu.css`** — add inside the file:
|
||||
```css
|
||||
.user-avatar img,
|
||||
.user-menu-avatar img {
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
object-fit: cover;
|
||||
border-radius: 50%;
|
||||
display: block;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 13. Frontend — Image resize helper (new shared utility)
|
||||
**`static/js/utils/imageResize.js`** — new file
|
||||
|
||||
```js
|
||||
/**
|
||||
* Load a File/Blob as an Image, draw it on a Canvas, resize to fit within
|
||||
* MAX_SIZE × MAX_SIZE, and return a data URI.
|
||||
*
|
||||
* @param {File} file
|
||||
* @param {number} [maxSize=102]
|
||||
* @returns {Promise<string>} data:image/webp;base64,… (or jpeg fallback)
|
||||
*/
|
||||
export async function resizeImageToDataUrl(file, maxSize = 104)
|
||||
```
|
||||
|
||||
Logic:
|
||||
1. Read file with `FileReader` → data URL
|
||||
2. Create `<img>` element and wait for `onload`
|
||||
3. Compute output dimensions: scale down proportionally if either dimension > `maxSize`; never scale up
|
||||
4. Draw onto `OffscreenCanvas` (or regular `<canvas>`) at the computed size
|
||||
5. Export with `canvas.toBlob('image/webp', 0.85)` (fallback to `image/jpeg` if WebP not supported)
|
||||
6. Convert Blob → base64 data URI via `FileReader`
|
||||
|
||||
Accepts only MIME types: `image/png`, `image/webp`, `image/jpeg` — reject others with a thrown `Error`.
|
||||
|
||||
---
|
||||
|
||||
### 14. Frontend — Profile Page
|
||||
**`static/profile.html`**
|
||||
- Make `#p-avatar` support both `<img>` and initials text
|
||||
- Add edit button (pencil icon) visible only when `user.can_edit_image === true`
|
||||
- Add collapsible edit panel with **two input modes** (tabs or toggle):
|
||||
- **URL tab**: `<input type="url" id="p-image-url" placeholder="https://…">` with validation hint
|
||||
- **Upload tab**: `<input type="file" id="p-image-file" accept="image/png,image/jpeg,image/webp">` + live preview thumbnail
|
||||
- Save / Cancel / Remove (clear) buttons
|
||||
|
||||
**`static/js/views/profile/profile.js`**
|
||||
|
||||
*Display:*
|
||||
- If `user.image`: set `#p-avatar` to `<img src="…">` (with `onerror` → initials fallback)
|
||||
- If `user.can_edit_image`: show edit pencil
|
||||
- For OIDC users: show photo if `user.image` set; show "Managed by your identity provider" note; no edit controls
|
||||
|
||||
*URL mode save:*
|
||||
- Validate prefix client-side (`https://`, `http://`, `data:image/…;base64,`)
|
||||
- `PUT /api/auth/me/image` with `{ image: url || null }`
|
||||
|
||||
*Upload mode save:*
|
||||
- On file selection: call `resizeImageToDataUrl(file, 104)` from the new utility
|
||||
- Show preview in a `<img id="p-image-preview">` (hidden until file chosen)
|
||||
- On Save: send resulting data URI via `PUT /api/auth/me/image` with `{ image: dataUri }`
|
||||
- Show progress indicator during resize + upload (data URIs for a 104×104 WebP are ~2-5 kB)
|
||||
|
||||
*After successful save (both modes):*
|
||||
- Re-fetch `/api/auth/me`, update `oxicloud_user` in localStorage
|
||||
- Call `updateUserMenuData()` to refresh top-right avatar immediately
|
||||
- Collapse the edit panel and update `#p-avatar` in-place
|
||||
|
||||
---
|
||||
|
||||
## Files to modify / create
|
||||
|
||||
| File | Action |
|
||||
|---|---|
|
||||
| `migrations/20260526000000_add_user_image.sql` | **CREATE** |
|
||||
| `src/domain/entities/user.rs` | add `image` field + getter/setter |
|
||||
| `src/infrastructure/repositories/pg/user_pg_repository.rs` | add to SELECT/UPDATE + `update_image()` |
|
||||
| `src/application/ports/auth_ports.rs` | add `picture` to `OidcIdClaims` |
|
||||
| `src/infrastructure/services/oidc_service.rs` | add `picture` to claims structs |
|
||||
| `src/application/services/auth_application_service.rs` | OIDC sync + `update_user_image()` |
|
||||
| `src/application/dtos/user_dto.rs` | add `image`, `can_edit_image` |
|
||||
| `src/interfaces/api/handlers/auth_handler.rs` | `update_user_image` handler |
|
||||
| `src/interfaces/api/routes.rs` | register `PUT /auth/me/image` |
|
||||
| `src/interfaces/api/handlers/contacts_handler.rs` | `user_to_contact()` maps `image` → `photo_url` |
|
||||
| `static/js/model/systemUsers.js` | add `_photoIndex`, `getPhoto()` |
|
||||
| `static/js/components/userVignette.js` | render `<img>` when photo available |
|
||||
| `static/css/components/userVignette.css` | add `img` rule inside avatar |
|
||||
| `static/js/utils/imageResize.js` | **CREATE** — Canvas resize → WebP/JPEG data URI |
|
||||
| `static/profile.html` | avatar image + URL input + file upload + preview |
|
||||
| `static/js/views/profile/profile.js` | photo display + URL/upload edit flow + post-save menu refresh |
|
||||
| `static/js/app/userMenu.js` | render `<img>` in both avatar circles when `user.image` present |
|
||||
| `static/css/components/userMenu.css` | add `img` cover rule for `.user-avatar` and `.user-menu-avatar` |
|
||||
|
||||
---
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
# Backend
|
||||
cargo fmt --all
|
||||
cargo clippy --all-features --all-targets -- -D warnings
|
||||
cargo test
|
||||
|
||||
# Frontend
|
||||
biome lint static/js/
|
||||
tsc -p jsconfig.json --noEmit
|
||||
stylelint static/css/
|
||||
```
|
||||
|
||||
**Smoke tests:**
|
||||
1. Local user → profile page → edit image → paste `https://example.com/me.jpg` → Save → avatar shows photo
|
||||
2. Local user → paste `data:image/png;base64,…` → Save → works
|
||||
3. Local user → paste invalid string → Save → 422 error shown
|
||||
4. Local user → clear image (empty) → Save → avatar reverts to initials
|
||||
5. OIDC user → `picture` claim present → after login, `GET /api/auth/me` returns `image` → profile shows photo, no edit button
|
||||
6. OIDC user → `picture` claim absent → `image` is null → profile shows initials
|
||||
7. SharedWithMe owner column → users with photos show `<img>`, others show initials
|
||||
8. ShareModal People section → member avatars show photos where available
|
||||
9. CardDAV client sync → system address book contact has `PHOTO` property set
|
||||
10. After saving a photo on the profile page → top-right avatar button and dropdown header both update immediately without a page reload
|
||||
11. Upload a large PNG (e.g. 2000×2000) → client resizes to 104×104 WebP, preview appears, Save sends data URI, backend accepts (< 10 KB)
|
||||
12. Upload a 300×300 image → client does NOT upscale, stores at original dimensions
|
||||
13. Upload a non-image file (PDF) → rejected client-side before any network call
|
||||
@@ -1,358 +0,0 @@
|
||||
# Plan — `UserLifecycleHook` + `is_external` flag
|
||||
|
||||
## Context
|
||||
|
||||
Today four code paths in `auth_application_service.rs` each call `create_personal_folder()` immediately after inserting an `auth.users` row: public `register`, `setup_create_admin`, admin `create_user`, and OIDC JIT (lines 283, 360, 832, 1277). A fifth self-heal at `folder_service.rs:350-365` retries home-folder creation when listing root folders returns empty. Five places, one concern, no shared abstraction — and adding a future service (calendar, address book, GPG keyring, external-user provenance for the upcoming magic-link feature) would have to touch all five again.
|
||||
|
||||
Separately, the upcoming "share with `external@example.com`" feature needs `auth.users` to distinguish recipients-with-no-storage from real internal users. The codebase already declares `Subject::External` (`domain/services/authorization.rs`) but no DB representation exists yet.
|
||||
|
||||
This plan introduces a `UserLifecycleHook` trait (mirroring the existing `FileLifecycleHook` / `BlobLifecycleHook` pattern at `application/ports/file_lifecycle.rs`), wires a dispatcher into the four lifecycle events, migrates the scattered eager work into services that own their own lifecycle (each implementing the trait with explicit no-ops for events they don't care about), and adds the `is_external` boolean to `auth.users` so hooks can short-circuit for non-internal users. The change is purely a refactor at first — behaviour is preserved — but it sets up the v2 external-user flow to land as a hook impl rather than a new auth code path.
|
||||
|
||||
## Design
|
||||
|
||||
### Trait shape
|
||||
|
||||
The trait diverges from `FileLifecycleHook`'s sync fire-and-forget model on purpose: file events fire on every upload (hot path, fire-and-forget appropriate); user events are rare (login is seconds-per-user, not requests-per-second) and some require synchronous semantics (provisioning must finish before the session token is returned; deletion cleanup must commit atomically with the user DELETE). The trait is async; the dispatcher decides per-event whether errors abort the flow.
|
||||
|
||||
```rust
|
||||
// src/application/ports/user_lifecycle.rs (new)
|
||||
#[async_trait]
|
||||
pub trait UserLifecycleHook: Send + Sync {
|
||||
/// Short identifier used in tracing / error logs. e.g. "home_folder".
|
||||
fn name(&self) -> &'static str;
|
||||
|
||||
/// Fires once after INSERT into auth.users succeeds, regardless of path.
|
||||
async fn on_user_created(&self, user: &User) -> Result<(), DomainError>;
|
||||
|
||||
/// Fires after every successful authentication, before the session
|
||||
/// token is returned. MUST be idempotent (safety net for services
|
||||
/// added after the user existed).
|
||||
async fn on_user_login(&self, user: &User) -> Result<(), DomainError>;
|
||||
|
||||
/// Fires on every session termination. `reason` lets hooks
|
||||
/// distinguish causes (audit cares; cache invalidation does not).
|
||||
async fn on_user_logout(&self, user: &User, reason: LogoutReason)
|
||||
-> Result<(), DomainError>;
|
||||
|
||||
/// Fires inside the same transaction as the auth.users DELETE.
|
||||
/// Returning Err aborts the deletion. `mode` distinguishes admin
|
||||
/// delete (policy-driven cleanup) from GDPR purge (force everything).
|
||||
async fn on_user_deleted(
|
||||
&self,
|
||||
user: &User,
|
||||
mode: DeletionMode,
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
) -> Result<(), DomainError>;
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum LogoutReason {
|
||||
UserInitiated, // explicit logout
|
||||
SessionExpired, // TTL hit
|
||||
AdminRevoked, // single-session revocation by admin
|
||||
AccountDisabled, // user.active flipped to FALSE → all sessions revoked
|
||||
PasswordChanged, // sibling sessions invalidated by a password change
|
||||
TokenReused, // session-family reuse detection (existing feature)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum DeletionMode { AdminDelete, GdprPurge }
|
||||
```
|
||||
|
||||
**No default impls.** Every hook must declare all four methods. Use explicit `Ok(())` for events you don't care about — matches the FileLifecycleHook convention and forces conscious acknowledgement.
|
||||
|
||||
### Dispatcher
|
||||
|
||||
```rust
|
||||
// src/application/services/user_lifecycle_service.rs (new)
|
||||
pub struct UserLifecycleService {
|
||||
hooks: Vec<Arc<dyn UserLifecycleHook>>,
|
||||
}
|
||||
|
||||
impl UserLifecycleService {
|
||||
pub fn new() -> Self { Self { hooks: Vec::new() } }
|
||||
pub fn with_hook(mut self, hook: Arc<dyn UserLifecycleHook>) -> Self {
|
||||
self.hooks.push(hook); self
|
||||
}
|
||||
|
||||
// Per-event dispatchers with event-specific failure semantics:
|
||||
|
||||
/// Created: log-and-continue. Next login's `on_user_login` retries
|
||||
/// idempotently if anything fails here.
|
||||
pub async fn dispatch_created(&self, user: &User) {
|
||||
for h in &self.hooks {
|
||||
if let Err(e) = h.on_user_created(user).await {
|
||||
tracing::error!(target: "user_lifecycle",
|
||||
hook = h.name(), user_id = %user.id(), error = %e,
|
||||
"on_user_created failed; will retry on next login");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Login: log-and-continue. Same reasoning.
|
||||
pub async fn dispatch_login(&self, user: &User) { /* same shape */ }
|
||||
|
||||
/// Logout: fire-and-forget (spawned), errors logged. The HTTP
|
||||
/// response shouldn't wait for cache flushes.
|
||||
pub fn dispatch_logout(&self, user: User, reason: LogoutReason) {
|
||||
let hooks = self.hooks.clone();
|
||||
tokio::spawn(async move {
|
||||
for h in &hooks {
|
||||
if let Err(e) = h.on_user_logout(&user, reason).await {
|
||||
tracing::error!(target: "user_lifecycle",
|
||||
hook = h.name(), reason = ?reason,
|
||||
user_id = %user.id(), error = %e,
|
||||
"on_user_logout failed");
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/// Deleted: propagate first Err to abort the transaction.
|
||||
pub async fn dispatch_deleted(
|
||||
&self,
|
||||
user: &User,
|
||||
mode: DeletionMode,
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
) -> Result<(), DomainError> {
|
||||
for h in &self.hooks {
|
||||
h.on_user_deleted(user, mode, tx).await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### `is_external` flag (additive migration)
|
||||
|
||||
New migration `migrations/20260612000002_auth_users_is_external.sql`:
|
||||
|
||||
```sql
|
||||
-- Adds the is_external flag distinguishing storage-owning internal users
|
||||
-- from grant-only external users (magic-link / OIDC-only / future OCM).
|
||||
ALTER TABLE auth.users
|
||||
ADD COLUMN IF NOT EXISTS is_external BOOLEAN NOT NULL DEFAULT FALSE;
|
||||
|
||||
-- Partial index — most queries are "list internal users" or "list
|
||||
-- external users for GDPR purge", never an unfiltered scan.
|
||||
CREATE INDEX IF NOT EXISTS idx_users_is_external_login
|
||||
ON auth.users (is_external, last_login_at)
|
||||
WHERE is_external = TRUE;
|
||||
|
||||
-- Guard against accidental storage attribution to external users.
|
||||
ALTER TABLE auth.users
|
||||
ADD CONSTRAINT users_external_no_storage
|
||||
CHECK (NOT is_external OR storage_used_bytes = 0);
|
||||
```
|
||||
|
||||
User entity (`src/domain/entities/user.rs`):
|
||||
- Add `is_external: bool` field
|
||||
- Add getter `pub fn is_external(&self) -> bool`
|
||||
- Add factory `User::new_external(username, email, ...)` for the magic-link flow
|
||||
- Existing factories (`User::new(...)`) default `is_external = false`
|
||||
|
||||
The `Subject::External(uuid)` variant in `domain/services/authorization.rs` becomes redundant once external users live in `auth.users` and are addressed as `Subject::User(uuid)`. Deprecate it in a follow-up — out of scope here to avoid scope creep.
|
||||
|
||||
### Concrete hook implementations
|
||||
|
||||
**Each hook impl lives in the module of the service that owns the work**, matching the existing convention (`ThumbnailRefreshHook` lives in `src/infrastructure/services/thumbnail_service.rs`; `AudioMetadataService impl FileLifecycleHook` lives in `audio_metadata_service.rs`). There is **no centralised `lifecycle/` directory** — that would invert ownership and make "lifecycle" look like the owner of folder-creation policy when really the folder service owns it.
|
||||
|
||||
All four trait methods are explicit per impl; no-ops are `Ok(())` one-liners.
|
||||
|
||||
| Hook | Lives in | Responsibility |
|
||||
|---|---|---|
|
||||
| `HomeFolderLifecycleHook` | `src/application/services/folder_service.rs` (same module as `FolderService`) | Replaces the 4 eager `create_personal_folder` calls + the self-heal. `on_user_created` & `on_user_login`: if `!user.is_external()` and home folder missing, create "My Folder - {username}". `on_user_deleted` (AdminDelete): trash the home folder. `on_user_deleted` (GdprPurge): hard-delete folder + files. `on_user_logout`: `Ok(())`. |
|
||||
| `AuthzCacheLifecycleHook` | `src/infrastructure/services/pg_acl_engine.rs` (same module as the Moka cache it invalidates) | Wraps `Arc<PgAclEngine>`. `on_user_logout` & `on_user_deleted`: `engine.invalidate_user_groups_cache(user.id())` (new public method on the engine — one line `self.user_groups_cache.invalidate(id).await`). `on_user_created` & `on_user_login`: `Ok(())`. |
|
||||
| `AuditLifecycleHook` | `src/application/services/user_lifecycle_service.rs` (co-located with the dispatcher — cross-cutting, no domain owner) | All four events: `tracing::info!(target: "audit", event = "user.{created\|login\|logout\|deleted}", user_id = %user.id(), is_external = user.is_external(), ...)`. Stays one place for user-lifecycle audit. |
|
||||
| `SessionRevocationLifecycleHook` | The session-service module (e.g. `src/application/services/session_service.rs` or wherever `revoke_all_user_sessions` lives — verify at PR-write time) | `on_user_deleted`: explicit `session_storage.revoke_all_user_sessions(user.id(), tx)` for traceable audit (the FK CASCADE would do it but produces no per-session audit event). `on_user_logout` / `on_user_login` / `on_user_created`: `Ok(())`. |
|
||||
| `ExternalIdentityLifecycleHook` *(stubbed; populated by the magic-link PR later)* | A future external-identity service module (created with the magic-link PR sequence; for the stub PR, place it in `src/application/services/external_identity_service.rs` as a new module) | `on_user_login`: if `user.is_external()`, bump a `last_verified_at` column on a future `auth.user_external_identity` side-table. Other events: `Ok(())`. Lands as no-op now so the slot exists. |
|
||||
|
||||
**Why owner-located, not lifecycle-located**: it preserves the rule that "code about folders lives in the folder module". A future maintainer reading the folder service sees the lifecycle reactions next to the rest of the folder logic. It also makes a future workspace split (see "Crate-split note" at the end of this plan) almost free — each domain takes its hooks with it.
|
||||
|
||||
### Tips for hook implementors
|
||||
|
||||
These belong in the module-level docstring of `application/ports/user_lifecycle.rs` so the next maintainer reading the trait sees them in IDE hover.
|
||||
|
||||
1. **First-ever login detection.** `on_user_login` fires after credentials validate but **before** `user.last_login_at` is updated for this session. So `user.last_login_at().is_none()` is a reliable "this is the first login since account creation" signal. Use it for welcome emails, one-shot default-folder seeding, "complete your profile" prompts, etc.
|
||||
|
||||
2. **External-user short-circuit.** Every hook that provisions or manages user-owned resources (folders, calendars, address books) should start with `if user.is_external() { return Ok(()); }`. External users are grant-only; they don't own storage. The `CHECK (NOT is_external OR storage_used_bytes = 0)` constraint catches violations at the DB level.
|
||||
|
||||
3. **Idempotency is mandatory.** `on_user_login` fires on every successful authentication. A hook that creates a resource must first check whether the resource already exists. Examples: `HomeFolderLifecycleHook` does `if folder_exists(user_id) { return Ok(()); }` before calling `create_home_folder`. `AuthzCacheLifecycleHook::on_user_logout` is naturally idempotent (cache `invalidate` is a no-op on a missing key).
|
||||
|
||||
4. **First call after `is_external = TRUE → FALSE`.** When admin converts an external user to internal (`UPDATE auth.users SET is_external = FALSE`), the user's next login fires `on_user_login` with the new flag value. The home-folder hook sees `!is_external` and that no folder exists → creates it. No special "convert" event needed; idempotency carries the load.
|
||||
|
||||
5. **Per-session logout firing.** Disabling a user revokes N sessions in a loop. The dispatcher fires `on_user_logout` **once per session revoked**, all with `reason = AccountDisabled`. Hook implementors must accept N redundant calls (idempotent invalidation, idempotent audit) — do **not** assume "one logout = one user state change". The same applies to `revoke_all_user_sessions` on password change.
|
||||
|
||||
6. **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 (`tracing::error!`) that a subsequent investigation can identify the user and retry manually. Failure on `on_user_deleted` aborts the transaction — be conservative about returning Err there.
|
||||
|
||||
7. **No transaction handle on create/login/logout.** Only `on_user_deleted` gets `&mut Transaction` because deletion is the only event with hard atomic-with-DB requirements. Other hooks open their own connections / pools as needed. This keeps the trait surface minimal.
|
||||
|
||||
8. **Hook registration is at DI time.** Hook order is registration order; document this in the DI factory if you ever add an ordering dependency (e.g., HomeFolderLifecycleHook before any future hook that wants to write to that folder).
|
||||
|
||||
## Documentation
|
||||
|
||||
A new architecture page `docs/architecture/user-lifecycle.md` lands alongside the trait (in PR 1) and grows incrementally with each subsequent PR. Mirrors the structure of the existing `docs/architecture/file-and-blob-lifecycle.md` so readers familiar with the file-side pattern can navigate the user-side analog.
|
||||
|
||||
**Outline** (~150 lines):
|
||||
|
||||
1. **Context** — why hooks (replaces 4 scattered `create_personal_folder` calls + the self-heal; sets up the magic-link / external-user flow as a pluggable concern).
|
||||
2. **The trait** — full signature, the 4 events, `LogoutReason` / `DeletionMode` enums.
|
||||
3. **Dispatcher semantics** — per-event failure model (log-and-continue for created/login, fire-and-forget spawn for logout, abort-on-Err for deleted-in-transaction). Diagram.
|
||||
4. **Implementation tips** (verbatim from the "Tips for hook implementors" section of this plan — first-login detection via `last_login_at.is_none()`, idempotency, external-user short-circuit, per-session logout firing, …).
|
||||
5. **Owner-located convention** — explains why hooks live with their service module rather than a centralised `lifecycle/` directory, with the FileLifecycleHook precedent.
|
||||
6. **Concrete hooks shipped today** — table of `HomeFolderLifecycleHook` / `AuthzCacheLifecycleHook` / `AuditLifecycleHook` / `SessionRevocationLifecycleHook` / `ExternalIdentityLifecycleHook` (stub) with one-line summaries and where each lives.
|
||||
7. **Recommended future triggers** — the "future triggers" table from this plan (`on_user_password_changed`, `on_user_role_changed`, etc.) so v2 contributors see the design door.
|
||||
8. **File map** — same shape as the file map at the bottom of `rebac-authorization.md`.
|
||||
|
||||
**VitePress sidebar update** in `docs/.vitepress/config.mts`. The Architecture section already lists "File and Blob lifecycle" (line 104); add immediately after:
|
||||
|
||||
```ts
|
||||
{ text: "User lifecycle", link: "/architecture/user-lifecycle" },
|
||||
```
|
||||
|
||||
**Incidental fix while we're in the file**: `docs/architecture/rebac-authorization.md` (created in a previous session) is missing from the sidebar. Add it in the same edit:
|
||||
|
||||
```ts
|
||||
{ text: "ReBAC Authorization", link: "/architecture/rebac-authorization" },
|
||||
```
|
||||
|
||||
Place it logically — probably right before "Share Integration" since shares depend on ReBAC concepts.
|
||||
|
||||
**Per-PR doc growth**:
|
||||
- PR 1: sections 1, 2, 3, 4, 5 (trait, dispatcher, conventions) + the AuditLifecycleHook entry in section 6
|
||||
- PR 2: short subsection in section 4 explaining the `is_external` short-circuit pattern
|
||||
- PR 3: HomeFolderLifecycleHook entry in section 6, plus a worked example "what happens when a brand-new user logs in"
|
||||
- PR 4: AuthzCacheLifecycleHook + SessionRevocationLifecycleHook entries, plus the `DeletionMode` section
|
||||
- PR 5: ExternalIdentityLifecycleHook entry + a "this is a placeholder for the upcoming magic-link feature" note
|
||||
|
||||
Sidebar entry lands in PR 1; subsequent PRs only edit the markdown content.
|
||||
|
||||
## Migration sequencing (5 PRs)
|
||||
|
||||
**PR 1: trait + dispatcher + audit hook only.**
|
||||
Lands the trait at `application/ports/user_lifecycle.rs`, the dispatcher at `application/services/user_lifecycle_service.rs`, and `AuditLifecycleHook` as the lone registered hook. Wires `dispatch_created` / `dispatch_login` / `dispatch_logout` / `dispatch_deleted` into the existing 4 auth code paths (no behaviour change for users; only audit log gains four new event types). Zero risk; validates plumbing.
|
||||
|
||||
**PR 2: `is_external` column + entity field.**
|
||||
Migration `20260612000002_auth_users_is_external.sql`, `User::is_external` getter, factory variant, DTO field. All existing rows have `is_external = FALSE` from the column default; no breaking changes. New `POST /api/admin/users` accepts `is_external` (default `false`).
|
||||
|
||||
**PR 3: `HomeFolderLifecycleHook`.**
|
||||
Register the hook. Remove the 4 eager `create_personal_folder` calls in `auth_application_service.rs:283 / 360 / 832 / 1277`. Remove the self-heal at `folder_service.rs:350-365`. Existing test suite should pass — folder still gets created, just by the hook now. The Hurl suite at `tests/api/run.sh` is the canary.
|
||||
|
||||
**PR 4: `AuthzCacheLifecycleHook` + `SessionRevocationLifecycleHook` + `on_user_deleted` policy.**
|
||||
Adds the `pub fn invalidate_user_groups_cache(&self, id: Uuid)` method on `PgAclEngine`. Wires the two hooks. Adds `DeletionMode` switching to `HomeFolderLifecycleHook::on_user_deleted` (trash vs hard-delete). Admin-delete endpoint now passes `mode = AdminDelete`; a (future) GDPR sweeper passes `GdprPurge`.
|
||||
|
||||
**PR 5: `ExternalIdentityLifecycleHook` stub.**
|
||||
Empty no-op hook landed in advance of the magic-link feature so the registration slot exists in DI. Populated in the magic-link PR sequence later.
|
||||
|
||||
After PR 3, the cleanup of `create_personal_folder` from `auth_application_service.rs` is complete and the service stops importing `FolderService` for that purpose.
|
||||
|
||||
## Recommended future triggers (DON'T ship now)
|
||||
|
||||
These are the events users / consumers will eventually want. Each has a "what would make us add it" rationale; absent that, **don't add the method to the trait** — every method adds a no-op to every hook impl forever.
|
||||
|
||||
| Future event | Why someone might want it | What would force adding it |
|
||||
|---|---|---|
|
||||
| `on_user_password_changed` | Notify the user via email; invalidate any cached credentials; trigger TOTP re-enrolment | A real per-user notification service. Today the password-change handler explicitly calls `revoke_all_user_sessions` which fires `on_user_logout(PasswordChanged)` for each session — sufficient for current consumers. |
|
||||
| `on_user_role_changed` | Admin grants admin role → audit + maybe send "you're now an admin" email; admin demotion → revoke admin-only sessions | A multi-role system (today only `admin` / `user` exist). Currently a one-liner audit log at the admin handler covers it. |
|
||||
| `on_user_email_changed` | External users: re-verify the new email via magic-link before trusting it; internal: notify both old and new addresses; update OIDC mapping | When external users start changing their email. Today email is immutable in the API. |
|
||||
| `on_user_username_changed` | Update display names in audit logs that captured the old username; rename the home folder if it embeds the username | When username changes ship. Today username is immutable. |
|
||||
| `on_user_avatar_changed` | Bust thumbnail caches downstream; sync to federated servers (OCM) | When OCM federation ships and remote partners need to learn about avatar changes. Today no downstream consumer. |
|
||||
| `on_user_quota_changed` | Future per-service quota counters react to admin-changed limits | When quota becomes per-service (today it's a single global counter per user). |
|
||||
| `on_user_disabled` / `on_user_enabled` | Audit-distinguishable state changes; pause per-user scheduled jobs | When per-user scheduled jobs land. Today `on_user_logout(AccountDisabled)` covers the only real consumer (sessions). Re-enable triggers `on_user_login` naturally. |
|
||||
| `on_user_external_to_internal_converted` | Welcome email; provision the catalog of internal-only resources at conversion time rather than on next login | If admins routinely promote external users and the next-login lag is unacceptable. Today the idempotent `on_user_login` recheck handles conversion fine. |
|
||||
| `on_user_oidc_linked` / `on_user_oidc_unlinked` | Audit; sync remote profile data | When users can link/unlink OIDC identities post-creation. Today OIDC linkage is fixed at user-creation time. |
|
||||
| `on_user_2fa_enabled` / `on_user_2fa_disabled` | Audit; force re-login of other sessions | When 2FA ships. |
|
||||
|
||||
**Rule of thumb for adding any of these later**: add the trait method with a default `Ok(())` body so existing hooks don't need to declare it explicitly (one-time exception to the "no defaults" rule, paid forever after by IDE-discoverable docstrings on the new method). Make sure the docstring states whether it's await-or-spawn semantics and whether failure aborts the parent operation.
|
||||
|
||||
## Critical files
|
||||
|
||||
**New files** (per PR):
|
||||
|
||||
- PR 1: `src/application/ports/user_lifecycle.rs` (trait + `LogoutReason` + `DeletionMode` enums), `src/application/services/user_lifecycle_service.rs` (dispatcher + `AuditLifecycleHook` co-located inside), `docs/architecture/user-lifecycle.md` (architecture doc, outline above)
|
||||
- PR 2: `migrations/20260612000002_auth_users_is_external.sql`
|
||||
- PR 3: No new files — `HomeFolderLifecycleHook` is added as a new `impl UserLifecycleHook for ...` block inside the **existing** `src/application/services/folder_service.rs` (or a sibling `folder_lifecycle.rs` if folder_service.rs gets too large; verify line count at PR-write time)
|
||||
- PR 4: No new files — `AuthzCacheLifecycleHook` added inside the existing `src/infrastructure/services/pg_acl_engine.rs`; `SessionRevocationLifecycleHook` added inside the session-service module
|
||||
- PR 5: `src/application/services/external_identity_service.rs` (new module hosting the stub hook)
|
||||
|
||||
**Modified files**:
|
||||
|
||||
- `src/domain/entities/user.rs` (PR 2): add `is_external` field + getter + factory
|
||||
- `src/application/services/auth_application_service.rs` (PRs 1, 3): wire dispatcher into the 4 create / 3 login / 2 logout / 1 delete sites; remove the 4 eager folder-creation calls in PR 3
|
||||
- `src/application/services/folder_service.rs` (PR 3): add the `HomeFolderLifecycleHook` impl; remove the self-heal at lines 350-365 (now handled by the hook on next login)
|
||||
- `src/infrastructure/services/pg_acl_engine.rs` (PR 4): add `pub fn invalidate_user_groups_cache(&self, id: Uuid)` exposing `user_groups_cache.invalidate(id)`; add the `AuthzCacheLifecycleHook` impl
|
||||
- `src/common/di.rs` (PRs 1, 3, 4, 5): construct the `UserLifecycleService` with builder chain, mirror the `FileLifecycleService` registration pattern at lines 264-301
|
||||
- `src/application/dtos/user_dto.rs` (PR 2): add `is_external: bool` field
|
||||
- `src/interfaces/api/handlers/admin_handler.rs` (PR 2): accept `is_external` in `POST /api/admin/users` request body
|
||||
- `docs/.vitepress/config.mts` (PR 1): add "User lifecycle" entry to the Architecture sidebar (line ~104). Also incidentally add the missing "ReBAC Authorization" entry that pre-dated this work
|
||||
- `docs/architecture/user-lifecycle.md` (PRs 2, 3, 4, 5): grow the doc incrementally as each hook lands — `is_external` short-circuit note in PR 2, HomeFolderLifecycleHook section in PR 3, etc.
|
||||
|
||||
**Existing patterns to reuse**:
|
||||
|
||||
- Hook trait + dispatcher pattern: `src/application/ports/file_lifecycle.rs` + `src/application/services/file_lifecycle_service.rs` (the closest analog)
|
||||
- DI builder chain: `src/common/di.rs:264-301` (FileLifecycleService construction)
|
||||
- Audit tracing convention: `target: "audit"` events emitted by `src/application/services/subject_group_service.rs::create / rename / delete / add_member / remove_member`
|
||||
- Per-cache invalidation method on engine: model after how `user_groups_cache` is accessed today in `src/infrastructure/services/pg_acl_engine.rs::expand_user`
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cargo fmt --all
|
||||
cargo clippy --all-features --all-targets -- -D warnings
|
||||
cargo test --workspace
|
||||
biome check --fix static/js/
|
||||
tsc -p jsconfig.json --noEmit
|
||||
```
|
||||
|
||||
After PR 1 (smoke test the plumbing):
|
||||
|
||||
1. `cargo run`, then via the UI: register a new user, log in, log out, delete via admin.
|
||||
2. `journalctl -t oxicloud | grep "target=user_lifecycle"` (or `RUST_LOG=user_lifecycle=info`) — exactly one event line per action.
|
||||
|
||||
After PR 3 (the migration of folder creation):
|
||||
|
||||
1. Hurl suite: `bash tests/api/run.sh` — all 13 test files still pass. `permissions.hurl` is the most relevant (it creates `bob` and verifies the home folder).
|
||||
2. Manual: register a fresh user via the UI → home folder appears in the file list immediately. Then drop the home folder via SQL (`DELETE FROM storage.folders WHERE user_id = $1`), log out, log back in → folder reappears (the safety-net path).
|
||||
3. Confirm via tracing that `dispatch_login` actually ran for an existing user whose folder was already there → no folder creation attempt, no error, just one `on_user_login` audit event.
|
||||
|
||||
After PR 4:
|
||||
|
||||
1. Authz cache: create a user, log them in, log them out. Inspect `RUST_LOG=oxicloud::infrastructure::services::pg_acl_engine=debug` — cache entry should be invalidated immediately on logout, not after 30s TTL.
|
||||
2. User deletion: admin-deletes a user → verify (via audit log) that `on_user_deleted` ran inside the transaction and all sessions were revoked before the `auth.users` row vanished.
|
||||
|
||||
After PR 5: no functional change; just confirm `external_identity_hook.rs` compiles and registers in DI as a no-op.
|
||||
|
||||
## Out of scope (do NOT bundle into these 5 PRs)
|
||||
|
||||
- **The magic-link external-user flow itself.** Lands in a later sequence; this plan only prepares the schema (`is_external`) and the hook slot (`ExternalIdentityLifecycleHook` stub).
|
||||
- **Removing `Subject::External` from the domain.** It's currently unused; the cleanup is a separate small PR after PR 2 demonstrates that external users live in `auth.users`.
|
||||
- **GDPR sweeper.** The `DeletionMode::GdprPurge` variant exists in PR 4 but no sweeper is wired up — admin-delete uses `AdminDelete`. A scheduled sweeper is its own future work.
|
||||
- **Moving the `active` flag transitions through a hook.** PR 4's `on_user_logout(AccountDisabled)` covers it; no `on_user_disabled` method is added (see "future triggers" section).
|
||||
- **Side-table for OIDC/OCM provenance** (`auth.user_external_identity`). Lands with the magic-link PR; not needed for `is_external` alone.
|
||||
|
||||
## Crate-split note (forward-looking, NOT in this work)
|
||||
|
||||
OxiCloud is currently a single Rust crate (~50 kLOC). The lifecycle-hook restructuring above intentionally aligns with the natural domain boundaries (each hook lives with its service) so that a future workspace split is incremental rather than a rewrite. **Not on the table for this work, but worth recording the intended split axis** so subsequent refactors don't paint into a corner:
|
||||
|
||||
- **Split by domain bounded context, NOT by hexagonal layer.** Layered split (`oxicloud-domain` / `oxicloud-application` / etc.) makes the common case painful: adding a field to an entity touches 4 crates. Domain split (`oxicloud-files`, `oxicloud-auth`, `oxicloud-rebac`, …) makes the common case stay in one crate.
|
||||
- Target shape, illustrative:
|
||||
```
|
||||
oxicloud-kernel ← errors, DI primitives, common port traits (incl. UserLifecycleHook)
|
||||
oxicloud-auth ← users, sessions, OIDC, app passwords; dispatcher lives here
|
||||
oxicloud-rebac ← groups, grants, engine; registers AuthzCacheLifecycleHook
|
||||
oxicloud-files ← files, folders, blobs, dedup, thumbnails; registers HomeFolderLifecycleHook
|
||||
oxicloud-sharing ← shares, magic-link, external identity
|
||||
oxicloud-calendar ← caldav
|
||||
oxicloud-contacts ← carddav
|
||||
oxicloud-server ← Axum wire-up, the binary, DI composition root
|
||||
```
|
||||
Each domain crate is internally layered. Cross-crate communication goes through `oxicloud-kernel` port traits. The DI factory at `oxicloud-server` is where crates compose into the full application.
|
||||
|
||||
- **What today's lifecycle work buys for that future split**: zero rework on hook locations. `HomeFolderLifecycleHook` already lives next to `FolderService`, so it moves with `oxicloud-files`. `AuthzCacheLifecycleHook` moves with `oxicloud-rebac`. The dispatcher in `oxicloud-auth` only knows the trait, never the impls.
|
||||
|
||||
- **Cheap things to do now that help the future split**, but are NOT bundled here:
|
||||
- Tighten visibility: prefer `pub(crate)` over `pub` wherever a type isn't intentionally part of the public surface. Catches accidental cross-module reaches at compile time.
|
||||
- Per-domain port traits: today `application/ports/file_lifecycle.rs` is a file-concern port living in the layer dir; eventually it should live under the files-domain module. Refactor when adjacent ports are touched, not as a one-shot move.
|
||||
- Avoid expanding `src/common/` — it tends to absorb anything-shared and become hard to split later.
|
||||
|
||||
These are convention recommendations for future PRs, not work items for this plan.
|
||||
Reference in New Issue
Block a user