persist rotated refresh tokens

src/actions.rs

@@ -137,7 +137,7 @@ pub async fn sync(
     access_token: &str,
     refresh_token: &str,
 ) -> Result<(
-    Option<String>,
+    Option<RefreshedTokens>,
     (
         String,
         String,
@@ -175,7 +175,7 @@ pub fn add(
     data: &crate::db::EntryData,
     notes: Option<&str>,
     folder_id: Option<&str>,
-) -> Result<(Option<String>, ())> {
+) -> Result<(Option<RefreshedTokens>, ())> {
     with_exchange_refresh_token(access_token, refresh_token, |access_token| {
         add_once(access_token, name, data, notes, folder_id)
     })
@@ -204,7 +204,7 @@ pub fn edit(
     notes: Option<&str>,
     folder_uuid: Option<&str>,
     history: &[crate::db::HistoryEntry],
-) -> Result<(Option<String>, ())> {
+) -> Result<(Option<RefreshedTokens>, ())> {
     with_exchange_refresh_token(access_token, refresh_token, |access_token| {
         edit_once(
             access_token,
@@ -250,7 +250,7 @@ pub fn remove(
     access_token: &str,
     refresh_token: &str,
     id: &str,
-) -> Result<(Option<String>, ())> {
+) -> Result<(Option<RefreshedTokens>, ())> {
     with_exchange_refresh_token(access_token, refresh_token, |access_token| {
         remove_once(access_token, id)
     })
@@ -265,7 +265,7 @@ fn remove_once(access_token: &str, id: &str) -> Result<()> {
 pub fn list_folders(
     access_token: &str,
     refresh_token: &str,
-) -> Result<(Option<String>, Vec<(String, String)>)> {
+) -> Result<(Option<RefreshedTokens>, Vec<(String, String)>)> {
     with_exchange_refresh_token(access_token, refresh_token, |access_token| {
         list_folders_once(access_token)
     })
@@ -280,7 +280,7 @@ pub fn create_folder(
     access_token: &str,
     refresh_token: &str,
     name: &str,
-) -> Result<(Option<String>, String)> {
+) -> Result<(Option<RefreshedTokens>, String)> {
     with_exchange_refresh_token(access_token, refresh_token, |access_token| {
         create_folder_once(access_token, name)
     })
@@ -291,20 +291,30 @@ fn create_folder_once(access_token: &str, name: &str) -> Result<String> {
     client.create_folder(access_token, name)
 }
 
+/// Tokens returned by a successful refresh.
+///
+/// `refresh_token` is `None` against servers that do not rotate it, in which
+/// case the stored one stays valid and should be left alone.
+#[derive(Debug, Clone)]
+pub struct RefreshedTokens {
+    pub access_token: String,
+    pub refresh_token: Option<String>,
+}
+
 fn with_exchange_refresh_token<F, T>(
     access_token: &str,
     refresh_token: &str,
     f: F,
-) -> Result<(Option<String>, T)>
+) -> Result<(Option<RefreshedTokens>, T)>
 where
     F: Fn(&str) -> Result<T>,
 {
     match f(access_token) {
         Ok(t) => Ok((None, t)),
         Err(Error::RequestUnauthorized) => {
-            let access_token = exchange_refresh_token(refresh_token)?;
-            let t = f(&access_token)?;
-            Ok((Some(access_token), t))
+            let tokens = exchange_refresh_token(refresh_token)?;
+            let t = f(&tokens.access_token)?;
+            Ok((Some(tokens), t))
         }
         Err(e) => Err(e),
     }
@@ -314,7 +324,7 @@ async fn with_exchange_refresh_token_async<F, T>(
     access_token: &str,
     refresh_token: &str,
     f: F,
-) -> Result<(Option<String>, T)>
+) -> Result<(Option<RefreshedTokens>, T)>
 where
     F: Fn(
             &str,
@@ -327,23 +337,34 @@ where
     match f(access_token).await {
         Ok(t) => Ok((None, t)),
         Err(Error::RequestUnauthorized) => {
-            let access_token =
-                exchange_refresh_token_async(refresh_token).await?;
-            let t = f(&access_token).await?;
-            Ok((Some(access_token), t))
+            let tokens = exchange_refresh_token_async(refresh_token).await?;
+            let t = f(&tokens.access_token).await?;
+            Ok((Some(tokens), t))
         }
         Err(e) => Err(e),
     }
 }
 
-fn exchange_refresh_token(refresh_token: &str) -> Result<String> {
+fn exchange_refresh_token(refresh_token: &str) -> Result<RefreshedTokens> {
     let (client, _) = api_client()?;
-    client.exchange_refresh_token(refresh_token)
+    let (access_token, refresh_token) =
+        client.exchange_refresh_token(refresh_token)?;
+    Ok(RefreshedTokens {
+        access_token,
+        refresh_token,
+    })
 }
 
-async fn exchange_refresh_token_async(refresh_token: &str) -> Result<String> {
+async fn exchange_refresh_token_async(
+    refresh_token: &str,
+) -> Result<RefreshedTokens> {
     let (client, _) = api_client()?;
-    client.exchange_refresh_token_async(refresh_token).await
+    let (access_token, refresh_token) =
+        client.exchange_refresh_token_async(refresh_token).await?;
+    Ok(RefreshedTokens {
+        access_token,
+        refresh_token,
+    })
 }
 
 fn api_client() -> Result<(crate::api::Client, crate::config::Config)> {

src/api.rs

@@ -372,6 +372,12 @@ struct ConnectRefreshTokenReq {
 #[derive(serde::Deserialize, Debug)]
 struct ConnectRefreshTokenRes {
     access_token: String,
+    // Bitwarden's own identity server returns long-lived refresh tokens and
+    // does not rotate them, so this is absent there. A Vaultwarden backed by an
+    // external identity provider relays a rotating token, and the previous one
+    // is consumed by the exchange -- reusing it fails every subsequent refresh.
+    #[serde(default)]
+    refresh_token: Option<String>,
 }
 
 #[derive(serde::Serialize, Debug)]
@@ -1545,7 +1551,7 @@ impl Client {
     pub fn exchange_refresh_token(
         &self,
         refresh_token: &str,
-    ) -> Result<String> {
+    ) -> Result<(String, Option<String>)> {
         let connect_req = ConnectRefreshTokenReq {
             grant_type: "refresh_token".to_string(),
             client_id: "cli".to_string(),
@@ -1557,14 +1563,20 @@ impl Client {
             .form(&connect_req)
             .send()
             .map_err(|source| Error::Reqwest { source })?;
+        if !res.status().is_success() {
+            return Err(classify_refresh_error_res(
+                res.status().as_u16(),
+                res.text(),
+            ));
+        }
         let connect_res: ConnectRefreshTokenRes = res.json_with_path()?;
-        Ok(connect_res.access_token)
+        Ok((connect_res.access_token, connect_res.refresh_token))
     }
 
     pub async fn exchange_refresh_token_async(
         &self,
         refresh_token: &str,
-    ) -> Result<String> {
+    ) -> Result<(String, Option<String>)> {
         let connect_req = ConnectRefreshTokenReq {
             grant_type: "refresh_token".to_string(),
             client_id: "cli".to_string(),
@@ -1577,9 +1589,13 @@ impl Client {
             .send()
             .await
             .map_err(|source| Error::Reqwest { source })?;
+        if !res.status().is_success() {
+            let code = res.status().as_u16();
+            return Err(classify_refresh_error_res(code, res.text().await));
+        }
         let connect_res: ConnectRefreshTokenRes =
             res.json_with_path().await?;
-        Ok(connect_res.access_token)
+        Ok((connect_res.access_token, connect_res.refresh_token))
     }
 
     fn api_url(&self, path: &str) -> String {
@@ -1707,6 +1723,34 @@ fn sso_query_code(
     Ok(sso_code.clone())
 }
 
+/// Classify a failed response from the `refresh_token` grant.
+///
+/// `invalid_grant` is unambiguous here in a way it is not during login: no
+/// password or second factor is involved, so the only thing the server can be
+/// rejecting is the refresh token itself. It will never become valid again, so
+/// callers should stop retrying and require a fresh login.
+fn classify_refresh_error_res(
+    code: u16,
+    body: std::result::Result<String, reqwest::Error>,
+) -> Error {
+    let Ok(body) = body else {
+        return Error::RequestFailed { status: code };
+    };
+    match body.clone().json_with_path::<ConnectErrorRes>() {
+        Ok(json) => {
+            if json.error == "invalid_grant" {
+                Error::RefreshTokenInvalid
+            } else {
+                classify_login_error(&json, code)
+            }
+        }
+        Err(e) => {
+            log::warn!("{e}: {body}");
+            Error::RequestFailed { status: code }
+        }
+    }
+}
+
 fn classify_login_error(error_res: &ConnectErrorRes, code: u16) -> Error {
     let error_desc = error_res.error_description.clone();
     let error_desc = error_desc.as_deref();

src/bin/rbw-agent/actions.rs

@@ -522,14 +522,42 @@ pub async fn sync(
         return Err(anyhow::anyhow!("failed to find refresh token in db"));
     };
     let (
-        access_token,
+        tokens,
         (protected_key, protected_private_key, protected_org_keys, entries),
-    ) = rbw::actions::sync(&access_token, &refresh_token)
-        .await
-        .context("failed to sync database from server")?;
+    ) = match rbw::actions::sync(&access_token, &refresh_token).await {
+        Ok(res) => res,
+        // The server will never accept this refresh token again, so retrying
+        // on the sync interval only repeats the same rejection forever. Drop
+        // the tokens instead: that makes db.needs_login() true, so the next
+        // command the user runs goes through the ordinary login flow, and the
+        // syncs in between fail locally without touching the network.
+        Err(rbw::error::Error::RefreshTokenInvalid) => {
+            // Re-read rather than trusting the copy loaded above: a concurrent
+            // sync may have rotated the token while this request was in
+            // flight, in which case the server is rejecting a token that has
+            // already been superseded and the stored replacement is valid.
+            // Only clear if what we sent is still what is stored.
+            let mut current = load_db().await?;
+            if current.refresh_token.as_deref()
+                == Some(refresh_token.as_str())
+            {
+                current.access_token = None;
+                current.refresh_token = None;
+                save_db(&current).await?;
+            }
+            return Err(rbw::error::Error::RefreshTokenInvalid)
+                .context("failed to sync database from server");
+        }
+        Err(e) => {
+            return Err(e).context("failed to sync database from server")
+        }
+    };
     state.lock().await.set_master_password_reprompt(&entries);
-    if let Some(access_token) = access_token {
-        db.access_token = Some(access_token);
+    if let Some(tokens) = tokens {
+        db.access_token = Some(tokens.access_token);
+        if let Some(refresh_token) = tokens.refresh_token {
+            db.refresh_token = Some(refresh_token);
+        }
     }
     db.protected_key = Some(protected_key);
     db.protected_private_key = Some(protected_private_key);