From be4897276cd4e95fdd87c47f953b30a98f894757 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 14:05:10 -0400 Subject: [PATCH] Android sharing sends the opened device token, not the sealed one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android has stored its device token sealed ("sealed:…") since 8592b83, and core's sharing calls read the token from the store themselves. The ffi opened it in credentials() and then threw the result away, so every Share-sheet request went out as `Bearer sealed:…` and the server refused it. The sharing functions now take the server address and token from the caller. The ffi passes what credentials() opened; the desktop, which stores its token plain, reads it through sharing::stored_link. A new ffi test serves one request on a loopback port and checks the bearer token that arrives (#5381). Co-Authored-By: Claude Opus 5.5 --- android/ffi/src/lib.rs | 99 +++++++++++++++++++++++--- core/src/sync/sharing.rs | 44 ++++++++---- desktop/src-tauri/src/commands/sync.rs | 12 ++-- 3 files changed, 128 insertions(+), 27 deletions(-) diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index e7a2aad..775f144 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -627,11 +627,14 @@ impl Inkwell { // // The Share dialog asks the server directly (#5175). Unlinked, each answers // `NotLinked`, which the dialog turns into "sharing needs a server". + // + // Each passes the token `credentials()` OPENED. The stored one is sealed, and the + // server refuses `sealed:…` (#5381). /// Everyone on the instance a note can be shared with, and every group. pub async fn share_directory(&self) -> Result { - self.credentials()?; - let directory = sharing::directory(&self.db) + let (url, token) = self.credentials()?; + let directory = sharing::directory(&url, &token) .await .map_err(CoreError::network)?; Ok(directory.into()) @@ -639,8 +642,8 @@ impl Inkwell { /// Who this note is shared with. pub async fn note_shares(&self, note_id: String) -> Result, CoreError> { - self.credentials()?; - let shares = sharing::list(&self.db, ¬e_id) + let (url, token) = self.credentials()?; + let shares = sharing::list(&self.db, &url, &token, ¬e_id) .await .map_err(CoreError::network)?; Ok(shares.into_iter().map(NoteShare::from).collect()) @@ -653,10 +656,17 @@ impl Inkwell { target: ShareTarget, permission: String, ) -> Result, CoreError> { - self.credentials()?; - let shares = sharing::share(&self.db, ¬e_id, &target.into(), &permission) - .await - .map_err(CoreError::network)?; + let (url, token) = self.credentials()?; + let shares = sharing::share( + &self.db, + &url, + &token, + ¬e_id, + &target.into(), + &permission, + ) + .await + .map_err(CoreError::network)?; Ok(shares.into_iter().map(NoteShare::from).collect()) } @@ -665,8 +675,8 @@ impl Inkwell { note_id: String, share_id: String, ) -> Result, CoreError> { - self.credentials()?; - let shares = sharing::unshare(&self.db, ¬e_id, &share_id) + let (url, token) = self.credentials()?; + let shares = sharing::unshare(&self.db, &url, &token, ¬e_id, &share_id) .await .map_err(CoreError::network)?; Ok(shares.into_iter().map(NoteShare::from).collect()) @@ -1188,6 +1198,75 @@ mod tests { std::fs::remove_dir_all(&dir).ok(); } + /// A seal that works, as Android's Keystore one does: reversal is enough to tell + /// a sealed token from an opened one. + struct Reverse; + + impl TokenSeal for Reverse { + fn seal_token(&self, token: String) -> Option { + Some(token.chars().rev().collect()) + } + fn open_token(&self, sealed: String) -> Option { + Some(sealed.chars().rev().collect()) + } + } + + /// Serve one request on a loopback port with `body` as JSON, and hand back the + /// request as it arrived, headers included. + fn one_reply(body: &'static str) -> (String, std::thread::JoinHandle) { + use std::io::{Read, Write}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind"); + let url = format!("http://{}", listener.local_addr().expect("addr")); + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept"); + let mut request = Vec::new(); + let mut buf = [0u8; 4096]; + while !request.windows(4).any(|w| w == b"\r\n\r\n") { + let n = stream.read(&mut buf).expect("read"); + if n == 0 { + break; + } + request.extend_from_slice(&buf[..n]); + } + let len = body.len(); + let head = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {len}\r\n" + ); + write!(stream, "{head}Connection: close\r\n\r\n{body}").expect("reply"); + String::from_utf8_lossy(&request).into_owned() + }); + (url, handle) + } + + /// #5381: the token is stored sealed, so a share call has to send the OPENED one. + /// It used to read the store itself and send `sealed:…`, which the server refused. + #[test] + fn sharing_sends_the_opened_token_not_the_stored_one() { + let (url, server) = one_reply(r#"{"members":[],"groups":[]}"#); + let dir = scratch_dir(); + let app = Inkwell::new(dir.clone(), Arc::new(Reverse)).expect("open"); + app.store_link(&url, "tok-1", None).expect("link"); + + let stored = { + let conn = app.db.conn().expect("conn"); + state::read(&conn).expect("read").device_token + }; + assert_eq!(stored.as_deref(), Some("sealed:1-kot"), "stored sealed"); + + tokio::runtime::Runtime::new() + .expect("runtime") + .block_on(app.share_directory()) + .expect("directory"); + + let request = server.join().expect("server").to_ascii_lowercase(); + assert!( + request.contains("authorization: bearer tok-1\r\n"), + "sent the wrong token:\n{request}" + ); + + std::fs::remove_dir_all(&dir).ok(); + } + /// A crude RFC3339 sanity check that doesn't pull a date crate into this /// crate's dev-dependencies to assert one field is well-formed. fn chrono_free_parse(raw: &str) -> usize { diff --git a/core/src/sync/sharing.rs b/core/src/sync/sharing.rs index 301aea5..38df0f9 100644 --- a/core/src/sync/sharing.rs +++ b/core/src/sync/sharing.rs @@ -5,6 +5,11 @@ //! device token. The one thing kept locally is the note's `shared` flag, set from //! the server's answer so the card's chip changes at once rather than at the next //! sync (which brings the same value). +//! +//! The server address and token come from the CALLER, never from the store here. +//! A stored token may be sealed (Android keeps it under a Keystore key, see +//! `state::TokenSeal`), and only the caller holds the seal that opens it. Reading it +//! here sent `sealed:…` as the bearer token, and every share call was refused (#5381). use rusqlite::params; @@ -17,7 +22,11 @@ use crate::local::Db; pub const NEEDS_SERVER: &str = "Sharing is between people on a server. Link this device to one in Sync to share notes."; -fn link(db: &Db) -> Result<(String, String), String> { +/// The server address and token AS STORED, or [`NEEDS_SERVER`]. +/// +/// Only for a client that stores its token plain, as the desktop does. A client +/// with a seal opens the token itself (`state::open_token`) and passes that. +pub fn stored_link(db: &Db) -> Result<(String, String), String> { let conn = db.conn()?; let link = state::read(&conn).map_err(|e| e.to_string())?; match (link.server_url, link.device_token) { @@ -38,33 +47,42 @@ fn mark_shared(db: &Db, note_id: &str, shares: &[NoteShare]) -> Result<(), Strin Ok(()) } -pub async fn directory(db: &Db) -> Result { - let (url, token) = link(db)?; - client::directory(&url, &token).await +pub async fn directory(url: &str, token: &str) -> Result { + client::directory(url, token).await } -pub async fn list(db: &Db, note_id: &str) -> Result, String> { - let (url, token) = link(db)?; - let shares = client::list_shares(&url, &token, note_id).await?; +pub async fn list( + db: &Db, + url: &str, + token: &str, + note_id: &str, +) -> Result, String> { + let shares = client::list_shares(url, token, note_id).await?; mark_shared(db, note_id, &shares)?; Ok(shares) } pub async fn share( db: &Db, + url: &str, + token: &str, note_id: &str, target: &ShareTarget, permission: &str, ) -> Result, String> { - let (url, token) = link(db)?; - let shares = client::share_note(&url, &token, note_id, target, permission).await?; + let shares = client::share_note(url, token, note_id, target, permission).await?; mark_shared(db, note_id, &shares)?; Ok(shares) } -pub async fn unshare(db: &Db, note_id: &str, share_id: &str) -> Result, String> { - let (url, token) = link(db)?; - let shares = client::unshare_note(&url, &token, note_id, share_id).await?; +pub async fn unshare( + db: &Db, + url: &str, + token: &str, + note_id: &str, + share_id: &str, +) -> Result, String> { + let shares = client::unshare_note(url, token, note_id, share_id).await?; mark_shared(db, note_id, &shares)?; Ok(shares) } @@ -85,7 +103,7 @@ mod tests { #[test] fn an_unlinked_device_explains_that_sharing_needs_a_server() { - assert_eq!(link(&db()).unwrap_err(), NEEDS_SERVER); + assert_eq!(stored_link(&db()).unwrap_err(), NEEDS_SERVER); } #[test] diff --git a/desktop/src-tauri/src/commands/sync.rs b/desktop/src-tauri/src/commands/sync.rs index 7c38f26..7663934 100644 --- a/desktop/src-tauri/src/commands/sync.rs +++ b/desktop/src-tauri/src/commands/sync.rs @@ -194,12 +194,14 @@ pub fn sync_has_pending(db: State<'_, Db>) -> Result { #[tauri::command] pub async fn shares_directory(db: State<'_, Db>) -> Result { - sharing::directory(&db).await + let (url, token) = sharing::stored_link(&db)?; + sharing::directory(&url, &token).await } #[tauri::command] pub async fn shares_list(note_id: String, db: State<'_, Db>) -> Result, String> { - sharing::list(&db, ¬e_id).await + let (url, token) = sharing::stored_link(&db)?; + sharing::list(&db, &url, &token, ¬e_id).await } /// Share with a person (`user_id`) or a group (`group_id`, #5177): exactly one. @@ -216,7 +218,8 @@ pub async fn shares_share( (None, Some(id)) => ShareTarget::Group(id), _ => return Err("Choose someone or a group to share with.".to_string()), }; - sharing::share(&db, ¬e_id, &target, &permission).await + let (url, token) = sharing::stored_link(&db)?; + sharing::share(&db, &url, &token, ¬e_id, &target, &permission).await } #[tauri::command] @@ -225,5 +228,6 @@ pub async fn shares_unshare( share_id: String, db: State<'_, Db>, ) -> Result, String> { - sharing::unshare(&db, ¬e_id, &share_id).await + let (url, token) = sharing::stored_link(&db)?; + sharing::unshare(&db, &url, &token, ¬e_id, &share_id).await }