fix: per-course auth guard, filter courses by tutor, CSV fixes, cascade check
This commit is contained in:
@@ -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<SqlitePool>,
|
||||
claims: TutorClaims,
|
||||
) -> Result<Json<Vec<Course>>, 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<SqlitePool>,
|
||||
Path(course_id): Path<i64>,
|
||||
) -> Result<Json<Vec<Student>>, 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<SqlitePool>,
|
||||
Path(course_id): Path<i64>,
|
||||
Json(req): Json<CreateStudent>,
|
||||
) -> Result<(StatusCode, Json<Value>), 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<SqlitePool>,
|
||||
Path(course_id): Path<i64>,
|
||||
mut multipart: Multipart,
|
||||
) -> Result<(StatusCode, Json<Value>), 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<SqlitePool>,
|
||||
Path(student_id): Path<i64>,
|
||||
) -> Result<StatusCode, AppError> {
|
||||
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<SqlitePool> {
|
||||
#[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::<Value>(&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::<Value>(&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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user