fix(auth): auto-recover from credential decryption failures after upgrade#398
fix(auth): auto-recover from credential decryption failures after upgrade#398zerone0x wants to merge 1 commit intogoogleworkspace:mainfrom
Conversation
…rade When the encryption key changes between versions (e.g. after the v0.11 keyring migration in googleworkspace#345), credentials.enc becomes undecryptable. The old behavior returned a hard error with no recovery path, leaving users stuck even after logout+login cycles. Changes: - auth.rs: When credentials.enc fails to decrypt, automatically remove the stale file (and associated token caches) and return a clear error directing the user to re-authenticate. This ensures the next `gws auth login` starts fresh with the current encryption key. - credential_store.rs: When the keyring returns a valid key but no .encryption_key file exists, create a file backup. This prevents key loss if the keyring becomes unavailable later (e.g. container restart, OS upgrade, daemon change). Fixes googleworkspace#389 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where users encountered persistent credential decryption failures after upgrading, particularly when the encryption key changed or became unavailable due to keyring migration. The changes introduce robust error handling to automatically clear stale credentials and improve the resilience of the encryption key storage, preventing users from getting stuck in an unrecoverable state and ensuring smoother transitions across application versions. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust auto-recovery mechanism for credential decryption failures, which is a significant improvement for user experience after upgrades. The changes automatically clean up stale credential files and provide clear instructions to the user. Additionally, it improves key persistence by creating a file backup of the keyring key. My main feedback is to correct some misleading comments in the error handling logic to ensure code clarity and maintainability.
| // 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. |
There was a problem hiding this comment.
The comment on line 216 is misleading. It states that the code will "fall through to other credential sources", but the anyhow::bail! macro on line 229 causes the function to return an error immediately, preventing any fall-through. This makes the code's control flow difficult to understand. The code's behavior (bailing out) is correct, but the comment should be updated to reflect this.
| // 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. | |
| // 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. |
| let sa_token_cache = enc_path.with_file_name("sa_token_cache.json"); | ||
| let _ = std::fs::remove_file(&sa_token_cache); | ||
|
|
||
| // Fall through to remaining credential sources below. |
There was a problem hiding this comment.
This comment is also misleading. The anyhow::bail! macro on the next line will cause the function to return an error immediately, preventing any fall-through to other credential sources. This comment should be removed to avoid confusion about the control flow.
2 participants
Summary
Fixes #389
After upgrading from a pre-keyring version to v0.11+, users encounter
Failed to decrypt credentials: Decryption failederrors that persist even afterlogout + logincycles. The root cause is that the encryption key can change between versions (the keyring migration in #345 could delete the.encryption_keyfile while the keyring entry might be lost later), leavingcredentials.encencrypted with a now-unavailable key.Changes:
auth.rs: Whencredentials.encfails to decrypt, automatically remove the stale file and associated token caches, then return a clear error directing the user togws auth login. This breaks the "stuck after logout+login" cycle by ensuring stale credentials don't block recovery.credential_store.rs: When the OS keyring returns a valid encryption key but no.encryption_keybackup file exists, create one. This prevents future key loss if the keyring becomes unavailable (container restart, daemon change, OS upgrade).Root Cause Analysis
~/.config/gws/.encryption_keycredentials.encis encrypted with the old (now lost) key and cannot be decryptedTest plan
cargo checkpassescargo clippypasses with no warningscargo test— all 552 tests passtest_load_credentials_corrupt_encrypted_file_is_removed— verifies stale credentials are cleaned up on decryption failurekeyring_backend_creates_file_backup_when_missing— verifies file backup is created when keyring has key but file is missing🤖 Generated with Claude Code