starsetbyte/peregrine
Watch
1
0
Fork
You've already forked peregrine
 0
Code Issues 14 Pull requests 1 Projects Releases Packages Wiki Activity Actions

fix(auth): wipe account data at login boundary on DID change (closes #31) #36

Merged
starsetbyte merged 5 commits from fix/login-boundary-account-wipe into main 2026-06-22 19:18:40 +00:00
Conversation 0 Commits 5 Files changed 13 +518 -82
starsetbyte commented 2026-06-21 20:16:19 +00:00
Owner
Copy link

Summary

Closes the remaining leak vector from #31 (Multi-account support). The account-data wipe from PR #30 only fired in AuthRepository.logout(), but three session-establishing paths bypassed it:

  1. TokenAuthenticator calls sessionManager.clearSession() directly on refresh failure (no wipe).
  2. OAuthLoginViewModel.completeLogin() called sessionManager.saveOAuthSession() directly (bypassed AuthRepository).
  3. AuthRepository.login() (app-password) saved the session without comparing against the previous account.

Result: token-expiry forced logout → different account logs in → sees the previous account's cached timeline/feeds/drafts/notifications.

Fix — login-boundary chokepoint

  • AccountDidTracker (new): persists the last-known DID in its own SharedPreferences file (peregrine_account_boundary) that survives SessionManager.clearSession(). Without this, a forced logout would erase the last-known DID and the next different-account login wouldn't trigger a wipe.
  • AuthRepository.establishSession(did, saveSession) (new private): when the new DID differs from the tracker's last-known DID, wipes account-bound data before saving the new session. Both login() and the new completeOAuthLogin(session) route through it.
  • OAuthLoginViewModel now calls authRepository.completeOAuthLogin(session) instead of sessionManager.saveOAuthSession() directly — the OAuth path no longer bypasses the boundary.
  • logout() clears the tracker (logout already wiped; the next login should start fresh, not be treated as a switch).
  • Same-account re-login and first-ever login skip the wipe (tracker DID matches, or is null).

Scope note

This closes the leak-vector / "no account sees another's data" acceptance criterion of #31. The other criterion — per-DID data scoping (preserving each account's cache across switches without re-download) — remains a larger follow-up (separate DB/DataStore per DID or accountDid columns). That's a schema-level change better tracked as its own issue; this PR makes the safety guarantee airtight first.

Test plan

  • AuthRepositoryTest (7 new tests, 9 total):
    • app-password login wipes when DID changes
    • app-password login skips wipe for same account
    • first-ever login skips wipe
    • OAuth login wipes when DID differs
    • OAuth login skips wipe for same account
    • post-logout login skips wipe (tracker cleared)
    • logout clears the tracker
  • OAuthLoginViewModelTest updated for the new constructor (verifies authRepository.completeOAuthLogin is called)
  • Full suite green: 222 tests, 0 failures
  • assembleDebug builds (Hilt wiring verified — OAuthLoginViewModel constructor changed)
  • Device verification: account A session expires → B logs in → confirm A's cached data is gone

💘 Generated with Crush

## Summary Closes the remaining leak vector from **#31** (Multi-account support). The account-data wipe from PR #30 only fired in `AuthRepository.logout()`, but three session-establishing paths bypassed it: 1. **`TokenAuthenticator`** calls `sessionManager.clearSession()` directly on refresh failure (no wipe). 2. **`OAuthLoginViewModel.completeLogin()`** called `sessionManager.saveOAuthSession()` directly (bypassed `AuthRepository`). 3. **`AuthRepository.login()`** (app-password) saved the session without comparing against the previous account. Result: token-expiry forced logout → different account logs in → sees the previous account's cached timeline/feeds/drafts/notifications. ### Fix — login-boundary chokepoint - **`AccountDidTracker`** (new): persists the last-known DID in its own SharedPreferences file (`peregrine_account_boundary`) that **survives** `SessionManager.clearSession()`. Without this, a forced logout would erase the last-known DID and the next different-account login wouldn't trigger a wipe. - **`AuthRepository.establishSession(did, saveSession)`** (new private): when the new DID differs from the tracker's last-known DID, wipes account-bound data **before** saving the new session. Both `login()` and the new `completeOAuthLogin(session)` route through it. - **`OAuthLoginViewModel`** now calls `authRepository.completeOAuthLogin(session)` instead of `sessionManager.saveOAuthSession()` directly — the OAuth path no longer bypasses the boundary. - **`logout()`** clears the tracker (logout already wiped; the next login should start fresh, not be treated as a switch). - Same-account re-login and first-ever login skip the wipe (tracker DID matches, or is null). ### Scope note This closes the leak-vector / "no account sees another's data" acceptance criterion of #31. The other criterion — **per-DID data scoping** (preserving each account's cache across switches without re-download) — remains a larger follow-up (separate DB/DataStore per DID or `accountDid` columns). That's a schema-level change better tracked as its own issue; this PR makes the safety guarantee airtight first. ## Test plan - [x] `AuthRepositoryTest` (7 new tests, 9 total): - app-password login wipes when DID changes - app-password login skips wipe for same account - first-ever login skips wipe - OAuth login wipes when DID differs - OAuth login skips wipe for same account - post-logout login skips wipe (tracker cleared) - logout clears the tracker - [x] `OAuthLoginViewModelTest` updated for the new constructor (verifies `authRepository.completeOAuthLogin` is called) - [x] Full suite green: **222 tests, 0 failures** - [x] `assembleDebug` builds (Hilt wiring verified — `OAuthLoginViewModel` constructor changed) - [ ] Device verification: account A session expires → B logs in → confirm A's cached data is gone 💘 Generated with Crush
fix(auth): wipe account data at login boundary on DID change (closes #31)
All checks were successful
peregrine-ci / assembleDebug Build succeeded (20s)
Details
47123ddc75 
The account-data wipe from PR #30 only fired in AuthRepository.logout(),
but three session-establishing paths bypassed it:

  1. TokenAuthenticator's forced clearSession() on refresh failure
  2. OAuthLoginViewModel.completeLogin() → SessionManager.saveOAuthSession()
  3. AuthRepository.login() (app-password) saved the session without wiping

So a token-expiry forced logout followed by a *different* account's login
left the previous account's cached timeline, feeds, drafts, and
notifications visible to the new account.

Add an AccountDidTracker (separate SharedPreferences that survives
clearSession) and a single establishSession() chokepoint in
AuthRepository: when a newly authenticated DID differs from the
last-known one, wipe account-bound local data *before* saving the new
session. Both app-password login and OAuth login now route through it.
Same-account re-login and first-ever login skip the wipe.

This closes the remaining leak vector from #31. Per-DID data scoping
(the other acceptance criterion — preserving each account's cache across
switches without re-download) remains a larger follow-up.

💘 Generated with Crush
Merge branch 'main' into fix/login-boundary-account-wipe
All checks were successful
peregrine-ci / assembleDebug Build succeeded (15s)
Details
ce3ba44386
Merge branch 'main' into fix/login-boundary-account-wipe
All checks were successful
peregrine-ci / assembleDebug Build succeeded (12s)
Details
f3c922332d
[verified] fix: auth durability + exception narrowing (closes #37, #38, #39)
All checks were successful
peregrine-ci / assembleDebug Build succeeded (1m13s)
Details
112386c91b 
- SessionManager: apply() → commit() for saveSession, saveOAuthSession, clearSession, saveAppPassword
- OAuthSessionStore: apply() → commit() for clear(), updateTokens()
- build.gradle.kts: fix redirect scheme placeholder to match manifest (com.ffxxi)
- ComposerRepository: narrow catch(Exception) → IOException | HttpException in publish loop
- Add CODE_REVIEW_2026-06-22.md with full project review findings
fix: expand app-view-only endpoint allowlist in PdsRoutingInterceptor (closes #35)
All checks were successful
peregrine-ci / assembleDebug Build succeeded (10s)
Details
46344ece04 
Custom feeds, feed generator hydration, post threads, search (posts + actors),
and author feeds all aggregate across DIDs/PDSes — routing them to the user's
own PDS returned empty/garbage for non-bsky.social PDS users.

Updated test to iterate over all 7 app-view-only paths.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
auth
Authentication and OAuth

  
bug
Something is broken

  
composer
Post composition and drafts

  
correctness
Logic and state correctness

  
database
Room, migrations, persistence

  
durability
Data safety, persistence, recovery

  
enhancement
New feature or request

  
feeds
Feed management and preferences

  
interactions
Like, repost, reply interactions

  
notifications
Notification system

  
performance
Performance issues

  
reliability
Resilience against interruptions

  
safety
User safety and trust

  
security
Security concerns

No labels
auth
bug
composer
correctness
database
durability
enhancement
feeds
interactions
notifications
performance
reliability
safety
security
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
starsetbyte/peregrine!36
No description provided.