ADR-0073 Android Auto Backup & Session Restore#4427
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds ADR 0073 documenting Android Auto Backup/restore behavior where Android 6.0+ backups of Hive DB and SharedPreferences are restored before first app launch, causing unintended auto-login after uninstall/reinstall. Specifies required behavior for three scenarios (app update, uninstall+reinstall after backup, fresh install), notes that update vs restore are indistinguishable on the client, evaluates four mitigation options (disable backup, secure storage, file-timestamp heuristic, server-side token validation), includes Android 12+ note, and records status as "Proposed". Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr/0073-android-auto-backup-and-session-restore.md`:
- Around line 30-35: The decision matrix has an internal contradiction for Case
2: choose one invariant (either "reinstall must require login" or "first
reinstall auto-login acceptable") and apply it consistently; update the "Three
Cases" table row for Case 2, the corresponding entry in the "Option 4" table,
the summary paragraph, and any mentions in "Open Questions"/relevant discussion
so they all state the same requirement (refer to "Case 2", "Three Cases" table,
and "Option 4" table to locate the mismatched entries) and remove the
conflicting wording about "first reinstall auto-login" so acceptance criteria
are unambiguous.
- Around line 57-63: The ADR incorrectly states that "Option 1 logs out update
users"—update the document to reflect that setting allowBackup=false does not
impact in-place APK updates: change the prose that claims Option 1 logs out
update users and modify the table row for "Case 1 | Update app" so the "After
Fix" column shows "✅ Auto login" instead of "❌ Logged out"; ensure any nearby
references to "Case 1" or "allowBackup=false" are corrected to note that normal
updates retain SharedPreferences/databases/files and do not cause logout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1af9f2c0-240e-4211-b86d-9b96041879f3
📒 Files selected for processing (1)
docs/adr/0073-android-auto-backup-and-session-restore.md
|
|
||
| ## Context | ||
|
|
||
| Android 6.0+ enables Auto Backup by default, backing up Hive databases and SharedPreferences to Google Drive. On reinstall, this data is restored **before the app first launches** — causing the app to auto-login without re-authentication. |
There was a problem hiding this comment.
Upon uninstall is there an option to also clean the backup?
There was a problem hiding this comment.
No. Uninstall only removes local data, it does not delete cloud/server backups.
|
|
||
| **Option 2** stores the auth token in `flutter_secure_storage` (Android Keystore, never backed up). Uninstall clears it → force login. But users updating from old version have no token yet → also logged out, violating Case 1. | ||
|
|
||
| **Option 4** validates the session token server-side on each startup. The server invalidates old tokens on re-login, making Case 2 correct on the second reinstall. Only complete solution but requires backend changes and adds startup latency. |
There was a problem hiding this comment.
Please @guimard review this.
Also how about
Option 5: upon uninstall the app triggers a logout effectively forcing to reconnect?
There was a problem hiding this comment.
Option 5 is not feasible because the app cannot execute any code during uninstall → it cannot trigger a logout.
|
|
||
| | # | Scenario | Required Behavior | | ||
| |---|---|---| | ||
| | 1 | Update from old → new version | ✅ Keep auto login (hard requirement) | |
There was a problem hiding this comment.
| | 1 | Update from old → new version | ✅ Keep auto login (hard requirement) | | |
| | 1 | Update from old → new version | Keep user not be logging out | |
| | # | Scenario | Required Behavior | | ||
| |---|---|---| | ||
| | 1 | Update from old → new version | ✅ Keep auto login (hard requirement) | | ||
| | 2 | Backup → uninstall → reinstall new version | ❌ Force login | |
There was a problem hiding this comment.
| | 2 | Backup → uninstall → reinstall new version | ❌ Force login | | |
| | 2 | Backup → uninstall → reinstall new version | Force login | |
| |---|---|---| | ||
| | 1 | Update from old → new version | ✅ Keep auto login (hard requirement) | | ||
| | 2 | Backup → uninstall → reinstall new version | ❌ Force login | | ||
| | 3 | Fresh install | ❌ Force login | |
There was a problem hiding this comment.
| | 3 | Fresh install | ❌ Force login | | |
| | 3 | Fresh install | Force login | |
|
I understand there is decision to be made. Let's do this on wednesday meeting. |
3 participants
Descriptions
An ADR suggests a solution to the problem https://github.com/linagora/james-project-private/issues/1177
Related
#4393
Summary by CodeRabbit