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:
@@ -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
@@ -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
@@ -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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user