From f6e74cbb1d2ffff1dfa9bce485cfcdb98316ee58 Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Mon, 10 Aug 2026 16:27:47 +0200 Subject: [PATCH] Implement structured desktop entry viewer --- apps/desktop/src/editor.rs | 65 ++++++- apps/desktop/src/main.rs | 343 ++++++++++++++++++++++++++++++++- crates/storage/src/document.rs | 31 +++ 3 files changed, 428 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/editor.rs b/apps/desktop/src/editor.rs index 30a06e5..b2fba3f 100644 --- a/apps/desktop/src/editor.rs +++ b/apps/desktop/src/editor.rs @@ -12,14 +12,25 @@ use ironstorage::{ pub struct EntryEditor { document: EntryDocument, revealed: BTreeSet, + focused: Option, dirty: bool, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum FieldNavigation { + First, + Last, + Next, + Previous, +} + impl EntryEditor { pub fn new(document: EntryDocument) -> Self { + let focused = document.fields().first().map(EntryField::id); Self { document, revealed: BTreeSet::new(), + focused, dirty: false, } } @@ -44,6 +55,44 @@ impl EntryEditor { self.revealed.contains(&id) } + pub fn focused(&self) -> Option { + self.focused + } + + pub fn focused_ratio(&self) -> f32 { + let fields = self.document.fields(); + self.focused + .and_then(|id| fields.iter().position(|field| field.id() == id)) + .map_or(0.0, |index| { + index as f32 / fields.len().saturating_sub(1).max(1) as f32 + }) + } + + pub fn select(&mut self, id: EntryFieldId) { + if self.document.field(id).is_some() { + self.focused = Some(id); + } + } + + pub fn navigate(&mut self, navigation: FieldNavigation) { + let fields = self.document.fields(); + if fields.is_empty() { + self.focused = None; + return; + } + let current = self + .focused + .and_then(|id| fields.iter().position(|field| field.id() == id)) + .unwrap_or(0); + let index = match navigation { + FieldNavigation::First => 0, + FieldNavigation::Last => fields.len() - 1, + FieldNavigation::Next => (current + 1).min(fields.len() - 1), + FieldNavigation::Previous => current.saturating_sub(1), + }; + self.focused = Some(fields[index].id()); + } + pub fn toggle_reveal(&mut self, id: EntryFieldId) -> Result<(), DocumentError> { let field = self .document @@ -81,14 +130,27 @@ impl EntryEditor { .position(|field| field.id() == id) }) .map_or(self.document.fields().len(), |index| index + 1); - self.document.add(index, EntryFieldDraft::blank())?; + self.focused = Some(self.document.add(index, EntryFieldDraft::blank())?); self.dirty = true; Ok(()) } pub fn remove(&mut self, id: EntryFieldId) -> Result<(), DocumentError> { + let index = self + .document + .fields() + .iter() + .position(|field| field.id() == id) + .ok_or(DocumentError::UnknownField { id })?; self.document.remove(id)?; self.revealed.remove(&id); + if self.focused == Some(id) { + self.focused = self + .document + .fields() + .get(index.min(self.document.fields().len().saturating_sub(1))) + .map(EntryField::id); + } self.dirty = true; Ok(()) } @@ -148,6 +210,7 @@ impl fmt::Debug for EntryEditor { .debug_struct("EntryEditor") .field("document", &self.document) .field("revealed", &self.revealed) + .field("focused", &self.focused) .field("dirty", &self.dirty) .finish() } diff --git a/apps/desktop/src/main.rs b/apps/desktop/src/main.rs index 594ed45..4a4748a 100644 --- a/apps/desktop/src/main.rs +++ b/apps/desktop/src/main.rs @@ -13,7 +13,7 @@ use std::{ time::{Duration, Instant}, }; -use editor::EntryEditor; +use editor::{EntryEditor, FieldNavigation}; use iced::{ Element, Event, Length, Size, Subscription, Task, event, keyboard, mouse, time, touch, widget::{button, column, container, pane_grid, row, scrollable, text, text_input}, @@ -26,7 +26,10 @@ use ironstorage::{ }, crypto::KeyInfo, desktop::{DesktopError, DesktopErrorKind, DesktopStorage}, - document::{DocumentError, EntryDocument, EntryFieldId, EntryFieldKind, EntrySensitivity}, + document::{ + DocumentError, EntryDocument, EntryField, EntryFieldDiagnostic, EntryFieldId, + EntryFieldKind, EntrySensitivity, + }, generate::GeneratorConfig, presentation::{ClipboardWait, NativeClipboardManager}, read::{TreeModel, TreeNodeId}, @@ -69,6 +72,8 @@ enum Message { Remove(EntryFieldId), MoveUp(EntryFieldId), MoveDown(EntryFieldId), + BeginEdit, + SelectField(EntryFieldId), ToggleReveal(EntryFieldId), RequestGenerate(EntryFieldId), ConfirmGenerate, @@ -151,6 +156,7 @@ struct App { after_authentication: Option, entry_path: String, editor: Option, + content_mode: ContentMode, saving: bool, confirmation: Option, after_save: Option, @@ -171,6 +177,12 @@ enum PaneFocus { Content, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ContentMode { + Viewer, + Editor, +} + #[derive(Clone, Debug, Eq, PartialEq)] enum TreeState { Loading, @@ -237,6 +249,7 @@ impl App { after_authentication: None, entry_path: String::new(), editor: None, + content_mode: ContentMode::Viewer, saving: false, confirmation: None, after_save: None, @@ -303,9 +316,15 @@ impl App { } Message::SidebarNavigate(key) => { self.touch_user_activity(); - if self.pane_focus == PaneFocus::Sidebar { - let intent = self.navigation.navigate(key); - return self.handle_navigation(intent); + match self.pane_focus { + PaneFocus::Sidebar => { + let intent = self.navigation.navigate(key); + return self.handle_navigation(intent); + } + PaneFocus::Content if self.content_mode == ContentMode::Viewer => { + return self.navigate_viewer(key); + } + PaneFocus::Content => {} } } Message::TogglePaneFocus => { @@ -349,8 +368,9 @@ impl App { let _selected = self.navigation.select_entry_path(&entry); self.pane_focus = PaneFocus::Content; self.editor = Some(EntryEditor::new(document)); + self.content_mode = ContentMode::Viewer; self.conflict = false; - self.status = format!("Editing {entry}"); + self.status = format!("Viewing {entry}"); } Err(error) => self.status = format!("Open failed: {error}"), } @@ -390,7 +410,36 @@ impl App { Message::Remove(id) => self.edit(|editor| editor.remove(id)), Message::MoveUp(id) => self.edit(|editor| editor.move_up(id)), Message::MoveDown(id) => self.edit(|editor| editor.move_down(id)), - Message::ToggleReveal(id) => self.edit(|editor| editor.toggle_reveal(id)), + Message::BeginEdit => { + if authentication_allows_content(&self.authentication) + && let Some(editor) = &self.editor + { + self.content_mode = ContentMode::Editor; + self.status = format!("Editing {}", editor.entry()); + } + } + Message::SelectField(id) => { + if let Some(editor) = self.editor.as_mut() { + editor.select(id); + } + } + Message::ToggleReveal(id) => { + if authentication_allows_content(&self.authentication) + && let Some(editor) = self.editor.as_mut() + { + match editor.toggle_reveal(id) { + Ok(()) => { + editor.select(id); + self.status = if editor.is_revealed(id) { + "Sensitive value revealed by explicit action.".to_owned() + } else { + "Sensitive value hidden.".to_owned() + }; + } + Err(error) => self.status = error.to_string(), + } + } + } Message::RequestGenerate(id) => { let has_value = self .editor @@ -410,6 +459,9 @@ impl App { } Message::CancelGenerate => self.generate_confirmation = None, Message::Copy(id) => { + if !authentication_allows_content(&self.authentication) { + return Task::none(); + } let (Some(storage), Some(editor)) = (&self.storage, &self.editor) else { return Task::none(); }; @@ -476,6 +528,7 @@ impl App { Message::ConfirmDiscard => { let action = self.confirmation.take(); self.editor = None; + self.content_mode = ContentMode::Viewer; self.conflict = false; if let Some(action) = action { return self.execute_action(action); @@ -485,6 +538,7 @@ impl App { Message::ReloadConflict => { if let Some(entry) = self.editor.as_ref().map(EntryEditor::entry) { self.editor = None; + self.content_mode = ContentMode::Viewer; self.conflict = false; return self.execute_action(PendingAction::Reload(entry)); } @@ -577,6 +631,7 @@ impl App { match action { PendingAction::OpenEntry(entry) | PendingAction::Reload(entry) => { self.editor = None; + self.content_mode = ContentMode::Viewer; self.begin_open(entry) } PendingAction::CloseWindow(id) => { @@ -631,6 +686,11 @@ impl App { fn begin_save(&mut self) -> Task { self.touch_user_activity(); + if self.content_mode != ContentMode::Editor + || !authentication_allows_content(&self.authentication) + { + return Task::none(); + } let (Some(storage), Some(handle)) = (self.storage.clone(), self.handle.clone()) else { return Task::none(); }; @@ -659,6 +719,11 @@ impl App { } fn edit(&mut self, operation: impl FnOnce(&mut EntryEditor) -> Result<(), DocumentError>) { + if self.content_mode != ContentMode::Editor + || !authentication_allows_content(&self.authentication) + { + return; + } let Some(editor) = self.editor.as_mut() else { return; }; @@ -677,6 +742,43 @@ impl App { } } + fn navigate_viewer(&mut self, navigation: NavigationKey) -> Task { + if !authentication_allows_content(&self.authentication) { + return Task::none(); + } + let Some(editor) = self.editor.as_mut() else { + return Task::none(); + }; + match navigation { + NavigationKey::Previous => editor.navigate(FieldNavigation::Previous), + NavigationKey::Next => editor.navigate(FieldNavigation::Next), + NavigationKey::First => editor.navigate(FieldNavigation::First), + NavigationKey::Last => editor.navigate(FieldNavigation::Last), + NavigationKey::Activate => { + if let Some(id) = editor.focused() + && editor.document().field(id).is_some_and(|field| { + field.metadata().sensitivity() == EntrySensitivity::Sensitive + }) + { + let _ignored = editor.toggle_reveal(id); + self.status = if editor.is_revealed(id) { + "Sensitive value revealed by explicit keyboard action.".to_owned() + } else { + "Sensitive value hidden.".to_owned() + }; + } + } + NavigationKey::Collapse | NavigationKey::Expand => {} + } + iced::widget::operation::snap_to( + viewer_scroll_id(), + scrollable::RelativeOffset { + x: 0.0, + y: editor.focused_ratio(), + }, + ) + } + fn touch_user_activity(&mut self) { if let Some(handle) = &self.handle { match handle @@ -696,6 +798,7 @@ impl App { self.handle = None; self.sensitive.clear(); self.editor = None; + self.content_mode = ContentMode::Viewer; self.saving = false; self.confirmation = None; self.after_save = None; @@ -768,6 +871,10 @@ fn sidebar_scroll_id() -> iced::widget::Id { iced::widget::Id::new("desktop-navigation-tree") } +fn viewer_scroll_id() -> iced::widget::Id { + iced::widget::Id::new("desktop-entry-viewer") +} + fn sidebar_view<'a>( navigation: &'a NavigationTree, state: &'a TreeState, @@ -859,18 +966,27 @@ fn content_view(app: &App) -> Element<'_, Message> { text_input("Entry path", &app.entry_path) .on_input(Message::EntryPathChanged) .on_submit(Message::OpenEntry), - button("Edit").on_press(Message::OpenEntry), + button("Open").on_press(Message::OpenEntry), button("Reload").on_press(Message::RequestReload), button("Lock").on_press(Message::Lock), ] .spacing(8); - let body: Element<'_, Message> = if app.saving { + let body: Element<'_, Message> = if !authentication_allows_content(&app.authentication) { + container(text( + "Protected entry content is locked. Select an entry to authenticate and open it.", + )) + .center(Length::Fill) + .into() + } else if app.saving { container(text("Saving without discarding the draft…")) .center(Length::Fill) .into() } else if let Some(editor) = &app.editor { - editor_view(editor, app.conflict) + match app.content_mode { + ContentMode::Viewer => viewer_view(editor), + ContentMode::Editor => editor_view(editor, app.conflict), + } } else { container(text( "Select an entry in the navigation tree. Entry names remain visible while locked; protected content authenticates only when opened.", @@ -885,6 +1001,130 @@ fn content_view(app: &App) -> Element<'_, Message> { .into() } +fn authentication_allows_content(authentication: &AuthenticationView) -> bool { + matches!(authentication, AuthenticationView::Unlocked(_)) +} + +#[derive(Debug, Eq, PartialEq)] +enum ViewerValue<'a> { + Masked, + Text(&'a str), + Unavailable, +} + +fn viewer_value(field: &EntryField, revealed: bool) -> ViewerValue<'_> { + if field.metadata().sensitivity() == EntrySensitivity::Sensitive && !revealed { + ViewerValue::Masked + } else { + std::str::from_utf8(field.value()) + .map(ViewerValue::Text) + .unwrap_or(ViewerValue::Unavailable) + } +} + +fn viewer_label(field: &EntryField, index: usize) -> String { + let label = field.metadata().name().map_or_else( + || match field.metadata().kind() { + EntryFieldKind::Password => "Password".to_owned(), + EntryFieldKind::Username => "Username".to_owned(), + EntryFieldKind::Email => "Email".to_owned(), + EntryFieldKind::Url => "URL".to_owned(), + EntryFieldKind::OtpUri => "One-time password".to_owned(), + EntryFieldKind::Field => "Field".to_owned(), + EntryFieldKind::Note => "Note".to_owned(), + EntryFieldKind::Blank => "Blank line".to_owned(), + }, + str::to_owned, + ); + format!("{label} · line {}", index + 1) +} + +fn viewer_diagnostic(diagnostic: EntryFieldDiagnostic) -> &'static str { + match diagnostic { + EntryFieldDiagnostic::MalformedOtpUri => { + "Malformed OTP URI preserved losslessly; OTP metadata is unavailable." + } + EntryFieldDiagnostic::NonUtf8Value => { + "Non-UTF-8 value preserved losslessly; text display is unavailable." + } + } +} + +fn viewer_view(editor: &EntryEditor) -> Element<'_, Message> { + let mut rows = column![ + row![ + text(editor.entry()).size(22), + button("Edit entry").on_press(Message::BeginEdit), + ] + .spacing(8), + text("Use Up/Down/Home/End to select fields; Enter reveals or hides a selected sensitive value.") + .size(12), + ] + .spacing(12); + + for (index, field) in editor.fields().iter().enumerate() { + let id = field.id(); + let label = viewer_label(field, index); + let selected = editor.focused() == Some(id); + let revealed = editor.is_revealed(id); + let value = match viewer_value(field, revealed) { + ViewerValue::Masked => "••••••••", + ViewerValue::Text("") => "(empty)", + ViewerValue::Text(value) => value, + ViewerValue::Unavailable => "(binary value)", + }; + let mut actions = + row![button(text(format!("Copy {label}"))).on_press(Message::Copy(id)),].spacing(6); + if field.metadata().sensitivity() == EntrySensitivity::Sensitive { + actions = actions.push( + button(text(format!( + "{} {label}", + if revealed { "Hide" } else { "Reveal" } + ))) + .on_press(Message::ToggleReveal(id)), + ); + } + let mut field_view = column![ + button(text(format!( + "{} {label}", + if selected { "●" } else { "○" } + ))) + .on_press(Message::SelectField(id)) + .style(if selected { + button::primary + } else { + button::text + }), + text(value), + actions, + ] + .spacing(5); + if let Some(otp) = field.metadata().otp() { + let cadence = otp.period().map_or_else( + || format!("counter {}", otp.counter().unwrap_or_default()), + |period| format!("{period}s period"), + ); + field_view = field_view.push(text(format!( + "{:?} · {} · {} · {:?} · {} digits · {cadence}", + otp.kind(), + otp.issuer().unwrap_or("unknown issuer"), + otp.account(), + otp.algorithm(), + otp.digits(), + ))); + } + if let Some(diagnostic) = field.metadata().diagnostic() { + field_view = field_view.push(text(viewer_diagnostic(diagnostic)).size(12)); + } + rows = rows.push(container(field_view).padding(10).width(Length::Fill)); + } + + scrollable(rows) + .id(viewer_scroll_id()) + .height(Length::Fill) + .into() +} + fn dirty_decision(editor: Option<&EntryEditor>) -> DirtyDecision { if editor.is_some_and(EntryEditor::is_dirty) { DirtyDecision::Confirm @@ -1327,6 +1567,7 @@ mod tests { after_authentication: None, entry_path: String::new(), editor, + content_mode: ContentMode::Viewer, saving: false, confirmation: None, after_save: None, @@ -1399,6 +1640,88 @@ mod tests { ); } + #[test] + fn viewer_covers_lossless_fields_navigation_and_explicit_sensitive_actions() { + let (_temporary, storage) = fixture_storage(); + let mut editor = empty_editor(&storage, "documents/viewer"); + for value in [ + b"password".as_slice(), + b"username: alice", + b"custom: first", + b"custom: second", + b"first note", + b"second note", + b"otpauth://totp/Example:alice?secret=JBSWY3DPEHPK3PXP&issuer=Example", + b"otpauth://broken", + b"binary: \xff", + ] { + let after = editor.fields().last().map(EntryField::id); + editor.add_after(after).expect("add viewer field"); + let id = editor.fields().last().expect("new viewer field").id(); + editor.update_raw(id, value).expect("populate viewer field"); + } + + let fields = editor.fields(); + assert_eq!(fields[0].metadata().kind(), EntryFieldKind::Password); + assert_eq!(fields[1].metadata().kind(), EntryFieldKind::Username); + assert_eq!(fields[2].metadata().name(), Some("custom")); + assert_eq!(fields[3].metadata().name(), Some("custom")); + assert_ne!(viewer_label(&fields[2], 2), viewer_label(&fields[3], 3)); + assert_eq!(fields[4].metadata().kind(), EntryFieldKind::Note); + assert_eq!(fields[5].metadata().kind(), EntryFieldKind::Note); + assert!(fields[6].metadata().otp().is_some()); + assert_eq!( + fields[7].metadata().diagnostic(), + Some(EntryFieldDiagnostic::MalformedOtpUri) + ); + assert_eq!( + fields[8].metadata().diagnostic(), + Some(EntryFieldDiagnostic::NonUtf8Value) + ); + + let password = fields[0].id(); + assert_eq!(viewer_value(&fields[0], false), ViewerValue::Masked); + editor.toggle_reveal(password).expect("reveal password"); + assert_eq!( + viewer_value(&editor.fields()[0], editor.is_revealed(password)), + ViewerValue::Text("password") + ); + editor.navigate(FieldNavigation::First); + assert_eq!(editor.focused(), Some(password)); + editor.navigate(FieldNavigation::Next); + assert_eq!(editor.focused(), Some(editor.fields()[1].id())); + editor.navigate(FieldNavigation::Last); + assert_eq!(editor.focused(), Some(editor.fields()[8].id())); + + assert!(!authentication_allows_content(&AuthenticationView::Locked)); + assert!(authentication_allows_content( + &AuthenticationView::Unlocked(Duration::from_secs(1)) + )); + editor.toggle_reveal(password).expect("hide password"); + let mut app = test_app(Some(editor)); + let _task = app.update(Message::ToggleReveal(password)); + assert!( + !app.editor + .as_ref() + .expect("locked document fixture") + .is_revealed(password) + ); + let _task = app.update(Message::BeginEdit); + assert_eq!(app.content_mode, ContentMode::Viewer); + app.authentication = AuthenticationView::Unlocked(Duration::from_secs(1)); + let _task = app.update(Message::ToggleReveal(password)); + assert!( + app.editor + .as_ref() + .expect("unlocked document fixture") + .is_revealed(password) + ); + let _task = app.update(Message::BeginEdit); + assert_eq!(app.content_mode, ContentMode::Editor); + app.authentication_lost("locked".to_owned()); + assert!(app.editor.is_none()); + } + #[test] fn every_destructive_path_uses_the_same_save_discard_cancel_guard() { let (_temporary, storage) = fixture_storage(); diff --git a/crates/storage/src/document.rs b/crates/storage/src/document.rs index b5a05d8..3dc22fd 100644 --- a/crates/storage/src/document.rs +++ b/crates/storage/src/document.rs @@ -41,12 +41,19 @@ pub enum EntrySensitivity { Empty, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum EntryFieldDiagnostic { + MalformedOtpUri, + NonUtf8Value, +} + #[derive(Clone, Debug, Eq, PartialEq)] pub struct EntryFieldMetadata { kind: EntryFieldKind, sensitivity: EntrySensitivity, name: Option, otp: Option, + diagnostic: Option, value: Range, } @@ -66,6 +73,10 @@ impl EntryFieldMetadata { pub fn otp(&self) -> Option<&EntryOtpMetadata> { self.otp.as_ref() } + + pub fn diagnostic(&self) -> Option { + self.diagnostic + } } /// Non-secret presentation metadata parsed from a validated `otpauth` URI. @@ -545,6 +556,8 @@ fn classify(index: usize, line: &[u8]) -> EntryFieldMetadata { sensitivity: EntrySensitivity::Sensitive, name: Some("password".to_owned()), otp: None, + diagnostic: (!line.is_ascii() && std::str::from_utf8(line).is_err()) + .then_some(EntryFieldDiagnostic::NonUtf8Value), value: 0..line.len(), }; } @@ -568,6 +581,17 @@ fn classify(index: usize, line: &[u8]) -> EntryFieldMetadata { sensitivity: EntrySensitivity::Sensitive, name: Some("otp".to_owned()), otp: Some(otp), + diagnostic: None, + value: 0..line.len(), + }; + } + if line.starts_with(b"otpauth://") { + return EntryFieldMetadata { + kind: EntryFieldKind::OtpUri, + sensitivity: EntrySensitivity::Sensitive, + name: Some("otp".to_owned()), + otp: None, + diagnostic: Some(EntryFieldDiagnostic::MalformedOtpUri), value: 0..line.len(), }; } @@ -583,6 +607,9 @@ fn classify(index: usize, line: &[u8]) -> EntryFieldMetadata { sensitivity: field_sensitivity(name, kind), name: Some(name.to_owned()), otp: None, + diagnostic: std::str::from_utf8(&line[value_start..]) + .is_err() + .then_some(EntryFieldDiagnostic::NonUtf8Value), value: value_start..line.len(), }; } @@ -592,6 +619,9 @@ fn classify(index: usize, line: &[u8]) -> EntryFieldMetadata { sensitivity: EntrySensitivity::Sensitive, name: None, otp: None, + diagnostic: std::str::from_utf8(line) + .is_err() + .then_some(EntryFieldDiagnostic::NonUtf8Value), value: 0..line.len(), } } @@ -624,6 +654,7 @@ fn blank_metadata() -> EntryFieldMetadata { sensitivity: EntrySensitivity::Empty, name: None, otp: None, + diagnostic: None, value: 0..0, } }