feat(security): add CSRF double-submit protection and login rate limiting
Non-HttpOnly rn_csrf_s/rn_csrf_i cookies, set on login, echoed back as X-CSRF-Token on every state-changing request — defense in depth on top of (not instead of) SameSite=Lax. In-memory sliding-window rate limiting on instructor login (8 attempts / 5 min per ip:email, no new dependency). Real bug found while wiring this up: the Gateway's own progress-relay call (triggered when a student solves a scenario) is a server-to-server POST to /api/progress/record relaying the browser's session — it needs to forward the CSRF token too, and needs to forward BOTH the rn_csrf_s cookie and a matching X-CSRF-Token header (forwarding only the header still 403'd, since the double-submit check needs both on the request Platform API actually receives). Also tightens the Gateway's session-resolution cache from 5000ms to 2000ms, so a revoked/expired session stops working at the Gateway within a tighter, tested bound. Frontend (admin + student API clients) now reads the relevant rn_csrf_* cookie via document.cookie and attaches it on every POST/PATCH/DELETE.
This commit is contained in:
@@ -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
|
// 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
|
// platform/api/src/sessions.js). No admin data is ever cached client-side beyond the current
|
||||||
// React state; nothing here reads or writes localStorage.
|
// 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) {
|
async function json(res) {
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
@@ -11,23 +21,30 @@ async function json(res) {
|
|||||||
return res.json()
|
return res.json()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function withCsrf(init) {
|
||||||
|
return { ...init, headers: { ...init.headers, 'X-CSRF-Token': getCookie(CSRF_COOKIE) || '' } }
|
||||||
|
}
|
||||||
function post(path, body) {
|
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) {
|
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) {
|
function del(path) {
|
||||||
return fetch(path, { method: 'DELETE' }).then(json)
|
return fetch(path, withCsrf({ method: 'DELETE' })).then(json)
|
||||||
}
|
}
|
||||||
function get(path) {
|
function get(path) {
|
||||||
return fetch(path).then(json)
|
return fetch(path).then(json)
|
||||||
}
|
}
|
||||||
|
|
||||||
export const adminApi = {
|
export const adminApi = {
|
||||||
login: (password) => post('/api/admin/login', { password }),
|
// No CSRF token yet at login (nothing authenticated to forge into).
|
||||||
logout: () => post('/api/admin/logout'),
|
login: (email, password) => fetch('/api/auth/instructor/login', {
|
||||||
whoami: () => get('/api/admin/whoami'),
|
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'),
|
dashboard: () => get('/api/admin/dashboard'),
|
||||||
|
|
||||||
@@ -47,10 +64,20 @@ export const adminApi = {
|
|||||||
cohortStudents: (id) => get(`/api/admin/cohorts/${id}/students`),
|
cohortStudents: (id) => get(`/api/admin/cohorts/${id}/students`),
|
||||||
cohortProgress: (id) => get(`/api/admin/cohorts/${id}/progress`),
|
cohortProgress: (id) => get(`/api/admin/cohorts/${id}/progress`),
|
||||||
assignCohortCurriculum: (id, curriculum_path) => post(`/api/admin/cohorts/${id}/curriculum`, { curriculum_path }),
|
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`),
|
prepareLabs: (id) => post(`/api/admin/cohorts/${id}/prepare-labs`),
|
||||||
destroyLabs: (id) => post(`/api/admin/cohorts/${id}/destroy-labs`),
|
destroyLabs: (id) => post(`/api/admin/cohorts/${id}/destroy-labs`),
|
||||||
|
|
||||||
resetEnvironment: (id) => post(`/api/admin/environments/${id}/reset`),
|
resetEnvironment: (id) => post(`/api/admin/environments/${id}/reset`),
|
||||||
destroyEnvironment: (id) => del(`/api/admin/environments/${id}`),
|
destroyEnvironment: (id) => del(`/api/admin/environments/${id}`),
|
||||||
environmentEvents: (id) => get(`/api/admin/environments/${id}/events`),
|
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'),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,14 +1,21 @@
|
|||||||
// Platform Shell API client. Phase 2B: identity is a server-controlled session cookie
|
// Platform Shell API client. Identity is a server-controlled session cookie (rn_session,
|
||||||
// (rn_session, HttpOnly — this code can't read it, and doesn't need to; the browser sends it
|
// HttpOnly — this code can't read it, and doesn't need to; the browser sends it automatically
|
||||||
// automatically on every same-origin request). This replaces Phase 2A's localStorage
|
// on every same-origin request). Roster name-picking is classroom/pilot identification, not
|
||||||
// student_id + X-Student-Id header, which was explicitly pilot-only and client-forgeable.
|
// production authentication (see docs/PHASE3A_IDENTITY_AND_INSTRUCTOR_ACCESS.md §5) — what's
|
||||||
// See docs/PHASE2B_ADMIN_AND_PLATFORM_DATA.md §3.
|
// 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,
|
// 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
|
// 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.
|
// 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 DISPLAY_CACHE_KEY = 'rootnode_student_display'
|
||||||
|
const CSRF_COOKIE = 'rn_csrf_s'
|
||||||
|
|
||||||
export function getCachedDisplay() {
|
export function getCachedDisplay() {
|
||||||
try { return JSON.parse(sessionStorage.getItem(DISPLAY_CACHE_KEY) || 'null') } catch { return null }
|
try { return JSON.parse(sessionStorage.getItem(DISPLAY_CACHE_KEY) || 'null') } catch { return null }
|
||||||
@@ -20,6 +27,10 @@ function clearCachedDisplay() {
|
|||||||
sessionStorage.removeItem(DISPLAY_CACHE_KEY)
|
sessionStorage.removeItem(DISPLAY_CACHE_KEY)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function getCookie(name) {
|
||||||
|
return document.cookie.split('; ').find((c) => c.startsWith(`${name}=`))?.split('=')[1]
|
||||||
|
}
|
||||||
|
|
||||||
async function json(res) {
|
async function json(res) {
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
const body = await res.json().catch(() => ({}))
|
const body = await res.json().catch(() => ({}))
|
||||||
@@ -28,10 +39,20 @@ async function json(res) {
|
|||||||
return res.json()
|
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 = {
|
export const api = {
|
||||||
cohorts: () => fetch('/api/roster/cohorts').then(json),
|
cohorts: () => fetch('/api/roster/cohorts').then(json),
|
||||||
roster: (cohort) => fetch(`/api/roster?cohort=${encodeURIComponent(cohort)}`).then(json),
|
roster: (cohort) => fetch(`/api/roster?cohort=${encodeURIComponent(cohort)}`).then(json),
|
||||||
login: async (student_id) => {
|
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', {
|
const student = await fetch('/api/login', {
|
||||||
method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ student_id }),
|
method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ student_id }),
|
||||||
}).then(json)
|
}).then(json)
|
||||||
@@ -39,7 +60,7 @@ export const api = {
|
|||||||
return student
|
return student
|
||||||
},
|
},
|
||||||
logout: async () => {
|
logout: async () => {
|
||||||
await fetch('/api/logout', { method: 'POST' }).catch(() => {})
|
await mutate('/api/logout').catch(() => {})
|
||||||
clearCachedDisplay()
|
clearCachedDisplay()
|
||||||
},
|
},
|
||||||
whoami: () => fetch('/api/session/whoami').then(json),
|
whoami: () => fetch('/api/session/whoami').then(json),
|
||||||
@@ -47,6 +68,6 @@ export const api = {
|
|||||||
curriculumPath: () => fetch('/api/curriculum/path').then(json),
|
curriculumPath: () => fetch('/api/curriculum/path').then(json),
|
||||||
|
|
||||||
environmentStatus: () => fetch('/api/environment/status').then(json),
|
environmentStatus: () => fetch('/api/environment/status').then(json),
|
||||||
launchEnvironment: () => fetch('/api/environment/launch', { method: 'POST' }).then(json),
|
launchEnvironment: () => mutate('/api/environment/launch'),
|
||||||
resetEnvironment: () => fetch('/api/environment/reset', { method: 'POST' }).then(json),
|
resetEnvironment: () => mutate('/api/environment/reset'),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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 };
|
||||||
@@ -23,6 +23,7 @@ const envCache = require('./env-cache');
|
|||||||
const PORT = Number(process.env.GATEWAY_PORT) || 4200;
|
const PORT = Number(process.env.GATEWAY_PORT) || 4200;
|
||||||
const PLATFORM_API_URL = process.env.PLATFORM_API_URL || 'http://localhost:4300';
|
const PLATFORM_API_URL = process.env.PLATFORM_API_URL || 'http://localhost:4300';
|
||||||
const STUDENT_COOKIE = 'rn_session';
|
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
|
// 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
|
// session, see checkOwnership/resolveOwnedRunningEnvironment). Its only job is disambiguating
|
||||||
// "this unprefixed request came from inside the Lab UI" from "this came from the dashboard,"
|
// "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
|
// 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.
|
// GET /api/session/whoami) to the student's Lab container once one existed. Found by testing.
|
||||||
const LAB_ACTIVE_COOKIE = 'rn_lab_active';
|
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();
|
const app = express();
|
||||||
// Deliberately no express.json() globally — see the Phase 2A note this carries forward:
|
// 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);
|
const passed = extractOutcome(body);
|
||||||
if (passed !== null) {
|
if (passed !== null) {
|
||||||
const validationCount = Array.isArray(body.checks) ? body.checks.length : undefined;
|
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`, {
|
fetch(`${PLATFORM_API_URL}/api/progress/record`, {
|
||||||
method: 'POST',
|
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({
|
body: JSON.stringify({
|
||||||
scenario_id: req.params.scenarioId, status: passed ? 'completed' : 'in_progress',
|
scenario_id: req.params.scenarioId, status: passed ? 'completed' : 'in_progress',
|
||||||
environment_id: env.environment_id, validation_count: validationCount,
|
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));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user