fix(auth): reconcile expired desktop sessions
This commit is contained in:
@@ -429,6 +429,9 @@ fn read_json_response<T: DeserializeOwned>(
|
||||
Ok(ok) => read_json_from_response(endpoint, ok),
|
||||
Err(ureq::Error::Status(status, raw)) => {
|
||||
let body = raw.into_string().unwrap_or_default();
|
||||
if session::response_ends_session(status, &body) {
|
||||
return Err(SyncClientError::SessionEnded);
|
||||
}
|
||||
Err(SyncClientError::HttpStatus { endpoint: endpoint.to_string(), status, body })
|
||||
}
|
||||
Err(other) => {
|
||||
|
||||
@@ -35,7 +35,7 @@ impl SyncApiClient {
|
||||
}
|
||||
Err(ureq::Error::Status(status, response)) => {
|
||||
let body = response.into_string().unwrap_or_default();
|
||||
if status == 401 && logout_is_already_complete(&body) {
|
||||
if response_ends_session(status, &body) {
|
||||
return Ok(());
|
||||
}
|
||||
Err(SyncClientError::HttpStatus { endpoint, status, body })
|
||||
@@ -45,8 +45,32 @@ impl SyncApiClient {
|
||||
}
|
||||
}
|
||||
|
||||
fn logout_is_already_complete(body: &str) -> bool {
|
||||
serde_json::from_str::<AuthErrorDocument>(body).is_ok_and(|document| {
|
||||
matches!(document.error.as_str(), "session_not_found" | "session_expired")
|
||||
})
|
||||
pub(super) fn response_ends_session(status: u16, body: &str) -> bool {
|
||||
status == 401
|
||||
&& serde_json::from_str::<AuthErrorDocument>(body).is_ok_and(|document| {
|
||||
matches!(document.error.as_str(), "session_not_found" | "session_expired")
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::response_ends_session;
|
||||
|
||||
#[test]
|
||||
fn terminal_session_parser_is_exact() {
|
||||
for body in [r#"{"error":"session_not_found"}"#, r#"{"error":"session_expired"}"#] {
|
||||
assert!(response_ends_session(401, body));
|
||||
}
|
||||
for (status, body) in [
|
||||
(401, r#"{"error":"authorization_missing"}"#),
|
||||
(401, r#"{"error":"authorization_invalid"}"#),
|
||||
(401, r#"{"error":"unknown"}"#),
|
||||
(401, r#"{"error":"session_not_found","extra":true}"#),
|
||||
(401, "{"),
|
||||
(403, r#"{"error":"session_not_found"}"#),
|
||||
(500, r#"{"error":"session_expired"}"#),
|
||||
] {
|
||||
assert!(!response_ends_session(status, body));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,6 +89,46 @@ fn sign_out_preserves_retryable_failures() -> Result<(), Box<dyn Error>> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn runtime_requests_classify_only_strict_terminal_sessions() -> Result<(), Box<dyn Error>> {
|
||||
for code in ["session_not_found", "session_expired"] {
|
||||
let body = format!(r#"{{"error":"{code}"}}"#);
|
||||
let (base_url, server) =
|
||||
spawn_authenticated_server("GET /api/devices HTTP/1.1\r\n", "401 Unauthorized", &body)?;
|
||||
let client = SyncApiClient::new(
|
||||
ApiClientConfig::custom(base_url, "auto"),
|
||||
BearerToken::new("a".repeat(64))?,
|
||||
)?;
|
||||
|
||||
assert!(matches!(client.list_devices(), Err(crate::SyncClientError::SessionEnded)));
|
||||
join_server(server)?;
|
||||
}
|
||||
|
||||
for (status_line, body, expected_status) in [
|
||||
("401 Unauthorized", r#"{"error":"authorization_missing"}"#, 401),
|
||||
("401 Unauthorized", r#"{"error":"authorization_invalid"}"#, 401),
|
||||
("401 Unauthorized", r#"{"error":"unknown"}"#, 401),
|
||||
("401 Unauthorized", r#"{"error":"session_not_found","extra":true}"#, 401),
|
||||
("401 Unauthorized", "{", 401),
|
||||
("403 Forbidden", r#"{"error":"session_not_found"}"#, 403),
|
||||
("500 Internal Server Error", r#"{"error":"session_expired"}"#, 500),
|
||||
] {
|
||||
let (base_url, server) =
|
||||
spawn_authenticated_server("GET /api/devices HTTP/1.1\r\n", status_line, body)?;
|
||||
let client = SyncApiClient::new(
|
||||
ApiClientConfig::custom(base_url, "auto"),
|
||||
BearerToken::new("a".repeat(64))?,
|
||||
)?;
|
||||
|
||||
assert!(matches!(
|
||||
client.list_devices(),
|
||||
Err(crate::SyncClientError::HttpStatus { status, .. }) if status == expected_status
|
||||
));
|
||||
join_server(server)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upload_parses_structured_snapshot_head_conflict() -> Result<(), Box<dyn Error>> {
|
||||
let (base_url, server) = spawn_conflict_server()?;
|
||||
@@ -179,6 +219,14 @@ fn spawn_conflict_server() -> Result<(String, TestServer), Box<dyn Error>> {
|
||||
fn spawn_logout_server(
|
||||
status_line: &'static str,
|
||||
body: &str,
|
||||
) -> Result<(String, TestServer), Box<dyn Error>> {
|
||||
spawn_authenticated_server("POST /api/session/logout HTTP/1.1\r\n", status_line, body)
|
||||
}
|
||||
|
||||
fn spawn_authenticated_server(
|
||||
expected_request_line: &'static str,
|
||||
status_line: &'static str,
|
||||
body: &str,
|
||||
) -> Result<(String, TestServer), Box<dyn Error>> {
|
||||
let listener = TcpListener::bind("127.0.0.1:0")?;
|
||||
let address = listener.local_addr()?;
|
||||
@@ -195,7 +243,7 @@ fn spawn_logout_server(
|
||||
request.extend_from_slice(&chunk[..read]);
|
||||
}
|
||||
let request = String::from_utf8_lossy(&request);
|
||||
if !request.starts_with("POST /api/session/logout HTTP/1.1\r\n")
|
||||
if !request.starts_with(expected_request_line)
|
||||
|| !request.contains(&format!("Authorization: Bearer {}\r\n", "a".repeat(64)))
|
||||
{
|
||||
return Err(std::io::Error::other("logout request contract mismatch"));
|
||||
|
||||
@@ -14,6 +14,12 @@ pub enum SyncClientError {
|
||||
#[error("Session logout response is invalid")]
|
||||
SessionLogoutInvalid,
|
||||
|
||||
#[error("Authenticated session ended")]
|
||||
SessionEnded,
|
||||
|
||||
#[error("Authenticated session changed during reconciliation")]
|
||||
SessionChanged,
|
||||
|
||||
#[error("HTTP request failed for {endpoint}: {source}")]
|
||||
Http {
|
||||
endpoint: String,
|
||||
|
||||
Reference in New Issue
Block a user