feat(student): admin student-account management + CSV import, session hardening
- admin-routes.js: create-with-credentials, /convert, /reset-password, /revoke-sessions, /sessions, /import (transactional CSV import — whole file validated, one Postgres transaction, never accepts a password/hash column from input). Fixed GET /api/admin/students to select an explicit safe column list instead of SELECT * — the new password_hash/credential_token_hash columns must never be exposed through this route. - server.js: /api/login now also blocks on credential_token_hash (not just password_hash) — closes a real window where a student mid-conversion stayed reachable via the old bare-student_id path. studentSession middleware now re-verifies status='active' on every request, not just at session creation, matching instructorSession's existing behavior. - sessions.js: revokeAllStudentSessions, listStudentSessions. - rbac.js: extracted checkCohortAccess/checkCohortWriteAccess as shared core functions so the new classroom/help-request RBAC (next commit) reuses the exact same authorization rule instead of a parallel copy. - student-csv.js + test: hand-rolled parser (no new dependency), unit tested. - prometheus.yml.j2: activated the platform-api scrape job.
This commit is contained in:
@@ -16,6 +16,9 @@
|
||||
const { listInstructors, createInstructor, assertNotLastOwner } = require('./instructors');
|
||||
const attemptsLib = require('./attempts'); // Phase 4.1 §13 — abandonment on instructor-driven reset/destroy
|
||||
const { parsePagination, paginatedResponse } = require('./pagination'); // Phase 4.1 §16
|
||||
const { issueCredentialToken } = require('./students-auth'); // Phase 5A §4/§5/§7
|
||||
const { prepareLabsForCohort } = require('./prepare-labs'); // Phase 5A §21 — shared with admin-classroom-routes.js
|
||||
const { parseStudentCsv } = require('./student-csv'); // Phase 5A §29
|
||||
|
||||
module.exports = function registerAdminRoutes(app, deps) {
|
||||
const { instructorSession, rbac, provisioner, curriculum, getPool, loadProgress, recordAudit } = deps;
|
||||
@@ -95,33 +98,168 @@ module.exports = function registerAdminRoutes(app, deps) {
|
||||
});
|
||||
|
||||
// ── Students ─────────────────────────────────────────────────────────────────────────
|
||||
// Phase 5A: explicit column list, not SELECT * — students now carries password_hash and
|
||||
// credential_token_hash (migration 017), which must never reach an API response
|
||||
// (docs/PHASE5A_STUDENT_IDENTITY_REMOTE_CLASSROOM.md §2). (password_hash IS NOT NULL) is
|
||||
// exposed as a boolean, matching students-auth.js's publicStudent() shape, so the admin UI
|
||||
// can tell "pilot, needs conversion" from "already credentialed" without ever seeing the hash.
|
||||
admin.get('/api/admin/students', requireAnyInstructor, async (req, res) => {
|
||||
const pool = getPool();
|
||||
const { rows } = req.instructor.role === 'instructor'
|
||||
? await pool.query(
|
||||
`SELECT DISTINCT s.* FROM students s
|
||||
`SELECT DISTINCT s.student_id, s.display_name, s.status, s.email, s.login_identifier,
|
||||
(s.password_hash IS NOT NULL) AS has_credentials, s.created_at, s.updated_at, s.last_active_at, s.last_login_at
|
||||
FROM students s
|
||||
JOIN cohort_enrollments e ON e.student_id = s.student_id
|
||||
JOIN cohort_instructors ci ON ci.cohort_id = e.cohort_id
|
||||
WHERE ci.instructor_id = $1 AND e.enrollment_status = 'enrolled' ORDER BY s.display_name`, [req.instructor.instructor_id])
|
||||
: await pool.query(`SELECT * FROM students ORDER BY display_name`);
|
||||
: await pool.query(
|
||||
`SELECT student_id, display_name, status, email, login_identifier,
|
||||
(password_hash IS NOT NULL) AS has_credentials, created_at, updated_at, last_active_at, last_login_at
|
||||
FROM students ORDER BY display_name`);
|
||||
res.json(rows);
|
||||
});
|
||||
|
||||
// Phase 5A §2/§3: email/login_identifier are both optional and independent — an older
|
||||
// self-paced student gets email, a younger instructor-managed one gets login_identifier
|
||||
// (any instructor-assigned safe string, not necessarily an email). Providing EITHER issues
|
||||
// an activation token immediately (§5's "instructor creates account -> temporary credential"
|
||||
// flow) and returns it ONCE in this response — the instructor relays it to the student
|
||||
// out-of-band (print it, read it to them, hand it over); it is never stored in plaintext or
|
||||
// logged anywhere (students-auth.js's issueCredentialToken only ever persists the hash).
|
||||
// Providing neither creates a plain pilot row, unchanged from before Phase 5A.
|
||||
admin.post('/api/admin/students', requireAdminOrOwner, async (req, res) => {
|
||||
const { display_name, cohort_id } = req.body || {};
|
||||
const { display_name, cohort_id, email, login_identifier } = req.body || {};
|
||||
if (!display_name) return res.status(400).json({ error: 'display_name is required' });
|
||||
const pool = getPool();
|
||||
const { rows: countRows } = await pool.query('SELECT count(*)::int AS n FROM students');
|
||||
const studentId = `student-${String(countRows[0].n + 1).padStart(3, '0')}`;
|
||||
await pool.query(`INSERT INTO students (student_id, display_name) VALUES ($1, $2)`, [studentId, display_name.trim()]);
|
||||
try {
|
||||
await pool.query(
|
||||
`INSERT INTO students (student_id, display_name, email, login_identifier) VALUES ($1, $2, $3, $4)`,
|
||||
[studentId, display_name.trim(), email ? String(email).toLowerCase().trim() : null, login_identifier ? String(login_identifier).trim() : null]
|
||||
);
|
||||
} catch (e) {
|
||||
if (e.code === '23505') return res.status(409).json({ error: 'email_or_identifier_in_use' });
|
||||
throw e;
|
||||
}
|
||||
if (cohort_id) {
|
||||
await pool.query(
|
||||
`INSERT INTO cohort_enrollments (cohort_id, student_id) VALUES ($1, $2) ON CONFLICT DO NOTHING`,
|
||||
[cohort_id, studentId]
|
||||
);
|
||||
}
|
||||
await recordAudit(pool, { actorType: 'instructor', actorId: req.instructor.instructor_id, action: 'roster_add', resourceType: 'student', resourceId: studentId, result: 'success', sourceIp: req.ip, metadata: { cohort_id } });
|
||||
res.status(201).json({ student_id: studentId, display_name: display_name.trim() });
|
||||
let activationToken = null;
|
||||
if (email || login_identifier) {
|
||||
const issued = await issueCredentialToken(pool, studentId, 'activation');
|
||||
activationToken = issued.token;
|
||||
}
|
||||
await recordAudit(pool, { actorType: 'instructor', actorId: req.instructor.instructor_id, action: 'roster_add', resourceType: 'student', resourceId: studentId, result: 'success', sourceIp: req.ip, metadata: { cohort_id, credentialed: !!activationToken } });
|
||||
res.status(201).json({ student_id: studentId, display_name: display_name.trim(), activation_token: activationToken });
|
||||
});
|
||||
|
||||
// §7 — converts an existing PILOT student (password_hash IS NULL) into a credentialed
|
||||
// account: assigns email/login_identifier and issues an activation token, same mechanism as
|
||||
// creation. Nothing about the student's existing progress/attempts/achievements/cohort
|
||||
// membership is touched — student_id, the FK every other table hangs off of, never changes.
|
||||
admin.post('/api/admin/students/:id/convert', requireAdminOrOwner, async (req, res) => {
|
||||
const { email, login_identifier } = req.body || {};
|
||||
if (!email && !login_identifier) return res.status(400).json({ error: 'email or login_identifier is required' });
|
||||
const pool = getPool();
|
||||
const { rows } = await pool.query(`SELECT student_id, password_hash FROM students WHERE student_id = $1`, [req.params.id]);
|
||||
if (!rows.length) return res.status(404).json({ error: 'not found' });
|
||||
if (rows[0].password_hash) return res.status(409).json({ error: 'already_converted', message: 'This student already has real credentials.' });
|
||||
try {
|
||||
await pool.query(
|
||||
`UPDATE students SET email = COALESCE($1, email), login_identifier = COALESCE($2, login_identifier), updated_at = now() WHERE student_id = $3`,
|
||||
[email ? String(email).toLowerCase().trim() : null, login_identifier ? String(login_identifier).trim() : null, req.params.id]
|
||||
);
|
||||
} catch (e) {
|
||||
if (e.code === '23505') return res.status(409).json({ error: 'email_or_identifier_in_use' });
|
||||
throw e;
|
||||
}
|
||||
const { token } = await issueCredentialToken(pool, req.params.id, 'activation');
|
||||
await recordAudit(pool, { actorType: 'instructor', actorId: req.instructor.instructor_id, action: 'student_converted', resourceType: 'student', resourceId: req.params.id, result: 'success', sourceIp: req.ip });
|
||||
res.json({ student_id: req.params.id, activation_token: token });
|
||||
});
|
||||
|
||||
// §4 — for an ALREADY-credentialed account (unlike convert, above). Issues a fresh 'reset'
|
||||
// token and revokes every existing session, same "instructor never needs to know the
|
||||
// student's password" principle as activation.
|
||||
admin.post('/api/admin/students/:id/reset-password', requireAdminOrOwner, async (req, res) => {
|
||||
const pool = getPool();
|
||||
const { rows } = await pool.query(`SELECT student_id, password_hash FROM students WHERE student_id = $1`, [req.params.id]);
|
||||
if (!rows.length) return res.status(404).json({ error: 'not found' });
|
||||
if (!rows[0].password_hash) return res.status(409).json({ error: 'not_yet_converted', message: 'Convert this student to a real account first.' });
|
||||
const { token } = await issueCredentialToken(pool, req.params.id, 'reset');
|
||||
await require('./sessions').revokeAllStudentSessions(pool, req.params.id);
|
||||
await recordAudit(pool, { actorType: 'instructor', actorId: req.instructor.instructor_id, action: 'student_password_reset', resourceType: 'student', resourceId: req.params.id, result: 'success', sourceIp: req.ip });
|
||||
res.json({ student_id: req.params.id, reset_token: token });
|
||||
});
|
||||
|
||||
admin.post('/api/admin/students/:id/revoke-sessions', requireAdminOrOwner, async (req, res) => {
|
||||
await require('./sessions').revokeAllStudentSessions(getPool(), req.params.id);
|
||||
await recordAudit(getPool(), { actorType: 'instructor', actorId: req.instructor.instructor_id, action: 'student_sessions_revoked', resourceType: 'student', resourceId: req.params.id, result: 'success', sourceIp: req.ip });
|
||||
res.json({ ok: true });
|
||||
});
|
||||
|
||||
admin.get('/api/admin/students/:id/sessions', requireAnyInstructor, requireStudentCohortAccess('id'), async (req, res) => {
|
||||
const sessionsList = await require('./sessions').listStudentSessions(getPool(), req.params.id);
|
||||
res.json(sessionsList);
|
||||
});
|
||||
|
||||
// §29 — CSV bulk roster import. Deliberately transactional: the WHOLE file is parsed and
|
||||
// validated first; if any row is invalid, NOTHING is imported and every row-level error is
|
||||
// returned, rather than silently importing the valid rows and skipping the rest (the safer
|
||||
// default per docs/PHASE5A_STUDENT_IDENTITY_REMOTE_CLASSROOM.md §29 — a partial import that
|
||||
// "mostly worked" is a worse failure mode than a rejected file with a clear error list).
|
||||
// Expected columns: display_name, login_identifier (or email), cohort_id (optional). Never
|
||||
// accepts a password/password_hash column — activation tokens are always generated server-side.
|
||||
admin.post('/api/admin/students/import', requireAdminOrOwner, async (req, res) => {
|
||||
const { csv } = req.body || {};
|
||||
if (!csv || typeof csv !== 'string') return res.status(400).json({ error: 'csv (text) is required' });
|
||||
const { rows: parsedRows, errors: parseErrors } = parseStudentCsv(csv);
|
||||
if (parseErrors.length) return res.status(400).json({ error: 'invalid_csv', row_errors: parseErrors });
|
||||
|
||||
const pool = getPool();
|
||||
const client = await pool.connect();
|
||||
const created = [];
|
||||
try {
|
||||
await client.query('BEGIN');
|
||||
const { rows: countRows } = await client.query('SELECT count(*)::int AS n FROM students');
|
||||
let nextN = countRows[0].n + 1;
|
||||
for (const row of parsedRows) {
|
||||
const studentId = `student-${String(nextN++).padStart(3, '0')}`;
|
||||
await client.query(
|
||||
`INSERT INTO students (student_id, display_name, email, login_identifier) VALUES ($1, $2, $3, $4)`,
|
||||
[studentId, row.display_name, row.email || null, row.login_identifier || null]
|
||||
);
|
||||
if (row.cohort_id) {
|
||||
const { rowCount } = await client.query(`SELECT 1 FROM cohorts WHERE cohort_id = $1`, [row.cohort_id]);
|
||||
if (!rowCount) throw Object.assign(new Error(`cohort_id '${row.cohort_id}' does not exist (row for ${row.display_name})`), { code: 'BAD_COHORT' });
|
||||
await client.query(`INSERT INTO cohort_enrollments (cohort_id, student_id) VALUES ($1, $2)`, [row.cohort_id, studentId]);
|
||||
}
|
||||
let activationToken = null;
|
||||
if (row.email || row.login_identifier) {
|
||||
// issueCredentialToken only ever calls .query() on what it's given — a transaction
|
||||
// client works identically to a pool here, so the whole import (including every
|
||||
// token) commits or rolls back together with everything else.
|
||||
const issued = await issueCredentialToken(client, studentId, 'activation');
|
||||
activationToken = issued.token;
|
||||
}
|
||||
created.push({ student_id: studentId, display_name: row.display_name, activation_token: activationToken });
|
||||
}
|
||||
await client.query('COMMIT');
|
||||
} catch (e) {
|
||||
await client.query('ROLLBACK');
|
||||
if (e.code === '23505') return res.status(409).json({ error: 'duplicate_email_or_identifier_in_file_or_existing_roster' });
|
||||
if (e.code === 'BAD_COHORT') return res.status(400).json({ error: 'invalid_cohort', message: e.message });
|
||||
throw e;
|
||||
} finally {
|
||||
client.release();
|
||||
}
|
||||
await recordAudit(getPool(), { actorType: 'instructor', actorId: req.instructor.instructor_id, action: 'roster_csv_import', result: 'success', sourceIp: req.ip, metadata: { count: created.length } });
|
||||
res.status(201).json({ created });
|
||||
});
|
||||
|
||||
admin.patch('/api/admin/students/:id', requireAdminOrOwner, async (req, res) => {
|
||||
@@ -306,23 +444,9 @@ module.exports = function registerAdminRoutes(app, deps) {
|
||||
|
||||
// ── Classroom prep: Prepare Labs / Destroy Cohort Labs ──────────────────────────────────
|
||||
admin.post('/api/admin/cohorts/:id/prepare-labs', requireAnyInstructor, requireCohortWriteAccess('id'), async (req, res) => {
|
||||
const { rows: students } = await getPool().query(
|
||||
`SELECT s.student_id, s.display_name FROM cohort_enrollments e JOIN students s ON s.student_id = e.student_id
|
||||
WHERE e.cohort_id = $1 AND e.enrollment_status = 'enrolled' AND s.status = 'active'`,
|
||||
[req.params.id]
|
||||
);
|
||||
const results = await Promise.allSettled(students.map(async (s) => {
|
||||
const existing = await provisioner.listForStudent(s.student_id);
|
||||
const active = existing.body?.find((e) => ['requested', 'provisioning', 'running', 'resetting'].includes(e.status));
|
||||
if (active) return { student_id: s.student_id, display_name: s.display_name, status: active.status };
|
||||
const { status, body } = await provisioner.provision(s.student_id);
|
||||
if (status !== 201) return { student_id: s.student_id, display_name: s.display_name, status: 'error', error: body?.error };
|
||||
return { student_id: s.student_id, display_name: s.display_name, status: body.status };
|
||||
}));
|
||||
await recordAudit(getPool(), { actorType: 'instructor', actorId: req.instructor.instructor_id, action: 'prepare_labs', resourceType: 'cohort', resourceId: req.params.id, result: 'success', sourceIp: req.ip, metadata: { student_count: students.length } });
|
||||
res.json({
|
||||
results: results.map((r, i) => r.status === 'fulfilled' ? r.value : { student_id: students[i].student_id, display_name: students[i].display_name, status: 'error', error: r.reason?.message }),
|
||||
});
|
||||
const results = await prepareLabsForCohort(getPool(), provisioner, req.params.id);
|
||||
await recordAudit(getPool(), { actorType: 'instructor', actorId: req.instructor.instructor_id, action: 'prepare_labs', resourceType: 'cohort', resourceId: req.params.id, result: 'success', sourceIp: req.ip, metadata: { student_count: results.length } });
|
||||
res.json({ results });
|
||||
});
|
||||
|
||||
admin.post('/api/admin/cohorts/:id/destroy-labs', requireAnyInstructor, requireCohortWriteAccess('id'), async (req, res) => {
|
||||
|
||||
Reference in New Issue
Block a user