From 9e9898cea763d76630c1d12db93df12ff9de3132 Mon Sep 17 00:00:00 2001 From: Linus Fischer Date: Sat, 3 Oct 2026 16:22:38 +0000 Subject: [PATCH 1/7] fix(startup): match the documented defaults and find migrations next to the binary - Default to port 14123 and to running migrations on startup, as the README and Dockerfile already state. An unset DLA_DB_AUTO_MIGRATE used to fall through to DB.init's `false`. - In the compiled binary, `import.meta.dir` is Bun's virtual /$bunfs/root, and `path.join` glued even an absolute DLA_DB_MIGRATION_DIR onto it, so the Docker image couldn't find its migrations. Absolute paths are now used as they are, and relative ones resolve next to the executable. --- CLAUDE.md | 2 +- README.md | 2 +- src/db/index.ts | 10 ++++++---- src/index.ts | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3d096f5..ff4cf87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,7 +77,7 @@ src/ - **API versioning**: Routes live under `src/api/versions/v{n}/routes/`. Each route module has an `index.ts` (router) and `model.ts` (Zod schemas). - **OpenAPI specs**: Use `hono-openapi` decorators on route handlers via `APIRouteSpec`/`APIResponseSpec` helpers in `specHelpers.ts` (`summary`, `description`, `tags`). Tags come from `DOCS_TAGS` in `versions/v1/docs/index.ts`. When adding a new tag, also register it in the `tags` array **and** an `x-tagGroups` group in `versions/v1/index.ts`, or it renders orphaned in Scalar. The spec is served at `/docs/v1/openapi` and the Scalar UI at `/docs/v1` (both mounted in `api/index.ts`, gated by `DLA_DISABLE_DOCS`). -- **Database**: Schema files are per-dialect in `src/db/schema/`. Migrations managed via Drizzle Kit, schema ones with `bun db:sqlite:generate` and data ones with `drizzle-kit generate --custom --name=…`; both are plain SQL under `drizzle/migrations/sqlite/` and run from `DB.init()` when `DLA_DB_AUTO_MIGRATE` is set. A data migration is written to be idempotent (guard with `WHERE NOT EXISTS`) and can only use what SQL can see — the mail account connection data is encrypted, so an address or credential is not reachable from a migration. +- **Database**: Schema files are per-dialect in `src/db/schema/`. Migrations managed via Drizzle Kit, schema ones with `bun db:sqlite:generate` and data ones with `drizzle-kit generate --custom --name=…`; both are plain SQL under `drizzle/migrations/sqlite/` and run from `DB.init()` unless `DLA_DB_AUTO_MIGRATE=false`. A data migration is written to be idempotent (guard with `WHERE NOT EXISTS`) and can only use what SQL can see — the mail account connection data is encrypted, so an address or credential is not reachable from a migration. - **Auth**: JWT-based auth via `authHandler.ts`. Middleware in `src/api/versions/v1/middleware/auth.ts`. - **Validation**: Zod schemas in `model.ts` files, validated via `@hono/standard-validator`. - **Tests**: Integration-heavy. `bunfig.toml` preloads `tests/helpers/preload.ts`, which builds the app with `API.init()` but never binds a port — requests go through `API.getApp().request()` in-process, so the suite runs while a dev server is up. Mock IMAP servers do listen: `11143` for the shared one, `11144`–`11148` for per-test servers. diff --git a/README.md b/README.md index 130d7ac..17c66cb 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ All configuration is environment-based (see [`example.env`](./example.env)): | `DLA_ENCRYPTION_KEY` | **Required. 32-character** key for credential encryption | — | | `DLA_DB_CONNECTION_URL` | Database connection string / path | `./data/db.sqlite` | | `DLA_DB_AUTO_MIGRATE` | Run migrations on startup | `true` | -| `DLA_DB_MIGRATION_DIR` | Dir with the migrations files | `has to be set manually` | +| `DLA_DB_MIGRATION_DIR` | Dir with the migrations files. The compiled binary resolves a relative path next to the executable, not the working directory | `has to be set manually` | | `DLA_LOG_DIR` | Log output directory | `./data/logs` | | `DLA_CONFIG_BASE_DIR` | Config base directory | `./config` | | `DLA_SMTP_HOST` | Outbound SMTP host for system mail (e.g. password-reset emails) | — | diff --git a/src/db/index.ts b/src/db/index.ts index f8f1e70..740e0a4 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -6,7 +6,7 @@ import { Logger } from '../utils/logger'; import { ConfigHandler } from '../utils/config'; import { migrate } from 'drizzle-orm/bun-sqlite/migrator'; import { mkdir as fs_mkdir } from 'fs/promises'; -import { dirname as path_dirname, join as path_join } from "path"; +import { dirname as path_dirname, isAbsolute as path_isAbsolute, resolve as path_resolve } from "path"; export class DB { @@ -25,9 +25,11 @@ export class DB { if (autoMigrate) { Logger.info("Running database migrations..."); - if (Bun?.isStandaloneExecutable) { - migrationsFolder = path_join(import.meta.dir, migrationsFolder); - } + // In a compiled binary, import.meta.dir is Bun's virtual /$bunfs/root, but the + // migrations ship on disk next to the executable — resolve relative paths there. + if (Bun?.isStandaloneExecutable && !path_isAbsolute(migrationsFolder)) { + migrationsFolder = path_resolve(path_dirname(process.execPath), migrationsFolder); + } await migrate(this.db, { migrationsFolder }); diff --git a/src/index.ts b/src/index.ts index 1f21711..257c2d7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,7 +26,7 @@ export class Main { await DB.init( config.DLA_DB_CONNECTION_URL ?? "./data/db.sqlite", - config.DLA_DB_AUTO_MIGRATE, + config.DLA_DB_AUTO_MIGRATE ?? true, config.DLA_CONFIG_BASE_DIR ?? "./config", config.DLA_DB_MIGRATION_DIR ); @@ -43,7 +43,7 @@ export class Main { await API.init([config.DLA_APP_URL || "https://api.delivr.local"], config.DLA_DISABLE_DOCS === true); await API.start( - parseInt(config.DLA_API_PORT ?? "12151"), + parseInt(config.DLA_API_PORT ?? "14123"), config.DLA_API_HOST ?? "::" ); From b12adafa7cecb66e6d6b8374fcc77a3e22c465a6 Mon Sep 17 00:00:00 2001 From: Linus Fischer Date: Sat, 3 Oct 2026 16:22:38 +0000 Subject: [PATCH 2/7] fix(docker): make the full-stack compose file valid - `depends_on` named a service `api` that doesn't exist (it is `delivr-api`), so `docker compose` refused the file. - The web image reads DELIVR_* only while it is built, so set the URLs and the signup switch through Nuxt's runtime NUXT_PUBLIC_* overrides. --- CLAUDE.md | 2 +- docker/docker-compose.prodlike.yml | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ff4cf87..2ac60d7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,7 +83,7 @@ src/ - **Tests**: Integration-heavy. `bunfig.toml` preloads `tests/helpers/preload.ts`, which builds the app with `API.init()` but never binds a port — requests go through `API.getApp().request()` in-process, so the suite runs while a dev server is up. Mock IMAP servers do listen: `11143` for the shared one, `11144`–`11148` for per-test servers. - **Config**: Environment-based config in `src/utils/config.ts`. See `example.env` for required vars. - **Crypto**: ECC-based encryption/signing utilities in `src/utils/crypto/`. -- **Docker**: `docker/Dockerfile` packages a binary compiled beforehand (`bun run compile linux-x64-baseline --no-version-tag`, as CI does) together with `drizzle/migrations`, which the binary reads from disk. A relative `DLA_DB_MIGRATION_DIR` therefore has to be resolved next to the executable (`process.execPath`), not against `import.meta.dir`, which is Bun's virtual `/$bunfs/root` in a compiled binary. `docker/docker-compose.yml` runs the full stack (API + Delivr-Web) from the ghcr.io images; it requires *public* URLs, since the web client's SSR server calls the API through the same `DELIVR_API_URL` as the browser. +- **Docker**: `docker/Dockerfile` packages a binary compiled beforehand (`bun run compile linux-x64-baseline --no-version-tag`, as CI does) together with `drizzle/migrations`, which the binary reads from disk. A relative `DLA_DB_MIGRATION_DIR` therefore has to be resolved next to the executable (`process.execPath`), not against `import.meta.dir`, which is Bun's virtual `/$bunfs/root` in a compiled binary. `docker/docker-compose.yml` runs the API alone and `docker/docker-compose.prodlike.yml` the full stack (API + Delivr-Web) from the ghcr.io images. The web image is configured at runtime through `NUXT_PUBLIC_*` variables (`DELIVR_*` is only read at build time), and it requires *public* URLs, since the web client's SSR server calls the API through the same `NUXT_PUBLIC_API_URL` as the browser. ## Architecture Notes diff --git a/docker/docker-compose.prodlike.yml b/docker/docker-compose.prodlike.yml index d2a383d..5c631d5 100644 --- a/docker/docker-compose.prodlike.yml +++ b/docker/docker-compose.prodlike.yml @@ -36,11 +36,13 @@ services: ports: - "14128:14128" environment: - DELIVR_API_URL: https://api.delivr.example.com/v1 - DELIVR_APP_URL: https://delivr.example.com + # The image is built once and configured at runtime through Nuxt's NUXT_PUBLIC_* + # overrides; DELIVR_* is only read while the image is built. + NUXT_PUBLIC_API_URL: https://api.delivr.example.com/v1 + NUXT_PUBLIC_APP_URL: https://delivr.example.com + + NUXT_PUBLIC_IS_SIGNUP_ENABLED: false - DELIVR_ENABLE_SIGNUP: false - depends_on: - api: + delivr-api: condition: service_healthy From ce84f4e6534667434dea078ec8e70219e2d4feef Mon Sep 17 00:00:00 2001 From: Linus Fischer Date: Sat, 3 Oct 2026 16:22:38 +0000 Subject: [PATCH 3/7] fix(auth): rate-limit logins by the real client address Bun's Request has no `remoteAddr`, so every client was keyed as "unknown" and the per-address limit acted as a second per-username limit. Read the socket address with `getConnInfo` instead, or the last X-Forwarded-For entry when the new DLA_TRUST_PROXY is set. --- CLAUDE.md | 1 + README.md | 1 + example.env | 3 + src/api/versions/v1/routes/auth/index.ts | 16 +++++- src/utils/config.ts | 1 + tests/api.routes.test.ts | 70 ++++++++++++++++++++++++ tests/helpers/preload.ts | 1 + 7 files changed, 90 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2ac60d7..20a9ad4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,6 +84,7 @@ src/ - **Config**: Environment-based config in `src/utils/config.ts`. See `example.env` for required vars. - **Crypto**: ECC-based encryption/signing utilities in `src/utils/crypto/`. - **Docker**: `docker/Dockerfile` packages a binary compiled beforehand (`bun run compile linux-x64-baseline --no-version-tag`, as CI does) together with `drizzle/migrations`, which the binary reads from disk. A relative `DLA_DB_MIGRATION_DIR` therefore has to be resolved next to the executable (`process.execPath`), not against `import.meta.dir`, which is Bun's virtual `/$bunfs/root` in a compiled binary. `docker/docker-compose.yml` runs the API alone and `docker/docker-compose.prodlike.yml` the full stack (API + Delivr-Web) from the ghcr.io images. The web image is configured at runtime through `NUXT_PUBLIC_*` variables (`DELIVR_*` is only read at build time), and it requires *public* URLs, since the web client's SSR server calls the API through the same `NUXT_PUBLIC_API_URL` as the browser. +- **Client addresses**: login rate limiting keys on the socket address (`getConnInfo` from `hono/bun`), or on the last `X-Forwarded-For` entry when `DLA_TRUST_PROXY=true`. In-process test requests have no socket and fall back to `unknown`, so one test serves the app on an ephemeral port to cover the socket path. ## Architecture Notes diff --git a/README.md b/README.md index 17c66cb..9025161 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ All configuration is environment-based (see [`example.env`](./example.env)): | `DLA_APP_URL` | **Required.** URL of the Delivr web client | — | | `DLA_API_HOST` | Bind address | `::` | | `DLA_API_PORT` | Listen port | `14123` | +| `DLA_TRUST_PROXY` | Read the client address from `X-Forwarded-For` for login rate limiting. Enable it only when the API is reachable solely through your reverse proxy | `false` | | `DLA_DISABLE_DOCS` | Disable the Scalar API reference | `false` | | `DLA_MAX_ATTACHMENT_SIZE_MB` | Maximum combined attachment size per composed mail, in MB, measured before encoding | `25` | | `DLA_ENCRYPTION_KEY` | **Required. 32-character** key for credential encryption | — | diff --git a/example.env b/example.env index c2862a2..b6263a7 100644 --- a/example.env +++ b/example.env @@ -4,6 +4,9 @@ DLA_APP_URL=https://app.delivr.is-on.net DLA_API_HOST=:: DLA_API_PORT=14123 +# Set to true when the API is only reachable through your reverse proxy, so login +# rate limiting sees each client's address (from X-Forwarded-For) instead of the proxy's. +DLA_TRUST_PROXY=false DLA_DISABLE_DOCS=false DLA_ENCRYPTION_KEY=your-32-character-encryption-key diff --git a/src/api/versions/v1/routes/auth/index.ts b/src/api/versions/v1/routes/auth/index.ts index d18a7ca..b6d214f 100644 --- a/src/api/versions/v1/routes/auth/index.ts +++ b/src/api/versions/v1/routes/auth/index.ts @@ -1,5 +1,6 @@ import { Hono } from "hono"; import type { Context } from "hono"; +import { getConnInfo } from "hono/bun"; import { AuthModel } from './model' import { validator as zValidator } from "hono-openapi"; import { DB } from "../../../../../db"; @@ -11,6 +12,7 @@ import { APIResponseSpec, APIRouteSpec } from "../../../../utils/specHelpers"; import { router as resetPasswordRouter } from "./reset-password"; import { DOCS_TAGS } from "../../docs"; import { Logger } from "../../../../../utils/logger"; +import { ConfigHandler } from "../../../../../utils/config"; // Dummy bcrypt hash for timing-normalized login failures — prevents username enumeration // Generated once at module load so it's a valid, cost-equivalent hash @@ -37,9 +39,17 @@ const LOGIN_CLEANUP_INTERVAL = setInterval(() => { LOGIN_CLEANUP_INTERVAL.unref(); function getClientId(c: Context) { - // @ts-ignore bun/hono provides a native request with connection info - const remote = (c.req.raw as any)?.remoteAddr?.hostname; - return remote || "unknown"; + if (ConfigHandler.getConfig()?.DLA_TRUST_PROXY) { + // The reverse proxy appends the address it saw, so the last entry is the one a client can't forge. + const forwarded = c.req.header("x-forwarded-for")?.split(",").at(-1)?.trim(); + if (forwarded) return forwarded; + } + try { + return getConnInfo(c).remote.address || "unknown"; + } catch { + // In-process requests (e.g. tests) have no socket to read the address from. + return "unknown"; + } } function getLoginAttemptKey(clientId: string, username: string) { diff --git a/src/utils/config.ts b/src/utils/config.ts index b8a7cf2..47f7add 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -111,6 +111,7 @@ export class ConfigHandler { .add("DLA_API_HOST", false) .add("DLA_API_PORT", false) + .add("DLA_TRUST_PROXY", false, [true, false]) .add("DLA_DISABLE_DOCS", false, [true, false]) .add("DLA_ENCRYPTION_KEY", true) diff --git a/tests/api.routes.test.ts b/tests/api.routes.test.ts index 58e153e..604db2a 100644 --- a/tests/api.routes.test.ts +++ b/tests/api.routes.test.ts @@ -1844,6 +1844,76 @@ describe("Mail Identity Backfill Migration", async () => { }); }); +describe("Login rate limiting", async () => { + + function attemptLogin(username: string, password: string, clientIP: string, expectedCode: number) { + return makeAPIRequest("/v1/auth/login", { + method: "POST", + body: { username, password }, + additionalOptions: { + headers: { "x-forwarded-for": clientIP } + } + }, expectedCode); + } + + // Five failed attempts per client and username are allowed; the sixth is refused. + async function exhaustAttempts(username: string, clientIP: string) { + for (let i = 0; i < 5; i++) { + await attemptLogin(username, "wrong-password", clientIP, 401); + } + await attemptLogin(username, "wrong-password", clientIP, 429); + } + + test("ignores X-Forwarded-For unless DLA_TRUST_PROXY is set", async () => { + const user = await seedUser("user", {}, "RateLimitP@ss1"); + + await exhaustAttempts(user.username, "198.51.100.1"); + + // A forged header must not get a client around the limit. + await attemptLogin(user.username, user.password, "198.51.100.2", 429); + }); + + test("limits each forwarded client separately with DLA_TRUST_PROXY", async () => { + const config = ConfigHandler.getConfig()!; + config.DLA_TRUST_PROXY = true; + + try { + const user = await seedUser("user", {}, "RateLimitP@ss1"); + + await exhaustAttempts(user.username, "198.51.100.1"); + + // Another client can still sign in to the same account. + await attemptLogin(user.username, user.password, "198.51.100.2", 200); + } finally { + config.DLA_TRUST_PROXY = false; + } + }); + + test("keys on the socket address of a real connection", async () => { + const user = await seedUser("user", {}, "RateLimitP@ss1"); + + // In-process requests have no socket, so serve the app for real on an ephemeral port. + const server = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch: API.getApp().fetch }); + try { + const loginOverSocket = (password: string) => fetch(`http://127.0.0.1:${server.port}/v1/auth/login`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ username: user.username, password }) + }).then(response => response.status); + + for (let i = 0; i < 5; i++) { + expect(await loginOverSocket("wrong-password")).toBe(401); + } + expect(await loginOverSocket("wrong-password")).toBe(429); + } finally { + server.stop(true); + } + + // Had the socket client been counted as "unknown", the in-process one would be locked out too. + await attemptLogin(user.username, user.password, "198.51.100.3", 200); + }); +}); + describe("Mail Mailbox Routes", async () => { let mailIdentityTestUser: SeededUser; diff --git a/tests/helpers/preload.ts b/tests/helpers/preload.ts index aca6b93..5c29752 100644 --- a/tests/helpers/preload.ts +++ b/tests/helpers/preload.ts @@ -21,6 +21,7 @@ function setTestEnv(rootDir: string) { DLA_API_HOST: "::", DLA_API_PORT: "1", + DLA_TRUST_PROXY: false, DLA_DISABLE_DOCS: true, DLA_ENCRYPTION_KEY: "67e3d03dc88682553deed5fa4484bd80a500783850efbb49f6912ad0935eedeb", From edc39dc60a994ca135021fa0ca76c1bae6ced26d Mon Sep 17 00:00:00 2001 From: Linus Fischer Date: Sat, 3 Oct 2026 16:22:38 +0000 Subject: [PATCH 4/7] fix(users): delete a user's mail accounts and settings with the user DELETE /admin/users/:userId removed only the user row, its sessions, API keys and password resets. SQLite doesn't enforce the foreign keys, so the user's mail accounts (with their encrypted credentials), identities, special-use mappings and preferences stayed behind. - AccountDeletionService now does every user and mail account deletion. It is synchronous, because the bun-sqlite driver commits an async transaction callback at its first `await`, which would leave nothing to roll back. - The admin route closes the removed accounts' pooled IMAP connections. MailClientsCache.deleteClient no longer throws when the logout fails. - Migration 0015 purges data already orphaned this way. --- CLAUDE.md | 2 + .../sqlite/0015_purge_orphaned_user_data.sql | 28 + .../migrations/sqlite/meta/0015_snapshot.json | 626 ++++++++++++++++++ drizzle/migrations/sqlite/meta/_journal.json | 7 + .../utils/services/accountDeletionService.ts | 68 ++ src/api/versions/v1/routes/account/index.ts | 20 +- .../versions/v1/routes/admin/users/index.ts | 30 +- .../versions/v1/routes/mail-accounts/index.ts | 15 +- src/utils/mails/mail-clients-cache.ts | 10 +- tests/api.routes.test.ts | 170 ++++- 10 files changed, 932 insertions(+), 44 deletions(-) create mode 100644 drizzle/migrations/sqlite/0015_purge_orphaned_user_data.sql create mode 100644 drizzle/migrations/sqlite/meta/0015_snapshot.json create mode 100644 src/api/utils/services/accountDeletionService.ts diff --git a/CLAUDE.md b/CLAUDE.md index 20a9ad4..8553326 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,6 +85,8 @@ src/ - **Crypto**: ECC-based encryption/signing utilities in `src/utils/crypto/`. - **Docker**: `docker/Dockerfile` packages a binary compiled beforehand (`bun run compile linux-x64-baseline --no-version-tag`, as CI does) together with `drizzle/migrations`, which the binary reads from disk. A relative `DLA_DB_MIGRATION_DIR` therefore has to be resolved next to the executable (`process.execPath`), not against `import.meta.dir`, which is Bun's virtual `/$bunfs/root` in a compiled binary. `docker/docker-compose.yml` runs the API alone and `docker/docker-compose.prodlike.yml` the full stack (API + Delivr-Web) from the ghcr.io images. The web image is configured at runtime through `NUXT_PUBLIC_*` variables (`DELIVR_*` is only read at build time), and it requires *public* URLs, since the web client's SSR server calls the API through the same `NUXT_PUBLIC_API_URL` as the browser. - **Client addresses**: login rate limiting keys on the socket address (`getConnInfo` from `hono/bun`), or on the last `X-Forwarded-For` entry when `DLA_TRUST_PROXY=true`. In-process test requests have no socket and fall back to `unknown`, so one test serves the app on an ephemeral port to cover the socket path. +- **Deleting users and mail accounts** goes through `AccountDeletionService`. SQLite doesn't enforce the schema's foreign keys, so every table that references a user or mail account must be cleaned up there, or its rows are orphaned. +- **Transactions are only atomic with a synchronous callback.** The bun-sqlite driver commits as soon as the `transaction()` callback returns, so an `async` callback commits at its first `await` and nothing after it rolls back. Many existing routes still pass `async` callbacks; new code that relies on rollback (like `AccountDeletionService`) must use `.run()`/`.all()`/`.get()` without `await`. ## Architecture Notes diff --git a/drizzle/migrations/sqlite/0015_purge_orphaned_user_data.sql b/drizzle/migrations/sqlite/0015_purge_orphaned_user_data.sql new file mode 100644 index 0000000..675ead0 --- /dev/null +++ b/drizzle/migrations/sqlite/0015_purge_orphaned_user_data.sql @@ -0,0 +1,28 @@ +-- `DELETE /admin/users/:userId` used to remove only the user row, its sessions, +-- API keys and password resets. SQLite doesn't enforce the schema's foreign +-- keys, so the user's mail accounts — including their encrypted IMAP/SMTP +-- credentials — identities, special-use mappings and preferences stayed behind, +-- unreachable through the API. Remove them. +-- +-- Only rows whose owner no longer exists match, so a repeated run is a no-op. +-- Dependents of a mail account go first, while the account still tells us whose +-- it was. +DELETE FROM `mail_identities` +WHERE `mail_account_id` NOT IN ( + SELECT `id` FROM `mail_accounts` WHERE `owner_user_id` IN (SELECT `id` FROM `users`) +); +--> statement-breakpoint +DELETE FROM `mail_account_special_use` +WHERE `mail_account_id` NOT IN ( + SELECT `id` FROM `mail_accounts` WHERE `owner_user_id` IN (SELECT `id` FROM `users`) +); +--> statement-breakpoint +DELETE FROM `mail_accounts` WHERE `owner_user_id` NOT IN (SELECT `id` FROM `users`); +--> statement-breakpoint +DELETE FROM `user_preferences` WHERE `user_id` NOT IN (SELECT `id` FROM `users`); +--> statement-breakpoint +DELETE FROM `sessions` WHERE `user_id` NOT IN (SELECT `id` FROM `users`); +--> statement-breakpoint +DELETE FROM `api_keys` WHERE `user_id` NOT IN (SELECT `id` FROM `users`); +--> statement-breakpoint +DELETE FROM `password_resets` WHERE `user_id` NOT IN (SELECT `id` FROM `users`); diff --git a/drizzle/migrations/sqlite/meta/0015_snapshot.json b/drizzle/migrations/sqlite/meta/0015_snapshot.json new file mode 100644 index 0000000..521efad --- /dev/null +++ b/drizzle/migrations/sqlite/meta/0015_snapshot.json @@ -0,0 +1,626 @@ +{ + "id": "1c4cb693-b6ed-478c-9194-583fb98465a1", + "prevId": "8d4f6a2f-debc-4c4a-9294-97ab69d75393", + "version": "6", + "dialect": "sqlite", + "tables": { + "api_keys": { + "name": "api_keys", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "hashed_token": { + "name": "hashed_token", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "user_role": { + "name": "user_role", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch() * 1000)" + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "api_keys_user_id_users_id_fk": { + "name": "api_keys_user_id_users_id_fk", + "tableFrom": "api_keys", + "columnsFrom": [ + "user_id" + ], + "tableTo": "users", + "columnsTo": [ + "id" + ], + "onUpdate": "no action", + "onDelete": "no action" + }, + "api_keys_user_role_users_role_fk": { + "name": "api_keys_user_role_users_role_fk", + "tableFrom": "api_keys", + "columnsFrom": [ + "user_role" + ], + "tableTo": "users", + "columnsTo": [ + "role" + ], + "onUpdate": "no action", + "onDelete": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "mail_account_special_use": { + "name": "mail_account_special_use", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "mail_account_id": { + "name": "mail_account_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch() * 1000)" + }, + "data": { + "name": "data", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "mail_account_special_use_account_unique": { + "name": "mail_account_special_use_account_unique", + "columns": [ + "mail_account_id" + ], + "isUnique": true + } + }, + "foreignKeys": { + "mail_account_special_use_mail_account_id_mail_accounts_id_fk": { + "name": "mail_account_special_use_mail_account_id_mail_accounts_id_fk", + "tableFrom": "mail_account_special_use", + "columnsFrom": [ + "mail_account_id" + ], + "tableTo": "mail_accounts", + "columnsTo": [ + "id" + ], + "onUpdate": "no action", + "onDelete": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "mail_accounts": { + "name": "mail_accounts", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch() * 1000)" + }, + "display_name": { + "name": "display_name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "smtp_encrypted_connection_data": { + "name": "smtp_encrypted_connection_data", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "imap_encrypted_connection_data": { + "name": "imap_encrypted_connection_data", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "is_default": { + "name": "is_default", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "onboarding_finished": { + "name": "onboarding_finished", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + } + }, + "indexes": {}, + "foreignKeys": { + "mail_accounts_owner_user_id_users_id_fk": { + "name": "mail_accounts_owner_user_id_users_id_fk", + "tableFrom": "mail_accounts", + "columnsFrom": [ + "owner_user_id" + ], + "tableTo": "users", + "columnsTo": [ + "id" + ], + "onUpdate": "no action", + "onDelete": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "mail_identities": { + "name": "mail_identities", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "mail_account_id": { + "name": "mail_account_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch() * 1000)" + }, + "display_name": { + "name": "display_name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "email_address": { + "name": "email_address", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "signature": { + "name": "signature", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "is_default": { + "name": "is_default", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + } + }, + "indexes": {}, + "foreignKeys": { + "mail_identities_mail_account_id_mail_accounts_id_fk": { + "name": "mail_identities_mail_account_id_mail_accounts_id_fk", + "tableFrom": "mail_identities", + "columnsFrom": [ + "mail_account_id" + ], + "tableTo": "mail_accounts", + "columnsTo": [ + "id" + ], + "onUpdate": "no action", + "onDelete": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "metadata": { + "name": "metadata", + "columns": { + "key": { + "name": "key", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "data": { + "name": "data", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "password_resets": { + "name": "password_resets", + "columns": { + "token": { + "name": "token", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch() * 1000)" + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "password_resets_user_id_users_id_fk": { + "name": "password_resets_user_id_users_id_fk", + "tableFrom": "password_resets", + "columnsFrom": [ + "user_id" + ], + "tableTo": "users", + "columnsTo": [ + "id" + ], + "onUpdate": "no action", + "onDelete": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "sessions": { + "name": "sessions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "hashed_token": { + "name": "hashed_token", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "user_role": { + "name": "user_role", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch() * 1000)" + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "sessions_user_id_users_id_fk": { + "name": "sessions_user_id_users_id_fk", + "tableFrom": "sessions", + "columnsFrom": [ + "user_id" + ], + "tableTo": "users", + "columnsTo": [ + "id" + ], + "onUpdate": "no action", + "onDelete": "no action" + }, + "sessions_user_role_users_role_fk": { + "name": "sessions_user_role_users_role_fk", + "tableFrom": "sessions", + "columnsFrom": [ + "user_role" + ], + "tableTo": "users", + "columnsTo": [ + "role" + ], + "onUpdate": "no action", + "onDelete": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "user_preferences": { + "name": "user_preferences", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "user_id": { + "name": "user_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch() * 1000)" + }, + "key": { + "name": "key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "data": { + "name": "data", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "user_preferences_user_id_key_unique": { + "name": "user_preferences_user_id_key_unique", + "columns": [ + "user_id", + "key" + ], + "isUnique": true + } + }, + "foreignKeys": { + "user_preferences_user_id_users_id_fk": { + "name": "user_preferences_user_id_users_id_fk", + "tableFrom": "user_preferences", + "columnsFrom": [ + "user_id" + ], + "tableTo": "users", + "columnsTo": [ + "id" + ], + "onUpdate": "no action", + "onDelete": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch() * 1000)" + }, + "username": { + "name": "username", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "display_name": { + "name": "display_name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "password_hash": { + "name": "password_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "role": { + "name": "role", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'user'" + } + }, + "indexes": { + "users_username_unique": { + "name": "users_username_unique", + "columns": [ + "username" + ], + "isUnique": true + }, + "users_email_unique": { + "name": "users_email_unique", + "columns": [ + "email" + ], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "columns": {}, + "schemas": {}, + "tables": {} + }, + "internal": { + "indexes": {} + } +} \ No newline at end of file diff --git a/drizzle/migrations/sqlite/meta/_journal.json b/drizzle/migrations/sqlite/meta/_journal.json index 0866ab6..533aaeb 100644 --- a/drizzle/migrations/sqlite/meta/_journal.json +++ b/drizzle/migrations/sqlite/meta/_journal.json @@ -106,6 +106,13 @@ "when": 1790196016887, "tag": "0014_backfill_default_mail_identities", "breakpoints": true + }, + { + "idx": 15, + "version": "6", + "when": 1791041428441, + "tag": "0015_purge_orphaned_user_data", + "breakpoints": true } ] } \ No newline at end of file diff --git a/src/api/utils/services/accountDeletionService.ts b/src/api/utils/services/accountDeletionService.ts new file mode 100644 index 0000000..9fc72b0 --- /dev/null +++ b/src/api/utils/services/accountDeletionService.ts @@ -0,0 +1,68 @@ +import { eq } from "drizzle-orm"; +import { DB } from "../../../db"; +import type { DrizzleDB } from "../../../db/utils"; + +/** + * Removes users and mail accounts together with every row that belongs to them. + * + * SQLite doesn't enforce the schema's foreign keys here, so anything left behind + * would linger as orphaned data — including encrypted IMAP/SMTP credentials. Every + * table that references a user or mail account must therefore be cleaned up here. + * + * Synchronous on purpose: the bun-sqlite driver commits a transaction as soon as its + * callback returns, so an `async` callback commits at its first `await` and nothing + * after it can be rolled back. Call these from a synchronous transaction callback, + * `DB.instance().transaction((tx) => AccountDeletionService.deleteUser(id, tx))`, and + * close the removed accounts' pooled IMAP connections with + * `MailClientsCache.deleteClient` once it has committed. + */ +export class AccountDeletionService { + + static deleteMailAccount(mailAccountID: number, tx: DrizzleDB = DB.instance()): void { + tx.delete(DB.Tables.mailIdentities).where( + eq(DB.Tables.mailIdentities.mail_account_id, mailAccountID) + ).run(); + + tx.delete(DB.Tables.mailAccountSpecialUse).where( + eq(DB.Tables.mailAccountSpecialUse.mail_account_id, mailAccountID) + ).run(); + + tx.delete(DB.Tables.mailAccounts).where( + eq(DB.Tables.mailAccounts.id, mailAccountID) + ).run(); + } + + /** @returns the IDs of the user's mail accounts that were removed with it. */ + static deleteUser(userID: number, tx: DrizzleDB = DB.instance()): number[] { + const mailAccountIDs = tx.select({ id: DB.Tables.mailAccounts.id }).from(DB.Tables.mailAccounts).where( + eq(DB.Tables.mailAccounts.owner_user_id, userID) + ).all().map(account => account.id); + + for (const mailAccountID of mailAccountIDs) { + this.deleteMailAccount(mailAccountID, tx); + } + + tx.delete(DB.Tables.userPreferences).where( + eq(DB.Tables.userPreferences.user_id, userID) + ).run(); + + tx.delete(DB.Tables.passwordResets).where( + eq(DB.Tables.passwordResets.user_id, userID) + ).run(); + + tx.delete(DB.Tables.sessions).where( + eq(DB.Tables.sessions.user_id, userID) + ).run(); + + tx.delete(DB.Tables.apiKeys).where( + eq(DB.Tables.apiKeys.user_id, userID) + ).run(); + + tx.delete(DB.Tables.users).where( + eq(DB.Tables.users.id, userID) + ).run(); + + return mailAccountIDs; + } + +} diff --git a/src/api/versions/v1/routes/account/index.ts b/src/api/versions/v1/routes/account/index.ts index 552c24d..70f2bdd 100644 --- a/src/api/versions/v1/routes/account/index.ts +++ b/src/api/versions/v1/routes/account/index.ts @@ -7,7 +7,7 @@ import { eq } from "drizzle-orm"; import { APIResponse } from "../../../../utils/api-res"; import { APIResponseSpec, APIRouteSpec } from "../../../../utils/specHelpers"; import { AuthHandler, SessionHandler } from "../../../../utils/authHandler"; -import { UserPreferencesHandler } from "../../../../utils/preferences"; +import { AccountDeletionService } from "../../../../utils/services/accountDeletionService"; import { DOCS_TAGS } from "../../docs"; import { router as apiKeyRouter } from "./apikeys"; import { router as preferencesRouter } from "./preferences"; @@ -212,22 +212,8 @@ router.delete('/', return APIResponse.badRequest(c, "Please delete all mail accounts associated with this account before deleting the account"); } - await DB.instance().transaction(async (tx: DrizzleDB) => { - // invalidate all sessions for the user - await AuthHandler.invalidateAllAuthContextsForUser(authContext.user_id, tx); - - // delete password resets - await tx.delete(DB.Tables.passwordResets).where( - eq(DB.Tables.passwordResets.user_id, authContext.user_id) - ).run(); - - // delete stored preferences - await UserPreferencesHandler.deleteAllForUser(authContext.user_id, tx); - - // finally, delete the user account - await tx.delete(DB.Tables.users).where( - eq(DB.Tables.users.id, authContext.user_id) - ).run(); + DB.instance().transaction((tx: DrizzleDB) => { + AccountDeletionService.deleteUser(authContext.user_id, tx); }); return APIResponse.successNoData(c, "Account deleted successfully"); diff --git a/src/api/versions/v1/routes/admin/users/index.ts b/src/api/versions/v1/routes/admin/users/index.ts index daff83a..09d8855 100644 --- a/src/api/versions/v1/routes/admin/users/index.ts +++ b/src/api/versions/v1/routes/admin/users/index.ts @@ -7,6 +7,10 @@ import { APIResponseSpec, APIRouteSpec } from "../../../../../utils/specHelpers" import { AdminUsersModel } from "./model"; import { AuthHandler, SessionHandler } from "../../../../../utils/authHandler"; import { DOCS_TAGS } from "../../../docs"; +import { AccountDeletionService } from "../../../../../utils/services/accountDeletionService"; +import type { DrizzleDB } from "../../../../../../db/utils"; +import { MailClientsCache } from "../../../../../../utils/mails/mail-clients-cache"; +import { Logger } from "../../../../../../utils/logger"; const TARGET_USER_KEY = "adminTargetUser"; @@ -260,13 +264,13 @@ router.delete('/:userId', APIRouteSpec.authenticated({ summary: "Delete user", - description: "Permanently remove a Delivr account after verifying it has no owned packages.", + description: "Permanently remove a Delivr account together with its mail accounts, identities, preferences, sessions and API keys. Mail on the mail servers is not touched.", tags: [DOCS_TAGS.ADMIN_API.USERS], responses: APIResponseSpec.describeBasic( APIResponseSpec.successNoData("User deleted successfully"), APIResponseSpec.notFound("User not found"), - APIResponseSpec.badRequest("Cannot delete user while packages are assigned") + APIResponseSpec.serverError("Failed to delete user") ) }), @@ -274,17 +278,19 @@ router.delete('/:userId', // @ts-ignore const user = c.get(TARGET_USER_KEY) as DB.Models.User; - // Check for user data later - - await AuthHandler.invalidateAllAuthContextsForUser(user.id); - - await DB.instance().delete(DB.Tables.passwordResets).where( - eq(DB.Tables.passwordResets.user_id, user.id) - ).run(); + let mailAccountIDs: number[]; + try { + mailAccountIDs = DB.instance().transaction((tx: DrizzleDB) => { + return AccountDeletionService.deleteUser(user.id, tx); + }); + } catch (error: any) { + Logger.error("Failed to delete user", error.stack || error.message || error); + return APIResponse.serverError(c, "Failed to delete user"); + } - await DB.instance().delete(DB.Tables.users).where( - eq(DB.Tables.users.id, user.id) - ).run(); + for (const mailAccountID of mailAccountIDs) { + await MailClientsCache.deleteClient(mailAccountID); + } return APIResponse.successNoData(c, "User deleted successfully"); } diff --git a/src/api/versions/v1/routes/mail-accounts/index.ts b/src/api/versions/v1/routes/mail-accounts/index.ts index a2f4eb6..cf6b965 100644 --- a/src/api/versions/v1/routes/mail-accounts/index.ts +++ b/src/api/versions/v1/routes/mail-accounts/index.ts @@ -12,6 +12,7 @@ import { router as searchRouter } from "./search"; import { router as specialUseRouter } from "./special-use"; import { router as onboardingRouter } from "./onboarding"; import { SpecialUseHandler } from "../../../../utils/services/specialUseService"; +import { AccountDeletionService } from "../../../../utils/services/accountDeletionService"; import { AuthHandler } from "../../../../utils/authHandler"; import { validator } from "hono-openapi"; import { MailClientsCache } from "../../../../../utils/mails/mail-clients-cache"; @@ -483,18 +484,8 @@ router.delete('/:mailAccountID', await MailClientsCache.deleteClient(mailAccount.id); try { - await DB.instance().transaction(async (tx: DrizzleDB) => { - // Delete all mail identities linked to this mail account - await tx.delete(DB.Tables.mailIdentities).where( - eq(DB.Tables.mailIdentities.mail_account_id, mailAccount.id) - ); - - // Drop the persisted special-use mapping for this account. - await SpecialUseHandler.deleteForAccount(mailAccount.id, tx); - - await tx.delete(DB.Tables.mailAccounts).where( - eq(DB.Tables.mailAccounts.id, mailAccount.id) - ); + DB.instance().transaction((tx: DrizzleDB) => { + AccountDeletionService.deleteMailAccount(mailAccount.id, tx); }); } catch (error: any) { Logger.error("Failed to delete mail account", error.stack || error.message || error); diff --git a/src/utils/mails/mail-clients-cache.ts b/src/utils/mails/mail-clients-cache.ts index 638712f..a6e08ea 100644 --- a/src/utils/mails/mail-clients-cache.ts +++ b/src/utils/mails/mail-clients-cache.ts @@ -1,5 +1,6 @@ import { MailAccountsModel } from "../../api/versions/v1/routes/mail-accounts/model"; import { IMAPAccount } from "./backends/imap"; +import { Logger } from "../logger"; export class MailClientsCache { @@ -33,12 +34,17 @@ export class MailClientsCache { return this.createClientData(settings); } + /** Never throws: a failed logout must not fail the deletion or update that dropped the client. */ static async deleteClient(accountID: number): Promise { const clientData = this.clients.get(accountID); + this.clients.delete(accountID); if (clientData) { - await clientData.imap.disconnect(); + try { + await clientData.imap.disconnect(); + } catch (error: any) { + Logger.warn(`Failed to disconnect the IMAP client of mail account ${accountID}`, error.message || error); + } } - this.clients.delete(accountID); } /** diff --git a/tests/api.routes.test.ts b/tests/api.routes.test.ts index 604db2a..ae943dc 100644 --- a/tests/api.routes.test.ts +++ b/tests/api.routes.test.ts @@ -1,7 +1,7 @@ import { afterAll, describe, expect, test, beforeAll, spyOn } from "bun:test"; import { API } from "../src/api"; import { DB } from "../src/db"; -import { AuthHandler, AuthUtils, SessionHandler } from "../src/api/utils/authHandler"; +import { APIKeyHandler, AuthHandler, AuthUtils, SessionHandler } from "../src/api/utils/authHandler"; import { randomUUID } from "crypto"; import { and, desc, eq } from "drizzle-orm"; import { AuthModel } from "../src/api/versions/v1/routes/auth/model"; @@ -13,6 +13,7 @@ import { MailIdentitiesModel } from "../src/api/versions/v1/routes/mail-accounts import { MailboxesModel } from "../src/api/versions/v1/routes/mail-accounts/mailboxes/model"; import { SpecialUseModel } from "../src/api/versions/v1/routes/mail-accounts/special-use/model"; import { SpecialUse, SpecialUseHandler } from "../src/api/utils/services/specialUseService"; +import { AccountDeletionService } from "../src/api/utils/services/accountDeletionService"; import { MailboxRessource } from "../src/utils/mails/ressources/mailbox"; import { IMAPAccount } from "../src/utils/mails/backends/imap"; import { MailAccountEncryption } from "../src/utils/crypto/mailCrypt"; @@ -1914,6 +1915,173 @@ describe("Login rate limiting", async () => { }); }); +/** Seeds a mail account with an identity, a special-use mapping, a preference, a session and an API key. */ +async function seedUserData(userID: number) { + const mailAccountID = (await seedMailAccount(userID)).id; + + DB.instance().insert(DB.Tables.mailIdentities).values({ + mail_account_id: mailAccountID, + display_name: "Seeded Identity", + email_address: `${randomUUID()}@example.com`, + is_default: true + }).run(); + + DB.instance().insert(DB.Tables.mailAccountSpecialUse).values({ + mail_account_id: mailAccountID, + data: { sent: { path: "Sent", source: "flag" } } + }).run(); + + DB.instance().insert(DB.Tables.userPreferences).values({ + user_id: userID, + key: "auto-mark-seen", + data: { enabled: false } + }).run(); + + await seedSession(userID); + await APIKeyHandler.createApiKey(userID, "Seeded API key"); + + return mailAccountID; +} + +/** Every row stored for a user and one of their mail accounts, per table. */ +function storedUserData(userID: number, mailAccountID: number) { + return { + users: DB.instance().select().from(DB.Tables.users).where(eq(DB.Tables.users.id, userID)).all(), + mailAccounts: DB.instance().select().from(DB.Tables.mailAccounts).where(eq(DB.Tables.mailAccounts.id, mailAccountID)).all(), + mailIdentities: DB.instance().select().from(DB.Tables.mailIdentities).where(eq(DB.Tables.mailIdentities.mail_account_id, mailAccountID)).all(), + specialUse: DB.instance().select().from(DB.Tables.mailAccountSpecialUse).where(eq(DB.Tables.mailAccountSpecialUse.mail_account_id, mailAccountID)).all(), + preferences: DB.instance().select().from(DB.Tables.userPreferences).where(eq(DB.Tables.userPreferences.user_id, userID)).all(), + sessions: DB.instance().select().from(DB.Tables.sessions).where(eq(DB.Tables.sessions.user_id, userID)).all(), + apiKeys: DB.instance().select().from(DB.Tables.apiKeys).where(eq(DB.Tables.apiKeys.user_id, userID)).all(), + }; +} + +const NO_USER_DATA = { + users: [], + mailAccounts: [], + mailIdentities: [], + specialUse: [], + preferences: [], + sessions: [], + apiKeys: [], +}; + +describe("Admin User Routes", async () => { + + let adminSessionToken: string; + + beforeAll(async () => { + adminSessionToken = (await seedSession(testAdmin.id)).token; + }); + + test("DELETE /v1/admin/users/:userId removes the user and everything stored for them", async () => { + const user = await seedUser("user", {}, "DeleteMeP@ss1"); + const mailAccountID = await seedUserData(user.id); + + await makeAPIRequest(`/v1/admin/users/${user.id}`, { + method: "DELETE", + authToken: adminSessionToken + }); + + expect(storedUserData(user.id, mailAccountID)).toEqual(NO_USER_DATA); + }); + + test("DELETE /v1/admin/users/:userId leaves other users' data untouched", async () => { + const user = await seedUser("user", {}, "DeleteMeP@ss1"); + await seedUserData(user.id); + + const otherUser = await seedUser("user", {}, "KeepMeP@ss1"); + const otherMailAccountID = await seedUserData(otherUser.id); + + await makeAPIRequest(`/v1/admin/users/${user.id}`, { + method: "DELETE", + authToken: adminSessionToken + }); + + const kept = storedUserData(otherUser.id, otherMailAccountID); + for (const rows of Object.values(kept)) { + expect(rows.length).toBe(1); + } + + AccountDeletionService.deleteUser(otherUser.id); + }); + + test("AccountDeletionService.deleteUser rolls back completely when its transaction fails", async () => { + const user = await seedUser("user", {}, "KeepMeP@ss1"); + const mailAccountID = await seedUserData(user.id); + + expect(() => DB.instance().transaction((tx) => { + AccountDeletionService.deleteUser(user.id, tx); + throw new Error("abort"); + })).toThrow("abort"); + + // Let anything queued after the commit run: an async deleteUser would delete here, outside the transaction. + await Bun.sleep(1); + + for (const rows of Object.values(storedUserData(user.id, mailAccountID))) { + expect(rows.length).toBe(1); + } + + AccountDeletionService.deleteUser(user.id); + }); +}); + +describe("Orphaned User Data Purge Migration", async () => { + + // Referenced by name on purpose: renaming the file should fail this loudly + // rather than quietly testing nothing. + const MIGRATION_FILE = "drizzle/migrations/sqlite/0015_purge_orphaned_user_data.sql"; + + let keptUser: SeededUser; + let keptMailAccountID: number; + let removedUserID: number; + let orphanedMailAccountID: number; + + async function runMigration() { + const sql = await Bun.file(MIGRATION_FILE).text(); + for (const statement of sql.split("--> statement-breakpoint")) { + if (statement.trim() === "") continue; + DB.instance().$client.exec(statement); + } + } + + beforeAll(async () => { + keptUser = await seedUser("user", {}, "KeepMeP@ss1"); + keptMailAccountID = await seedUserData(keptUser.id); + + const removedUser = await seedUser("user", {}, "DeleteMeP@ss1"); + removedUserID = removedUser.id; + orphanedMailAccountID = await seedUserData(removedUser.id); + + // What the admin route used to do: drop the user row and leave the rest behind. + DB.instance().delete(DB.Tables.users).where(eq(DB.Tables.users.id, removedUserID)).run(); + }); + + test("removes everything that belonged to a user who no longer exists", async () => { + await runMigration(); + + expect(storedUserData(removedUserID, orphanedMailAccountID)).toEqual(NO_USER_DATA); + }); + + test("leaves data of existing users untouched", async () => { + for (const rows of Object.values(storedUserData(keptUser.id, keptMailAccountID))) { + expect(rows.length).toBe(1); + } + }); + + test("is idempotent", async () => { + await runMigration(); + + for (const rows of Object.values(storedUserData(keptUser.id, keptMailAccountID))) { + expect(rows.length).toBe(1); + } + }); + + afterAll(() => { + AccountDeletionService.deleteUser(keptUser.id); + }); +}); + describe("Mail Mailbox Routes", async () => { let mailIdentityTestUser: SeededUser; From 11440278c349ca3e84620b428f2bfaebe91d11b8 Mon Sep 17 00:00:00 2001 From: Linus Fischer Date: Sat, 3 Oct 2026 16:22:38 +0000 Subject: [PATCH 5/7] docs: state that only SQLite is supported at runtime The PostgreSQL and MySQL schemas are kept in sync, but nothing wires up those drivers yet (#10). --- CLAUDE.md | 2 +- README.md | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8553326..7a1c8ac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ - **ORM**: Drizzle ORM 0.45.x with Drizzle Kit 0.31.x - **Validation**: Zod 4.x + `@hono/standard-validator` - **OpenAPI**: `hono-openapi` + `@scalar/hono-api-reference` -- **Database**: SQLite (default, via `@libsql/client`), PostgreSQL, MySQL +- **Database**: SQLite at runtime (`drizzle-orm/bun-sqlite`); PostgreSQL and MySQL schemas are kept in sync, but runtime support isn't wired up yet (#10) - **Mail**: IMAP (`imapflow`) + SMTP (`nodemailer`) + `postal-mime` parsing - **Crypto**: `elliptic` (ECC), custom encryption utilities - **Scheduling**: `cron` package diff --git a/README.md b/README.md index 9025161..ef3a889 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ speaking IMAP and SMTP so your inbox stays yours. - 🔐 **JWT auth + API keys** — token-based sessions plus scoped API keys for programmatic access. - 🔒 **ECC crypto** — mail-backend credentials protected with elliptic-curve encryption and signing. - 📖 **First-class OpenAPI** — every route is documented via `hono-openapi` and browsable through an embedded [Scalar](https://scalar.com) reference. -- 🗄️ **Bring your own database** — SQLite out of the box, with PostgreSQL and MySQL fully supported through Drizzle. +- 🗄️ **Zero-setup database** — SQLite out of the box, with no separate database server to run. PostgreSQL and MySQL schemas exist and runtime support is planned ([#10](https://github.com/Delivr-Project/Delivr-API/issues/10)). - ⏰ **Scheduled tasks** — background jobs via the `cron` package. - ✅ **Integration-tested** — a mock IMAP/SMTP harness exercises the real request paths. @@ -47,7 +47,7 @@ speaking IMAP and SMTP so your inbox stays yours. | ORM | **Drizzle ORM** + Drizzle Kit | | Validation | **Zod 4.x** + `@hono/standard-validator` | | API Docs | `hono-openapi` + `@scalar/hono-api-reference` | -| Database | SQLite · PostgreSQL · MySQL | +| Database | SQLite (PostgreSQL · MySQL planned) | | Mail | `imapflow` (IMAP) · `nodemailer` (SMTP) · `postal-mime` | | Crypto | `elliptic` (ECC) | @@ -113,8 +113,8 @@ The attachment limit counts the files' raw size. Base64 encoding makes the sent | `bun run start` | Production entry point | | `bun run db:sqlite:generate` | Generate SQLite migrations | | `bun run db:sqlite:migrate` | Run SQLite migrations | -| `bun run db:postgresql:generate` · `:migrate` | PostgreSQL migrations | -| `bun run db:mysql:generate` · `:migrate` | MySQL migrations | +| `bun run db:postgresql:generate` · `:migrate` | PostgreSQL migrations (schema only, runtime support planned) | +| `bun run db:mysql:generate` · `:migrate` | MySQL migrations (schema only, runtime support planned) | ## 🗺️ Project Structure From 5569f49ecb59bcf5f30c55bd08fdf7e3d6366ac8 Mon Sep 17 00:00:00 2001 From: Linus Fischer Date: Sat, 3 Oct 2026 16:50:59 +0000 Subject: [PATCH 6/7] revert(docker): go back to DELIVR_* for the web container Restores DELIVR_API_URL, DELIVR_APP_URL and DELIVR_ENABLE_SIGNUP in the full-stack compose file and the CLAUDE.md note. The depends_on fix stays. --- CLAUDE.md | 2 +- docker/docker-compose.prodlike.yml | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7a1c8ac..d7dd878 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,7 +83,7 @@ src/ - **Tests**: Integration-heavy. `bunfig.toml` preloads `tests/helpers/preload.ts`, which builds the app with `API.init()` but never binds a port — requests go through `API.getApp().request()` in-process, so the suite runs while a dev server is up. Mock IMAP servers do listen: `11143` for the shared one, `11144`–`11148` for per-test servers. - **Config**: Environment-based config in `src/utils/config.ts`. See `example.env` for required vars. - **Crypto**: ECC-based encryption/signing utilities in `src/utils/crypto/`. -- **Docker**: `docker/Dockerfile` packages a binary compiled beforehand (`bun run compile linux-x64-baseline --no-version-tag`, as CI does) together with `drizzle/migrations`, which the binary reads from disk. A relative `DLA_DB_MIGRATION_DIR` therefore has to be resolved next to the executable (`process.execPath`), not against `import.meta.dir`, which is Bun's virtual `/$bunfs/root` in a compiled binary. `docker/docker-compose.yml` runs the API alone and `docker/docker-compose.prodlike.yml` the full stack (API + Delivr-Web) from the ghcr.io images. The web image is configured at runtime through `NUXT_PUBLIC_*` variables (`DELIVR_*` is only read at build time), and it requires *public* URLs, since the web client's SSR server calls the API through the same `NUXT_PUBLIC_API_URL` as the browser. +- **Docker**: `docker/Dockerfile` packages a binary compiled beforehand (`bun run compile linux-x64-baseline --no-version-tag`, as CI does) together with `drizzle/migrations`, which the binary reads from disk. A relative `DLA_DB_MIGRATION_DIR` therefore has to be resolved next to the executable (`process.execPath`), not against `import.meta.dir`, which is Bun's virtual `/$bunfs/root` in a compiled binary. `docker/docker-compose.yml` runs the API alone and `docker/docker-compose.prodlike.yml` the full stack (API + Delivr-Web) from the ghcr.io images; it requires *public* URLs, since the web client's SSR server calls the API through the same `DELIVR_API_URL` as the browser. - **Client addresses**: login rate limiting keys on the socket address (`getConnInfo` from `hono/bun`), or on the last `X-Forwarded-For` entry when `DLA_TRUST_PROXY=true`. In-process test requests have no socket and fall back to `unknown`, so one test serves the app on an ephemeral port to cover the socket path. - **Deleting users and mail accounts** goes through `AccountDeletionService`. SQLite doesn't enforce the schema's foreign keys, so every table that references a user or mail account must be cleaned up there, or its rows are orphaned. - **Transactions are only atomic with a synchronous callback.** The bun-sqlite driver commits as soon as the `transaction()` callback returns, so an `async` callback commits at its first `await` and nothing after it rolls back. Many existing routes still pass `async` callbacks; new code that relies on rollback (like `AccountDeletionService`) must use `.run()`/`.all()`/`.get()` without `await`. diff --git a/docker/docker-compose.prodlike.yml b/docker/docker-compose.prodlike.yml index 5c631d5..0664a2e 100644 --- a/docker/docker-compose.prodlike.yml +++ b/docker/docker-compose.prodlike.yml @@ -36,13 +36,11 @@ services: ports: - "14128:14128" environment: - # The image is built once and configured at runtime through Nuxt's NUXT_PUBLIC_* - # overrides; DELIVR_* is only read while the image is built. - NUXT_PUBLIC_API_URL: https://api.delivr.example.com/v1 - NUXT_PUBLIC_APP_URL: https://delivr.example.com - - NUXT_PUBLIC_IS_SIGNUP_ENABLED: false + DELIVR_API_URL: https://api.delivr.example.com/v1 + DELIVR_APP_URL: https://delivr.example.com + DELIVR_ENABLE_SIGNUP: false + depends_on: delivr-api: condition: service_healthy From 4238d822bba0bc189e72d3de14e4a005587801ec Mon Sep 17 00:00:00 2001 From: Linus Fischer Date: Sat, 3 Oct 2026 16:50:59 +0000 Subject: [PATCH 7/7] revert(auth): drop DLA_TRUST_PROXY Login rate limiting keys on the socket address only, so X-Forwarded-For is always ignored. Removes the setting from the config, example.env, the README, the test preload and CLAUDE.md, along with its test. --- CLAUDE.md | 2 +- README.md | 1 - example.env | 3 --- src/api/versions/v1/routes/auth/index.ts | 6 ------ src/utils/config.ts | 1 - tests/api.routes.test.ts | 18 +----------------- tests/helpers/preload.ts | 1 - 7 files changed, 2 insertions(+), 30 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d7dd878..eda5392 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,7 +84,7 @@ src/ - **Config**: Environment-based config in `src/utils/config.ts`. See `example.env` for required vars. - **Crypto**: ECC-based encryption/signing utilities in `src/utils/crypto/`. - **Docker**: `docker/Dockerfile` packages a binary compiled beforehand (`bun run compile linux-x64-baseline --no-version-tag`, as CI does) together with `drizzle/migrations`, which the binary reads from disk. A relative `DLA_DB_MIGRATION_DIR` therefore has to be resolved next to the executable (`process.execPath`), not against `import.meta.dir`, which is Bun's virtual `/$bunfs/root` in a compiled binary. `docker/docker-compose.yml` runs the API alone and `docker/docker-compose.prodlike.yml` the full stack (API + Delivr-Web) from the ghcr.io images; it requires *public* URLs, since the web client's SSR server calls the API through the same `DELIVR_API_URL` as the browser. -- **Client addresses**: login rate limiting keys on the socket address (`getConnInfo` from `hono/bun`), or on the last `X-Forwarded-For` entry when `DLA_TRUST_PROXY=true`. In-process test requests have no socket and fall back to `unknown`, so one test serves the app on an ephemeral port to cover the socket path. +- **Client addresses**: login rate limiting keys on the socket address (`getConnInfo` from `hono/bun`). In-process test requests have no socket and fall back to `unknown`, so one test serves the app on an ephemeral port to cover the socket path. - **Deleting users and mail accounts** goes through `AccountDeletionService`. SQLite doesn't enforce the schema's foreign keys, so every table that references a user or mail account must be cleaned up there, or its rows are orphaned. - **Transactions are only atomic with a synchronous callback.** The bun-sqlite driver commits as soon as the `transaction()` callback returns, so an `async` callback commits at its first `await` and nothing after it rolls back. Many existing routes still pass `async` callbacks; new code that relies on rollback (like `AccountDeletionService`) must use `.run()`/`.all()`/`.get()` without `await`. diff --git a/README.md b/README.md index ef3a889..3f24d28 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,6 @@ All configuration is environment-based (see [`example.env`](./example.env)): | `DLA_APP_URL` | **Required.** URL of the Delivr web client | — | | `DLA_API_HOST` | Bind address | `::` | | `DLA_API_PORT` | Listen port | `14123` | -| `DLA_TRUST_PROXY` | Read the client address from `X-Forwarded-For` for login rate limiting. Enable it only when the API is reachable solely through your reverse proxy | `false` | | `DLA_DISABLE_DOCS` | Disable the Scalar API reference | `false` | | `DLA_MAX_ATTACHMENT_SIZE_MB` | Maximum combined attachment size per composed mail, in MB, measured before encoding | `25` | | `DLA_ENCRYPTION_KEY` | **Required. 32-character** key for credential encryption | — | diff --git a/example.env b/example.env index b6263a7..c2862a2 100644 --- a/example.env +++ b/example.env @@ -4,9 +4,6 @@ DLA_APP_URL=https://app.delivr.is-on.net DLA_API_HOST=:: DLA_API_PORT=14123 -# Set to true when the API is only reachable through your reverse proxy, so login -# rate limiting sees each client's address (from X-Forwarded-For) instead of the proxy's. -DLA_TRUST_PROXY=false DLA_DISABLE_DOCS=false DLA_ENCRYPTION_KEY=your-32-character-encryption-key diff --git a/src/api/versions/v1/routes/auth/index.ts b/src/api/versions/v1/routes/auth/index.ts index b6d214f..f708c12 100644 --- a/src/api/versions/v1/routes/auth/index.ts +++ b/src/api/versions/v1/routes/auth/index.ts @@ -12,7 +12,6 @@ import { APIResponseSpec, APIRouteSpec } from "../../../../utils/specHelpers"; import { router as resetPasswordRouter } from "./reset-password"; import { DOCS_TAGS } from "../../docs"; import { Logger } from "../../../../../utils/logger"; -import { ConfigHandler } from "../../../../../utils/config"; // Dummy bcrypt hash for timing-normalized login failures — prevents username enumeration // Generated once at module load so it's a valid, cost-equivalent hash @@ -39,11 +38,6 @@ const LOGIN_CLEANUP_INTERVAL = setInterval(() => { LOGIN_CLEANUP_INTERVAL.unref(); function getClientId(c: Context) { - if (ConfigHandler.getConfig()?.DLA_TRUST_PROXY) { - // The reverse proxy appends the address it saw, so the last entry is the one a client can't forge. - const forwarded = c.req.header("x-forwarded-for")?.split(",").at(-1)?.trim(); - if (forwarded) return forwarded; - } try { return getConnInfo(c).remote.address || "unknown"; } catch { diff --git a/src/utils/config.ts b/src/utils/config.ts index 47f7add..b8a7cf2 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -111,7 +111,6 @@ export class ConfigHandler { .add("DLA_API_HOST", false) .add("DLA_API_PORT", false) - .add("DLA_TRUST_PROXY", false, [true, false]) .add("DLA_DISABLE_DOCS", false, [true, false]) .add("DLA_ENCRYPTION_KEY", true) diff --git a/tests/api.routes.test.ts b/tests/api.routes.test.ts index ae943dc..d8a633c 100644 --- a/tests/api.routes.test.ts +++ b/tests/api.routes.test.ts @@ -1865,7 +1865,7 @@ describe("Login rate limiting", async () => { await attemptLogin(username, "wrong-password", clientIP, 429); } - test("ignores X-Forwarded-For unless DLA_TRUST_PROXY is set", async () => { + test("ignores X-Forwarded-For", async () => { const user = await seedUser("user", {}, "RateLimitP@ss1"); await exhaustAttempts(user.username, "198.51.100.1"); @@ -1874,22 +1874,6 @@ describe("Login rate limiting", async () => { await attemptLogin(user.username, user.password, "198.51.100.2", 429); }); - test("limits each forwarded client separately with DLA_TRUST_PROXY", async () => { - const config = ConfigHandler.getConfig()!; - config.DLA_TRUST_PROXY = true; - - try { - const user = await seedUser("user", {}, "RateLimitP@ss1"); - - await exhaustAttempts(user.username, "198.51.100.1"); - - // Another client can still sign in to the same account. - await attemptLogin(user.username, user.password, "198.51.100.2", 200); - } finally { - config.DLA_TRUST_PROXY = false; - } - }); - test("keys on the socket address of a real connection", async () => { const user = await seedUser("user", {}, "RateLimitP@ss1"); diff --git a/tests/helpers/preload.ts b/tests/helpers/preload.ts index 5c29752..aca6b93 100644 --- a/tests/helpers/preload.ts +++ b/tests/helpers/preload.ts @@ -21,7 +21,6 @@ function setTestEnv(rootDir: string) { DLA_API_HOST: "::", DLA_API_PORT: "1", - DLA_TRUST_PROXY: false, DLA_DISABLE_DOCS: true, DLA_ENCRYPTION_KEY: "67e3d03dc88682553deed5fa4484bd80a500783850efbb49f6912ad0935eedeb",