diff --git a/openvtc-core/src/persona/profile.rs b/openvtc-core/src/persona/profile.rs index 0f606f2..d7b9970 100644 --- a/openvtc-core/src/persona/profile.rs +++ b/openvtc-core/src/persona/profile.rs @@ -143,17 +143,47 @@ impl ResolvedClaim { /// minus the "hidden" case: a resolve was asked for, so an absent value is /// an answer rather than a question that was never put. /// - /// There is no `revealed_value` counterpart here, and that is a decision - /// rather than an omission. A resolved claim has no identity of its own to - /// reveal *one* of — a face is read as a whole — so the only reveal this - /// type could offer is the blanket one the mask exists to avoid. A holder - /// who wants to check a value reads it among their attributes, one at a time. + /// See [`revealed_value`](Self::revealed_value) for lifting the mask on one + /// claim. #[must_use] pub fn display_value(&self) -> String { + self.value_line(false) + } + + /// The same line with the mask lifted, for a holder who asked for this one + /// claim. + /// + /// This used to be deliberately absent, on the argument that a resolved + /// claim has no identity of its own to reveal *one* of — a face was read as + /// a whole, so the only reveal the type could offer was the blanket one the + /// mask exists to avoid. That argument was about the **pane**, not the + /// type: it held only for as long as the face view had no cursor over its + /// claims. It has one now, so "the selected claim" is a thing a holder can + /// name, and the one-at-a-time reveal that the attributes tab has always + /// offered works here on the same terms. + /// + /// Still a separate method rather than a `reveal: bool` on + /// [`display_value`](Self::display_value), for the reason + /// [`PoolAttribute::revealed_value`](crate::persona::pool::PoolAttribute::revealed_value) + /// gives: reading a masked value in the clear should be something a call + /// site had to *name*. A boolean gets passed through, and the caller that + /// ends up passing `true` is rarely the one that meant to. + #[must_use] + pub fn revealed_value(&self) -> String { + self.value_line(true) + } + + fn value_line(&self, reveal: bool) -> String { if self.stale { return "stale — can no longer be proven".to_string(); } - let shown = |text: String| claim_types::resolve(&self.claim_type).render(&text); + let shown = |text: String| { + if reveal { + text + } else { + claim_types::resolve(&self.claim_type).render(&text) + } + }; match &self.value { Some(Value::String(s)) => shown(s.clone()), Some(other) => shown(other.to_string()), @@ -448,6 +478,39 @@ mod tests { assert!(claim.display_value().contains("can no longer be proven")); } + /// A masked claim reads back whole when a caller asks for that one claim. + /// + /// The pairing is the point, and it is the same one the pool makes: the + /// mask has to be liftable, or a holder cannot check what a community + /// actually sees; and lifting it has to be a different call, or it is not a + /// decision anyone made. + #[test] + fn a_masked_claim_is_only_whole_when_it_is_asked_for() { + let claim = ResolvedClaim::from_wire(&serde_json::json!({ + "type": "phone.mobile", + "value": "+61400123456", + "valueType": "string", + "provenance": { "kind": "selfAsserted" }, + })); + assert_eq!(claim.display_value(), "••••••••••56"); + assert_eq!(claim.revealed_value(), "+61400123456"); + } + + /// A stale claim says it is stale under a reveal too. + /// + /// The reason it cannot be shown is not that it is masked, and a reveal + /// that turned the explanation into a blank would hide the one thing the + /// holder needs to act on. + #[test] + fn a_stale_claim_still_says_it_is_stale_when_revealed() { + let claim = ResolvedClaim::from_wire(&serde_json::json!({ + "type": "phone.mobile", + "value": "+61400123456", + "stale": true, + })); + assert!(claim.revealed_value().contains("can no longer be proven")); + } + /// A face masks what its type says to mask, and says that it did. /// /// The face detail view is a screen a holder opens to check what a diff --git a/openvtc/src/state_handler/actions/mod.rs b/openvtc/src/state_handler/actions/mod.rs index 95ccd4d..7886198 100644 --- a/openvtc/src/state_handler/actions/mod.rs +++ b/openvtc/src/state_handler/actions/mod.rs @@ -225,6 +225,17 @@ pub enum PersonaAction { ProfileOpen(usize), /// Close that view. ProfileClose, + /// Move the cursor within the opened face's claims. + /// + /// Distinct from [`Select`](Self::Select) because the detail view is a mode + /// of the profiles tab with a cursor of its own: `Select` moves the face + /// list behind it, which still has to be where closing the detail lands. + FaceClaimSelect(usize), + /// Show the selected claim of the opened face unmasked, or stop showing it. + /// + /// The face-view counterpart to [`RevealValue`](Self::RevealValue), and one + /// claim rather than a mode for the same reason. + RevealFaceClaim(usize), ProfileNew, ProfileEdit(usize), ProfileDeleteArm(usize), diff --git a/openvtc/src/state_handler/main_page/content.rs b/openvtc/src/state_handler/main_page/content.rs index c67b2df..09407ec 100644 --- a/openvtc/src/state_handler/main_page/content.rs +++ b/openvtc/src/state_handler/main_page/content.rs @@ -764,6 +764,22 @@ pub struct IdentityState { pub profile_selected: usize, /// The profile opened with Enter, resolved to what it would present. pub open_profile: Option, + /// The claim under the cursor inside that opened face. + /// + /// Separate from [`profile_selected`](Self::profile_selected), which keeps + /// pointing at the face in the list behind the detail — closing the detail + /// has to land back on the face that was opened, so the two cursors cannot + /// share a field. + pub face_claim_selected: usize, + /// The one claim of the opened face being shown unmasked, by index. + /// + /// The same grant the attributes tab makes, on the same terms: one claim, + /// and only while it is also the selected one — the render checks both. + /// Indices rather than identifiers because a resolved claim has no id of + /// its own to key on (an inline value has no pool attribute behind it), and + /// the order is fixed for as long as a detail is open: every path that + /// replaces `open_profile` clears this too. + pub revealed_face_claim: Option, // ── Disclosures (from the agent) ───────────────────────────────────── /// What has actually left, newest first, across every context. diff --git a/openvtc/src/state_handler/persona_actions.rs b/openvtc/src/state_handler/persona_actions.rs index 21303ad..36bc5c6 100644 --- a/openvtc/src/state_handler/persona_actions.rs +++ b/openvtc/src/state_handler/persona_actions.rs @@ -118,8 +118,10 @@ pub(crate) fn apply(state: &mut State, action: &PersonaAction) -> PersonaEffect p.open_profile = None; p.status_message = None; // A reveal is granted to one row on one tab. Coming back to the - // attributes should find them masked again, not still open. + // attributes — or to a face — should find them masked again, not + // still open. p.revealed_attribute = None; + p.revealed_face_claim = None; // Read on arrival, once. The agent-served tabs are not polled: a // pane nobody has opened should not be asking the agent about the // holder's identity every few seconds. @@ -235,7 +237,38 @@ pub(crate) fn apply(state: &mut State, action: &PersonaAction) -> PersonaEffect }) } PersonaAction::ProfileClose => { - state.main_page.content_panel.identity.open_profile = None; + let p = &mut state.main_page.content_panel.identity; + p.open_profile = None; + p.revealed_face_claim = None; + PersonaEffect::None + } + PersonaAction::FaceClaimSelect(index) => { + let p = &mut state.main_page.content_panel.identity; + // Same rule as the attributes tab: the selection moved, so the + // reveal it was granted for is over. Carrying it down the list is + // how "one value" becomes "all of them", one press of ↓ at a time. + p.revealed_face_claim = None; + p.face_claim_selected = *index; + PersonaEffect::None + } + PersonaAction::RevealFaceClaim(index) => { + let p = &mut state.main_page.content_panel.identity; + // Bounds-checked against the open face rather than assumed: the + // detail can be replaced by a re-read between the keypress and here. + let claims = p.open_profile.as_ref().map_or(0, |d| d.resolved.len()); + if *index >= claims { + return PersonaEffect::None; + } + // A second press puts it back, so the key that showed the value is + // also the one that hides it again. + p.revealed_face_claim = match p.revealed_face_claim { + Some(i) if i == *index => None, + _ => Some(*index), + }; + // No read: a face detail is resolved in full when it is opened, so + // this lifts a mask over a value already in memory. Which is + // exactly why the mask is not a security control — see + // `openvtc_core::persona::claim_types`. PersonaEffect::None } PersonaAction::ProfileNew => { @@ -855,6 +888,12 @@ impl PersonaOutcome { } } else { p.open_profile = Some(detail); + // A fresh detail is a fresh set of rows: the cursor + // starts at the top and nothing is revealed. Carrying + // an index over would grant a reveal on whatever + // happens to sit at that position now. + p.face_claim_selected = 0; + p.revealed_face_claim = None; } } Err(e) => { @@ -1164,6 +1203,136 @@ mod tests { assert!(personas(&state).revealed_attribute.is_none()); } + /// A face detail with three claims, two of which carry a mask style. + fn open_face() -> openvtc_core::persona::profile::ProfileDetail { + use openvtc_core::persona::profile::{ProfileDetail, ProfileSummary, ResolvedClaim}; + ProfileDetail { + summary: ProfileSummary { + profile_id: "01P".into(), + name: "OSS Developer".into(), + ..ProfileSummary::default() + }, + resolved: vec![ + ResolvedClaim { + claim_type: "name.legal".into(), + value: Some(serde_json::json!("Glenn Gore")), + ..ResolvedClaim::default() + }, + ResolvedClaim { + claim_type: "email.work".into(), + value: Some(serde_json::json!("glenn@example.com")), + ..ResolvedClaim::default() + }, + ], + ..ProfileDetail::default() + } + } + + /// `s` on a face opens one claim, and pressing it again closes it — the key + /// that showed the value is the one that hides it. + #[test] + fn a_face_reveal_toggles_on_the_same_key() { + let mut state = state_with(IdentityState { + tab: PersonaTab::Profiles, + open_profile: Some(open_face()), + face_claim_selected: 1, + ..IdentityState::default() + }); + + apply(&mut state, &PersonaAction::RevealFaceClaim(1)); + assert_eq!(personas(&state).revealed_face_claim, Some(1)); + + apply(&mut state, &PersonaAction::RevealFaceClaim(1)); + assert!( + personas(&state).revealed_face_claim.is_none(), + "toggled off" + ); + } + + /// A reveal aimed past the end of the face opens nothing. + /// + /// The index comes from a keypress against what was on screen, and the + /// detail can be replaced between the two — so it is checked against the + /// open face rather than trusted. + #[test] + fn a_face_reveal_past_the_end_reveals_nothing() { + let mut state = state_with(IdentityState { + tab: PersonaTab::Profiles, + open_profile: Some(open_face()), + ..IdentityState::default() + }); + + apply(&mut state, &PersonaAction::RevealFaceClaim(7)); + assert!(personas(&state).revealed_face_claim.is_none()); + } + + /// A reveal with no face open at all opens nothing, rather than arming a + /// grant that the next face to be opened would inherit. + #[test] + fn a_face_reveal_with_nothing_open_reveals_nothing() { + let mut state = state_with(IdentityState { + tab: PersonaTab::Profiles, + ..IdentityState::default() + }); + + apply(&mut state, &PersonaAction::RevealFaceClaim(0)); + assert!(personas(&state).revealed_face_claim.is_none()); + } + + /// Everything that changes what is on screen puts a face's mask back too. + /// + /// Same rule as the attributes tab, and the same reason: a reveal is + /// granted to one claim on one open face, and moving the cursor, closing + /// the face or leaving the tab each ends it. + #[test] + fn moving_anywhere_puts_a_face_mask_back() { + let revealed = || { + state_with(IdentityState { + tab: PersonaTab::Profiles, + open_profile: Some(open_face()), + face_claim_selected: 1, + revealed_face_claim: Some(1), + loaded: true, + ..IdentityState::default() + }) + }; + + let mut moved = revealed(); + apply(&mut moved, &PersonaAction::FaceClaimSelect(0)); + assert!(personas(&moved).revealed_face_claim.is_none(), "cursor"); + assert_eq!(personas(&moved).face_claim_selected, 0); + + let mut closed = revealed(); + apply(&mut closed, &PersonaAction::ProfileClose); + assert!(personas(&closed).revealed_face_claim.is_none(), "closed"); + + let mut tabbed = revealed(); + apply(&mut tabbed, &PersonaAction::TabNext); + assert!(personas(&tabbed).revealed_face_claim.is_none(), "tab"); + } + + /// Opening a face starts at the top with nothing revealed. + /// + /// A carried-over index would grant a reveal on whatever now sits at that + /// position, which is a different claim in a different face. + #[test] + fn opening_a_face_starts_closed_and_at_the_top() { + let mut state = state_with(IdentityState { + tab: PersonaTab::Profiles, + face_claim_selected: 1, + revealed_face_claim: Some(1), + ..IdentityState::default() + }); + + PersonaOutcome::ProfileRead { + edit: false, + result: Ok(open_face()), + } + .apply(&mut state); + assert_eq!(personas(&state).face_claim_selected, 0); + assert!(personas(&state).revealed_face_claim.is_none()); + } + /// Everything that changes what is on screen puts the mask back. /// /// This is what keeps the reveal from becoming a global unmask reached one diff --git a/openvtc/src/ui/pages/main/components/identity_panel.rs b/openvtc/src/ui/pages/main/components/identity_panel.rs index f73fe44..7c1172e 100644 --- a/openvtc/src/ui/pages/main/components/identity_panel.rs +++ b/openvtc/src/ui/pages/main/components/identity_panel.rs @@ -475,17 +475,47 @@ fn render_profiles(state: &IdentityState, lines: &mut Vec>) { .fg(COLOR_TEXT_DEFAULT), ); lines.push(Line::from("")); - for claim in &detail.resolved { - lines.push(Line::from(vec![ + for (i, claim) in detail.resolved.iter().enumerate() { + let is_selected = i == state.face_claim_selected; + // A reveal is granted to one claim, and only while it is the + // selected one — the same pairing the attributes tab makes, so + // a stale grant cannot open a row nobody chose. + let revealed = is_selected && state.revealed_face_claim == Some(i); + let value = if revealed { + claim.revealed_value() + } else { + claim.display_value() + }; + let mut spans = vec![ Span::styled( - format!(" {:<22}", truncate(&claim.claim_type, 21)), - Style::new().fg(COLOR_SOFT_PURPLE), + if is_selected { " ▸ " } else { " " }, + if is_selected { + Style::new().fg(COLOR_SUCCESS).bold() + } else { + Style::new().fg(COLOR_TEXT_DEFAULT) + }, ), Span::styled( - truncate(&claim.display_value(), 44), - Style::new().fg(COLOR_TEXT_DEFAULT), + format!("{:<22}", truncate(&claim.claim_type, 21)), + Style::new().fg(COLOR_SOFT_PURPLE), ), - ])); + Span::styled(truncate(&value, 44), Style::new().fg(COLOR_TEXT_DEFAULT)), + ]; + // 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 the face + // shows nothing. + if claim.is_masked() && !revealed { + spans.push(Span::styled( + if is_selected { + " masked — s to show" + } else { + " masked" + }, + Style::new().fg(COLOR_DARK_GRAY), + )); + } + lines.push(Line::from(spans)); // A value that lives only in this face is not among the // holder's attributes, so correcting it there will not correct it // here. Saying so on the row is the only place they find out. @@ -500,20 +530,28 @@ fn render_profiles(state: &IdentityState, lines: &mut Vec>) { } } lines.push(Line::from("")); - // No per-claim reveal here: a face has no cursor over its claims, so - // the only reveal this view could offer is the blanket one the mask - // exists to avoid. The one-at-a-time reveal lives with the attributes. + // Said once, above the keys, rather than on every row — and only when + // something on screen is actually masked, because explaining a + // mechanism the holder is not looking at is noise. if detail.resolved.iter().any(ResolvedClaim::is_masked) { lines.push( Line::from( - " Some values are masked by what they are. Read one of them among your \ - attributes, where they open one at a time.", + " Some values are masked by what they are — `s` shows the selected one. The \ + mask is against someone reading over your shoulder; this face already \ + shows the value to whoever wears it.", ) .fg(COLOR_DARK_GRAY), ); lines.push(Line::from("")); } - lines.push(Line::from(" ⏎/Esc: back e: edit").fg(COLOR_DARK_GRAY)); + lines.push( + Line::from(if detail.resolved.is_empty() { + " ⏎/Esc: back e: edit".to_string() + } else { + " ↑/↓ select s: show one ⏎/Esc: back e: edit".to_string() + }) + .fg(COLOR_DARK_GRAY), + ); return; } @@ -1731,6 +1769,127 @@ mod tests { ); } + /// How many *rows* are marked masked, as distinct from the paragraph above + /// them that explains the mask and contains the same word. + fn masked_rows(out: &str) -> usize { + out.lines() + .filter(|l| l.contains("masked") && !l.contains("Some values are masked")) + .count() + } + + /// A face builds a detail with two masked claims, for the reveal tests below. + fn face_with_two_masked_claims() -> IdentityState { + use openvtc_core::persona::profile::{ProfileDetail, ProfileSummary, ResolvedClaim}; + let mut state = IdentityState { + tab: PersonaTab::Profiles, + open_profile: Some(ProfileDetail { + summary: ProfileSummary { + profile_id: "01P".into(), + name: "OSS Developer".into(), + ..ProfileSummary::default() + }, + resolved: vec![ + ResolvedClaim { + claim_type: "name.legal".into(), + value: Some(serde_json::json!("Glenn Gore")), + ..ResolvedClaim::default() + }, + ResolvedClaim { + claim_type: "email.work".into(), + value: Some(serde_json::json!("glenn@example.com")), + ..ResolvedClaim::default() + }, + ResolvedClaim { + claim_type: "phone.mobile".into(), + value: Some(serde_json::json!("+6591234567")), + ..ResolvedClaim::default() + }, + ], + ..ProfileDetail::default() + }), + ..IdentityState::default() + }; + loaded(&mut state); + state + } + + /// A face's masked claims say they are masked, and say which key opens the + /// selected one — rather than sending the holder to another tab. + /// + /// This is the half the view used to be missing. A face was read as a whole + /// with no cursor, so it could offer no per-claim reveal and told the reader + /// to go and find the value among their attributes instead. That is a real + /// answer to a question nobody asked: the holder is looking at *this* face + /// because they want to know what *this* face shows. + #[test] + fn a_face_says_which_key_opens_the_selected_masked_claim() { + let state = face_with_two_masked_claims(); + let out = text(&render(&state)); + + assert!(out.contains("s: show one"), "the key is offered: {out}"); + assert!( + !out.contains("among your attributes"), + "no longer sends the reader to another tab: {out}" + ); + // Two of the three claim types carry a mask style; `name.legal` does + // not. The selected row here is index 0 (`name.legal`), so neither + // masked row carries the "s to show" tail — the key hint follows the + // cursor rather than sitting on every masked row. + assert_eq!( + masked_rows(&out), + 2, + "exactly the two masked claims are marked: {out}" + ); + assert!( + !out.contains("masked — s to show"), + "the key hint follows the cursor, which is on an unmasked row: {out}" + ); + assert!( + out.contains("Glenn Gore"), + "an unmasked type is still shown whole: {out}" + ); + } + + /// The reveal opens one claim — the selected one — and nothing else on the + /// face opens with it. + #[test] + fn a_face_reveal_opens_only_the_selected_claim() { + let mut state = face_with_two_masked_claims(); + state.face_claim_selected = 1; + state.revealed_face_claim = Some(1); + + let out = text(&render(&state)); + assert!(out.contains("glenn@example.com"), "the selected one: {out}"); + assert!( + !out.contains("+6591234567"), + "the other masked claim stays masked: {out}" + ); + assert_eq!( + masked_rows(&out), + 1, + "the revealed row drops its marker: {out}" + ); + } + + /// A grant that no longer names the selected row opens nothing. + /// + /// Belt and braces over the handler, which clears the grant when the + /// selection moves: the render checks the pairing itself, so a grant that + /// somehow outlived its row cannot unmask whatever moved under it. + #[test] + fn a_face_reveal_that_is_not_the_selected_row_opens_nothing() { + let mut state = face_with_two_masked_claims(); + state.face_claim_selected = 2; + state.revealed_face_claim = Some(1); + + let out = text(&render(&state)); + assert!( + !out.contains("glenn@example.com"), + "a stale grant does not open a row nobody chose: {out}" + ); + assert!(!out.contains("+6591234567"), "{out}"); + } + /// An attribute whose claim type carries a mask style is masked in the holder's /// own list, and the row says so rather than reading as empty. /// @@ -1852,8 +2011,12 @@ mod tests { assert!(!out.contains("masked"), "{out}"); } - /// A face masks what it shows too, and points at where a value can be read - /// one at a time — the detail view has no cursor of its own to reveal from. + /// A face masks what it shows too, and a claim left unselected stays masked. + /// + /// This used to assert that the view sent the reader "among your + /// attributes" to read a value, because the detail had no cursor of its own + /// to reveal from. It has one now, so the pointer to another tab is gone + /// and what is left to check is the masking itself. #[test] fn a_face_masks_its_values_and_says_where_to_read_one() { use openvtc_core::persona::profile::{ProfileDetail, ProfileSummary, ResolvedClaim}; @@ -1880,7 +2043,12 @@ mod tests { let out = text(&render(&state)); assert!(out.contains("••••••••••••4242"), "{out}"); assert!(!out.contains("4242424242424242"), "{out}"); - assert!(out.contains("among your"), "{out}"); + assert!( + !out.contains("among your"), + "no longer sends the reader to another tab: {out}" + ); + // The one claim is also the selected one, so the row names the key. + assert!(out.contains("masked — s to show"), "{out}"); } /// The one masked attribute the tests above share. diff --git a/openvtc/src/ui/pages/main/mod.rs b/openvtc/src/ui/pages/main/mod.rs index 2d1b6ca..4ea8150 100644 --- a/openvtc/src/ui/pages/main/mod.rs +++ b/openvtc/src/ui/pages/main/mod.rs @@ -641,9 +641,25 @@ impl MainPage { let selected = personas.profile_selected; // The opened detail view is a mode of this tab, not of the // pane, so its keys live here. - if personas.open_profile.is_some() { + if let Some(detail) = personas.open_profile.as_ref() { + let claims = detail.resolved.len(); + let claim = personas.face_claim_selected; return match key.code { KeyCode::Enter => send(PA::ProfileClose), + KeyCode::Up if claims > 0 => { + send(PA::FaceClaimSelect(claim.saturating_sub(1))) + } + KeyCode::Down if claims > 0 => { + send(PA::FaceClaimSelect((claim + 1).min(claims - 1))) + } + // The same `s` the attributes tab uses, on the same + // terms: one claim, the selected one, and pressing it + // again puts the mask back. There is no `v` here + // because there is nothing for it to ask — a face + // detail is resolved in full when it is opened, so + // every value is already in hand and the mask is the + // only thing between it and the screen. + KeyCode::Char('s') if claim < claims => send(PA::RevealFaceClaim(claim)), KeyCode::Char('e') => send(PA::ProfileEdit(selected)), _ => false, };