From b052496cf368cbe089b8e59e022e36a1a8f4ca39 Mon Sep 17 00:00:00 2001 From: tomos Date: Thu, 13 Nov 2025 15:05:48 +0000 Subject: [PATCH 1/9] feat: Multiple cookie support via GUID Cookie name --- e2e/src/tests/auth.rs | 177 ++++++++++++++++++ pubky-homeserver/src/core/layers/authz.rs | 30 +-- pubky-homeserver/src/core/routes/auth.rs | 39 ++-- .../src/core/routes/tenants/session.rs | 4 +- pubky-sdk/src/actors/session/core.rs | 33 +++- pubky-sdk/src/actors/session/persist.rs | 54 ++++-- pubky-sdk/src/actors/storage/core.rs | 6 +- 7 files changed, 292 insertions(+), 51 deletions(-) diff --git a/e2e/src/tests/auth.rs b/e2e/src/tests/auth.rs index 9cb8b8fa6..6c99e12e1 100644 --- a/e2e/src/tests/auth.rs +++ b/e2e/src/tests/auth.rs @@ -497,3 +497,180 @@ 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()) +} + +/// Test backward compatibility: Server sends both UUID and legacy cookies +/// This comprehensive test verifies: +/// 1. Server sends BOTH cookie formats (UUID + pubkey) +/// 2. Both cookies can be used for authentication independently +/// 3. Multiple sessions with UUID cookies work (no collision) +/// 4. Multiple sessions with legacy cookies collide (expected legacy behavior) +#[tokio::test] +#[pubky_testnet::test] +async fn test_backward_compatibility_dual_cookies() { + 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 + let signer = pubky.signer(keypair); + signer.signup(&server.public_key(), None).await.unwrap(); + + println!("\n=== Phase 1: Create first session ==="); + let session_a = signer.signin().await.unwrap(); + let export_a = session_a.export_secret(); + println!("Session A export: {}", export_a); + + // Parse the export to extract both cookie formats + let (uuid_a, secret_a) = extract_cookie_from_export(&export_a); + let legacy_cookie_name = public_key.to_string(); + + println!("Session A UUID cookie: {}={}", uuid_a, secret_a); + println!( + "Session A legacy cookie: {}={}", + legacy_cookie_name, secret_a + ); + + // Test that Session A works (uses UUID cookie internally) + session_a + .storage() + .put("/pub/test_a.txt", "data from session A") + .await + .unwrap(); + println!("✅ Session A works for writing data"); + + println!("\n=== Phase 2: Create second session (same user) ==="); + let session_b = signer.signin().await.unwrap(); + let export_b = session_b.export_secret(); + println!("Session B export: {}", export_b); + + let (uuid_b, secret_b) = extract_cookie_from_export(&export_b); + + println!("Session B UUID cookie: {}={}", uuid_b, secret_b); + println!( + "Session B legacy cookie: {}={}", + legacy_cookie_name, secret_b + ); + + // Verify UUIDs are different + assert_ne!(uuid_a, uuid_b, "UUID cookies should be different"); + println!("✅ UUID cookies are different: no collision!"); + + println!("\n=== Phase 3: Test both sessions work independently ==="); + + // Session A should still work (UUID cookie prevents collision) + session_a + .storage() + .put("/pub/test_a2.txt", "more data from session A") + .await + .unwrap(); + println!("✅ Session A still works after Session B created"); + + // Session B should work + session_b + .storage() + .put("/pub/test_b.txt", "data from session B") + .await + .unwrap(); + println!("✅ Session B works"); + + println!("\n=== Phase 4: Verify both sessions remain valid ==="); + + // Verify Session A can read data + let response = session_a.storage().get("/pub/test_a.txt").await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + println!("✅ Session A can still read its data"); + + // Verify Session B can read data + let response = session_b.storage().get("/pub/test_b.txt").await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + println!("✅ Session B can read its data"); + + println!("\n=== Summary ==="); + println!("✅ Server sends both UUID and legacy cookies (backward compatible)"); + println!("✅ Both cookie formats work for authentication"); + println!("✅ UUID cookies: {} != {} (NO COLLISION)", uuid_a, uuid_b); + println!( + "⚠️ Legacy cookies: {} == {} (COLLISION in browser)", + legacy_cookie_name, legacy_cookie_name + ); + println!("✅ Both sessions remain valid in database"); + println!("✅ New SDK clients use UUID cookies → no collision"); + println!("✅ Old SDK clients use legacy cookies → work but have collision"); +} + +/// 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); + + println!("✅ Legacy 2-part format import works!"); + println!("✅ Backward compatibility maintained"); +} diff --git a/pubky-homeserver/src/core/layers/authz.rs b/pubky-homeserver/src/core/layers/authz.rs index b251fe7d4..be0ae547d 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; @@ -124,7 +124,7 @@ async fn authorize( )); } - let session_secret = match session_secret_from_cookies(cookies, public_key) { + let session_secret = match session_secret_from_cookies(cookies) { Some(session_secret) => session_secret, None => { tracing::warn!( @@ -186,14 +186,20 @@ async fn authorize( } } -/// 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() +/// Get the session secret from the cookies by iterating and validating. +/// Returns None if no valid session secret is found. +/// +/// Note: With UUID-based cookie names, we can't do direct lookup by public_key anymore. +/// Instead, we iterate all cookies in the request and find one with a valid session secret. +/// The caller (authorize function) validates the session belongs to the expected user. +pub fn session_secret_from_cookies(cookies: &Cookies) -> Option { + for cookie in cookies.list() { + // Try to parse cookie value as a session secret + if let Ok(secret) = SessionSecret::from_str(cookie.value()) { + // Found a valid session secret format + // The caller will validate it matches the expected user via DB lookup + return Some(secret); + } + } + None } 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..fd76022a6 100644 --- a/pubky-homeserver/src/core/routes/tenants/session.rs +++ b/pubky-homeserver/src/core/routes/tenants/session.rs @@ -21,7 +21,7 @@ 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 Some(secret) = session_secret_from_cookies(&cookies) { if let Ok(session) = SessionRepository::get_by_secret(&secret, &mut state.sql_db.pool().into()).await { @@ -50,7 +50,7 @@ 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()) { + if let Some(secret) = session_secret_from_cookies(&cookies) { SessionRepository::delete(&secret, &mut state.sql_db.pool().into()).await?; } diff --git a/pubky-sdk/src/actors/session/core.rs b/pubky-sdk/src/actors/session/core.rs index 944e6128c..af43650a4 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}"), ) } } From f8169455cefc50f2cce8a1159d0984c3193b8d95 Mon Sep 17 00:00:00 2001 From: tomos Date: Thu, 13 Nov 2025 15:18:55 +0000 Subject: [PATCH 2/9] feat: Support multipel Cookies in single request with valid sessions --- pubky-homeserver/src/core/layers/authz.rs | 110 ++++++++---------- .../src/core/routes/tenants/session.rs | 45 ++++--- 2 files changed, 74 insertions(+), 81 deletions(-) diff --git a/pubky-homeserver/src/core/layers/authz.rs b/pubky-homeserver/src/core/layers/authz.rs index be0ae547d..b5aca8f55 100644 --- a/pubky-homeserver/src/core/layers/authz.rs +++ b/pubky-homeserver/src/core/layers/authz.rs @@ -124,82 +124,68 @@ async fn authorize( )); } - let session_secret = match session_secret_from_cookies(cookies) { - 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 session_secrets = session_secrets_from_cookies(cookies); + 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 until we find one that: + // 1. Exists in the database + // 2. Belongs to the correct user + // 3. Has the required capabilities for the path + 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 { + continue; + } - 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 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 the session secret from the cookies by iterating and validating. -/// Returns None if no valid session secret is found. -/// -/// Note: With UUID-based cookie names, we can't do direct lookup by public_key anymore. -/// Instead, we iterate all cookies in the request and find one with a valid session secret. -/// The caller (authorize function) validates the session belongs to the expected user. -pub fn session_secret_from_cookies(cookies: &Cookies) -> Option { +/// Get all session secrets from the cookies by iterating and validating. +/// Returns a vector of all valid session secrets found. +pub fn session_secrets_from_cookies(cookies: &Cookies) -> Vec { + let mut secrets = Vec::new(); for cookie in cookies.list() { - // Try to parse cookie value as a session secret if let Ok(secret) = SessionSecret::from_str(cookie.value()) { - // Found a valid session secret format - // The caller will validate it matches the expected user via DB lookup - return Some(secret); + secrets.push(secret); } } - None + secrets } diff --git a/pubky-homeserver/src/core/routes/tenants/session.rs b/pubky-homeserver/src/core/routes/tenants/session.rs index fd76022a6..c545a401c 100644 --- a/pubky-homeserver/src/core/routes/tenants/session.rs +++ b/pubky-homeserver/src/core/routes/tenants/session.rs @@ -8,36 +8,42 @@ 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::session_secrets_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, pubky: PubkyHost, ) -> HttpResult { - get_user_or_http_error(pubky.public_key(), &mut state.sql_db.pool().into(), false).await?; + let user = 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) { + // Try each session secret until we find one that belongs to this user + for secret in session_secrets_from_cookies(&cookies) { 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); + // Check if this session belongs to the requesting user + if session.user_pubkey == user.public_key { + 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); + } }; } @@ -46,12 +52,13 @@ pub async fn session( pub async fn signout( State(state): State, cookies: Cookies, - pubky: PubkyHost, ) -> HttpResult { // TODO: Set expired cookie to delete the cookie on client side. - if let Some(secret) = session_secret_from_cookies(&cookies) { - SessionRepository::delete(&secret, &mut state.sql_db.pool().into()).await?; + // Delete all sessions found in all cookies + for secret in session_secrets_from_cookies(&cookies) { + // Ignore errors - session might not exist in DB + let _ = SessionRepository::delete(&secret, &mut state.sql_db.pool().into()).await; } // Idempotent Success Response (200 OK) From a31b5be0182f91fb199f7abe68afcbcae9e817d1 Mon Sep 17 00:00:00 2001 From: tomos Date: Thu, 13 Nov 2025 16:35:38 +0000 Subject: [PATCH 3/9] test: Comprehensive test for multiple Cookie scenarios --- e2e/src/tests/auth.rs | 313 ++++++++++++------ .../src/core/routes/tenants/session.rs | 5 +- 2 files changed, 214 insertions(+), 104 deletions(-) diff --git a/e2e/src/tests/auth.rs b/e2e/src/tests/auth.rs index 6c99e12e1..66c859850 100644 --- a/e2e/src/tests/auth.rs +++ b/e2e/src/tests/auth.rs @@ -509,107 +509,15 @@ fn extract_cookie_from_export(export: &str) -> (String, String) { (parts[1].to_string(), parts[2].to_string()) } -/// Test backward compatibility: Server sends both UUID and legacy cookies -/// This comprehensive test verifies: -/// 1. Server sends BOTH cookie formats (UUID + pubkey) -/// 2. Both cookies can be used for authentication independently -/// 3. Multiple sessions with UUID cookies work (no collision) -/// 4. Multiple sessions with legacy cookies collide (expected legacy behavior) -#[tokio::test] -#[pubky_testnet::test] -async fn test_backward_compatibility_dual_cookies() { - 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 - let signer = pubky.signer(keypair); - signer.signup(&server.public_key(), None).await.unwrap(); - - println!("\n=== Phase 1: Create first session ==="); - let session_a = signer.signin().await.unwrap(); - let export_a = session_a.export_secret(); - println!("Session A export: {}", export_a); - - // Parse the export to extract both cookie formats - let (uuid_a, secret_a) = extract_cookie_from_export(&export_a); - let legacy_cookie_name = public_key.to_string(); - - println!("Session A UUID cookie: {}={}", uuid_a, secret_a); - println!( - "Session A legacy cookie: {}={}", - legacy_cookie_name, secret_a - ); - - // Test that Session A works (uses UUID cookie internally) - session_a - .storage() - .put("/pub/test_a.txt", "data from session A") - .await - .unwrap(); - println!("✅ Session A works for writing data"); - - println!("\n=== Phase 2: Create second session (same user) ==="); - let session_b = signer.signin().await.unwrap(); - let export_b = session_b.export_secret(); - println!("Session B export: {}", export_b); - - let (uuid_b, secret_b) = extract_cookie_from_export(&export_b); - - println!("Session B UUID cookie: {}={}", uuid_b, secret_b); - println!( - "Session B legacy cookie: {}={}", - legacy_cookie_name, secret_b - ); - - // Verify UUIDs are different - assert_ne!(uuid_a, uuid_b, "UUID cookies should be different"); - println!("✅ UUID cookies are different: no collision!"); - - println!("\n=== Phase 3: Test both sessions work independently ==="); - - // Session A should still work (UUID cookie prevents collision) - session_a - .storage() - .put("/pub/test_a2.txt", "more data from session A") - .await - .unwrap(); - println!("✅ Session A still works after Session B created"); - - // Session B should work - session_b - .storage() - .put("/pub/test_b.txt", "data from session B") - .await - .unwrap(); - println!("✅ Session B works"); - - println!("\n=== Phase 4: Verify both sessions remain valid ==="); - - // Verify Session A can read data - let response = session_a.storage().get("/pub/test_a.txt").await.unwrap(); - assert_eq!(response.status(), StatusCode::OK); - println!("✅ Session A can still read its data"); - - // Verify Session B can read data - let response = session_b.storage().get("/pub/test_b.txt").await.unwrap(); - assert_eq!(response.status(), StatusCode::OK); - println!("✅ Session B can read its data"); - - println!("\n=== Summary ==="); - println!("✅ Server sends both UUID and legacy cookies (backward compatible)"); - println!("✅ Both cookie formats work for authentication"); - println!("✅ UUID cookies: {} != {} (NO COLLISION)", uuid_a, uuid_b); - println!( - "⚠️ Legacy cookies: {} == {} (COLLISION in browser)", - legacy_cookie_name, legacy_cookie_name +/// 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" ); - println!("✅ Both sessions remain valid in database"); - println!("✅ New SDK clients use UUID cookies → no collision"); - println!("✅ Old SDK clients use legacy cookies → work but have collision"); + (parts[0].to_string(), parts[2].to_string()) } /// Test backward compatibility: SDK can import legacy 2-part format @@ -670,7 +578,208 @@ async fn test_backward_compatibility_legacy_export_format() { .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 ===" - println!("✅ Legacy 2-part format import works!"); - println!("✅ Backward compatibility maintained"); + // 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/routes/tenants/session.rs b/pubky-homeserver/src/core/routes/tenants/session.rs index c545a401c..889732836 100644 --- a/pubky-homeserver/src/core/routes/tenants/session.rs +++ b/pubky-homeserver/src/core/routes/tenants/session.rs @@ -14,14 +14,15 @@ use crate::{ shared::{HttpError, HttpResult}, }; -/// Return session information +/// 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, pubky: PubkyHost, ) -> HttpResult { - let user = get_user_or_http_error(pubky.public_key(), &mut state.sql_db.pool().into(), false).await?; + let user = + get_user_or_http_error(pubky.public_key(), &mut state.sql_db.pool().into(), false).await?; // Try each session secret until we find one that belongs to this user for secret in session_secrets_from_cookies(&cookies) { From 67079ff9cfde025805541ebfeaa7c830763503ad Mon Sep 17 00:00:00 2001 From: tomos Date: Thu, 13 Nov 2025 17:28:44 +0000 Subject: [PATCH 4/9] test: Use Electron cookie management to confirm bug fixed --- pubky-sdk/bindings/js/pkg/tests/auth.ts | 1 - pubky-sdk/bindings/js/pkg/tests/session.ts | 124 +++++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/pubky-sdk/bindings/js/pkg/tests/auth.ts b/pubky-sdk/bindings/js/pkg/tests/auth.ts index 527e6347f..0ce3c1945 100644 --- a/pubky-sdk/bindings/js/pkg/tests/auth.ts +++ b/pubky-sdk/bindings/js/pkg/tests/auth.ts @@ -17,7 +17,6 @@ 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) => { diff --git a/pubky-sdk/bindings/js/pkg/tests/session.ts b/pubky-sdk/bindings/js/pkg/tests/session.ts index bf79a32d7..196cc0cad 100644 --- a/pubky-sdk/bindings/js/pkg/tests/session.ts +++ b/pubky-sdk/bindings/js/pkg/tests/session.ts @@ -18,6 +18,9 @@ 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"; + type Facade = ReturnType; type Signer = ReturnType; type SignupSession = Awaited>; @@ -363,3 +366,124 @@ 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. + * + * This test demonstrates BOTH the bug and the fix: + * + * 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 three 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"); + + // Session C: write access to /pub/legacy/ only + const flowC = sdk.startAuthFlow("/pub/legacy/:rw", TESTNET_HTTP_RELAY); + await signer.approveAuthRequest(flowC.authorizationUrl); + const sessionC = await flowC.awaitApproval(); + t.ok(sessionC, "Session C created with /pub/legacy/ access"); + + // === THE KEY INSIGHT === + // Server sends BOTH cookie formats for backward compatibility: + // 1. UUID cookie: = (unique name, won't overwrite) + // 2. Legacy cookie: = (same name for all sessions, WILL overwrite) + // + // WITHOUT the UUID fix: + // - All 3 sessions would send cookies named + // - Browser would only keep the last one (Session C) + // - Session A and B would fail because their cookies were overwritten + // + // WITH the UUID fix: + // - Each session also has a UUID cookie with unique name + // - All 3 UUID cookies coexist in browser jar + // - SDK uses UUID cookies internally, so all sessions work + + // === CRITICAL TEST: Verify Session A still works after creating B and C === + // Without UUID fix, Session A's cookie would have been overwritten by B and C + // (all would have same cookie name = pubkey, browser keeps only the last one) + // With UUID fix, all three UUID cookies coexist in the browser jar + try { + await sessionA.storage.putText("/pub/posts/critical-test.txt" as any, "A works!"); + t.pass( + "✓ FIX VERIFIED: Session A STILL works after creating B and C (UUID cookies coexist)", + ); + } catch (error) { + t.fail( + "✗ REGRESSION: Session A failed after creating B and C. UUID cookies may have been removed!", + ); + } + + // Verify Session B also works + try { + await sessionB.storage.putText("/pub/admin/settings" as any, "B works!"); + t.pass("✓ Session B works for /pub/admin/"); + } catch (error) { + t.fail("Session B should work for /pub/admin/"); + } + + // Verify Session C works + try { + await sessionC.storage.putText("/pub/legacy/data.txt" as any, "C works!"); + t.pass("✓ Session C works for /pub/legacy/"); + } catch (error) { + t.fail("Session C should work for /pub/legacy/"); + } + + // Verify capability isolation still works + try { + await sessionA.storage.putText("/pub/admin/test" as any, "should fail"); + t.fail("Session A should NOT have access to /pub/admin/"); + } catch (error) { + assertPubkyError(t, error); + t.pass("✓ Session A correctly denied access to /pub/admin/ (capability isolation works)"); + } + + // Test with credentials:include (browser automatically sends all cookies) + { + const url = `https://_pubky.${userPk}/pub/posts/auto-cookies.txt`; + const response = await sdk.client.fetch(url, { + method: "PUT", + body: "auto cookie selection", + credentials: "include", + }); + t.ok( + response.ok, + "✓ Browser sent all cookies with credentials:include, server selected Session A's UUID cookie", + ); + } + + t.end(); +}); From 715499bc900b31ddf59845d465f793b08f3c74eb Mon Sep 17 00:00:00 2001 From: tomos Date: Fri, 14 Nov 2025 10:27:04 +0000 Subject: [PATCH 5/9] fix: signout() to remove sessions from cookies beloning to tenant only --- e2e/src/tests/auth.rs | 40 +++++ pubky-homeserver/src/core/layers/authz.rs | 71 ++++++--- .../src/core/routes/tenants/session.rs | 61 ++++---- pubky-sdk/bindings/js/pkg/node-header.cjs | 6 +- pubky-sdk/bindings/js/pkg/tests/auth.ts | 3 +- pubky-sdk/bindings/js/pkg/tests/session.ts | 140 +++++++----------- pubky-sdk/bindings/js/pkg/tests/storage.ts | 2 +- pubky-sdk/bindings/js/pkg/tests/utils.ts | 3 + pubky-sdk/src/actors/session/core.rs | 2 +- 9 files changed, 185 insertions(+), 143 deletions(-) diff --git a/e2e/src/tests/auth.rs b/e2e/src/tests/auth.rs index 66c859850..853369fb7 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 403 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::FORBIDDEN); + + assert!( + is_expected_error, + "bob session should fail after signout, got: {:?}", + bob_restore_err + ); } #[tokio::test] diff --git a/pubky-homeserver/src/core/layers/authz.rs b/pubky-homeserver/src/core/layers/authz.rs index b5aca8f55..e5c70e587 100644 --- a/pubky-homeserver/src/core/layers/authz.rs +++ b/pubky-homeserver/src/core/layers/authz.rs @@ -124,6 +124,41 @@ async fn authorize( )); } + 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. +/// Returns 403 if cookies exist but no valid sessions found for the 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 session_secrets = session_secrets_from_cookies(cookies); if session_secrets.is_empty() { tracing::warn!( @@ -135,10 +170,10 @@ async fn authorize( )); } - // Try each session secret until we find one that: - // 1. Exists in the database - // 2. Belongs to the correct user - // 3. Has the required capabilities for the path + // 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, @@ -153,29 +188,19 @@ async fn authorize( Err(e) => return Err(e.into()), }; - if &session.user_pubkey != public_key { - continue; + 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) - }) { - // Found a valid session with required capabilities - return Ok(()); - } + if user_sessions.is_empty() { + tracing::warn!("No valid sessions found for pubky-host: {}", public_key); + return Err(HttpError::forbidden_with_message( + "No valid session found for user", + )); } - 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", - )) + Ok(user_sessions) } /// Get all session secrets from the cookies by iterating and validating. diff --git a/pubky-homeserver/src/core/routes/tenants/session.rs b/pubky-homeserver/src/core/routes/tenants/session.rs index 889732836..5cc46fdca 100644 --- a/pubky-homeserver/src/core/routes/tenants/session.rs +++ b/pubky-homeserver/src/core/routes/tenants/session.rs @@ -6,10 +6,7 @@ use axum::{ use tower_cookies::Cookies; use crate::{ - core::{ - err_if_user_is_invalid::get_user_or_http_error, extractors::PubkyHost, - layers::authz::session_secrets_from_cookies, AppState, - }, + core::{err_if_user_is_invalid::get_user_or_http_error, extractors::PubkyHost, AppState}, persistence::sql::session::SessionRepository, shared::{HttpError, HttpResult}, }; @@ -21,31 +18,27 @@ pub async fn session( cookies: Cookies, pubky: PubkyHost, ) -> HttpResult { - let user = - get_user_or_http_error(pubky.public_key(), &mut state.sql_db.pool().into(), false).await?; + get_user_or_http_error(pubky.public_key(), &mut state.sql_db.pool().into(), false).await?; - // Try each session secret until we find one that belongs to this user - for secret in session_secrets_from_cookies(&cookies) { - if let Ok(session) = - SessionRepository::get_by_secret(&secret, &mut state.sql_db.pool().into()).await - { - // Check if this session belongs to the requesting user - if session.user_pubkey == user.public_key { - 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 = + crate::core::layers::authz::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()) @@ -53,13 +46,17 @@ pub async fn session( pub async fn signout( State(state): State, cookies: Cookies, + pubky: PubkyHost, ) -> HttpResult { // TODO: Set expired cookie to delete the cookie on client side. - // Delete all sessions found in all cookies - for secret in session_secrets_from_cookies(&cookies) { - // Ignore errors - session might not exist in DB - let _ = SessionRepository::delete(&secret, &mut state.sql_db.pool().into()).await; + let sessions = + crate::core::layers::authz::sessions_from_cookies(&state, &cookies, pubky.public_key()) + .await + .unwrap_or_default(); + + for session in sessions { + let _ = SessionRepository::delete(&session.secret, &mut state.sql_db.pool().into()).await; } // 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 0ce3c1945..6e31c6929 100644 --- a/pubky-sdk/bindings/js/pkg/tests/auth.ts +++ b/pubky-sdk/bindings/js/pkg/tests/auth.ts @@ -11,14 +11,13 @@ import { IsExact, assertPubkyError, createSignupToken, + TESTNET_HTTP_RELAY, } from "./utils.js"; const HOMESERVER_PUBLICKEY = PublicKey.from( "8pinxxgqs41n4aididenw5apqp1urfmzdztr8jt4abrkdn435ewo", ); -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 196cc0cad..db4041393 100644 --- a/pubky-sdk/bindings/js/pkg/tests/session.ts +++ b/pubky-sdk/bindings/js/pkg/tests/session.ts @@ -12,15 +12,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"; - type Facade = ReturnType; type Signer = ReturnType; type SignupSession = Awaited>; @@ -87,7 +85,7 @@ test("Auth: basic", async (t) => { body: "should fail", credentials: "include", }); - t.equal(res401.status, 401, "PUT without session returns 401"); + t.equal(res401.status, 403, "PUT without session returns 403"); // 5) Sign in again (local key proves identity) const session2 = await signer.signin(); @@ -102,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(); @@ -125,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`; @@ -147,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 @@ -161,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, { @@ -169,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, 403, "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("403") || error.message.includes("Forbidden"), + "bob's second session invalidated by signout", + ); } t.end(); @@ -331,7 +351,7 @@ test("Auth: signup/signout loops keep cookies and host in sync", async (t) => { body: "should-401", credentials: "include", }); - t.equal(r.status, 401, "signed-out user cannot write"); + t.equal(r.status, 403, "signed-out user cannot write"); } const u2 = await signupAndMark("user#2:hello"); @@ -371,8 +391,6 @@ test("Auth: signup/signout loops keep cookies and host in sync", async (t) => { * Tests that multiple session cookies with different capabilities for the same user * don't overwrite each other in the browser's cookie jar. * - * This test demonstrates BOTH the bug and the fix: - * * LEGACY COOKIES (bug - they overwrite): * - Session A → Legacy cookie: pubkey=secretA * - Session B → Legacy cookie: pubkey=secretB (overwrites A!) @@ -394,7 +412,7 @@ test("Auth: multiple session cookies don't overwrite each other", async (t) => { const userPk = signer.publicKey.z32(); - // === Create three sessions with different scoped capabilities === + // === 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 @@ -410,80 +428,36 @@ test("Auth: multiple session cookies don't overwrite each other", async (t) => { const sessionB = await flowB.awaitApproval(); t.ok(sessionB, "Session B created with /pub/admin/ access"); - // Session C: write access to /pub/legacy/ only - const flowC = sdk.startAuthFlow("/pub/legacy/:rw", TESTNET_HTTP_RELAY); - await signer.approveAuthRequest(flowC.authorizationUrl); - const sessionC = await flowC.awaitApproval(); - t.ok(sessionC, "Session C created with /pub/legacy/ access"); - - // === THE KEY INSIGHT === - // Server sends BOTH cookie formats for backward compatibility: - // 1. UUID cookie: = (unique name, won't overwrite) - // 2. Legacy cookie: = (same name for all sessions, WILL overwrite) - // - // WITHOUT the UUID fix: - // - All 3 sessions would send cookies named - // - Browser would only keep the last one (Session C) - // - Session A and B would fail because their cookies were overwritten - // - // WITH the UUID fix: - // - Each session also has a UUID cookie with unique name - // - All 3 UUID cookies coexist in browser jar - // - SDK uses UUID cookies internally, so all sessions work - - // === CRITICAL TEST: Verify Session A still works after creating B and C === - // Without UUID fix, Session A's cookie would have been overwritten by B and C - // (all would have same cookie name = pubkey, browser keeps only the last one) - // With UUID fix, all three UUID cookies coexist in the browser jar - try { - await sessionA.storage.putText("/pub/posts/critical-test.txt" as any, "A works!"); - t.pass( - "✓ FIX VERIFIED: Session A STILL works after creating B and C (UUID cookies coexist)", - ); - } catch (error) { - t.fail( - "✗ REGRESSION: Session A failed after creating B and C. UUID cookies may have been removed!", - ); - } - - // Verify Session B also works - try { - await sessionB.storage.putText("/pub/admin/settings" as any, "B works!"); - t.pass("✓ Session B works for /pub/admin/"); - } catch (error) { - t.fail("Session B should work for /pub/admin/"); - } - - // Verify Session C works - try { - await sessionC.storage.putText("/pub/legacy/data.txt" as any, "C works!"); - t.pass("✓ Session C works for /pub/legacy/"); - } catch (error) { - t.fail("Session C should work for /pub/legacy/"); - } - - // Verify capability isolation still works - try { - await sessionA.storage.putText("/pub/admin/test" as any, "should fail"); - t.fail("Session A should NOT have access to /pub/admin/"); - } catch (error) { - assertPubkyError(t, error); - t.pass("✓ Session A correctly denied access to /pub/admin/ (capability isolation works)"); - } - - // Test with credentials:include (browser automatically sends all cookies) - { - const url = `https://_pubky.${userPk}/pub/posts/auto-cookies.txt`; - const response = await sdk.client.fetch(url, { - method: "PUT", - body: "auto cookie selection", - credentials: "include", + // === 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); + }, + ); }); - t.ok( - response.ok, - "✓ Browser sent all cookies with credentials:include, server selected Session A's UUID cookie", + + 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/storage.ts b/pubky-sdk/bindings/js/pkg/tests/storage.ts index 7ff12d885..3b5df22f3 100644 --- a/pubky-sdk/bindings/js/pkg/tests/storage.ts +++ b/pubky-sdk/bindings/js/pkg/tests/storage.ts @@ -469,7 +469,7 @@ test("unauthorized (no cookie) PUT returns 401", async (t) => { credentials: "include", }); - t.equal(resp.status, 401, "PUT without valid session cookie is 401"); + t.equal(resp.status, 403, "PUT without valid session cookie is 403"); 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 af43650a4..cf0865f34 100644 --- a/pubky-sdk/src/actors/session/core.rs +++ b/pubky-sdk/src/actors/session/core.rs @@ -37,7 +37,7 @@ pub struct PubkySession { pub(crate) info: SessionInfo, /// 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. + /// Stored as (`cookie_name`, `cookie_value`) where name is UUID and value is session secret. #[cfg(not(target_arch = "wasm32"))] pub(crate) cookie: (String, String), } From 764e6d28205e9a79cef64eddc5bf2b97fa08f98c Mon Sep 17 00:00:00 2001 From: tomos Date: Fri, 14 Nov 2025 12:29:27 +0000 Subject: [PATCH 6/9] feat: Tidying up --- e2e/src/tests/auth.rs | 4 +-- pubky-homeserver/src/core/layers/authz.rs | 28 ++++++++----------- .../src/core/routes/tenants/session.rs | 27 ++++++++++++------ pubky-sdk/bindings/js/pkg/tests/session.ts | 8 +++--- pubky-sdk/bindings/js/pkg/tests/storage.ts | 2 +- 5 files changed, 37 insertions(+), 32 deletions(-) diff --git a/e2e/src/tests/auth.rs b/e2e/src/tests/auth.rs index 853369fb7..4b7737889 100644 --- a/e2e/src/tests/auth.rs +++ b/e2e/src/tests/auth.rs @@ -251,9 +251,9 @@ async fn multiple_users() { .await .unwrap_err(); - // Should get either Authentication error or 403 Server error (no valid session found) + // 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::FORBIDDEN); + || matches!(bob_restore_err, Error::Request(RequestError::Server { status, .. }) if status == StatusCode::UNAUTHORIZED); assert!( is_expected_error, diff --git a/pubky-homeserver/src/core/layers/authz.rs b/pubky-homeserver/src/core/layers/authz.rs index e5c70e587..c619aac8d 100644 --- a/pubky-homeserver/src/core/layers/authz.rs +++ b/pubky-homeserver/src/core/layers/authz.rs @@ -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, @@ -151,15 +154,20 @@ async fn authorize( /// Get all valid sessions from cookies that belong to the specified user. /// -/// Returns 401 if no session secrets found in cookies. -/// Returns 403 if cookies exist but no valid sessions found for the 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 session_secrets = session_secrets_from_cookies(cookies); + 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: {}", @@ -195,22 +203,10 @@ pub async fn sessions_from_cookies( if user_sessions.is_empty() { tracing::warn!("No valid sessions found for pubky-host: {}", public_key); - return Err(HttpError::forbidden_with_message( + return Err(HttpError::unauthorized_with_message( "No valid session found for user", )); } Ok(user_sessions) } - -/// Get all session secrets from the cookies by iterating and validating. -/// Returns a vector of all valid session secrets found. -pub fn session_secrets_from_cookies(cookies: &Cookies) -> Vec { - let mut secrets = Vec::new(); - for cookie in cookies.list() { - if let Ok(secret) = SessionSecret::from_str(cookie.value()) { - secrets.push(secret); - } - } - secrets -} diff --git a/pubky-homeserver/src/core/routes/tenants/session.rs b/pubky-homeserver/src/core/routes/tenants/session.rs index 5cc46fdca..05913474b 100644 --- a/pubky-homeserver/src/core/routes/tenants/session.rs +++ b/pubky-homeserver/src/core/routes/tenants/session.rs @@ -6,7 +6,10 @@ use axum::{ use tower_cookies::Cookies; use crate::{ - core::{err_if_user_is_invalid::get_user_or_http_error, extractors::PubkyHost, AppState}, + core::{ + err_if_user_is_invalid::get_user_or_http_error, extractors::PubkyHost, + layers::authz::sessions_from_cookies, AppState, + }, persistence::sql::session::SessionRepository, shared::{HttpError, HttpResult}, }; @@ -20,9 +23,7 @@ pub async fn session( ) -> HttpResult { get_user_or_http_error(pubky.public_key(), &mut state.sql_db.pool().into(), false).await?; - let sessions = - crate::core::layers::authz::sessions_from_cookies(&state, &cookies, pubky.public_key()) - .await?; + let sessions = sessions_from_cookies(&state, &cookies, pubky.public_key()).await?; // Return the first session if let Some(session) = sessions.into_iter().next() { @@ -50,13 +51,21 @@ pub async fn signout( ) -> HttpResult { // TODO: Set expired cookie to delete the cookie on client side. - let sessions = - crate::core::layers::authz::sessions_from_cookies(&state, &cookies, pubky.public_key()) - .await - .unwrap_or_default(); + let sessions = sessions_from_cookies(&state, &cookies, pubky.public_key()) + .await + .unwrap_or_default(); for session in sessions { - let _ = SessionRepository::delete(&session.secret, &mut state.sql_db.pool().into()).await; + 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 + ); + } } // Idempotent Success Response (200 OK) diff --git a/pubky-sdk/bindings/js/pkg/tests/session.ts b/pubky-sdk/bindings/js/pkg/tests/session.ts index db4041393..64344bd27 100644 --- a/pubky-sdk/bindings/js/pkg/tests/session.ts +++ b/pubky-sdk/bindings/js/pkg/tests/session.ts @@ -85,7 +85,7 @@ test("Auth: basic", async (t) => { body: "should fail", credentials: "include", }); - t.equal(res401.status, 403, "PUT without session returns 403"); + t.equal(res401.status, 401, "PUT without session returns 401"); // 5) Sign in again (local key proves identity) const session2 = await signer.signin(); @@ -178,7 +178,7 @@ test("Auth: multi-user (cookies)", async (t) => { body: "should-fail", credentials: "include", }); - t.equal(r.status, 403, "bob's root session 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) @@ -187,7 +187,7 @@ test("Auth: multi-user (cookies)", async (t) => { t.fail("bob's second session should be invalidated after signout"); } catch (error: any) { t.ok( - error.message.includes("403") || error.message.includes("Forbidden"), + error.message.includes("401") || error.message.includes("Unauthorized"), "bob's second session invalidated by signout", ); } @@ -351,7 +351,7 @@ test("Auth: signup/signout loops keep cookies and host in sync", async (t) => { body: "should-401", credentials: "include", }); - t.equal(r.status, 403, "signed-out user cannot write"); + t.equal(r.status, 401, "signed-out user cannot write"); } const u2 = await signupAndMark("user#2:hello"); diff --git a/pubky-sdk/bindings/js/pkg/tests/storage.ts b/pubky-sdk/bindings/js/pkg/tests/storage.ts index 3b5df22f3..7ff12d885 100644 --- a/pubky-sdk/bindings/js/pkg/tests/storage.ts +++ b/pubky-sdk/bindings/js/pkg/tests/storage.ts @@ -469,7 +469,7 @@ test("unauthorized (no cookie) PUT returns 401", async (t) => { credentials: "include", }); - t.equal(resp.status, 403, "PUT without valid session cookie is 403"); + t.equal(resp.status, 401, "PUT without valid session cookie is 401"); t.end(); }); From 3bf962b7181b692082fcc1e2f649e72f2889bb0a Mon Sep 17 00:00:00 2001 From: tomos Date: Fri, 14 Nov 2025 21:20:14 +0000 Subject: [PATCH 7/9] feat: Set Cookie.path for client side cookie filtering --- pubky-common/src/capabilities.rs | 88 +++++++++++++ pubky-homeserver/src/core/routes/auth.rs | 41 +++--- pubky-sdk/bindings/js/pkg/tests/session.ts | 142 ++++++++++++++++++++- 3 files changed, 251 insertions(+), 20 deletions(-) diff --git a/pubky-common/src/capabilities.rs b/pubky-common/src/capabilities.rs index 2bd0e1ffd..aefbc9652 100644 --- a/pubky-common/src/capabilities.rs +++ b/pubky-common/src/capabilities.rs @@ -428,6 +428,94 @@ impl Capabilities { pub fn to_vec(&self) -> Vec { self.0.clone() } + + /// Returns the most specific (shortest) common scope path among all capabilities. + /// + /// This is useful for setting HTTP cookie paths to minimize unnecessary cookie transmission. + /// The browser will only send cookies when the request path matches the cookie's path prefix. + /// + /// # Examples + /// + /// ``` + /// use pubky_common::capabilities::Capabilities; + /// + /// // Single scope + /// let caps = Capabilities::builder().read_write("/pub/posts/").finish(); + /// assert_eq!(caps.most_specific_scope(), "/pub/posts/"); + /// + /// // Multiple scopes with common prefix + /// let caps = Capabilities::builder() + /// .read_write("/pub/posts/") + /// .read("/pub/posts/drafts/") + /// .finish(); + /// assert_eq!(caps.most_specific_scope(), "/pub/posts/"); + /// + /// // No common prefix - falls back to root + /// let caps = Capabilities::builder() + /// .read_write("/pub/posts/") + /// .read("/pub/admin/") + /// .finish(); + /// assert_eq!(caps.most_specific_scope(), "/"); + /// + /// // Root capability + /// let caps = Capabilities::builder().read_write("/").finish(); + /// assert_eq!(caps.most_specific_scope(), "/"); + /// ``` + pub fn most_specific_scope(&self) -> &str { + // Empty or root capability - use root path + if self.0.is_empty() || self.0.iter().any(|cap| cap.scope == "/") { + return "/"; + } + + // Single capability - use its scope directly + if self.0.len() == 1 { + return &self.0[0].scope; + } + + // Find longest common prefix among all scopes + let first_scope = &self.0[0].scope; + let mut common_prefix = first_scope.as_str(); + + for cap in &self.0[1..] { + // Find common prefix between current common_prefix and this scope + let scope = &cap.scope; + let min_len = common_prefix.len().min(scope.len()); + + let mut prefix_len = 0; + for i in 0..min_len { + if common_prefix.as_bytes()[i] == scope.as_bytes()[i] { + prefix_len = i + 1; + } else { + break; + } + } + + common_prefix = &common_prefix[..prefix_len]; + + // If no common prefix at all, fall back to root + if common_prefix.is_empty() || common_prefix == "" { + return "/"; + } + } + + // Ensure the common prefix ends at a path boundary (after a '/') + // For example, if scopes are "/pub/posts/" and "/pub/profile/", + // common prefix is "/pub/p" but we want "/pub/" + if let Some(last_slash) = common_prefix.rfind('/') { + let boundary_prefix = &common_prefix[..=last_slash]; + + // If the boundary is not just "/", return it + // Otherwise return root "/" + if boundary_prefix.len() > 1 { + boundary_prefix + } else { + "/" + } + } else { + // No slash found - fall back to root + "/" + } + } } /// Fluent builder for multiple [`Capability`] entries. diff --git a/pubky-homeserver/src/core/routes/auth.rs b/pubky-homeserver/src/core/routes/auth.rs index 42a12d4a6..5fb7759a8 100644 --- a/pubky-homeserver/src/core/routes/auth.rs +++ b/pubky-homeserver/src/core/routes/auth.rs @@ -137,32 +137,37 @@ async fn create_session_and_cookie( // For backward compatibility we need send BOTH cookie formats: // - New format: UUID cookie name // - Legacy format: pubkey cookie name + + // Determine cookie path based on capabilities to reduce unnecessary cookie transmission + let cookie_path = capabilities.most_specific_scope().to_owned(); + let cookie_id = Uuid::new_v4().to_string(); - let mut new_cookie = Cookie::new(cookie_id, session_secret.to_string()); - new_cookie.set_path("/"); + let mut new_cookie = Cookie::build((cookie_id, session_secret.to_string())) + .path(cookie_path.clone()) + .http_only(true) + .max_age(Duration::days(365)) + .expires(OffsetDateTime::now_utc() + Duration::days(365)); + if is_secure(host) { - new_cookie.set_secure(true); - new_cookie.set_same_site(SameSite::None); + new_cookie = new_cookie.secure(true).same_site(SameSite::None); } - new_cookie.set_http_only(true); - let one_year = Duration::days(365); - let expiry = OffsetDateTime::now_utc() + one_year; - new_cookie.set_max_age(one_year); - new_cookie.set_expires(expiry); - cookies.add(new_cookie); + + cookies.add(new_cookie.build()); // 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("/"); + let mut legacy_cookie = + Cookie::build((user.public_key.to_string(), session_secret.to_string())) + .path(cookie_path) + .http_only(true) + .max_age(Duration::days(365)) + .expires(OffsetDateTime::now_utc() + Duration::days(365)); + if is_secure(host) { - legacy_cookie.set_secure(true); - legacy_cookie.set_same_site(SameSite::None); + legacy_cookie = legacy_cookie.secure(true).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); + + cookies.add(legacy_cookie.build()); let session = SessionInfo::new(&user.public_key, capabilities.clone(), None); let mut resp = session.serialize().into_response(); diff --git a/pubky-sdk/bindings/js/pkg/tests/session.ts b/pubky-sdk/bindings/js/pkg/tests/session.ts index 64344bd27..b4a01d0e1 100644 --- a/pubky-sdk/bindings/js/pkg/tests/session.ts +++ b/pubky-sdk/bindings/js/pkg/tests/session.ts @@ -447,7 +447,7 @@ test("Auth: multiple session cookies don't overwrite each other", async (t) => { t.equal( thisUserLegacyCookies.length, 1, - "✓ Exactly 1 legacy cookie for this user (last session overwrites previous)", + "Exactly 1 legacy cookie for this user (last session overwrites previous)", ); } @@ -457,7 +457,145 @@ test("Auth: multiple session cookies don't overwrite each other", async (t) => { "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.pass("Session B works for /pub/admin/"); + + t.end(); +}); + +/** + * Test that cookie paths are set based on session capabilities, + * and browsers only send cookies matching the request path. + * + * With path-based scoping: + * - Session A with /pub/posts/:rw → cookie path=/pub/posts/ + * - Session B with /pub/admin/:rw → cookie path=/pub/admin/ + * - Browser only sends cookie A to /pub/posts/* requests + * - Browser only sends cookie B to /pub/admin/* requests + */ +test("Auth: cookie paths reduce transmission based on capabilities", 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); + + // === Create two sessions with different scoped capabilities === + + // 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 Paths in Browser Jar === + 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); + }, + ); + }); + + // Find UUID cookies (exclude legacy pubkey-named cookie) + const uuidCookies = cookies.filter((c: any) => + c.key !== signer.publicKey.z32() && + c.key.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i) + ); + + t.ok(uuidCookies.length >= 2, "At least 2 UUID cookies exist"); + + // Verify that cookies have appropriate paths + const postsPathCookies = uuidCookies.filter((c: any) => c.path === "/pub/posts/"); + const adminPathCookies = uuidCookies.filter((c: any) => c.path === "/pub/admin/"); + + t.ok( + postsPathCookies.length > 0, + "Cookie with path=/pub/posts/ exists for Session A" + ); + t.ok( + adminPathCookies.length > 0, + "Cookie with path=/pub/admin/ exists for Session B" + ); + + // === Test browser cookie path matching behavior === + // When making a request to /pub/posts/*, browser should only send cookies + // whose path is a prefix of the request path + + // Get cookies that browser would send to /pub/posts/file.txt + const cookiesForPosts: any[] = await new Promise((resolve, reject) => { + jar.getCookies( + "http://localhost:15411/pub/posts/file.txt", + (err: any, cookies: any[]) => { + if (err) reject(err); + else resolve(cookies); + }, + ); + }); + + const postsUuidCookies = cookiesForPosts.filter((c: any) => + c.key.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i) + ); + + // Get cookies that browser would send to /pub/admin/settings + const cookiesForAdmin: any[] = await new Promise((resolve, reject) => { + jar.getCookies( + "http://localhost:15411/pub/admin/settings", + (err: any, cookies: any[]) => { + if (err) reject(err); + else resolve(cookies); + }, + ); + }); + + const adminUuidCookies = cookiesForAdmin.filter((c: any) => + c.key.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i) + ); + + t.ok( + postsUuidCookies.length >= 1 && postsUuidCookies.some((c: any) => c.path === "/pub/posts/"), + "Browser sends /pub/posts/ cookie to /pub/posts/* requests" + ); + + t.ok( + adminUuidCookies.length >= 1 && adminUuidCookies.some((c: any) => c.path === "/pub/admin/"), + "Browser sends /pub/admin/ cookie to /pub/admin/* requests" + ); + + // Verify path isolation: /pub/admin/ cookie should NOT be sent to /pub/posts/ requests + const adminCookieSentToPosts = postsUuidCookies.some((c: any) => c.path === "/pub/admin/"); + t.notOk( + adminCookieSentToPosts, + "Browser does NOT send /pub/admin/ cookie to /pub/posts/* (path isolation)" + ); + + // Verify path isolation: /pub/posts/ cookie should NOT be sent to /pub/admin/ requests + const postsCookieSentToAdmin = adminUuidCookies.some((c: any) => c.path === "/pub/posts/"); + t.notOk( + postsCookieSentToAdmin, + "Browser does NOT send /pub/posts/ cookie to /pub/admin/* (path isolation)" + ); + } else { + t.skip("Cookie jar not available in this environment"); + } + + // Verify sessions still work functionally + await sessionA.storage.putText("/pub/posts/test.txt" as any, "A works!"); + t.pass("Session A works for /pub/posts/"); + + await sessionB.storage.putText("/pub/admin/settings" as any, "B works!"); + t.pass("Session B works for /pub/admin/"); t.end(); }); From b23d8b0ccca8a822389dffcfb88efa1e199879d3 Mon Sep 17 00:00:00 2001 From: tomos Date: Mon, 17 Nov 2025 12:09:16 +0000 Subject: [PATCH 8/9] Revert "feat: Set Cookie.path for client side cookie filtering" This reverts commit 3bf962b7181b692082fcc1e2f649e72f2889bb0a. --- pubky-common/src/capabilities.rs | 88 ------------- pubky-homeserver/src/core/routes/auth.rs | 41 +++--- pubky-sdk/bindings/js/pkg/tests/session.ts | 142 +-------------------- 3 files changed, 20 insertions(+), 251 deletions(-) diff --git a/pubky-common/src/capabilities.rs b/pubky-common/src/capabilities.rs index aefbc9652..2bd0e1ffd 100644 --- a/pubky-common/src/capabilities.rs +++ b/pubky-common/src/capabilities.rs @@ -428,94 +428,6 @@ impl Capabilities { pub fn to_vec(&self) -> Vec { self.0.clone() } - - /// Returns the most specific (shortest) common scope path among all capabilities. - /// - /// This is useful for setting HTTP cookie paths to minimize unnecessary cookie transmission. - /// The browser will only send cookies when the request path matches the cookie's path prefix. - /// - /// # Examples - /// - /// ``` - /// use pubky_common::capabilities::Capabilities; - /// - /// // Single scope - /// let caps = Capabilities::builder().read_write("/pub/posts/").finish(); - /// assert_eq!(caps.most_specific_scope(), "/pub/posts/"); - /// - /// // Multiple scopes with common prefix - /// let caps = Capabilities::builder() - /// .read_write("/pub/posts/") - /// .read("/pub/posts/drafts/") - /// .finish(); - /// assert_eq!(caps.most_specific_scope(), "/pub/posts/"); - /// - /// // No common prefix - falls back to root - /// let caps = Capabilities::builder() - /// .read_write("/pub/posts/") - /// .read("/pub/admin/") - /// .finish(); - /// assert_eq!(caps.most_specific_scope(), "/"); - /// - /// // Root capability - /// let caps = Capabilities::builder().read_write("/").finish(); - /// assert_eq!(caps.most_specific_scope(), "/"); - /// ``` - pub fn most_specific_scope(&self) -> &str { - // Empty or root capability - use root path - if self.0.is_empty() || self.0.iter().any(|cap| cap.scope == "/") { - return "/"; - } - - // Single capability - use its scope directly - if self.0.len() == 1 { - return &self.0[0].scope; - } - - // Find longest common prefix among all scopes - let first_scope = &self.0[0].scope; - let mut common_prefix = first_scope.as_str(); - - for cap in &self.0[1..] { - // Find common prefix between current common_prefix and this scope - let scope = &cap.scope; - let min_len = common_prefix.len().min(scope.len()); - - let mut prefix_len = 0; - for i in 0..min_len { - if common_prefix.as_bytes()[i] == scope.as_bytes()[i] { - prefix_len = i + 1; - } else { - break; - } - } - - common_prefix = &common_prefix[..prefix_len]; - - // If no common prefix at all, fall back to root - if common_prefix.is_empty() || common_prefix == "" { - return "/"; - } - } - - // Ensure the common prefix ends at a path boundary (after a '/') - // For example, if scopes are "/pub/posts/" and "/pub/profile/", - // common prefix is "/pub/p" but we want "/pub/" - if let Some(last_slash) = common_prefix.rfind('/') { - let boundary_prefix = &common_prefix[..=last_slash]; - - // If the boundary is not just "/", return it - // Otherwise return root "/" - if boundary_prefix.len() > 1 { - boundary_prefix - } else { - "/" - } - } else { - // No slash found - fall back to root - "/" - } - } } /// Fluent builder for multiple [`Capability`] entries. diff --git a/pubky-homeserver/src/core/routes/auth.rs b/pubky-homeserver/src/core/routes/auth.rs index 5fb7759a8..42a12d4a6 100644 --- a/pubky-homeserver/src/core/routes/auth.rs +++ b/pubky-homeserver/src/core/routes/auth.rs @@ -137,37 +137,32 @@ async fn create_session_and_cookie( // For backward compatibility we need send BOTH cookie formats: // - New format: UUID cookie name // - Legacy format: pubkey cookie name - - // Determine cookie path based on capabilities to reduce unnecessary cookie transmission - let cookie_path = capabilities.most_specific_scope().to_owned(); - let cookie_id = Uuid::new_v4().to_string(); - let mut new_cookie = Cookie::build((cookie_id, session_secret.to_string())) - .path(cookie_path.clone()) - .http_only(true) - .max_age(Duration::days(365)) - .expires(OffsetDateTime::now_utc() + Duration::days(365)); - + let mut new_cookie = Cookie::new(cookie_id, session_secret.to_string()); + new_cookie.set_path("/"); if is_secure(host) { - new_cookie = new_cookie.secure(true).same_site(SameSite::None); + new_cookie.set_secure(true); + new_cookie.set_same_site(SameSite::None); } - - cookies.add(new_cookie.build()); + new_cookie.set_http_only(true); + let one_year = Duration::days(365); + let expiry = OffsetDateTime::now_utc() + one_year; + 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::build((user.public_key.to_string(), session_secret.to_string())) - .path(cookie_path) - .http_only(true) - .max_age(Duration::days(365)) - .expires(OffsetDateTime::now_utc() + Duration::days(365)); - + 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 = legacy_cookie.secure(true).same_site(SameSite::None); + legacy_cookie.set_secure(true); + legacy_cookie.set_same_site(SameSite::None); } - - cookies.add(legacy_cookie.build()); + 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-sdk/bindings/js/pkg/tests/session.ts b/pubky-sdk/bindings/js/pkg/tests/session.ts index b4a01d0e1..64344bd27 100644 --- a/pubky-sdk/bindings/js/pkg/tests/session.ts +++ b/pubky-sdk/bindings/js/pkg/tests/session.ts @@ -447,7 +447,7 @@ test("Auth: multiple session cookies don't overwrite each other", async (t) => { t.equal( thisUserLegacyCookies.length, 1, - "Exactly 1 legacy cookie for this user (last session overwrites previous)", + "✓ Exactly 1 legacy cookie for this user (last session overwrites previous)", ); } @@ -457,145 +457,7 @@ test("Auth: multiple session cookies don't overwrite each other", async (t) => { "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(); -}); - -/** - * Test that cookie paths are set based on session capabilities, - * and browsers only send cookies matching the request path. - * - * With path-based scoping: - * - Session A with /pub/posts/:rw → cookie path=/pub/posts/ - * - Session B with /pub/admin/:rw → cookie path=/pub/admin/ - * - Browser only sends cookie A to /pub/posts/* requests - * - Browser only sends cookie B to /pub/admin/* requests - */ -test("Auth: cookie paths reduce transmission based on capabilities", 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); - - // === Create two sessions with different scoped capabilities === - - // 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 Paths in Browser Jar === - 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); - }, - ); - }); - - // Find UUID cookies (exclude legacy pubkey-named cookie) - const uuidCookies = cookies.filter((c: any) => - c.key !== signer.publicKey.z32() && - c.key.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i) - ); - - t.ok(uuidCookies.length >= 2, "At least 2 UUID cookies exist"); - - // Verify that cookies have appropriate paths - const postsPathCookies = uuidCookies.filter((c: any) => c.path === "/pub/posts/"); - const adminPathCookies = uuidCookies.filter((c: any) => c.path === "/pub/admin/"); - - t.ok( - postsPathCookies.length > 0, - "Cookie with path=/pub/posts/ exists for Session A" - ); - t.ok( - adminPathCookies.length > 0, - "Cookie with path=/pub/admin/ exists for Session B" - ); - - // === Test browser cookie path matching behavior === - // When making a request to /pub/posts/*, browser should only send cookies - // whose path is a prefix of the request path - - // Get cookies that browser would send to /pub/posts/file.txt - const cookiesForPosts: any[] = await new Promise((resolve, reject) => { - jar.getCookies( - "http://localhost:15411/pub/posts/file.txt", - (err: any, cookies: any[]) => { - if (err) reject(err); - else resolve(cookies); - }, - ); - }); - - const postsUuidCookies = cookiesForPosts.filter((c: any) => - c.key.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i) - ); - - // Get cookies that browser would send to /pub/admin/settings - const cookiesForAdmin: any[] = await new Promise((resolve, reject) => { - jar.getCookies( - "http://localhost:15411/pub/admin/settings", - (err: any, cookies: any[]) => { - if (err) reject(err); - else resolve(cookies); - }, - ); - }); - - const adminUuidCookies = cookiesForAdmin.filter((c: any) => - c.key.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i) - ); - - t.ok( - postsUuidCookies.length >= 1 && postsUuidCookies.some((c: any) => c.path === "/pub/posts/"), - "Browser sends /pub/posts/ cookie to /pub/posts/* requests" - ); - - t.ok( - adminUuidCookies.length >= 1 && adminUuidCookies.some((c: any) => c.path === "/pub/admin/"), - "Browser sends /pub/admin/ cookie to /pub/admin/* requests" - ); - - // Verify path isolation: /pub/admin/ cookie should NOT be sent to /pub/posts/ requests - const adminCookieSentToPosts = postsUuidCookies.some((c: any) => c.path === "/pub/admin/"); - t.notOk( - adminCookieSentToPosts, - "Browser does NOT send /pub/admin/ cookie to /pub/posts/* (path isolation)" - ); - - // Verify path isolation: /pub/posts/ cookie should NOT be sent to /pub/admin/ requests - const postsCookieSentToAdmin = adminUuidCookies.some((c: any) => c.path === "/pub/posts/"); - t.notOk( - postsCookieSentToAdmin, - "Browser does NOT send /pub/posts/ cookie to /pub/admin/* (path isolation)" - ); - } else { - t.skip("Cookie jar not available in this environment"); - } - - // Verify sessions still work functionally - await sessionA.storage.putText("/pub/posts/test.txt" as any, "A works!"); - t.pass("Session A works for /pub/posts/"); - - await sessionB.storage.putText("/pub/admin/settings" as any, "B works!"); - t.pass("Session B works for /pub/admin/"); + t.pass("✓ Session B works for /pub/admin/"); t.end(); }); From fcbda288810d0bf6146a5d440eb92f6ffe8d8c2a Mon Sep 17 00:00:00 2001 From: tomos Date: Mon, 17 Nov 2025 12:23:11 +0000 Subject: [PATCH 9/9] fix: remove regression from Signout upon deletion error --- pubky-homeserver/src/core/routes/tenants/session.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pubky-homeserver/src/core/routes/tenants/session.rs b/pubky-homeserver/src/core/routes/tenants/session.rs index 05913474b..e81167aa7 100644 --- a/pubky-homeserver/src/core/routes/tenants/session.rs +++ b/pubky-homeserver/src/core/routes/tenants/session.rs @@ -55,6 +55,7 @@ pub async fn signout( .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 @@ -65,9 +66,15 @@ pub async fn signout( 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) Ok(()) }