diff --git a/crates/orbyn-device-client/src/approvals.rs b/crates/orbyn-device-client/src/approvals.rs new file mode 100644 index 0000000..ede765e --- /dev/null +++ b/crates/orbyn-device-client/src/approvals.rs @@ -0,0 +1,323 @@ +//! Approving a new device and removing one: the device's side of the two-way comparison, over +//! HTTP. Every signature and every check is the engine's (`orbyn_client::Engine`); this file only +//! moves messages between the engine and the server and keeps the order of the steps. +//! +//! The approver (a trusted device) calls [`DeviceClient::start_approval`], then +//! [`DeviceClient::approval_state`] until it shows digits, and when the person sees the same digits +//! on the new device [`DeviceClient::confirm_approval`]. The new device calls +//! [`DeviceClient::join_approval`] when [`DeviceClient::pending_approval`] shows one, then +//! [`DeviceClient::device_digits`] until it shows digits, and when the person confirms there (or +//! types the approver's digits) [`DeviceClient::finish_approval`]. Either side cancels with +//! `deny_*`. Every approval is used once. + +use crate::client::{DeviceClient, json, parse}; +use crate::{Clock, DeviceApi, DeviceError, KeyStore, Method}; +use orbyn_client::{ApproverPairing, ClientError, DevicePairing, Engine}; +use orbyn_contracts as wire; + +fn mismatch(error: ClientError) -> DeviceError { + match error { + ClientError::Mismatch => DeviceError::ApprovalMismatch, + ClientError::InvalidInput => { + DeviceError::InvalidInput("the device cannot be used for this") + } + other => crate::sync::engine_error(other), + } +} + +/// An approval this trusted device started. Use it once. +pub struct ApproverApproval { + pairing: ApproverPairing, + revealed: bool, +} + +impl ApproverApproval { + pub fn id(&self) -> &str { + self.pairing.approval_id() + } +} + +/// What the approver's screen shows. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ApproverState { + /// The new device has not joined yet. + Waiting, + /// Show these on this screen and ask the person whether the new device shows the same. + Digits { + digits: String, + /// The new device's key fingerprint, to show beside the digits. + fingerprint: String, + }, +} + +/// An approval the new device joined. Use it once. +pub struct JoinedApproval { + pairing: DevicePairing, +} + +impl JoinedApproval { + pub fn id(&self) -> &str { + self.pairing.approval_id() + } +} + +impl DeviceClient { + fn approval_path(id: &str, step: Option<&str>) -> String { + match step { + Some(step) => format!("/v1/devices/approvals/{id}/{step}"), + None => format!("/v1/devices/approvals/{id}"), + } + } + + /// Best effort: a cancel that fails changes nothing, and the approval ends in ten minutes. + async fn cancel_quietly(&self, id: &str) { + let _ = self + .authed(Method::Post, &Self::approval_path(id, Some("cancel")), None) + .await; + } + + // ---- the approver ---- + + /// Starts approving `device_id`, a device from [`DeviceClient::devices`] that this device does + /// not trust by its own view. Refused for a device it trusts or revoked, whatever the server + /// lists. + pub async fn start_approval( + &self, + engine: &Engine, + device_id: &str, + ) -> Result { + let devices = self.devices().await?; + let device = + devices + .devices + .iter() + .find(|d| d.id == device_id) + .ok_or(DeviceError::InvalidInput( + "the device is not in the account", + ))?; + let (pairing, create) = engine + .start_approval(device, self.clock.now_secs()) + .map_err(mismatch)?; + let response = self + .authed(Method::Post, "/v1/devices/approvals", Some(json(&create)?)) + .await?; + let created: wire::ApprovalCreated = parse(&response.body)?; + if created.id != pairing.approval_id() { + return Err(DeviceError::ApprovalMismatch); + } + Ok(ApproverApproval { + pairing, + revealed: false, + }) + } + + /// Asks the server how far the approval is. Once the new device joined, reveals this device's + /// nonce (once) and returns the digits. A server that shows other keys than this device + /// started with cancels the approval: [`DeviceError::ApprovalMismatch`]. + pub async fn approval_state( + &self, + approval: &mut ApproverApproval, + ) -> Result { + let id = approval.id().to_owned(); + let response = self + .authed(Method::Get, &Self::approval_path(&id, None), None) + .await?; + let view: wire::PendingApproval = parse(&response.body)?; + if view.device_nonce.is_none() { + return Ok(ApproverState::Waiting); + } + if !approval.revealed { + let reveal = match approval.pairing.reveal(&view) { + Ok(reveal) => reveal, + Err(error) => { + self.cancel_quietly(&id).await; + return Err(mismatch(error)); + } + }; + self.authed( + Method::Post, + &Self::approval_path(&id, Some("reveal")), + Some(json(&reveal)?), + ) + .await?; + approval.revealed = true; + } + let digits = approval + .pairing + .sas() + .ok_or(DeviceError::ApprovalMismatch)?; + Ok(ApproverState::Digits { + digits, + fingerprint: approval.pairing.device_fingerprint(), + }) + } + + /// The person confirmed that both screens show the same digits: signs the approval with the + /// devices this device trusts and sends it. Not before: [`DeviceError::ApprovalMismatch`] if + /// the digits were never shown. The host then calls [`DeviceClient::send_keys`] for each zone + /// so the new device can read them. + pub async fn confirm_approval( + &self, + engine: &mut Engine, + approval: ApproverApproval, + ) -> Result<(), DeviceError> { + let id = approval.id().to_owned(); + let devices = self.devices().await?; + let confirm = engine + .confirm_approval(approval.pairing, &devices) + .map_err(mismatch)?; + self.authed( + Method::Post, + &Self::approval_path(&id, Some("confirm")), + Some(json(&confirm)?), + ) + .await?; + Ok(()) + } + + /// The person said the digits differ, or cancelled: ends the approval on the server. + pub async fn deny_approval(&self, approval: ApproverApproval) -> Result<(), DeviceError> { + self.authed( + Method::Post, + &Self::approval_path(approval.id(), Some("cancel")), + None, + ) + .await?; + Ok(()) + } + + // ---- the new device ---- + + /// The approval a trusted device started for this device, if there is one. + pub async fn pending_approval(&self) -> Result, DeviceError> { + let response = self + .authed(Method::Get, "/v1/devices/current", None) + .await?; + let current: wire::CurrentDevice = parse(&response.body)?; + Ok(current.approval) + } + + /// Joins the approval a trusted device started, sending this device's nonce while it holds + /// only the approver's commitment. `None` when there is no approval. A device that named its + /// app joins with the description it signed in with, so the digits it shows only match the + /// approver's if the approver was shown the same. + pub async fn join_approval( + &self, + engine: &Engine, + ) -> Result, DeviceError> { + let Some(view) = self.pending_approval().await? else { + return Ok(None); + }; + let now = self.clock.now_secs(); + let (pairing, nonce) = match self.config.app { + Some(app) => engine.join_approval_labelled( + &view, + now, + &self.config.device_name, + self.config.platform, + app, + ), + None => engine.join_approval(&view, now), + } + .map_err(mismatch)?; + self.authed( + Method::Post, + &Self::approval_path(&view.id, Some("nonce")), + Some(json(&nonce)?), + ) + .await?; + Ok(Some(JoinedApproval { pairing })) + } + + /// The digits to show, once the approver revealed its nonce; `None` before. A reveal that does + /// not match the commitment is [`DeviceError::ApprovalMismatch`] and cancels the approval. + pub async fn device_digits( + &self, + joined: &JoinedApproval, + ) -> Result, DeviceError> { + let view = self + .pending_approval() + .await? + .filter(|v| v.id == joined.id()) + .ok_or(DeviceError::ApprovalExpired)?; + if view.approver_nonce.is_none() { + return Ok(None); + } + match joined.pairing.sas(&view) { + Ok(digits) => Ok(Some(digits)), + Err(error) => { + self.cancel_quietly(joined.id()).await; + Err(mismatch(error)) + } + } + } + + /// The person confirmed on this device that the digits match, or typed the approver's digits + /// as `typed`. The engine checks the reveal, the age, the approver's signature and its + /// endorsement and pins the approver and the devices it endorsed; only then are the digits + /// sent, once. On any failure nothing new is trusted and the approval is cancelled. + pub async fn finish_approval( + &self, + engine: &mut Engine, + joined: JoinedApproval, + typed: Option<&str>, + ) -> Result<(), DeviceError> { + let id = joined.id().to_owned(); + let view = self + .pending_approval() + .await? + .filter(|v| v.id == id) + .ok_or(DeviceError::ApprovalExpired)?; + let digits = + match engine.finish_approval(joined.pairing, &view, typed, self.clock.now_secs()) { + Ok(digits) => digits, + Err(error) => { + self.cancel_quietly(&id).await; + return Err(mismatch(error)); + } + }; + self.authed( + Method::Post, + &Self::approval_path(&id, Some("code")), + Some(json(&wire::ApprovalCode { code: digits })?), + ) + .await?; + Ok(()) + } + + /// The person said the digits differ, or cancelled: ends the approval on the server. + pub async fn deny_joined(&self, joined: JoinedApproval) -> Result<(), DeviceError> { + self.authed( + Method::Post, + &Self::approval_path(joined.id(), Some("cancel")), + None, + ) + .await?; + Ok(()) + } + + // ---- devices ---- + + /// Removes `device_id` from the account, signed by this device. A session whose password + /// sign-in is older than ten minutes is [`DeviceError::ReauthenticationRequired`]; the last + /// trusted device is [`DeviceError::LastTrustedDevice`]. + pub async fn remove_device(&self, engine: &Engine, device_id: &str) -> Result<(), DeviceError> { + let devices = self.devices().await?; + let device = + devices + .devices + .iter() + .find(|d| d.id == device_id) + .ok_or(DeviceError::InvalidInput( + "the device is not in the account", + ))?; + let body = engine.sign_removal(device).map_err(mismatch)?; + self.authed( + Method::Delete, + &format!("/v1/devices/{}", device.id), + Some(json(&body)?), + ) + .await?; + Ok(()) + } +} diff --git a/crates/orbyn-device-client/src/client.rs b/crates/orbyn-device-client/src/client.rs index 93d172e..836323d 100644 --- a/crates/orbyn-device-client/src/client.rs +++ b/crates/orbyn-device-client/src/client.rs @@ -151,7 +151,11 @@ pub(crate) fn failure(response: &Response) -> DeviceError { .unwrap_or_default(); match (response.status, code.as_str()) { (401, "reauthentication_required") => DeviceError::ReauthenticationRequired, - (400, "invalid_email_code") => DeviceError::InvalidCode, + (400, "invalid_email_code" | "wrong_code") => DeviceError::InvalidCode, + (410, "approval_expired") => DeviceError::ApprovalExpired, + (409, "approval_out_of_order" | "approval_not_confirmed") => DeviceError::ApprovalNotReady, + (400, "invalid_commitment") => DeviceError::ApprovalMismatch, + (409, "last_trusted_device") => DeviceError::LastTrustedDevice, (429, _) => DeviceError::RateLimited { retry_after: response.retry_after.map(Duration::from_secs), }, diff --git a/crates/orbyn-device-client/src/error.rs b/crates/orbyn-device-client/src/error.rs index a414d48..8a2eb05 100644 --- a/crates/orbyn-device-client/src/error.rs +++ b/crates/orbyn-device-client/src/error.rs @@ -17,7 +17,7 @@ pub enum DeviceError { /// [`crate::DeviceClient::reauthenticate`] and send the same request again. #[error("sign in again to do this")] ReauthenticationRequired, - /// The emailed code, or a second-factor code, is wrong or has expired. + /// The emailed code, a second-factor code or the approval digits are wrong or have expired. #[error("the code is wrong or has expired")] InvalidCode, /// Too many attempts; try again after `retry_after` when the server said when. @@ -56,6 +56,20 @@ pub enum DeviceError { /// A value the caller gave is not valid (an email, an address, a name). #[error("{0}")] InvalidInput(&'static str), + /// The approval ended: ten minutes passed, or either device cancelled or answered wrongly. + /// Start a new one. + #[error("the approval ended")] + ApprovalExpired, + /// The other device has not done its step yet; ask again shortly. + #[error("the other device has not answered yet")] + ApprovalNotReady, + /// The two devices did not agree, or the server showed this device something other than what + /// the approval was made with. The approval is cancelled; start a new one. + #[error("the approval did not match")] + ApprovalMismatch, + /// The last trusted device cannot be removed. + #[error("this is the account's last trusted device")] + LastTrustedDevice, /// The zone was deleted, here or on another device. #[error("the zone was deleted")] ZoneDeleted, diff --git a/crates/orbyn-device-client/src/lib.rs b/crates/orbyn-device-client/src/lib.rs index f4f6556..4abeff7 100644 --- a/crates/orbyn-device-client/src/lib.rs +++ b/crates/orbyn-device-client/src/lib.rs @@ -4,6 +4,7 @@ //! `docs/device-client.md`. mod api; +mod approvals; mod client; mod clock; mod error; @@ -14,6 +15,7 @@ mod store; mod sync; pub use api::{DeviceApi, HttpApi, Method, Request, Response, is_loopback_literal}; +pub use approvals::{ApproverApproval, ApproverState, JoinedApproval}; pub use client::{ ClientConfig, DeviceClient, Password, SecondFactor, SecondFactorChallenge, SessionInfo, SignInOutcome, SignUpRequest, diff --git a/crates/orbyn-server/tests/device_client.rs b/crates/orbyn-server/tests/device_client.rs index 781a309..baf4a80 100644 --- a/crates/orbyn-server/tests/device_client.rs +++ b/crates/orbyn-server/tests/device_client.rs @@ -2,30 +2,24 @@ //! two devices, zone keys, push and pull, a conflict, the snapshot, the notices stream and the //! status. Ignored unless run with `--include-ignored` and `DATABASE_URL`. -use axum::body::{Body, to_bytes}; -use axum::extract::ConnectInfo; -use axum::http::{Method, Request, StatusCode, header}; use orbyn_auth_client as auth; use orbyn_client::Engine; use orbyn_contracts as wire; -use orbyn_core::accounts::AuthSecrets; +use orbyn_core::accounts::{AuthSecrets, Limit, Limits}; use orbyn_core::{Accounts, Database}; use orbyn_device_client::{ - ClientConfig, DeviceClient, DeviceError, HttpApi, KeyStore, MemoryStore, Notice, NoticeConfig, - SignInOutcome, SignUpRequest, SyncState, SystemClock, + ApproverState, ClientConfig, DeviceApi, DeviceClient, DeviceError, HttpApi, KeyStore, + MemoryStore, Notice, NoticeConfig, SignInOutcome, SignUpRequest, SyncState, SystemClock, }; use orbyn_server::{AppState, Readiness, router}; use orbyn_store::{Hub, Store}; use orbyn_sync::Fields; -use serde::Serialize; -use serde::de::DeserializeOwned; use serde_json::{Value, json}; use std::future::Future; -use std::net::{IpAddr, Ipv4Addr, SocketAddr}; +use std::net::SocketAddr; use std::pin::Pin; use std::sync::{Arc, OnceLock}; use std::time::Duration; -use tower::ServiceExt; use uuid::Uuid; use zeroize::Zeroizing; @@ -60,7 +54,6 @@ async fn code_sent_to(pool: &orbyn_core::sqlx::PgPool, email: &str) -> String { } struct Server { - app: axum::Router, db: Database, base: String, } @@ -74,8 +67,18 @@ impl Server { db.migrate().await.expect("migrate"); let setup = SETUP.get_or_init(|| AuthSecrets::generate().to_string()); let secrets = AuthSecrets::from_encoded(setup).expect("setup"); - let accounts = - Accounts::new(db.pool().clone(), secrets).with_signup(orbyn_core::SignupMode::Open); + // Every test signs up from the one loopback address: the start throttle is not what these + // tests are about. + let limits = Limits { + signup_start_free_per_prefix: Limit::new(100_000, Duration::from_secs(3600)), + signup_start_max_per_prefix: Limit::new(100_000, Duration::from_secs(3600)), + register_per_ip: Limit::new(100_000, Duration::from_secs(3600)), + login_per_ip: Limit::new(100_000, Duration::from_secs(3600)), + ..Limits::default() + }; + let accounts = Accounts::new(db.pool().clone(), secrets) + .with_limits(limits) + .with_signup(orbyn_core::SignupMode::Open); let hub = Hub::start(db.pool().clone()); let store = Store::new(db.pool().clone(), hub); let state = AppState::new(Arc::new(Up)) @@ -86,12 +89,9 @@ impl Server { .await .expect("bind"); let address = listener.local_addr().expect("address"); - let service = app - .clone() - .into_make_service_with_connect_info::(); + let service = app.into_make_service_with_connect_info::(); tokio::spawn(async move { axum::serve(listener, service).await }); Self { - app, db, base: format!("http://{address}"), } @@ -109,56 +109,6 @@ impl Server { }, ) } - - /// A call straight into the app (no socket), for the approval steps. - async fn call( - &self, - method: Method, - path: &str, - token: &str, - body: Option, - ) -> (StatusCode, Value) { - let mut request = Request::builder() - .method(method) - .uri(path) - .header(header::AUTHORIZATION, format!("Bearer {token}")); - let body = match body { - Some(value) => { - request = request.header(header::CONTENT_TYPE, "application/json"); - Body::from(value.to_string()) - } - None => Body::empty(), - }; - let mut request = request.body(body).expect("request"); - request.extensions_mut().insert(ConnectInfo(SocketAddr::new( - IpAddr::V4(Ipv4Addr::LOCALHOST), - 40_000, - ))); - let response = self.app.clone().oneshot(request).await.expect("response"); - let status = response.status(); - let bytes = to_bytes(response.into_body(), 16 << 20) - .await - .expect("body"); - let json = if bytes.is_empty() { - Value::Null - } else { - serde_json::from_slice(&bytes).expect("json") - }; - (status, json) - } - - async fn ok( - &self, - method: Method, - path: &str, - token: &str, - body: Option, - expected: StatusCode, - ) -> T { - let (status, value) = self.call(method, path, token, body).await; - assert_eq!(status, expected, "{path}: {value}"); - serde_json::from_value(value).expect("wire type") - } } type Client = DeviceClient; @@ -171,7 +121,11 @@ fn email() -> String { format!("device-{}@example.test", Uuid::new_v4().simple()) } -async fn sign_up(server: &Server, client: &Client, email: &str) { +async fn sign_up( + server: &Server, + client: &DeviceClient, + email: &str, +) { let mut pending = client .begin_sign_up(SignUpRequest { email: email.to_owned(), @@ -207,119 +161,39 @@ fn now_secs() -> i64 { .map_or(0, |d| i64::try_from(d.as_secs()).unwrap_or(0)) } -fn value(v: &T) -> Value { - serde_json::to_value(v).expect("json") -} - -/// The whole two-way approval over HTTP, with the person confirming that both screens show the -/// same digits (the real client API for this is the next change). -async fn approve( - server: &Server, - (approver_token, approver): (&str, &mut Engine), - (new_token, new, new_id): (&str, &mut Engine, &str), -) { - let list: wire::DeviceList = server - .ok( - Method::GET, - "/v1/devices", - approver_token, - None, - StatusCode::OK, - ) - .await; - let pending = list - .devices - .iter() - .find(|d| d.id == new_id) - .expect("the new device is listed") - .clone(); - let (mut pa, create) = approver - .start_approval(&pending, now_secs()) +/// The whole two-way approval through the client API, with the person confirming that both +/// screens show the same digits. +async fn approve(a: &Client, engine_a: &mut Engine, b: &Client, engine_b: &mut Engine) { + let mut approval = a + .start_approval(engine_a, &device_id_of(b)) + .await .expect("start"); - let created: wire::ApprovalCreated = server - .ok( - Method::POST, - "/v1/devices/approvals", - approver_token, - Some(value(&create)), - StatusCode::CREATED, - ) - .await; - let step = |name: &str| format!("/v1/devices/approvals/{}/{name}", created.id); - let view = current_approval(server, new_token).await; - // These devices named their app, so each joins with the description it signed in with - // (the honest server lists the same name). - let (pd, nonce) = new - .join_approval_labelled( - &view, - now_secs(), - &pending.name, - wire::DevicePlatform::Linux, - wire::DeviceApp::Engineering, - ) - .expect("join"); - let (status, body) = server - .call(Method::POST, &step("nonce"), new_token, Some(value(&nonce))) - .await; - assert_eq!(status, StatusCode::NO_CONTENT, "{body}"); - let on_approver: wire::PendingApproval = server - .ok( - Method::GET, - &format!("/v1/devices/approvals/{}", created.id), - approver_token, - None, - StatusCode::OK, - ) - .await; - let reveal = pa.reveal(&on_approver).expect("reveal"); - let (status, body) = server - .call( - Method::POST, - &step("reveal"), - approver_token, - Some(value(&reveal)), - ) - .await; - assert_eq!(status, StatusCode::NO_CONTENT, "{body}"); - let view = current_approval(server, new_token).await; - let on_new = pd.sas(&view).expect("sas"); - assert_eq!(pa.sas(), Some(on_new), "both screens show the same digits"); - let confirm = approver.confirm_approval(pa, &list).expect("confirm"); - let (status, body) = server - .call( - Method::POST, - &step("confirm"), - approver_token, - Some(value(&confirm)), - ) - .await; - assert_eq!(status, StatusCode::NO_CONTENT, "{body}"); - let view = current_approval(server, new_token).await; - let code = new - .finish_approval(pd, &view, None, now_secs()) + let joined = b + .join_approval(engine_b) + .await + .expect("join") + .expect("the approval is waiting for the new device"); + assert_eq!( + b.device_digits(&joined).await.expect("early"), + None, + "no digits before the approver reveals" + ); + let shown = match a.approval_state(&mut approval).await.expect("state") { + ApproverState::Digits { digits, .. } => digits, + ApproverState::Waiting => panic!("the new device joined"), + }; + let on_new = b + .device_digits(&joined) + .await + .expect("digits") + .expect("the approver revealed"); + assert_eq!(shown, on_new, "both screens show the same digits"); + a.confirm_approval(engine_a, approval) + .await + .expect("confirm"); + b.finish_approval(engine_b, joined, None) + .await .expect("finish"); - let (status, body) = server - .call( - Method::POST, - &step("code"), - new_token, - Some(json!({"code": code})), - ) - .await; - assert_eq!(status, StatusCode::OK, "{body}"); -} - -async fn current_approval(server: &Server, token: &str) -> wire::PendingApproval { - let current: wire::CurrentDevice = server - .ok( - Method::GET, - "/v1/devices/current", - token, - None, - StatusCode::OK, - ) - .await; - current.approval.expect("an open approval") } /// One field set on a record, stamped by the engine's clock. @@ -396,12 +270,7 @@ async fn two_devices_sync_records_keys_and_a_conflict_through_the_real_server() ); // A approves B and hands it the zone key; B's pass takes the key and pulls the record. - approve( - &server, - (&token_of(&a), &mut engine_a), - (&token_of(&b), &mut engine_b, &device_id_of(&b)), - ) - .await; + approve(&a, &mut engine_a, &b, &mut engine_b).await; // Trusted but without the key yet, B stages work: its pass reports it is waiting for the key // and fails nothing; the work stays staged. set( @@ -574,12 +443,7 @@ async fn the_notices_stream_tells_a_device_that_another_changed_a_zone() { json!("dark"), ); a.sync_all(&mut engine_a).await.expect("pass"); - approve( - &server, - (&token_of(&a), &mut engine_a), - (&token_of(&b), &mut engine_b, &device_id_of(&b)), - ) - .await; + approve(&a, &mut engine_a, &b, &mut engine_b).await; let devices = a.devices().await.expect("devices"); a.send_keys(&mut engine_a, ZONE, &devices) .await @@ -645,12 +509,7 @@ async fn two_trusted_devices(server: &Server) -> (Client, Engine, Client, Engine json!("dark"), ); a.sync_all(&mut engine_a).await.expect("pass"); - approve( - server, - (&token_of(&a), &mut engine_a), - (&token_of(&b), &mut engine_b, &device_id_of(&b)), - ) - .await; + approve(&a, &mut engine_a, &b, &mut engine_b).await; let devices = a.devices().await.expect("devices"); a.send_keys(&mut engine_a, ZONE, &devices) .await @@ -791,3 +650,266 @@ async fn a_zone_made_after_the_approval_reaches_the_other_device_with_its_key() Some(json!("fast")) ); } + +// ---- approving, denying and removing devices through the client API --------------------------- + +/// A signed-up trusted device A and a pending device B of one account, each with an engine. +async fn pending_pair(server: &Server) -> (Client, Engine, Client, Engine) { + let email = email(); + let a = server.device("Laptop"); + sign_up(server, &a, &email).await; + let b = server.device("Phone"); + b.sign_in(&email, &password()).await.expect("sign in"); + let engine_a = a.open_engine(None).expect("engine"); + let engine_b = b.open_engine(None).expect("engine"); + (a, engine_a, b, engine_b) +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn either_device_can_deny_and_the_approval_ends() { + let server = Server::start().await; + let (a, engine_a, b, engine_b) = pending_pair(&server).await; + // The approver denies after the new device joined. + let mut approval = a + .start_approval(&engine_a, &device_id_of(&b)) + .await + .expect("start"); + assert_eq!( + a.approval_state(&mut approval).await.expect("state"), + ApproverState::Waiting + ); + let joined = b + .join_approval(&engine_b) + .await + .expect("join") + .expect("an approval"); + a.deny_approval(approval).await.expect("deny"); + assert_eq!( + b.device_digits(&joined).await.err(), + Some(DeviceError::ApprovalExpired) + ); + assert_eq!(b.pending_approval().await.expect("current"), None); + + // The new device denies before the approver reveals. + let mut approval = a + .start_approval(&engine_a, &device_id_of(&b)) + .await + .expect("start again"); + let joined = b + .join_approval(&engine_b) + .await + .expect("join") + .expect("an approval"); + b.deny_joined(joined).await.expect("deny"); + assert_eq!( + a.approval_state(&mut approval).await.err(), + Some(DeviceError::ApprovalExpired) + ); +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn wrong_digits_typed_on_the_new_device_trust_nothing_and_end_the_approval() { + let server = Server::start().await; + let (a, mut engine_a, b, mut engine_b) = pending_pair(&server).await; + let mut approval = a + .start_approval(&engine_a, &device_id_of(&b)) + .await + .expect("start"); + let joined = b + .join_approval(&engine_b) + .await + .expect("join") + .expect("an approval"); + let ApproverState::Digits { digits, .. } = + a.approval_state(&mut approval).await.expect("state") + else { + panic!("the new device joined"); + }; + let wrong = if digits == "000000" { + "000001" + } else { + "000000" + }; + a.confirm_approval(&mut engine_a, approval) + .await + .expect("confirm"); + assert_eq!( + b.finish_approval(&mut engine_b, joined, Some(wrong)) + .await + .err(), + Some(DeviceError::ApprovalMismatch) + ); + // Nothing was pinned and the approval is gone: a new one is needed. + assert_eq!(b.pending_approval().await.expect("current"), None); +} + +/// A transport that shows the approver another name for the new device. +struct Lying { + inner: HttpApi, +} + +impl DeviceApi for Lying { + async fn send( + &self, + request: orbyn_device_client::Request, + ) -> Result { + let list = request.path == "/v1/devices"; + let mut response = self.inner.send(request).await?; + if list { + let text = String::from_utf8_lossy(&response.body).replace("\"Phone\"", "\"Tablet\""); + response.body = Zeroizing::new(text.into_bytes()); + } + Ok(response) + } + + fn base_url(&self) -> Option<&str> { + self.inner.base_url() + } +} + +/// A transport that answers an approval's creation with another approval's id. +struct WrongId { + inner: HttpApi, +} + +impl DeviceApi for WrongId { + async fn send( + &self, + request: orbyn_device_client::Request, + ) -> Result { + let create = request.path == "/v1/devices/approvals"; + let mut response = self.inner.send(request).await?; + if create { + let mut body: Value = serde_json::from_slice(&response.body).expect("json"); + body["id"] = json!("00000000-0000-4000-8000-000000000001"); + response.body = Zeroizing::new(body.to_string().into_bytes()); + } + Ok(response) + } +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn an_approval_the_server_answers_with_another_id_is_refused() { + let server = Server::start().await; + let email = email(); + let a = DeviceClient::new( + WrongId { + inner: HttpApi::new(&server.base).expect("loopback"), + }, + MemoryStore::new(), + SystemClock, + ClientConfig { + device_name: "Laptop".to_owned(), + platform: wire::DevicePlatform::Linux, + app: None, + }, + ); + sign_up(&server, &a, &email).await; + let b = server.device("Phone"); + b.sign_in(&email, &password()).await.expect("sign in"); + let engine_a = a.open_engine(None).expect("engine"); + assert_eq!( + a.start_approval(&engine_a, &device_id_of(&b)).await.err(), + Some(DeviceError::ApprovalMismatch) + ); +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn an_approver_shown_another_description_cannot_get_past_the_servers_commitment_check() { + let server = Server::start().await; + let email = email(); + let a = DeviceClient::new( + Lying { + inner: HttpApi::new(&server.base).expect("loopback"), + }, + MemoryStore::new(), + SystemClock, + ClientConfig { + device_name: "Laptop".to_owned(), + platform: wire::DevicePlatform::Linux, + app: Some(wire::DeviceApp::Engineering), + }, + ); + sign_up(&server, &a, &email).await; + let b = server.device("Phone"); + b.sign_in(&email, &password()).await.expect("sign in"); + let engine_a = a.open_engine(None).expect("engine"); + let engine_b = b.open_engine(None).expect("engine"); + + // The approver was shown "Tablet", so its commitment covers that name. The server rebuilds + // the commitment from the stored "Phone" and refuses the reveal, ending the approval. (A + // server that did not check would still be caught by the new device: see the engine tests.) + let mut approval = a + .start_approval(&engine_a, &device_id_of(&b)) + .await + .expect("start"); + let joined = b + .join_approval(&engine_b) + .await + .expect("join") + .expect("an approval"); + assert_eq!( + a.approval_state(&mut approval).await.err(), + Some(DeviceError::ApprovalMismatch) + ); + assert_eq!( + b.device_digits(&joined).await.err(), + Some(DeviceError::ApprovalExpired) + ); +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn a_trusted_device_removes_another_and_the_last_one_stays() { + let server = Server::start().await; + let (a, mut engine_a, b, mut engine_b) = two_trusted_devices(&server).await; + let b_id = device_id_of(&b); + a.remove_device(&engine_a, &b_id).await.expect("remove"); + let listed = a.devices().await.expect("devices"); + assert!(listed.devices.iter().all(|d| d.id != b_id), "{listed:?}"); + assert_eq!(listed.removals.len(), 1); + // The removed device is signed out. + assert_eq!(b.devices().await.err(), Some(DeviceError::SessionExpired)); + // The last trusted device cannot remove itself. + assert_eq!( + a.remove_device(&engine_a, &device_id_of(&a)).await.err(), + Some(DeviceError::LastTrustedDevice) + ); + assert_eq!( + a.remove_device(&engine_a, "00000000-0000-4000-8000-000000000000") + .await + .err(), + Some(DeviceError::InvalidInput( + "the device is not in the account" + )) + ); + let _ = (&mut engine_a, &mut engine_b); +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn a_device_whose_name_has_spaces_is_approved_with_matching_digits() { + let server = Server::start().await; + let email = email(); + let a = server.device("Laptop"); + sign_up(&server, &a, &email).await; + // The host passes the name as the person typed it; the server stores it trimmed, so the new + // device must bind the trimmed name into its digits or they would never match. + let b = server.device(" Phone "); + b.sign_in(&email, &password()).await.expect("sign in"); + let mut engine_a = a.open_engine(None).expect("engine"); + let mut engine_b = b.open_engine(None).expect("engine"); + approve(&a, &mut engine_a, &b, &mut engine_b).await; + let listed = a.devices().await.expect("devices"); + let phone = listed + .devices + .iter() + .find(|d| d.id == device_id_of(&b)) + .expect("listed"); + assert_eq!(phone.name, "Phone"); + assert_eq!(phone.state, wire::DeviceState::Trusted); +} diff --git a/docs/device-client.md b/docs/device-client.md index b0fa56e..7799b56 100644 --- a/docs/device-client.md +++ b/docs/device-client.md @@ -6,7 +6,7 @@ It ships in three pull requests: 1. **Sign-up and sign-in** (merged). The design, the crate skeleton and sign-up, sign-in, the re-login and sign-out. 2. **Sync** (this document's second half). Push and pull per zone, zone keys in and out, the notices stream and the status watch. -3. **Approval and the label.** Approval digits, approve and deny, `devices()`, `remove_device`, and the device `app` label bound into the signed register message (engine#73). +3. **Approval and the label.** The `app` label bound into the signed register and login messages (merged), the description bound into the approval (merged), and the client API for approval digits, approve and deny, `devices()` and `remove_device` (this document's last section). ## What the host supplies @@ -56,12 +56,16 @@ Removing a device, turning the second factor off or starting it, changing protec | --- | --- | | `Transport` | No answer: the network failed or timed out | | `ReauthenticationRequired` | 401 `reauthentication_required`: call `reauthenticate` and retry | -| `InvalidCode` | 400 `invalid_email_code` | +| `InvalidCode` | 400 `invalid_email_code` or `wrong_code` (the approval digits) | | `RateLimited { retry_after }` | 429, with the `Retry-After` seconds when sent | | `DeletionScheduled` | 409 `deletion_scheduled` | | `AccountGone` | A session call got 401 **and** `POST /v1/accounts/status` answered 200 with a deletion time. Nothing else gives it: the app destroys key material on this error | | `SessionExpired` | 401 `unauthorized` and the account is alive, or the status call failed or answered anything but 200 | | `TooLarge` | 413 | +| `ApprovalExpired` | 410 `approval_expired`, or the approval is gone (cancelled, used or older than ten minutes) | +| `ApprovalNotReady` | 409 `approval_out_of_order` or `approval_not_confirmed`: the other device has not done its step yet | +| `ApprovalMismatch` | The engine refused the approval (other keys, a reveal that does not match its commitment, other digits, a bad signature) or the server refused the commitment: the approval is cancelled | +| `LastTrustedDevice` | 409 `last_trusted_device` | | `LoginFailed`, `InviteInvalid`, `SecondFactorFailed`, `DeviceNotTrusted` | the matching server code | | `NotSignedIn`, `StoreNotEmpty`, `InvalidInput`, `Store`, `Crypto` | local conditions | | `Server { status, code }` | anything else, carrying only the status and the machine code | @@ -95,10 +99,22 @@ The records go in and out as plain fields (`orbyn_sync::Fields`: a value and a s `notices(config)` opens the live stream (`/v1/sync`, a WebSocket over the same address: `wss` for `https`, and `ws` **only for a loopback IP literal**, so a transport that reports some other `http://` address gets no stream; rustls with ring and the web roots). Frames and messages are capped at 16 KiB (a notice is a few hundred bytes), the connect and handshake have a timeout (ten seconds), and the token is written into a wiped buffer for the sign-in message. It signs in first with the session token, then tells the host: `Connected` (compare every zone now: changes made while disconnected are not replayed), `ZoneChanged { zone, token }`, `Resync`, `Disconnected`, `Rejected { code }`. A notice never carries record data; the host answers one with a pass. The library answers the server's pings as it reads; a connection that hears nothing for the idle timeout (two minutes) is dropped. The stream **reconnects by itself** with a delay that doubles from one second to one minute, with up to a fifth of jitter, and starts again from the first delay once the server says `Ready`. `Rejected { code: "unauthorized" }` stops it: the session is gone, so `reauthenticate` and open a new one. Dropping the handle closes the connection. +## Approving devices + +The client only moves messages between the engine and the server, in order. Every signature, the digits and every check are the engine's (`Engine::start_approval`, `join_approval[_labelled]`, `confirm_approval`, `finish_approval`, `sign_removal`); the client signs nothing itself. + +**The approver** (a trusted device): `start_approval(engine, device_id)` lists the devices, lets the engine refuse a device it trusts or revoked, and opens the approval. `approval_state(&mut approval)` is asked until it answers `Digits { digits, fingerprint }`: it answers `Waiting` until the new device joined, then reveals this device's nonce once (a server that shows other keys than this device started with cancels the approval: `ApprovalMismatch`). The person compares the digits with the new device. Same: `confirm_approval(engine, approval)` fetches the current device list, signs the approval and the endorsement and sends them (the host then calls `send_keys` for each zone); different or cancelled: `deny_approval`. + +**The new device**: `pending_approval()` shows an approval when a trusted device started one. `join_approval(engine)` sends this device's nonce; a device that named its app (`ClientConfig.app`) joins with the name, platform and app it signed in with, so its digits match the approver's only if the approver was shown the same description. `device_digits(&joined)` answers `None` until the approver revealed, then the digits (or `ApprovalMismatch`, cancelling). `finish_approval(engine, joined, typed)` is called when the person confirms or types the approver's digits: the engine checks the reveal, the age, the approver's signature and the endorsement and pins the approver and the devices it endorsed, and only then are the digits sent to the server, once. On any failure nothing new is trusted, the approval is cancelled, and a new one is needed. `deny_joined` cancels. + +**Removing**: `remove_device(engine, device_id)` signs the removal with a sequence one above every removal the engine counted. A password sign-in older than ten minutes is `ReauthenticationRequired`; the last trusted device is `LastTrustedDevice`. + +The label is the device's own signed claim, never proof of which app is running. + ### Not here yet -The approval flow itself (digits, approve and deny, `remove_device`) and the device `app` label are the next change; the integration test does the approval over HTTP with the engine's own pairing calls until then. The account's protection chain is not signed by the client; the host's first protection setting follows the rules in [architecture.md](architecture.md). +The account's protection chain is not signed by the client; the host's first protection setting follows the rules in [architecture.md](architecture.md). ### Tests -`orbyn-server/tests/device_client.rs` runs the client over a real loopback socket against the real server on Postgres: two devices, the zone key, a sealed record that the server holds only as ciphertext, a conflict merged both ways, the snapshot round trip, a pending device refused, offline work kept and sent later, and the notices stream hearing another device's change and being rejected when the session is revoked. `tests/notices.rs` serves the stream by hand to check the sign-in message, the pong, the reconnect and the stop. +`orbyn-server/tests/device_client.rs` runs the client over a real loopback socket against the real server on Postgres: two devices, the zone key, a sealed record that the server holds only as ciphertext, a conflict merged both ways, the snapshot round trip, a pending device refused, the whole approval through the client API (digits compared, confirmed and finished), a deny from either side, wrong digits typed, a relabelled listing refused by the server's commitment check, a removal and the last device that stays, offline work kept and sent later, and the notices stream hearing another device's change and being rejected when the session is revoked. `tests/notices.rs` serves the stream by hand to check the sign-in message, the pong, the reconnect and the stop.