From c54b3306bca62ef5ce5b904d46fd96c250b15985 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 06:33:30 +0200 Subject: [PATCH 1/5] idp: self-service DELETE /idp/account endpoint (#352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authenticated owner can now delete their own account via HTTP. Mirrors the password-rotation pattern from #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 //. 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. --- src/idp/credentials.js | 117 +++++++++++- src/idp/index.js | 16 ++ test/idp-delete-account.test.js | 304 ++++++++++++++++++++++++++++++++ 3 files changed, 436 insertions(+), 1 deletion(-) create mode 100644 test/idp-delete-account.test.js diff --git a/src/idp/credentials.js b/src/idp/credentials.js index 3bf5950c..b1ce5eb2 100644 --- 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,119 @@ 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 //. + * + * 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 + * 404 — no account at all (already deleted) + * + * 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 //, with + // username already validated at registration (#321 alphanum/dash/dot + // rules) so no traversal risk in practice; defensive normalize + // anyway. + let purgedPath = null; + if (purgeData) { + const dataRoot = process.env.DATA_ROOT || './data'; + const candidate = path.resolve(dataRoot, 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 + // usernames; protects against config drift / future bugs. + if (candidate.startsWith(root + path.sep) && candidate !== root) { + await fs.remove(candidate); + purgedPath = candidate; + } + } + + reply.header('Cache-Control', 'no-store'); + reply.header('Pragma', 'no-cache'); + return { + ok: true, + webid: account.webId, + purged: purgedPath !== null, + }; +} + /** * Handle GET /idp/credentials * Returns info about the credentials endpoint diff --git a/src/idp/index.js b/src/idp/index.js index c963609c..740e864f 100644 --- 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 new file mode 100644 index 00000000..2db24ba5 --- /dev/null +++ b/test/idp-delete-account.test.js @@ -0,0 +1,304 @@ +/** + * DELETE /idp/account — authenticated owner deletes their own account (#352) + */ + +import { describe, it, before, after } from 'node:test'; +import assert from 'node:assert'; +import { createServer } from '../src/server.js'; +import fs from 'fs-extra'; +import path from 'path'; +import { createServer as createNetServer } from 'net'; + +const TEST_HOST = 'localhost'; + +function getAvailablePort() { + return new Promise((resolve, reject) => { + const srv = createNetServer(); + srv.on('error', reject); + srv.listen(0, TEST_HOST, () => { + const port = srv.address().port; + srv.close(() => resolve(port)); + }); + }); +} + +async function createPod(baseUrl, name, email, password) { + const res = await fetch(`${baseUrl}/.pods`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ name, email, password }), + }); + const body = await res.json().catch(() => ({})); + assert.strictEqual(res.status, 201, `pod create failed: ${JSON.stringify(body)}`); + return body; +} + +async function loginToken(baseUrl, email, password) { + const res = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email, password }), + }); + const body = await res.json().catch(() => ({})); + assert.strictEqual(res.status, 200, `login failed: ${JSON.stringify(body)}`); + return body.access_token; +} + +describe('DELETE /idp/account — self-delete', () => { + let server; + let baseUrl; + let originalDataRoot; + const DATA_DIR = './test-data-delete-account'; + + before(async () => { + originalDataRoot = process.env.DATA_ROOT; + await fs.remove(DATA_DIR); + await fs.ensureDir(DATA_DIR); + const port = await getAvailablePort(); + baseUrl = `http://${TEST_HOST}:${port}`; + server = createServer({ + logger: false, + root: DATA_DIR, + idp: true, + idpIssuer: baseUrl, + forceCloseConnections: true, + }); + await server.listen({ port, host: TEST_HOST }); + }); + + after(async () => { + await server.close(); + await fs.remove(DATA_DIR); + if (originalDataRoot === undefined) delete process.env.DATA_ROOT; + else process.env.DATA_ROOT = originalDataRoot; + }); + + it('rejects unauthenticated request with 401', async () => { + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ currentPassword: 'whatever' }), + }); + assert.strictEqual(res.status, 401); + }); + + it('rejects missing currentPassword with 400', async () => { + const id = `alice${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + const token = await loginToken(baseUrl, `${id}@example.com`, 'password123'); + + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token}`, + }, + body: JSON.stringify({}), + }); + assert.strictEqual(res.status, 400); + }); + + it('rejects wrong currentPassword with 401, account untouched', async () => { + const id = `bob${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + const token = await loginToken(baseUrl, `${id}@example.com`, 'password123'); + + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token}`, + }, + body: JSON.stringify({ currentPassword: 'wrongpassword' }), + }); + assert.strictEqual(res.status, 401); + + // Account still works + const reLogin = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: `${id}@example.com`, password: 'password123' }), + }); + assert.strictEqual(reLogin.status, 200); + }); + + it('happy path: deletes account; subsequent login fails with 401', async () => { + const id = `carol${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + const token = await loginToken(baseUrl, `${id}@example.com`, 'password123'); + + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token}`, + }, + body: JSON.stringify({ currentPassword: 'password123' }), + }); + assert.strictEqual(res.status, 200); + const body = await res.json(); + assert.strictEqual(body.ok, true); + assert.ok(body.webid.includes(id), 'response carries webid'); + assert.strictEqual(body.purged, false, 'purgeData defaults to false'); + + // Login as the same user now fails + const reLogin = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: `${id}@example.com`, password: 'password123' }), + }); + assert.strictEqual(reLogin.status, 401); + }); + + it('purgeData: true also wipes the pod filesystem tree', async () => { + const id = `dave${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + const token = await loginToken(baseUrl, `${id}@example.com`, 'password123'); + + // Pod tree exists before deletion + const podPath = path.join(DATA_DIR, id); + assert.strictEqual(await fs.pathExists(podPath), true, + 'pod data should exist before deletion'); + + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token}`, + }, + body: JSON.stringify({ currentPassword: 'password123', purgeData: true }), + }); + assert.strictEqual(res.status, 200); + const body = await res.json(); + assert.strictEqual(body.purged, true); + + // Pod tree gone + assert.strictEqual(await fs.pathExists(podPath), false, + 'pod data should be purged'); + }); + + it('purgeData: false (default) preserves the pod filesystem tree', async () => { + const id = `frank${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + const token = await loginToken(baseUrl, `${id}@example.com`, 'password123'); + + const podPath = path.join(DATA_DIR, id); + assert.strictEqual(await fs.pathExists(podPath), true); + + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token}`, + }, + // Note: no purgeData flag at all + body: JSON.stringify({ currentPassword: 'password123' }), + }); + assert.strictEqual(res.status, 200); + + // Account is gone but pod data preserved + assert.strictEqual(await fs.pathExists(podPath), true, + 'pod data should be preserved when purgeData is omitted'); + }); + + it('cross-account: A authenticated, sending B\'s password — fails 401, neither account touched', async () => { + const aId = `eve${Date.now()}`; + const bId = `mallory${Date.now() + 1}`; + await createPod(baseUrl, aId, `${aId}@example.com`, 'apassword123'); + await createPod(baseUrl, bId, `${bId}@example.com`, 'bpassword123'); + + const aToken = await loginToken(baseUrl, `${aId}@example.com`, 'apassword123'); + + // A sends B's password — handler resolves account from A's WebID, so the + // currentPassword must match A's. With B's password it fails 401 (and + // crucially doesn't touch either account). + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${aToken}`, + }, + body: JSON.stringify({ currentPassword: 'bpassword123' }), + }); + assert.strictEqual(res.status, 401); + + // Both accounts still functional + const aLogin = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: `${aId}@example.com`, password: 'apassword123' }), + }); + assert.strictEqual(aLogin.status, 200); + + const bLogin = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: `${bId}@example.com`, password: 'bpassword123' }), + }); + assert.strictEqual(bLogin.status, 200); + }); +}); + +describe('DELETE /idp/account — single-user mode', () => { + let server; + let baseUrl; + let originalDataRoot; + let originalPassword; + const DATA_DIR = './test-data-delete-account-single'; + + before(async () => { + originalDataRoot = process.env.DATA_ROOT; + originalPassword = process.env.JSS_SINGLE_USER_PASSWORD; + process.env.JSS_SINGLE_USER_PASSWORD = 'singletest'; + await fs.remove(DATA_DIR); + await fs.ensureDir(DATA_DIR); + const port = await getAvailablePort(); + baseUrl = `http://${TEST_HOST}:${port}`; + server = createServer({ + logger: false, + root: DATA_DIR, + idp: true, + idpIssuer: baseUrl, + singleUser: true, + singleUserName: 'me', + singleUserPassword: 'singletest', + forceCloseConnections: true, + }); + await server.listen({ port, host: TEST_HOST }); + }); + + after(async () => { + await server.close(); + await fs.remove(DATA_DIR); + if (originalDataRoot === undefined) delete process.env.DATA_ROOT; + else process.env.DATA_ROOT = originalDataRoot; + if (originalPassword === undefined) delete process.env.JSS_SINGLE_USER_PASSWORD; + else process.env.JSS_SINGLE_USER_PASSWORD = originalPassword; + }); + + it('returns 403 in single-user mode (deletion would brick the server)', async () => { + // Even with a valid token, the endpoint refuses in single-user mode. + // Operator must use the CLI (`jss account delete`) instead. + const token = await loginToken(baseUrl, 'me', 'singletest'); + + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token}`, + }, + body: JSON.stringify({ currentPassword: 'singletest' }), + }); + assert.strictEqual(res.status, 403); + const body = await res.json(); + assert.match(body.error_description || '', /single-user/i); + + // Account still functional + const reLogin = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: 'me', password: 'singletest' }), + }); + assert.strictEqual(reLogin.status, 200); + }); +}); From 471f759c8411aed8979496773366d776623e178a Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 06:38:30 +0200 Subject: [PATCH 2/5] =?UTF-8?q?idp:=20address=20Copilot=20review=20on=20#3?= =?UTF-8?q?91=20=E2=80=94=20JSDoc=20accuracy,=20purge=20best-effort?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/idp/credentials.js | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index b1ce5eb2..70faac75 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -287,8 +287,9 @@ export async function handleChangePassword(request, reply) { * 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 - * 404 — no account at all (already deleted) + * 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 @@ -364,7 +365,14 @@ export async function handleDeleteAccount(request, reply, options = {}) { // username already validated at registration (#321 alphanum/dash/dot // rules) so no traversal risk in practice; defensive normalize // anyway. - let purgedPath = null; + // + // 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.username); @@ -373,8 +381,16 @@ export async function handleDeleteAccount(request, reply, options = {}) { // proper child of the data root. Won't trigger on registered // usernames; protects against config drift / future bugs. if (candidate.startsWith(root + path.sep) && candidate !== root) { - await fs.remove(candidate); - purgedPath = candidate; + 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. + } } } @@ -383,7 +399,7 @@ export async function handleDeleteAccount(request, reply, options = {}) { return { ok: true, webid: account.webId, - purged: purgedPath !== null, + purged, }; } From 82589e715c0b649a34bcdae0e1fd2976410081ca Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 06:47:38 +0200 Subject: [PATCH 3/5] =?UTF-8?q?idp:=20Copilot=20review=20pass=202=20on=20#?= =?UTF-8?q?391=20=E2=80=94=20purge=20by=20podName,=20not=20username?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: /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. --- bin/jss.js | 8 ++++++-- src/idp/credentials.js | 19 +++++++++++++------ test/idp-delete-account.test.js | 30 ++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/bin/jss.js b/bin/jss.js index ffd5a53a..48134651 100755 --- 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 /${account.username}/ (use --purge to remove).\n`); + const podDir = account.podName || account.username; + console.log(`\nDeleted account ${account.username}. Pod data preserved at /${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 index 70faac75..a82cc1c7 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -361,10 +361,17 @@ export async function handleDeleteAccount(request, reply, options = {}) { await deleteAccount(account.id); // 6. Optionally purge the pod's filesystem data. Mirrors the CLI - // `--purge` semantics. The path is //, with - // username already validated at registration (#321 alphanum/dash/dot - // rules) so no traversal risk in practice; defensive normalize - // anyway. + // `--purge` semantics. The path is //. + // + // 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 @@ -375,11 +382,11 @@ export async function handleDeleteAccount(request, reply, options = {}) { let purged = false; if (purgeData) { const dataRoot = process.env.DATA_ROOT || './data'; - const candidate = path.resolve(dataRoot, account.username); + 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 - // usernames; protects against config drift / future bugs. + // pod names; protects against config drift / future bugs. if (candidate.startsWith(root + path.sep) && candidate !== root) { try { await fs.remove(candidate); diff --git a/test/idp-delete-account.test.js b/test/idp-delete-account.test.js index 2db24ba5..f8652bd0 100644 --- a/test/idp-delete-account.test.js +++ b/test/idp-delete-account.test.js @@ -177,6 +177,36 @@ describe('DELETE /idp/account — self-delete', () => { 'pod data should be purged'); }); + it('purgeData: true removes the on-disk pod dir even when it has uppercase letters', async () => { + // Regression for the bug where purge derived its path from + // account.username (which createAccount lowercases) instead of + // account.podName (which preserves the original case). On + // case-sensitive filesystems the pod dir at /Greta…/ + // wouldn't match the derived /greta…/ path. + const id = `Greta${Date.now()}`; + await createPod(baseUrl, id, `${id.toLowerCase()}@example.com`, 'password123'); + const token = await loginToken(baseUrl, `${id.toLowerCase()}@example.com`, 'password123'); + + const podPath = path.join(DATA_DIR, id); // mixed-case as created + assert.strictEqual(await fs.pathExists(podPath), true, + 'pod data should exist at the mixed-case path before deletion'); + + const res = await fetch(`${baseUrl}/idp/account`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token}`, + }, + body: JSON.stringify({ currentPassword: 'password123', purgeData: true }), + }); + assert.strictEqual(res.status, 200); + const body = await res.json(); + assert.strictEqual(body.purged, true, 'purge should report success'); + + assert.strictEqual(await fs.pathExists(podPath), false, + 'mixed-case pod dir should be gone (regression: not stranded by username lowercasing)'); + }); + it('purgeData: false (default) preserves the pod filesystem tree', async () => { const id = `frank${Date.now()}`; await createPod(baseUrl, id, `${id}@example.com`, 'password123'); From 2e059c91d68287e9f62ddc510c06479757119fdb Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 06:53:00 +0200 Subject: [PATCH 4/5] =?UTF-8?q?idp:=20Copilot=20review=20pass=203=20on=20#?= =?UTF-8?q?391=20=E2=80=94=20JSDoc=20match,=20FS-root-safe=20purge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes: 1. JSDoc no longer says `//` — corrected to `` 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. --- src/idp/credentials.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index a82cc1c7..f675d5a3 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -361,7 +361,7 @@ export async function handleDeleteAccount(request, reply, options = {}) { await deleteAccount(account.id); // 6. Optionally purge the pod's filesystem data. Mirrors the CLI - // `--purge` semantics. The path is //. + // `--purge` semantics. The path is `//`. // // Use account.podName, NOT account.username: createAccount normalizes // username to lowercase (`username.toLowerCase().trim()`) but the pod @@ -385,9 +385,14 @@ export async function handleDeleteAccount(request, reply, options = {}) { 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. - if (candidate.startsWith(root + path.sep) && candidate !== root) { + // 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; From 88a40cbfe24d700b8249a318b97fc2d05624fc1c Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 07:01:48 +0200 Subject: [PATCH 5/5] =?UTF-8?q?idp:=20pass=204=20on=20#391=20=E2=80=94=20f?= =?UTF-8?q?ix=20the=20OTHER=20JSDoc=20line=20that=20still=20said=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I caught the inline comment in pass 3 but missed this higher up in the function-level JSDoc. Same correction: → with fallback note. --- src/idp/credentials.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index f675d5a3..d09caa0c 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -280,7 +280,8 @@ export async function handleChangePassword(request, reply) { * 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 //. + * removes the pod's filesystem tree at `//` (falling + * back to `` only if podName is absent on the account record). * * Failure modes: * 401 — unauthenticated, or wrong currentPassword