diff --git a/src/api.rs b/src/api.rs index 1624eac..5c13e41 100644 --- a/src/api.rs +++ b/src/api.rs @@ -1,16 +1,17 @@ -//! Authenticated calls to the api-platform account service. Endpoints -//! that require a session expect `Authorization: Bearer `. - -use std::io::Read; -use std::time::Duration; - +//! Authenticated calls to the api-platform account and applications +//! services. Endpoints that require a session expect +//! `Authorization: Bearer `. +//! +//! Functions accept a `&Client` so a single command builds one client +//! and reuses its connection pool across multiple calls (e.g. `me` + +//! `get_module` + `create_module` from `module init`). + +use reqwest::blocking::{Client, Response}; use serde::{Deserialize, Serialize}; use thiserror::Error; use crate::http; -const MAX_RESPONSE_BYTES: u64 = 64 * 1024; - #[derive(Debug, Deserialize)] #[allow(dead_code)] // profile_url is part of the API surface; whoami doesn't print it yet pub struct Identity { @@ -75,9 +76,8 @@ pub struct CreateModuleInput<'a> { } /// GET /v1/auth/me — returns the authenticated user's identity. -pub fn me(api_base: &str, access_token: &str) -> Result { +pub fn me(http: &Client, api_base: &str, access_token: &str) -> Result { let endpoint = format!("{}/v1/auth/me", api_base.trim_end_matches('/')); - let http = http::client(Duration::from_secs(15))?; let resp = http .get(&endpoint) @@ -86,26 +86,13 @@ pub fn me(api_base: &str, access_token: &str) -> Result { .send()?; let status = resp.status(); - if status.is_success() { return Ok(resp.json::()?); } if status == reqwest::StatusCode::UNAUTHORIZED || status == reqwest::StatusCode::FORBIDDEN { return Err(ApiError::Unauthenticated); } - // Bound the error-path body so a hostile endpoint can't OOM us. - let mut body = Vec::with_capacity(1024); - resp.take(MAX_RESPONSE_BYTES) - .read_to_end(&mut body) - .map_err(|e| ApiError::Unexpected { - status: status.as_u16(), - body: format!("(read body failed: {e})"), - })?; - let body = String::from_utf8_lossy(&body).into_owned(); - Err(ApiError::Unexpected { - status: status.as_u16(), - body, - }) + Err(unexpected_body_error(resp)) } /// GET /v1/modules/{slug} — returns the caller's module by slug. @@ -114,6 +101,7 @@ pub fn me(api_base: &str, access_token: &str) -> Result { /// per-owner, so 404 here does NOT mean the platform-wide name is unique /// (reserved/format checks still happen on POST). pub fn get_module( + http: &Client, apps_base: &str, access_token: &str, slug: &str, @@ -121,7 +109,6 @@ pub fn get_module( // Slug is pre-validated against `^[a-z][a-z0-9-]{1,38}[a-z0-9]$` before // this call, so it's URL-safe with no encoding needed. let endpoint = format!("{}/v1/modules/{}", apps_base.trim_end_matches('/'), slug); - let http = http::client(Duration::from_secs(15))?; let resp = http .get(&endpoint) @@ -139,27 +126,17 @@ pub fn get_module( if status == reqwest::StatusCode::UNAUTHORIZED || status == reqwest::StatusCode::FORBIDDEN { return Err(ApiError::Unauthenticated); } - let mut body = Vec::with_capacity(1024); - resp.take(MAX_RESPONSE_BYTES) - .read_to_end(&mut body) - .map_err(|e| ApiError::Unexpected { - status: status.as_u16(), - body: format!("(read body failed: {e})"), - })?; - Err(ApiError::Unexpected { - status: status.as_u16(), - body: String::from_utf8_lossy(&body).into_owned(), - }) + Err(unexpected_body_error(resp)) } /// POST /v1/modules — create a developer-owned module. pub fn create_module( + http: &Client, apps_base: &str, access_token: &str, input: &CreateModuleInput, ) -> Result { let endpoint = format!("{}/v1/modules", apps_base.trim_end_matches('/')); - let http = http::client(Duration::from_secs(15))?; let resp = http .post(&endpoint) @@ -179,33 +156,59 @@ pub fn create_module( // 4xx with platform error-envelope: surface code + message so callers // can branch on `slug_taken` / `slug_reserved` / `slug_invalid` without // re-parsing the body. - let mut body = Vec::with_capacity(1024); - resp.take(MAX_RESPONSE_BYTES) - .read_to_end(&mut body) - .map_err(|e| ApiError::Unexpected { - status: status.as_u16(), - body: format!("(read body failed: {e})"), - })?; + let status_u16 = status.as_u16(); + let body = match http::read_capped(resp) { + Ok(b) => b, + Err(e) => { + return Err(ApiError::Unexpected { + status: status_u16, + body: format!("(read body failed: {e})"), + }); + } + }; if let Ok(env) = serde_json::from_slice::(&body) { return Err(ApiError::Server { - status: status.as_u16(), + status: status_u16, code: env.error.code, message: env.error.message, }); } Err(ApiError::Unexpected { - status: status.as_u16(), + status: status_u16, body: String::from_utf8_lossy(&body).into_owned(), }) } +/// Common tail for unexpected (non-success, non-typed) responses: read +/// the body with [`http::read_capped`] and wrap as `ApiError::Unexpected`. +/// If reading fails, the io error is folded into the body for diagnosis. +fn unexpected_body_error(resp: Response) -> ApiError { + let status = resp.status().as_u16(); + match http::read_capped(resp) { + Ok(bytes) => ApiError::Unexpected { + status, + body: String::from_utf8_lossy(&bytes).into_owned(), + }, + Err(e) => ApiError::Unexpected { + status, + body: format!("(read body failed: {e})"), + }, + } +} + #[cfg(test)] mod tests { use super::*; + use std::time::Duration; + use mockito::Server; use serde_json::json; + fn test_client() -> Client { + http::client(Duration::from_secs(15)).expect("client") + } + #[test] fn me_success() { let mut server = Server::new(); @@ -225,7 +228,7 @@ mod tests { ) .create(); - let id = me(&server.url(), "AT").expect("ok"); + let id = me(&test_client(), &server.url(), "AT").expect("ok"); assert_eq!(id.email, "user@example.com"); assert_eq!(id.slug.as_deref(), Some("test-user")); } @@ -239,10 +242,29 @@ mod tests { .with_body(r#"{"error":{"code":"token_invalid"}}"#) .create(); - let err = me(&server.url(), "expired").unwrap_err(); + let err = me(&test_client(), &server.url(), "expired").unwrap_err(); assert!(matches!(err, ApiError::Unauthenticated), "got {err:?}"); } + #[test] + fn me_5xx_is_unexpected_with_body() { + let mut server = Server::new(); + let _m = server + .mock("GET", "/v1/auth/me") + .with_status(503) + .with_body("upstream timeout") + .create(); + + let err = me(&test_client(), &server.url(), "AT").unwrap_err(); + match err { + ApiError::Unexpected { status, body } => { + assert_eq!(status, 503); + assert!(body.contains("upstream timeout"), "got body {body:?}"); + } + other => panic!("expected Unexpected, got {other:?}"), + } + } + #[test] fn create_module_success() { let mut server = Server::new(); @@ -266,6 +288,7 @@ mod tests { .create(); let m = create_module( + &test_client(), &server.url(), "AT", &CreateModuleInput { @@ -296,7 +319,7 @@ mod tests { ) .create(); - let m = get_module(&server.url(), "AT", "media").expect("ok"); + let m = get_module(&test_client(), &server.url(), "AT", "media").expect("ok"); assert!(m.is_some()); assert_eq!(m.unwrap().slug, "media"); } @@ -306,13 +329,27 @@ mod tests { let mut server = Server::new(); let _m = server .mock("GET", "/v1/modules/none") + .match_header("authorization", "Bearer AT") .with_status(404) .create(); - let m = get_module(&server.url(), "AT", "none").expect("ok"); + let m = get_module(&test_client(), &server.url(), "AT", "none").expect("ok"); assert!(m.is_none()); } + #[test] + fn get_module_401_is_unauthenticated() { + let mut server = Server::new(); + let _m = server + .mock("GET", "/v1/modules/forbidden-slug") + .with_status(401) + .create(); + + let err = + get_module(&test_client(), &server.url(), "expired", "forbidden-slug").unwrap_err(); + assert!(matches!(err, ApiError::Unauthenticated), "got {err:?}"); + } + #[test] fn create_module_409_surfaces_code() { let mut server = Server::new(); @@ -325,6 +362,7 @@ mod tests { .create(); let err = create_module( + &test_client(), &server.url(), "AT", &CreateModuleInput { @@ -341,4 +379,32 @@ mod tests { other => panic!("expected Server, got {other:?}"), } } + + #[test] + fn create_module_4xx_without_envelope_is_unexpected() { + let mut server = Server::new(); + let _m = server + .mock("POST", "/v1/modules") + .with_status(400) + .with_body("not json") + .create(); + + let err = create_module( + &test_client(), + &server.url(), + "AT", + &CreateModuleInput { + name: "x", + slug: "x", + }, + ) + .unwrap_err(); + match err { + ApiError::Unexpected { status, body } => { + assert_eq!(status, 400); + assert!(body.contains("not json"), "got body {body:?}"); + } + other => panic!("expected Unexpected, got {other:?}"), + } + } } diff --git a/src/auth/mod.rs b/src/auth/mod.rs index 964e30c..e6b37b8 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -5,8 +5,6 @@ //! displayed on the consent page back into the terminal. Custom-scheme //! and per-OS handler registration is the follow-up tracked at #7. -use std::io::Read; - use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; use rand::TryRngCore; @@ -17,10 +15,7 @@ use sha2::{Digest, Sha256}; use thiserror::Error; use url::Url; -/// Cap on response bodies we'll deserialize. /token and /me return -/// sub-1KB JSON in practice; 64 KiB is generous slack and protects -/// against a hostile endpoint trying to OOM the CLI. -const MAX_RESPONSE_BYTES: u64 = 64 * 1024; +use crate::http; /// Identifies the CLI to the platform; matches the seed in /// api-platform migration `011_oauth.up.sql`. @@ -176,15 +171,10 @@ fn random_b64url(n: usize) -> Result { Ok(URL_SAFE_NO_PAD.encode(buf)) } -/// Read a response body into memory with a hard size cap. Truncated -/// bodies are returned as-is — the JSON parse downstream will fail -/// loudly rather than silently misinterpret a partial document. +/// Wrap [`http::read_capped`] with auth-specific error mapping. fn read_capped(resp: Response) -> Result { - let mut buf = Vec::with_capacity(1024); - resp.take(MAX_RESPONSE_BYTES) - .read_to_end(&mut buf) - .map_err(|e| AuthError::Decode(e.to_string()))?; - String::from_utf8(buf).map_err(|e| AuthError::Decode(e.to_string())) + let bytes = http::read_capped(resp).map_err(|e| AuthError::Decode(e.to_string()))?; + String::from_utf8(bytes).map_err(|e| AuthError::Decode(e.to_string())) } #[cfg(test)] diff --git a/src/commands/module.rs b/src/commands/module.rs index 1bf317e..5c45ce2 100644 --- a/src/commands/module.rs +++ b/src/commands/module.rs @@ -17,6 +17,7 @@ use indicatif::{ProgressBar, ProgressStyle}; use crate::api::{self, ApiError, CreateModuleInput}; use crate::credentials; +use crate::http; use super::{ DEFAULT_API_BASE, DEFAULT_APPS_API_BASE, DEFAULT_WEB_BASE, ENV_API_URL, ENV_APPS_API_URL, @@ -68,11 +69,12 @@ fn init(args: InitArgs) -> Result<()> { let api_base = resolve_base(ENV_API_URL, DEFAULT_API_BASE); let apps_base = resolve_base(ENV_APPS_API_URL, DEFAULT_APPS_API_BASE); let web_base = resolve_base(ENV_WEB_URL, DEFAULT_WEB_BASE); + let client = http::client(Duration::from_secs(15))?; // The platform stores ownership by user id, but the CLI surfaces the // full namespaced `@/` — so we refuse to POST until the // caller has claimed a username, and point them at the web flow. - let identity = match api::me(&api_base, &creds.access_token) { + let identity = match api::me(&client, &api_base, &creds.access_token) { Ok(id) => id, Err(ApiError::Unauthenticated) => return Err(session_expired()), Err(e) => return Err(e.into()), @@ -95,7 +97,7 @@ fn init(args: InitArgs) -> Result<()> { // the common "I forgot I made this last week" case before we POST. // Reserved/invalid still surface server-side from the POST below. let pre_check = with_spinner("Checking availability…", || { - api::get_module(&apps_base, &creds.access_token, &slug) + api::get_module(&client, &apps_base, &creds.access_token, &slug) }); match pre_check { Ok(Some(existing)) if args.used => { @@ -132,6 +134,7 @@ fn init(args: InitArgs) -> Result<()> { let create_result = with_spinner("Creating module…", || { api::create_module( + &client, &apps_base, &creds.access_token, &CreateModuleInput { diff --git a/src/commands/whoami.rs b/src/commands/whoami.rs index e3faeca..5b87f25 100644 --- a/src/commands/whoami.rs +++ b/src/commands/whoami.rs @@ -5,11 +5,14 @@ //! tells the user to run `mirrorstack login`. Auto-refresh on token //! expiry is a follow-up — for v1 the user re-runs login. +use std::time::Duration; + use anyhow::{Result, anyhow}; use clap::Args; use crate::api::{self, ApiError}; use crate::credentials; +use crate::http; use super::{DEFAULT_API_BASE, ENV_API_URL, resolve_base}; @@ -19,8 +22,9 @@ pub struct WhoamiArgs {} pub fn run(_args: WhoamiArgs) -> Result<()> { let creds = credentials::load_or_login_hint()?; let api_base = resolve_base(ENV_API_URL, DEFAULT_API_BASE); + let client = http::client(Duration::from_secs(15))?; - match api::me(&api_base, &creds.access_token) { + match api::me(&client, &api_base, &creds.access_token) { Ok(id) => { println!("{}", id.email); if !id.name.is_empty() { diff --git a/src/http.rs b/src/http.rs index 6c6effd..c644806 100644 --- a/src/http.rs +++ b/src/http.rs @@ -3,9 +3,15 @@ //! session list (and any audit log) can then render "MirrorStack CLI" //! instead of falling through to a generic browser-UA parser. +use std::io::{self, Read}; use std::time::Duration; -use reqwest::blocking::{Client, ClientBuilder}; +use reqwest::blocking::{Client, ClientBuilder, Response}; + +/// Cap on response bodies we'll deserialize. The platform's success +/// payloads are sub-1 KB; 64 KiB is generous slack for error envelopes +/// and protects against a hostile endpoint trying to OOM the CLI. +const MAX_RESPONSE_BYTES: u64 = 64 * 1024; /// User-Agent shape: `mirrorstack-cli/ (; )`. /// Mirrors the convention of curl, gh CLI, brew, etc. @@ -32,6 +38,16 @@ pub fn builder(timeout: Duration) -> ClientBuilder { Client::builder().timeout(timeout).user_agent(user_agent()) } +/// Read a response body into memory with a hard size cap. Truncated +/// bodies are returned as-is — the caller's parse step fails loudly +/// rather than silently misinterpreting a partial document. Each +/// caller maps the io::Error into its own error type. +pub fn read_capped(resp: Response) -> io::Result> { + let mut buf = Vec::with_capacity(1024); + resp.take(MAX_RESPONSE_BYTES).read_to_end(&mut buf)?; + Ok(buf) +} + #[cfg(test)] mod tests { use super::*;