diff --git a/src/client/login.ts b/src/client/login.ts index 3595247..ae1fdb3 100644 --- a/src/client/login.ts +++ b/src/client/login.ts @@ -119,7 +119,11 @@ async function initConditionalUI() { const verifyRes = await fetch("/auth/login/verify", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ response: authResponse, conditional: true }), + body: JSON.stringify({ + response: authResponse, + conditional: true, + challenge: options.challenge, + }), }); if (!verifyRes.ok) { @@ -199,7 +203,11 @@ loginForm.addEventListener("submit", async (e) => { const verifyRes = await fetch("/auth/login/verify", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ username, response: authResponse }), + body: JSON.stringify({ + username, + response: authResponse, + challenge: options.challenge, + }), }); if (!verifyRes.ok) { diff --git a/src/routes/auth.ts b/src/routes/auth.ts index 6a11391..b234453 100644 --- a/src/routes/auth.ts +++ b/src/routes/auth.ts @@ -16,6 +16,15 @@ import { checkLdapGroupMembership, checkLdapUser } from "../ldap-cleanup"; const RP_NAME = "Indiko"; +// Thrown inside the registration transaction when the invite is exhausted so +// the whole registration rolls back. +class InviteExhaustedError extends Error { + constructor() { + super("Invite code fully used"); + this.name = "InviteExhaustedError"; + } +} + export function canRegister(_req: Request): Response { const userCount = db.query("SELECT COUNT(*) as count FROM users").get() as { count: number; @@ -208,6 +217,12 @@ export async function registerVerify(req: Request): Promise { return Response.json({ error: "Challenge expired" }, { status: 400 }); } + // Burn the challenge immediately — it must not be reusable regardless + // of whether verification succeeds or fails. + db.query("DELETE FROM challenges WHERE challenge = ?").run( + challenge.challenge, + ); + // Check if this is bootstrap (first user) const userCount = db.query("SELECT COUNT(*) as count FROM users").get() as { count: number; @@ -285,93 +300,101 @@ export async function registerVerify(req: Request): Promise { const { credential } = verification.registrationInfo; - // Check if this user is being provisioned via LDAP - let isLdapProvisioned = false; - if (inviteId) { - const invite = db - .query("SELECT ldap_username FROM invites WHERE id = ?") - .get(inviteId) as { ldap_username: string | null } | undefined; - isLdapProvisioned = - invite?.ldap_username !== null && invite?.ldap_username !== undefined; - } - - let userId: number; - let userIsAdmin: boolean; - - if (isPasskeyReset && existingUser) { - // Passkey reset: use existing user, just add credential - userId = existingUser.id; - userIsAdmin = existingUser.is_admin === 1; - } else { - // Create new user (bootstrap is always admin, invited users are regular users) - const insertUser = db.query( - "INSERT INTO users (username, name, is_admin, tier, role, provisioned_via_ldap) VALUES (?, ?, ?, ?, ?, ?) RETURNING id", - ); - const user = insertUser.get( - username, - username, - isBootstrap ? 1 : 0, - isBootstrap ? "admin" : "user", - isBootstrap ? "admin" : "user", - isLdapProvisioned ? 1 : 0, - ) as { id: number }; - userId = user.id; - userIsAdmin = isBootstrap; - } + // Consume the invite and create user/credential/session in one + // transaction. If the invite is exhausted the whole registration rolls + // back — no orphaned accounts or sessions. + const usedAt = Math.floor(Date.now() / 1000); + let userId = 0; + let userIsAdmin = false; - // Store credential - // credential.id is a Uint8Array, convert to Buffer for storage - db.query( - "INSERT INTO credentials (user_id, credential_id, public_key, counter, name) VALUES (?, ?, ?, ?, ?)", - ).run( - userId, - Buffer.from(credential.id), - Buffer.from(credential.publicKey), - credential.counter, - isPasskeyReset ? "Reset Passkey" : "Primary Passkey", - ); + try { + db.transaction(() => { + // Atomically consume the invite first — before any user rows exist. + if (inviteId) { + const result = db + .query( + "UPDATE invites SET current_uses = current_uses + 1 WHERE id = ? AND current_uses < max_uses", + ) + .run(inviteId); + + if (result.changes === 0) { + throw new InviteExhaustedError(); + } + } - // Mark invite as used if applicable - if (inviteId) { - const usedAt = Math.floor(Date.now() / 1000); + // Check if this user is being provisioned via LDAP + let isLdapProvisioned = false; + if (inviteId) { + const invite = db + .query("SELECT ldap_username FROM invites WHERE id = ?") + .get(inviteId) as { ldap_username: string | null } | undefined; + isLdapProvisioned = + invite?.ldap_username !== null && + invite?.ldap_username !== undefined; + } - // Atomically increment invite usage counter while checking max_uses limit - const result = db - .query( - "UPDATE invites SET current_uses = current_uses + 1 WHERE id = ? AND current_uses < max_uses", - ) - .run(inviteId); + if (isPasskeyReset && existingUser) { + // Passkey reset: use existing user, just add credential + userId = existingUser.id; + userIsAdmin = existingUser.is_admin === 1; + } else { + // Create new user (bootstrap is always admin, invited users are regular users) + const insertUser = db.query( + "INSERT INTO users (username, name, is_admin, tier, role, provisioned_via_ldap) VALUES (?, ?, ?, ?, ?, ?) RETURNING id", + ); + const user = insertUser.get( + username, + username, + isBootstrap ? 1 : 0, + isBootstrap ? "admin" : "user", + isBootstrap ? "admin" : "user", + isLdapProvisioned ? 1 : 0, + ) as { id: number }; + userId = user.id; + userIsAdmin = isBootstrap; + } + + // Store credential + // credential.id is a Uint8Array, convert to Buffer for storage + db.query( + "INSERT INTO credentials (user_id, credential_id, public_key, counter, name) VALUES (?, ?, ?, ?, ?)", + ).run( + userId, + Buffer.from(credential.id), + Buffer.from(credential.publicKey), + credential.counter, + isPasskeyReset ? "Reset Passkey" : "Primary Passkey", + ); - // Check if update was successful (0 rows affected means invite was already fully used) - if (result.changes === 0) { + if (inviteId) { + // Record this invite use + db.query( + "INSERT INTO invite_uses (invite_id, user_id, used_at) VALUES (?, ?, ?)", + ).run(inviteId, userId, usedAt); + + // Assign app roles to the new user (skip for passkey reset - they already have roles) + if (inviteRoles.length > 0 && !isPasskeyReset) { + const insertPermission = db.query( + "INSERT INTO permissions (user_id, app_id, role) VALUES (?, ?, ?)", + ); + for (const { app_id, role } of inviteRoles) { + insertPermission.run(userId, app_id, role); + } + } + } + })(); + } catch (err) { + if (err instanceof InviteExhaustedError) { return Response.json( { error: "Invite code fully used" }, { status: 403 }, ); } - - // Record this invite use - db.query( - "INSERT INTO invite_uses (invite_id, user_id, used_at) VALUES (?, ?, ?)", - ).run(inviteId, userId, usedAt); - - // Assign app roles to the new user (skip for passkey reset - they already have roles) - if (inviteRoles.length > 0 && !isPasskeyReset) { - const insertPermission = db.query( - "INSERT INTO permissions (user_id, app_id, role) VALUES (?, ?, ?)", - ); - for (const { app_id, role } of inviteRoles) { - insertPermission.run(userId, app_id, role); - } - } + throw err; } - // Delete challenge - db.query("DELETE FROM challenges WHERE challenge = ?").run( - challenge.challenge, - ); - - // Create session + // Create session (outside the transaction — if this fails the account + // still exists and the user can log in again) const token = crypto.randomUUID(); const expiresAt = Math.floor(Date.now() / 1000) + 86400; // 24 hours db.query( @@ -516,10 +539,16 @@ export async function loginOptions(req: Request): Promise { export async function loginVerify(req: Request): Promise { try { const body = await req.json(); - const { username, response, conditional } = body as { + const { + username, + response, + conditional, + challenge: clientChallenge, + } = body as { username?: string; response: AuthenticationResponseJSON; conditional?: boolean; + challenge?: string; }; if (!response) { @@ -530,6 +559,10 @@ export async function loginVerify(req: Request): Promise { return Response.json({ error: "Username required" }, { status: 400 }); } + if (!clientChallenge) { + return Response.json({ error: "Challenge required" }, { status: 400 }); + } + // Look up credential by ID to discover the username const credentialIdString = response.id; @@ -570,14 +603,15 @@ export async function loginVerify(req: Request): Promise { const user = { id: credentialWithUser.user_id }; const resolvedUsername = credentialWithUser.username; - // Verify challenge exists and is valid - // Conditional UI stores challenge with empty username; normal flow uses the actual username + // Verify challenge exists and is valid — look up by the challenge value + // the client used (returned in the verify body), not "latest row wins". + // This prevents cross-session challenge confusion and login DoS. const challengeUsername = conditional ? "" : resolvedUsername; const challenge = db .query( - "SELECT challenge, expires_at FROM challenges WHERE username = ? AND type = 'authentication' ORDER BY created_at DESC LIMIT 1", + "SELECT challenge, expires_at FROM challenges WHERE challenge = ? AND username = ? AND type = 'authentication'", ) - .get(challengeUsername) as + .get(clientChallenge, challengeUsername) as | { challenge: string; expires_at: number } | undefined; @@ -590,6 +624,12 @@ export async function loginVerify(req: Request): Promise { return Response.json({ error: "Challenge expired" }, { status: 400 }); } + // Burn the challenge immediately — it must not be reusable regardless + // of whether verification succeeds or fails. + db.query("DELETE FROM challenges WHERE challenge = ?").run( + challenge.challenge, + ); + // Verify authentication response let verification: VerifiedAuthenticationResponse; try { @@ -613,20 +653,25 @@ export async function loginVerify(req: Request): Promise { return Response.json({ error: "Verification failed" }, { status: 400 }); } + // Reject cloned authenticators: counter must advance. Authenticators + // that don't support counters always report 0; skip the check then. + const newCounter = verification.authenticationInfo.newCounter; + if (credential.counter > 0 && newCounter <= credential.counter) { + console.warn( + `[auth] counter regression for credential ${credential.credential_id.toString()} — possible cloned passkey (stored=${credential.counter}, got=${newCounter})`, + ); + return Response.json({ error: "Verification failed" }, { status: 400 }); + } + // Update credential counter db.query( "UPDATE credentials SET counter = ? WHERE user_id = ? AND credential_id = ?", ).run( - verification.authenticationInfo.newCounter, + newCounter, user.id, credential.credential_id, ); - // Delete challenge - db.query("DELETE FROM challenges WHERE challenge = ?").run( - challenge.challenge, - ); - // Create session const token = crypto.randomUUID(); const expiresAt = Math.floor(Date.now() / 1000) + 86400; // 24 hours diff --git a/src/routes/oauth/authorize.ts b/src/routes/oauth/authorize.ts index bc667af..1ff7040 100644 --- a/src/routes/oauth/authorize.ts +++ b/src/routes/oauth/authorize.ts @@ -1,7 +1,7 @@ import crypto from "node:crypto"; import { db } from "../../db"; import { ensureApp } from "../../lib/oauth/client-metadata"; -import { consentPage, errorPage } from "../../lib/oauth/pages"; +import { consentPage, errorPage, escapeHtml } from "../../lib/oauth/pages"; import { canonicalizeURL } from "../../lib/oauth/urls"; import { getUserFromCookie } from "../../lib/session"; import { token } from "./token"; @@ -113,7 +113,7 @@ export async function authorizeGet(req: Request): Promise { message: "The OAuth authorization request failed because the provided redirect_uri is not registered for this client application.", details, - hint: `The redirect_uri must exactly match a registered URI for ${appName}. If you are the application developer, please ensure your redirect_uri matches the one registered with this authorization server.`, + hint: `The redirect_uri must exactly match a registered URI for ${escapeHtml(appName)}. If you are the application developer, please ensure your redirect_uri matches the one registered with this authorization server.`, }); } @@ -205,9 +205,11 @@ export async function authorizeGet(req: Request): Promise { ).run(Math.floor(Date.now() / 1000), user.userId, clientId); const origin = process.env.ORIGIN || "http://localhost:3000"; - return Response.redirect( - `${redirectUri}?code=${code}&state=${state}&iss=${encodeURIComponent(origin)}`, - ); + const autoApproveUrl = new URL(redirectUri); + autoApproveUrl.searchParams.set("code", code); + autoApproveUrl.searchParams.set("state", state); + autoApproveUrl.searchParams.set("iss", origin); + return Response.redirect(autoApproveUrl.toString(), 302); } } @@ -338,11 +340,31 @@ export async function authorizePost(req: Request): Promise { }); } + // Re-validate redirect_uri against the app's registered set — the consent + // form POSTs attacker-controllable hidden fields, so we must not trust the + // submitted redirect_uri without checking it against the registered list. + const app = db + .query("SELECT redirect_uris FROM apps WHERE client_id = ?") + .get(clientId) as { redirect_uris: string } | undefined; + + if (!app) { + return new Response("Invalid client_id", { status: 400 }); + } + + const allowedRedirects = JSON.parse(app.redirect_uris) as string[]; + if (!allowedRedirects.includes(redirectUri)) { + return new Response("redirect_uri not registered for this client", { + status: 400, + }); + } + if (action === "deny") { const origin = process.env.ORIGIN || "http://localhost:3000"; - return Response.redirect( - `${redirectUri}?error=access_denied&state=${state}&iss=${encodeURIComponent(origin)}`, - ); + const denyUrl = new URL(redirectUri); + denyUrl.searchParams.set("error", "access_denied"); + denyUrl.searchParams.set("state", state); + denyUrl.searchParams.set("iss", origin); + return Response.redirect(denyUrl.toString(), 302); } // Get the scopes the user actually approved (from checkboxes) @@ -407,7 +429,9 @@ export async function authorizePost(req: Request): Promise { ); const origin = process.env.ORIGIN || "http://localhost:3000"; - return Response.redirect( - `${redirectUri}?code=${code}&state=${state}&iss=${encodeURIComponent(origin)}`, - ); + const successUrl = new URL(redirectUri); + successUrl.searchParams.set("code", code); + successUrl.searchParams.set("state", state); + successUrl.searchParams.set("iss", origin); + return Response.redirect(successUrl.toString(), 302); }