diff --git a/frontend/src/admin/api.js b/frontend/src/admin/api.js index 72e3f71..fc30f34 100644 --- a/frontend/src/admin/api.js +++ b/frontend/src/admin/api.js @@ -1,7 +1,17 @@ -// Admin API client. Uses the rn_admin session cookie exclusively (same-origin, sent +// Admin API client. Uses the rn_instructor session cookie exclusively (same-origin, sent // automatically) — completely separate credential/cookie from student sessions (see // platform/api/src/sessions.js). No admin data is ever cached client-side beyond the current // React state; nothing here reads or writes localStorage. +// +// rn_csrf_i is a deliberately non-HttpOnly cookie (double-submit CSRF pattern, mirrored on the +// student side in frontend/src/platform/api.js) — read here and echoed back as a header on +// every state-changing request. + +const CSRF_COOKIE = 'rn_csrf_i' + +function getCookie(name) { + return document.cookie.split('; ').find((c) => c.startsWith(`${name}=`))?.split('=')[1] +} async function json(res) { if (!res.ok) { @@ -11,23 +21,30 @@ async function json(res) { return res.json() } +function withCsrf(init) { + return { ...init, headers: { ...init.headers, 'X-CSRF-Token': getCookie(CSRF_COOKIE) || '' } } +} function post(path, body) { - return fetch(path, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: body ? JSON.stringify(body) : undefined }).then(json) + return fetch(path, withCsrf({ method: 'POST', headers: { 'Content-Type': 'application/json' }, body: body ? JSON.stringify(body) : undefined })).then(json) } function patch(path, body) { - return fetch(path, { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) }).then(json) + return fetch(path, withCsrf({ method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) })).then(json) } function del(path) { - return fetch(path, { method: 'DELETE' }).then(json) + return fetch(path, withCsrf({ method: 'DELETE' })).then(json) } function get(path) { return fetch(path).then(json) } export const adminApi = { - login: (password) => post('/api/admin/login', { password }), - logout: () => post('/api/admin/logout'), - whoami: () => get('/api/admin/whoami'), + // No CSRF token yet at login (nothing authenticated to forge into). + login: (email, password) => fetch('/api/auth/instructor/login', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ email, password }), + }).then(json), + logout: () => post('/api/auth/logout'), + session: () => get('/api/auth/session'), + changePassword: (old_password, new_password) => post('/api/auth/change-password', { old_password, new_password }), dashboard: () => get('/api/admin/dashboard'), @@ -47,10 +64,20 @@ export const adminApi = { cohortStudents: (id) => get(`/api/admin/cohorts/${id}/students`), cohortProgress: (id) => get(`/api/admin/cohorts/${id}/progress`), assignCohortCurriculum: (id, curriculum_path) => post(`/api/admin/cohorts/${id}/curriculum`, { curriculum_path }), + assignCohortInstructor: (id, instructor_id, role_in_cohort) => post(`/api/admin/cohorts/${id}/instructors`, { instructor_id, role_in_cohort }), prepareLabs: (id) => post(`/api/admin/cohorts/${id}/prepare-labs`), destroyLabs: (id) => post(`/api/admin/cohorts/${id}/destroy-labs`), resetEnvironment: (id) => post(`/api/admin/environments/${id}/reset`), destroyEnvironment: (id) => del(`/api/admin/environments/${id}`), environmentEvents: (id) => get(`/api/admin/environments/${id}/events`), + + instructors: () => get('/api/admin/instructors'), + createInstructor: (fields) => post('/api/admin/instructors', fields), + instructor: (id) => get(`/api/admin/instructors/${id}`), + updateInstructor: (id, fields) => patch(`/api/admin/instructors/${id}`, fields), + revokeInstructorSessions: (id) => post(`/api/admin/instructors/${id}/revoke-sessions`), + resetInstructorPassword: (id, new_password) => post(`/api/admin/instructors/${id}/reset-password`, { new_password }), + + auditEvents: () => get('/api/admin/audit-events'), } diff --git a/frontend/src/platform/api.js b/frontend/src/platform/api.js index 7748ed3..d48be89 100644 --- a/frontend/src/platform/api.js +++ b/frontend/src/platform/api.js @@ -1,14 +1,21 @@ -// Platform Shell API client. Phase 2B: identity is a server-controlled session cookie -// (rn_session, HttpOnly — this code can't read it, and doesn't need to; the browser sends it -// automatically on every same-origin request). This replaces Phase 2A's localStorage -// student_id + X-Student-Id header, which was explicitly pilot-only and client-forgeable. -// See docs/PHASE2B_ADMIN_AND_PLATFORM_DATA.md §3. +// Platform Shell API client. Identity is a server-controlled session cookie (rn_session, +// HttpOnly — this code can't read it, and doesn't need to; the browser sends it automatically +// on every same-origin request). Roster name-picking is classroom/pilot identification, not +// production authentication (see docs/PHASE3A_IDENTITY_AND_INSTRUCTOR_ACCESS.md §5) — what's +// hardened here is the session that results from it: persistent (survives an API restart), +// unforgeable, and checked on every request. // // display_name is still cached in sessionStorage purely for UI convenience (showing "Hi, // Alice" without an extra round trip) — it carries no authority; every actual API call is // authorized by the cookie alone, not by anything read from here. +// +// rn_csrf_s is a deliberately non-HttpOnly cookie (double-submit CSRF pattern) — this code +// reads it and echoes it back as a header on every state-changing request, proving the request +// came from our own origin's JS (a cross-origin attacker can trigger the cookie but can't read +// its value or attach a matching custom header without CORS, which we don't grant elsewhere). const DISPLAY_CACHE_KEY = 'rootnode_student_display' +const CSRF_COOKIE = 'rn_csrf_s' export function getCachedDisplay() { try { return JSON.parse(sessionStorage.getItem(DISPLAY_CACHE_KEY) || 'null') } catch { return null } @@ -20,6 +27,10 @@ function clearCachedDisplay() { sessionStorage.removeItem(DISPLAY_CACHE_KEY) } +function getCookie(name) { + return document.cookie.split('; ').find((c) => c.startsWith(`${name}=`))?.split('=')[1] +} + async function json(res) { if (!res.ok) { const body = await res.json().catch(() => ({})) @@ -28,10 +39,20 @@ async function json(res) { return res.json() } +function mutate(path, body) { + return fetch(path, { + method: 'POST', + headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': getCookie(CSRF_COOKIE) || '' }, + body: body ? JSON.stringify(body) : undefined, + }).then(json) +} + export const api = { cohorts: () => fetch('/api/roster/cohorts').then(json), roster: (cohort) => fetch(`/api/roster?cohort=${encodeURIComponent(cohort)}`).then(json), login: async (student_id) => { + // No CSRF token yet at login (nothing authenticated to forge into) — same reasoning as the + // instructor login route. const student = await fetch('/api/login', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ student_id }), }).then(json) @@ -39,7 +60,7 @@ export const api = { return student }, logout: async () => { - await fetch('/api/logout', { method: 'POST' }).catch(() => {}) + await mutate('/api/logout').catch(() => {}) clearCachedDisplay() }, whoami: () => fetch('/api/session/whoami').then(json), @@ -47,6 +68,6 @@ export const api = { curriculumPath: () => fetch('/api/curriculum/path').then(json), environmentStatus: () => fetch('/api/environment/status').then(json), - launchEnvironment: () => fetch('/api/environment/launch', { method: 'POST' }).then(json), - resetEnvironment: () => fetch('/api/environment/reset', { method: 'POST' }).then(json), + launchEnvironment: () => mutate('/api/environment/launch'), + resetEnvironment: () => mutate('/api/environment/reset'), } diff --git a/platform/api/src/security.js b/platform/api/src/security.js new file mode 100644 index 0000000..4928336 --- /dev/null +++ b/platform/api/src/security.js @@ -0,0 +1,57 @@ +// CSRF (double-submit cookie) and login rate limiting. No new dependency — both are small +// enough to hand-roll, and SameSite=Lax cookies already do most of the CSRF work; this is +// defense in depth on top of that, not a replacement for it. +// +// CSRF: on login, a non-HttpOnly token is set as a cookie (rn_csrf_s for students, rn_csrf_i +// for instructors) — non-HttpOnly *on purpose*, so our own frontend JS can read it via +// document.cookie and echo it back as a header. A cross-origin attacker's page can trigger a +// cookie-carrying request but cannot read the cookie's value (browser same-origin policy) or +// add a custom header to it without CORS, which we don't grant to other origins — so it can +// never produce a request where the header matches the cookie. + +const crypto = require('crypto'); + +function makeCsrfToken() { + return crypto.randomBytes(24).toString('base64url'); +} + +function parseCookies(req) { + const header = req.headers.cookie; + if (!header) return {}; + return Object.fromEntries( + header.split(';').map((p) => p.trim().split('=')).filter((p) => p.length === 2).map(([k, v]) => [k, decodeURIComponent(v)]) + ); +} + +// Verifies X-CSRF-Token header against the given cookie for any state-changing method. +// GET/HEAD/OPTIONS are read-only and exempt (nothing to protect). +function checkCsrf(req, cookieName) { + if (['GET', 'HEAD', 'OPTIONS'].includes(req.method)) return true; + const cookieValue = parseCookies(req)[cookieName]; + const header = req.headers['x-csrf-token']; + return !!cookieValue && !!header && cookieValue === header; +} + +// ── Login rate limiting ───────────────────────────────────────────────────────────────── +// In-memory sliding window, keyed by "ip:identifier" (e.g. ip:email). Fine for a single-process +// Platform API — a shared store (Postgres/Redis) would be needed for multi-instance, not a +// concern at this scale (see docs/PHASE3A... known limitations). +const attempts = new Map(); + +function tooManyAttempts(key, max, windowMs) { + const now = Date.now(); + const arr = (attempts.get(key) || []).filter((t) => now - t < windowMs); + arr.push(now); + attempts.set(key, arr); + return arr.length > max; +} + +setInterval(() => { + const cutoff = Date.now() - 60 * 60 * 1000; + for (const [key, arr] of attempts) { + const kept = arr.filter((t) => t > cutoff); + if (kept.length) attempts.set(key, kept); else attempts.delete(key); + } +}, 15 * 60 * 1000).unref(); + +module.exports = { makeCsrfToken, parseCookies, checkCsrf, tooManyAttempts }; diff --git a/platform/gateway/src/server.js b/platform/gateway/src/server.js index cb19d3b..2276f49 100644 --- a/platform/gateway/src/server.js +++ b/platform/gateway/src/server.js @@ -23,6 +23,7 @@ const envCache = require('./env-cache'); const PORT = Number(process.env.GATEWAY_PORT) || 4200; const PLATFORM_API_URL = process.env.PLATFORM_API_URL || 'http://localhost:4300'; const STUDENT_COOKIE = 'rn_session'; +const STUDENT_CSRF_COOKIE = 'rn_csrf_s'; // Phase 3A — see relayScenarioOutcome for why the Gateway needs this too // Routing hint only — NOT a security check by itself (ownership is always re-verified via the // session, see checkOwnership/resolveOwnedRunningEnvironment). Its only job is disambiguating // "this unprefixed request came from inside the Lab UI" from "this came from the dashboard," @@ -30,7 +31,9 @@ const STUDENT_COOKIE = 'rn_session'; // dashboard too, and naively using it hijacked EVERY unmatched dashboard request (including // GET /api/session/whoami) to the student's Lab container once one existed. Found by testing. const LAB_ACTIVE_COOKIE = 'rn_lab_active'; -const SESSION_CACHE_TTL_MS = 5000; +// Phase 3A: shortened from 5000ms so a revoked/expired session stops working here within a +// tighter, testable bound (see platform/test/e2e-phase3a.js "revoked mid-use" case). +const SESSION_CACHE_TTL_MS = 2000; const app = express(); // Deliberately no express.json() globally — see the Phase 2A note this carries forward: @@ -120,15 +123,26 @@ async function relayScenarioOutcome({ env, req, res, scenarioPath, extractOutcom const passed = extractOutcome(body); if (passed !== null) { const validationCount = Array.isArray(body.checks) ? body.checks.length : undefined; - const token = parseCookies(req)[STUDENT_COOKIE]; + const cookies = parseCookies(req); + const token = cookies[STUDENT_COOKIE]; + const csrf = cookies[STUDENT_CSRF_COOKIE] || ''; + // Phase 3A: /api/progress/record is CSRF-protected like every other student mutation. The + // double-submit check needs BOTH the rn_csrf_s cookie AND a matching X-CSRF-Token header on + // the request Platform API actually sees — so both are forwarded here, not just the header + // (an earlier version of this fix sent only the header, which still 403'd). fetch(`${PLATFORM_API_URL}/api/progress/record`, { method: 'POST', - headers: { 'Content-Type': 'application/json', Cookie: `${STUDENT_COOKIE}=${token}` }, + headers: { + 'Content-Type': 'application/json', + Cookie: `${STUDENT_COOKIE}=${token}; ${STUDENT_CSRF_COOKIE}=${csrf}`, + 'X-CSRF-Token': csrf, + }, body: JSON.stringify({ scenario_id: req.params.scenarioId, status: passed ? 'completed' : 'in_progress', environment_id: env.environment_id, validation_count: validationCount, }), - }).catch((e) => console.error('[progress relay] failed:', e.message)); + }).then((r) => { if (!r.ok) console.error('[progress relay] Platform API rejected the update:', r.status); }) + .catch((e) => console.error('[progress relay] failed:', e.message)); } }