diff --git a/Cargo.lock b/Cargo.lock index f8e0a55..23f3ab5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1886,6 +1886,7 @@ dependencies = [ "orbyn-client", "orbyn-contracts", "orbyn-crypto", + "orbyn-engineering", "orbyn-sync", "reqwest", "serde", diff --git a/crates/orbyn-device-client/Cargo.toml b/crates/orbyn-device-client/Cargo.toml index d91c4e3..767a798 100644 --- a/crates/orbyn-device-client/Cargo.toml +++ b/crates/orbyn-device-client/Cargo.toml @@ -15,6 +15,7 @@ orbyn-auth-client.workspace = true orbyn-client.workspace = true orbyn-contracts.workspace = true orbyn-crypto.workspace = true +orbyn-engineering.workspace = true orbyn-sync.workspace = true reqwest.workspace = true serde.workspace = true diff --git a/crates/orbyn-device-client/src/lib.rs b/crates/orbyn-device-client/src/lib.rs index 196d934..6500458 100644 --- a/crates/orbyn-device-client/src/lib.rs +++ b/crates/orbyn-device-client/src/lib.rs @@ -39,4 +39,10 @@ pub use orbyn_client::{ Snapshot, Stamp, Trashed, }; pub use orbyn_contracts::{DeviceRemoval, DeviceState}; +// The names Engineering gives its zone and record, and the settings type that holds them, so the +// app never spells them by hand. `orbyn-engineering` does not depend on this crate. +pub use orbyn_engineering::{ + Accepted, PERSON_RECORD_ID, PERSON_SETTINGS, Rejected, Rule, Settings, SettingsError, + ZONE_SETTINGS, +}; pub use zeroize::Zeroizing; diff --git a/crates/orbyn-device-client/src/signup.rs b/crates/orbyn-device-client/src/signup.rs index b885726..af43e21 100644 --- a/crates/orbyn-device-client/src/signup.rs +++ b/crates/orbyn-device-client/src/signup.rs @@ -171,6 +171,30 @@ impl DeviceClient { Ok(info) } + /// Asks the server whether an invite code can start a sign-up, before the person types the + /// rest (`POST /v1/invites/check`). `Ok` means it can; nothing is used up. An unknown, used or + /// expired code is [`DeviceError::InviteInvalid`] with one answer for all three, and too many + /// wrong tries from this address are [`DeviceError::RateLimited`]. A server that does not ask + /// for invites says yes to any code. The code is sent as typed; the server ignores case, + /// spaces and hyphens. + pub async fn check_invite(&self, code: &str) -> Result<(), DeviceError> { + let response = self + .call( + Method::Post, + "/v1/invites/check", + None, + Some(json(&wire::InviteCheck { + invite: code.to_owned(), + })?), + ) + .await?; + if is_ok(&response) { + Ok(()) + } else { + Err(failure(&response)) + } + } + /// Asks for the code again (the server allows three an hour and answers the same whatever /// happens). pub async fn resend_sign_up(&self, pending: &PendingSignUp) -> Result<(), DeviceError> { diff --git a/crates/orbyn-device-client/tests/client.rs b/crates/orbyn-device-client/tests/client.rs index bfa3bf2..02be9bb 100644 --- a/crates/orbyn-device-client/tests/client.rs +++ b/crates/orbyn-device-client/tests/client.rs @@ -759,3 +759,76 @@ async fn retry_save_with_no_spent_code_or_a_store_that_holds_a_device_saves_noth "x" ); } + +#[tokio::test(flavor = "multi_thread")] +async fn check_invite_says_yes_to_a_good_code_and_one_answer_to_every_bad_one() { + let engine = FakeEngine::new(); + let a = client(&engine, "Laptop"); + // A server that asks for no invites accepts any code. + assert_eq!(a.check_invite("anything").await, Ok(())); + + engine.add_invite("ABCD1234"); + // Case, spaces and hyphens are the server's business; the code goes as typed. + assert_eq!(a.check_invite("abcd-1234").await, Ok(())); + for bad in ["WRONG-CODE", "", "ABCD12345"] { + assert_eq!( + a.check_invite(bad).await.err(), + Some(DeviceError::InviteInvalid), + "{bad:?}" + ); + } + // Checking spends nothing and signs nothing up. + assert_eq!(a.check_invite("abcd-1234").await, Ok(())); + assert_eq!(a.identity().expect("store"), None); + let sent: Vec<_> = engine + .log() + .into_iter() + .filter(|(path, _)| path.contains("invites/check")) + .collect(); + assert_eq!(sent.len(), 6); + assert!(sent.iter().any(|(_, body)| body.contains("abcd-1234"))); +} + +#[tokio::test(flavor = "multi_thread")] +async fn check_invite_maps_a_limit_and_a_dead_network() { + struct Status(u16, Option); + impl DeviceApi for Status { + async fn send(&self, _: Request) -> Result { + match self.0 { + 0 => Err(DeviceError::Transport), + status => Ok(Response { + status, + retry_after: self.1, + body: Zeroizing::new(br#"{"code":"too_many_attempts","message":"x"}"#.to_vec()), + }), + } + } + } + let config = || ClientConfig { + device_name: "Laptop".to_owned(), + platform: DevicePlatform::Linux, + app: None, + }; + let limited = DeviceClient::new( + Status(429, Some(30)), + MemoryStore::new(), + SharedClock(Arc::new(AtomicI64::new(0))), + config(), + ); + assert_eq!( + limited.check_invite("X").await.err(), + Some(DeviceError::RateLimited { + retry_after: Some(Duration::from_secs(30)) + }) + ); + let offline = DeviceClient::new( + Status(0, None), + MemoryStore::new(), + SharedClock(Arc::new(AtomicI64::new(0))), + config(), + ); + assert_eq!( + offline.check_invite("X").await.err(), + Some(DeviceError::Transport) + ); +} diff --git a/crates/orbyn-device-client/tests/reexports.rs b/crates/orbyn-device-client/tests/reexports.rs new file mode 100644 index 0000000..793bc6d --- /dev/null +++ b/crates/orbyn-device-client/tests/reexports.rs @@ -0,0 +1,29 @@ +//! The settings names and types are nameable from this crate alone, and they are the settings +//! crate's own. + +use orbyn_device_client::{ + Accepted, PERSON_RECORD_ID, PERSON_SETTINGS, Rejected, Rule, Settings, SettingsError, + ZONE_SETTINGS, +}; + +#[allow(dead_code)] +type Named = ( + Option, + Option, + Option, + Option, + Option, +); + +#[test] +fn the_zone_and_record_names_are_the_ones_the_app_syncs_under() { + assert_eq!(ZONE_SETTINGS, "engineering.settings"); + assert_eq!(PERSON_SETTINGS, "person_settings"); + assert_eq!(PERSON_RECORD_ID, "person"); +} + +#[test] +fn settings_is_usable_through_this_crate() { + let settings = Settings::default(); + assert!(format!("{settings:?}").contains("Settings")); +} diff --git a/crates/orbyn-device-client/tests/support/mod.rs b/crates/orbyn-device-client/tests/support/mod.rs index a55fc7c..e004c58 100644 --- a/crates/orbyn-device-client/tests/support/mod.rs +++ b/crates/orbyn-device-client/tests/support/mod.rs @@ -254,6 +254,20 @@ impl FakeEngine { match (request.method, request.path.as_str()) { (Method::Post, "/v1/accounts/register/start") => register_start(&mut s, &body), (Method::Post, "/v1/accounts/register/finish") => register_finish(&mut s, body), + (Method::Post, "/v1/invites/check") => { + // As the server: any code is fine when invites are not asked for; else a known, + // unused one (ignoring case, spaces and hyphens). + let code = body["invite"] + .as_str() + .unwrap_or_default() + .replace(['-', ' '], "") + .to_uppercase(); + if s.invites.is_empty() || s.invites.contains(&code) { + answer(204, Value::Null) + } else { + error(400, "invite_invalid") + } + } (Method::Post, "/v1/accounts/email/verify") => email_verify(&mut s, &body, now), (Method::Post, "/v1/accounts/email/resend") => answer(204, Value::Null), (Method::Post, "/v1/sessions/login/start") => login_start(&mut s, &body, now), diff --git a/crates/orbyn-server/tests/device_client.rs b/crates/orbyn-server/tests/device_client.rs index 404700e..26d2c62 100644 --- a/crates/orbyn-server/tests/device_client.rs +++ b/crates/orbyn-server/tests/device_client.rs @@ -62,6 +62,11 @@ struct Server { impl Server { /// The real app on Postgres, served on a loopback port. async fn start() -> Self { + Self::start_mode(orbyn_core::SignupMode::Open).await + } + + /// The same, with sign-up open or by invite. + async fn start_mode(mode: orbyn_core::SignupMode) -> Self { let url = std::env::var("DATABASE_URL").expect("set DATABASE_URL to run the Postgres tests"); let db = Database::connect(&url).await.expect("connect to Postgres"); @@ -79,7 +84,7 @@ impl Server { }; let accounts = Accounts::new(db.pool().clone(), secrets) .with_limits(limits) - .with_signup(orbyn_core::SignupMode::Open); + .with_signup(mode); let hub = Hub::start(db.pool().clone()); let store = Store::new(db.pool().clone(), hub); let state = AppState::new(Arc::new(Up)) @@ -959,3 +964,62 @@ async fn the_approver_cannot_confirm_or_deny_an_approval_the_new_device_already_ Some(DeviceError::ApprovalExpired) ); } + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn check_invite_tells_a_good_code_from_a_used_an_expired_and_an_unknown_one() { + let server = Server::start_mode(orbyn_core::SignupMode::Invite).await; + let device = server.device("Laptop"); + let pool = server.db.pool(); + let made = orbyn_core::accounts::invites::create(pool, 3, 14, "check") + .await + .expect("invites"); + + // A good code is fine, however it is typed, and checking spends nothing. + assert_eq!(device.check_invite(&made[0].code).await, Ok(())); + assert_eq!( + device.check_invite(&made[0].code.to_lowercase()).await, + Ok(()) + ); + assert_eq!(device.check_invite(&made[0].code).await, Ok(())); + + // Unknown, empty, expired and used codes get the same answer. + assert_eq!( + device.check_invite("ZZZZ-ZZZZ-ZZZZ-ZZZZ").await.err(), + Some(DeviceError::InviteInvalid) + ); + assert_eq!( + device.check_invite("").await.err(), + Some(DeviceError::InviteInvalid) + ); + orbyn_core::sqlx::query( + "UPDATE invites SET expires_at = now() - interval '1 day' WHERE short_id = $1", + ) + .bind(&made[1].id) + .execute(pool) + .await + .expect("expire"); + assert_eq!( + device.check_invite(&made[1].code).await.err(), + Some(DeviceError::InviteInvalid) + ); + let email = email(); + let mut pending = device + .begin_sign_up(SignUpRequest { + email: email.clone(), + password: password(), + invite: Some(made[2].code.clone()), + }) + .await + .expect("begin with the third invite"); + let code = code_sent_to(pool, &email).await; + device + .verify_sign_up(&mut pending, &code) + .await + .expect("verify"); + let other = server.device("Phone"); + assert_eq!( + other.check_invite(&made[2].code).await.err(), + Some(DeviceError::InviteInvalid) + ); +} diff --git a/docs/device-client.md b/docs/device-client.md index 7d3b087..e18b2da 100644 --- a/docs/device-client.md +++ b/docs/device-client.md @@ -32,6 +32,8 @@ 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. +`check_invite(code)` asks `POST /v1/invites/check` whether an invite can start a sign-up, before the person types the rest, and spends nothing: `Ok` means yes, an unknown, used or expired code is `InviteInvalid` (one answer for all three), too many wrong tries are `RateLimited`, and a server that asks for no invites says yes to any code. The code goes as typed; the server ignores case, spaces and hyphens. + ### 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`. @@ -158,6 +160,7 @@ The app may not depend on or name `orbyn-contracts` (rule D6). Every type of ano | `orbyn-sync` (through `orbyn-client`) | `Fields`, `Stamp` | staging and reading records | | `orbyn-crypto` (through `orbyn-client`) | `DeviceKeys`, `DevicePublic` | `StoredIdentity::keys()`, device keys | | `zeroize` | `Zeroizing` | `StoredIdentity`, `SecondFactor` | +| `orbyn-engineering` | `ZONE_SETTINGS` (`engineering.settings`), `PERSON_SETTINGS` (`person_settings`), `PERSON_RECORD_ID` (`person`), `Settings`, `SettingsError`, `Rule`, `Accepted`, `Rejected` | the names the app syncs its settings under, and the type that holds them, so it does not spell them by hand; `orbyn-engineering` does not depend on this crate | `orbyn-client` re-exports, besides those, the other contract types its `Engine` methods take or return (`ZoneList`, `UploadChanges`, `UploadResult`, `RemoveDevice`, `DeleteZone`, `KeyEnvelope`, `NewKeyEnvelope`, `KeyRotation`, `ApprovalNonce`, `CreateApproval`, `ConfirmApproval`, `ProtectionChain`, `ProtectionLink`, `ProtectionLevel`, `SetProtection`, `Record`, `RecordChanged`, `ZoneChanges`). diff --git a/scripts/check_structure.py b/scripts/check_structure.py index 782700b..4ead31e 100644 --- a/scripts/check_structure.py +++ b/scripts/check_structure.py @@ -55,6 +55,7 @@ "orbyn-client", "orbyn-contracts", "orbyn-crypto", + "orbyn-engineering", "orbyn-sync", }, "orbyn-engineering": {"orbyn-sync"},