From beee9b4f4144553e6d7e6030dc8fdfd40ccdf064 Mon Sep 17 00:00:00 2001 From: Yoshihiro OKUMURA Date: Thu, 7 May 2026 15:46:39 +0900 Subject: [PATCH] fix(auth): allow anonymous read-only API access Match the Python client for read-only commands by falling back to anonymous API requests when no valid login cache is available. Keep mutating commands login-only while letting ls, labs, metadata, and file-metadata resolve laboratories and folders without a cached token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/cache/mod.rs | 173 +++++++++++++++++++++++++++++++++- src/commands/file_metadata.rs | 11 +-- src/commands/labs.rs | 5 +- src/commands/ls.rs | 9 +- src/commands/metadata.rs | 9 +- src/commands/shared.rs | 66 +++++++++++++ 6 files changed, 249 insertions(+), 24 deletions(-) diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 55d34fe..06adfb6 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -185,6 +185,43 @@ fn load_cache_from_dir(remote: &str, config_dir: &Path) -> Result Result, anyhow::Error> { + let cache_path = cache_file_path_in(config_dir, remote); + let snapshot = match read_cache_snapshot(&cache_path) { + Ok(snapshot) => snapshot, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + invalidate_cached_entry(config_dir, remote); + return Ok(None); + } + Err(e) => return Err(e.into()), + }; + + if let Some(cache) = cached_entry(config_dir, remote, &snapshot) { + return Ok(Some(cache)); + } + + let data = match fs::read_to_string(&cache_path) { + Ok(data) => data, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + invalidate_cached_entry(config_dir, remote); + return Ok(None); + } + Err(e) => return Err(e.into()), + }; + let cache = match parse_cache(remote, &data) { + Ok(cache) => cache, + Err(_) => { + remove_cache_in_dir(remote, config_dir)?; + return Ok(None); + } + }; + update_cached_entry(config_dir, remote, snapshot, cache.clone()); + Ok(Some(cache)) +} + fn persist_cache_in_dir( remote: &str, config_dir: &Path, @@ -247,6 +284,7 @@ pub async fn load_cache_with_token_refresh(remote: &str) -> Result Result Result, anyhow::Error> { + let lock = get_remote_lock(remote); + let _guard = lock.lock().await; + + ensure_cache_dir(&cache_dir_path(config_dir))?; + let lock_path = cache_file_path_in(config_dir, remote).with_extension("lock"); + use fs2::FileExt; + let lock_file = fs::OpenOptions::new() + .write(true) + .create(true) + .open(&lock_path)?; + lock_file.lock_exclusive()?; + + let result: Result, anyhow::Error> = async { + let Some(mut cache) = load_cache_if_present_from_dir(remote, config_dir)? else { + return Ok(None); + }; + + if crate::token::is_expired(&cache.token.refresh) { + remove_cache_in_dir(remote, config_dir)?; + return Ok(None); + } + + if crate::token::is_refresh_required(&cache.token.access, &cache.token.refresh) { + cache = refresh_and_persist_in_dir(remote, config_dir, &cache).await?; + } + + Ok(Some(cache)) + } + .await; + + lock_file.unlock()?; + result +} + +/// Load cache when present and refresh its token if needed. +/// +/// Unlike `load_cache_with_token_refresh`, this returns `Ok(None)` when the user +/// is effectively anonymous: no cache file exists, the cache is invalid, or the +/// refresh token has already expired. This mirrors the Python client behavior +/// used by read-only commands. +pub async fn load_cache_with_token_refresh_optional( + remote: &str, +) -> Result, anyhow::Error> { + load_cache_with_token_refresh_optional_from_dir( + remote, + &crate::settings::SETTINGS.config_dirname, + ) + .await +} + /// Call the token-refresh endpoint and write the new access token back to the /// cache file. The caller must already hold the per-remote async mutex. async fn refresh_and_persist(remote: &str, cache: &Cache) -> Result { + refresh_and_persist_in_dir(remote, &crate::settings::SETTINGS.config_dirname, cache).await +} + +async fn refresh_and_persist_in_dir( + remote: &str, + config_dir: &Path, + cache: &Cache, +) -> Result { let url = crate::commands::config::get_remote_url(remote)? .ok_or_else(|| anyhow!("Remote `{}` is not configured.", remote))?; let conn = MDRSConnection::new(&url); @@ -298,7 +398,7 @@ async fn refresh_and_persist(remote: &str, cache: &Cache) -> Result Result { + Ok(create_remote_conn(remote)?.with_token(cache.token.access.clone())) +} + +/// Create an unauthenticated `MDRSConnection` for the given remote label. +pub fn create_remote_conn(remote: &str) -> Result { let url = crate::commands::config::get_remote_url(remote)? .ok_or_else(|| anyhow!("Remote `{}` is not configured.", remote))?; - Ok(MDRSConnection::new(&url) - .with_remote(remote) - .with_token(cache.token.access.clone())) + Ok(MDRSConnection::new(&url).with_remote(remote)) +} + +/// Create a connection for read-only commands, attaching a bearer token only +/// when a valid login cache is available. +pub async fn create_readonly_conn( + remote: &str, +) -> Result<(MDRSConnection, Option), anyhow::Error> { + let conn = create_remote_conn(remote)?; + match load_cache_with_token_refresh_optional(remote).await? { + Some(cache) => Ok((conn.with_token(cache.token.access.clone()), Some(cache))), + None => Ok((conn, None)), + } } #[cfg(test)] @@ -438,4 +553,54 @@ mod tests { .contains(&format!("Not logged in to `{remote}`")) ); } + + fn make_jwt_with_exp(exp: i64) -> String { + use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; + + let header = URL_SAFE_NO_PAD.encode(r#"{"alg":"none","typ":"JWT"}"#); + let payload = URL_SAFE_NO_PAD.encode(format!(r#"{{"exp":{exp}}}"#)); + format!("{header}.{payload}.") + } + + #[test] + fn load_cache_if_present_returns_none_when_cache_missing() { + let dir = tempdir().unwrap(); + let remote = remote_name("missing", dir.path()); + + let loaded = load_cache_if_present_from_dir(&remote, dir.path()).unwrap(); + + assert!(loaded.is_none()); + } + + #[test] + fn load_cache_if_present_clears_invalid_cache() { + let dir = tempdir().unwrap(); + let remote = remote_name("invalid", dir.path()); + let cache_dir = cache_dir_path(dir.path()); + ensure_cache_dir(&cache_dir).unwrap(); + let cache_path = cache_file_path_in(dir.path(), &remote); + fs::write(&cache_path, b"{invalid json").unwrap(); + + let loaded = load_cache_if_present_from_dir(&remote, dir.path()).unwrap(); + + assert!(loaded.is_none()); + assert!(!cache_path.exists()); + } + + #[tokio::test] + async fn optional_cache_load_treats_expired_session_as_anonymous() { + let dir = tempdir().unwrap(); + let remote = remote_name("expired", dir.path()); + let mut cache = sample_cache("alice"); + cache.token.access = make_jwt_with_exp(0); + cache.token.refresh = make_jwt_with_exp(0); + persist_cache_in_dir(&remote, dir.path(), &cache).unwrap(); + + let loaded = load_cache_with_token_refresh_optional_from_dir(&remote, dir.path()) + .await + .unwrap(); + + assert!(loaded.is_none()); + assert!(!cache_file_path_in(dir.path(), &remote).exists()); + } } diff --git a/src/commands/file_metadata.rs b/src/commands/file_metadata.rs index d661ba9..638ffa4 100644 --- a/src/commands/file_metadata.rs +++ b/src/commands/file_metadata.rs @@ -1,15 +1,12 @@ -use crate::cache::{create_authenticated_conn, load_cache_with_token_refresh}; -use crate::commands::shared::{ - find_file_by_name, find_folder, find_lab_in_cache, parse_remote_path, -}; +use crate::cache::create_readonly_conn; +use crate::commands::shared::{find_file_by_name, find_folder, find_laboratory, parse_remote_path}; use anyhow::anyhow; pub async fn file_metadata(remote_path: &str, password: Option<&str>) -> Result<(), anyhow::Error> { let (remote, labname, r_path) = parse_remote_path(remote_path)?; - let cache = load_cache_with_token_refresh(&remote).await?; - let conn = create_authenticated_conn(&remote, &cache)?; - let lab = find_lab_in_cache(&cache, &labname)?; + let (conn, cache) = create_readonly_conn(&remote).await?; + let lab = find_laboratory(&conn, cache.as_ref(), &labname).await?; let lab_id = lab.id; // Split the file path into parent directory and filename diff --git a/src/commands/labs.rs b/src/commands/labs.rs index 3b7759a..4c0f4d3 100644 --- a/src/commands/labs.rs +++ b/src/commands/labs.rs @@ -1,8 +1,7 @@ -use crate::cache::{create_authenticated_conn, load_cache_with_token_refresh}; +use crate::cache::create_readonly_conn; pub async fn labs(remote: &str) -> Result<(), anyhow::Error> { - let cache = load_cache_with_token_refresh(remote).await?; - let conn = create_authenticated_conn(remote, &cache)?; + let (conn, _) = create_readonly_conn(remote).await?; let labs = conn.list_laboratories().await?; let header = ("Name", "PI", "Laboratory"); diff --git a/src/commands/ls.rs b/src/commands/ls.rs index 8272efe..44758ac 100644 --- a/src/commands/ls.rs +++ b/src/commands/ls.rs @@ -1,5 +1,5 @@ -use crate::cache::{create_authenticated_conn, load_cache_with_token_refresh}; -use crate::commands::shared::{find_folder, find_lab_in_cache, fmt_datetime, parse_remote_path}; +use crate::cache::create_readonly_conn; +use crate::commands::shared::{find_folder, find_laboratory, fmt_datetime, parse_remote_path}; use crate::connection::MDRSConnection; use crate::models::file::File; use crate::models::folder::{FolderDetail, FolderSimple}; @@ -15,9 +15,8 @@ pub async fn ls( is_quiet: bool, ) -> Result<(), anyhow::Error> { let (remote, labname, path) = parse_remote_path(remote_path)?; - let cache = load_cache_with_token_refresh(&remote).await?; - let conn = create_authenticated_conn(&remote, &cache)?; - let lab = find_lab_in_cache(&cache, &labname)?; + let (conn, cache) = create_readonly_conn(&remote).await?; + let lab = find_laboratory(&conn, cache.as_ref(), &labname).await?; let folder = find_folder(&conn, lab.id, &path, password).await?; diff --git a/src/commands/metadata.rs b/src/commands/metadata.rs index 98bf01a..12a0bad 100644 --- a/src/commands/metadata.rs +++ b/src/commands/metadata.rs @@ -1,11 +1,10 @@ -use crate::cache::{create_authenticated_conn, load_cache_with_token_refresh}; -use crate::commands::shared::{find_folder, find_lab_in_cache, parse_remote_path}; +use crate::cache::create_readonly_conn; +use crate::commands::shared::{find_folder, find_laboratory, parse_remote_path}; pub async fn metadata(remote_path: &str, password: Option<&str>) -> Result<(), anyhow::Error> { let (remote, labname, folder_path) = parse_remote_path(remote_path)?; - let cache = load_cache_with_token_refresh(&remote).await?; - let conn = create_authenticated_conn(&remote, &cache)?; - let lab = find_lab_in_cache(&cache, &labname)?; + let (conn, cache) = create_readonly_conn(&remote).await?; + let lab = find_laboratory(&conn, cache.as_ref(), &labname).await?; let folder = find_folder(&conn, lab.id, &folder_path, password).await?; let resp = conn diff --git a/src/commands/shared.rs b/src/commands/shared.rs index 3815977..e880e62 100644 --- a/src/commands/shared.rs +++ b/src/commands/shared.rs @@ -3,6 +3,7 @@ use crate::connection::ApiRequestLimiter; use crate::connection::MDRSConnection; use crate::models::file::File; use crate::models::folder::{FolderDetail, FolderSimple}; +use crate::models::laboratory::Laboratory; use anyhow::{anyhow, bail}; use unicode_normalization::UnicodeNormalization; @@ -48,6 +49,32 @@ pub fn find_lab_in_cache<'a>( .ok_or_else(|| anyhow!("Laboratory `{}` not found.", labname)) } +/// Resolve a laboratory by name using cached laboratories when available, and +/// falling back to the API when the user is anonymous. +pub async fn find_laboratory( + conn: &MDRSConnection, + cache: Option<&Cache>, + labname: &str, +) -> Result { + if let Some(cache) = cache { + if let Ok(lab) = find_lab_in_cache(cache, labname) { + return Ok(Laboratory { + id: lab.id, + name: lab.name.clone(), + pi_name: lab.pi_name.clone(), + full_name: lab.full_name.clone(), + }); + } + } + + conn.list_laboratories() + .await? + .items + .into_iter() + .find(|lab| lab.name == labname) + .ok_or_else(|| anyhow!("Laboratory `{}` not found.", labname)) +} + // --------------------------------------------------------------------------- // Unicode helpers // --------------------------------------------------------------------------- @@ -178,3 +205,42 @@ pub fn fmt_datetime(iso: &str) -> String { iso.to_string() } } + +#[cfg(test)] +mod tests { + use super::*; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + #[tokio::test] + async fn find_laboratory_falls_back_to_api_without_authorization() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut buf = [0u8; 4096]; + let n = stream.read(&mut buf).await.unwrap(); + let req = String::from_utf8_lossy(&buf[..n]); + + assert!(req.starts_with("GET /v3/laboratories/ HTTP/1.1")); + assert!(!req.contains("\r\nAuthorization: Bearer ")); + + let body = + r#"[{"id":1,"name":"public-lab","pi_name":"PI","full_name":"Public Laboratory"}]"#; + let response = format!( + "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", + body.len(), + body + ); + stream.write_all(response.as_bytes()).await.unwrap(); + }); + + let conn = MDRSConnection::new(&format!("http://{addr}")); + let lab = find_laboratory(&conn, None, "public-lab").await.unwrap(); + + assert_eq!(lab.id, 1); + assert_eq!(lab.name, "public-lab"); + server.await.unwrap(); + } +}