diff --git a/openvtc-core/src/persona/claim_types.rs b/openvtc-core/src/persona/claim_types.rs index 4b926ac..51ed380 100644 --- a/openvtc-core/src/persona/claim_types.rs +++ b/openvtc-core/src/persona/claim_types.rs @@ -268,6 +268,15 @@ impl ClaimTypeDefaults { self.mask.hides_anything() } + /// Whether a value of this type is withheld from a listing that did not ask + /// for sensitive values (`sensitivity: high`). A valueless row of such a type + /// is "not fetched yet" (press `s`), not "absent" — the distinction the pane + /// must keep so a card number does not read as "you hold nothing". + #[must_use] + pub fn is_sensitive(self) -> bool { + matches!(self.sensitivity, Sensitivity::High) + } + /// The value as this type shows it — masked when the type asks for it. #[must_use] pub fn render(self, text: &str) -> String { diff --git a/openvtc-core/src/persona/pool.rs b/openvtc-core/src/persona/pool.rs index d092b36..dfb3933 100644 --- a/openvtc-core/src/persona/pool.rs +++ b/openvtc-core/src/persona/pool.rs @@ -195,6 +195,23 @@ impl PoolAttribute { !self.stale && self.value.is_some() && self.claim_defaults(registry).masks_by_default() } + /// Whether this row is valueless *because it is sensitive and the listing + /// did not fetch sensitive values*, rather than genuinely absent. + /// + /// The pane uses this to offer `s` (a per-attribute reveal that fetches the + /// one value) and to render "sensitive — press s" instead of "(no value)": + /// `••••••••`, "(no value)" and a withheld card number are one glance apart, + /// and confusing them misinforms the holder about what they hold. Only + /// meaningful once values were requested (`values_requested`): in picker mode + /// nothing has a value and none of it is "withheld". + #[must_use] + pub fn is_withheld_sensitive(&self, registry: &Registry, values_requested: bool) -> bool { + values_requested + && !self.stale + && self.value.is_none() + && self.claim_defaults(registry).is_sensitive() + } + /// The value as one line, or the reason there is none — masked when its /// claim type asks for that. /// @@ -311,28 +328,26 @@ impl AttributeEdit { /// Enumerate the pool. /// -/// `include_values` is the whole of the difference between a picker and a read -/// of the holder's identity — see the module header. +/// Two independent escalations, both the holder's to make: +/// +/// - `include_values` — the difference between a picker and a read of the +/// holder's identity (see the module header). +/// - `include_sensitive` — vta-sdk's *second* escalation, and it only ever +/// widens the first. It is kept separate on purpose. The bulk listing behind +/// the pane's `show_values` (`v`) passes `false`, so it does **not** carry +/// every `sensitivity: high` value (card numbers, passport ids) into this +/// process; a `sensitivity: high` attribute then comes back valueless, which +/// the pane renders as "sensitive — press s" rather than "(no value)" (it +/// knows the type is sensitive). The per-attribute reveal ([`reveal`]) fetches +/// that one value on its own with `include_sensitive: true`, so the default +/// read stays lean and the reveal stays truthful. pub async fn list( client: &VtaClient, include_values: bool, + include_sensitive: bool, ) -> Result, OpenVTCError> { let value = client - // `include_sensitive` is vta-sdk 0.34's second escalation, and it is - // passed `include_values` rather than a constant on purpose. In this - // client the two questions have one answer: the only caller is the - // identity pane's `show_values` toggle, which is the holder saying - // "show me what I hold" — and when it is on, the pane reveals masked - // values whole from what this call returned. Passing `false` here would - // not narrow that read, it would make every `sensitivity: high` - // attribute come back valueless and render as "(no value)" under the - // reveal — which is a *wrong answer* about what the holder holds, and - // one glance away from "nothing is stored". Honouring the escalation - // properly means the `s` reveal fetching that one value on its own, so - // the default read stops carrying every card number into this process - // while the reveal stays truthful. That is a pane change, not a - // dependency bump. - .persona_attribute_list(None, include_values, include_values, None, None, None) + .persona_attribute_list(None, include_values, include_sensitive, None, None, None) .await .map_err(|e| OpenVTCError::Vta(format!("persona attribute list failed: {e}")))?; @@ -352,6 +367,32 @@ pub async fn list( Ok(attributes) } +/// Fetch one attribute's value, sensitive values included, for the per-attribute +/// reveal (`s`) — so the holder gets the one value they asked for without the +/// bulk [`list`] having carried every sensitive value into memory. +/// +/// The SDK has no by-id read, so this is a `type_prefix`-scoped list (a prefix +/// match on the claim type, which may return several rows) filtered to +/// `attribute_id`. Returns `Ok(None)` when the store no longer has it (deleted +/// or renamed out from under the pane). +pub async fn reveal( + client: &VtaClient, + claim_type: &str, + attribute_id: &str, +) -> Result, OpenVTCError> { + let value = client + .persona_attribute_list(Some(claim_type), true, true, None, None, None) + .await + .map_err(|e| OpenVTCError::Vta(format!("persona attribute reveal failed: {e}")))?; + Ok(value + .get("attributes") + .and_then(Value::as_array) + .into_iter() + .flatten() + .map(PoolAttribute::from_wire) + .find(|a| a.attribute_id == attribute_id)) +} + /// Create or update a self-asserted attribute. /// /// A create is a `put` with no `attributeId`; the VTA mints one and returns it. @@ -618,6 +659,34 @@ mod tests { assert!(attr.is_masked(®())); } + /// A value withheld because it is sensitive is "not loaded", a third state + /// distinct from masked (in memory) and absent (nothing held) — so the pane + /// can offer `s` to fetch it rather than render "(no value)". + #[test] + fn a_withheld_sensitive_value_is_distinct_from_absent() { + // Fixture assumptions, asserted so a classification change fails loudly. + let sensitive = "medical.condition"; + let ordinary = "name.given"; + assert!(reg().resolve(sensitive).is_sensitive()); + assert!(!reg().resolve(ordinary).is_sensitive()); + + let mut attr = PoolAttribute::from_wire(&wire("selfAsserted")); + attr.value = None; + + // Sensitive + valueless + values requested → withheld (fetchable via `s`). + attr.claim_type = sensitive.into(); + assert!(attr.is_withheld_sensitive(®(), true)); + // Non-sensitive + valueless → genuinely absent, not withheld. + attr.claim_type = ordinary.into(); + assert!(!attr.is_withheld_sensitive(®(), true)); + // Picker mode (values not requested): nothing is "withheld". + attr.claim_type = sensitive.into(); + assert!(!attr.is_withheld_sensitive(®(), false)); + // Once the value is in hand it is no longer withheld. + attr.value = Some(Value::String("held".into())); + assert!(!attr.is_withheld_sensitive(®(), true)); + } + /// A stale value keeps saying it is stale. The reason it cannot be shown is /// not that it is masked, and a mask over it would hide the one thing the /// holder needs to act on. diff --git a/openvtc/src/state_handler/persona_actions.rs b/openvtc/src/state_handler/persona_actions.rs index 63a3ed0..a02454f 100644 --- a/openvtc/src/state_handler/persona_actions.rs +++ b/openvtc/src/state_handler/persona_actions.rs @@ -69,6 +69,13 @@ pub(crate) enum PersonaJob { attribute_id: String, cascade: bool, }, + /// Fetch one attribute's value on its own (`s` on a sensitive row), so the + /// bulk listing never has to carry every sensitive value into memory. The + /// result is spliced into the in-memory row and the mask lifted. + AttributeReveal { + attribute_id: String, + claim_type: String, + }, ProfilePut { profile_id: Option, name: String, @@ -178,17 +185,25 @@ pub(crate) fn apply(state: &mut State, action: &PersonaAction) -> PersonaEffect let Some(attr) = p.attributes.get(*index) else { return PersonaEffect::None; }; - // A second press puts it back, so the key the holder used to show - // the value is also the one that hides it again. - p.revealed_attribute = match &p.revealed_attribute { - Some(id) if id == &attr.attribute_id => None, - _ => Some(attr.attribute_id.clone()), - }; - // No read: this lifts a mask over a value already in memory, which - // is exactly why the mask is not a security control. The read-path - // control — a listing that is never *sent* sensitive values — - // would belong here and does not exist; see - // `openvtc_core::persona::claim_types`. + // A second press puts it back, so the key the holder used to show the + // value is also the one that hides it again. + if p.revealed_attribute.as_deref() == Some(attr.attribute_id.as_str()) { + p.revealed_attribute = None; + return PersonaEffect::None; + } + // A sensitive value the bulk listing withheld is not in memory — fetch + // this one on its own (`pool::reveal`). The value is spliced in and the + // mask lifted when it returns (`AttributeRevealed`); nothing is + // revealed until then. This is the only case that becomes a read. + if attr.is_withheld_sensitive(&p.claim_types, p.show_values) { + return PersonaEffect::Job(PersonaJob::AttributeReveal { + attribute_id: attr.attribute_id.clone(), + claim_type: attr.claim_type.clone(), + }); + } + // Otherwise just lift the mask over a value already in memory — no + // read, which is exactly why the mask is not a security control. + p.revealed_attribute = Some(attr.attribute_id.clone()); PersonaEffect::None } @@ -869,7 +884,11 @@ impl PersonaReadJob { /// I/O only. pub(crate) async fn run(self) -> PersonaOutcome { - let attributes = pool::list(&self.admin_vta, self.include_values) + // The bulk listing never carries sensitive values (card numbers, ids): + // `include_sensitive: false`. A `sensitivity: high` attribute comes back + // valueless and the pane renders "sensitive — press s"; the per-attribute + // reveal (`s`) fetches that one value on its own via `pool::reveal`. + let attributes = pool::list(&self.admin_vta, self.include_values, false) .await .map_err(|e| format!("{e}")); let profiles = profile::list(&self.admin_vta) @@ -938,6 +957,15 @@ impl PersonaJobRun { .err() .map(|e| format!("{e}")), }, + PersonaJob::AttributeReveal { + attribute_id, + claim_type, + } => PersonaOutcome::AttributeRevealed { + attribute_id: attribute_id.clone(), + result: pool::reveal(&client, &claim_type, &attribute_id) + .await + .map_err(|e| format!("{e}")), + }, PersonaJob::ProfilePut { profile_id, name, @@ -1086,6 +1114,12 @@ pub(crate) enum PersonaOutcome { edit: bool, result: Result, }, + /// One attribute's value, fetched on its own for the `s` reveal. Spliced into + /// the in-memory row and the mask lifted, without a full re-read. + AttributeRevealed { + attribute_id: String, + result: Result, String>, + }, Bound { community: String, cleared: bool, @@ -1284,6 +1318,47 @@ impl PersonaOutcome { } }, + PersonaOutcome::AttributeRevealed { + attribute_id, + result, + } => match result { + // Splice the fetched value into the in-memory row (rebuilding the + // shared slice) and lift the mask. Only when a value actually came + // back — a fetch that returns nothing must not reveal a blank. + Ok(Some(fetched)) if fetched.value.is_some() => { + let updated: Vec = p + .attributes + .iter() + .map(|a| { + if a.attribute_id == attribute_id { + PoolAttribute { + value: fetched.value.clone(), + stale: fetched.stale, + stale_reason: fetched.stale_reason.clone(), + version: fetched.version, + ..a.clone() + } + } else { + a.clone() + } + }) + .collect(); + p.attributes = updated.into(); + p.revealed_attribute = Some(attribute_id); + } + Ok(_) => { + p.status_message = Some( + "That value can't be shown — it has no single stored value.".to_string(), + ); + } + Err(e) => { + p.status_message = Some(e.clone()); + state + .main_page + .log_error("Revealing the value failed", e.as_str()); + } + }, + PersonaOutcome::Bound { community, cleared, @@ -1543,6 +1618,39 @@ mod tests { assert!(personas(&state).revealed_attribute.is_none()); } + /// A reveal of a *sensitive* value the bulk listing withheld becomes a + /// targeted fetch — the value was never carried into memory — not an + /// in-memory mask-lift. Nothing is revealed until the fetch returns. + #[test] + fn a_reveal_of_a_withheld_sensitive_value_fetches_it() { + let mut attr = attribute("01C"); + attr.claim_type = "medical.condition".into(); + attr.value = None; + assert!( + attr.is_withheld_sensitive(&claim_types::Registry::vendored(), true), + "the fixture has to be a withheld sensitive attribute" + ); + + let mut state = state_with(IdentityState { + attributes: vec![attr].into(), + show_values: true, + ..IdentityState::default() + }); + + let effect = apply(&mut state, &PersonaAction::RevealValue(0)); + assert!( + matches!( + effect, + PersonaEffect::Job(PersonaJob::AttributeReveal { .. }) + ), + "a withheld sensitive value is fetched, not mask-lifted" + ); + assert!( + personas(&state).revealed_attribute.is_none(), + "nothing is revealed until the fetched value is spliced in" + ); + } + /// The editor opens on the value, never on the mask. /// /// A mask is a rendering and must never reach what is stored or sent — diff --git a/openvtc/src/ui/pages/main/components/identity_panel.rs b/openvtc/src/ui/pages/main/components/identity_panel.rs index fbc9f1d..b0c800b 100644 --- a/openvtc/src/ui/pages/main/components/identity_panel.rs +++ b/openvtc/src/ui/pages/main/components/identity_panel.rs @@ -524,8 +524,15 @@ fn render_attributes(state: &IdentityState, lines: &mut Vec>) { // that re-sorted under a stale grant cannot open a row nobody chose. let revealed = is_selected && state.revealed_attribute.as_deref() == Some(attr.attribute_id.as_str()); + // A sensitive value the bulk listing withheld: it is *not loaded*, which + // is a different answer from "(no value)". Say so, and offer `s` to fetch + // it — the listing never carried it into memory. + let withheld = + !revealed && attr.is_withheld_sensitive(&state.claim_types, state.show_values); let value = if revealed { attr.revealed_value(&state.claim_types, state.show_values) + } else if withheld { + "sensitive — not loaded".to_string() } else { attr.display_value(&state.claim_types, state.show_values) }; @@ -538,8 +545,10 @@ fn render_attributes(state: &IdentityState, lines: &mut Vec>) { }, )]; // Without this the row is a wrong answer rather than a reduced one: - // `••••••••` and "(no value)" are the same shape, and a holder reading - // the first as the second believes they hold nothing. + // `••••••••`, "sensitive — not loaded" and "(no value)" are the same + // shape, and a holder reading one as another believes they hold nothing. + // A masked value is in memory (`s` lifts the mask); a withheld one is not + // (`s` fetches it) — both offer `s`. if attr.is_masked(&state.claim_types) { value_spans.push(Span::styled( if revealed { @@ -553,6 +562,11 @@ fn render_attributes(state: &IdentityState, lines: &mut Vec>) { Style::new().fg(COLOR_SOFT_PURPLE) }, )); + } else if withheld { + value_spans.push(Span::styled( + " s to show", + Style::new().fg(COLOR_SOFT_PURPLE), + )); } lines.push(Line::from(value_spans));