Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/idp/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
handleLogin,
handleConsent,
handleAbort,
handleSwitchAccount,
handleRegisterGet,
handleRegisterPost,
handlePasskeyComplete,
Expand Down Expand Up @@ -324,6 +325,13 @@ export async function idpPlugin(fastify, options) {
return handleAbort(request, reply, provider);
});

// POST "Sign in as a different user" (#384) — destroys the OIDC
// session and bounces back to the login prompt while preserving the
// in-flight authz request.
fastify.post('/idp/interaction/:uid/switch', async (request, reply) => {
return handleSwitchAccount(request, reply, provider);
});
Comment on lines +328 to +333

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0b8bf9f: added a `Switch account on consent (#384)` describe block in test/idp.test.js with three tests — 302 + interaction mutated to login + cookies cleared on success, 400 on non-consent prompt with interaction untouched, 404 on unknown uid. Tests write synthetic Interaction records to the filesystem adapter so they don't need a full OIDC client walkthrough. 51/51 idp.test.js tests pass.\n\nBonus: the test suite caught a real bug — `reply.clearCookie` doesn't exist on JSS's reply (no @fastify/cookie registered). Replaced with `reply.header('Set-Cookie', [...])` which Fastify emits as multiple Set-Cookie response headers.


// Registration routes (disabled in single-user mode)
if (singleUser) {
// Single-user mode: registration disabled
Expand Down
83 changes: 83 additions & 0 deletions src/idp/interactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,89 @@ export async function handleConsent(request, reply, provider) {
}
}

/**
* Handle POST /idp/interaction/:uid/switch
*
* "Sign in as a different user" from the consent page (#384). Destroys
* the current OIDC session, mutates the in-flight interaction back to
* the login prompt, and redirects the user to the same /idp/interaction
* URL — which `handleInteractionGet` will render as the login page.
*
* Re-using the same interaction uid (rather than starting a fresh
* /idp/auth flow) preserves the original authz request params so the
* caller's redirect_uri / state / nonce all flow through unchanged.
*/
export async function handleSwitchAccount(request, reply, provider) {
const { uid } = request.params;

try {
const interaction = await provider.Interaction.find(uid);
if (!interaction) {
return reply.code(404).type('text/html').send(errorPage('Interaction not found', 'This interaction may have expired. Try signing in again from your app.'));
}

// The UI entrypoint is the consent page only. Refusing on other
// prompt states (login, passkey, etc.) prevents a crafted request
// from corrupting an in-flight non-consent interaction.
if (interaction.prompt?.name !== 'consent') {
return reply.code(400).type('text/html').send(errorPage('Cannot switch account here', 'Account switching is only available from the consent page.'));
}

// Destroy the bound session so the new login starts cold. The cookie
// becomes a stale reference; oidc-provider's Session.get treats a
// missing session blob as "new browser", which is the shape we want.
if (interaction.session?.uid) {
const sess = await provider.Session.findByUid(interaction.session.uid);
if (sess) await sess.destroy();
}

// Reset the interaction back to the login prompt, dropping the
// session reference and any prior `result` snapshot. `prompt`,
// `session`, and `result` are all in the oidc-provider Interaction
// IN_PAYLOAD allowlist, so the mutations persist through the
// adapter. Original `params` (client_id, redirect_uri, state, etc.)
// are untouched, so resume picks them up after login. Clearing
// `result` prevents a stale `result.login` from a previous identity
// influencing the next resume.
interaction.session = undefined;
interaction.result = undefined;
interaction.prompt = { name: 'login', reasons: ['no_session'], details: {} };
interaction.lastError = undefined;
const ttl = Math.max(1, interaction.exp - Math.floor(Date.now() / 1000));
await interaction.save(ttl);
Comment on lines +316 to +349

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0b8bf9f: handleSwitchAccount now returns 400 unless `interaction.prompt?.name === 'consent'`. Also clears `interaction.result` (in IN_PAYLOAD too) so a stale `result.login` from a prior identity can't influence the next resume.


// Clear the user-agent's session cookie too. The IdP runs with
// signed cookies (provider.js cookies.long.signed = true), so each
// session cookie has a paired `.sig`. The `.legacy` variant is
// created during identifier rotation and likewise has its own
// `.sig`. Clearing all four keeps the browser fully tidy. JSS
// doesn't register @fastify/cookie, so we emit Set-Cookie headers
// directly with an expired Expires + Max-Age=0. Server-side state
// is already gone via session.destroy() above — these expirations
// are belt-and-suspenders.
const expired = 'Path=/; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly';
reply.header('Set-Cookie', [
`_session=; ${expired}`,
`_session.sig=; ${expired}`,
`_session.legacy=; ${expired}`,
`_session.legacy.sig=; ${expired}`,
]);

// 303 See Other — explicitly forces the UA to issue GET on the
// Location target. 302 leaves it ambiguous (and some legacy UAs
// repeat the POST), which would re-trigger this handler in a loop.
// Status-then-URL arg order matches the rest of the codebase
// (src/server.js:637, src/tunnel/index.js:222).
return reply.redirect(303, `/idp/interaction/${uid}`);
} catch (err) {
request.log.error(err, 'Switch-account error');
// Don't surface raw err.message — adapter errors and stack-leaking
// strings on an auth endpoint are a soft info-leak. Full error is
// already in the server log via request.log.error above.
return reply.code(500).type('text/html').send(errorPage('Error', 'Something went wrong. Please try signing in again.'));
}
Comment on lines +374 to +380

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0b8bf9f: 500 path now returns a generic 'Something went wrong' message. Full error is already going to `request.log.error`; nothing leaks to the browser.

}

