From daf53832993e406132c1b9470a4ab9d1ad1de2c8 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 07:18:03 +0200 Subject: [PATCH 1/8] idp: HTML form for self-service account deletion (#392) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Human-friendly UI on top of the JSON DELETE /idp/account endpoint (#391). Public unauthenticated page at /idp/account/delete; auth happens at submission time via username + password. Matches the existing /idp landing and /idp/register pattern — no JavaScript required, accessibility-first form-encoded POST. ## Form fields - Username or email - Current password (re-entry as proof of possession) - Type your username again to confirm (destructive-action UX guard) - Optional checkbox: also delete pod data ## Routes added - GET /idp/account/delete → render the form (or single-user disabled message) - POST /idp/account/delete → process submission Same 5/min/IP rate-limit as the JSON endpoint. Single-user mode shows the disabled-message page instead of the form on both GET and POST, matching the JSON endpoint's 403 policy. Operator path remains the CLI (`jss account delete`). ## Internal refactor Factored the shared "delete + optional purge" logic into `deleteAccountAndOptionallyPurge(request, account, purgeData)` so the JSON endpoint and form endpoint share the same path. Pod-data purge uses `account.podName` (per #391 pass 2) and path.relative (per #391 pass 3) so the case-sensitivity fix and FS-root edge case carry over. ## Tests added (8) - GET renders form HTML with all required fields - POST happy path → success page + login fails after - POST with purgeData=on wipes pod tree - POST with mismatched confirmUsername → form re-rendered with error + username pre-filled for retry, account untouched - POST with wrong password → form re-rendered with error, account untouched - POST with missing fields → form re-rendered with error - Single-user mode: GET renders disabled message (no form) - Single-user mode: POST also renders disabled message — does NOT delete (verified: account still functional after the POST) 636/636 full suite passes. --- src/idp/credentials.js | 116 ++++++++++++++++ src/idp/index.js | 24 +++- src/idp/views.js | 163 ++++++++++++++++++++++ test/idp-delete-account.test.js | 235 ++++++++++++++++++++++++++++++++ 4 files changed, 537 insertions(+), 1 deletion(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index d09caa0c..f2b2dafa 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -10,6 +10,7 @@ import path from 'path'; import { authenticate, findByWebId, updatePassword, verifyPassword, deleteAccount } from './accounts.js'; import { getJwks } from './keys.js'; import { getWebIdFromRequestAsync } from '../auth/token.js'; +import { accountDeletePage } from './views.js'; /** * Handle POST /idp/credentials @@ -416,6 +417,121 @@ export async function handleDeleteAccount(request, reply, options = {}) { }; } +/** + * Internal: delete an account record + optional pod-data purge. + * Shared between the JSON endpoint (handleDeleteAccount) and the + * form-driven endpoint (handleAccountDeletePost in #392). + * + * Best-effort purge — fs.remove can throw, but the account is already + * gone and we want a clean signal rather than a 500. Path is derived + * from account.podName (NOT username, which createAccount lowercases — + * pod dir on disk is original case, see #391 pass 2). Belt-and- + * suspenders path-relative check rejects ../traversal and works at FS + * roots (see #391 pass 3). + * + * @param {object} request - Fastify request, used only for log access + * @param {object} account - Account record (with username, podName, webId) + * @param {boolean} purgeData - If true, also remove the pod's filesystem tree + * @returns {Promise<{purged: boolean}>} + */ +async function deleteAccountAndOptionallyPurge(request, account, purgeData) { + await deleteAccount(account.id); + + 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); + 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'); + } + } + } + return { purged }; +} + +/** + * Handle POST /idp/account/delete (#392) — form-driven account deletion. + * + * Public unauthenticated endpoint that takes a form-encoded body with + * username + currentPassword + confirmUsername (+ optional purgeData + * checkbox). Authenticates the user via password directly (no Bearer + * token round-trip required), validates the destructive-action UX + * guard, then calls into the same deleteAccount logic as the JSON + * endpoint. Returns HTML — success page on completion, redirect back + * to the GET form with an error message on failure. + * + * Single-user mode: redirects to GET which renders the disabled + * message instead of the form. Same policy as the JSON endpoint. + * + * @param {object} request - Fastify request + * @param {object} reply - Fastify reply + * @param {object} options + * @param {boolean} [options.singleUser] - Single-user mode flag + */ +export async function handleAccountDeleteForm(request, reply, options = {}) { + if (options.singleUser) { + return reply.type('text/html').send(accountDeletePage({ singleUser: true })); + } + + // Parse form-encoded body. Fastify with @fastify/formbody (registered + // for /idp/register etc.) puts fields directly on request.body. + let body = request.body; + if (Buffer.isBuffer(body)) body = body.toString('utf-8'); + if (typeof body === 'string') { + // Manual urlencoded parse fallback if formbody isn't registered for + // this content-type on this route. + try { + const params = new URLSearchParams(body); + body = Object.fromEntries(params); + } catch { body = {}; } + } + + const username = (body?.username || '').trim(); + const currentPassword = body?.currentPassword || ''; + const confirmUsername = (body?.confirmUsername || '').trim(); + const purgeData = body?.purgeData === 'on' || body?.purgeData === true; + + if (!username || !currentPassword || !confirmUsername) { + return reply.type('text/html').send(accountDeletePage({ + error: 'All fields are required.', + username, + })); + } + + // Destructive-action UX guard: typed username must match. Compare + // case-sensitively against the *typed* form value, not the resolved + // account — the user shouldn't be able to typo their way to a delete + // that hits a different (lowercased) record. + if (username !== confirmUsername) { + return reply.type('text/html').send(accountDeletePage({ + error: 'Confirmation does not match the username you entered.', + username, + })); + } + + // Authenticate. authenticate() looks up by username then by email, + // verifies bcrypt, and returns the account (sans password hash) or null. + const account = await authenticate(username, currentPassword); + if (!account) { + return reply.type('text/html').send(accountDeletePage({ + error: 'Username or password is incorrect.', + username, + })); + } + + await deleteAccountAndOptionallyPurge(request, account, purgeData); + + return reply.type('text/html').send(accountDeletePage({ success: true })); +} + /** * Handle GET /idp/credentials * Returns info about the credentials endpoint diff --git a/src/idp/index.js b/src/idp/index.js index 740e864f..b7735a69 100644 --- a/src/idp/index.js +++ b/src/idp/index.js @@ -24,10 +24,11 @@ import { handleCredentialsInfo, handleChangePassword, handleDeleteAccount, + handleAccountDeleteForm, } from './credentials.js'; import * as passkey from './passkey.js'; import { addTrustedIssuer } from '../auth/solid-oidc.js'; -import { landingPage } from './views.js'; +import { landingPage, accountDeletePage } from './views.js'; /** * IdP Fastify Plugin @@ -295,6 +296,27 @@ export async function idpPlugin(fastify, options) { return handleDeleteAccount(request, reply, { singleUser }); }); + // GET account-delete form (#392) - human-friendly UI for #352. Public + // unauthenticated page; auth happens at form submission via password. + fastify.get('/idp/account/delete', async (request, reply) => { + return reply.type('text/html').send(accountDeletePage({ singleUser })); + }); + + // POST account-delete form (#392) - processes the form submission. + // Same rate-limit as the JSON endpoint to keep the destructive-action + // surface consistent across both paths. + fastify.post('/idp/account/delete', { + config: { + rateLimit: { + max: 5, + timeWindow: '1 minute', + keyGenerator: (request) => request.ip + } + } + }, async (request, reply) => { + return handleAccountDeleteForm(request, reply, { singleUser }); + }); + // Interaction routes (our custom login/consent UI) // These bypass oidc-provider and use our handlers diff --git a/src/idp/views.js b/src/idp/views.js index f5f554e9..faf4f0c9 100644 --- a/src/idp/views.js +++ b/src/idp/views.js @@ -533,6 +533,169 @@ export function consentPage(uid, client, params, account) { `; } +/** + * Account-deletion form HTML (#392). + * + * Public unauthenticated page (matches the existing /idp landing and + * /idp/register pattern). Auth happens at submission time: the user + * supplies username + password, which the server validates and uses as + * proof-of-possession for the delete. The "type your username to + * confirm" field is the destructive-action UX guard. + * + * @param {object} opts + * @param {string|null} opts.error - Error message (e.g. wrong password) to display + * @param {string|null} opts.username - Pre-fill on redirect-back-with-error + * @param {boolean} opts.singleUser - When true, render a disabled message + * instead of the form. Deletion via HTTP is blocked in single-user mode + * (would brick the IdP until re-seed); operator path stays the CLI. + * @param {boolean} opts.success - When true, render the post-delete confirmation + */ +export function accountDeletePage({ error = null, username = '', singleUser = false, success = false } = {}) { + if (singleUser) { + return ` + + + + + + Account deletion disabled - Solid IdP + + + +
+ +

Account deletion disabled

+

This server runs in single-user mode. Deleting the single account + via HTTP would leave the server with no IdP account until re-seed, + so this endpoint is disabled.

+

The operator can still delete the account at the shell with:

+ 
jss account delete <username>
+ Back +
+ + + `; + } + + if (success) { + return ` + + + + + + Account deleted - Solid IdP + + + +
+ +

Account deleted

+

Your account has been permanently removed. Any active sessions are now invalid.

+ Return to sign-in +
+ + + `; + } + + return ` + + + + + + Delete account - Solid IdP + + + +
+ +

Delete your account

+ +
+ This is permanent. Your account record, all credentials, and any active + sessions will be removed. Anyone holding your WebID URI will see it become a + tombstone — federated references (ActivityPub, Nostr, type indexes) cannot be + retracted from this server. +
+ + ${error ? `
${escapeHtml(error)}
` : ''} + +
+ + + + + + + + + +
+ + +
+ + +
+ +

+ Cancel and go back +

+
+ + + `; +} + /** * Error page HTML */ diff --git a/test/idp-delete-account.test.js b/test/idp-delete-account.test.js index f8652bd0..327c9d79 100644 --- a/test/idp-delete-account.test.js +++ b/test/idp-delete-account.test.js @@ -269,6 +269,241 @@ describe('DELETE /idp/account — self-delete', () => { }); }); +describe('GET/POST /idp/account/delete — HTML form (#392)', () => { + let server; + let baseUrl; + let originalDataRoot; + const DATA_DIR = './test-data-delete-form'; + + 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('GET renders the form HTML', async () => { + const res = await fetch(`${baseUrl}/idp/account/delete`); + assert.strictEqual(res.status, 200); + const ct = res.headers.get('content-type') || ''; + assert.match(ct, /text\/html/); + const html = await res.text(); + assert.match(html, /]*action="\/idp\/account\/delete"/); + assert.match(html, /name="username"/); + assert.match(html, /name="currentPassword"/); + assert.match(html, /name="confirmUsername"/); + assert.match(html, /name="purgeData"/); + assert.match(html, /Delete my account permanently/); + }); + + it('POST happy path: deletes account, returns success HTML, login fails after', async () => { + const id = `harry${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + + const formBody = new URLSearchParams({ + username: id, + currentPassword: 'password123', + confirmUsername: id, + }); + const res = await fetch(`${baseUrl}/idp/account/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: formBody, + }); + assert.strictEqual(res.status, 200); + const html = await res.text(); + assert.match(html, /Account deleted/); + + // Login 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('POST with purgeData=on also wipes the pod tree', async () => { + const id = `iris${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + const podPath = path.join(DATA_DIR, id); + assert.strictEqual(await fs.pathExists(podPath), true); + + const formBody = new URLSearchParams({ + username: id, + currentPassword: 'password123', + confirmUsername: id, + purgeData: 'on', + }); + const res = await fetch(`${baseUrl}/idp/account/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: formBody, + }); + assert.strictEqual(res.status, 200); + assert.strictEqual(await fs.pathExists(podPath), false); + }); + + it('POST with mismatched confirmUsername renders form with error, account untouched', async () => { + const id = `jack${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + + const formBody = new URLSearchParams({ + username: id, + currentPassword: 'password123', + confirmUsername: 'totally-different', + }); + const res = await fetch(`${baseUrl}/idp/account/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: formBody, + }); + assert.strictEqual(res.status, 200); + const html = await res.text(); + assert.match(html, /Confirmation does not match/); + // Username pre-filled in the form for retry + assert.match(html, new RegExp(`value="${id}"`)); + + // Account intact + const login = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: `${id}@example.com`, password: 'password123' }), + }); + assert.strictEqual(login.status, 200); + }); + + it('POST with wrong password renders form with error, account untouched', async () => { + const id = `kelly${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + + const formBody = new URLSearchParams({ + username: id, + currentPassword: 'wrong', + confirmUsername: id, + }); + const res = await fetch(`${baseUrl}/idp/account/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: formBody, + }); + assert.strictEqual(res.status, 200); + const html = await res.text(); + assert.match(html, /incorrect/i); + + // Account intact + const login = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: `${id}@example.com`, password: 'password123' }), + }); + assert.strictEqual(login.status, 200); + }); + + it('POST with missing fields renders form with error', async () => { + const formBody = new URLSearchParams({ + username: 'someone', + // currentPassword and confirmUsername omitted + }); + const res = await fetch(`${baseUrl}/idp/account/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: formBody, + }); + assert.strictEqual(res.status, 200); + const html = await res.text(); + assert.match(html, /required/i); + }); +}); + +describe('GET/POST /idp/account/delete — single-user mode renders disabled message', () => { + let server; + let baseUrl; + let originalDataRoot; + let originalPassword; + const DATA_DIR = './test-data-delete-form-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('GET renders the disabled message instead of the form', async () => { + const res = await fetch(`${baseUrl}/idp/account/delete`); + assert.strictEqual(res.status, 200); + const html = await res.text(); + assert.match(html, /single-user mode/i); + assert.match(html, /jss account delete/); + // No form + assert.doesNotMatch(html, /]*action="\/idp\/account\/delete"/); + }); + + it('POST also returns the disabled message — does not delete', async () => { + const formBody = new URLSearchParams({ + username: 'me', + currentPassword: 'singletest', + confirmUsername: 'me', + }); + const res = await fetch(`${baseUrl}/idp/account/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: formBody, + }); + assert.strictEqual(res.status, 200); + const html = await res.text(); + assert.match(html, /single-user mode/i); + + // Account still works + const login = await fetch(`${baseUrl}/idp/credentials`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: 'me', password: 'singletest' }), + }); + assert.strictEqual(login.status, 200); + }); +}); + describe('DELETE /idp/account — single-user mode', () => { let server; let baseUrl; From b8a2afc8d0a8a9cb98ede0d66ac4a039245e6ccc Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 07:30:07 +0200 Subject: [PATCH 2/8] =?UTF-8?q?idp:=20address=20Copilot=20review=20on=20#3?= =?UTF-8?q?93=20=E2=80=94=20finish=20refactor=20+=20honest=20copy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six fixes per the review: 1. handleDeleteAccount now actually uses the new deleteAccountAndOptionallyPurge helper. The helper was added in the prior commit but the original function still had its own inline purge block — divergence risk. Now both endpoints share the same path verbatim. 2. JSDoc reference \`handleAccountDeletePost\` → \`handleAccountDeleteForm\` (the actual exported name). Two route methods (GET + POST) live in the same handler; doc updated to reflect that. 3. JSDoc claimed failures "redirect back to the GET form with an error message" — the implementation actually re-renders the form in-place with status 200. Doc corrected to match (single response, no redirect). 4. Form handler now respects the \`purged\` return from the helper. When the user requested purgeData but the purge didn't run (fs threw, path-relative check rejected), the success page surfaces a "your account was deleted but the pod-data purge did not complete" notice. Account deletion succeeded — that part can't be undone — but the operator may need to finish cleanup. 5. Warning copy on the form softened: was "any active sessions will be removed", which the server doesn't actually implement. Now: "future sign-ins with this username will fail" + an italic note that already-issued access tokens may remain usable until their expiry. 6. Same correction on the success-page copy. 17/17 delete-account tests still pass. --- src/idp/credentials.js | 77 ++++++++++++------------------------------ src/idp/views.js | 31 +++++++++++++---- 2 files changed, 45 insertions(+), 63 deletions(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index f2b2dafa..1551a837 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -359,54 +359,10 @@ export async function handleDeleteAccount(request, reply, options = {}) { }); } - // 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 `//`. - // - // 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. - } - } - } + // 5. Delete via the shared helper (also handles optional pod-data purge). + // See deleteAccountAndOptionallyPurge below for the purge semantics + // and rationale (#391 pass 2 / pass 3). + const { purged } = await deleteAccountAndOptionallyPurge(request, account, purgeData); reply.header('Cache-Control', 'no-store'); reply.header('Pragma', 'no-cache'); @@ -458,18 +414,22 @@ async function deleteAccountAndOptionallyPurge(request, account, purgeData) { } /** - * Handle POST /idp/account/delete (#392) — form-driven account deletion. + * Handle GET / POST /idp/account/delete (#392) — form-driven account deletion. * * Public unauthenticated endpoint that takes a form-encoded body with * username + currentPassword + confirmUsername (+ optional purgeData * checkbox). Authenticates the user via password directly (no Bearer * token round-trip required), validates the destructive-action UX - * guard, then calls into the same deleteAccount logic as the JSON - * endpoint. Returns HTML — success page on completion, redirect back - * to the GET form with an error message on failure. + * guard, then calls into the same delete logic as the JSON endpoint + * via deleteAccountAndOptionallyPurge. Returns HTML directly: + * - success → success page + * - any failure → the form re-rendered with an error message and the + * username field pre-filled (no redirect — single response, status + * 200 with the rendered form) * - * Single-user mode: redirects to GET which renders the disabled - * message instead of the form. Same policy as the JSON endpoint. + * Single-user mode: rendered as the disabled-message page instead of + * the form, both on GET and POST. Same policy as the JSON endpoint + * (which 403s with the equivalent message). * * @param {object} request - Fastify request * @param {object} reply - Fastify reply @@ -527,9 +487,14 @@ export async function handleAccountDeleteForm(request, reply, options = {}) { })); } - await deleteAccountAndOptionallyPurge(request, account, purgeData); + const { purged } = await deleteAccountAndOptionallyPurge(request, account, purgeData); - return reply.type('text/html').send(accountDeletePage({ success: true })); + // If the user asked for a purge but it didn't run (fs.remove threw, + // path-relative check rejected, etc.), surface that on the success + // page. Account deletion succeeded — don't roll that back — but the + // operator may need to finish cleanup. Don't leak server paths. + const purgeFailed = purgeData && !purged; + return reply.type('text/html').send(accountDeletePage({ success: true, purgeFailed })); } /** diff --git a/src/idp/views.js b/src/idp/views.js index faf4f0c9..c4ee9264 100644 --- a/src/idp/views.js +++ b/src/idp/views.js @@ -550,7 +550,7 @@ export function consentPage(uid, client, params, account) { * (would brick the IdP until re-seed); operator path stays the CLI. * @param {boolean} opts.success - When true, render the post-delete confirmation */ -export function accountDeletePage({ error = null, username = '', singleUser = false, success = false } = {}) { +export function accountDeletePage({ error = null, username = '', singleUser = false, success = false, purgeFailed = false } = {}) { if (singleUser) { return ` @@ -591,8 +591,20 @@ export function accountDeletePage({ error = null, username = '', singleUser = fa

Account deleted

-

Your account has been permanently removed. Any active sessions are now invalid.

- Return to sign-in +

Your account record has been removed from this server. Future sign-ins + with this username will fail.

+

+ Note: any access tokens already issued may remain usable until they + expire — the server does not currently revoke them on account deletion. +

+ ${purgeFailed ? ` +
+ Your account was deleted, but the pod-data purge did not complete on + this server. Some files may still exist. Contact the operator to + finish the cleanup if needed. +
+ ` : ''} + Return to sign-in
@@ -650,10 +662,15 @@ export function accountDeletePage({ error = null, username = '', singleUser = fa

Delete your account

- This is permanent. Your account record, all credentials, and any active - sessions will be removed. Anyone holding your WebID URI will see it become a - tombstone — federated references (ActivityPub, Nostr, type indexes) cannot be - retracted from this server. + This is permanent. Your account record and credentials will be + removed; future sign-ins with this username will fail. Anyone holding your + WebID URI will see it become a tombstone — federated references (ActivityPub, + Nostr, type indexes) cannot be retracted from this server. +

+ + Note: access tokens already issued may remain usable until they + expire — the server does not currently revoke them on deletion. +
${error ? `
${escapeHtml(error)}
` : ''} From 8448f94af9bf96fe13b3cdbaaf111ea8a7ce8c94 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 07:36:52 +0200 Subject: [PATCH 3/8] =?UTF-8?q?idp:=20Copilot=20review=20pass=202=20on=20#?= =?UTF-8?q?393=20=E2=80=94=20JSDoc=20accuracy,=20drop=20email-or-username?= =?UTF-8?q?=20UX=20confusion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four polish fixes: 1. handleAccountDeleteForm reference in deleteAccountAndOptionallyPurge's JSDoc was still mis-named handleAccountDeletePost (one stray spot the pass-1 fix missed). 2. accountDeletePage's JSDoc updated: "redirect-back-with-error" was wrong (we re-render in place); also documented the new purgeFailed param added in pass 1. 3. + 4. Form label simplified from "Username or email" → "Username". Confirms a user point: JSS doesn't really do email — no SMTP, no verification flow, no recovery flow, the HTML register form doesn't ask for email at all. The "email" field in accounts.js is a data-layer artifact (auto-generates @jss if absent) and isn't surfaced anywhere user-facing. Saying "username or email" on this form mismatched reality and confused the destructive-action confirm field ("Type your username again to confirm"). Now both fields consistently say username, matching what users will actually enter. Server-side authenticate() still tries findByUsername then findByEmail for back-compat with anyone who registered programmatically with a real email; that fallback is just unreachable from this form since the field never accepts email anymore. Cleaning that up across the wider auth surface is a separate PR. 17/17 delete-account tests still pass. --- src/idp/credentials.js | 2 +- src/idp/views.js | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index 1551a837..f268aa70 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -376,7 +376,7 @@ export async function handleDeleteAccount(request, reply, options = {}) { /** * Internal: delete an account record + optional pod-data purge. * Shared between the JSON endpoint (handleDeleteAccount) and the - * form-driven endpoint (handleAccountDeletePost in #392). + * form-driven endpoint (handleAccountDeleteForm in #392). * * Best-effort purge — fs.remove can throw, but the account is already * gone and we want a clean signal rather than a 500. Path is derived diff --git a/src/idp/views.js b/src/idp/views.js index c4ee9264..c949ae54 100644 --- a/src/idp/views.js +++ b/src/idp/views.js @@ -538,17 +538,25 @@ export function consentPage(uid, client, params, account) { * * Public unauthenticated page (matches the existing /idp landing and * /idp/register pattern). Auth happens at submission time: the user - * supplies username + password, which the server validates and uses as - * proof-of-possession for the delete. The "type your username to - * confirm" field is the destructive-action UX guard. + * supplies an identifier (username or email) + password, which the + * server validates and uses as proof-of-possession for the delete. + * The "type your identifier to confirm" field is the destructive-action + * UX guard. + * + * On any failure (wrong password, mismatched confirmation, etc.) the + * handler re-renders this same form in place at status 200 with an + * error message and the identifier field pre-filled — no redirect. * * @param {object} opts * @param {string|null} opts.error - Error message (e.g. wrong password) to display - * @param {string|null} opts.username - Pre-fill on redirect-back-with-error + * @param {string} opts.username - Pre-fill the identifier field on re-render after error * @param {boolean} opts.singleUser - When true, render a disabled message * instead of the form. Deletion via HTTP is blocked in single-user mode * (would brick the IdP until re-seed); operator path stays the CLI. * @param {boolean} opts.success - When true, render the post-delete confirmation + * @param {boolean} opts.purgeFailed - When true (only on success), include a + * notice that the user requested a pod-data purge but it didn't complete. + * Account deletion still succeeded. */ export function accountDeletePage({ error = null, username = '', singleUser = false, success = false, purgeFailed = false } = {}) { if (singleUser) { @@ -676,10 +684,10 @@ export function accountDeletePage({ error = null, username = '', singleUser = fa ${error ? `
${escapeHtml(error)}
` : ''}
- + + placeholder="alice"> Date: Sat, 9 May 2026 07:42:44 +0200 Subject: [PATCH 4/8] =?UTF-8?q?idp:=20pass=203=20on=20#393=20=E2=80=94=20J?= =?UTF-8?q?SDoc=20/=20copy=20accuracy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small fixes: 1. handleAccountDeleteForm JSDoc clarifies it's POST-only; GET is a separate route in index.js that just renders the form via accountDeletePage. Pass-1 wording was 'GET / POST' which implied both routes flow through this handler. 2. accountDeletePage's JSDoc still mentioned 'identifier (username or email)' from before the email simplification in pass 2. Now says 'username + password' to match the actual UI. 3. The checkbox copy promised 'Removes //' but the purge path is derived from account.podName (which can differ from the lowercased username on case-sensitive FS, per #391 pass 2). Softened to 'Removes the pod folder on disk' — accurate without leaking the path shape. 17/17 tests pass. --- src/idp/credentials.js | 12 ++++++++---- src/idp/views.js | 14 ++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index f268aa70..426a3d70 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -414,7 +414,7 @@ async function deleteAccountAndOptionallyPurge(request, account, purgeData) { } /** - * Handle GET / POST /idp/account/delete (#392) — form-driven account deletion. + * Handle POST /idp/account/delete (#392) — form-driven account deletion. * * Public unauthenticated endpoint that takes a form-encoded body with * username + currentPassword + confirmUsername (+ optional purgeData @@ -427,9 +427,13 @@ async function deleteAccountAndOptionallyPurge(request, account, purgeData) { * username field pre-filled (no redirect — single response, status * 200 with the rendered form) * - * Single-user mode: rendered as the disabled-message page instead of - * the form, both on GET and POST. Same policy as the JSON endpoint - * (which 403s with the equivalent message). + * GET /idp/account/delete is a separate route that just renders the + * form via accountDeletePage(); see src/idp/index.js. This handler is + * POST-only. + * + * Single-user mode: this handler short-circuits to the disabled-message + * page (matches the GET route's behavior). Same policy as the JSON + * endpoint, which 403s with the equivalent message. * * @param {object} request - Fastify request * @param {object} reply - Fastify reply diff --git a/src/idp/views.js b/src/idp/views.js index c949ae54..4c4f858e 100644 --- a/src/idp/views.js +++ b/src/idp/views.js @@ -538,10 +538,9 @@ export function consentPage(uid, client, params, account) { * * Public unauthenticated page (matches the existing /idp landing and * /idp/register pattern). Auth happens at submission time: the user - * supplies an identifier (username or email) + password, which the - * server validates and uses as proof-of-possession for the delete. - * The "type your identifier to confirm" field is the destructive-action - * UX guard. + * supplies username + password, which the server validates and uses as + * proof-of-possession for the delete. The "type your username again to + * confirm" field is the destructive-action UX guard. * * On any failure (wrong password, mismatched confirmation, etc.) the * handler re-renders this same form in place at status 200 with an @@ -700,10 +699,9 @@ export function accountDeletePage({ error = null, username = '', singleUser = fa
From 63aae7f5d253695925a2f4f921b3dd1046c0de8c Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 07:48:12 +0200 Subject: [PATCH 5/8] =?UTF-8?q?idp:=20pass=204=20on=20#393=20=E2=80=94=20f?= =?UTF-8?q?ix=20body-parsing=20comment=20to=20match=20reality?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment claimed @fastify/formbody puts fields directly on request.body, but no such plugin is installed. JSS's wildcard parseAs:'buffer' content parser at server.js:190 means request.body arrives as a Buffer; the URLSearchParams parse below is what's actually load-bearing. Comment updated to describe what the code genuinely does. --- src/idp/credentials.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index 426a3d70..b8ac9d70 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -445,13 +445,15 @@ export async function handleAccountDeleteForm(request, reply, options = {}) { return reply.type('text/html').send(accountDeletePage({ singleUser: true })); } - // Parse form-encoded body. Fastify with @fastify/formbody (registered - // for /idp/register etc.) puts fields directly on request.body. + // Parse form-encoded body. JSS registers a wildcard parseAs:'buffer' + // content parser (server.js:190), so request.body arrives here as a + // Buffer. We coerce to a string and parse the application/x-www-form- + // urlencoded shape via URLSearchParams. (No @fastify/formbody is + // installed; doing it inline keeps this self-contained and matches + // what handleChangePassword does for JSON.) let body = request.body; if (Buffer.isBuffer(body)) body = body.toString('utf-8'); if (typeof body === 'string') { - // Manual urlencoded parse fallback if formbody isn't registered for - // this content-type on this route. try { const params = new URLSearchParams(body); body = Object.fromEntries(params); From 8f83cb93bec78a5e41f5466ca8cef472a53d1787 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 07:58:21 +0200 Subject: [PATCH 6/8] =?UTF-8?q?idp:=20pass=205=20on=20#393=20=E2=80=94=20s?= =?UTF-8?q?ide-effect-free=20verify,=20honest=20copy,=20403=20single-user,?= =?UTF-8?q?=20flip=20purge=20default=20to=20ON?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five fixes (4 from Copilot, 1 from maintainer call to flip the default): 1. handleAccountDeleteForm now uses findByUsername + verifyPassword instead of authenticate(). authenticate() writes lastLogin as a side effect, which is wrong shape for a destructive proof-of- possession check (and would fail the deletion if the account file weren't writable). Mirrors handleChangePassword and the JSON handleDeleteAccount. 2. Warning copy corrected. Previous text said "any active sessions will be removed" + "WebID URI will become a tombstone" — the server does neither. Now: "Future sign-ins with this username will fail" + "external links to your WebID URI will become dangling" (since pod data is being wiped, the URI no longer resolves). 3. Single-user mode now returns 403 from both GET and POST routes (matched HTML body kept). Consistency: /idp/register's disabled-route policy is 403, the JSON DELETE endpoint is 403, and now the form path is too. 4. Body-parsing comment corrected: was claiming @fastify/formbody is registered (it isn't); JSS uses parseAs:'buffer' for `*` so body arrives as a Buffer and the URLSearchParams parse is the load-bearing logic. 5. Form's purge default flipped from off → ON. The checkbox is now `keepData` (inverse of the JSON endpoint's `purgeData`), default unchecked = purge-on. A user filling out the form is leaving the server; wiping their pod data with the account is the expected shape, and "Keep my pod data on this server" is the explicit opt-out. The JSON DELETE endpoint keeps `purgeData: false` as the default (matches the CLI's --purge being opt-in for operator scripts; programmatic callers shouldn't accidentally wipe data). Form and JSON endpoint diverge deliberately because the use cases differ. Tests: - happy path now expects pod data wiped by default + login fails - new test for keepData=on (opts out of purge; account still deleted, pod data preserved) - single-user GET / POST now expect 403 - form-renders test now checks for name="keepData" 17/17 delete-account tests pass. --- src/idp/credentials.js | 40 ++++++++++++++++++++--------- src/idp/index.js | 9 ++++++- src/idp/views.js | 19 +++++++------- test/idp-delete-account.test.js | 45 ++++++++++++++++++++++++++------- 4 files changed, 82 insertions(+), 31 deletions(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index b8ac9d70..55367707 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -7,7 +7,7 @@ import * as jose from 'jose'; import crypto from 'crypto'; import fs from 'fs-extra'; import path from 'path'; -import { authenticate, findByWebId, updatePassword, verifyPassword, deleteAccount } from './accounts.js'; +import { authenticate, findByUsername, findByWebId, updatePassword, verifyPassword, deleteAccount } from './accounts.js'; import { getJwks } from './keys.js'; import { getWebIdFromRequestAsync } from '../auth/token.js'; import { accountDeletePage } from './views.js'; @@ -417,11 +417,13 @@ async function deleteAccountAndOptionallyPurge(request, account, purgeData) { * Handle POST /idp/account/delete (#392) — form-driven account deletion. * * Public unauthenticated endpoint that takes a form-encoded body with - * username + currentPassword + confirmUsername (+ optional purgeData - * checkbox). Authenticates the user via password directly (no Bearer - * token round-trip required), validates the destructive-action UX - * guard, then calls into the same delete logic as the JSON endpoint - * via deleteAccountAndOptionallyPurge. Returns HTML directly: + * username + currentPassword + confirmUsername (+ optional keepData + * opt-out checkbox; default behavior is purge-on for the leaving-user + * UX, opposite the JSON endpoint's purge-off default). Authenticates + * the user via password directly (no Bearer token round-trip), validates + * the destructive-action UX guard, then calls into the same delete + * logic as the JSON endpoint via deleteAccountAndOptionallyPurge. + * Returns HTML directly: * - success → success page * - any failure → the form re-rendered with an error message and the * username field pre-filled (no redirect — single response, status @@ -442,7 +444,10 @@ async function deleteAccountAndOptionallyPurge(request, account, purgeData) { */ export async function handleAccountDeleteForm(request, reply, options = {}) { if (options.singleUser) { - return reply.type('text/html').send(accountDeletePage({ singleUser: true })); + // 403 matches the GET route, /idp/register's disabled-route policy, + // and the JSON DELETE endpoint's 403 — consistent status across + // every disabled-in-single-user surface. + return reply.code(403).type('text/html').send(accountDeletePage({ singleUser: true })); } // Parse form-encoded body. JSS registers a wildcard parseAs:'buffer' @@ -463,7 +468,14 @@ export async function handleAccountDeleteForm(request, reply, options = {}) { const username = (body?.username || '').trim(); const currentPassword = body?.currentPassword || ''; const confirmUsername = (body?.confirmUsername || '').trim(); - const purgeData = body?.purgeData === 'on' || body?.purgeData === true; + // Form field is `keepData` (inverse of the JSON endpoint's `purgeData`) + // so the form's default is purge-on: a user who is leaving the server + // probably wants their files gone too. Checking "Keep my pod data" is + // the opt-out. JSON endpoint (DELETE /idp/account) keeps the + // explicit `purgeData` shape that matches the CLI's default-off + // semantics for operator scripts; the form intentionally diverges. + const keepData = body?.keepData === 'on' || body?.keepData === true; + const purgeData = !keepData; if (!username || !currentPassword || !confirmUsername) { return reply.type('text/html').send(accountDeletePage({ @@ -483,10 +495,14 @@ export async function handleAccountDeleteForm(request, reply, options = {}) { })); } - // Authenticate. authenticate() looks up by username then by email, - // verifies bcrypt, and returns the account (sans password hash) or null. - const account = await authenticate(username, currentPassword); - if (!account) { + // Look up + verify password without side effects. authenticate() is + // tempting (looks up + verifies in one call) but writes lastLogin on + // success — wrong shape for a destructive proof-of-possession check + // (and would fail the deletion if the account file weren't writable). + // Mirrors handleChangePassword / handleDeleteAccount which both use + // verifyPassword for the same reason. + const account = await findByUsername(username); + if (!account || !(await verifyPassword(account, currentPassword))) { return reply.type('text/html').send(accountDeletePage({ error: 'Username or password is incorrect.', username, diff --git a/src/idp/index.js b/src/idp/index.js index b7735a69..3b84f638 100644 --- a/src/idp/index.js +++ b/src/idp/index.js @@ -298,8 +298,15 @@ export async function idpPlugin(fastify, options) { // GET account-delete form (#392) - human-friendly UI for #352. Public // unauthenticated page; auth happens at form submission via password. + // Single-user mode returns 403 to stay consistent with /idp/register's + // disabled-route policy and the JSON DELETE /idp/account endpoint + // (which also 403s in single-user mode). Body is still HTML so a + // browser visitor sees the explanation. fastify.get('/idp/account/delete', async (request, reply) => { - return reply.type('text/html').send(accountDeletePage({ singleUser })); + if (singleUser) { + return reply.code(403).type('text/html').send(accountDeletePage({ singleUser: true })); + } + return reply.type('text/html').send(accountDeletePage({ singleUser: false })); }); // POST account-delete form (#392) - processes the form submission. diff --git a/src/idp/views.js b/src/idp/views.js index 4c4f858e..22f9118e 100644 --- a/src/idp/views.js +++ b/src/idp/views.js @@ -669,10 +669,11 @@ export function accountDeletePage({ error = null, username = '', singleUser = fa

Delete your account

- This is permanent. Your account record and credentials will be - removed; future sign-ins with this username will fail. Anyone holding your - WebID URI will see it become a tombstone — federated references (ActivityPub, - Nostr, type indexes) cannot be retracted from this server. + This is permanent. Your account record, credentials, and pod data + (every file you've stored — including your WebID profile document) will be removed. + Future sign-ins with this username will fail. Federated references (ActivityPub + follows, Nostr relays, type indexes) cannot be retracted from this server, so + external links to your WebID URI will become dangling.

Note: access tokens already issued may remain usable until they @@ -697,11 +698,11 @@ export function accountDeletePage({ error = null, username = '', singleUser = fa placeholder="Must match the username above">
- -
diff --git a/test/idp-delete-account.test.js b/test/idp-delete-account.test.js index 327c9d79..a9ff56b3 100644 --- a/test/idp-delete-account.test.js +++ b/test/idp-delete-account.test.js @@ -308,18 +308,28 @@ describe('GET/POST /idp/account/delete — HTML form (#392)', () => { assert.match(html, /name="username"/); assert.match(html, /name="currentPassword"/); assert.match(html, /name="confirmUsername"/); - assert.match(html, /name="purgeData"/); + // Form's checkbox is `keepData` (inverse of the JSON endpoint's + // purgeData) so the form's default is purge-on for the leaving-user UX. + assert.match(html, /name="keepData"/); assert.match(html, /Delete my account permanently/); }); - it('POST happy path: deletes account, returns success HTML, login fails after', async () => { + it('POST happy path: deletes account AND wipes pod data by default; login fails after', async () => { + // Form's default is purge-on (the user is leaving — wipe everything). + // The JSON DELETE endpoint keeps purge-off as default (matches CLI for + // programmatic / operator use); the form diverges deliberately for the + // leaving-user UX. const id = `harry${Date.now()}`; await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + const podPath = path.join(DATA_DIR, id); + assert.strictEqual(await fs.pathExists(podPath), true, + 'pod tree should exist before deletion'); const formBody = new URLSearchParams({ username: id, currentPassword: 'password123', confirmUsername: id, + // No keepData — defaults to purge-on }); const res = await fetch(`${baseUrl}/idp/account/delete`, { method: 'POST', @@ -337,9 +347,13 @@ describe('GET/POST /idp/account/delete — HTML form (#392)', () => { body: JSON.stringify({ email: `${id}@example.com`, password: 'password123' }), }); assert.strictEqual(reLogin.status, 401); + + // Pod data also wiped (default behavior on the form) + assert.strictEqual(await fs.pathExists(podPath), false, + 'pod data should be wiped by default on the form path'); }); - it('POST with purgeData=on also wipes the pod tree', async () => { + it('POST with keepData=on opts out of the purge, account still deleted', async () => { const id = `iris${Date.now()}`; await createPod(baseUrl, id, `${id}@example.com`, 'password123'); const podPath = path.join(DATA_DIR, id); @@ -349,7 +363,7 @@ describe('GET/POST /idp/account/delete — HTML form (#392)', () => { username: id, currentPassword: 'password123', confirmUsername: id, - purgeData: 'on', + keepData: 'on', }); const res = await fetch(`${baseUrl}/idp/account/delete`, { method: 'POST', @@ -357,7 +371,18 @@ describe('GET/POST /idp/account/delete — HTML form (#392)', () => { body: formBody, }); assert.strictEqual(res.status, 200); - assert.strictEqual(await fs.pathExists(podPath), false); + + // Account gone + 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); + + // Pod data preserved (the user opted to keep it) + assert.strictEqual(await fs.pathExists(podPath), true, + 'keepData=on should preserve pod data even though account is deleted'); }); it('POST with mismatched confirmUsername renders form with error, account untouched', async () => { @@ -469,9 +494,11 @@ describe('GET/POST /idp/account/delete — single-user mode renders disabled mes else process.env.JSS_SINGLE_USER_PASSWORD = originalPassword; }); - it('GET renders the disabled message instead of the form', async () => { + it('GET returns 403 with the disabled-message HTML', async () => { + // 403 keeps single-user disabled routes consistent: /idp/register, + // the JSON DELETE /idp/account, and this GET all 403 in single-user. const res = await fetch(`${baseUrl}/idp/account/delete`); - assert.strictEqual(res.status, 200); + assert.strictEqual(res.status, 403); const html = await res.text(); assert.match(html, /single-user mode/i); assert.match(html, /jss account delete/); @@ -479,7 +506,7 @@ describe('GET/POST /idp/account/delete — single-user mode renders disabled mes assert.doesNotMatch(html, /]*action="\/idp\/account\/delete"/); }); - it('POST also returns the disabled message — does not delete', async () => { + it('POST returns 403 with disabled-message HTML — does not delete', async () => { const formBody = new URLSearchParams({ username: 'me', currentPassword: 'singletest', @@ -490,7 +517,7 @@ describe('GET/POST /idp/account/delete — single-user mode renders disabled mes headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, body: formBody, }); - assert.strictEqual(res.status, 200); + assert.strictEqual(res.status, 403); const html = await res.text(); assert.match(html, /single-user mode/i); From 93e8f40c27acde65d0b6a9a346016d49e22597b6 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 08:11:17 +0200 Subject: [PATCH 7/8] =?UTF-8?q?idp:=20pass=207=20on=20#393=20=E2=80=94=20c?= =?UTF-8?q?onditional=20warning=20copy,=20log=20path-safety=20rejection,?= =?UTF-8?q?=20fix=20confirm=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes: 1. Warning copy now describes the conditional behavior accurately: 'By default your pod data is also wiped — check the box below if you want to keep it.' Previous wording said pod data 'will be removed' unconditionally, which becomes wrong if the user checks the keep-data opt-out. 2. Path-safety rejection (when isProperChild is false) now logs a warn with path/dataRoot/podName context so operators can diagnose any 'purge did not complete' message that wasn't a fs.remove throw. No path leaks to the client; signal stays in server logs. 3. Comment on the destructive-action UX guard was misleading. It claimed the case-sensitive compare prevents typos hitting a different (lowercased) record — but findByUsername lowercases internally, so 'Alice' + 'Alice' both resolve to the 'alice' record. The compare is purely the typing-it-twice confirmation pattern (typo catch / accidental-submit guard); doesn't gate which record gets deleted. Comment now reflects what the code actually does. 17/17 tests pass. --- src/idp/credentials.js | 20 ++++++++++++++++---- src/idp/views.js | 11 ++++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index 55367707..0199f6a8 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -408,6 +408,14 @@ async function deleteAccountAndOptionallyPurge(request, account, purgeData) { request.log.error({ err, path: candidate, username: account.username }, 'Pod data purge failed after account deletion'); } + } else { + // Belt-and-suspenders rejection — shouldn't trigger on registered + // pod names but logging it surfaces config drift (e.g. someone + // changed dataRoot, podName field is malformed, etc.) and makes + // the "purge did not complete" UX message diagnosable from the + // operator's logs without leaking the path to the client. + request.log.warn({ path: candidate, dataRoot: root, podName: account.podName, username: account.username }, + 'Pod data purge skipped: candidate path is not a proper child of dataRoot'); } } return { purged }; @@ -484,10 +492,14 @@ export async function handleAccountDeleteForm(request, reply, options = {}) { })); } - // Destructive-action UX guard: typed username must match. Compare - // case-sensitively against the *typed* form value, not the resolved - // account — the user shouldn't be able to typo their way to a delete - // that hits a different (lowercased) record. + // Destructive-action UX guard: the user must type the same string + // twice. The string-equality check is case-sensitive on the typed + // form values; that's purely the typing-it-again confirmation + // pattern (catch typos / accidental submits). Note: findByUsername() + // lowercases internally, so "Alice" + "Alice" both resolve to the + // same account record as "alice" — the case-sensitive comparison + // here doesn't gate which record gets deleted, only whether the + // user typed the same thing twice. if (username !== confirmUsername) { return reply.type('text/html').send(accountDeletePage({ error: 'Confirmation does not match the username you entered.', diff --git a/src/idp/views.js b/src/idp/views.js index 22f9118e..4bc6d9a2 100644 --- a/src/idp/views.js +++ b/src/idp/views.js @@ -669,11 +669,12 @@ export function accountDeletePage({ error = null, username = '', singleUser = fa

Delete your account

- This is permanent. Your account record, credentials, and pod data - (every file you've stored — including your WebID profile document) will be removed. - Future sign-ins with this username will fail. Federated references (ActivityPub - follows, Nostr relays, type indexes) cannot be retracted from this server, so - external links to your WebID URI will become dangling. + This is permanent. Your account record and credentials will be + removed; future sign-ins with this username will fail. By default, your pod + data (every file you've stored, including your WebID profile document) is + also wiped — check the box below if you want to keep it. Federated references + (ActivityPub follows, Nostr relays, type indexes) cannot be retracted from + this server.

Note: access tokens already issued may remain usable until they From 91275c204c5a270c803617b55a063644b8e28c4d Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Sat, 9 May 2026 08:21:27 +0200 Subject: [PATCH 8/8] =?UTF-8?q?idp:=20pass=208=20on=20#393=20=E2=80=94=20a?= =?UTF-8?q?nti-clickjacking=20+=20no-store=20headers=20on=20every=20delete?= =?UTF-8?q?-form=20response?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real security findings. The delete-account form takes a current-password re-entry and the success/error pages echo state — neither should be cacheable or embeddable in an iframe. Both threats (cache replay, clickjacking) are real on a destructive endpoint. Added a small exported helper: setNoCacheClickjackHeaders(reply): Cache-Control: no-store Pragma: no-cache X-Frame-Options: DENY Content-Security-Policy: frame-ancestors 'none' Wired into: - GET /idp/account/delete (form + 403 disabled-message variant) - POST handler entry point — covers every return path (success, error re-render, single-user 403, missing-field, mismatched confirm, wrong password) Tests added (2): - GET response carries all four headers - POST response carries them on both error (missing fields) and success (full delete) paths 19/19 delete-account tests pass. --- src/idp/credentials.js | 24 ++++++++++++++++++++ src/idp/index.js | 5 ++++- test/idp-delete-account.test.js | 39 +++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/idp/credentials.js b/src/idp/credentials.js index 0199f6a8..b74b39a6 100644 --- a/src/idp/credentials.js +++ b/src/idp/credentials.js @@ -373,6 +373,26 @@ export async function handleDeleteAccount(request, reply, options = {}) { }; } +/** + * Apply anti-clickjacking + no-store cache headers to a response. + * Used on every account-deletion HTML response (form, success, error + * re-render, disabled-message page) and on the corresponding routes. + * + * - Cache-Control: no-store — destructive form/success page should + * never sit in a shared cache; if a token gets shared via the URL + * the browser shouldn't replay the action from cache either. + * - X-Frame-Options + frame-ancestors — block clickjacking. + * Embedding this form in a hostile iframe and tricking a user into + * submitting a captured action is exactly the threat shape these + * headers exist to mitigate. + */ +export function setNoCacheClickjackHeaders(reply) { + reply.header('Cache-Control', 'no-store'); + reply.header('Pragma', 'no-cache'); + reply.header('X-Frame-Options', 'DENY'); + reply.header('Content-Security-Policy', "frame-ancestors 'none'"); +} + /** * Internal: delete an account record + optional pod-data purge. * Shared between the JSON endpoint (handleDeleteAccount) and the @@ -451,6 +471,10 @@ async function deleteAccountAndOptionallyPurge(request, account, purgeData) { * @param {boolean} [options.singleUser] - Single-user mode flag */ export async function handleAccountDeleteForm(request, reply, options = {}) { + // Every response from this handler is a destructive-action surface + // that re-takes the user's password — never cache, never embed. + setNoCacheClickjackHeaders(reply); + if (options.singleUser) { // 403 matches the GET route, /idp/register's disabled-route policy, // and the JSON DELETE endpoint's 403 — consistent status across diff --git a/src/idp/index.js b/src/idp/index.js index 3b84f638..0efe72c9 100644 --- a/src/idp/index.js +++ b/src/idp/index.js @@ -25,6 +25,7 @@ import { handleChangePassword, handleDeleteAccount, handleAccountDeleteForm, + setNoCacheClickjackHeaders, } from './credentials.js'; import * as passkey from './passkey.js'; import { addTrustedIssuer } from '../auth/solid-oidc.js'; @@ -301,8 +302,10 @@ export async function idpPlugin(fastify, options) { // Single-user mode returns 403 to stay consistent with /idp/register's // disabled-route policy and the JSON DELETE /idp/account endpoint // (which also 403s in single-user mode). Body is still HTML so a - // browser visitor sees the explanation. + // browser visitor sees the explanation. Every response sets + // anti-clickjacking + no-store headers — destructive-action page. fastify.get('/idp/account/delete', async (request, reply) => { + setNoCacheClickjackHeaders(reply); if (singleUser) { return reply.code(403).type('text/html').send(accountDeletePage({ singleUser: true })); } diff --git a/test/idp-delete-account.test.js b/test/idp-delete-account.test.js index a9ff56b3..da8cb985 100644 --- a/test/idp-delete-account.test.js +++ b/test/idp-delete-account.test.js @@ -314,6 +314,45 @@ describe('GET/POST /idp/account/delete — HTML form (#392)', () => { assert.match(html, /Delete my account permanently/); }); + it('GET sets anti-clickjacking + no-store headers', async () => { + // Destructive-action page (form for account deletion) — must not + // be cacheable or embeddable in an iframe. + const res = await fetch(`${baseUrl}/idp/account/delete`); + assert.strictEqual(res.status, 200); + assert.match(res.headers.get('cache-control') || '', /no-store/i); + assert.strictEqual(res.headers.get('x-frame-options'), 'DENY'); + assert.match(res.headers.get('content-security-policy') || '', /frame-ancestors 'none'/); + }); + + it('POST sets the same security headers on success and error responses', async () => { + // Error response: missing fields + const errRes = await fetch(`${baseUrl}/idp/account/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ username: 'x' }), + }); + assert.match(errRes.headers.get('cache-control') || '', /no-store/i); + assert.strictEqual(errRes.headers.get('x-frame-options'), 'DENY'); + assert.match(errRes.headers.get('content-security-policy') || '', /frame-ancestors 'none'/); + + // Success response: full delete flow + const id = `lyle${Date.now()}`; + await createPod(baseUrl, id, `${id}@example.com`, 'password123'); + const okRes = await fetch(`${baseUrl}/idp/account/delete`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ + username: id, + currentPassword: 'password123', + confirmUsername: id, + }), + }); + assert.strictEqual(okRes.status, 200); + assert.match(okRes.headers.get('cache-control') || '', /no-store/i); + assert.strictEqual(okRes.headers.get('x-frame-options'), 'DENY'); + assert.match(okRes.headers.get('content-security-policy') || '', /frame-ancestors 'none'/); + }); + it('POST happy path: deletes account AND wipes pod data by default; login fails after', async () => { // Form's default is purge-on (the user is leaving — wipe everything). // The JSON DELETE endpoint keeps purge-off as default (matches CLI for