diff --git a/backend/src/routes/courses.rs b/backend/src/routes/courses.rs index 1521c19..44be139 100644 --- a/backend/src/routes/courses.rs +++ b/backend/src/routes/courses.rs @@ -13,13 +13,19 @@ use crate::{ models::{Course, CreateCourse, CreateStudent, Student}, }; +// Fix 2: filter courses to only those the tutor is a member of async fn list_courses( - _claims: TutorClaims, State(pool): State, + claims: TutorClaims, ) -> Result>, AppError> { - let courses = sqlx::query_as::<_, Course>("SELECT id, name, semester FROM courses ORDER BY id") - .fetch_all(&pool) - .await?; + let courses = sqlx::query_as::<_, Course>( + "SELECT c.id, c.name, c.semester FROM courses c + JOIN tutor_courses tc ON tc.course_id = c.id + WHERE tc.tutor_id = ?" + ) + .bind(claims.sub) + .fetch_all(&pool) + .await?; Ok(Json(courses)) } @@ -37,11 +43,13 @@ async fn create_course( Ok((StatusCode::CREATED, Json(json!({"id": id, "name": req.name, "semester": req.semester})))) } +// Fix 3: verify tutor has access to this course async fn list_students( - _claims: TutorClaims, + claims: TutorClaims, State(pool): State, Path(course_id): Path, ) -> Result>, AppError> { + super::verify_tutor_course_access(&pool, claims.sub, course_id).await?; let students = sqlx::query_as::<_, Student>( "SELECT id, course_id, name FROM students WHERE course_id = ? ORDER BY id", ) @@ -51,12 +59,14 @@ async fn list_students( Ok(Json(students)) } +// Fix 3: verify tutor has access to this course async fn add_student( - _claims: TutorClaims, + claims: TutorClaims, State(pool): State, Path(course_id): Path, Json(req): Json, ) -> Result<(StatusCode, Json), AppError> { + super::verify_tutor_course_access(&pool, claims.sub, course_id).await?; let id = sqlx::query("INSERT INTO students (course_id, name) VALUES (?, ?)") .bind(course_id) .bind(&req.name) @@ -69,12 +79,15 @@ async fn add_student( )) } +// Fix 3 + Fix 4: verify access, validate CSV header, wrap in transaction, size check async fn import_students( - _claims: TutorClaims, + claims: TutorClaims, State(pool): State, Path(course_id): Path, mut multipart: Multipart, ) -> Result<(StatusCode, Json), AppError> { + super::verify_tutor_course_access(&pool, claims.sub, course_id).await?; + let mut count = 0i64; while let Some(field) = multipart.next_field().await.map_err(|e| { @@ -84,10 +97,21 @@ async fn import_students( AppError::BadRequest(format!("field read error: {e}")) })?; - let mut lines = text.lines(); - // Skip header row - lines.next(); + // Fix 4: body size check + if text.len() > 100_000 { + return Err(AppError::BadRequest("file too large".into())); + } + let mut lines = text.lines(); + + // Fix 4: validate header row + let header = lines.next().unwrap_or("").trim(); + if !header.eq_ignore_ascii_case("name") { + return Err(AppError::BadRequest("CSV must have 'name' header row".into())); + } + + // Fix 4: wrap insert loop in a transaction + let mut tx = pool.begin().await?; for line in lines { let name = line.trim(); if name.is_empty() { @@ -96,27 +120,54 @@ async fn import_students( sqlx::query("INSERT INTO students (course_id, name) VALUES (?, ?)") .bind(course_id) .bind(name) - .execute(&pool) + .execute(&mut *tx) .await?; count += 1; } + tx.commit().await?; } - Ok((StatusCode::CREATED, Json(json!({"imported": count})))) + // Fix 4: return 200 if count == 0, else 201 + let status = if count == 0 { StatusCode::OK } else { StatusCode::CREATED }; + Ok((status, Json(json!({"imported": count})))) } +// Fix 3: fetch student's course_id, verify access, check for attendance records async fn delete_student( - _claims: TutorClaims, + claims: TutorClaims, State(pool): State, Path(student_id): Path, ) -> Result { - let result = sqlx::query("DELETE FROM students WHERE id = ?") + // Fetch the student's course_id first + let row: Option<(i64,)> = sqlx::query_as( + "SELECT course_id FROM students WHERE id = ?" + ) + .bind(student_id) + .fetch_optional(&pool) + .await?; + + let (course_id,) = row.ok_or(AppError::NotFound)?; + + // Verify tutor has access to the course + super::verify_tutor_course_access(&pool, claims.sub, course_id).await?; + + // Check for attendance records + let (att_count,): (i64,) = sqlx::query_as( + "SELECT COUNT(*) FROM attendances WHERE student_id = ?" + ) + .bind(student_id) + .fetch_one(&pool) + .await?; + + if att_count > 0 { + return Err(AppError::Conflict("student has attendance records".into())); + } + + sqlx::query("DELETE FROM students WHERE id = ?") .bind(student_id) .execute(&pool) .await?; - if result.rows_affected() == 0 { - return Err(AppError::NotFound); - } + Ok(StatusCode::NO_CONTENT) } @@ -134,11 +185,25 @@ pub fn router() -> Router { #[cfg(test)] mod tests { use super::*; - use crate::test_helpers::{build_test_app, get, post_json}; + use crate::test_helpers::{build_test_app, delete, get, post_json}; use axum::http::StatusCode; use serde_json::{json, Value}; use sqlx::SqlitePool; + // Fix 6: helper to seed tutor_courses membership + async fn add_tutor_to_course(pool: &SqlitePool, course_id: i64) { + let tutor_id: (i64,) = sqlx::query_as("SELECT id FROM tutors WHERE email = 'tutor@test.com'") + .fetch_one(pool) + .await + .unwrap(); + sqlx::query("INSERT INTO tutor_courses (tutor_id, course_id) VALUES (?, ?)") + .bind(tutor_id.0) + .bind(course_id) + .execute(pool) + .await + .unwrap(); + } + #[sqlx::test(migrations = "./migrations")] async fn create_and_list_courses(pool: SqlitePool) { let (app, auth) = build_test_app(pool.clone()).await; @@ -154,6 +219,9 @@ mod tests { .as_i64() .unwrap(); + // Fix 6: seed tutor_courses membership so the course appears in filtered list + add_tutor_to_course(&pool, id).await; + let (status, body) = get(app, "/api/admin/courses", &auth).await; assert_eq!(status, StatusCode::OK); assert!(serde_json::from_slice::(&body) @@ -179,6 +247,9 @@ mod tests { .as_i64() .unwrap(); + // Fix 6: seed tutor_courses membership before adding students + add_tutor_to_course(&pool, course_id).await; + // Add student let path = format!("/api/admin/courses/{course_id}/students"); let (status, body) = @@ -212,11 +283,26 @@ mod tests { assert_eq!(status, StatusCode::UNAUTHORIZED); } + // Fix 5: GET auth rejection tests + #[sqlx::test(migrations = "./migrations")] + async fn list_courses_requires_auth(pool: SqlitePool) { + let (app, _) = build_test_app(pool).await; + let (status, _) = get(app, "/api/admin/courses", "").await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + } + + #[sqlx::test(migrations = "./migrations")] + async fn list_students_requires_auth(pool: SqlitePool) { + let (app, _) = build_test_app(pool).await; + let (status, _) = get(app, "/api/admin/courses/1/students", "").await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + } + #[sqlx::test(migrations = "./migrations")] async fn delete_student_returns_204(pool: SqlitePool) { let (app, auth) = build_test_app(pool.clone()).await; - // Create course and student + // Create course and seed membership let (_, body) = post_json( app.clone(), "/api/admin/courses", @@ -227,6 +313,7 @@ mod tests { let course_id = serde_json::from_slice::(&body).unwrap()["id"] .as_i64() .unwrap(); + add_tutor_to_course(&pool, course_id).await; let path = format!("/api/admin/courses/{course_id}/students"); let (_, body) = post_json(app.clone(), &path, &auth, json!({"name":"Bob"})).await; @@ -234,32 +321,17 @@ mod tests { .as_i64() .unwrap(); - // Delete student - use tower::ServiceExt; - use axum::http::Request; - let req = Request::builder() - .method("DELETE") - .uri(format!("/api/admin/students/{student_id}")) - .header("Authorization", &auth) - .body(axum::body::Body::empty()) - .unwrap(); - let res = app.oneshot(req).await.unwrap(); - assert_eq!(res.status(), StatusCode::NO_CONTENT); + // Fix 5: use delete helper + let (status, _) = delete(app, &format!("/api/admin/students/{student_id}"), &auth).await; + assert_eq!(status, StatusCode::NO_CONTENT); } #[sqlx::test(migrations = "./migrations")] async fn delete_student_not_found(pool: SqlitePool) { let (app, auth) = build_test_app(pool.clone()).await; - use tower::ServiceExt; - use axum::http::Request; - let req = Request::builder() - .method("DELETE") - .uri("/api/admin/students/9999") - .header("Authorization", &auth) - .body(axum::body::Body::empty()) - .unwrap(); - let res = app.oneshot(req).await.unwrap(); - assert_eq!(res.status(), StatusCode::NOT_FOUND); + // Fix 5: use delete helper + let (status, _) = delete(app, "/api/admin/students/9999", &auth).await; + assert_eq!(status, StatusCode::NOT_FOUND); } } diff --git a/backend/src/routes/mod.rs b/backend/src/routes/mod.rs index ad5da9d..6be180b 100644 --- a/backend/src/routes/mod.rs +++ b/backend/src/routes/mod.rs @@ -1,6 +1,8 @@ use axum::Router; use sqlx::SqlitePool; +use crate::error::AppError; + mod auth_routes; mod courses; @@ -10,3 +12,20 @@ pub fn build(pool: SqlitePool) -> Router { .merge(courses::router()) .with_state(pool) } + +/// Verify that `tutor_id` is a member of `course_id` via the tutor_courses join table. +/// Returns `AppError::Unauthorized` (403-equivalent) if not a member, propagates DB errors. +pub async fn verify_tutor_course_access( + pool: &SqlitePool, + tutor_id: i64, + course_id: i64, +) -> Result<(), AppError> { + let row: Option<(i64,)> = sqlx::query_as( + "SELECT 1 FROM tutor_courses WHERE tutor_id = ? AND course_id = ?" + ) + .bind(tutor_id) + .bind(course_id) + .fetch_optional(pool) + .await?; + row.map(|_| ()).ok_or(AppError::Unauthorized) +} diff --git a/backend/src/test_helpers.rs b/backend/src/test_helpers.rs index ac3c018..320db74 100644 --- a/backend/src/test_helpers.rs +++ b/backend/src/test_helpers.rs @@ -59,3 +59,16 @@ pub async fn get(app: Router, path: &str, auth: &str) -> (StatusCode, bytes::Byt let body = res.into_body().collect().await.unwrap().to_bytes(); (status, body) } + +/// DELETE from the app (one-shot), returns (StatusCode, response body bytes). +pub async fn delete(app: Router, path: &str, auth: &str) -> (StatusCode, bytes::Bytes) { + let mut builder = Request::builder().method("DELETE").uri(path); + if !auth.is_empty() { + builder = builder.header("Authorization", auth); + } + let req = builder.body(axum::body::Body::empty()).unwrap(); + let res = app.oneshot(req).await.unwrap(); + let status = res.status(); + let body = res.into_body().collect().await.unwrap().to_bytes(); + (status, body) +}