HarperFast / HarperFast/studio
Sign-out aborts when localStorage is full, leaving the previous user's cached data and explorer token in the tab
- Dominant language
- TypeScript
- Stars
- 5
- Forks
- 4
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 40
Description
## Summary
When `localStorage` is at quota, signing out throws part-way through and never finishes. The previous user's cached data and the API Explorer's bearer token survive in the tab, and a subsequent sign-in as a **different user** in that same tab inherits them.
## Mechanism
`flagKeyAsSignedOut` writes to `localStorage` without a guard:
```ts
private flagKeyAsSignedOut(id: EntityIds) {
if (this.potentiallyAuthenticated[id]) {
delete this.potentiallyAuthenticated[id];
localStorage.setItem(this.potentiallyAuthenticatedKey, JSON.stringify(this.potentiallyAuthenticated));
}
}
```
It runs *third* in `signOutLocally`, before the two steps that actually drop credentials:
```ts
public signOutLocally(id: EntityIds): void {
this.flagForBasicAuth(id, null);
this.flagForFabricConnect(id, false);
this.flagKeyAsSignedOut(id); // <- throws here on a full store
this.updateConnectionIfChanged(id, false, null);
forgetEntitySettings(id); // never runs
this.bumpExplorerAuthEpoch(id); // never runs
}
```
So on `QuotaExceededError` the explorer's stored credential is neither deleted nor invalidated.
It gets worse one level up. `clearAuthStateLocally` calls `signOutAllLocally()` **first**:
```ts
export function clearAuthStateLocally(): void {
authStore.signOutAllLocally(); // <- throws out of here
queryClient.getMutationCache().clear();
queryClient.getQueryCache().clear();
clearLocalStorage();
clearSessionStorage();
}
```
The throw propagates, so the query-cache clear and both storage wipes never run. `signOutAllLocally`'s loop also aborts on the first entity, so `bumpExplorerAuthEpochAll` never fires either — meaning no other tab is told to drop its credentials.
Net: a sign-out that the user believes succeeded leaves the prior user's cached query data and explorer bearer token in place, for the life of the tab and across reloads.
## Reproduction
Verified with a probe test against the current `stage`:
```ts
authStore.setUserForIdAndKey('ins-a', 'ins-a-fqdn', { username: 'u' });
sessionStorage.setItem('ApiExplorerSettings', JSON.stringify({
'ins-a': { auth: { type: 'bearer', token: 'secret-token' }, authServer: 'http://x', authGeneration: 0 },
}));
const before = authStore.getExplorerAuthEpoch('ins-a');
vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { throw new DOMException('QuotaExceededError'); });
authStore.signOutLocally('ins-a');
```
`signOutLocally` throws `QuotaExceededError`, and `getExplorerAuthEpoch('ins-a')` is unchanged at `before`.
The entity must be in `potentiallyAuthenticated` for this to fire — `flagKeyAsSignedOut` is a no-op otherwise, which is exactly why existing tests don't catch it.
## Suggested direction
Not investigated to a conclusion, but the shape seems to be:
- Guard the bookkeeping write, the same way `writeExplorerInvalidation` and the `settings.ts` helpers already guard theirs. Losing the `potentiallyAuthenticated` marker is far less bad than aborting the sign-out.
- Order sign-out so credential destruction precedes best-effort bookkeeping, and make `clearAuthStateLocally` wipe storage even if the store's own teardown throws.
- Give the per-entity purge a `removeItem` fallback. `forgetEntitySettings` deletes by rewriting the map through `sessionStorage.setItem`, and `settings.ts` swallows that failure — so under shared quota pressure the credential stays on disk. `forgetAllEntitySettings` avoids this only because it uses `removeItem`.
## How this was found
A cross-model review round on #1660, which tried to fix a narrower symptom (a failed epoch write leaving a signed-out credential comparing as current). That PR is closed: its fix sat behind this bug and could never run on the path it targeted.
## Notes
- No known customer report — found by inspection and confirmed by probe.
- Requires `localStorage` at quota. A policy-disabled `localStorage` is a different case: it throws in the `AuthStore` constructor instead.
Contributor guide
Research direction
Start with signOutLocally and clearAuthStateLocally, then inspect flagKeyAsSignedOut and the storage helpers in settings.ts. Reproduce the provided quota-error probe and trace which credential, cache, storage, and epoch cleanup steps are skipped. Done means sign-out completes safely under quota pressure and all described local cleanup and invalidation steps still occur.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authentication
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100