diff --git a/crates/orbyn-device-client/src/signup.rs b/crates/orbyn-device-client/src/signup.rs index 97a8c1e..b885726 100644 --- a/crates/orbyn-device-client/src/signup.rs +++ b/crates/orbyn-device-client/src/signup.rs @@ -15,10 +15,18 @@ const MAX_PROOF_ROUNDS: usize = 2; /// A sign-up that waits for the emailed code. It holds the new device's keys in memory only: an /// abandoned sign-up leaves nothing behind. Drop it to cancel. +/// +/// Once the code is spent the account exists and the session the server answered with is held +/// here too (in memory, wiped on drop) until it is saved: if the key store fails at that moment, +/// the keys and the session stay here and [`DeviceClient::retry_save`] saves them again with no +/// new code. Dropping it before that leaves an account no device holds; see "A sign-up that +/// could not be saved" in `docs/device-client.md`. pub struct PendingSignUp { signup_id: String, email: String, keys: Option, + /// The session of a spent code that is not saved yet. + created: Option, } impl PendingSignUp { @@ -97,6 +105,7 @@ impl DeviceClient { signup_id: pending.signup_id, email, keys: Some(keys), + created: None, }); } if response.status != 428 || round == MAX_PROOF_ROUNDS { @@ -111,15 +120,23 @@ impl DeviceClient { /// Finishes a sign-up with the emailed code (any case, hyphen or not). A wrong code is /// [`DeviceError::InvalidCode`] and leaves the sign-up open: try again or ask for a new code. /// On success the keys, ids and session are saved and the account exists. + /// + /// If the code was accepted but the key store failed ([`DeviceError::Store`]), the code is + /// spent and the account exists: the keys and the session stay in `pending`, and + /// [`DeviceClient::retry_save`] (or this call again, whatever the code) saves them without + /// asking the server for anything. pub async fn verify_sign_up( &self, pending: &mut PendingSignUp, code: &str, ) -> Result { - let keys = pending - .keys - .as_ref() - .ok_or(DeviceError::InvalidInput("this sign-up is finished"))?; + if pending.keys.is_none() { + return Err(DeviceError::InvalidInput("this sign-up is finished")); + } + // A spent code cannot be sent again: finish saving what it made. + if pending.created.is_some() { + return self.retry_save(pending); + } // A sign-in finished in between would be overwritten: refuse before the code is spent. if self.store.load()?.is_some() { return Err(DeviceError::StoreNotEmpty); @@ -133,8 +150,24 @@ impl DeviceClient { }, ) .await?; - let info = self.save_session(&created, keys, &pending.email, None)?; + pending.created = Some(created); + self.retry_save(pending) + } + + /// Saves the keys and the session of a sign-up whose code was spent but whose save failed. + /// Nothing is sent to the server. A failure keeps everything in `pending` for another try; + /// a key store that already holds a device is [`DeviceError::StoreNotEmpty`]. With nothing + /// to save it is [`DeviceError::InvalidInput`]. + pub fn retry_save(&self, pending: &mut PendingSignUp) -> Result { + let (Some(keys), Some(created)) = (pending.keys.as_ref(), pending.created.as_ref()) else { + return Err(DeviceError::InvalidInput("there is nothing to save")); + }; + if self.store.load()?.is_some() { + return Err(DeviceError::StoreNotEmpty); + } + let info = self.save_session(created, keys, &pending.email, None)?; pending.keys = None; + pending.created = None; Ok(info) } diff --git a/crates/orbyn-device-client/tests/client.rs b/crates/orbyn-device-client/tests/client.rs index c36c87b..e37b638 100644 --- a/crates/orbyn-device-client/tests/client.rs +++ b/crates/orbyn-device-client/tests/client.rs @@ -6,7 +6,8 @@ mod support; use orbyn_contracts::{DeviceApp, DevicePlatform, DeviceState}; use orbyn_device_client::{ ClientConfig, Clock, DeviceApi, DeviceClient, DeviceError, HttpApi, KeyStore, MemoryStore, - Method, Request, Response, SecondFactor, SignInOutcome, SignUpRequest, StoredIdentity, + Method, Request, Response, SecondFactor, SignInOutcome, SignUpRequest, StoreError, + StoredIdentity, }; use std::sync::Arc; use std::sync::atomic::{AtomicI64, Ordering}; @@ -580,3 +581,155 @@ async fn the_app_label_is_signed_at_sign_up_and_sign_in_and_a_client_with_none_s ] ); } + +/// A store whose next `failures` saves fail, then behaves as a memory store. +struct FlakyStore { + inner: MemoryStore, + failures: std::sync::Mutex, +} + +impl FlakyStore { + fn failing(failures: usize) -> Self { + Self { + inner: MemoryStore::new(), + failures: std::sync::Mutex::new(failures), + } + } +} + +impl KeyStore for FlakyStore { + fn load(&self) -> Result, StoreError> { + self.inner.load() + } + + fn save(&self, identity: &StoredIdentity) -> Result<(), StoreError> { + let mut left = self.failures.lock().expect("lock"); + if *left > 0 { + *left -= 1; + return Err(StoreError); + } + drop(left); + self.inner.save(identity) + } + + fn clear(&self) -> Result<(), StoreError> { + self.inner.clear() + } +} + +fn flaky_client( + engine: &FakeEngine, + failures: usize, +) -> DeviceClient { + DeviceClient::new( + engine.clone(), + FlakyStore::failing(failures), + SharedClock(engine.now.clone()), + ClientConfig { + device_name: "Laptop".to_owned(), + platform: DevicePlatform::Linux, + app: None, + }, + ) +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_failed_save_after_the_code_is_spent_keeps_the_keys_and_retry_save_finishes() { + let engine = FakeEngine::new(); + let a = flaky_client(&engine, 2); + let mut pending = a.begin_sign_up(request(None)).await.expect("begin"); + let code = engine.code_for(NORMAL); + // The code is accepted, the save fails: the account exists and the code is spent. + assert!(matches!( + a.verify_sign_up(&mut pending, &code).await, + Err(DeviceError::Store(_)) + )); + assert_eq!(engine.device_count(NORMAL), 1); + assert_eq!(a.identity().expect("store"), None, "nothing was saved"); + let verifies = |engine: &FakeEngine| { + engine + .log() + .iter() + .filter(|(path, _)| path.ends_with("email/verify")) + .count() + }; + assert_eq!(verifies(&engine), 1); + + // The same call again, whatever the code, retries the save without asking the server. + assert!(matches!( + a.verify_sign_up(&mut pending, "00000000").await, + Err(DeviceError::Store(_)) + )); + assert_eq!(verifies(&engine), 1, "the spent code was not sent again"); + + // The store recovered: the keys and the session saved are the ones the server answered with. + let session = a.retry_save(&mut pending).expect("retry"); + assert_eq!(session.device_state, DeviceState::Trusted); + assert_eq!(verifies(&engine), 1); + let (account, device, email) = a.identity().expect("store").expect("saved"); + assert_eq!( + (account, device, email), + ( + session.account_id.clone(), + session.device_id.clone(), + NORMAL.to_owned() + ) + ); + // The saved device key is the one the server knows: it signs a trusted start. + a.reauthenticate(&password(PASSWORD)) + .await + .expect("a trusted start with the saved key"); + + // Nothing is left to save, and a finished sign-up is not verified again. + assert_eq!( + a.retry_save(&mut pending).err(), + Some(DeviceError::InvalidInput("there is nothing to save")) + ); + assert_eq!( + a.verify_sign_up(&mut pending, &code).await.err(), + Some(DeviceError::InvalidInput("this sign-up is finished")) + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn retry_save_with_no_spent_code_or_a_store_that_holds_a_device_saves_nothing() { + let engine = FakeEngine::new(); + let a = flaky_client(&engine, 0); + let mut pending = a.begin_sign_up(request(None)).await.expect("begin"); + assert_eq!( + a.retry_save(&mut pending).err(), + Some(DeviceError::InvalidInput("there is nothing to save")) + ); + + // The save fails, and a device lands in the store before the retry: it is never overwritten. + let b = flaky_client(&engine, 1); + let mut second = b + .begin_sign_up(SignUpRequest { + email: "other@example.com".to_owned(), + password: password(PASSWORD), + invite: None, + }) + .await + .expect("begin"); + let code = engine.code_for("other@example.com"); + assert!(b.verify_sign_up(&mut second, &code).await.is_err()); + let stored = StoredIdentity { + device_keys: orbyn_crypto::DeviceKeys::generate() + .expect("keys") + .to_bytes(), + account_id: "x".into(), + device_id: "y".into(), + email: "z@example.com".into(), + session: None, + vault_key: Zeroizing::new([9u8; 32]), + }; + b.store().save(&stored).expect("save"); + assert_eq!( + b.retry_save(&mut second).err(), + Some(DeviceError::StoreNotEmpty) + ); + assert_eq!( + b.store().load().expect("load").expect("kept").account_id, + "x" + ); +} diff --git a/docs/device-client.md b/docs/device-client.md index b0fa56e..c4aad20 100644 --- a/docs/device-client.md +++ b/docs/device-client.md @@ -32,6 +32,12 @@ The **zone-key cache is not here.** The host keeps zone keys in its own sealed s The result is a `PendingSignUp`. It holds the new device's keys **in memory only**: nothing is stored, and dropping it cancels, so an abandoned sign-up leaves nothing behind. `verify_sign_up(&mut pending, code)` sends the emailed code (any case; spaces and hyphens ignored). A wrong code is `InvalidCode` and leaves the sign-up open; `resend_sign_up` asks for a new one. Only on success are the keys, ids, session and a fresh vault key saved. A store that already holds a device is refused up front (`StoreNotEmpty`), and again before the code is sent, so a sign-in that finished in between is never overwritten and the code is not spent. +### A sign-up that could not be saved + +The code is spent when the server accepts it: the account exists from then on. If the key store then fails, `verify_sign_up` returns `DeviceError::Store` and the **keys and the session stay in the `PendingSignUp`**, in memory, wiped on drop. `retry_save(&mut pending)` saves them again and sends nothing to the server, so there is no new code and no second account; calling `verify_sign_up` again does the same whatever code is passed. A store that now holds a device is `StoreNotEmpty`. + +If the `PendingSignUp` is dropped first (the app quits, or the store never recovers), the account exists and no device holds its keys. The address cannot sign up again, and signing in would need a device that does not exist. The way out is the delete-unused link: ask for it from the sign-in screen's "my sign-up did not finish" path (`POST /v1/accounts/delete-unused {email}`, always answered 202), open the link in the email that comes within 24 hours, and the server erases the account, which was never used, and releases its invite (see `docs/architecture.md`, "Deleting an unused account"); then sign up again. A host shows this path when `verify_sign_up` ended in `Store` and the person leaves without a successful `retry_save`. + ## Sign-in `sign_in(email, password)` runs OPAQUE login. A wrong password fails **on the device** (`LoginFailed`) and nothing more is sent.