From e4bea44be13657e149c792f7c37118a1716ef523 Mon Sep 17 00:00:00 2001 From: SHAcollision Date: Tue, 18 Nov 2025 09:21:35 +0100 Subject: [PATCH] fix(sdk wasm): remove cookies at signout --- pubky-sdk/bindings/js/pkg/.gitignore | 1 + pubky-sdk/bindings/js/pkg/package.json | 2 +- pubky-sdk/bindings/js/pkg/tests/session.ts | 89 +++++++++++++++++ pubky-sdk/bindings/js/scripts/patch.mjs | 21 +++- pubky-sdk/bindings/js/src/actors/session.rs | 22 ++++- pubky-sdk/bindings/js/src/cookies.rs | 100 ++++++++++++++++++++ pubky-sdk/bindings/js/src/lib.rs | 1 + 7 files changed, 233 insertions(+), 3 deletions(-) create mode 100644 pubky-sdk/bindings/js/src/cookies.rs diff --git a/pubky-sdk/bindings/js/pkg/.gitignore b/pubky-sdk/bindings/js/pkg/.gitignore index 2104163c4..773827ca1 100644 --- a/pubky-sdk/bindings/js/pkg/.gitignore +++ b/pubky-sdk/bindings/js/pkg/.gitignore @@ -8,3 +8,4 @@ nodejs/ dist/ index.js index.cjs +snippets/ diff --git a/pubky-sdk/bindings/js/pkg/package.json b/pubky-sdk/bindings/js/pkg/package.json index e6a46fce4..a7150bcb4 100644 --- a/pubky-sdk/bindings/js/pkg/package.json +++ b/pubky-sdk/bindings/js/pkg/package.json @@ -10,7 +10,7 @@ }, "scripts": { "testnet": "cargo run -p pubky-testnet", - "build:test": "tsc --project tsconfig.json && mkdir -p dist && cp index.js dist/index.js", + "build:test": "rm -rf dist && tsc --project tsconfig.json && mkdir -p dist && cp index.js dist/index.js && cp -r snippets dist/snippets", "test": "npm run test-nodejs && npm run test-browser", "test-nodejs": "npm run build:test && node --require ./node-header.cjs node_modules/tape/bin/tape dist/tests/*.js -cov", "test-browser": "npm run build:test && browserify dist/tests/*.js -p esmify | npx tape-run", diff --git a/pubky-sdk/bindings/js/pkg/tests/session.ts b/pubky-sdk/bindings/js/pkg/tests/session.ts index bf79a32d7..7b8ffb7fe 100644 --- a/pubky-sdk/bindings/js/pkg/tests/session.ts +++ b/pubky-sdk/bindings/js/pkg/tests/session.ts @@ -36,6 +36,54 @@ type _PublicGetText = Assert< const PATH_AUTH_BASIC: Path = "/pub/example.com/auth-basic.txt"; +async function listEnvironmentCookieNames(): Promise { + if (typeof document !== "undefined" && typeof document.cookie === "string") { + return document.cookie + .split(";") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0) + .map((entry) => entry.split("=")[0]?.trim() ?? "") + .filter((name) => name.length > 0); + } + + const maybeFetch = globalThis.fetch as typeof fetch & { cookieJar?: unknown }; + const jar = maybeFetch?.cookieJar as + | { + getAllCookies?: (cb: (err: unknown, cookies?: unknown) => void) => void; + store?: { + getAllCookies?: (cb: (err: unknown, cookies?: unknown) => void) => void; + }; + } + | undefined; + + const getAllCookies = + jar?.getAllCookies?.bind(jar) ?? jar?.store?.getAllCookies?.bind(jar.store); + + if (typeof getAllCookies === "function") { + const cookies = (await new Promise((resolve, reject) => { + getAllCookies((err, items) => { + if (err) { + reject(err); + return; + } + + if (!Array.isArray(items)) { + resolve([]); + return; + } + + resolve(items); + }); + })) as Array<{ key?: unknown }>; + + return cookies + .map((cookie) => cookie?.key) + .filter((name): name is string => typeof name === "string" && name.length > 0); + } + + return []; +} + /** * Basic auth lifecycle: * - signer -> signup -> session (cookie stored) @@ -96,6 +144,47 @@ test("Auth: basic", async (t) => { t.end(); }); +test("Auth: signout removes persisted session cookies", async (t) => { + const sdk = Pubky.testnet(); + + const sessions: Array<{ session: SignupSession; user: string }> = []; + + for (let i = 0; i < 3; i += 1) { + const signer = sdk.signer(Keypair.random()); + const token = await createSignupToken(); + const session = await signer.signup(HOMESERVER_PUBLICKEY, token); + const user = session.info.publicKey.z32(); + + sessions.push({ session, user }); + } + + { + const cookieNames = await listEnvironmentCookieNames(); + const missing = sessions + .map(({ user }) => user) + .filter((user) => !cookieNames.includes(user)); + + t.deepEqual(missing, [], "signup should install session cookies for each user"); + } + + for (const { session } of sessions) { + await session.signout(); + } + + const remainingCookies = await listEnvironmentCookieNames(); + const lingering = sessions + .map(({ user }) => user) + .filter((user) => remainingCookies.includes(user)); + + t.deepEqual( + lingering, + [], + "cookies belonging to signed-out sessions should be removed from the environment", + ); + + t.end(); +}); + /** * Multi-user cookie isolation in one process: * - signup Alice and Bob (both cookies stored) diff --git a/pubky-sdk/bindings/js/scripts/patch.mjs b/pubky-sdk/bindings/js/scripts/patch.mjs index f9da4a515..c188597b6 100644 --- a/pubky-sdk/bindings/js/scripts/patch.mjs +++ b/pubky-sdk/bindings/js/scripts/patch.mjs @@ -2,7 +2,7 @@ // // Based on hacks from [this issue](https://github.com/rustwasm/wasm-pack/issues/1334) -import { readFile, writeFile, rename } from "node:fs/promises"; +import { readFile, writeFile, rename, rm } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import path, { dirname } from "node:path"; @@ -29,6 +29,13 @@ let patched = content .replace("require(`util`)", "globalThis") // attach to `imports` instead of module.exports .replace("= module.exports", "= imports") + // convert inline snippet require to ESM import + .replace( + "const { removeSessionCookie } = require(String.raw`", + "import { removeSessionCookie } from String.raw`", + ) + .replace("from String.raw`", "from \"") + .replace(/inline0\.js`\);/, 'inline0.js";') // Export classes .replace(/\nclass (.*?) \{/g, "\n export class $1 {") // Export functions @@ -113,6 +120,18 @@ await Promise.all( ), ); +const nodeSnippets = path.join(__dirname, "../pkg/nodejs/snippets"); +const pkgSnippets = path.join(__dirname, "../pkg/snippets"); + +try { + await rm(pkgSnippets, { recursive: true, force: true }); + await rename(nodeSnippets, pkgSnippets); +} catch (error) { + if (error.code !== "ENOENT") { + throw error; + } +} + // Add index.cjs headers const indexcjsPath = path.join(__dirname, `../pkg/index.cjs`); diff --git a/pubky-sdk/bindings/js/src/actors/session.rs b/pubky-sdk/bindings/js/src/actors/session.rs index 682804488..5ec8957d2 100644 --- a/pubky-sdk/bindings/js/src/actors/session.rs +++ b/pubky-sdk/bindings/js/src/actors/session.rs @@ -1,7 +1,14 @@ // js/src/wrappers/session.rs use wasm_bindgen::prelude::*; +#[cfg(target_arch = "wasm32")] +use wasm_bindgen::JsValue; +#[cfg(target_arch = "wasm32")] +use web_sys::console; + use super::storage::SessionStorage; +#[cfg(target_arch = "wasm32")] +use crate::cookies; use crate::js_error::{JsResult, PubkyError}; use crate::wrappers::session_info::SessionInfo; @@ -35,8 +42,21 @@ impl Session { /// @returns {Promise} #[wasm_bindgen] pub async fn signout(&self) -> JsResult<()> { + let cookie_name = self.0.info().public_key().to_string(); match self.0.clone().signout().await { - Ok(()) => Ok(()), + Ok(()) => { + #[cfg(target_arch = "wasm32")] + { + if let Err(err) = cookies::clear_session_cookie(&cookie_name).await { + console::warn_2( + &JsValue::from_str("Failed to clear session cookie locally"), + &err, + ); + } + } + + Ok(()) + } Err((e, _s)) => Err(PubkyError::from(e)), } } diff --git a/pubky-sdk/bindings/js/src/cookies.rs b/pubky-sdk/bindings/js/src/cookies.rs new file mode 100644 index 000000000..fa5c1f10e --- /dev/null +++ b/pubky-sdk/bindings/js/src/cookies.rs @@ -0,0 +1,100 @@ +// ## Why inline JS here? +// Rust + wasm-bindgen can *technically* mirror the same logic by reflecting on `globalThis.fetch`, constructing `Closure`s around the callback-based `getAllCookies` / `removeCookie` APIs, and wiring them into a `Promise`. I experimented with that approach first, but the result was brittle for a few reasons: +// 1. `fetch-cookie` exposes its `cookieJar` field and the underlying `tough-cookie` store only dynamically. Expressing Node\'s duck-typed callbacks with `wasm-bindgen` requires lots of `JsValue` casting, manual `Closure` lifetimes, and extra error handling that is hard to read and easy to leak. +// 2. Browser support needs a completely different code path (`document.cookie`), so the Rust version would still have to toggle on `cfg(target_arch = "wasm32")` and dive back into JS APIs there. +// 3. wasm-bindgen already emits a JS "snippet" file for inline glue. Hooking into that mechanism means we can write a small, idiomatic async function in JS while keeping the Rust surface area clean (`async fn clear_session_cookie(...)`). + +#[cfg(target_arch = "wasm32")] +use wasm_bindgen::prelude::*; + +#[cfg(target_arch = "wasm32")] +#[wasm_bindgen(inline_js = r#" +export async function removeSessionCookie(name) { + const global = globalThis; + const fetchImpl = global.fetch; + + if (fetchImpl && fetchImpl.cookieJar) { + const jar = fetchImpl.cookieJar; + const getAllCookies = + typeof jar.getAllCookies === "function" + ? jar.getAllCookies.bind(jar) + : jar.store && typeof jar.store.getAllCookies === "function" + ? jar.store.getAllCookies.bind(jar.store) + : undefined; + + if (typeof getAllCookies === "function") { + const cookies = await new Promise((resolve, reject) => { + getAllCookies((err, items) => { + if (err) { + reject(err); + return; + } + + if (!Array.isArray(items)) { + resolve([]); + return; + } + + resolve(items); + }); + }); + + const removals = cookies + .filter((cookie) => cookie && cookie.key === name) + .map((cookie) => { + const store = jar.store; + if ( + !store || + typeof store.removeCookie !== "function" || + typeof cookie.domain !== "string" || + typeof cookie.path !== "string" + ) { + return Promise.resolve(); + } + + return new Promise((resolve, reject) => { + store.removeCookie(cookie.domain, cookie.path, cookie.key, (err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); + }); + + if (removals.length > 0) { + await Promise.all(removals); + } + } + } + + if (typeof document !== "undefined" && typeof document.cookie === "string") { + const attributes = ["Max-Age=0", "Path=/", "SameSite=Lax"]; + + try { + if (global.location && global.location.protocol === "https:") { + attributes.push("Secure"); + } + } catch (_) { + // Ignore access errors (e.g. when location is unavailable). + } + + document.cookie = `${name}=; ${attributes.join("; ")}`; + } +} +"#)] +extern "C" { + #[wasm_bindgen(catch, js_name = "removeSessionCookie")] + pub async fn remove_session_cookie_js(name: &str) -> Result<(), JsValue>; +} + +/// Remove the session cookie for the provided user id from the current runtime. +/// +/// In Node.js environments this targets the `fetch-cookie` jar that wraps the +/// global `fetch` implementation (see `node-header.cjs`). In browser contexts it +/// falls back to clearing `document.cookie` for the given name. +#[cfg(target_arch = "wasm32")] +pub async fn clear_session_cookie(name: &str) -> Result<(), JsValue> { + remove_session_cookie_js(name).await +} diff --git a/pubky-sdk/bindings/js/src/lib.rs b/pubky-sdk/bindings/js/src/lib.rs index bd0b32813..b4d3eb68a 100644 --- a/pubky-sdk/bindings/js/src/lib.rs +++ b/pubky-sdk/bindings/js/src/lib.rs @@ -1,5 +1,6 @@ pub mod actors; pub mod client; +mod cookies; mod js_error; pub mod pubky; pub mod utils;