idp: self-service DELETE /idp/account endpoint (JavaScriptSolidServer#352) (JavaScriptSolidServer#391)

melvincarvalho · web-flow · commit aaeae1e8b51f · 2026-05-09T07:06:55.000+02:00
* idp: self-service DELETE /idp/account endpoint (JavaScriptSolidServer#352) Authenticated owner can now delete their own account via HTTP. Mirrors the password-rotation pattern from JavaScriptSolidServer#351 (PUT /idp/credentials): caller authenticates with their existing session, re-proves possession by sending currentPassword in the body, server resolves the account from the caller's WebID and deletes it. Owner-only by construction — the cross-account attack (A authenticated, sends B's password) returns 401 and touches nothing. Two new shape elements vs. password rotation: 1. Single-user mode rejects with 403. Deleting the single account would brick the IdP until re-seed; the operator path stays the CLI (`jss account delete`). Test asserts both 403 status and account-still-functional after. 2. Optional `purgeData: true` body flag mirrors the CLI's `--purge`. Removes the pod's filesystem tree at <dataRoot>/<username>/. Path is normalized + verified to be a proper child of dataRoot (belt-and-suspenders against future config drift; usernames are already validated by registration). Default is false — account record gone, pod data preserved, matches the CLI's default. Endpoint surface: DELETE /idp/account Headers: Authorization (existing session / DPoP) Body: { currentPassword: string, purgeData?: boolean } Returns: 200 { ok, webid, purged } — success 400 — missing/invalid currentPassword 401 — unauthenticated, or wrong password 403 — single-user mode, or no account for caller's WebID Rate-limited 5/min/IP. UI deferred — the issue allows it; auth for a /idp HTML page is its own concern (existing /idp landing is unauthenticated). Endpoint + tests is the unit of value here. Doctor-app or pane work in a follow-up. Tests cover: unauthenticated, missing field, wrong password, happy path (subsequent login fails), purgeData=true wipes filesystem, purgeData=false preserves it, cross-account safety, single-user 403. 8 new tests; 627/627 full suite passes. * idp: address Copilot review on JavaScriptSolidServer#391 — JSDoc accuracy, purge best-effort Two fixes per the review: 1. JSDoc no longer claims a 404 failure mode the code doesn't return. "No account for caller's WebID" lands at 403, not 404 — the caller had a valid token (so they're identifying themselves), just not for an account this server holds. Added the rationale to the comment. 2. Purge is now best-effort. fs.remove() can throw (permissions, FS races, etc.); the account is deleted *before* the purge, so a throw would have surfaced as a 500 over a partially-completed deletion (account gone, pod data lingering). New shape: - try/catch around fs.remove - on throw: log server-side via request.log.error (with err / path / username for the operator log) and continue with purged: false - response.purged accurately signals outcome; an operator can finish the cleanup with the CLI `--purge` against the orphaned directory if needed - raw err message NOT surfaced to the user — file paths and permission detail are sensitive 8/8 delete-account tests still pass. * idp: Copilot review pass 2 on JavaScriptSolidServer#391 — purge by podName, not username Real bug. createAccount() in src/idp/accounts.js normalizes username to lowercase (`username.toLowerCase().trim()`), but the pod directory on disk is created using the original-case input to handleCreatePod. On case-sensitive filesystems these can differ: Account record: { username: "alice", podName: "Alice" } On disk: <dataRoot>/Alice/ The previous purge logic derived its path from account.username, so it would either no-op (path didn't exist) or hit a different directory if one happened to exist at the lowercased name. Both outcomes are wrong: pod data lingers, or the wrong tree is removed. Fix: use account.podName, falling back to account.username only if podName is somehow absent (older account records or edge cases). Same fix applied to the CLI at bin/jss.js:776 — same code path, same bug, would only have been hidden on case-insensitive devboxes (macOS default). Test: new "purgeData: true removes the on-disk pod dir even when it has uppercase letters" — creates a pod with mixed-case name "Greta…", verifies the on-disk dir at the mixed-case path is gone after purge. This would fail on the previous code path on a case-sensitive FS. Also corrected the inline comment about pod-name validation: it was describing the slug regex (allows dots), not the pod-name regex (/^[a-zA-Z0-9_-]+$/, no dots, allows underscores and uppercase). 9/9 delete-account tests pass. * idp: Copilot review pass 3 on JavaScriptSolidServer#391 — JSDoc match, FS-root-safe purge Two fixes: 1. JSDoc no longer says `<dataRoot>/<username>/` — corrected to `<podName>` to match the implementation (which now uses account.podName, falling back to username only if podName is absent). 2. The "candidate is a proper child of dataRoot" guard switched from `candidate.startsWith(root + path.sep)` to `path.relative(root, candidate)`. The startsWith check failed at filesystem roots: if DATA_ROOT is `/`, then `root` is `/` (1 char) and `root + path.sep` is `//` (2 chars), so any legitimate child like `/alice` fails the prefix check and the purge silently no-ops. path.relative handles that correctly — and rejects `..` traversal up front, which is tighter anyway. 9/9 delete-account tests still pass. * idp: pass 4 on JavaScriptSolidServer#391 — fix the OTHER JSDoc line that still said <username> I caught the inline comment in pass 3 but missed this higher up in the function-level JSDoc. Same correction: <username> → <podName> with fallback note.
diff --git a/bin/jss.js b/bin/jss.js
@@ -773,11 +773,15 @@ accountCmd
 
       if (options.purge) {
         const dataRoot = process.env.DATA_ROOT || './data';
-        const podPath = path.join(dataRoot, account.username);
+        // Use podName, not username — createAccount lowercases the
+        // username but pod directories on disk preserve the original
+        // case. On case-sensitive filesystems they can differ.
+        const podPath = path.join(dataRoot, account.podName || account.username);
         await fs.remove(podPath);
         console.log(`\nDeleted account ${account.username}. Pod data removed from ${podPath}.\n`);
       } else {
-        console.log(`\nDeleted account ${account.username}. Pod data preserved at <dataRoot>/${account.username}/ (use --purge to remove).\n`);
+        const podDir = account.podName || account.username;
+        console.log(`\nDeleted account ${account.username}. Pod data preserved at <dataRoot>/${podDir}/ (use --purge to remove).\n`);
       }
     } catch (err) {
       console.error(`Error: ${err.message}`);
diff --git a/src/idp/credentials.js b/src/idp/credentials.js
@@ -5,7 +5,9 @@
 
 import * as jose from 'jose';
 import crypto from 'crypto';
-import { authenticate, findByWebId, updatePassword, verifyPassword } from './accounts.js';
+import fs from 'fs-extra';
+import path from 'path';
+import { authenticate, findByWebId, updatePassword, verifyPassword, deleteAccount } from './accounts.js';
 import { getJwks } from './keys.js';
 import { getWebIdFromRequestAsync } from '../auth/token.js';
 
@@ -272,6 +274,148 @@ export async function handleChangePassword(request, reply) {
   };
 }
 
+/**
+ * Handle DELETE /idp/account (#352)
+ *
+ * Owner-initiated account deletion. Authenticated caller proves
+ * possession via re-entering currentPassword (matches the
+ * password-rotation pattern in #351). Optional `purgeData: true` also
+ * removes the pod's filesystem tree at `<dataRoot>/<podName>/` (falling
+ * back to `<username>` only if podName is absent on the account record).
+ *
+ * Failure modes:
+ *   401 — unauthenticated, or wrong currentPassword
+ *   400 — invalid request body / missing password
+ *   403 — single-user mode (deletion would brick the server until
+ *         re-seed; operator should use the CLI), or no account for the
+ *         caller's WebID. The "no account" case lands here rather than
+ *         404 because the caller had a valid token — they're proving
+ *         identity, just not for an account this server holds.
+ *
+ * Out of scope: invalidating in-flight access tokens. Tokens reference
+ * the WebID; once the account record is gone, follow-up auth attempts
+ * fail at findByWebId(). Existing bearer tokens that don't round-trip
+ * through findByWebId() will appear valid until they expire — same
+ * shape as the password-change endpoint.
+ *
+ * @param {object} request - Fastify request
+ * @param {object} reply - Fastify reply
+ * @param {object} options
+ * @param {boolean} [options.singleUser] - When true, the endpoint
+ *   refuses (deletion would leave the server with no IDP account).
+ */
+export async function handleDeleteAccount(request, reply, options = {}) {
+  // Single-user mode: deletion via HTTP is blocked. The single-user
+  // pod has exactly one account; deleting it bricks the server until
+  // re-seed. The CLI (`jss account delete`) stays available for the
+  // operator who has filesystem access.
+  if (options.singleUser) {
+    return reply.code(403).send({
+      error: 'forbidden',
+      error_description: 'Account deletion via HTTP is disabled in single-user mode. Use the `jss account delete` CLI on the server.',
+    });
+  }
+
+  // 1. Authenticate caller
+  const { webId, error: authError } = await getWebIdFromRequestAsync(request);
+  if (!webId) {
+    return reply.code(401).send({
+      error: 'invalid_token',
+      error_description: authError || 'Authentication required',
+    });
+  }
+
+  // 2. Parse body — same flexible shape as handleChangePassword
+  let body = request.body;
+  if (Buffer.isBuffer(body)) body = body.toString('utf-8');
+  if (typeof body === 'string') {
+    try { body = JSON.parse(body); } catch { body = {}; }
+  }
+  const currentPassword = body?.currentPassword;
+  const purgeData = body?.purgeData === true;
+
+  if (typeof currentPassword !== 'string' || !currentPassword) {
+    return reply.code(400).send({
+      error: 'invalid_request',
+      error_description: 'currentPassword is required (string)',
+    });
+  }
+
+  // 3. Resolve account from caller's WebID
+  const account = await findByWebId(webId);
+  if (!account) {
+    return reply.code(403).send({
+      error: 'forbidden',
+      error_description: 'No account found for authenticated WebID',
+    });
+  }
+
+  // 4. Verify currentPassword (re-auth proof)
+  if (!(await verifyPassword(account, currentPassword))) {
+    return reply.code(401).send({
+      error: 'invalid_grant',
+      error_description: 'Current password is incorrect',
+    });
+  }
+
+  // 5. Delete the account record + indexes
+  await deleteAccount(account.id);
+
+  // 6. Optionally purge the pod's filesystem data. Mirrors the CLI
+  // `--purge` semantics. The path is `<dataRoot>/<podName>/`.
+  //
+  // Use account.podName, NOT account.username: createAccount normalizes
+  // username to lowercase (`username.toLowerCase().trim()`) but the pod
+  // directory on disk is created with the original case (per the input
+  // to handleCreatePod). On case-sensitive filesystems, deriving the
+  // purge path from username would either no-op (path doesn't exist)
+  // or hit a different directory if one exists at the lowercased name.
+  // Pod-name validation regex is /^[a-zA-Z0-9_-]+$/ (alphanum + dash +
+  // underscore; no dots, no traversal sequences) so podName is safe to
+  // join — defensive normalize stays as belt-and-suspenders.
+  //
+  // Best-effort: if fs.remove throws (permissions, transient FS error,
+  // race with another consumer), the account is already deleted and we
+  // shouldn't 500 over the leftover files. Log server-side and return
+  // purged: false so the caller knows pod data may still exist; an
+  // operator can finish the cleanup with a follow-up `rm -rf` or
+  // CLI `--purge` against the now-orphaned directory.
+  let purged = false;
+  if (purgeData) {
+    const dataRoot = process.env.DATA_ROOT || './data';
+    const candidate = path.resolve(dataRoot, account.podName || account.username);
+    const root = path.resolve(dataRoot);
+    // Belt-and-suspenders: refuse to remove anything that isn't a
+    // proper child of the data root. Won't trigger on registered pod
+    // names; protects against config drift / future bugs. Use
+    // path.relative so the check works when dataRoot is a filesystem
+    // root like `/` (where startsWith(root + path.sep) would compare
+    // against `//`, false-negative all valid children).
+    const rel = path.relative(root, candidate);
+    const isProperChild = rel && rel !== '' && !rel.startsWith('..') && !path.isAbsolute(rel);
+    if (isProperChild) {
+      try {
+        await fs.remove(candidate);
+        purged = true;
+      } catch (err) {
+        request.log.error({ err, path: candidate, username: account.username },
+          'Pod data purge failed after account deletion');
+        // Don't surface the raw error to the user (file paths,
+        // permission detail leak); response.purged signals the
+        // outcome.
+      }
+    }
+  }
+
+  reply.header('Cache-Control', 'no-store');
+  reply.header('Pragma', 'no-cache');
+  return {
+    ok: true,
+    webid: account.webId,
+    purged,
+  };
+}
+
 /**
  * Handle GET /idp/credentials
  * Returns info about the credentials endpoint
diff --git a/src/idp/index.js b/src/idp/index.js
@@ -23,6 +23,7 @@ import {
   handleCredentials,
   handleCredentialsInfo,
   handleChangePassword,
+  handleDeleteAccount,
 } from './credentials.js';
 import * as passkey from './passkey.js';
 import { addTrustedIssuer } from '../auth/solid-oidc.js';
@@ -279,6 +280,21 @@ export async function idpPlugin(fastify, options) {
     return handleChangePassword(request, reply);
   });
 
+  // DELETE account - authenticated owner deletes their own account (#352).
+  // Single-user mode is rejected at the handler (deletion would leave the
+  // server with no IDP account until re-seed; CLI is the operator path).
+  fastify.delete('/idp/account', {
+    config: {
+      rateLimit: {
+        max: 5,
+        timeWindow: '1 minute',
+        keyGenerator: (request) => request.ip
+      }
+    }
+  }, async (request, reply) => {
+    return handleDeleteAccount(request, reply, { singleUser });
+  });
+
   // Interaction routes (our custom login/consent UI)
   // These bypass oidc-provider and use our handlers
 
diff --git a/test/idp-delete-account.test.js b/test/idp-delete-account.test.js
