mirror of
https://github.com/purrgrammer/grimoire.git
synced 2026-06-05 10:11:12 +02:00
fix: prevent auth prompts when signer is unavailable
Three bugs fixed: 1. Race condition in emitState(): states$ was emitted before pendingChallenges$, so relay-state-manager's states$ subscriber would read stale pendingChallenges$.value. Now pendingChallenges$ is emitted first for consistent reads. 2. relay-state-manager only subscribed to states$, missing pendingChallenges$ changes. Now subscribes to both. 3. canAccountSign used constructor.name which is fragile under minification. Now uses account.type !== "readonly" (stable property from applesauce-accounts). Added 3 regression tests verifying pendingChallenges$.value consistency when observed from states$ subscribers. https://claude.ai/code/session_01XqrjeQVtJKw9uC1XAw6rqd
This commit is contained in:
@@ -677,6 +677,99 @@ describe("RelayAuthManager", () => {
|
||||
signer$.next(null);
|
||||
expect(manager.hasSignerAvailable()).toBe(false);
|
||||
});
|
||||
|
||||
it("pendingChallenges$.value should be consistent when states$ fires (no stale reads)", () => {
|
||||
// This test simulates what relay-state-manager does: subscribes to states$
|
||||
// and reads pendingChallenges$.value inside the callback. The value must
|
||||
// always reflect the current signer state (not stale data from a previous emission).
|
||||
signer$.next(createMockSigner());
|
||||
|
||||
const relay = createMockRelay("wss://relay.example.com");
|
||||
relay.connected$.next(true);
|
||||
manager.monitorRelay(relay);
|
||||
relay.challenge$.next("test-challenge");
|
||||
|
||||
// Collect pendingChallenges$.value as seen from within states$ subscriber
|
||||
const valuesSeenFromStates: number[] = [];
|
||||
manager.states$.subscribe(() => {
|
||||
valuesSeenFromStates.push(manager.pendingChallenges$.value.length);
|
||||
});
|
||||
|
||||
// Clear tracking (ignore emissions from setup)
|
||||
valuesSeenFromStates.length = 0;
|
||||
|
||||
// Signer removed (logout) — when states$ fires, pendingChallenges$.value must be 0
|
||||
signer$.next(null);
|
||||
|
||||
// Every states$ emission after signer removal should see empty pendingChallenges
|
||||
expect(valuesSeenFromStates.length).toBeGreaterThan(0);
|
||||
for (const count of valuesSeenFromStates) {
|
||||
expect(count).toBe(0);
|
||||
}
|
||||
});
|
||||
|
||||
it("pendingChallenges$.value should be consistent when signer goes null→non-null→null", () => {
|
||||
const relay = createMockRelay("wss://relay.example.com");
|
||||
relay.connected$.next(true);
|
||||
manager.monitorRelay(relay);
|
||||
relay.challenge$.next("test-challenge");
|
||||
|
||||
const valuesSeenFromStates: number[] = [];
|
||||
manager.states$.subscribe(() => {
|
||||
valuesSeenFromStates.push(manager.pendingChallenges$.value.length);
|
||||
});
|
||||
|
||||
// Signer available then immediately removed (e.g., brief account switch)
|
||||
valuesSeenFromStates.length = 0;
|
||||
signer$.next(createMockSigner());
|
||||
signer$.next(null);
|
||||
|
||||
// The last states$ emission should see 0 pending challenges
|
||||
expect(valuesSeenFromStates[valuesSeenFromStates.length - 1]).toBe(0);
|
||||
});
|
||||
|
||||
it("should never transiently expose stale challenges during signer removal", () => {
|
||||
// Regression test: pendingChallenges$ must emit [] BEFORE states$ when signer goes null
|
||||
signer$.next(createMockSigner());
|
||||
|
||||
const relay1 = createMockRelay("wss://relay1.example.com");
|
||||
const relay2 = createMockRelay("wss://relay2.example.com");
|
||||
relay1.connected$.next(true);
|
||||
relay2.connected$.next(true);
|
||||
manager.monitorRelay(relay1);
|
||||
manager.monitorRelay(relay2);
|
||||
relay1.challenge$.next("challenge-1");
|
||||
relay2.challenge$.next("challenge-2");
|
||||
|
||||
// Verify challenges are pending
|
||||
expect(manager.pendingChallenges$.value).toHaveLength(2);
|
||||
|
||||
// Track ALL emissions from both observables during signer removal
|
||||
const emissions: Array<{ source: string; pendingCount: number }> = [];
|
||||
manager.pendingChallenges$.subscribe((c) => {
|
||||
emissions.push({
|
||||
source: "pendingChallenges$",
|
||||
pendingCount: c.length,
|
||||
});
|
||||
});
|
||||
manager.states$.subscribe(() => {
|
||||
emissions.push({
|
||||
source: "states$",
|
||||
pendingCount: manager.pendingChallenges$.value.length,
|
||||
});
|
||||
});
|
||||
emissions.length = 0;
|
||||
|
||||
// Remove signer
|
||||
signer$.next(null);
|
||||
|
||||
// pendingChallenges$ must emit before states$, so by the time states$ fires,
|
||||
// pendingChallenges$.value is already 0
|
||||
const statesEmissions = emissions.filter((e) => e.source === "states$");
|
||||
for (const emission of statesEmissions) {
|
||||
expect(emission.pendingCount).toBe(0);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("states$ observable", () => {
|
||||
|
||||
@@ -431,10 +431,9 @@ export class RelayAuthManager {
|
||||
}
|
||||
|
||||
private emitState(): void {
|
||||
// Emit a snapshot of current states
|
||||
this.states$.next(new Map(this._relayStates));
|
||||
|
||||
// Derive and emit pending challenges
|
||||
// Derive and emit pending challenges FIRST.
|
||||
// This ensures that subscribers to states$ who read pendingChallenges$.value
|
||||
// see consistent, up-to-date data (not stale values from a previous emission).
|
||||
const now = Date.now();
|
||||
const challenges: PendingAuthChallenge[] = [];
|
||||
|
||||
@@ -456,6 +455,9 @@ export class RelayAuthManager {
|
||||
}
|
||||
|
||||
this.pendingChallenges$.next(challenges);
|
||||
|
||||
// Emit states snapshot after pendingChallenges is updated
|
||||
this.states$.next(new Map(this._relayStates));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -6,13 +6,15 @@ import accounts from "@/services/accounts";
|
||||
* Check if an account can sign events
|
||||
* Read-only accounts cannot sign and should not be prompted for auth
|
||||
*
|
||||
* Uses account.type (from applesauce-accounts) instead of constructor.name
|
||||
* to be robust against minification.
|
||||
*
|
||||
* @param account - The account to check (can be undefined)
|
||||
* @returns true if the account can sign, false otherwise
|
||||
*/
|
||||
export function canAccountSign(account: typeof accounts.active): boolean {
|
||||
if (!account) return false;
|
||||
const accountType = account.constructor.name;
|
||||
return accountType !== "ReadonlyAccount";
|
||||
return account.type !== "readonly";
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -50,11 +50,17 @@ class RelayStateManager {
|
||||
|
||||
this.initialized = true;
|
||||
|
||||
// Subscribe to auth manager state changes to re-notify listeners
|
||||
const authSub = relayAuthManager.states$.subscribe(() => {
|
||||
// Subscribe to auth manager state and pending challenge changes
|
||||
const stateSub = relayAuthManager.states$.subscribe(() => {
|
||||
this.notifyListeners();
|
||||
});
|
||||
this.authUnsubscribe = () => authSub.unsubscribe();
|
||||
const challengeSub = relayAuthManager.pendingChallenges$.subscribe(() => {
|
||||
this.notifyListeners();
|
||||
});
|
||||
this.authUnsubscribe = () => {
|
||||
stateSub.unsubscribe();
|
||||
challengeSub.unsubscribe();
|
||||
};
|
||||
|
||||
// Subscribe to existing relays
|
||||
pool.relays.forEach((relay) => {
|
||||
|
||||
Reference in New Issue
Block a user