/**
* Handle POST /idp/interaction/:uid/abort
* User cancelled the flow
Expand Down
10 changes: 9 additions & 1 deletion src/idp/views.js
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,15 @@ export function consentPage(uid, client, params, account) {
${clientUri ? `<div class="client-uri">${escapeHtml(clientUri)}</div>` : ''}
</div>

${account ? `<p>Signed in as <strong>${escapeHtml(account.email)}</strong></p>` : ''}
${account ? `
<div style="display: flex; align-items: center; justify-content: center; gap: 8px; flex-wrap: wrap; margin: 12px 0;">
<span>Signed in as <strong>${escapeHtml(account.email)}</strong></span>
<span style="color: #94a3b8;">·</span>
<form method="POST" action="/idp/interaction/${uid}/switch" style="display: inline; margin: 0;">
<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>
</form>
</div>
` : ''}

<div class="scopes">
<label>This app is requesting access to:</label>
Expand Down
117 changes: 117 additions & 0 deletions test/idp.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import { describe, it, before, after, beforeEach } from 'node:test';
import assert from 'node:assert';
import http from 'node:http';
import { createServer } from '../src/server.js';
import fs from 'fs-extra';
import path from 'path';
Expand Down Expand Up @@ -182,6 +183,122 @@ describe('Identity Provider', () => {
});
});

// Regression coverage for #384 — "Sign in as a different user" on consent
describe('Switch account on consent (#384)', () => {
// The IDP's filesystem adapter stores Interaction records as JSON at
// <DATA_ROOT>/.idp/interaction/<uid>.json (model name "Interaction"
// → dir "interaction" via the adapter's modelToDir camelCase split).
// Tests write a synthetic interaction directly so we don't have to
// walk a full OIDC client flow to set up state.
const interactionDir = `${DATA_DIR}/.idp/interaction`;

function writeInteraction(uid, payload) {
const ttlSec = 3600;
const data = {
...payload,
kind: 'Interaction',
jti: uid,
exp: Math.floor(Date.now() / 1000) + ttlSec,
iat: Math.floor(Date.now() / 1000),
_id: uid,
_expiresAt: Date.now() + ttlSec * 1000,
};
return fs.outputJson(`${interactionDir}/${uid}.json`, data, { spaces: 2 });
}

// Use node:http directly for the cookie-clearing assertion. fetch's
// Headers.getSetCookie() is only available on Node 19.7+, but the
// package declares engines.node >= 18. http.request gives us
// res.headers['set-cookie'] as a real array on every supported
// Node version, no version-gated branches needed.
function rawPost(urlString) {
return new Promise((resolve, reject) => {
const u = new URL(urlString);
const req = http.request({
method: 'POST',
hostname: u.hostname,
port: u.port,
path: u.pathname + u.search,
}, (res) => {
let body = '';
res.on('data', (c) => body += c);
res.on('end', () => resolve({
statusCode: res.statusCode,
headers: res.headers,
body,
}));
});
req.on('error', reject);
req.end();
});
}

it('redirects back to /idp/interaction/:uid and resets the prompt to login', async () => {
const uid = 'test-switch-' + Math.random().toString(36).slice(2);
await writeInteraction(uid, {
prompt: { name: 'consent', reasons: [], details: {} },
session: { uid: 'fake-session-uid', accountId: 'acct-foo' },
params: { client_id: 'test-client', redirect_uri: 'http://localhost', state: 'xyz' },
});

const res = await rawPost(`${baseUrl}/idp/interaction/${uid}/switch`);

// 303 See Other — forces UA to GET the Location target so a
// (broken) UA can't loop by re-POSTing to /switch.
assert.strictEqual(res.statusCode, 303);
assert.strictEqual(res.headers.location, `/idp/interaction/${uid}`);

// Verify the interaction was mutated as expected.
const saved = await fs.readJson(`${interactionDir}/${uid}.json`);
assert.strictEqual(saved.prompt.name, 'login');
assert.ok(saved.session === undefined || saved.session === null,
'session should be cleared');
// Original params survive so resume can continue the authz request.
assert.strictEqual(saved.params.client_id, 'test-client');
assert.strictEqual(saved.params.state, 'xyz');

// Cookies should be cleared so the user's UA forgets the prior
// session. Node's http module gives Set-Cookie as an array on
// every supported version, so no Node 19.7+ gating needed.
const setCookies = Array.isArray(res.headers['set-cookie']) ? res.headers['set-cookie'] : [];
assert.ok(setCookies.length >= 4, `expected at least 4 Set-Cookie headers, got ${setCookies.length}`);
// All four signed-cookie names should be cleared:
// _session + _session.sig + _session.legacy + _session.legacy.sig.
for (const name of ['_session=', '_session.sig=', '_session.legacy=', '_session.legacy.sig=']) {
assert.ok(setCookies.some(c => c.startsWith(name)),
`should clear ${name.slice(0, -1)}`);
}
assert.ok(setCookies.every(c => /Max-Age=0|Expires=Thu, 01 Jan 1970/.test(c)),
'all Set-Cookies should be expirations');
});

it('returns 400 when the interaction is not on the consent prompt', async () => {
const uid = 'test-switch-bad-' + Math.random().toString(36).slice(2);
await writeInteraction(uid, {
prompt: { name: 'login', reasons: ['no_session'], details: {} },
params: { client_id: 'test-client' },
});

const res = await fetch(`${baseUrl}/idp/interaction/${uid}/switch`, {
method: 'POST',
redirect: 'manual',
});

assert.strictEqual(res.status, 400);
// Original interaction should be untouched.
const saved = await fs.readJson(`${interactionDir}/${uid}.json`);
assert.strictEqual(saved.prompt.name, 'login');
});

it('returns 404 for an unknown interaction uid', async () => {
const res = await fetch(`${baseUrl}/idp/interaction/does-not-exist-${Date.now()}/switch`, {
method: 'POST',
redirect: 'manual',
});
assert.strictEqual(res.status, 404);
});
});

// Regression coverage for #286 — friendly /idp landing + /idp/auth guard.
describe('Landing page', () => {
it('GET /idp returns the landing HTML', async () => {
Expand Down