From 6d3fb886779d00ad2b23581a72dd37a724c74361 Mon Sep 17 00:00:00 2001 From: Dragan Spiridonov Date: Wed, 22 Jul 2026 18:21:49 +0200 Subject: [PATCH] fix(cli): resolve scope from the token on read, and add `whoami --refresh` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps that only appear with a credential file written by an earlier build — i.e. exactly the case a fresh-login test never exercises. 1. `whoami` printed "Scope: (not reported)" for an existing file. The previous commit resolved scope from the token claim at WRITE time only, so files written before it stayed blank forever. `effective_scope()` now falls back at READ time, which fixes existing files and any client that stored only what the token response carried. The token is authoritative either way; the stored field is a convenience copy. 2. `whoami` said the token "will refresh on next use" — a promise nothing kept, because no command called `ensure_fresh`. The refresh path was implemented and unit-tested but never actually run. `--refresh` exercises it, and goes through `Session::ensure_fresh` rather than reimplementing a second, subtly different refresh, so it inherits the single-flight guarantee and the persist-before-return ordering. It is a flag, not silent behaviour: refreshing rotates the stored refresh token — identity spends the old one — so it is a state change, not a read. VERIFIED AGAINST PRODUCTION, end to end: whoami on a pre-existing file -> Scope: sensing:read (was "not reported") whoami --refresh -> both tokens rotated refresh sha256[:12] 50cfa06b -> 2d37617a access sha256[:12] 61eebe8d -> 14d8e8de status: expired -> valid refreshed token GET /api/v1/models -> 200 refreshed token POST /api/v1/train/start -> 401 (still read-scoped) That exercises the rotating-refresh path against identity's real reuse detection — the one place a bug costs the user their session rather than a retry — and confirms the scope gate survives a refresh. Tests: 82 with --features login, 44 default, 501 sensing-server. Co-Authored-By: Ruflo & AQE --- v2/Cargo.lock | 1 + v2/crates/ruview-auth/src/login/store.rs | 14 ++++++++++++ v2/crates/wifi-densepose-cli/Cargo.toml | 2 ++ v2/crates/wifi-densepose-cli/src/auth.rs | 28 +++++++++++++++++++++--- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/v2/Cargo.lock b/v2/Cargo.lock index 0b219542..370fc26b 100644 --- a/v2/Cargo.lock +++ b/v2/Cargo.lock @@ -11060,6 +11060,7 @@ dependencies = [ "ndarray 0.17.2", "num-complex", "predicates", + "reqwest 0.12.28", "ruview-auth", "serde", "serde_json", diff --git a/v2/crates/ruview-auth/src/login/store.rs b/v2/crates/ruview-auth/src/login/store.rs index c7990365..ac9d2b62 100644 --- a/v2/crates/ruview-auth/src/login/store.rs +++ b/v2/crates/ruview-auth/src/login/store.rs @@ -93,6 +93,20 @@ impl StoredCredentials { } } + /// The granted scope, falling back to the access token's own claim. + /// + /// Resolved at *read* time, not just at write time, so credential files + /// written before the fallback existed — or by any client that stores only + /// what the token response carried — still report correctly instead of + /// showing "(not reported)" forever. The token is the authoritative source + /// either way; the stored field is a convenience copy. + pub fn effective_scope(&self) -> Option { + self.scope + .clone() + .filter(|s| !s.is_empty()) + .or_else(|| scope_from_access_token(&self.access_token)) + } + /// Does the access token need replacing? /// /// A missing `expires_at` counts as expired. Guessing a lifetime here would diff --git a/v2/crates/wifi-densepose-cli/Cargo.toml b/v2/crates/wifi-densepose-cli/Cargo.toml index e87e0e9d..f5082939 100644 --- a/v2/crates/wifi-densepose-cli/Cargo.toml +++ b/v2/crates/wifi-densepose-cli/Cargo.toml @@ -62,6 +62,8 @@ anyhow = "1.0" # the sensing server depends on this same crate with default features and # gets only the verifier. ruview-auth = { path = "../ruview-auth", features = ["login"] } +# Only for constructing the HTTP client hands to Session. +reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } thiserror = "2.0" # Time diff --git a/v2/crates/wifi-densepose-cli/src/auth.rs b/v2/crates/wifi-densepose-cli/src/auth.rs index 03cecb4c..080cb6c2 100644 --- a/v2/crates/wifi-densepose-cli/src/auth.rs +++ b/v2/crates/wifi-densepose-cli/src/auth.rs @@ -45,6 +45,15 @@ pub struct LogoutArgs { pub struct WhoamiArgs { #[arg(long, env = ruview_auth::login::CREDENTIALS_PATH_ENV)] pub credentials_path: Option, + + /// Refresh the access token now if it has expired, instead of only + /// reporting that it will be refreshed on next use. + /// + /// Refreshing rotates the stored refresh token — identity spends the old + /// one — so this is a real state change, not a read. That is why it is a + /// flag rather than something `whoami` does silently. + #[arg(long)] + pub refresh: bool, } fn path_or_default(p: Option) -> PathBuf { @@ -89,7 +98,18 @@ pub async fn logout_cmd(args: LogoutArgs) -> anyhow::Result<()> { pub async fn whoami_cmd(args: WhoamiArgs) -> anyhow::Result<()> { let path = path_or_default(args.credentials_path); - let creds = ruview_auth::login::store::load(&path)?; + let mut creds = ruview_auth::login::store::load(&path)?; + + if args.refresh && creds.needs_refresh() { + println!("Access token expired — refreshing…"); + let session = ruview_auth::login::Session::load_from(path.clone(), reqwest::Client::new())?; + // Goes through ensure_fresh, so it inherits the single-flight guarantee + // and the persist-before-return ordering rather than reimplementing a + // second, subtly different refresh path. + session.ensure_fresh().await?; + creds = session.snapshot().await; + println!("Refreshed.\n"); + } println!("Credentials: {}", path.display()); println!("Issuer: {}", creds.issuer); @@ -97,7 +117,9 @@ pub async fn whoami_cmd(args: WhoamiArgs) -> anyhow::Result<()> { Some(e) => println!("Account: {e}"), None => println!("Account: (not reported)"), } - match &creds.scope { + // Falls back to the token's own claim, so a file written before that + // fallback existed still reports its real scope. + match creds.effective_scope() { Some(s) => println!("Scope: {s}"), None => println!("Scope: (not reported)"), } @@ -105,7 +127,7 @@ pub async fn whoami_cmd(args: WhoamiArgs) -> anyhow::Result<()> { // common reason a command starts 401ing, so say it plainly here rather than // letting the user infer it from a failure elsewhere. if creds.needs_refresh() { - println!("Status: access token expired or expiring — it will refresh on next use"); + println!("Status: access token expired or expiring — pass --refresh to renew it now"); } else { println!("Status: access token valid"); }