HarperFast / HarperFast/studio

Sign-out aborts when localStorage is full, leaving the previous user's cached data and explorer token in the tab

Open
#1,662 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.