diff --git a/src/auth/dcr.rs b/src/auth/dcr.rs index 088fed5..918998b 100644 --- a/src/auth/dcr.rs +++ b/src/auth/dcr.rs @@ -203,7 +203,13 @@ impl DcrClient { subdomain: Option<&str>, org_uuid: Option<&str>, ) -> String { - let scope = scopes.join(" "); + // Sort scopes so the printed authorize URL has a deterministic + // `scope=` parameter order — easier to diff and grep across runs. + // OAuth treats `scope` as an unordered set, so this is a no-op for + // the issuer. + let mut sorted_scopes: Vec<&str> = scopes.to_vec(); + sorted_scopes.sort(); + let scope = sorted_scopes.join(" "); let mut serializer = url::form_urlencoded::Serializer::new(String::new()); serializer .append_pair("response_type", "code") diff --git a/src/commands/auth.rs b/src/commands/auth.rs index b597ecc..69362be 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -14,6 +14,16 @@ where f(&mut **store) } +/// Parse a token's space-delimited scope claim and return scopes sorted +/// alphabetically. The OAuth issuer returns scopes in non-deterministic +/// order; sorting makes `pup auth list`/`pup auth status` output stable +/// and easier to scan or diff between invocations. +fn sorted_scopes(scope_claim: &str) -> Vec<&str> { + let mut scopes: Vec<&str> = scope_claim.split_whitespace().collect(); + scopes.sort(); + scopes +} + /// Format the trailing " (org: )" segment used in user-facing log lines. /// Returns an empty string when there's no org so the surrounding messages stay /// clean for the default-org case. @@ -64,10 +74,12 @@ pub async fn login( scopes.len() ); } else { + let mut display_scopes = scopes.clone(); + display_scopes.sort(); eprintln!( "🔑 Requesting {} scope(s): {}", scopes.len(), - scopes.join(", ") + display_scopes.join(", ") ); } @@ -380,11 +392,7 @@ pub fn status(cfg: &Config) -> Result<()> { .map(|dt| dt.with_timezone(&chrono::Local).to_rfc3339()) .unwrap_or_default(); - let scopes: Vec<&str> = tokens - .scope - .split_whitespace() - .filter(|s| !s.is_empty()) - .collect(); + let scopes = sorted_scopes(&tokens.scope); let json = serde_json::json!({ "authenticated": true, @@ -504,17 +512,12 @@ pub fn list(cfg: &Config) -> Result<()> { let expires_at = chrono::DateTime::from_timestamp(expires_at_ts, 0) .map(|dt| dt.with_timezone(&chrono::Local).to_rfc3339()) .unwrap_or_default(); - let scopes: Vec<&str> = t - .scope - .split_whitespace() - .filter(|s| !s.is_empty()) - .collect(); serde_json::json!({ "expires_at": expires_at, "has_refresh": !t.refresh_token.is_empty(), "org": s.org, "org_uuid": s.org_uuid, - "scopes": scopes, + "scopes": sorted_scopes(&t.scope), "site": s.site, "status": status, }) @@ -588,6 +591,22 @@ mod tests { } } + // ------------------------------------------------------------------ + // sorted_scopes — pure helper, no env / storage state. + // ------------------------------------------------------------------ + + #[test] + fn sorted_scopes_alphabetises_the_claim() { + let got = sorted_scopes("monitors_read apm_read dashboards_read"); + assert_eq!(got, vec!["apm_read", "dashboards_read", "monitors_read"]); + } + + #[test] + fn sorted_scopes_handles_empty_and_whitespace_input() { + assert!(sorted_scopes("").is_empty()); + assert!(sorted_scopes(" ").is_empty()); + } + // ------------------------------------------------------------------ // token() — the one function with an access_token bypass that never // touches the global STORAGE singleton. Hermetic.