diff --git a/.changeset/auth-constants-refactor.md b/.changeset/auth-constants-refactor.md new file mode 100644 index 00000000..4877b9aa --- /dev/null +++ b/.changeset/auth-constants-refactor.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Centralize token cache filenames as constants and support ServiceAccount credentials at the default plaintext path diff --git a/.changeset/fix-stale-credentials-recovery.md b/.changeset/fix-stale-credentials-recovery.md new file mode 100644 index 00000000..6da86cd5 --- /dev/null +++ b/.changeset/fix-stale-credentials-recovery.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Auto-recover from stale encrypted credentials after upgrade: remove undecryptable `credentials.enc` and fall through to other credential sources (plaintext, ADC) instead of hard-erroring. Also sync encryption key file backup when keyring has key but file is missing. diff --git a/src/auth.rs b/src/auth.rs index 2b6716f1..0879c93a 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -205,36 +205,45 @@ async fn load_credentials_inner( ); } - // 2. Encrypted credentials (always AuthorizedUser for now) + // 2. Encrypted credentials if enc_path.exists() { - let json_str = credential_store::load_encrypted_from_path(enc_path) - .context("Failed to decrypt credentials")?; - - let creds: serde_json::Value = - serde_json::from_str(&json_str).context("Failed to parse decrypted credentials")?; - - let client_id = creds["client_id"] - .as_str() - .ok_or_else(|| anyhow::anyhow!("Missing client_id in encrypted credentials"))?; - let client_secret = creds["client_secret"] - .as_str() - .ok_or_else(|| anyhow::anyhow!("Missing client_secret in encrypted credentials"))?; - // refresh_token is optional now in some flows, but strictly required for this storage format - let refresh_token = creds["refresh_token"] - .as_str() - .ok_or_else(|| anyhow::anyhow!("Missing refresh_token in encrypted credentials"))?; - - return Ok(Credential::AuthorizedUser( - yup_oauth2::authorized_user::AuthorizedUserSecret { - client_id: client_id.to_string(), - client_secret: client_secret.to_string(), - refresh_token: refresh_token.to_string(), - key_type: "authorized_user".to_string(), - }, - )); + match credential_store::load_encrypted_from_path(enc_path) { + Ok(json_str) => { + return parse_credential_file(enc_path, &json_str).await; + } + Err(e) => { + // Decryption failed — the encryption key likely changed (e.g. after + // an upgrade that migrated keys between keyring and file storage). + // Remove the stale file so the next `gws auth login` starts fresh, + // and fall through to other credential sources (plaintext, ADC). + eprintln!( + "Warning: removing undecryptable credentials file ({}): {e:#}", + enc_path.display() + ); + if let Err(err) = tokio::fs::remove_file(enc_path).await { + eprintln!( + "Warning: failed to remove stale credentials file '{}': {err}", + enc_path.display() + ); + } + // Also remove stale token caches that used the old key. + for cache_file in ["token_cache.json", "sa_token_cache.json"] { + let path = enc_path.with_file_name(cache_file); + if let Err(err) = tokio::fs::remove_file(&path).await { + if err.kind() != std::io::ErrorKind::NotFound { + eprintln!( + "Warning: failed to remove stale token cache '{}': {err}", + path.display() + ); + } + } + } + // Fall through to remaining credential sources below. + } + } } - // 3. Plaintext credentials at default path (Default to AuthorizedUser) + // 3. Plaintext credentials at default path (AuthorizedUser) if default_path.exists() { return Ok(Credential::AuthorizedUser( yup_oauth2::read_authorized_user_secret(default_path) @@ -614,6 +623,79 @@ mod tests { } } + #[tokio::test] + #[serial_test::serial] + async fn test_load_credentials_corrupt_encrypted_file_is_removed() { + // When credentials.enc cannot be decrypted, the file should be removed + // automatically and the function should fall through to other sources. + let tmp = tempfile::tempdir().unwrap(); + let _home_guard = EnvVarGuard::set("HOME", tmp.path()); + let _adc_guard = EnvVarGuard::remove("GOOGLE_APPLICATION_CREDENTIALS"); + + let dir = tempfile::tempdir().unwrap(); + let enc_path = dir.path().join("credentials.enc"); + + // Write garbage data that cannot be decrypted. + tokio::fs::write(&enc_path, b"not-valid-encrypted-data-at-all-1234567890") + .await + .unwrap(); + assert!(enc_path.exists()); + + let result = + load_credentials_inner(None, &enc_path, &PathBuf::from("/does/not/exist")).await; + + // Should fall through to "No credentials found" (not a decryption error). + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!( + msg.contains("No credentials found"), + "Should fall through to final error, got: {msg}" + ); + assert!( + !enc_path.exists(), + "Stale credentials.enc must be removed after decryption failure" + ); + } + + #[tokio::test] + #[serial_test::serial] + async fn test_load_credentials_corrupt_encrypted_falls_through_to_plaintext() { + // When credentials.enc is corrupt but a valid plaintext file exists, + // the function should fall through and use the plaintext credentials. + let dir = tempfile::tempdir().unwrap(); + let enc_path = dir.path().join("credentials.enc"); + let plain_path = dir.path().join("credentials.json"); + + // Write garbage encrypted data. + tokio::fs::write(&enc_path, b"not-valid-encrypted-data-at-all-1234567890") + .await + .unwrap(); + + // Write valid plaintext credentials. + let plain_json = r#"{ + "client_id": "fallback_id", + "client_secret": "fallback_secret", + "refresh_token": "fallback_refresh", + "type": "authorized_user" + }"#; + tokio::fs::write(&plain_path, plain_json).await.unwrap(); + + let res = load_credentials_inner(None, &enc_path, &plain_path) + .await + .unwrap(); + + match res { + Credential::AuthorizedUser(secret) => { + assert_eq!( + secret.client_id, "fallback_id", + "Should fall through to plaintext credentials" + ); + } + _ => panic!("Expected AuthorizedUser from plaintext fallback"), + } + assert!(!enc_path.exists(), "Stale credentials.enc must be removed"); + } + #[tokio::test] #[serial_test::serial] async fn test_get_token_env_var_empty_falls_through() { diff --git a/src/credential_store.rs b/src/credential_store.rs index 43e76ff8..e985b76b 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -221,6 +221,15 @@ fn resolve_key( if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); + // Ensure file backup stays in sync with keyring so + // credentials survive keyring loss (e.g. after OS + // upgrades, container restarts, daemon changes). + if let Err(err) = save_key_file(key_file, &b64_key) { + eprintln!( + "Warning: failed to sync keyring backup file at '{}': {err}", + key_file.display() + ); + } return Ok(arr); } } @@ -527,13 +536,43 @@ mod tests { } #[test] - fn keyring_backend_keeps_file_when_keyring_succeeds() { + fn keyring_backend_creates_file_backup_when_missing() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + let expected = [7u8; 32]; + let mock = MockKeyring::with_password(&STANDARD.encode(expected)); + assert!(!key_file.exists(), "file must not exist before test"); + let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(result, expected); + assert!( + key_file.exists(), + "file backup must be created when keyring succeeds but file is missing" + ); + let file_key = read_key_file(&key_file).unwrap(); + assert_eq!( + file_key, expected, + "file backup must contain the keyring key" + ); + } + + #[test] + fn keyring_backend_syncs_file_when_keyring_differs() { use base64::{engine::general_purpose::STANDARD, Engine as _}; let dir = tempfile::tempdir().unwrap(); - let (_, key_file) = write_test_key(dir.path()); - let mock = MockKeyring::with_password(&STANDARD.encode([7u8; 32])); - let _ = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + // Write a file with one key, but put a different key in the keyring. + let (file_key, key_file) = write_test_key(dir.path()); + let keyring_key = [7u8; 32]; + assert_ne!(file_key, keyring_key, "keys must differ for this test"); + let mock = MockKeyring::with_password(&STANDARD.encode(keyring_key)); + let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(result, keyring_key, "should return keyring key"); assert!(key_file.exists(), "file must NOT be deleted"); + let synced = read_key_file(&key_file).unwrap(); + assert_eq!( + synced, keyring_key, + "file must be updated to match keyring key" + ); } #[test]