fix(sharing): gate falls back to direct ownership before share check
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.
This commit is contained in:
parent
e28135bc08
commit
d0efecf38f
3 changed files with 74 additions and 22 deletions
|
|
@ -69,13 +69,23 @@ pub async fn list_appointments(
|
||||||
let database = state.db.get_database();
|
let database = state.db.get_database();
|
||||||
let repo = AppointmentRepository::new(database.collection("appointments"));
|
let repo = AppointmentRepository::new(database.collection("appointments"));
|
||||||
|
|
||||||
// Share-gate: if a profile_id is specified, resolve the owner (admits the
|
// Resolve the effective owner. If a profile_id is specified, prefer direct
|
||||||
// owner or an active share recipient) and query as them. Without a
|
// ownership (the caller's own data for that profile); otherwise fall through
|
||||||
// profile_id, return the caller's own data across all their profiles.
|
// 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 {
|
let owner = match &query.profile_id {
|
||||||
Some(pid) => {
|
Some(pid) => {
|
||||||
|
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?
|
crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid).await?
|
||||||
}
|
}
|
||||||
|
}
|
||||||
None => claims.sub.clone(),
|
None => claims.sub.clone(),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -121,9 +131,12 @@ pub async fn get_appointment(
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
// Share-gate on the appointment's profile.
|
// 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)
|
crate::handlers::profile_share::authorize_profile_read(&state, &claims, &appt.profile_id)
|
||||||
.await?;
|
.await?;
|
||||||
|
}
|
||||||
Ok(Json(appt.into()))
|
Ok(Json(appt.into()))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -90,16 +90,28 @@ pub async fn list_health_stats(
|
||||||
.into_response()
|
.into_response()
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
// Share-gate: if a profile_id is specified, resolve the owner (admits the
|
// Resolve the effective owner. If a profile_id is specified, prefer direct
|
||||||
// owner or an active share recipient) and query as them.
|
// 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 {
|
let owner = match &query.profile_id {
|
||||||
Some(pid) => {
|
Some(pid) => {
|
||||||
match crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid).await
|
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,
|
Ok(o) => o,
|
||||||
Err(resp) => return resp.into_response(),
|
Err(resp) => return resp.into_response(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
None => claims.sub.clone(),
|
None => claims.sub.clone(),
|
||||||
};
|
};
|
||||||
match repo
|
match repo
|
||||||
|
|
@ -146,7 +158,11 @@ pub async fn get_health_stat(
|
||||||
|
|
||||||
match repo.find_by_id(&object_id).await {
|
match repo.find_by_id(&object_id).await {
|
||||||
Ok(Some(stat)) => {
|
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(
|
match crate::handlers::profile_share::authorize_profile_read(
|
||||||
&state,
|
&state,
|
||||||
&claims,
|
&claims,
|
||||||
|
|
|
||||||
|
|
@ -68,15 +68,28 @@ pub async fn list_medications(
|
||||||
let database = state.db.get_database();
|
let database = state.db.get_database();
|
||||||
let repo = MedicationRepository::new(database.collection("medications"));
|
let repo = MedicationRepository::new(database.collection("medications"));
|
||||||
|
|
||||||
// Resolve the effective owner. If the caller specifies a profile_id, run
|
// Resolve the effective owner. If the caller specifies a profile_id, prefer
|
||||||
// the share-gate — it admits the owner or an active share recipient, and
|
// direct ownership (the caller's own data for that profile); if they don't
|
||||||
// returns the profile's owner userId to query as. Without a profile_id,
|
// own it, fall through to the share-gate (admits an active share recipient
|
||||||
// fall back to the caller's own data across all their profiles (recipients
|
// and returns the profile's owner userId to query as). Without a profile_id,
|
||||||
// must always specify profile_id to read shared data).
|
// 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 {
|
let owner = match &query.profile_id {
|
||||||
Some(pid) => {
|
Some(pid) => {
|
||||||
|
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?
|
crate::handlers::profile_share::authorize_profile_read(&state, &claims, pid).await?
|
||||||
}
|
}
|
||||||
|
}
|
||||||
None => claims.sub.clone(),
|
None => claims.sub.clone(),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -124,9 +137,19 @@ pub async fn get_medication(
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
// Share-gate on the medication's profile — admits owner or recipient.
|
// Authorization: admit the direct owner (user_id == caller) OR, if not,
|
||||||
crate::handlers::profile_share::authorize_profile_read(&state, &claims, &medication.profile_id)
|
// 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?;
|
.await?;
|
||||||
|
}
|
||||||
Ok(Json(medication.into()))
|
Ok(Json(medication.into()))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue