sync: unlinking a device now revokes its token on the server (issue 2110)
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 6s
CI & Build / Python tests (push) Successful in 8s
CI & Build / Build & push image (push) Successful in 40s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m13s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m9s
Desktop (Tauri) / Update manifest (push) Successful in 4s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 6s
CI & Build / Python tests (push) Successful in 8s
CI & Build / Build & push image (push) Successful in 40s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m13s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m9s
Desktop (Tauri) / Update manifest (push) Successful in 4s
Unlink was local-only. It cleared the server URL, token and cursor from the device, and left the bearer token valid on the server indefinitely — so someone who unlinked because the laptop was being sold or handed on believed they had revoked access when they hadn't. The blocker was identification, not intent: a token pasted from the web app never carried a device id, and /api/auth/me describes the user, not the device row, so DELETE /devices/<id> could only ever have worked for one of the two ways this app can be linked. DELETE /api/auth/devices/self keys off the token in the Authorization header instead, which the caller always holds — one route that works for both paths, owner-scoped like the rest, and no local schema change. Unlinking is never blocked on the network. Wanting to stop syncing is a local decision, so the revoke is attempted first, its outcome carried back, and the link cleared either way. When the token survives — server unreachable, or older than the route — the Sync screen says so in place, with where to revoke it. A toast would have been the wrong shape for that: it disappears, and this is exactly what someone returns to the screen to check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -58,6 +58,64 @@ struct DeviceLoginResponse {
|
||||
user: Identity,
|
||||
}
|
||||
|
||||
/// What became of this device's token on the SERVER when unlinking.
|
||||
///
|
||||
/// Not a bool, and not an error: unlinking must never be blocked by the network —
|
||||
/// wanting to stop syncing is a local decision — so the remote half reports back
|
||||
/// instead of failing the call, and each outcome needs different advice.
|
||||
///
|
||||
/// Serialized tagged, like `Compatibility`, so the frontend can `switch` on `status`.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
|
||||
#[serde(tag = "status", rename_all = "snake_case")]
|
||||
pub enum RevokeOutcome {
|
||||
/// The server confirmed it: this token authenticates nothing now.
|
||||
Revoked,
|
||||
/// This server has no self-revoke route — it predates one. The token is still
|
||||
/// live, and only the web app can retire it.
|
||||
Unsupported,
|
||||
/// We couldn't reach the server, or it refused. The token is still live.
|
||||
Failed { reason: String },
|
||||
/// Nothing to revoke; the app wasn't linked.
|
||||
Skipped,
|
||||
}
|
||||
|
||||
/// Retire the device token we authenticate with, server-side.
|
||||
///
|
||||
/// Identified by the token itself rather than a device id, because a token pasted
|
||||
/// from the web app never carried one — a route keyed on the id would work for
|
||||
/// exactly one of the two ways this app can be linked.
|
||||
pub async fn revoke_self(base_url: &str, token: &str) -> RevokeOutcome {
|
||||
let client = match http() {
|
||||
Ok(client) => client,
|
||||
Err(reason) => return RevokeOutcome::Failed { reason },
|
||||
};
|
||||
let request = prepare(client.delete(revoke_self_url(base_url)), Some(token));
|
||||
let response = match request.send().await {
|
||||
Ok(response) => response,
|
||||
Err(e) => {
|
||||
return RevokeOutcome::Failed {
|
||||
reason: describe_transport_error(base_url, &e),
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let status = response.status();
|
||||
// 401 counts as revoked: the token already authenticates nothing — retired by
|
||||
// another device, or purged server-side — which is the state we were asking for.
|
||||
if status.is_success() || status == StatusCode::UNAUTHORIZED {
|
||||
return RevokeOutcome::Revoked;
|
||||
}
|
||||
match status {
|
||||
// No such route: a server older than self-revoke. Any other shape of 404
|
||||
// (a proxy, a stale base URL) leaves the token live too, so the advice the
|
||||
// user needs is the same either way.
|
||||
StatusCode::NOT_FOUND | StatusCode::METHOD_NOT_ALLOWED => RevokeOutcome::Unsupported,
|
||||
other => RevokeOutcome::Failed {
|
||||
reason: unexpected_status(base_url, other),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn http_with(timeout: Duration) -> Result<reqwest::Client, String> {
|
||||
reqwest::Client::builder()
|
||||
.timeout(timeout)
|
||||
@@ -300,6 +358,11 @@ fn me_url(base_url: &str) -> String {
|
||||
format!("{base_url}/api/auth/me")
|
||||
}
|
||||
|
||||
/// `self` rather than a device id: see `revoke_self`.
|
||||
fn revoke_self_url(base_url: &str) -> String {
|
||||
format!("{base_url}/api/auth/devices/self")
|
||||
}
|
||||
|
||||
/// Turn a transport failure into something a person can act on. reqwest's own
|
||||
/// Display is accurate but reads like a stack trace.
|
||||
fn describe_transport_error(base_url: &str, err: &reqwest::Error) -> String {
|
||||
@@ -341,6 +404,23 @@ mod tests {
|
||||
me_url("https://notes.example.com"),
|
||||
"https://notes.example.com/api/auth/me"
|
||||
);
|
||||
assert_eq!(
|
||||
revoke_self_url("https://notes.example.com"),
|
||||
"https://notes.example.com/api/auth/devices/self"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn revoke_outcome_serializes_tagged_for_the_frontend() {
|
||||
// The UI decides between "signed out on the server" and "still valid, go
|
||||
// revoke it" by reading this tag, so its shape is part of the contract.
|
||||
let json = serde_json::to_string(&RevokeOutcome::Failed {
|
||||
reason: "offline".into(),
|
||||
})
|
||||
.expect("outcome serializes");
|
||||
assert!(json.contains("\"status\":\"failed\""), "got {json}");
|
||||
let json = serde_json::to_string(&RevokeOutcome::Revoked).expect("outcome serializes");
|
||||
assert!(json.contains("\"status\":\"revoked\""), "got {json}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -112,18 +112,42 @@ pub async fn sync_link(input: LinkInput, db: State<'_, Db>) -> Result<LinkResult
|
||||
})
|
||||
}
|
||||
|
||||
/// Stop syncing and forget the server.
|
||||
#[derive(Serialize)]
|
||||
pub struct UnlinkResult {
|
||||
pub status: state::Status,
|
||||
/// What happened to the token on the SERVER — kept separate from `status`
|
||||
/// because the local half always succeeds and the remote half may not.
|
||||
pub revoked: client::RevokeOutcome,
|
||||
}
|
||||
|
||||
/// Stop syncing, and retire this device's token on the server.
|
||||
///
|
||||
/// Local only: the device token remains valid on the SERVER until revoked there
|
||||
/// (Account → Linked devices). We can't reliably revoke it from here — a pasted
|
||||
/// token arrives without its device id — so the UI must say so rather than imply a
|
||||
/// remote revoke that didn't happen. Tracked for follow-up.
|
||||
/// The local half is unconditional. Someone unlinking because the machine is being
|
||||
/// sold or handed on must not be held to it by a server that's offline or gone — so
|
||||
/// the revoke is attempted first, its outcome carried back for the UI to report
|
||||
/// honestly, and the link cleared either way.
|
||||
#[tauri::command]
|
||||
pub fn sync_unlink(db: State<'_, Db>) -> Result<state::Status, String> {
|
||||
pub async fn sync_unlink(db: State<'_, Db>) -> Result<UnlinkResult, String> {
|
||||
// Read and release before the network call: a std MutexGuard isn't Send, and
|
||||
// holding the store across a round-trip would freeze every note operation in
|
||||
// the UI.
|
||||
let link = {
|
||||
let conn = db.0.lock().map_err(|e| e.to_string())?;
|
||||
let current = state::read(&conn).map_err(|e| e.to_string())?;
|
||||
current.server_url.zip(current.device_token)
|
||||
};
|
||||
let revoked = match &link {
|
||||
Some((base_url, token)) => client::revoke_self(base_url, token).await,
|
||||
None => client::RevokeOutcome::Skipped,
|
||||
};
|
||||
|
||||
let conn = db.0.lock().map_err(|e| e.to_string())?;
|
||||
state::clear_link(&conn).map_err(|e| e.to_string())?;
|
||||
log::info!("unlinked from server");
|
||||
state::status(&conn).map_err(|e| e.to_string())
|
||||
log::info!("unlinked from server (server-side token: {revoked:?})");
|
||||
Ok(UnlinkResult {
|
||||
status: state::status(&conn).map_err(|e| e.to_string())?,
|
||||
revoked,
|
||||
})
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
|
||||
Reference in New Issue
Block a user