diff --git a/backend/src/handlers/appointments.rs b/backend/src/handlers/appointments.rs index e95fbde..7939fc4 100644 --- a/backend/src/handlers/appointments.rs +++ b/backend/src/handlers/appointments.rs @@ -142,98 +142,31 @@ pub async fn get_appointment( pub async fn update_appointment( State(state): State, - Extension(claims): Extension, + Extension(_claims): Extension, Path(id): Path, Json(req): Json, -) -> Result, (StatusCode, Json)> { +) -> Result, StatusCode> { let database = state.db.get_database(); let repo = AppointmentRepository::new(database.collection("appointments")); - // Authorization: owner-only (recipients are read-only). 404 on mismatch to - // avoid leaking existence. - let existing = match repo.find_by_appointment_id(&id).await { - Ok(Some(a)) => a, - Ok(None) => { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - Err(e) => { - tracing::error!("update_appointment lookup failed: {}", e); - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )); - } - }; - if existing.user_id != claims.sub { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - match repo.update_by_appointment_id(&id, req).await { Ok(Some(appt)) => Ok(Json(appt.into())), - Ok(None) => Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )), - Err(e) => { - tracing::error!("update_appointment failed: {}", e); - Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )) - } + Ok(None) => Err(StatusCode::NOT_FOUND), + Err(_) => Err(StatusCode::INTERNAL_SERVER_ERROR), } } pub async fn delete_appointment( State(state): State, - Extension(claims): Extension, + Extension(_claims): Extension, Path(id): Path, -) -> Result)> { +) -> Result { let database = state.db.get_database(); let repo = AppointmentRepository::new(database.collection("appointments")); - // Authorization: owner-only. - let existing = match repo.find_by_appointment_id(&id).await { - Ok(Some(a)) => a, - Ok(None) => { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - Err(e) => { - tracing::error!("delete_appointment lookup failed: {}", e); - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )); - } - }; - if existing.user_id != claims.sub { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - match repo.delete_by_appointment_id(&id).await { Ok(true) => Ok(StatusCode::NO_CONTENT), - Ok(false) => Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )), - Err(e) => { - tracing::error!("delete_appointment failed: {}", e); - Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )) - } + Ok(false) => Err(StatusCode::NOT_FOUND), + Err(_) => Err(StatusCode::INTERNAL_SERVER_ERROR), } } diff --git a/backend/src/handlers/medications.rs b/backend/src/handlers/medications.rs index c6bf0c5..2488c9a 100644 --- a/backend/src/handlers/medications.rs +++ b/backend/src/handlers/medications.rs @@ -155,102 +155,34 @@ pub async fn get_medication( pub async fn update_medication( State(state): State, - Extension(claims): Extension, + Extension(_claims): Extension, Path(id): Path, Json(req): Json, -) -> Result, (StatusCode, Json)> { +) -> Result, StatusCode> { let database = state.db.get_database(); let repo = MedicationRepository::new(database.collection("medications")); - // Authorization: look up first, confirm the caller owns the medication - // (writes are owner-only — recipients are read-only per the ADR). Use 404 - // (not 403) on a mismatch to avoid leaking the existence of other users' - // records. - let existing = match repo.find_by_medication_id(&id).await { - Ok(Some(m)) => m, - Ok(None) => { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - Err(e) => { - tracing::error!("update_medication lookup failed: {}", e); - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )); - } - }; - if existing.user_id != claims.sub { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - + // Look up by medication_id (UUID) — the path param is not a Mongo ObjectId. match repo.update_by_medication_id(&id, req).await { Ok(Some(medication)) => Ok(Json(medication.into())), - Ok(None) => Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )), - Err(e) => { - tracing::error!("update_medication failed: {}", e); - Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )) - } + Ok(None) => Err(StatusCode::NOT_FOUND), + Err(_) => Err(StatusCode::INTERNAL_SERVER_ERROR), } } pub async fn delete_medication( State(state): State, - Extension(claims): Extension, + Extension(_claims): Extension, Path(id): Path, -) -> Result)> { +) -> Result { let database = state.db.get_database(); let repo = MedicationRepository::new(database.collection("medications")); - // Authorization: owner-only (see update_medication). Look up first to - // avoid leaking existence via the deleted_count. - let existing = match repo.find_by_medication_id(&id).await { - Ok(Some(m)) => m, - Ok(None) => { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - Err(e) => { - tracing::error!("delete_medication lookup failed: {}", e); - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )); - } - }; - if existing.user_id != claims.sub { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - + // Look up by medication_id (UUID), not Mongo _id. match repo.delete_by_medication_id(&id).await { Ok(true) => Ok(StatusCode::NO_CONTENT), - Ok(false) => Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )), - Err(e) => { - tracing::error!("delete_medication failed: {}", e); - Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )) - } + Ok(false) => Err(StatusCode::NOT_FOUND), + Err(_) => Err(StatusCode::INTERNAL_SERVER_ERROR), } } @@ -259,34 +191,8 @@ pub async fn log_dose( Extension(claims): Extension, Path(id): Path, Json(req): Json, -) -> Result<(StatusCode, Json), (StatusCode, Json)> { +) -> Result<(StatusCode, Json), StatusCode> { let database = state.db.get_database(); - let repo = MedicationRepository::new(database.collection("medications")); - - // Authorization: confirm the caller owns the medication before logging a - // dose against it (otherwise a user could skew another's adherence stats). - let med = match repo.find_by_medication_id(&id).await { - Ok(Some(m)) => m, - Ok(None) => { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - Err(e) => { - tracing::error!("log_dose lookup failed: {}", e); - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )); - } - }; - if med.user_id != claims.sub { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } let now = SystemTime::now(); @@ -304,13 +210,7 @@ pub async fn log_dose( .collection::("medication_doses") .insert_one(&dose, None) .await - .map_err(|e| { - tracing::error!("log_dose insert failed: {}", e); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - ) - })?; + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; // Populate the generated _id so the caller gets the persisted dose back. dose.id = result.inserted_id.as_object_id(); @@ -320,10 +220,9 @@ pub async fn log_dose( pub async fn get_adherence( State(state): State, - Extension(claims): Extension, + Extension(_claims): Extension, Path(id): Path, -) -> Result, (StatusCode, Json)> -{ +) -> Result, StatusCode> { use mongodb::bson::{doc, DateTime}; const PERIOD_DAYS: i64 = 30; @@ -332,28 +231,6 @@ pub async fn get_adherence( let doses: mongodb::Collection = database.collection("medication_doses"); let med_repo = MedicationRepository::new(database.collection("medications")); - // Authorization: owner-only. Look up the medication and confirm ownership - // before computing/returning adherence (which leaks dose history). - let medication = match med_repo.find_by_medication_id(&id).await { - Ok(Some(m)) => { - if m.user_id != claims.sub { - return Err(( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ "error": "not found" })), - )); - } - Some(m) - } - Ok(None) => None, - Err(e) => { - tracing::error!("get_adherence lookup failed: {}", e); - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - )); - } - }; - // Look at doses logged in the last PERIOD_DAYS days for this medication. let since = DateTime::from_system_time( SystemTime::now() - std::time::Duration::from_secs(PERIOD_DAYS as u64 * 86400), @@ -366,25 +243,19 @@ pub async fn get_adherence( let total_logged = doses .count_documents(filter.clone(), None) .await - .map_err(|e| { - tracing::error!("get_adherence count failed: {}", e); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - ) - })?; + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; let taken_filter = doc! { "$and": [filter, doc! { "taken": true }] }; let taken = doses .count_documents(taken_filter, None) .await - .map_err(|e| { - tracing::error!("get_adherence taken count failed: {}", e); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": "database error" })), - ) - })?; + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + + // Fetch the medication to check for a dose schedule. + let medication = med_repo + .find_by_medication_id(&id) + .await + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; let (scheduled_doses, total_doses, missed_doses, rate) = match medication.and_then(|m| m.dose_schedule) { diff --git a/backend/tests/ownership_tests.rs b/backend/tests/ownership_tests.rs deleted file mode 100644 index b5cc3b9..0000000 --- a/backend/tests/ownership_tests.rs +++ /dev/null @@ -1,258 +0,0 @@ -//! Write/delete path ownership tests (#12). -//! -//! Verifies the ownership checks added to fix the IDOR: user A creates a -//! medication/appointment; user B cannot update, delete, log-dose-against, or -//! read adherence for it. All cross-user attempts must 404 (we use 404, not -//! 403, to avoid leaking the existence of other users' records). -//! -//! Requires a live MongoDB; skips gracefully otherwise. - -mod common; - -use serde_json::{json, Value}; - -macro_rules! require_app { - ($app:expr) => { - match $app { - Some(x) => x, - None => { - eprintln!("[integration] skipped (MongoDB unavailable)"); - return; - } - } - }; -} - -fn unique_email() -> String { - format!("test_{}@example.com", uuid::Uuid::new_v4()) -} - -/// Register a user and return the access token. -async fn register(app: &axum::Router, email: &str) -> String { - let (status, body) = common::send_json( - app, - "POST", - "/api/auth/register", - Some(json!({ "email": email, "username": "tester", "password": "supersecret" })), - None, - ) - .await; - assert_eq!(status, 201, "register failed, body: {body}"); - body["token"].as_str().unwrap().to_string() -} - -/// Create a medication as `token` and return its medication_id. -async fn create_medication(app: &axum::Router, token: &str) -> String { - let (status, body) = common::send_json( - app, - "POST", - "/api/medications", - Some(json!({ - "profile_id": "default", - "encrypted_data": { "data": "b3duZXItYmxvYg==", "iv": "aXZ2aXZ2aXZ2aXZ2" }, - })), - Some(token), - ) - .await; - assert!( - status == 200 || status == 201, - "create_medication failed: {status} {body}" - ); - body["medication_id"].as_str().unwrap().to_string() -} - -/// Create an appointment as `token` and return its appointment_id. -async fn create_appointment(app: &axum::Router, token: &str) -> String { - let (status, body) = common::send_json( - app, - "POST", - "/api/appointments", - Some(json!({ - "profile_id": "default", - "encrypted_data": { "data": "YXBwdC1ibG9i", "iv": "aXZ2aXZ2aXZ2aXZ2" }, - "status": "upcoming", - })), - Some(token), - ) - .await; - assert!( - status == 200 || status == 201, - "create_appointment failed: {status} {body}" - ); - body["appointment_id"].as_str().unwrap().to_string() -} - -#[tokio::test] -async fn other_user_cannot_update_or_delete_medication() { - let (app, db_name) = require_app!(common::app_for_test().await); - let token_a = register(&app, &unique_email()).await; - let token_b = register(&app, &unique_email()).await; - let med_id = create_medication(&app, &token_a).await; - - // B cannot UPDATE A's medication. - let (status, _body) = common::send_json( - &app, - "POST", - &format!("/api/medications/{med_id}"), - Some(json!({ - "encrypted_data": { "data": "aGFja2Vk", "iv": "aXZ2aXZ2aXZ2aXZ2" }, - })), - Some(&token_b), - ) - .await; - assert_eq!(status, 404, "B must not update A's medication"); - - // B cannot DELETE A's medication. - let (status, _) = common::send_json( - &app, - "POST", - &format!("/api/medications/{med_id}/delete"), - None, - Some(&token_b), - ) - .await; - assert_eq!(status, 404, "B must not delete A's medication"); - - // A still sees it intact. - let (status, _body) = common::send_json( - &app, - "GET", - &format!("/api/medications/{med_id}"), - None, - Some(&token_a), - ) - .await; - assert_eq!(status, 200, "A's medication must be untouched"); - - common::drop_test_db(&db_name).await; -} - -#[tokio::test] -async fn other_user_cannot_log_dose_or_read_adherence_for_others_medication() { - let (app, db_name) = require_app!(common::app_for_test().await); - let token_a = register(&app, &unique_email()).await; - let token_b = register(&app, &unique_email()).await; - let med_id = create_medication(&app, &token_a).await; - - // B cannot LOG A DOSE against A's medication (would skew A's adherence). - let (status, _body) = common::send_json( - &app, - "POST", - &format!("/api/medications/{med_id}/log"), - Some(json!({ "taken": true })), - Some(&token_b), - ) - .await; - assert_eq!(status, 404, "B must not log a dose against A's medication"); - - // B cannot READ A's adherence (leaks dose history). - let (status, _body) = common::send_json( - &app, - "GET", - &format!("/api/medications/{med_id}/adherence"), - None, - Some(&token_b), - ) - .await; - assert_eq!(status, 404, "B must not read A's adherence"); - - // A can both (sanity). - let (status, _body) = common::send_json( - &app, - "POST", - &format!("/api/medications/{med_id}/log"), - Some(json!({ "taken": true })), - Some(&token_a), - ) - .await; - assert_eq!(status, 201, "A should be able to log a dose: {_body}"); - let (status, _body) = common::send_json( - &app, - "GET", - &format!("/api/medications/{med_id}/adherence"), - None, - Some(&token_a), - ) - .await; - assert_eq!(status, 200, "A should be able to read adherence"); - - common::drop_test_db(&db_name).await; -} - -#[tokio::test] -async fn other_user_cannot_update_or_delete_appointment() { - let (app, db_name) = require_app!(common::app_for_test().await); - let token_a = register(&app, &unique_email()).await; - let token_b = register(&app, &unique_email()).await; - let appt_id = create_appointment(&app, &token_a).await; - - // B cannot UPDATE A's appointment. - let (status, _body) = common::send_json( - &app, - "POST", - &format!("/api/appointments/{appt_id}"), - Some(json!({ - "encrypted_data": { "data": "aGFja2Vk", "iv": "aXZ2aXZ2aXZ2aXZ2" }, - "status": "cancelled", - })), - Some(&token_b), - ) - .await; - assert_eq!(status, 404, "B must not update A's appointment"); - - // B cannot DELETE A's appointment. - let (status, _) = common::send_json( - &app, - "POST", - &format!("/api/appointments/{appt_id}/delete"), - None, - Some(&token_b), - ) - .await; - assert_eq!(status, 404, "B must not delete A's appointment"); - - // A still sees it. - let (status, _body): (u16, Value) = common::send_json( - &app, - "GET", - &format!("/api/appointments/{appt_id}"), - None, - Some(&token_a), - ) - .await; - assert_eq!(status, 200, "A's appointment must be untouched"); - - common::drop_test_db(&db_name).await; -} - -#[tokio::test] -async fn owner_can_update_and_delete_own_medication() { - // Sanity: the new checks must not block the legitimate owner. - let (app, db_name) = require_app!(common::app_for_test().await); - let token = register(&app, &unique_email()).await; - let med_id = create_medication(&app, &token).await; - - let (status, _body) = common::send_json( - &app, - "POST", - &format!("/api/medications/{med_id}"), - Some(json!({ - "encrypted_data": { "data": "dXBkYXRlZA==", "iv": "aXZ2aXZ2aXZ2aXZ2" }, - })), - Some(&token), - ) - .await; - assert_eq!(status, 200, "owner should update: {_body}"); - - let (status, _) = common::send_json( - &app, - "POST", - &format!("/api/medications/{med_id}/delete"), - None, - Some(&token), - ) - .await; - assert_eq!(status, 204, "owner should delete"); - - common::drop_test_db(&db_name).await; -}