From 7b8a0f0711f30a776649810bc267da6d41631202 Mon Sep 17 00:00:00 2001 From: ottovlotto <142217647+ottovlotto@users.noreply.github.com> Date: Thu, 21 May 2026 20:11:45 +0200 Subject: [PATCH 1/2] test(e2e): cover logout cleanup with a synthesized session Uses createTestSession from @parity/product-sdk-terminal/testing to write valid session files to disk, then asserts dot logout removes them. Exercises the waitForLogout fallback path: the synthesized local account isn't registered on People chain so disconnect() fails with NoAllowanceError, and clearLocalAppStorage() is what actually deletes the files. Closes the only real coverage gap from #50; the rest of that issue's task list is either stale (allowances model changed) or out-of-reach for an automated test (real mobile signing). --- e2e/cli/session.test.ts | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/e2e/cli/session.test.ts b/e2e/cli/session.test.ts index a275b0e..66c3b6f 100644 --- a/e2e/cli/session.test.ts +++ b/e2e/cli/session.test.ts @@ -24,6 +24,7 @@ import { describe, test, expect, beforeEach, afterEach } from "vitest"; import { mkdtempSync, mkdirSync, writeFileSync, readdirSync, rmSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; +import { createTestSession } from "@parity/product-sdk-terminal/testing"; import { dot } from "./helpers/dot.js"; import { fixturePath } from "./fixtures/templates.js"; @@ -103,4 +104,33 @@ describe("session management", () => { // console.log(" No account is signed in.\n"); expect(result.stdout).toContain("No account is signed in"); }); + + test("logout clears local session files left by a previous login", async () => { + // Synthesize a session on disk as if QR pairing had completed. + // `createTestSession` is a "dev utility, not a stable contract" per the + // SDK — it hand-rolls the on-disk SCALE codec — so a minor bump of + // `@parity/product-sdk-terminal` could break this test if the format + // drifts. The SDK's own `testing.interop.test.ts` round-trip catches + // that upstream first. + const storageDir = join(tempHome, ".polkadot-apps"); + await createTestSession({ appId: "dot-cli", storageDir }); + const before = getSessionFiles(storageDir); + expect(before.length, "createTestSession should write at least one dot-cli_* file").toBeGreaterThan(0); + + // `adapter.sessions.disconnect()` will reject — the synthesized local + // account was never registered on the People chain so the statement + // store write fails with NoAllowanceError. `waitForLogout` catches + // that and falls back to `clearLocalAppStorage`, which is the path + // we want to validate: the user is unblocked even when the phone / + // statement store is unreachable. Status comes back `partial` rather + // than `success`, but the CLI still exits 0. + const result = await dot(["logout"], { home: tempHome, timeout: 90_000 }); + expect( + result.exitCode, + `logout exited non-zero: ${result.stdout}\n${result.stderr}`, + ).toBe(0); + + const after = getSessionFiles(storageDir); + expect(after, "logout should remove all dot-cli_* files").toEqual([]); + }); }); From 2c5343c328b818a1e853e0bc2e23a644b17000a1 Mon Sep 17 00:00:00 2001 From: ottovlotto <142217647+ottovlotto@users.noreply.github.com> Date: Tue, 26 May 2026 13:44:09 +0200 Subject: [PATCH 2/2] fix(auth): also clear local app storage on logout happy path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The e2e test added by the previous commit was failing on `dot-cli_SsoSessions.json` not being removed after a clean logout. Tracked to a real product gap, not a test assertion mismatch. ## What was happening The SDK's `adapter.sessions.disconnect(session)` does three things: 1. `sendDisconnectMessage()` — publishes a Disconnected statement so the paired mobile app drops its side 2. `ssoSessionRepository.filter(s => s.id !== session.id)` — removes this session from the in-memory list and writes the (now possibly empty) list back to disk 3. `userSecretRepository.clear(sessionId)` — unlinks the per-session UserSecrets file Step 2 writes BACK to `${DAPP_ID}_SsoSessions.json` with the filtered list — never unlinks the file. So after a single-session logout the file lingers on disk as `[]`. The CLI's `clearLocalAppStorage()` would unlink it, but `waitForLogout` only invoked it on the failure path — the happy path returned right after `onStatus({ step: "success" })`. Net effect across login → logout cycles: empty `dot-cli_SsoSessions.json` files accumulate in `~/.polkadot-apps/`. Not a security issue (no live credentials in the empty list), but it contradicts the spec's "dot logout cleans up the local session" guarantee and makes "is this user signed in?" harder to answer by inspecting the directory. ## Fix One-line addition in `src/utils/auth.ts::waitForLogout`: also call `clearLocalAppStorage()` in the `result.isOk()` branch, mirroring what the failure branch already does. The cleanup is scoped to `${DAPP_ID}_` files so it can't touch other polkadot-apps' state. Doc comment on `waitForLogout` updated to reflect both branches running the cleanup. The function is still scoped narrower than `rm -rf` and still best-effort under the hood (per-file unlinks are `.catch(() => {})`). ## Tests - New `src/utils/auth.test.ts` unit test ("clears local DAPP_ID files on the happy path too, not just the failure path") seeds `${DAPP_ID}_SsoSessions.json`, `${DAPP_ID}_UserSecrets_abc.json`, and a foreign-app file in the temp dir, runs `waitForLogout` with a disconnect adapter that returns Ok, and asserts the two DAPP_ID files are unlinked while the foreign-app file is preserved. Catches a future regression that reverts the cleanup call. - The existing happy-path test still passes (no files seeded, no cleanup to observe). - `e2e/cli/session.test.ts` comment refreshed — the test no longer depends on the disconnect call rejecting; the cleanup invariant holds on both branches. 497 unit tests pass; format/license/build all clean. --- e2e/cli/session.test.ts | 17 ++++++++++------- src/utils/auth.test.ts | 35 +++++++++++++++++++++++++++++++++++ src/utils/auth.ts | 23 +++++++++++++++++------ 3 files changed, 62 insertions(+), 13 deletions(-) diff --git a/e2e/cli/session.test.ts b/e2e/cli/session.test.ts index 66c3b6f..f3b5adf 100644 --- a/e2e/cli/session.test.ts +++ b/e2e/cli/session.test.ts @@ -117,13 +117,16 @@ describe("session management", () => { const before = getSessionFiles(storageDir); expect(before.length, "createTestSession should write at least one dot-cli_* file").toBeGreaterThan(0); - // `adapter.sessions.disconnect()` will reject — the synthesized local - // account was never registered on the People chain so the statement - // store write fails with NoAllowanceError. `waitForLogout` catches - // that and falls back to `clearLocalAppStorage`, which is the path - // we want to validate: the user is unblocked even when the phone / - // statement store is unreachable. Status comes back `partial` rather - // than `success`, but the CLI still exits 0. + // `waitForLogout` runs `clearLocalAppStorage()` on both the success and + // failure paths of `adapter.sessions.disconnect()` — so this test + // passes regardless of whether the disconnect statement actually + // round-trips on the testnet. The synthesized session has no real + // mobile peer, so the disconnect call's behaviour is implementation + // defined (statement-store may accept it as a fire-and-forget, + // or reject if Bulletin allowance is missing). What we're locking + // in here is the local-cleanup invariant: after a clean logout no + // `${DAPP_ID}_*` files remain in `~/.polkadot-apps/`, regardless of + // whether the user's phone is reachable. const result = await dot(["logout"], { home: tempHome, timeout: 90_000 }); expect( result.exitCode, diff --git a/src/utils/auth.test.ts b/src/utils/auth.test.ts index ec7111e..7f9e9e7 100644 --- a/src/utils/auth.test.ts +++ b/src/utils/auth.test.ts @@ -220,6 +220,41 @@ describe("waitForLogout", () => { expect(adapter.destroyCalls).toBe(1); }); + it("clears local DAPP_ID files on the happy path too, not just the failure path", async () => { + // Regression catcher: the SDK's `disconnect()` filters the session + // out of its in-memory list and writes the (now-empty) list back to + // `${DAPP_ID}_SsoSessions.json` — but doesn't unlink the file. Before + // this fix, the happy path returned `success` with the empty file + // still on disk, so `~/.polkadot-apps/` accumulated leftovers across + // login → logout cycles. We now run clearLocalAppStorage() on success + // too, so the file is gone after a clean logout. + const staleDir = join(appsDir, ".polkadot-apps"); + const { mkdirSync } = await import("node:fs"); + mkdirSync(staleDir, { recursive: true }); + const sessionsFile = join(staleDir, `${DAPP_ID}_SsoSessions.json`); + const secretsFile = join(staleDir, `${DAPP_ID}_UserSecrets_abc.json`); + const foreignFile = join(staleDir, "other-app_SsoSessions.json"); + writeFileSync(sessionsFile, "[]"); + writeFileSync(secretsFile, "{}"); + writeFileSync(foreignFile, "leave-me-alone"); + + const adapter = fakeAdapter(() => Promise.resolve(okResult(undefined))); + const handle = { + adapter, + address: "5Gxyz", + session: fakeSession(), + } as unknown as LogoutHandle; + const events: LogoutStatus[] = []; + + await waitForLogout(handle, (s) => events.push(s)); + + expect(events.at(-1)).toEqual({ step: "success", address: "5Gxyz" }); + expect(existsSync(sessionsFile)).toBe(false); + expect(existsSync(secretsFile)).toBe(false); + // Foreign app's files MUST remain untouched. + expect(existsSync(foreignFile)).toBe(true); + }); + it("falls back to local clear and emits partial when disconnect returns err", async () => { // Seed a stale session file so we can verify the fallback actually deletes it. const staleDir = join(appsDir, ".polkadot-apps"); diff --git a/src/utils/auth.ts b/src/utils/auth.ts index c05cc98..139cf0a 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -418,14 +418,17 @@ export async function findSession(): Promise { * Disconnect the given session. Reports progress via callback. * * Happy path: `adapter.sessions.disconnect()` sends a `Disconnected` statement - * so the paired mobile app drops its side of the connection, then clears the - * local session + user-secret files. + * so the paired mobile app drops its side of the connection, then we run + * `clearLocalAppStorage()` to unlink the `${DAPP_ID}_*` files. The SDK's + * `disconnect()` itself only filters the session out of the in-memory list + * and writes the (possibly-empty) list back to disk — without the explicit + * cleanup the SsoSessions file would linger as `[]`. * * If the remote notification fails (statement store unreachable, WebSocket - * torn down, …) we fall back to deleting the `${DAPP_ID}_*` files in - * `~/.polkadot-apps/` directly — strictly narrower than `rm -rf ~/.polkadot-apps` - * and keeps the user unblocked. The mobile app will show a stale pairing - * until it reconnects, which we surface via `partial`. + * torn down, …) we still run `clearLocalAppStorage()` — strictly narrower + * than `rm -rf ~/.polkadot-apps` and keeps the user unblocked. The mobile + * app will show a stale pairing until it reconnects, which we surface via + * `partial`. * * Always releases the adapter before returning. */ @@ -442,6 +445,14 @@ export async function waitForLogout( onStatus({ step: "disconnecting", address }); const result = await adapter.sessions.disconnect(session); if (result.isOk()) { + // Run the local cleanup pass on success too. The SDK's + // `disconnect()` filters the session out of `ssoSessionRepository` + // and writes the (now-empty) list back to disk, but it doesn't + // unlink the file — so `${DAPP_ID}_SsoSessions.json` lingers as + // `[]` on the filesystem. `clearLocalAppStorage()` removes it + // outright so `~/.polkadot-apps/` ends up tidy regardless of + // whether the mobile notification round-tripped. + await clearLocalAppStorage(); onStatus({ step: "success", address }); return; }