From d0efecf38f70ca41b2c650f6abad72b7fc44279d Mon Sep 17 00:00:00 2001 From: goose Date: Sun, 19 Jul 2026 11:40:44 -0300 Subject: [PATCH] fix(sharing): gate falls back to direct ownership before share check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The share-gate I added in Phase B was too strict: it only admitted the owner via the profiles collection, so any data item whose profile_id didn't map to a real Profile document (legacy/test data, or a profile_id the caller used at create time without a backing Profile) got 404'd on read — even for the item's actual owner. CI caught this: zk_integration_tests::medication_stored_as_ciphertext_not_plaintext creates a medication with profile_id="default" (no Profile doc) and the new get_medication gate returned 404. Fix: in each list/get data handler, check direct ownership (user_id == claims.sub, or has own data for the profile) FIRST, and only fall through to the share-gate if the caller isn't the direct owner. This preserves the original ownership model AND adds share- recipient access, without breaking items whose profile_id isn't a real Profile document. The share-only path (recipient reading shared data they don't own) still goes through the gate as before. --- backend/src/handlers/appointments.rs | 27 ++++++++++++++----- backend/src/handlers/health_stats.rs | 30 ++++++++++++++++----- backend/src/handlers/medications.rs | 39 ++++++++++++++++++++++------ 3 files changed, 74 insertions(+), 22 deletions(-) diff --git a/backend/src/handlers/appointments.rs b/backend/src/handlers/appointments.rs index b4e3244..7939fc4 100644 --- a/backend/src/handlers/appointments.rs +++ b/backend/src/handlers/appointments.rs @@ -69,12 +69,22 @@ pub async fn list_appointments( let database = state.db.get_database(); let repo = AppointmentRepository::new(database.collection("appointments")); - // Share-gate: if a profile_id is specified, resolve the owner (admits the - // owner or an active share recipient) and query as them. Without a - // profile_id, return the caller's own data across all their profiles. + // Resolve the effective owner. If a profile_id is specified, prefer direct + // ownership (the caller's own data for that profile); otherwise fall through + // to the share-gate (admits an active share recipient). profile_id is a + // free-form string at create time and may not map to a Profile document. let owner = match &query.profile_id { Some(pid) => { - crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid).await? + let has_own = repo + .find_by_user_filtered(&claims.sub, None, Some(pid)) + .await + .map(|v| !v.is_empty()) + .unwrap_or(false); + if has_own { + claims.sub.clone() + } else { + crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid).await? + } } None => claims.sub.clone(), }; @@ -121,9 +131,12 @@ pub async fn get_appointment( )); } }; - // Share-gate on the appointment's profile. - crate::handlers::profile_share::authorize_profile_read(&state, &claims, &appt.profile_id) - .await?; + // Authorization: direct owner (user_id == caller) OR share-gate on the + // appointment's profile (admits an active share recipient). + if appt.user_id != claims.sub { + crate::handlers::profile_share::authorize_profile_read(&state, &claims, &appt.profile_id) + .await?; + } Ok(Json(appt.into())) } diff --git a/backend/src/handlers/health_stats.rs b/backend/src/handlers/health_stats.rs index 3042702..5397d8e 100644 --- a/backend/src/handlers/health_stats.rs +++ b/backend/src/handlers/health_stats.rs @@ -90,14 +90,26 @@ pub async fn list_health_stats( .into_response() } }; - // Share-gate: if a profile_id is specified, resolve the owner (admits the - // owner or an active share recipient) and query as them. + // Resolve the effective owner. If a profile_id is specified, prefer direct + // ownership (the caller's own data for that profile); otherwise fall through + // to the share-gate (admits an active share recipient). profile_id is a + // free-form string at create time and may not map to a Profile document. let owner = match &query.profile_id { Some(pid) => { - match crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid).await - { - Ok(o) => o, - Err(resp) => return resp.into_response(), + let has_own = repo + .find_by_user_filtered(&claims.sub, Some(pid)) + .await + .map(|v| !v.is_empty()) + .unwrap_or(false); + if has_own { + claims.sub.clone() + } else { + match crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid) + .await + { + Ok(o) => o, + Err(resp) => return resp.into_response(), + } } } None => claims.sub.clone(), @@ -146,7 +158,11 @@ pub async fn get_health_stat( match repo.find_by_id(&object_id).await { Ok(Some(stat)) => { - // Share-gate on the stat's profile (admits owner or recipient). + // Authorization: direct owner (user_id == caller) OR share-gate on + // the stat's profile (admits an active share recipient). + if stat.user_id == claims.sub { + return (StatusCode::OK, Json(HealthStatResponse::from(stat))).into_response(); + } match crate::handlers::profile_share::authorize_profile_read( &state, &claims, diff --git a/backend/src/handlers/medications.rs b/backend/src/handlers/medications.rs index b3a31b8..2488c9a 100644 --- a/backend/src/handlers/medications.rs +++ b/backend/src/handlers/medications.rs @@ -68,14 +68,27 @@ pub async fn list_medications( let database = state.db.get_database(); let repo = MedicationRepository::new(database.collection("medications")); - // Resolve the effective owner. If the caller specifies a profile_id, run - // the share-gate — it admits the owner or an active share recipient, and - // returns the profile's owner userId to query as. Without a profile_id, - // fall back to the caller's own data across all their profiles (recipients - // must always specify profile_id to read shared data). + // Resolve the effective owner. If the caller specifies a profile_id, prefer + // direct ownership (the caller's own data for that profile); if they don't + // own it, fall through to the share-gate (admits an active share recipient + // and returns the profile's owner userId to query as). Without a profile_id, + // return the caller's own data across all their profiles. + // + // The direct-ownership-first check matters because a medication's profile_id + // is a free-form string at create time and may not map to a Profile document + // (legacy/test data) — the gate would 404 on those even for the real owner. let owner = match &query.profile_id { Some(pid) => { - crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid).await? + let has_own = repo + .find_by_user_and_profile(&claims.sub, pid) + .await + .map(|v| !v.is_empty()) + .unwrap_or(false); + if has_own { + claims.sub.clone() + } else { + crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid).await? + } } None => claims.sub.clone(), }; @@ -124,9 +137,19 @@ pub async fn get_medication( )); } }; - // Share-gate on the medication's profile — admits owner or recipient. - crate::handlers::profile_share::authorize_profile_read(&state, &claims, &medication.profile_id) + // Authorization: admit the direct owner (user_id == caller) OR, if not, + // fall through to the share-gate on the medication's profile (admits an + // active share recipient). The direct-owner check preserves the original + // ownership model for medications whose profile_id may not map to a real + // Profile document (e.g. legacy/test data). + if medication.user_id != claims.sub { + crate::handlers::profile_share::authorize_profile_read( + &state, + &claims, + &medication.profile_id, + ) .await?; + } Ok(Json(medication.into())) }