fix: auth failures when accounts.json registry is missing (#175)

Three related bugs caused all API calls to fail with "Access denied.
No credentials provided" even after successful `gws auth login`:

1. resolve_account() rejected valid credentials.enc as "legacy" when
   accounts.json was absent, instead of falling through to use them.

2. main.rs silently swallowed all auth errors (Err(_) => None),
   masking real failures behind a generic "no credentials" message.

3. auth login didn't include openid/email scopes, so
   fetch_userinfo_email() couldn't identify the user, causing
   credentials to be saved without an accounts.json entry.

{attribution.commit: ""}
This commit is contained in:
Harshal Patil
2026-03-05 17:24:37 -05:00
committed by GitHub
parent 78817b048c
commit a926e3f4c0
5 changed files with 176 additions and 14 deletions
+11
View File
@@ -0,0 +1,11 @@
---
"@googleworkspace/cli": patch
---
Fix auth failures when accounts.json registry is missing
Three related bugs caused all API calls to fail with "Access denied. No credentials provided" even after a successful `gws auth login`:
1. `resolve_account()` rejected valid `credentials.enc` as "legacy" when `accounts.json` was absent, instead of using them.
2. `main.rs` silently swallowed all auth errors, masking real failures behind a generic message.
3. `auth login` didn't include `openid`/`email` scopes, so `fetch_userinfo_email()` couldn't identify the user, causing credentials to be saved without an `accounts.json` entry.
+85 -11
View File
@@ -148,18 +148,10 @@ fn resolve_account(account: Option<&str>) -> anyhow::Result<Option<String>> {
);
}
}
// No account, no registry — check for legacy credentials
// No account, no registry — use legacy credentials if they exist
(None, None) => {
let legacy_path = credential_store::encrypted_credentials_path();
if legacy_path.exists() {
anyhow::bail!(
"Legacy credentials found at {}. \
gws now supports multiple accounts. \
Please run 'gws auth login' to upgrade your credentials.",
legacy_path.display()
);
}
// No registry, no legacy — fall through to standard credential loading
// Fall through to standard credential loading which will pick up
// the legacy credentials.enc file if it exists.
Ok(None)
}
}
@@ -611,6 +603,88 @@ mod tests {
assert_eq!(result.unwrap(), "my-test-token");
}
#[test]
fn test_resolve_account_no_registry_no_account_returns_none() {
// When there is no accounts.json and no explicit account,
// resolve_account should return Ok(None) to allow legacy
// credentials.enc to be picked up by load_credentials_inner.
let result = resolve_account(None);
// This will return Ok(None) if accounts.json doesn't exist,
// or Ok(Some(...)) if it does with a default. Either way, it
// should NOT error for the no-registry case.
assert!(result.is_ok());
}
#[tokio::test]
async fn test_load_credentials_encrypted_file() {
// Simulate an encrypted credentials file
let json = r#"{
"client_id": "enc_test_id",
"client_secret": "enc_test_secret",
"refresh_token": "enc_test_refresh",
"type": "authorized_user"
}"#;
let dir = tempfile::tempdir().unwrap();
let enc_path = dir.path().join("credentials.enc");
// Encrypt and write
let encrypted = crate::credential_store::encrypt(json.as_bytes()).unwrap();
std::fs::write(&enc_path, &encrypted).unwrap();
let res = load_credentials_inner(None, &enc_path, &PathBuf::from("/does/not/exist"))
.await
.unwrap();
match res {
Credential::AuthorizedUser(secret) => {
assert_eq!(secret.client_id, "enc_test_id");
assert_eq!(secret.client_secret, "enc_test_secret");
assert_eq!(secret.refresh_token, "enc_test_refresh");
}
_ => panic!("Expected AuthorizedUser from encrypted credentials"),
}
}
#[tokio::test]
async fn test_load_credentials_encrypted_takes_priority_over_default() {
// Encrypted credentials should be loaded before the default plaintext path
let enc_json = r#"{
"client_id": "encrypted_id",
"client_secret": "encrypted_secret",
"refresh_token": "encrypted_refresh",
"type": "authorized_user"
}"#;
let plain_json = r#"{
"client_id": "plaintext_id",
"client_secret": "plaintext_secret",
"refresh_token": "plaintext_refresh",
"type": "authorized_user"
}"#;
let dir = tempfile::tempdir().unwrap();
let enc_path = dir.path().join("credentials.enc");
let plain_path = dir.path().join("credentials.json");
let encrypted = crate::credential_store::encrypt(enc_json.as_bytes()).unwrap();
std::fs::write(&enc_path, &encrypted).unwrap();
std::fs::write(&plain_path, plain_json).unwrap();
let res = load_credentials_inner(None, &enc_path, &plain_path)
.await
.unwrap();
match res {
Credential::AuthorizedUser(secret) => {
assert_eq!(
secret.client_id, "encrypted_id",
"Encrypted credentials should take priority over plaintext"
);
}
_ => panic!("Expected AuthorizedUser"),
}
}
#[tokio::test]
#[serial_test::serial]
async fn test_get_token_env_var_empty_falls_through() {
+30 -1
View File
@@ -266,7 +266,7 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> {
}
// Determine scopes: explicit flags > interactive TUI > defaults
let scopes = resolve_scopes(
let mut scopes = resolve_scopes(
&filtered_args,
project_id.as_deref(),
services_filter.as_ref(),
@@ -311,6 +311,15 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> {
.await
.map_err(|e| GwsError::Auth(format!("Failed to build authenticator: {e}")))?;
// Ensure openid + email scopes are always present so we can identify the user
// via the userinfo endpoint after login.
let identity_scopes = ["openid", "https://www.googleapis.com/auth/userinfo.email"];
for s in &identity_scopes {
if !scopes.iter().any(|existing| existing == s) {
scopes.push(s.to_string());
}
}
// Request a token — this triggers the browser OAuth flow
let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
let token = auth
@@ -2085,4 +2094,24 @@ mod tests {
let result = filter_scopes_by_services(scopes.clone(), Some(&empty));
assert_eq!(result, scopes);
}
#[test]
fn mask_secret_long_string() {
let masked = mask_secret("GOCSPX-abcdefghijklmnopqrstuvwxyz");
assert_eq!(masked, "GOCS...wxyz");
}
#[test]
fn mask_secret_short_string() {
// 8 chars or fewer should be fully masked
assert_eq!(mask_secret("12345678"), "***");
assert_eq!(mask_secret("short"), "***");
assert_eq!(mask_secret(""), "***");
}
#[test]
fn mask_secret_boundary() {
// Exactly 9 chars — first 4 + last 4 with "..." in between
assert_eq!(mask_secret("123456789"), "1234...6789");
}
}
+38
View File
@@ -1527,6 +1527,44 @@ mod tests {
}
}
#[test]
fn test_handle_error_response_401_with_oauth_does_not_mask_error() {
// When auth was attempted (OAuth) but the server still returns 401,
// the error should be an API error with the actual message, NOT
// the generic "Access denied. No credentials provided" message.
let json_err = json!({
"error": {
"code": 401,
"message": "Request had invalid authentication credentials.",
"errors": [{ "reason": "authError" }]
}
})
.to_string();
let err = handle_error_response::<()>(
reqwest::StatusCode::UNAUTHORIZED,
&json_err,
&AuthMethod::OAuth,
)
.unwrap_err();
match err {
GwsError::Api {
code,
message,
reason,
..
} => {
assert_eq!(code, 401);
assert!(message.contains("invalid authentication credentials"));
assert_eq!(reason, "authError");
}
GwsError::Auth(msg) => {
panic!("Should NOT get generic Auth error when OAuth was used, got: {msg}");
}
other => panic!("Expected Api error, got: {other:?}"),
}
}
#[test]
fn test_handle_error_response_api_error() {
let json_err = json!({
+12 -2
View File
@@ -235,10 +235,20 @@ async fn run() -> Result<(), GwsError> {
// Get scopes from the method
let scopes: Vec<&str> = method.scopes.iter().map(|s| s.as_str()).collect();
// Authenticate: try OAuth, otherwise proceed unauthenticated
// Authenticate: try OAuth, fail with error if credentials exist but are broken
let (token, auth_method) = match auth::get_token(&scopes, account.as_deref()).await {
Ok(t) => (Some(t), executor::AuthMethod::OAuth),
Err(_) => (None, executor::AuthMethod::None),
Err(e) => {
// If credentials were found but failed (e.g. decryption error, invalid token),
// propagate the error instead of silently falling back to unauthenticated.
// Only fall back to None if no credentials exist at all.
let err_msg = format!("{e:#}");
if err_msg.starts_with("No credentials found") {
(None, executor::AuthMethod::None)
} else {
return Err(GwsError::Auth(format!("Authentication failed: {err_msg}")));
}
}
};
// Execute