From f5dfb2290e4440522d660c6530ca9bdaa2da0165 Mon Sep 17 00:00:00 2001 From: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:58:28 +0200 Subject: [PATCH] Show the signed-in user in whoami from /v1/me whoami now reads the user of a sign-in from the body of GET /v1/me as received (data.user), not from the generated type, because the vendored API document does not describe the field yet. It is read leniently: a missing field, null or a malformed value is no user, never an error. The User line and the --json user string keep their name and type. They are filled from the API first, formatted as Name , and the authorization server's userinfo endpoint is only asked when the API gives no user. For a key the field is ignored. Every string is cleaned before it is printed. No extra request is made, and nothing changes in the output until the API returns the field. Closes #182 Signed-off-by: d0ubl3g <48954024+d0ubl3g@users.noreply.github.com> --- CHANGELOG.md | 5 + README.md | 3 +- .../hodeishield-cli/src/commands/session.rs | 304 ++++++++++++++++-- crates/hodeishield-cli/tests/contract.rs | 27 ++ 4 files changed, 308 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76ddb06..383afb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,11 @@ commands, `--json` output and exit codes. Each release's notes start with its se `portfolio` and `credential_scopes` fields, null when the API cannot be read; the exit code does not depend on that call. ([#48](https://github.com/Hodeitek/hodeishield-cli/issues/48)) +- `whoami` shows the signed-in user (`Name `, in the `User` line and the `--json` `user` + string) from `GET /v1/me` when the API provides it for a sign-in, with no extra call. The + authorization server's userinfo endpoint stays as the fallback; a key still shows no user, and + nothing changes while the API does not send one. + ([#182](https://github.com/Hodeitek/hodeishield-cli/issues/182)) ### Fixed diff --git a/README.md b/README.md index 585f1a7..e2bc4ee 100644 --- a/README.md +++ b/README.md @@ -302,7 +302,8 @@ When `HODEISHIELD_API_KEY` is set it is used for every call, whatever else is co your CI's secret store; the CLI never prints it. `whoami` asks the API who the credential is and shows the tenant it reads (name and id), its -organisation and, for a key, its scopes; for a portfolio key, the MSSP and how many client tenants it +organisation and, for a key, its scopes; for a sign-in, also the signed-in user when the service +provides it; for a portfolio key, the MSSP and how many client tenants it can select. With `--json` these are the `tenant`, `organization`, `portfolio` and `credential_scopes` fields. If the API cannot be reached, `whoami` still shows the local settings, leaves those fields null and says why on standard error. diff --git a/crates/hodeishield-cli/src/commands/session.rs b/crates/hodeishield-cli/src/commands/session.rs index 9d4928c..1a133f8 100644 --- a/crates/hodeishield-cli/src/commands/session.rs +++ b/crates/hodeishield-cli/src/commands/session.rs @@ -221,11 +221,11 @@ fn note_api_key() { pub fn whoami(ctx: &Context, out: &mut dyn Write) -> Result<()> { let settings = ctx.settings()?; // `GET /v1/me` describes the credential itself: it takes no tenant selection. - let me = || -> std::result::Result { + let me = || -> std::result::Result { let (client, source) = ctx.api(&settings, None).map_err(|f| f.message)?; client .who_am_i() - .map(|answer| answer.data.data) + .map(|answer| Me::from_answer(answer.data.data, &answer.raw)) .map_err(|e| { let mut call = ctx.call(source, None); call.tenant_selected = false; @@ -238,12 +238,78 @@ pub fn whoami(ctx: &Context, out: &mut dyn Write) -> Result<()> { ctx.store.as_ref(), ctx.json, &me, + &userinfo_label, out, ) } -/// `whoami` with the settings, whether an API key is set and the call to `GET /v1/me` given, so it -/// can be tested without the process environment. With a key, the store is not read. The call is +/// What `GET /v1/me` says about the credential: the generated type, and the signed-in user, which +/// the document of the API does not describe yet and so is read from the body as received. +struct Me { + who: WhoAmI, + user: Option, +} + +impl Me { + fn from_answer(who: WhoAmI, raw: &Value) -> Self { + Self { + who, + user: MeUser::from_raw(&raw["data"]["user"]), + } + } +} + +/// The user of a sign-in, as `GET /v1/me` gives it. Read leniently: anything that is not an object +/// with at least one text field is no user, never an error. +struct MeUser { + id: Option, + name: Option, + email: Option, +} + +impl MeUser { + fn from_raw(raw: &Value) -> Option { + let object = raw.as_object()?; + let text = |key: &str| object.get(key).and_then(Value::as_str).map(str::to_owned); + let user = Self { + id: text("id"), + name: text("name"), + email: text("email"), + }; + (user.id.is_some() || user.name.is_some() || user.email.is_some()).then_some(user) + } + + /// Labelled like the user of the userinfo endpoint: `Name `, whichever of them there is, + /// or the id. + fn label(&self) -> Option { + let mut info = Map::new(); + for (key, value) in [ + ("name", &self.name), + ("email", &self.email), + ("sub", &self.id), + ] { + if let Some(value) = value { + info.insert(key.into(), json!(value)); + } + } + user_label(&info) + } +} + +/// The user from the authorization server's userinfo endpoint: the fallback for an API that does +/// not say who the user is. Nothing when the app has no such endpoint or cannot be reached. +fn userinfo_label(token: &StoredToken) -> Option { + let issuer = auth::issuer_url(token).ok()?; + let http = oauth::http_client(&issuer).ok()?; + let metadata = oauth::discover(&http, &issuer).ok()?; + let info = oauth::userinfo(&http, &metadata, &token.access_token) + .ok() + .flatten()?; + user_label(&info) +} + +/// `whoami` with the settings, whether an API key is set, the call to `GET /v1/me` and the userinfo +/// fallback given, so it can be tested without the process environment. With a key, the store is not read. The call is /// made only when there is a credential; when it fails, the local lines are still printed, the /// API's lines are left out (null in `--json`) and a warning says why. fn whoami_with( @@ -251,7 +317,8 @@ fn whoami_with( has_api_key: bool, store: &dyn auth::TokenStore, json: bool, - me: &dyn Fn() -> std::result::Result, + me: &dyn Fn() -> std::result::Result, + userinfo: &dyn Fn(&StoredToken) -> Option, out: &mut dyn Write, ) -> Result<()> { let mut info = Map::new(); @@ -259,6 +326,7 @@ fn whoami_with( info.insert("api_url".into(), json!(settings.api_url.as_str())); info.insert("app_url".into(), json!(settings.app_url.as_str())); + let mut signed_in_token = None; let source = if has_api_key { Some(CredentialSource::ApiKey) } else { @@ -273,16 +341,9 @@ fn whoami_with( .map_or(Value::Null, |t| json!(unix_to_rfc3339(t))), ); info.insert("scope".into(), json!(token.scope)); - let user = auth::issuer_url(&token) - .and_then(|issuer| { - let http = oauth::http_client(&issuer)?; - let metadata = oauth::discover(&http, &issuer)?; - oauth::userinfo(&http, &metadata, &token.access_token) - }) - .ok() - .flatten() - .and_then(|u| user_label(&u)); - info.insert("user".into(), json!(user)); + // Filled in below, once the API has been asked who the user is. + info.insert("user".into(), Value::Null); + signed_in_token = Some(token); Some(CredentialSource::OAuth) } None => None, @@ -307,21 +368,29 @@ fn whoami_with( } None => None, }; + if let Some(token) = &signed_in_token { + // The API first; the userinfo endpoint only when it gives no user. A key has no user, + // whatever the API sends. + let user = me + .as_ref() + .and_then(|me| me.user.as_ref()) + .and_then(MeUser::label) + .or_else(|| userinfo(token)); + info.insert("user".into(), json!(user)); + } + let who = me.as_ref().map(|me| &me.who); info.insert( "tenant".into(), - json!(me.as_ref().and_then(|me| me.tenant.as_ref())), - ); - info.insert( - "organization".into(), - json!(me.as_ref().map(|me| &me.organization)), + json!(who.and_then(|me| me.tenant.as_ref())), ); + info.insert("organization".into(), json!(who.map(|me| &me.organization))); info.insert( "portfolio".into(), - json!(me.as_ref().and_then(|me| me.portfolio.as_ref())), + json!(who.and_then(|me| me.portfolio.as_ref())), ); info.insert( "credential_scopes".into(), - json!(me.as_ref().map(|me| &me.credential.scopes)), + json!(who.map(|me| &me.credential.scopes)), ); if json { @@ -349,7 +418,7 @@ fn whoami_with( rows.push((label, clean(value))); } } - if let Some(me) = &me { + if let Some(me) = who { rows.extend(me_rows(me, source == Some(CredentialSource::ApiKey))); } for (label, value) in rows { @@ -791,6 +860,7 @@ mod tests { &UnreachableStore, false, &no_call, + &no_userinfo, &mut out, ) .expect_err("no credential"); @@ -817,7 +887,16 @@ mod tests { let settings = settings_for(&api, &app); let mut out = Vec::new(); let me = || Err("offline".to_owned()); - whoami_with(&settings, true, &UnreachableStore, true, &me, &mut out).expect("key set"); + whoami_with( + &settings, + true, + &UnreachableStore, + true, + &me, + &no_userinfo, + &mut out, + ) + .expect("key set"); let printed: Value = serde_json::from_slice(&out).expect("json"); assert_eq!(printed["credential"], "api_key"); // The API could not be read: its fields are there, null. @@ -826,12 +905,150 @@ mod tests { } } - fn no_call() -> std::result::Result { + fn no_call() -> std::result::Result { panic!("GET /v1/me is not called without a credential") } - fn who(json: &str) -> WhoAmI { - serde_json::from_str(json).expect("WhoAmI") + fn no_userinfo(_: &StoredToken) -> Option { + panic!("the userinfo endpoint is not asked here") + } + + /// The `data` of a `GET /v1/me` answer, read the way the client reads it. + fn who(json: &str) -> Me { + let data: Value = serde_json::from_str(json).expect("json"); + let typed = serde_json::from_value(data.clone()).expect("WhoAmI"); + Me::from_answer(typed, &json!({ "data": data })) + } + + /// A `data` for a sign-in, with `user` spliced in as written (nothing when `None`). + fn sign_in_me(user: Option<&str>) -> Me { + let user = user.map_or_else(String::new, |u| format!(r#","user":{u}"#)); + who(&format!( + r#"{{"credential":{{"type":"oauth","prefix":null,"scopes":["supply_risk:read"]}}, + "tenant":{{"id":"6f1c2d3e-4b5a-4c6d-8e7f-0a1b2c3d4e5f","name":"Acme S.L.","type":"direct"}}, + "organization":{{"id":"7a1c2d3e-4b5a-4c6d-8e7f-0a1b2c3d4e5f","name":"Acme Group","type":"direct"}}, + "portfolio":null{user}}}"# + )) + } + + /// `whoami` of a sign-in whose `GET /v1/me` answers with `user` (as JSON text), and the + /// userinfo fallback gives `fallback`; the table and the JSON are returned. + fn whoami_of_a_sign_in(user: Option<&str>, fallback: Option<&str>) -> (String, Value) { + let (api, app) = (mockito::Server::new(), mockito::Server::new()); + let settings = settings_for(&api, &app); + let store = MemoryStore::default(); + store.save("default", &signed_in(&app, None)).expect("save"); + let me = || Ok(sign_in_me(user)); + let userinfo = |_: &StoredToken| fallback.map(str::to_owned); + let mut table = Vec::new(); + whoami_with(&settings, false, &store, false, &me, &userinfo, &mut table) + .expect("signed in"); + let mut json = Vec::new(); + whoami_with(&settings, false, &store, true, &me, &userinfo, &mut json).expect("signed in"); + ( + String::from_utf8(table).expect("utf8"), + serde_json::from_slice(&json).expect("json"), + ) + } + + #[test] + fn whoami_shows_the_user_that_the_api_gives_for_a_sign_in() { + let (text, printed) = whoami_of_a_sign_in( + Some(r#"{"id":"u-1","name":"Ana\u001b[31m Ruiz","email":"ana@acme.example"}"#), + None, + ); + assert!( + text.contains("User Ana\u{fffd}[31m Ruiz \n"), + "{text}" + ); + assert!(!text.contains('\u{1b}'), "the name is cleaned: {text:?}"); + // The same text and type as before: a string. + assert_eq!(printed["user"], "Ana\u{fffd}[31m Ruiz "); + assert!(!printed["user"].as_str().expect("string").contains('\u{1b}')); + let keys: Vec<&String> = printed.as_object().expect("object").keys().collect(); + let position = |k: &str| keys.iter().position(|key| *key == k).expect(k); + assert!(position("scope") < position("user") && position("user") < position("credential")); + } + + #[test] + fn whoami_prefers_the_api_over_the_userinfo_endpoint() { + let (text, printed) = whoami_of_a_sign_in( + Some(r#"{"id":"u-1","name":"Ana","email":"ana@acme.example"}"#), + Some("Someone Else"), + ); + assert!( + text.contains("User Ana \n"), + "{text}" + ); + assert_eq!(printed["user"], "Ana "); + } + + #[test] + fn whoami_without_a_user_from_the_api_falls_back_to_userinfo() { + // Today's API (no `user` field), `user: null`, and a malformed `user`. + for user in [None, Some("null"), Some(r#""x""#), Some("[]"), Some("{}")] { + let (text, printed) = whoami_of_a_sign_in(user, Some("Old Way ")); + assert!( + text.contains("User Old Way \n"), + "{user:?}: {text}" + ); + assert_eq!(printed["user"], "Old Way ", "{user:?}"); + } + } + + #[test] + fn whoami_without_any_user_has_no_user_line_and_does_not_fail() { + for user in [None, Some("null"), Some(r#""x""#), Some("42"), Some("{}")] { + let (text, printed) = whoami_of_a_sign_in(user, None); + assert!(!text.contains("User"), "{user:?}: {text}"); + assert_eq!(printed["user"], Value::Null, "{user:?}"); + // The rest of the answer is still shown. + assert!(text.contains("Org Acme Group (direct)\n"), "{text}"); + assert_eq!(printed["organization"]["name"], "Acme Group"); + } + } + + #[test] + fn whoami_uses_the_id_when_the_api_gives_no_name_or_email() { + let (text, _) = whoami_of_a_sign_in(Some(r#"{"id":"u-1","name":null}"#), None); + assert!(text.contains("User u-1\n"), "{text}"); + } + + #[test] + fn whoami_ignores_the_user_the_api_sends_for_a_key() { + let (api, app) = (mockito::Server::new(), mockito::Server::new()); + let settings = settings_for(&api, &app); + let me = || { + Ok(sign_in_me(Some( + r#"{"id":"u-1","name":"Ana","email":"a@b.example"}"#, + ))) + }; + let mut out = Vec::new(); + whoami_with( + &settings, + true, + &UnreachableStore, + false, + &me, + &no_userinfo, + &mut out, + ) + .expect("key set"); + let text = String::from_utf8(out).expect("utf8"); + assert!(!text.contains("User") && !text.contains("Ana"), "{text}"); + let mut out = Vec::new(); + whoami_with( + &settings, + true, + &UnreachableStore, + true, + &me, + &no_userinfo, + &mut out, + ) + .expect("key set"); + let printed: Value = serde_json::from_slice(&out).expect("json"); + assert!(printed.get("user").is_none(), "{printed}"); } #[test] @@ -847,7 +1064,16 @@ mod tests { )) }; let mut out = Vec::new(); - whoami_with(&settings, true, &UnreachableStore, false, &me, &mut out).expect("key set"); + whoami_with( + &settings, + true, + &UnreachableStore, + false, + &me, + &no_userinfo, + &mut out, + ) + .expect("key set"); let text = String::from_utf8(out).expect("utf8"); assert!( text.contains( @@ -864,7 +1090,16 @@ mod tests { assert!(!text.contains("Tenants"), "{text}"); let mut out = Vec::new(); - whoami_with(&settings, true, &UnreachableStore, true, &me, &mut out).expect("key set"); + whoami_with( + &settings, + true, + &UnreachableStore, + true, + &me, + &no_userinfo, + &mut out, + ) + .expect("key set"); let printed: Value = serde_json::from_slice(&out).expect("json"); assert_eq!( printed["tenant"]["id"], @@ -889,7 +1124,16 @@ mod tests { )) }; let mut out = Vec::new(); - whoami_with(&settings, true, &UnreachableStore, false, &me, &mut out).expect("key set"); + whoami_with( + &settings, + true, + &UnreachableStore, + false, + &me, + &no_userinfo, + &mut out, + ) + .expect("key set"); let text = String::from_utf8(out).expect("utf8"); assert!(!text.contains("Tenant "), "{text}"); assert!(text.contains("Org - (mssp)\n"), "{text}"); diff --git a/crates/hodeishield-cli/tests/contract.rs b/crates/hodeishield-cli/tests/contract.rs index ef72df6..3f2974b 100644 --- a/crates/hodeishield-cli/tests/contract.rs +++ b/crates/hodeishield-cli/tests/contract.rs @@ -528,6 +528,33 @@ fn json_whoami_fields() { me.assert(); } +/// A key has no user: whatever `user` the API sends in `GET /v1/me`, neither the table nor `--json` +/// shows one, and the fields stay as they were. +#[test] +fn whoami_with_a_key_ignores_a_user_in_the_answer() { + let env = Env::new(); + let mut server = mockito::Server::new(); + server + .mock("GET", "/v1/me") + .with_body( + r#"{"data":{"credential":{"type":"tenant","prefix":null,"scopes":["supply_risk:read"]}, + "tenant":{"id":"6f1c2d3e-4b5a-4c6d-8e7f-0a1b2c3d4e5f","name":"Acme S.L.","type":"direct"}, + "organization":{"id":"7a1c2d3e-4b5a-4c6d-8e7f-0a1b2c3d4e5f","name":"Acme Group","type":"direct"}, + "portfolio":null, + "user":{"id":"u-1","name":"Ana Ruiz","email":"ana@acme.example"}}}"#, + ) + .expect(2) + .create(); + let output = env.api(&server.url(), &["--json", "whoami"]); + assert_eq!(output.status.code(), Some(0), "{output:?}"); + let printed: Value = serde_json::from_str(&out(&output)).expect("json"); + assert!(printed.get("user").is_none(), "{printed}"); + assert!(!out(&output).contains("Ana Ruiz")); + let output = env.api(&server.url(), &["whoami"]); + assert_eq!(output.status.code(), Some(0), "{output:?}"); + assert!(!out(&output).contains("User") && !out(&output).contains("Ana")); +} + /// When `GET /v1/me` fails, `whoami` still succeeds with the local fields; the API's are null and /// standard error says why. #[test]