diff --git a/apps/cli/src/main.rs b/apps/cli/src/main.rs index 6082505..64ebd3e 100644 --- a/apps/cli/src/main.rs +++ b/apps/cli/src/main.rs @@ -29,7 +29,7 @@ use ironstorage::{ mutation::{ MutationError, NoGitTreeCommitter, TreeCommit, TreeCommitError, TreeCommitter, TreeMutator, }, - otp::{OtpError, OtpInput, OtpService}, + otp::{OtpCodeOutcome, OtpCodeValidity, OtpError, OtpInput, OtpService}, presentation::{ ClipboardError, ClipboardTimeout, ClipboardWait, NativeClipboardManager, QrError, QrMatrix, }, @@ -837,6 +837,7 @@ fn execute_otp return operation_error(stderr, error), }; if request.clipboard { + write_otp_validity(&outcome, timestamp, stderr)?; match presentation.clipboard( outcome.code(), config.clipboard_timeout(), @@ -851,6 +852,7 @@ fn execute_otp Result<(), ()> { + match outcome.validity() { + OtpCodeValidity::Timed { .. } => { + let remaining = outcome.remaining_at(unix_seconds).unwrap_or_default(); + let unit = if remaining == 1 { "second" } else { "seconds" }; + writeln!(feedback, "TOTP code is valid for {remaining} {unit}.").map_err(|_| ()) + } + OtpCodeValidity::CounterBased { counter } => writeln!( + feedback, + "HOTP counter {counter} is counter-based and has no time expiry." + ) + .map_err(|_| ()), + } +} + trait OtpInteraction { fn standard_input_is_terminal(&self) -> bool; @@ -2169,13 +2190,43 @@ mod tests { assert_eq!(code.len(), 6); assert!(code.iter().all(u8::is_ascii_digit)); assert!(!stdout.windows(code.len()).any(|part| part == code)); - assert!(stderr.is_empty()); + assert_eq!( + String::from_utf8_lossy(&stderr), + "TOTP code is valid for 1 second.\n" + ); assert_eq!(fs::read(vault.join("otp/totp.gpg"))?, ciphertext_before); assert_eq!(fs::read(&git_config)?, config_before); let git = GitRepository::open(&repository, identity)?; assert_eq!(git.log(None)?.len(), commits_before); stdout.clear(); + stderr.clear(); + assert_eq!( + execute_secure_with_services( + &config, + &CommandRequest::Otp(OtpRequest::Code(OtpCodeRequest { + entry: "otp/totp".to_owned(), + clipboard: false, + })), + &mut secrets, + &mut presentation, + &mut interaction, + || Ok(59), + &mut stdout, + &mut stderr, + ) + .expect("memory output cannot fail"), + EXIT_SUCCESS + ); + assert_eq!(stdout.len(), 7); + assert!(stdout[..6].iter().all(u8::is_ascii_digit)); + assert_eq!( + String::from_utf8_lossy(&stderr), + "TOTP code is valid for 1 second.\n" + ); + + stdout.clear(); + stderr.clear(); let uri = fs::read(fixtures.join("expected/basic/otp/totp.txt"))?; let uri = uri .split(|byte| *byte == b'\n') @@ -2203,6 +2254,7 @@ mod tests { assert!(stderr.is_empty()); stdout.clear(); + stderr.clear(); assert_eq!( execute_secure_with_services( &config, @@ -2321,6 +2373,7 @@ mod tests { ); stdout.clear(); + stderr.clear(); assert_eq!( execute_secure_with_services( &config, @@ -2340,6 +2393,10 @@ mod tests { ); assert_eq!(stdout.len(), 9); assert!(stdout[..8].iter().all(u8::is_ascii_digit)); + assert_eq!( + String::from_utf8_lossy(&stderr), + "HOTP counter 1 is counter-based and has no time expiry.\n" + ); let repository = Repository::open(&vault)?; let keys = ironstorage::crypto::KeyStore::load(fixtures.join("keys"))?; @@ -2366,7 +2423,6 @@ mod tests { &mut secrets, )?; assert!(hotp.expose().windows(9).any(|part| part == b"counter=1")); - assert!(stderr.is_empty()); Ok(()) } diff --git a/apps/tui/src/app.rs b/apps/tui/src/app.rs index 641ca10..368ab50 100644 --- a/apps/tui/src/app.rs +++ b/apps/tui/src/app.rs @@ -11,6 +11,7 @@ use ironstorage::{ crypto::KeyInfo, document::{DocumentError, EntryDocument, EntryFieldId}, git::{GitConflict, GitProgressPhase, GitSnapshot}, + otp::OtpCodeValidity, presentation::{ClipboardDisposition, QrMatrix}, read::{FindResults, GrepResults, TreeModel}, repository::SecretBytes, @@ -95,8 +96,8 @@ pub struct OtpDisplay { entry: String, field: Option, code: SecretBytes, + validity: OtpCodeValidity, remaining_seconds: Option, - counter: Option, } #[derive(Debug)] @@ -135,7 +136,7 @@ impl OtpDisplay { self.remaining_seconds } pub fn counter(&self) -> Option { - self.counter + self.validity.counter() } } @@ -207,8 +208,8 @@ pub enum AsyncPayload { entry: String, field: Option, code: SecretBytes, - remaining_seconds: Option, - counter: Option, + validity: OtpCodeValidity, + observed_at: u64, clipboard: bool, tree: Option, }, @@ -528,18 +529,16 @@ impl App { pub fn tick(&mut self) { self.ticks = self.ticks.wrapping_add(1); - if self.ticks.is_multiple_of(4) - && let Some(remaining) = self - .otp_display - .as_mut() - .and_then(|display| display.remaining_seconds.as_mut()) - { - *remaining = remaining.saturating_sub(1); + } + + pub fn observe_time(&mut self, unix_seconds: u64) { + if let Some(display) = self.otp_display.as_mut() { + display.remaining_seconds = display.validity.remaining_at(unix_seconds); } } pub fn begin_totp_refresh(&mut self) -> Option<(String, EntryFieldId)> { - if self.otp_pending || !self.ticks.is_multiple_of(4) || self.mode != Mode::Viewer { + if self.otp_pending || self.mode != Mode::Viewer { return None; } let viewer = self.viewer.as_ref()?; @@ -787,8 +786,8 @@ impl App { entry, field, code, - remaining_seconds, - counter, + validity, + observed_at, clipboard, tree, }) => { @@ -799,7 +798,7 @@ impl App { if clipboard { self.clipboard_request = Some(SecretBytes::new(code.expose().to_vec())); } - self.status = if let Some(counter) = counter { + self.status = if let Some(counter) = validity.counter() { format!("Generated and committed HOTP counter {counter}") } else { "TOTP code refreshed".to_owned() @@ -808,8 +807,8 @@ impl App { entry, field, code, - remaining_seconds, - counter, + validity, + remaining_seconds: validity.remaining_at(observed_at), }); self.hotp_confirmation = None; } diff --git a/apps/tui/src/lib.rs b/apps/tui/src/lib.rs index 616b46e..aa3d4c6 100644 --- a/apps/tui/src/lib.rs +++ b/apps/tui/src/lib.rs @@ -111,39 +111,15 @@ pub fn run(terminal: &mut DefaultTerminal) -> io::Result<()> { apply_authentication_event(&mut app, coordinator, &executor, &mut git_control, event); } + let unix_seconds = current_unix_seconds().map_err(io::Error::other)?; + app.observe_time(unix_seconds); + schedule_totp_refresh(&mut app, authentication.as_ref(), &executor); + let size = terminal.size()?; app.resize(size.width, size.height); terminal.draw(|frame| ui::draw_with_color_capability(frame, &app, color_capability))?; if !event::poll(TICK_INTERVAL)? { app.tick(); - if let Some((entry, field)) = app.begin_totp_refresh() { - if let (Some(handle), Some(config)) = ( - authentication - .as_ref() - .and_then(AuthenticationCoordinator::handle), - app.config().cloned(), - ) { - let token = app.begin_request(); - executor.submit(token, move || { - execute_otp_ui( - &config, - crate::app::OtpUiRequest { - request: ironstorage::command::OtpRequest::Code( - ironstorage::command::OtpCodeRequest { - entry, - clipboard: false, - }, - ), - confirmed_hotp: false, - field: Some(field), - }, - handle, - ) - }); - } else { - app.authentication_failed("authentication lease expired".to_owned()); - } - } if let Some(coordinator) = authentication.as_mut() { if let Some(event) = coordinator.poll_lease() { apply_authentication_event( @@ -265,6 +241,46 @@ pub fn run(terminal: &mut DefaultTerminal) -> io::Result<()> { Ok(()) } +fn current_unix_seconds() -> Result { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|duration| duration.as_secs()) +} + +fn schedule_totp_refresh( + app: &mut App, + authentication: Option<&AuthenticationCoordinator>, + executor: &AsyncExecutor, +) { + let Some((entry, field)) = app.begin_totp_refresh() else { + return; + }; + let (Some(handle), Some(config)) = ( + authentication.and_then(AuthenticationCoordinator::handle), + app.config().cloned(), + ) else { + app.authentication_failed("authentication lease expired".to_owned()); + return; + }; + let token = app.begin_request(); + executor.submit(token, move || { + execute_otp_ui( + &config, + crate::app::OtpUiRequest { + request: ironstorage::command::OtpRequest::Code( + ironstorage::command::OtpCodeRequest { + entry, + clipboard: false, + }, + ), + confirmed_hotp: false, + field: Some(field), + }, + handle, + ) + }); +} + fn handle_terminal_ownership_lost( app: &mut App, authentication: &mut Option, @@ -822,23 +838,20 @@ fn execute_otp_ui( if uri.kind() == OtpKind::Hotp && !request.confirmed_hotp { return Err("HOTP generation requires explicit confirmation".to_owned()); } - let unix_seconds = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map_err(|_| "the system clock is before the Unix epoch".to_owned())? - .as_secs(); + let unix_seconds = current_unix_seconds() + .map_err(|_| "the system clock is before the Unix epoch".to_owned())?; let outcome = service .code_automatic(&code_request.entry, unix_seconds, None, &mut provider) .map_err(|error| error.to_string())?; - let remaining_seconds = outcome.remaining_at(unix_seconds); - let counter = outcome.counter(); + let validity = outcome.validity(); let code = ironstorage::repository::SecretBytes::new(outcome.code().expose().to_vec()); - let tree = counter.map(|_| load_tree(config)).transpose()?; + let tree = validity.counter().map(|_| load_tree(config)).transpose()?; Ok(AsyncPayload::OtpCodeFinished { entry: code_request.entry, field, code, - remaining_seconds, - counter, + validity, + observed_at: unix_seconds, clipboard: code_request.clipboard, tree, }) diff --git a/apps/tui/src/ui.rs b/apps/tui/src/ui.rs index d4d3965..3efb07a 100644 --- a/apps/tui/src/ui.rs +++ b/apps/tui/src/ui.rs @@ -1028,8 +1028,8 @@ mod tests { entry: "otp/totp".to_owned(), field: Some(wrong_field), code: ironstorage::repository::SecretBytes::new(b"123456".to_vec()), - remaining_seconds: Some(12), - counter: None, + validity: ironstorage::otp::OtpCodeValidity::Timed { valid_until: 72 }, + observed_at: 60, clipboard: false, tree: None, }), @@ -1042,8 +1042,8 @@ mod tests { entry: "otp/totp".to_owned(), field: Some(otp_field), code: ironstorage::repository::SecretBytes::new(b"123456".to_vec()), - remaining_seconds: Some(12), - counter: None, + validity: ironstorage::otp::OtpCodeValidity::Timed { valid_until: 72 }, + observed_at: 60, clipboard: false, tree: None, }), @@ -1053,6 +1053,30 @@ mod tests { assert!(code.contains("12s remaining")); assert!(!code.contains("JBSWY3DPEHPK3PXP")); + app.observe_time(72); + assert!(render(120, 20, &app).contains("0s remaining")); + assert_eq!( + app.begin_totp_refresh(), + Some(("otp/totp".to_owned(), otp_field)) + ); + let token = app.begin_request(); + app.apply_result(crate::app::AsyncResult { + token, + payload: Ok(crate::app::AsyncPayload::OtpCodeFinished { + entry: "otp/totp".to_owned(), + field: Some(otp_field), + code: ironstorage::repository::SecretBytes::new(b"654321".to_vec()), + validity: ironstorage::otp::OtpCodeValidity::Timed { valid_until: 102 }, + observed_at: 72, + clipboard: false, + tree: None, + }), + }); + let refreshed = render(120, 20, &app); + assert!(refreshed.contains("654321")); + assert!(refreshed.contains("30s remaining")); + assert!(!refreshed.contains("123456")); + let payload = ironstorage::repository::SecretBytes::new( b"otpauth://totp/test?secret=NEVER-RENDER".to_vec(), ); diff --git a/crates/storage/src/otp.rs b/crates/storage/src/otp.rs index 5bf44c6..f4a7a15 100644 --- a/crates/storage/src/otp.rs +++ b/crates/storage/src/otp.rs @@ -426,8 +426,39 @@ impl OtpWriteOutcome { pub struct OtpCodeOutcome { code: SecretBytes, - counter: Option, - valid_until: Option, + validity: OtpCodeValidity, +} + +/// Storage-owned validity information for presenting an OTP code. +/// +/// Frontends use this value instead of deriving TOTP periods or inferring HOTP +/// behavior from display strings. A timed code carries its exclusive Unix-time +/// boundary, while a counter-based code identifies the committed HOTP counter. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum OtpCodeValidity { + Timed { valid_until: u64 }, + CounterBased { counter: u64 }, +} + +impl OtpCodeValidity { + pub fn valid_until(self) -> Option { + match self { + Self::Timed { valid_until } => Some(valid_until), + Self::CounterBased { .. } => None, + } + } + + pub fn counter(self) -> Option { + match self { + Self::Timed { .. } => None, + Self::CounterBased { counter } => Some(counter), + } + } + + pub fn remaining_at(self, unix_seconds: u64) -> Option { + self.valid_until() + .map(|valid_until| valid_until.saturating_sub(unix_seconds)) + } } impl OtpCodeOutcome { @@ -436,16 +467,19 @@ impl OtpCodeOutcome { } pub fn counter(&self) -> Option { - self.counter + self.validity.counter() } pub fn valid_until(&self) -> Option { - self.valid_until + self.validity.valid_until() + } + + pub fn validity(&self) -> OtpCodeValidity { + self.validity } pub fn remaining_at(&self, unix_seconds: u64) -> Option { - self.valid_until - .map(|valid_until| valid_until.saturating_sub(unix_seconds)) + self.validity.remaining_at(unix_seconds) } } @@ -454,8 +488,7 @@ impl fmt::Debug for OtpCodeOutcome { formatter .debug_struct("OtpCodeOutcome") .field("code", &"[REDACTED]") - .field("counter", &self.counter) - .field("valid_until", &self.valid_until) + .field("validity", &self.validity) .finish() } } @@ -657,13 +690,12 @@ impl<'a> OtpService<'a> { let period = uri.period().ok_or(OtpError::NotTotp)?; return Ok(OtpCodeOutcome { code: uri.code_at(unix_seconds)?, - counter: None, - valid_until: Some( - (unix_seconds / period) + validity: OtpCodeValidity::Timed { + valid_until: (unix_seconds / period) .checked_add(1) .and_then(|counter| counter.checked_mul(period)) .ok_or(OtpError::CounterOverflow)?, - ), + }, }); } let entry = path.to_string(); @@ -713,14 +745,15 @@ impl<'a> OtpService<'a> { match uri.kind() { OtpKind::Totp => Ok(OtpCodeOutcome { code: uri.code_at(unix_seconds)?, - counter: None, - valid_until: Some({ - let period = uri.period().ok_or(OtpError::NotTotp)?; - (unix_seconds / period) - .checked_add(1) - .and_then(|counter| counter.checked_mul(period)) - .ok_or(OtpError::CounterOverflow)? - }), + validity: OtpCodeValidity::Timed { + valid_until: { + let period = uri.period().ok_or(OtpError::NotTotp)?; + (unix_seconds / period) + .checked_add(1) + .and_then(|counter| counter.checked_mul(period)) + .ok_or(OtpError::CounterOverflow)? + }, + }, }), OtpKind::Hotp => { let (counter, incremented) = uri.incremented_hotp()?; @@ -742,8 +775,7 @@ impl<'a> OtpService<'a> { )?; Ok(OtpCodeOutcome { code, - counter: Some(counter), - valid_until: None, + validity: OtpCodeValidity::CounterBased { counter }, }) } } diff --git a/crates/storage/tests/otp.rs b/crates/storage/tests/otp.rs index 8a468f9..741e4d2 100644 --- a/crates/storage/tests/otp.rs +++ b/crates/storage/tests/otp.rs @@ -9,7 +9,7 @@ use ironstorage::{ command::{OtpAppendRequest, OtpInputSource, OtpInsertRequest}, crypto::{KeyInfo, KeyStore, SecretProvider, SecretProviderError}, git::{GitIdentity, GitRepository}, - otp::{OtpAlgorithm, OtpError, OtpInput, OtpKind, OtpService, OtpUri}, + otp::{OtpAlgorithm, OtpCodeValidity, OtpError, OtpInput, OtpKind, OtpService, OtpUri}, recipient::RecipientPolicyManager, repository::{EntryPath, Repository, SecretBytes}, write::{EntryCommit, EntryCommitError, EntryCommitter, OverwriteDecision}, @@ -478,7 +478,9 @@ fn automatic_code_supports_pass_diff_config_and_opens_git_only_for_hotp() -> Tes let totp = service.code_automatic("otp/totp", 59, None, &mut provider)?; assert_eq!(totp.counter(), None); assert_eq!(totp.valid_until(), Some(60)); + assert_eq!(totp.validity(), OtpCodeValidity::Timed { valid_until: 60 }); assert_eq!(totp.remaining_at(59), Some(1)); + assert_eq!(totp.remaining_at(60), Some(0)); assert_eq!(repository.read_entry(&totp_path)?, totp_before); let git = GitRepository::open(&repository, identity.clone())?; assert_eq!(git.log(None)?.len(), initial_commits); @@ -487,6 +489,11 @@ fn automatic_code_supports_pass_diff_config_and_opens_git_only_for_hotp() -> Tes let hotp = service.code_automatic("otp/hotp", 0, None, &mut provider)?; assert_eq!(hotp.counter(), Some(1)); assert_eq!(hotp.valid_until(), None); + assert_eq!( + hotp.validity(), + OtpCodeValidity::CounterBased { counter: 1 } + ); + assert_eq!(hotp.remaining_at(0), None); assert_eq!(service.uri("otp/hotp", &mut provider)?.counter(), Some(1)); let git = GitRepository::open(&repository, identity)?; assert_eq!(git.log(None)?.len(), initial_commits + 1); diff --git a/docs/otp.md b/docs/otp.md index b5d347b..1688ce3 100644 --- a/docs/otp.md +++ b/docs/otp.md @@ -48,3 +48,19 @@ code whose counter update was not committed. OTP codes support terminal or secret-safe clipboard presentation. URI output supports terminal, clipboard, and the shared storage-owned QR matrix renderer. Clipboard and QR requests never print the underlying code or URI as plaintext. + +Every generated code carries an `OtpCodeValidity` value from `crates/storage`. +`Timed { valid_until }` identifies the exclusive Unix-time boundary for TOTP; +frontends call its `remaining_at` method to present a countdown and request a +replacement at zero. `CounterBased { counter }` identifies the HOTP counter +whose increment was committed and must be described as counter-based rather +than time-limited. Frontends must not recover periods from OTP URIs, decrement +an assumed interval, or infer the kind from formatted text. This same contract +is intended for the terminal, desktop, Apple, AutoFill, and watch interfaces. + +The CLI preserves code-only standard output for pass-compatible pipelines and +reports the non-secret validity description on standard error. Clipboard +lifecycle feedback remains separate, and clipboard-only presentation does not +echo the code. The TUI observes the system clock during its normal repaint loop, +asks the storage validity value for the remaining seconds, and refreshes at the +exact boundary without treating repainting as user activity.