Skip to content

Commit b03192b

Browse files
melvincarvalhomelvincarvalho
authored
idp: add 'Sign in as a different user' on consent page (JavaScriptSolidServer#384) (JavaScriptSolidServer#385)
* idp: add 'Sign in as a different user' on consent page (JavaScriptSolidServer#384) Adds a switch-account link to the OIDC Authorize Access page next to the "Signed in as <email>" line, so users with multiple identities don't have to manually clear browser state to switch. Implementation (~70 LOC across 3 files): - src/idp/interactions.js: new handleSwitchAccount() — looks up the in-flight Interaction, destroys the bound oidc-provider Session via provider.Session.findByUid(...).destroy(), mutates the interaction's prompt back to {name:'login'} (preserving the original authz params), clears the user-agent's session cookies, and redirects to the same /idp/interaction/:uid URL. handleInteractionGet then re-renders as the login page; the resume action picks up the original client_id / redirect_uri / state on next login. Returns 404 on missing interaction (cleaner than abort's 500). - src/idp/index.js: import + register POST /idp/interaction/:uid/switch. - src/idp/views.js: small inline-style link inside the "Signed in as …" paragraph on consentPage. Uses an unstyled <button> in a <form> so it's a real POST (no GET-via-link CSRF surface). Why re-use the same interaction uid (rather than ending the session and issuing a fresh /idp/auth) — preserves the OIDC params (state, nonce, PKCE challenge, redirect_uri) so the requesting app's flow continues unchanged. New /idp/auth would also re-trigger loadExistingGrant which might silently auto-approve from another stale grant. Verified: - 48/48 idp.test.js tests pass (no regressions to existing flows) - Boot smoke test: /idp/interaction/nonexistent/switch returns 404, /idp landing renders 200 - Static analysis of the consent flow matches the agent's research: Session.findByUid + .destroy() in oidc-provider 9.6 work cleanly, `prompt` and `session` are in Interaction's IN_PAYLOAD so the mutation persists through the filesystem adapter. Out of scope (follow-ups worth filing if pushed): - CSRF tokens on consent forms (matches existing confirm/abort behavior; the interaction uid is the unguessable token) - Multi-account picker (vs a hard switch) for users with >2 stored sessions * idp: address Copilot review on JavaScriptSolidServer#384 — prompt validation, error masking, HTML, tests Four fixes per the review pass: 1. handleSwitchAccount now validates interaction.prompt.name === 'consent' before mutating. Returns 400 otherwise. Prevents a crafted POST from corrupting an in-flight non-consent interaction (login, passkey, etc.). Also clears interaction.result so a stale result.login from a prior identity can't influence the next resume. 2. The 500 error path no longer surfaces err.message to the browser. Adapter / fs / oidc-provider errors can leak file paths and other internals; full error is already going to request.log.error. Generic "Something went wrong" is what the user sees. 3. <p> with <form> inside it was invalid HTML — browsers implicitly close the <p> before the <form>, breaking the intended inline layout. The "Signed in as <email> · Sign in as a different user" element is now a <div> with the same flex styling. 4. Added test coverage in test/idp.test.js — three tests for the new route: success path (302 + interaction mutated to login + cookies cleared), 400 on non-consent prompt (interaction untouched), 404 on unknown uid. Tests write synthetic Interaction records directly to the filesystem adapter at <DATA_DIR>/.idp/interaction/<uid>.json so they don't have to walk a full OIDC client flow. Also caught and fixed: reply.clearCookie() doesn't exist (JSS doesn't register @fastify/cookie). Replaced with reply.header('Set-Cookie', [...expired cookie strings]) which Fastify emits as multiple Set-Cookie headers. 51/51 idp.test.js tests pass. * idp: Copilot review pass 2 — 303 See Other + getSetCookie() in test Two more legit fixes: 1. Redirect changed from 302 to 303 (See Other). 302 leaves the UA free to repeat the POST to the Location target, which on this handler would re-trigger /switch in a loop. 303 unambiguously forces GET on the Location, which is what we want. 2. Test uses res.headers.getSetCookie() instead of headers.get(). Per the Fetch spec, Set-Cookie is a "forbidden header name" on the standard .get() — return value is implementation-defined and may collapse multiple Set-Cookie response headers into a single string or return null. .getSetCookie() returns an array, which is what we want for asserting on multiple cookie clears. Test asserts now stricter: - status === 303 - >= 3 Set-Cookie headers (one per cleared cookie name) - one of them clears _session - every Set-Cookie carries an expiration (Max-Age=0 or 1970 Expires) 51/51 idp.test.js tests pass. * idp: Copilot review pass 3 — also clear _session.legacy.sig The IdP config uses signed cookies (provider.js cookies.long.signed = true), so every session cookie name has a paired `.sig`. We were already clearing _session and _session.sig and _session.legacy, but missed _session.legacy.sig. Without it, the browser would carry a lingering signature cookie until the normal maxAge. Test now asserts >= 4 Set-Cookie headers and verifies every one of the four expected names is among them. 51/51 idp.test.js tests pass. * idp: Copilot review pass 4 — match redirect arg order, fail loud on missing getSetCookie Two fixes: 1. reply.redirect now uses (statusCode, url) — matches the rest of the codebase (src/server.js:637, src/tunnel/index.js:222) and avoids relying on the alternate Fastify overload (which is removed in Fastify 5). 2. Test now hard-fails when Headers.getSetCookie() is missing instead of silently asserting on []. The previous `getSetCookie?.() || []` pattern was a phantom fallback: if the method were missing, the array would be empty and the next assertion (length >= 4) would fail with "got 0" — but the failure mode would point at "no cookies were sent" rather than "your Node is too old to inspect Set-Cookies via fetch." Explicit assertion clarifies which is which. 51/51 idp.test.js tests pass. * idp: Copilot review pass 5 — Node 18 compat for cookie-clearing test The previous test asserted Headers.getSetCookie() exists, but package.json declares engines.node >= 18 and getSetCookie is only on Node 19.7+. Failing loud was an improvement but still meant the test suite was broken on a declared-supported Node. Switched the cookie-inspection test to use node:http directly via a small rawPost() helper. http.request gives res.headers['set-cookie'] as a real array on every Node version that has node:http (i.e. all of them), so engines.node >= 18 stays honest with no version-gated test branches. Other tests in the block still use fetch — they don't need raw header access and the existing pattern is preserved. 51/51 idp.test.js tests pass.
1 parent 95e076b commit b03192b

4 files changed

Lines changed: 217 additions & 1 deletion

File tree

‎src/idp/index.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
handleLogin,
1212
handleConsent,
1313
handleAbort,
14+
handleSwitchAccount,
1415
handleRegisterGet,
1516
handleRegisterPost,
1617
handlePasskeyComplete,
@@ -324,6 +325,13 @@ export async function idpPlugin(fastify, options) {
324325
return handleAbort(request, reply, provider);
325326
});
326327

328+
// POST "Sign in as a different user" (#384) — destroys the OIDC
329+
// session and bounces back to the login prompt while preserving the
330+
// in-flight authz request.
331+
fastify.post('/idp/interaction/:uid/switch', async (request, reply) => {
332+
return handleSwitchAccount(request, reply, provider);
333+
});
334+
327335
// Registration routes (disabled in single-user mode)
328336
if (singleUser) {
329337
// Single-user mode: registration disabled

‎src/idp/interactions.js‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,89 @@ export async function handleConsent(request, reply, provider) {
297297
}
298298
}
299299

300+
/**
301+
* Handle POST /idp/interaction/:uid/switch
302+
*
303+
* "Sign in as a different user" from the consent page (#384). Destroys
304+
* the current OIDC session, mutates the in-flight interaction back to
305+
* the login prompt, and redirects the user to the same /idp/interaction
306+
* URL — which `handleInteractionGet` will render as the login page.
307+
*
308+
* Re-using the same interaction uid (rather than starting a fresh
309+
* /idp/auth flow) preserves the original authz request params so the
310+
* caller's redirect_uri / state / nonce all flow through unchanged.
311+
*/
312+
export async function handleSwitchAccount(request, reply, provider) {
313+
const { uid } = request.params;
314+
315+
try {
316+
const interaction = await provider.Interaction.find(uid);
317+
if (!interaction) {
318+
return reply.code(404).type('text/html').send(errorPage('Interaction not found', 'This interaction may have expired. Try signing in again from your app.'));
319+
}
320+
321+
// The UI entrypoint is the consent page only. Refusing on other
322+
// prompt states (login, passkey, etc.) prevents a crafted request
323+
// from corrupting an in-flight non-consent interaction.
324+
if (interaction.prompt?.name !== 'consent') {
325+
return reply.code(400).type('text/html').send(errorPage('Cannot switch account here', 'Account switching is only available from the consent page.'));
326+
}
327+
328+
// Destroy the bound session so the new login starts cold. The cookie
329+
// becomes a stale reference; oidc-provider's Session.get treats a
330+
// missing session blob as "new browser", which is the shape we want.
331+
if (interaction.session?.uid) {
332+
const sess = await provider.Session.findByUid(interaction.session.uid);
333+
if (sess) await sess.destroy();
334+
}
335+
336+
// Reset the interaction back to the login prompt, dropping the
337+
// session reference and any prior `result` snapshot. `prompt`,
338+
// `session`, and `result` are all in the oidc-provider Interaction
339+
// IN_PAYLOAD allowlist, so the mutations persist through the
340+
// adapter. Original `params` (client_id, redirect_uri, state, etc.)
341+
// are untouched, so resume picks them up after login. Clearing
342+
// `result` prevents a stale `result.login` from a previous identity
343+
// influencing the next resume.
344+
interaction.session = undefined;
345+
interaction.result = undefined;
346+
interaction.prompt = { name: 'login', reasons: ['no_session'], details: {} };
347+
interaction.lastError = undefined;
348+
const ttl = Math.max(1, interaction.exp - Math.floor(Date.now() / 1000));
349+
await interaction.save(ttl);
350+
351+
// Clear the user-agent's session cookie too. The IdP runs with
352+
// signed cookies (provider.js cookies.long.signed = true), so each
353+
// session cookie has a paired `.sig`. The `.legacy` variant is
354+
// created during identifier rotation and likewise has its own
355+
// `.sig`. Clearing all four keeps the browser fully tidy. JSS
356+
// doesn't register @fastify/cookie, so we emit Set-Cookie headers
357+
// directly with an expired Expires + Max-Age=0. Server-side state
358+
// is already gone via session.destroy() above — these expirations
359+
// are belt-and-suspenders.
360+
const expired = 'Path=/; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly';
361+
reply.header('Set-Cookie', [
362+
`_session=; ${expired}`,
363+
`_session.sig=; ${expired}`,
364+
`_session.legacy=; ${expired}`,
365+
`_session.legacy.sig=; ${expired}`,
366+
]);
367+
368+
// 303 See Other — explicitly forces the UA to issue GET on the
369+
// Location target. 302 leaves it ambiguous (and some legacy UAs
370+
// repeat the POST), which would re-trigger this handler in a loop.
371+
// Status-then-URL arg order matches the rest of the codebase
372+
// (src/server.js:637, src/tunnel/index.js:222).
373+
return reply.redirect(303, `/idp/interaction/${uid}`);
374+
} catch (err) {
375+
request.log.error(err, 'Switch-account error');
376+
// Don't surface raw err.message — adapter errors and stack-leaking
377+
// strings on an auth endpoint are a soft info-leak. Full error is
378+
// already in the server log via request.log.error above.
379+
return reply.code(500).type('text/html').send(errorPage('Error', 'Something went wrong. Please try signing in again.'));
380+
}
381+
}
382+
300383
/**
301384
* Handle POST /idp/interaction/:uid/abort
302385
* User cancelled the flow

‎src/idp/views.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,15 @@ export function consentPage(uid, client, params, account) {
503503
${clientUri ? `<div class="client-uri">${escapeHtml(clientUri)}</div>` : ''}
504504
</div>
505505
506-
${account ? `<p>Signed in as <strong>${escapeHtml(account.email)}</strong></p>` : ''}
506+
${account ? `
507+
<div style="display: flex; align-items: center; justify-content: center; gap: 8px; flex-wrap: wrap; margin: 12px 0;">
508+
<span>Signed in as <strong>${escapeHtml(account.email)}</strong></span>
509+
<span style="color: #94a3b8;">·</span>
510+
<form method="POST" action="/idp/interaction/${uid}/switch" style="display: inline; margin: 0;">
511+
<button type="submit" style="background: none; border: 0; padding: 0; color: #2563eb; font: inherit; cursor: pointer; text-decoration: underline;">Sign in as a different user</button>
512+
</form>
513+
</div>
514+
` : ''}
507515
508516
<div class="scopes">
509517
<label>This app is requesting access to:</label>

‎test/idp.test.js‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import { describe, it, before, after, beforeEach } from 'node:test';
66
import assert from 'node:assert';
7+
import http from 'node:http';
78
import { createServer } from '../src/server.js';
89
import fs from 'fs-extra';
910
import path from 'path';
@@ -182,6 +183,122 @@ describe('Identity Provider', () => {
182183
});
183184
});
184185

186+
// Regression coverage for #384 — "Sign in as a different user" on consent
187+
describe('Switch account on consent (#384)', () => {
188+
// The IDP's filesystem adapter stores Interaction records as JSON at
189+
// <DATA_ROOT>/.idp/interaction/<uid>.json (model name "Interaction"
190+
// → dir "interaction" via the adapter's modelToDir camelCase split).
191+
// Tests write a synthetic interaction directly so we don't have to
192+
// walk a full OIDC client flow to set up state.
193+
const interactionDir = `${DATA_DIR}/.idp/interaction`;
194+
195+
function writeInteraction(uid, payload) {
196+
const ttlSec = 3600;
197+
const data = {
198+
...payload,
199+
kind: 'Interaction',
200+
jti: uid,
201+
exp: Math.floor(Date.now() / 1000) + ttlSec,
202+
iat: Math.floor(Date.now() / 1000),
203+
_id: uid,
204+
_expiresAt: Date.now() + ttlSec * 1000,
205+
};
206+
return fs.outputJson(`${interactionDir}/${uid}.json`, data, { spaces: 2 });
207+
}
208+
209+
// Use node:http directly for the cookie-clearing assertion. fetch's
210+
// Headers.getSetCookie() is only available on Node 19.7+, but the
211+
// package declares engines.node >= 18. http.request gives us
212+
// res.headers['set-cookie'] as a real array on every supported
213+
// Node version, no version-gated branches needed.
214+
function rawPost(urlString) {
215+
return new Promise((resolve, reject) => {
216+
const u = new URL(urlString);
217+
const req = http.request({
218+
method: 'POST',
219+
hostname: u.hostname,
220+
port: u.port,
221+
path: u.pathname + u.search,
222+
}, (res) => {
223+
let body = '';
224+
res.on('data', (c) => body += c);
225+
res.on('end', () => resolve({
226+
statusCode: res.statusCode,
227+
headers: res.headers,
228+
body,
229+
}));
230+
});
231+
req.on('error', reject);
232+
req.end();
233+
});
234+
}
235+
236+
it('redirects back to /idp/interaction/:uid and resets the prompt to login', async () => {
237+
const uid = 'test-switch-' + Math.random().toString(36).slice(2);
238+
await writeInteraction(uid, {
239+
prompt: { name: 'consent', reasons: [], details: {} },
240+
session: { uid: 'fake-session-uid', accountId: 'acct-foo' },
241+
params: { client_id: 'test-client', redirect_uri: 'http://localhost', state: 'xyz' },
242+
});
243+
244+
const res = await rawPost(`${baseUrl}/idp/interaction/${uid}/switch`);
245+
246+
// 303 See Other — forces UA to GET the Location target so a
247+
// (broken) UA can't loop by re-POSTing to /switch.
248+
assert.strictEqual(res.statusCode, 303);
249+
assert.strictEqual(res.headers.location, `/idp/interaction/${uid}`);
250+
251+
// Verify the interaction was mutated as expected.
252+
const saved = await fs.readJson(`${interactionDir}/${uid}.json`);
253+
assert.strictEqual(saved.prompt.name, 'login');
254+
assert.ok(saved.session === undefined || saved.session === null,
255+
'session should be cleared');
256+
// Original params survive so resume can continue the authz request.
257+
assert.strictEqual(saved.params.client_id, 'test-client');
258+
assert.strictEqual(saved.params.state, 'xyz');
259+
260+
// Cookies should be cleared so the user's UA forgets the prior
261+
// session. Node's http module gives Set-Cookie as an array on
262+
// every supported version, so no Node 19.7+ gating needed.
263+
const setCookies = Array.isArray(res.headers['set-cookie']) ? res.headers['set-cookie'] : [];
264+
assert.ok(setCookies.length >= 4, `expected at least 4 Set-Cookie headers, got ${setCookies.length}`);
265+
// All four signed-cookie names should be cleared:
266+
// _session + _session.sig + _session.legacy + _session.legacy.sig.
267+
for (const name of ['_session=', '_session.sig=', '_session.legacy=', '_session.legacy.sig=']) {
268+
assert.ok(setCookies.some(c => c.startsWith(name)),
269+
`should clear ${name.slice(0, -1)}`);
270+
}
271+
assert.ok(setCookies.every(c => /Max-Age=0|Expires=Thu, 01 Jan 1970/.test(c)),
272+
'all Set-Cookies should be expirations');
273+
});
274+
275+
it('returns 400 when the interaction is not on the consent prompt', async () => {
276+
const uid = 'test-switch-bad-' + Math.random().toString(36).slice(2);
277+
await writeInteraction(uid, {
278+
prompt: { name: 'login', reasons: ['no_session'], details: {} },
279+
params: { client_id: 'test-client' },
280+
});
281+
282+
const res = await fetch(`${baseUrl}/idp/interaction/${uid}/switch`, {
283+
method: 'POST',
284+
redirect: 'manual',
285+
});
286+
287+
assert.strictEqual(res.status, 400);
288+
// Original interaction should be untouched.
289+
const saved = await fs.readJson(`${interactionDir}/${uid}.json`);
290+
assert.strictEqual(saved.prompt.name, 'login');
291+
});
292+
293+
it('returns 404 for an unknown interaction uid', async () => {
294+
const res = await fetch(`${baseUrl}/idp/interaction/does-not-exist-${Date.now()}/switch`, {
295+
method: 'POST',
296+
redirect: 'manual',
297+
});
298+
assert.strictEqual(res.status, 404);
299+
});
300+
});
301+
185302
// Regression coverage for #286 — friendly /idp landing + /idp/auth guard.
186303
describe('Landing page', () => {
187304
it('GET /idp returns the landing HTML', async () => {

0 commit comments

Comments
 (0)