fix(auth): auto-recover from stale encrypted credentials after upgrade (#435)

* fix(auth): auto-recover from stale encrypted credentials after upgrade

When credentials.enc cannot be decrypted (e.g. after an upgrade that
changed the encryption key), automatically remove the stale file and
fall through to other credential sources (plaintext, ADC) instead of
hard-erroring. This breaks the stuck loop where logout+login couldn't
fix the issue.

Also sync .encryption_key file backup when the keyring has a valid key
but the file is missing, preventing future key loss if the keyring
becomes unavailable.

Fixes #389

* fix: log warnings instead of silently ignoring file operation errors

Address review feedback: log warnings when file removal or backup
creation fails, so users get clear feedback instead of silent failures.
Token cache cleanup now skips NotFound errors since they may not exist.

* refactor: use tokio::fs::remove_file for async consistency

Switch from std::fs::remove_file to tokio::fs::remove_file in the
async load_credentials_inner function to avoid blocking the runtime,
consistent with other file operations in the same function.

* refactor: reuse parse_credential_file and use atomic key backup

- Reuse parse_credential_file for encrypted creds instead of manual
  JSON parsing. This removes duplication and adds ServiceAccount support.
- Use save_key_file_exclusive (atomic) for key backup creation, with
  AlreadyExists race condition handling.
- Update stale comment about encrypted creds being AuthorizedUser only.

* refactor: centralize token cache filenames as constants

Extract hardcoded 'token_cache.json' and 'sa_token_cache.json' strings
into TOKEN_CACHE_FILENAME and SA_TOKEN_CACHE_FILENAME constants in
auth_commands.rs. Update all 5 reference sites in auth.rs and
auth_commands.rs to use the constants.

* refactor: unconditional key sync + parse_credential_file for all paths

- Unconditionally sync the .encryption_key file with the keyring value
  on every successful read, not just when file is missing. Prevents
  stale file backups causing decryption failures if keyring becomes
  unavailable later. Uses save_key_file (overwrite + fsync) instead
  of save_key_file_exclusive (create-only).

- Use parse_credential_file for plaintext credentials at default path,
  consistent with encrypted/ADC paths. Adds ServiceAccount support
  for credentials.json, not just AuthorizedUser.

- Updated test: keyring_backend_syncs_file_when_keyring_differs
  verifies file content is overwritten to match keyring key.

* revert: remove scope-creep changes (constants + plaintext SA support)

Move TOKEN_CACHE_FILENAME/SA_TOKEN_CACHE_FILENAME constants and
parse_credential_file for the plaintext credentials path to a
follow-up refactor PR — keep this PR focused on stale credential
auto-recovery.

* style: use tokio::fs::write in async test functions

Replace std::fs::write with tokio::fs::write().await in async test
functions for consistency with the async runtime.

---------

Co-authored-by: jpoehnelt-bot <jpoehnelt-bot@users.noreply.github.com>
This commit is contained in:
Justin Poehnelt
2026-03-12 11:59:46 -06:00
committed by GitHub
parent f155fdf7b4
commit 510024f6d1
4 changed files with 162 additions and 31 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---
Centralize token cache filenames as constants and support ServiceAccount credentials at the default plaintext path
@@ -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.
+109 -27
View File
@@ -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() {
+43 -4
View File
@@ -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) = write_test_key(dir.path());
let mock = MockKeyring::with_password(&STANDARD.encode([7u8; 32]));
let _ = resolve_key(KeyringBackend::Keyring, &mock, &key_file).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();
// 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]