diff --git a/e2e/src/tests/auth.rs b/e2e/src/tests/auth.rs index 9cb8b8fa6..4b7737889 100644 --- a/e2e/src/tests/auth.rs +++ b/e2e/src/tests/auth.rs @@ -220,6 +220,46 @@ async fn multiple_users() { let b_sess = bob_session.info(); assert_eq!(b_sess.public_key(), &bob.public_key()); assert!(b_sess.capabilities().contains(&Capability::root())); + + // Export Bob's secret before signout to test later + let bob_secret = bob_session.export_secret(); + + // Both users can write + alice_session + .storage() + .put("/pub/test.txt", "alice-data") + .await + .unwrap(); + bob_session + .storage() + .put("/pub/test.txt", "bob-data") + .await + .unwrap(); + + // Sign out Bob + bob_session.signout().await.unwrap(); + + // Alice should still be able to write (cookie isolation) + alice_session + .storage() + .put("/pub/test2.txt", "alice-still-works") + .await + .unwrap(); + + // Bob's session should no longer work - import will fail because session was deleted + let bob_restore_err = PubkySession::import_secret(&bob_secret, Some(pubky.client().clone())) + .await + .unwrap_err(); + + // Should get either Authentication error or 401 Server error (no valid session found) + let is_expected_error = matches!(bob_restore_err, Error::Authentication(_)) + || matches!(bob_restore_err, Error::Request(RequestError::Server { status, .. }) if status == StatusCode::UNAUTHORIZED); + + assert!( + is_expected_error, + "bob session should fail after signout, got: {:?}", + bob_restore_err + ); } #[tokio::test] @@ -497,3 +537,289 @@ async fn test_republish_homeserver() { assert!(ts3 > ts2, "record should be republished when stale"); } + +/// Helper function to extract cookie ID and secret from exported token +/// Format: "pubkey:cookie_id:cookie_secret" +fn extract_cookie_from_export(export: &str) -> (String, String) { + let parts: Vec<&str> = export.split(':').collect(); + assert!( + parts.len() == 3, + "Export should have format pubkey:cookie_id:cookie_secret" + ); + (parts[1].to_string(), parts[2].to_string()) +} + +/// Helper function to extract pubkey and secret from exported token +/// Format: "pubkey:cookie_id:cookie_secret" +fn extract_pubkey_and_secret_from_export(export: &str) -> (String, String) { + let parts: Vec<&str> = export.split(':').collect(); + assert!( + parts.len() == 3, + "Export should have format pubkey:cookie_id:cookie_secret" + ); + (parts[0].to_string(), parts[2].to_string()) +} + +/// Test backward compatibility: SDK can import legacy 2-part format +#[tokio::test] +#[pubky_testnet::test] +async fn test_backward_compatibility_legacy_export_format() { + let testnet = EphemeralTestnet::start().await.unwrap(); + let server = testnet.homeserver(); + let pubky = testnet.sdk().unwrap(); + + let keypair = Keypair::random(); + let public_key = keypair.public_key(); + + // Create user and session + let signer = pubky.signer(keypair); + signer.signup(&server.public_key(), None).await.unwrap(); + let session = signer.signin().await.unwrap(); + + // Export in new format (3 parts) + let new_export = session.export_secret(); + println!("New format export: {}", new_export); + assert_eq!( + new_export.split(':').count(), + 3, + "New format should have 3 parts" + ); + + // Simulate legacy format (2 parts: pubkey:secret) + let parts: Vec<&str> = new_export.split(':').collect(); + let legacy_export = format!("{}:{}", parts[0], parts[2]); // pubkey:secret (skip cookie_id) + println!("Legacy format export: {}", legacy_export); + assert_eq!( + legacy_export.split(':').count(), + 2, + "Legacy format should have 2 parts" + ); + + // Test: Import legacy format should work + let restored_session = + PubkySession::import_secret(&legacy_export, Some(pubky.client().clone())) + .await + .unwrap(); + + // Verify the restored session works + let session_info = restored_session.info(); + assert_eq!(session_info.public_key(), &public_key); + + // Verify we can use the restored session + restored_session + .storage() + .put("/pub/test_legacy.txt", "legacy test") + .await + .unwrap(); + + let response = restored_session + .storage() + .get("/pub/test_legacy.txt") + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); +} + +/// Test that when multiple session cookies are present in a request: +/// 1. Invalid/malformed cookies are skipped +/// 2. Valid cookies are tried until one with proper capabilities is found +/// 3. First valid cookie lacking capabilities doesn't block second cookie with capabilities +/// 4. Legacy cookies (pubkey-named) are also checked along with UUID cookies +#[tokio::test] +#[pubky_testnet::test] +async fn test_multiple_session_cookies_authorization() { + let testnet = EphemeralTestnet::start().await.unwrap(); + let server = testnet.homeserver(); + let pubky = testnet.sdk().unwrap(); + let http_relay_url = testnet.http_relay().local_link_url(); + + let keypair = Keypair::random(); + let public_key = keypair.public_key(); + + // Create user with root session + let signer = pubky.signer(keypair); + signer.signup(&server.public_key(), None).await.unwrap(); + + // === Phase 1: Create three sessions with different scoped capabilities ===" + + // Session A: write access to /pub/posts/ only (UUID cookie) + let caps_a = Capabilities::builder().read_write("/pub/posts/").finish(); + + let auth_a = PubkyAuthFlow::builder(&caps_a) + .relay(http_relay_url.clone()) + .client(pubky.client().clone()) + .start() + .unwrap(); + + signer + .approve_auth(&auth_a.authorization_url()) + .await + .unwrap(); + + let session_a = auth_a.await_approval().await.unwrap(); + let export_a = session_a.export_secret(); + let (cookie_name_a, cookie_secret_a) = extract_cookie_from_export(&export_a); + + // Session B: write access to /pub/admin/ only (UUID cookie) + let caps_b = Capabilities::builder().read_write("/pub/admin/").finish(); + + let auth_b = PubkyAuthFlow::builder(&caps_b) + .relay(http_relay_url.clone()) + .client(pubky.client().clone()) + .start() + .unwrap(); + + signer + .approve_auth(&auth_b.authorization_url()) + .await + .unwrap(); + + let session_b = auth_b.await_approval().await.unwrap(); + let export_b = session_b.export_secret(); + let (cookie_name_b, cookie_secret_b) = extract_cookie_from_export(&export_b); + + // Session C: write access to /pub/legacy/ only using legacy cookie format + let caps_c = Capabilities::builder().read_write("/pub/legacy/").finish(); + + let auth_c = PubkyAuthFlow::builder(&caps_c) + .relay(http_relay_url) + .client(pubky.client().clone()) + .start() + .unwrap(); + + signer + .approve_auth(&auth_c.authorization_url()) + .await + .unwrap(); + + let session_c = auth_c.await_approval().await.unwrap(); + let export_c = session_c.export_secret(); + let (legacy_cookie_name, cookie_secret_c) = extract_pubkey_and_secret_from_export(&export_c); + + // Get the homeserver HTTP URL + let base_url = server + .icann_http_url() + .to_string() + .trim_end_matches('/') + .to_string(); + + // === Phase 2: Test Case 1 - Invalid cookie before valid one ===" + + // Make request to /pub/posts/file.txt with invalid cookie first, then Session A + let url = format!("{}/pub/posts/file.txt", base_url); + let client = pubky.client(); + let response = client + .request(Method::PUT, &url) + .header( + "Cookie", + format!( + "invalid_uuid=garbage_secret_1234567890; {}={}; {}={}; {}={}", + cookie_name_a, + cookie_secret_a, + cookie_name_b, + cookie_secret_b, + legacy_cookie_name, + cookie_secret_c + ), + ) + .header("Pubky-Host", public_key.to_string()) + .body(Vec::::new()) + .send() + .await + .unwrap(); + + assert!( + response.status().is_success(), + "Should skip invalid cookie and use Session A for /pub/posts/" + ); + + // === Phase 3: Test Case 2 - Wrong capability cookie before right one ===" + + // Make request to /pub/admin/settings with Session A first (lacks capability), then Session B + let url = format!("{}/pub/admin/settings", base_url); + let response = client + .request(Method::PUT, &url) + .header( + "Cookie", + format!( + "{}={}; {}={}; {}={}", + cookie_name_a, + cookie_secret_a, + cookie_name_b, + cookie_secret_b, + legacy_cookie_name, + cookie_secret_c + ), + ) + .header("Pubky-Host", public_key.to_string()) + .body(Vec::::new()) + .send() + .await + .unwrap(); + + assert!( + response.status().is_success(), + "Should skip Session A (no /pub/admin/ access) and use Session B, got: {}", + response.status() + ); + + // === Phase 4: Test Case 3 - Legacy cookie authorizes request ===" + + // Make request to /pub/legacy/data.txt where only Session C (legacy cookie) has access + let url = format!("{}/pub/legacy/data.txt", base_url); + let response = client + .request(Method::PUT, &url) + .header( + "Cookie", + format!( + "{}={}; {}={}; {}={}", + cookie_name_a, + cookie_secret_a, + cookie_name_b, + cookie_secret_b, + legacy_cookie_name, + cookie_secret_c + ), + ) + .header("Pubky-Host", public_key.to_string()) + .body(Vec::::new()) + .send() + .await + .unwrap(); + + assert!( + response.status().is_success(), + "Should skip UUID sessions and use Session C (legacy cookie) for /pub/legacy/, got: {}", + response.status() + ); + + // === Phase 5: Test Case 4 - No valid session has capability ===" + + // Make request to /pub/other/file.txt where no session has access + let url = format!("{}/pub/other/file.txt", base_url); + let response = client + .request(Method::PUT, &url) + .header( + "Cookie", + format!( + "{}={}; {}={}; {}={}", + cookie_name_a, + cookie_secret_a, + cookie_name_b, + cookie_secret_b, + legacy_cookie_name, + cookie_secret_c + ), + ) + .header("Pubky-Host", public_key.to_string()) + .body(Vec::::new()) + .send() + .await + .unwrap(); + + assert_eq!( + response.status(), + StatusCode::FORBIDDEN, + "Should return 403 when no session has required capability" + ); +} diff --git a/pubky-homeserver/src/core/layers/authz.rs b/pubky-homeserver/src/core/layers/authz.rs index b251fe7d4..c619aac8d 100644 --- a/pubky-homeserver/src/core/layers/authz.rs +++ b/pubky-homeserver/src/core/layers/authz.rs @@ -9,7 +9,7 @@ use axum::{ }; use futures_util::future::BoxFuture; use pkarr::PublicKey; -use std::{convert::Infallible, task::Poll}; +use std::{convert::Infallible, str::FromStr, task::Poll}; use tower::{Layer, Service}; use tower_cookies::Cookies; @@ -99,6 +99,9 @@ where } /// Authorize write (PUT or DELETE) for Public paths. +/// +/// Returns 401 if no session secrets found in cookies or if no valid sessions found for user. +/// Returns 403 if session exists for user but without required Capabilities async fn authorize( state: &AppState, method: &Method, @@ -124,76 +127,86 @@ async fn authorize( )); } - let session_secret = match session_secret_from_cookies(cookies, public_key) { - Some(session_secret) => session_secret, - None => { - tracing::warn!( - "No session secret found in cookies for pubky-host: {}", - public_key - ); - return Err(HttpError::unauthorized_with_message( - "No session secret found in cookies", - )); + let sessions = sessions_from_cookies(state, cookies, public_key).await?; + + // Check if any session has write access to the path + for session in sessions { + if session.capabilities.iter().any(|cap| { + path.starts_with(&cap.scope) + && cap + .actions + .contains(&pubky_common::capabilities::Action::Write) + }) { + // Found a valid session with required capabilities + return Ok(()); + } + } + + tracing::warn!( + "No session with write access to {} found for pubky-host: {}", + path, + public_key + ); + Err(HttpError::forbidden_with_message( + "Session does not have write access to path", + )) +} + +/// Get all valid sessions from cookies that belong to the specified user. +/// +/// Returns 401 if no session secrets found in cookies or if no valid sessions found for user. +/// Returns the list of valid sessions for the user. +pub async fn sessions_from_cookies( + state: &AppState, + cookies: &Cookies, + public_key: &PublicKey, +) -> HttpResult> { + let mut session_secrets = Vec::new(); + for cookie in cookies.list() { + if let Ok(secret) = SessionSecret::from_str(cookie.value()) { + session_secrets.push(secret); } - }; + } + + if session_secrets.is_empty() { + tracing::warn!( + "No session secret found in cookies for pubky-host: {}", + public_key + ); + return Err(HttpError::unauthorized_with_message( + "No session secret found in cookies", + )); + } - let session = - match SessionRepository::get_by_secret(&session_secret, &mut state.sql_db.pool().into()) - .await + // Try each session secret and collect those that: + // 1. Exist in the database + // 2. Belong to the correct user + let mut user_sessions = Vec::new(); + for session_secret in session_secrets { + let session = match SessionRepository::get_by_secret( + &session_secret, + &mut state.sql_db.pool().into(), + ) + .await { Ok(session) => session, Err(sqlx::Error::RowNotFound) => { - tracing::warn!( - "No session found in the database for session secret: {}, pubky: {}", - session_secret, - public_key - ); - return Err(HttpError::unauthorized_with_message( - "No session found for session secret", - )); + continue; } Err(e) => return Err(e.into()), }; - if &session.user_pubkey != public_key { - tracing::warn!( - "SessionInfo public key does not match pubky-host: {} != {}", - session.user_pubkey, - public_key - ); - return Err(HttpError::unauthorized_with_message( - "SessionInfo public key does not match pubky-host", - )); + if &session.user_pubkey == public_key { + user_sessions.push(session); + } } - if session.capabilities.iter().any(|cap| { - path.starts_with(&cap.scope) - && cap - .actions - .contains(&pubky_common::capabilities::Action::Write) - }) { - Ok(()) - } else { - tracing::warn!( - "SessionInfo {} pubkey {} does not have write access to {}. Access forbidden", - session_secret, - public_key, - path - ); - Err(HttpError::forbidden_with_message( - "Session does not have write access to path", - )) + if user_sessions.is_empty() { + tracing::warn!("No valid sessions found for pubky-host: {}", public_key); + return Err(HttpError::unauthorized_with_message( + "No valid session found for user", + )); } -} -/// Get the session secret from the cookies. -/// Returns None if the session secret is not found or invalid. -pub fn session_secret_from_cookies( - cookies: &Cookies, - public_key: &PublicKey, -) -> Option { - let value = cookies - .get(&public_key.to_string()) - .map(|c| c.value().to_string())?; - SessionSecret::new(value).ok() + Ok(user_sessions) } diff --git a/pubky-homeserver/src/core/routes/auth.rs b/pubky-homeserver/src/core/routes/auth.rs index 7c50dec1a..42a12d4a6 100644 --- a/pubky-homeserver/src/core/routes/auth.rs +++ b/pubky-homeserver/src/core/routes/auth.rs @@ -22,6 +22,7 @@ use tower_cookies::{ cookie::SameSite, Cookie, Cookies, }; +use uuid::Uuid; /// Creates a brand-new user if they do not exist, then logs them in by creating a session. /// 1) Check if signup tokens are required (signup mode is token_required). @@ -132,22 +133,36 @@ async fn create_session_and_cookie( let session_secret = SessionRepository::create(user.id, capabilities, &mut state.sql_db.pool().into()).await?; - // 3) Build and set cookie - let mut cookie = Cookie::new(user.public_key.to_string(), session_secret.to_string()); - cookie.set_path("/"); + // 3) Build and set cookies + // For backward compatibility we need send BOTH cookie formats: + // - New format: UUID cookie name + // - Legacy format: pubkey cookie name + let cookie_id = Uuid::new_v4().to_string(); + let mut new_cookie = Cookie::new(cookie_id, session_secret.to_string()); + new_cookie.set_path("/"); if is_secure(host) { - // Allow this cookie only to be sent over HTTPS. - cookie.set_secure(true); - cookie.set_same_site(SameSite::None); + new_cookie.set_secure(true); + new_cookie.set_same_site(SameSite::None); } - // Prevent javascript from accessing the cookie. - cookie.set_http_only(true); - // Set the cookie to expire in one year. + new_cookie.set_http_only(true); let one_year = Duration::days(365); let expiry = OffsetDateTime::now_utc() + one_year; - cookie.set_max_age(one_year); - cookie.set_expires(expiry); - cookies.add(cookie); + new_cookie.set_max_age(one_year); + new_cookie.set_expires(expiry); + cookies.add(new_cookie); + + // LEGACY FORMAT: pubkey named cookie (for backward compatibility with old SDK clients) + // TODO: Remove this after sufficient SDK adoption + let mut legacy_cookie = Cookie::new(user.public_key.to_string(), session_secret.to_string()); + legacy_cookie.set_path("/"); + if is_secure(host) { + legacy_cookie.set_secure(true); + legacy_cookie.set_same_site(SameSite::None); + } + legacy_cookie.set_http_only(true); + legacy_cookie.set_max_age(one_year); + legacy_cookie.set_expires(expiry); + cookies.add(legacy_cookie); let session = SessionInfo::new(&user.public_key, capabilities.clone(), None); let mut resp = session.serialize().into_response(); diff --git a/pubky-homeserver/src/core/routes/tenants/session.rs b/pubky-homeserver/src/core/routes/tenants/session.rs index 54f969dd9..e81167aa7 100644 --- a/pubky-homeserver/src/core/routes/tenants/session.rs +++ b/pubky-homeserver/src/core/routes/tenants/session.rs @@ -8,12 +8,14 @@ use tower_cookies::Cookies; use crate::{ core::{ err_if_user_is_invalid::get_user_or_http_error, extractors::PubkyHost, - layers::authz::session_secret_from_cookies, AppState, + layers::authz::sessions_from_cookies, AppState, }, persistence::sql::session::SessionRepository, shared::{HttpError, HttpResult}, }; +/// Return session information +/// Note: If there are multiple Cookies with valid sessions then only the first in the list will be returned. pub async fn session( State(state): State, cookies: Cookies, @@ -21,24 +23,23 @@ pub async fn session( ) -> HttpResult { get_user_or_http_error(pubky.public_key(), &mut state.sql_db.pool().into(), false).await?; - if let Some(secret) = session_secret_from_cookies(&cookies, pubky.public_key()) { - if let Ok(session) = - SessionRepository::get_by_secret(&secret, &mut state.sql_db.pool().into()).await - { - let legacy_session = session.to_legacy(); - let mut resp = legacy_session.serialize().into_response(); - resp.headers_mut().insert( - header::CONTENT_TYPE, - HeaderValue::from_static("application/octet-stream"), - ); - resp.headers_mut() - .insert(header::VARY, HeaderValue::from_static("cookie, pubky-host")); - resp.headers_mut().insert( - header::CACHE_CONTROL, - HeaderValue::from_static("private, must-revalidate"), - ); - return Ok(resp); - }; + let sessions = sessions_from_cookies(&state, &cookies, pubky.public_key()).await?; + + // Return the first session + if let Some(session) = sessions.into_iter().next() { + let legacy_session = session.to_legacy(); + let mut resp = legacy_session.serialize().into_response(); + resp.headers_mut().insert( + header::CONTENT_TYPE, + HeaderValue::from_static("application/octet-stream"), + ); + resp.headers_mut() + .insert(header::VARY, HeaderValue::from_static("cookie, pubky-host")); + resp.headers_mut().insert( + header::CACHE_CONTROL, + HeaderValue::from_static("private, must-revalidate"), + ); + return Ok(resp); } Err(HttpError::not_found()) @@ -50,8 +51,28 @@ pub async fn signout( ) -> HttpResult { // TODO: Set expired cookie to delete the cookie on client side. - if let Some(secret) = session_secret_from_cookies(&cookies, pubky.public_key()) { - SessionRepository::delete(&secret, &mut state.sql_db.pool().into()).await?; + let sessions = sessions_from_cookies(&state, &cookies, pubky.public_key()) + .await + .unwrap_or_default(); + + let mut errors = Vec::new(); + for session in sessions { + if let Err(e) = + SessionRepository::delete(&session.secret, &mut state.sql_db.pool().into()).await + { + tracing::error!( + "Failed to delete session {} for user {}: {}", + session.secret, + pubky.public_key(), + e + ); + errors.push(e); + } + } + + // If any sessions failed to delete then return the first error that occurred + if !errors.is_empty() { + return Err(errors.into_iter().next().unwrap().into()); } // Idempotent Success Response (200 OK) diff --git a/pubky-sdk/bindings/js/pkg/node-header.cjs b/pubky-sdk/bindings/js/pkg/node-header.cjs index 07de852dd..d5228bd71 100644 --- a/pubky-sdk/bindings/js/pkg/node-header.cjs +++ b/pubky-sdk/bindings/js/pkg/node-header.cjs @@ -1,4 +1,8 @@ const makeFetchCookie = require("fetch-cookie").default; +const tough = require("tough-cookie"); +// Create cookie jar explicitly so we can access it in tests +const jar = new tough.CookieJar(); let originalFetch = globalThis.fetch; -globalThis.fetch = makeFetchCookie(originalFetch); +globalThis.fetch = makeFetchCookie(originalFetch, jar); +globalThis.__cookieJar = jar; diff --git a/pubky-sdk/bindings/js/pkg/tests/auth.ts b/pubky-sdk/bindings/js/pkg/tests/auth.ts index 527e6347f..6e31c6929 100644 --- a/pubky-sdk/bindings/js/pkg/tests/auth.ts +++ b/pubky-sdk/bindings/js/pkg/tests/auth.ts @@ -11,15 +11,13 @@ import { IsExact, assertPubkyError, createSignupToken, + TESTNET_HTTP_RELAY, } from "./utils.js"; const HOMESERVER_PUBLICKEY = PublicKey.from( "8pinxxgqs41n4aididenw5apqp1urfmzdztr8jt4abrkdn435ewo", ); -// relay base (no trailing slash is fine; the flow will append the channel id) -const TESTNET_HTTP_RELAY = "http://localhost:15412/link"; - test("Auth: 3rd party signin", async (t) => { const sdk = Pubky.testnet(); diff --git a/pubky-sdk/bindings/js/pkg/tests/session.ts b/pubky-sdk/bindings/js/pkg/tests/session.ts index bf79a32d7..64344bd27 100644 --- a/pubky-sdk/bindings/js/pkg/tests/session.ts +++ b/pubky-sdk/bindings/js/pkg/tests/session.ts @@ -12,6 +12,7 @@ import { IsExact, assertPubkyError, createSignupToken, + TESTNET_HTTP_RELAY, } from "./utils.js"; const HOMESERVER_PUBLICKEY = PublicKey.from( @@ -99,9 +100,10 @@ test("Auth: basic", async (t) => { /** * Multi-user cookie isolation in one process: * - signup Alice and Bob (both cookies stored) + * - create a second scoped session for Bob (tests multiple sessions per user) * - generic client.fetch PUT with credentials:include writes under the correct user's host * - signout Bob; Alice remains authenticated and can still write - * - Bob can no longer write (401) + * - Bob can no longer write (both his root session AND scoped session invalidated) */ test("Auth: multi-user (cookies)", async (t) => { const sdk = Pubky.testnet(); @@ -122,6 +124,12 @@ test("Auth: multi-user (cookies)", async (t) => { t.ok(bobSession, "bob signed up"); const bobPk = bobSession.info.publicKey.z32(); + // 2b) Create a second scoped session for Bob + const bobFlow = sdk.startAuthFlow("/pub/posts/:rw", TESTNET_HTTP_RELAY); + await bob.approveAuthRequest(bobFlow.authorizationUrl); + const bobSession2 = await bobFlow.awaitApproval(); + t.ok(bobSession2, "bob created second scoped session"); + // 3) Write for Bob via generic client.fetch { const url = `https://_pubky.${bobPk}/pub/example.com/multi-bob.txt`; @@ -144,7 +152,11 @@ test("Auth: multi-user (cookies)", async (t) => { t.ok(r.ok, "alice can still write"); } - // 5) Sign out Bob + // 4b) Bob's second session can also write (to /pub/posts/) + await bobSession2.storage.putText("/pub/posts/test.txt" as any, "bob-session2"); + t.pass("bob's second scoped session works"); + + // 5) Sign out Bob (should invalidate BOTH of his sessions) await bobSession.signout(); // 6) Alice still authenticated after Bob signs out @@ -158,7 +170,7 @@ test("Auth: multi-user (cookies)", async (t) => { t.ok(r.ok, "alice still can write after bob signout"); } - // 7) Bob can no longer write + // 7) Bob's root session can no longer write { const url = `https://_pubky.${bobPk}/pub/example.com/multi-bob-2.txt`; const r = await sdk.client.fetch(url, { @@ -166,7 +178,18 @@ test("Auth: multi-user (cookies)", async (t) => { body: "should-fail", credentials: "include", }); - t.equal(r.status, 401, "bob write fails after signout"); + t.equal(r.status, 401, "bob's root session fails after signout"); + } + + // 7b) Bob's second session ALSO invalidated (signout removes ALL user sessions) + try { + await bobSession2.storage.putText("/pub/posts/should-fail.txt" as any, "fail"); + t.fail("bob's second session should be invalidated after signout"); + } catch (error: any) { + t.ok( + error.message.includes("401") || error.message.includes("Unauthorized"), + "bob's second session invalidated by signout", + ); } t.end(); @@ -363,3 +386,78 @@ test("Auth: signup/signout loops keep cookies and host in sync", async (t) => { t.end(); }); + +/** + * Tests that multiple session cookies with different capabilities for the same user + * don't overwrite each other in the browser's cookie jar. + * + * LEGACY COOKIES (bug - they overwrite): + * - Session A → Legacy cookie: pubkey=secretA + * - Session B → Legacy cookie: pubkey=secretB (overwrites A!) + * - Result: Only Session B's legacy cookie remains + * + * UUID COOKIES (fix - they coexist): + * - Session A → UUID cookie: uuid-A=secretA + * - Session B → UUID cookie: uuid-B=secretB (doesn't overwrite!) + * - Result: Both UUID cookies coexist in browser jar + */ +test("Auth: multiple session cookies don't overwrite each other", async (t) => { + const sdk = Pubky.testnet(); + + // Create user with root session + const keypair = Keypair.random(); + const signer = sdk.signer(keypair); + const signupToken = await createSignupToken(); + await signer.signup(HOMESERVER_PUBLICKEY, signupToken); + + const userPk = signer.publicKey.z32(); + + // === Create two sessions with different scoped capabilities === + // Server sends BOTH UUID and legacy cookies for each session + // In browsers, cookies are managed automatically by the browser's cookie jar + + // Session A: write access to /pub/posts/ only + const flowA = sdk.startAuthFlow("/pub/posts/:rw", TESTNET_HTTP_RELAY); + await signer.approveAuthRequest(flowA.authorizationUrl); + const sessionA = await flowA.awaitApproval(); + t.ok(sessionA, "Session A created with /pub/posts/ access"); + + // Session B: write access to /pub/admin/ only + const flowB = sdk.startAuthFlow("/pub/admin/:rw", TESTNET_HTTP_RELAY); + await signer.approveAuthRequest(flowB.authorizationUrl); + const sessionB = await flowB.awaitApproval(); + t.ok(sessionB, "Session B created with /pub/admin/ access"); + + // === Verify Cookie Jar Structure === + // After creating 2 sessions for THIS user, we should have exactly 1 legacy cookie for this user (pubkey name, last one overwrites previous) + const jar = (globalThis as any).__cookieJar; + if (jar) { + const cookies: any[] = await new Promise((resolve, reject) => { + jar.getCookies( + "http://localhost:15411", // homeserver URL + (err: any, cookies: any[]) => { + if (err) reject(err); + else resolve(cookies); + }, + ); + }); + + const thisUserLegacyCookies = cookies.filter((c: any) => c.key === userPk); + // Legacy cookies overwrite (only last one survives per user) + t.equal( + thisUserLegacyCookies.length, + 1, + "✓ Exactly 1 legacy cookie for this user (last session overwrites previous)", + ); + } + + // Verify that both GUID cookies are still present + await sessionA.storage.putText("/pub/posts/critical-test.txt" as any, "A works!"); + t.pass( + "Session A STILL works after creating B (UUID cookies coexist)", + ); + await sessionB.storage.putText("/pub/admin/settings" as any, "B works!"); + t.pass("✓ Session B works for /pub/admin/"); + + t.end(); +}); diff --git a/pubky-sdk/bindings/js/pkg/tests/utils.ts b/pubky-sdk/bindings/js/pkg/tests/utils.ts index 24e9c85b0..e67d739a1 100644 --- a/pubky-sdk/bindings/js/pkg/tests/utils.ts +++ b/pubky-sdk/bindings/js/pkg/tests/utils.ts @@ -10,6 +10,9 @@ import type { PubkyError } from "../index.js"; import type { Test } from "tape"; +// relay base (no trailing slash is fine; the flow will append the channel id) +export const TESTNET_HTTP_RELAY = "http://localhost:15412/link"; + export type PubkyErrorInstance = Error & PubkyError; export type Assert = T; diff --git a/pubky-sdk/src/actors/session/core.rs b/pubky-sdk/src/actors/session/core.rs index 944e6128c..cf0865f34 100644 --- a/pubky-sdk/src/actors/session/core.rs +++ b/pubky-sdk/src/actors/session/core.rs @@ -36,9 +36,10 @@ pub struct PubkySession { /// Known session for this session. pub(crate) info: SessionInfo, - /// Native-only, single session cookie secret for `_pubky.`. Never shared across agents. + /// Native-only, single session cookie for `_pubky.`. Never shared across agents. + /// Stored as (`cookie_name`, `cookie_value`) where name is UUID and value is session secret. #[cfg(not(target_arch = "wasm32"))] - pub(crate) cookie: String, + pub(crate) cookie: (String, String), } impl PubkySession { @@ -101,20 +102,36 @@ impl PubkySession { let bytes = response.bytes().await?; let info = SessionInfo::deserialize(&bytes)?; - // 3) Find the cookie named exactly as the user's pubky. - let cookie_name = info.public_key().to_string(); - let cookie = raw_set_cookies + // 3) Find the session cookie + // Support both legacy (name = pubkey) and new (name = UUID) formats + // Prefer the UUID cookie if both exist + let parsed_cookies: Vec<_> = raw_set_cookies .iter() .filter_map(|raw| cookie::Cookie::parse(raw.clone()).ok()) - .find(|c| c.name() == cookie_name) - .map(|c| c.value().to_string()) + .collect(); + + // Try to find UUID-based cookie first (new format - preferred) + let cookie = parsed_cookies + .iter() + .find(|c| { + // UUID format: name is not pubkey, value is 26 chars + c.value().len() == 26 && c.name() != info.public_key().to_string() + }) + // Fallback to legacy format (name = pubkey) + .or_else(|| { + parsed_cookies.iter().find(|c| { + c.name() == info.public_key().to_string() && c.value().len() == 26 + }) + }) .ok_or_else(|| AuthError::Validation("missing session cookie".into()))?; + let cookie_tuple = (cookie.name().to_string(), cookie.value().to_string()); + cross_log!(info, "Hydrated native session for {}", info.public_key()); Ok(Self { client, info, - cookie, + cookie: cookie_tuple, }) } } diff --git a/pubky-sdk/src/actors/session/persist.rs b/pubky-sdk/src/actors/session/persist.rs index c1579bfce..04a8229d3 100644 --- a/pubky-sdk/src/actors/session/persist.rs +++ b/pubky-sdk/src/actors/session/persist.rs @@ -11,7 +11,7 @@ use crate::{ impl PubkySession { /// Export the minimum data needed to restore this session later. - /// Returns a single compact secret token `:` + /// Returns a compact secret token `::` /// /// Useful for scripts that need restarting. Helps avoiding a new Auth flow /// from a signer on a script restart. @@ -20,12 +20,16 @@ impl PubkySession { /// securely. pub fn export_secret(&self) -> String { let public_key = self.info().public_key().to_string(); - let cookie = self.cookie.clone(); + let (cookie_id, cookie_secret) = &self.cookie; cross_log!(info, "Exporting session secret for {}", public_key); - format!("{public_key}:{cookie}") + format!("{public_key}:{cookie_id}:{cookie_secret}") } - /// Rehydrate a session from a compact secret token `:`. + /// Rehydrate a session from a compact secret token. + /// + /// Supports both formats for backward compatibility: + /// - New format: `::` (3 parts) + /// - Legacy format: `:` (2 parts, assumes cookie name = pubkey) /// /// Useful for scripts that need restarting. Helps avoiding a new Auth flow /// from a signer on a script restart. @@ -42,16 +46,38 @@ impl PubkySession { None => PubkyHttpClient::new()?, }; - // 2) Parse `:` (cookie may contain `:`, so split at the first one) - let (pk_str, cookie) = token - .split_once(':') - .ok_or_else(|| RequestError::Validation { - message: "invalid secret: expected `:`".into(), - })?; + // 2) Parse token + let parts: Vec<&str> = token.split(':').collect(); + let (public_key, cookie_id, cookie_secret) = match parts.len() { + // New format: :: + 3 => { + let public_key = + PublicKey::try_from(parts[0]).map_err(|_err| RequestError::Validation { + message: "invalid public key".into(), + })?; + (public_key, parts[1].to_string(), parts[2].to_string()) + } + // Legacy format: : (cookie name = pubkey) + 2 => { + let public_key = + PublicKey::try_from(parts[0]).map_err(|_err| RequestError::Validation { + message: "invalid public key".into(), + })?; + // Legacy: cookie name was the public key + ( + public_key.clone(), + public_key.to_string(), + parts[1].to_string(), + ) + } + _ => { + return Err(RequestError::Validation { + message: "invalid secret: expected `::` or legacy `:`".into(), + } + .into()); + } + }; - let public_key = PublicKey::try_from(pk_str).map_err(|_err| RequestError::Validation { - message: "invalid public key".into(), - })?; cross_log!(info, "Importing session secret for {}", public_key); // 3) Build minimal session; placeholder SessionInfo will be replaced after validation. @@ -59,7 +85,7 @@ impl PubkySession { let mut session = Self { client, info: placeholder, - cookie: cookie.to_string(), + cookie: (cookie_id, cookie_secret), }; // 4) Validate cookie and fetch authoritative SessionInfo diff --git a/pubky-sdk/src/actors/storage/core.rs b/pubky-sdk/src/actors/storage/core.rs index 4f9155017..0d4e16b1f 100644 --- a/pubky-sdk/src/actors/storage/core.rs +++ b/pubky-sdk/src/actors/storage/core.rs @@ -18,7 +18,7 @@ pub struct SessionStorage { pub(crate) client: PubkyHttpClient, pub(crate) user: PublicKey, #[cfg(not(target_arch = "wasm32"))] - pub(crate) cookie: String, + pub(crate) cookie: (String, String), } impl SessionStorage { @@ -67,10 +67,10 @@ impl SessionStorage { #[cfg(not(target_arch = "wasm32"))] pub(crate) fn with_session_cookie(&self, rb: RequestBuilder) -> RequestBuilder { - let cookie_name = self.user.to_string(); + let (cookie_name, cookie_value) = &self.cookie; rb.header( reqwest::header::COOKIE, - format!("{cookie_name}={}", self.cookie), + format!("{cookie_name}={cookie_value}"), ) } }