From 60a6680eaf4cf9baa4cac0ea1a2f148467a936b930db713f5d09ddac5a94a2d6 Mon Sep 17 00:00:00 2001 From: Evan Carroll Date: Fri, 23 Jan 2026 08:18:09 -0600 Subject: [PATCH] fix: guests * make guest status a flag on users * add logout handlers * add logout notification for other users --- crates/chattyness-db/src/models.rs | 10 +- crates/chattyness-db/src/pool.rs | 20 - crates/chattyness-db/src/queries.rs | 1 - .../src/queries/channel_members.rs | 6 +- crates/chattyness-db/src/queries/guests.rs | 98 ---- crates/chattyness-db/src/queries/users.rs | 8 + crates/chattyness-db/src/ws_messages.rs | 38 +- crates/chattyness-user-ui/src/api/auth.rs | 4 +- .../chattyness-user-ui/src/api/websocket.rs | 57 +- crates/chattyness-user-ui/src/auth/rls.rs | 40 +- crates/chattyness-user-ui/src/auth/session.rs | 3 - .../src/components/avatar_canvas.rs | 11 +- .../src/components/chat_types.rs | 16 +- .../src/components/scene_viewer.rs | 19 +- .../src/components/ws_client.rs | 536 +++++++++++------- crates/chattyness-user-ui/src/pages/realm.rs | 114 +++- db/schema/functions/001_helpers.sql | 48 +- db/schema/policies/001_rls.sql | 26 +- db/schema/tables/030_realm.sql | 29 - db/schema/tables/045_scene.sql | 27 +- db/schema/tables/050_chat.sql | 13 +- 21 files changed, 523 insertions(+), 601 deletions(-) delete mode 100644 crates/chattyness-db/src/queries/guests.rs diff --git a/crates/chattyness-db/src/models.rs b/crates/chattyness-db/src/models.rs index b210956..a1d11c5 100644 --- a/crates/chattyness-db/src/models.rs +++ b/crates/chattyness-db/src/models.rs @@ -3041,13 +3041,13 @@ pub struct SpotListResponse { // ============================================================================= /// A user's presence in a channel. +/// Note: Guests are regular users with the 'guest' tag, so all members have a user_id. #[derive(Debug, Clone, Serialize, Deserialize)] #[cfg_attr(feature = "ssr", derive(sqlx::FromRow))] pub struct ChannelMember { pub id: Uuid, pub channel_id: Uuid, - pub user_id: Option, - pub guest_session_id: Option, + pub user_id: Uuid, /// X coordinate in scene space pub position_x: f64, /// Y coordinate in scene space @@ -3061,14 +3061,14 @@ pub struct ChannelMember { } /// Channel member with user info for display. +/// Note: Guests are regular users with the 'guest' tag, identified via is_guest field. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[cfg_attr(feature = "ssr", derive(sqlx::FromRow))] pub struct ChannelMemberInfo { pub id: Uuid, pub channel_id: Uuid, - pub user_id: Option, - pub guest_session_id: Option, - /// Display name (user's display_name or guest's guest_name) + pub user_id: Uuid, + /// Display name (user's display_name) pub display_name: String, /// X coordinate in scene space pub position_x: f64, diff --git a/crates/chattyness-db/src/pool.rs b/crates/chattyness-db/src/pool.rs index 09d3575..35e9e27 100644 --- a/crates/chattyness-db/src/pool.rs +++ b/crates/chattyness-db/src/pool.rs @@ -62,23 +62,3 @@ pub async fn clear_user_context(pool: &PgPool) -> Result<(), AppError> { .await?; Ok(()) } - -/// Set the current guest session context for Row-Level Security. -/// -/// This should be called for guest users to enable RLS policies -/// that depend on the current guest session ID. -pub async fn set_guest_context(pool: &PgPool, guest_session_id: Uuid) -> Result<(), AppError> { - sqlx::query("SELECT public.set_current_guest_session_id($1)") - .bind(guest_session_id) - .execute(pool) - .await?; - Ok(()) -} - -/// Clear the current guest session context. -pub async fn clear_guest_context(pool: &PgPool) -> Result<(), AppError> { - sqlx::query("SELECT public.set_current_guest_session_id(NULL)") - .execute(pool) - .await?; - Ok(()) -} diff --git a/crates/chattyness-db/src/queries.rs b/crates/chattyness-db/src/queries.rs index 540e5fe..1f7eeb6 100644 --- a/crates/chattyness-db/src/queries.rs +++ b/crates/chattyness-db/src/queries.rs @@ -3,7 +3,6 @@ pub mod avatars; pub mod channel_members; pub mod channels; -pub mod guests; pub mod inventory; pub mod loose_props; pub mod memberships; diff --git a/crates/chattyness-db/src/queries/channel_members.rs b/crates/chattyness-db/src/queries/channel_members.rs index bd02239..877604f 100644 --- a/crates/chattyness-db/src/queries/channel_members.rs +++ b/crates/chattyness-db/src/queries/channel_members.rs @@ -39,7 +39,6 @@ pub async fn join_channel<'e>( id, instance_id as channel_id, user_id, - guest_session_id, ST_X(position) as position_x, ST_Y(position) as position_y, facing_direction, @@ -169,8 +168,7 @@ pub async fn get_channel_members<'e>( cm.id, cm.instance_id as channel_id, cm.user_id, - cm.guest_session_id, - COALESCE(u.display_name, gs.guest_name, 'Anonymous') as display_name, + COALESCE(u.display_name, 'Anonymous') as display_name, ST_X(cm.position) as position_x, ST_Y(cm.position) as position_y, cm.facing_direction, @@ -181,7 +179,6 @@ pub async fn get_channel_members<'e>( COALESCE('guest' = ANY(u.tags), false) as is_guest FROM scene.instance_members cm LEFT JOIN auth.users u ON cm.user_id = u.id - LEFT JOIN auth.guest_sessions gs ON cm.guest_session_id = gs.id LEFT JOIN auth.active_avatars aa ON cm.user_id = aa.user_id AND aa.realm_id = $2 WHERE cm.instance_id = $1 ORDER BY cm.joined_at ASC @@ -208,7 +205,6 @@ pub async fn get_channel_member<'e>( cm.id, cm.instance_id as channel_id, cm.user_id, - cm.guest_session_id, COALESCE(u.display_name, 'Anonymous') as display_name, ST_X(cm.position) as position_x, ST_Y(cm.position) as position_y, diff --git a/crates/chattyness-db/src/queries/guests.rs b/crates/chattyness-db/src/queries/guests.rs deleted file mode 100644 index fcf4274..0000000 --- a/crates/chattyness-db/src/queries/guests.rs +++ /dev/null @@ -1,98 +0,0 @@ -//! Guest session database queries. - -use chrono::{DateTime, TimeDelta, Utc}; -use sqlx::PgPool; -use uuid::Uuid; - -use chattyness_error::AppError; - -/// Guest session record. -#[derive(Debug, Clone)] -#[cfg_attr(feature = "ssr", derive(sqlx::FromRow))] -pub struct GuestSession { - pub id: Uuid, - pub guest_name: String, - pub current_realm_id: Option, - pub expires_at: DateTime, - pub created_at: DateTime, -} - -/// Create a new guest session. -/// -/// Returns the guest session ID. -pub async fn create_guest_session( - pool: &PgPool, - guest_name: &str, - realm_id: Uuid, - token_hash: &str, - user_agent: Option<&str>, - ip_address: Option<&str>, - expires_at: DateTime, -) -> Result { - let (session_id,): (Uuid,) = sqlx::query_as( - r#" - INSERT INTO auth.guest_sessions (guest_name, token_hash, user_agent, ip_address, current_realm_id, expires_at) - VALUES ($1, $2, $3, $4::inet, $5, $6) - RETURNING id - "#, - ) - .bind(guest_name) - .bind(token_hash) - .bind(user_agent) - .bind(ip_address) - .bind(realm_id) - .bind(expires_at) - .fetch_one(pool) - .await?; - - Ok(session_id) -} - -/// Get a guest session by ID. -pub async fn get_guest_session( - pool: &PgPool, - session_id: Uuid, -) -> Result, AppError> { - let session = sqlx::query_as::<_, GuestSession>( - r#" - SELECT id, guest_name, current_realm_id, expires_at, created_at - FROM auth.guest_sessions - WHERE id = $1 AND expires_at > now() - "#, - ) - .bind(session_id) - .fetch_optional(pool) - .await?; - - // Update last activity if session exists - if session.is_some() { - sqlx::query("UPDATE auth.guest_sessions SET last_activity_at = now() WHERE id = $1") - .bind(session_id) - .execute(pool) - .await?; - } - - Ok(session) -} - -/// Delete a guest session. -pub async fn delete_guest_session(pool: &PgPool, session_id: Uuid) -> Result<(), AppError> { - sqlx::query("DELETE FROM auth.guest_sessions WHERE id = $1") - .bind(session_id) - .execute(pool) - .await?; - Ok(()) -} - -/// Generate a random guest name like "Guest_12345". -pub fn generate_guest_name() -> String { - use rand::Rng; - let mut rng = rand::thread_rng(); - let number: u32 = rng.gen_range(10000..100000); - format!("Guest_{}", number) -} - -/// Calculate guest session expiry (24 hours from now). -pub fn guest_session_expiry() -> DateTime { - Utc::now() + TimeDelta::hours(24) -} diff --git a/crates/chattyness-db/src/queries/users.rs b/crates/chattyness-db/src/queries/users.rs index bf8555c..877f96a 100644 --- a/crates/chattyness-db/src/queries/users.rs +++ b/crates/chattyness-db/src/queries/users.rs @@ -716,3 +716,11 @@ pub async fn update_user_preferences_conn( Ok(()) } + +/// Generate a random guest name like "Guest_12345". +pub fn generate_guest_name() -> String { + use rand::Rng; + let mut rng = rand::thread_rng(); + let number: u32 = rng.gen_range(10000..100000); + format!("Guest_{}", number) +} diff --git a/crates/chattyness-db/src/ws_messages.rs b/crates/chattyness-db/src/ws_messages.rs index 35c39c1..2951fba 100644 --- a/crates/chattyness-db/src/ws_messages.rs +++ b/crates/chattyness-db/src/ws_messages.rs @@ -26,6 +26,8 @@ pub mod close_codes { pub const SCENE_CHANGE: u16 = 4000; /// Server timeout (no message received within timeout period). pub const SERVER_TIMEOUT: u16 = 4001; + /// User explicitly logged out. + pub const LOGOUT: u16 = 4003; } /// Reason for member disconnect. @@ -126,20 +128,16 @@ pub enum ServerMessage { /// A member left the channel. MemberLeft { - /// User ID (if authenticated user). - user_id: Option, - /// Guest session ID (if guest). - guest_session_id: Option, + /// User ID of the member who left. + user_id: Uuid, /// Reason for disconnect. reason: DisconnectReason, }, /// A member updated their position. PositionUpdated { - /// User ID (if authenticated user). - user_id: Option, - /// Guest session ID (if guest). - guest_session_id: Option, + /// User ID of the member. + user_id: Uuid, /// New X coordinate. x: f64, /// New Y coordinate. @@ -148,10 +146,8 @@ pub enum ServerMessage { /// A member changed their emotion. EmotionUpdated { - /// User ID (if authenticated user). - user_id: Option, - /// Guest session ID (if guest). - guest_session_id: Option, + /// User ID of the member. + user_id: Uuid, /// Emotion name (e.g., "happy", "sad", "neutral"). emotion: String, /// Asset paths for all 9 positions of the new emotion layer. @@ -173,10 +169,8 @@ pub enum ServerMessage { ChatMessageReceived { /// Unique message ID. message_id: Uuid, - /// User ID of sender (if authenticated user). - user_id: Option, - /// Guest session ID (if guest). - guest_session_id: Option, + /// User ID of sender. + user_id: Uuid, /// Display name of sender. display_name: String, /// Message content. @@ -217,10 +211,8 @@ pub enum ServerMessage { PropPickedUp { /// ID of the prop that was picked up. prop_id: Uuid, - /// User ID who picked it up (if authenticated). - picked_up_by_user_id: Option, - /// Guest session ID who picked it up (if guest). - picked_up_by_guest_id: Option, + /// User ID who picked it up. + picked_up_by_user_id: Uuid, }, /// A prop expired and was removed. @@ -231,10 +223,8 @@ pub enum ServerMessage { /// A member updated their avatar appearance. AvatarUpdated { - /// User ID (if authenticated user). - user_id: Option, - /// Guest session ID (if guest). - guest_session_id: Option, + /// User ID of the member. + user_id: Uuid, /// Updated avatar render data. avatar: AvatarRenderData, }, diff --git a/crates/chattyness-user-ui/src/api/auth.rs b/crates/chattyness-user-ui/src/api/auth.rs index ed6d175..beb3757 100644 --- a/crates/chattyness-user-ui/src/api/auth.rs +++ b/crates/chattyness-user-ui/src/api/auth.rs @@ -12,7 +12,7 @@ use chattyness_db::{ RealmSummary, RegisterGuestRequest, RegisterGuestResponse, SignupRequest, SignupResponse, UserSummary, }, - queries::{guests, memberships, realms, users}, + queries::{memberships, realms, users}, }; use chattyness_error::AppError; use chattyness_shared::{AgeConfig, GenderConfig, SignupConfig}; @@ -388,7 +388,7 @@ pub async fn guest_login( } // Generate guest name - let guest_name = guests::generate_guest_name(); + let guest_name = users::generate_guest_name(); // Create guest user (no password) - trigger creates avatar automatically let user_id = users::create_guest_user(&pool, &guest_name).await?; diff --git a/crates/chattyness-user-ui/src/api/websocket.rs b/crates/chattyness-user-ui/src/api/websocket.rs index 0d65357..2bb109c 100644 --- a/crates/chattyness-user-ui/src/api/websocket.rs +++ b/crates/chattyness-user-ui/src/api/websocket.rs @@ -472,8 +472,7 @@ async fn handle_socket( continue; } let _ = tx.send(ServerMessage::PositionUpdated { - user_id: Some(user_id), - guest_session_id: None, + user_id, x, y, }); @@ -508,8 +507,7 @@ async fn handle_socket( } }; let _ = tx.send(ServerMessage::EmotionUpdated { - user_id: Some(user_id), - guest_session_id: None, + user_id, emotion, emotion_layer, }); @@ -573,8 +571,7 @@ async fn handle_socket( let msg = ServerMessage::ChatMessageReceived { message_id: Uuid::new_v4(), - user_id: Some(user_id), - guest_session_id: None, + user_id, display_name: member.display_name.clone(), content: content.clone(), emotion: emotion_name.clone(), @@ -595,8 +592,7 @@ async fn handle_socket( let sender_msg = ServerMessage::ChatMessageReceived { message_id: Uuid::new_v4(), - user_id: Some(user_id), - guest_session_id: None, + user_id, display_name: member.display_name.clone(), content, emotion: emotion_name, @@ -627,8 +623,7 @@ async fn handle_socket( // Broadcast: send to all users in the channel let msg = ServerMessage::ChatMessageReceived { message_id: Uuid::new_v4(), - user_id: Some(user_id), - guest_session_id: None, + user_id, display_name: member.display_name.clone(), content, emotion: emotion_name, @@ -733,8 +728,7 @@ async fn handle_socket( ); let _ = tx.send(ServerMessage::PropPickedUp { prop_id: loose_prop_id, - picked_up_by_user_id: Some(user_id), - picked_up_by_guest_id: None, + picked_up_by_user_id: user_id, }); } Err(e) => { @@ -763,8 +757,7 @@ async fn handle_socket( user_id ); let _ = tx.send(ServerMessage::AvatarUpdated { - user_id: Some(user_id), - guest_session_id: None, + user_id, avatar: render_data, }); } @@ -1268,8 +1261,7 @@ async fn handle_socket( // Broadcast avatar update to channel let _ = tx.send(ServerMessage::AvatarUpdated { - user_id: Some(target_user_id), - guest_session_id: None, + user_id: target_user_id, avatar: avatar_render_data, }); @@ -1360,8 +1352,7 @@ async fn handle_socket( // Broadcast avatar update to channel let _ = tx.send(ServerMessage::AvatarUpdated { - user_id: Some(target_user_id), - guest_session_id: None, + user_id: target_user_id, avatar: original_avatar, }); @@ -1416,10 +1407,15 @@ async fn handle_socket( } } Message::Close(close_frame) => { - // Check close code for scene change + // Check close code for scene change or logout if let Some(CloseFrame { code, .. }) = close_frame { if code == close_codes::SCENE_CHANGE { disconnect_reason = DisconnectReason::SceneChange; + } else if code == close_codes::LOGOUT { + // Explicit logout - treat as graceful disconnect + #[cfg(debug_assertions)] + tracing::debug!("[WS] User {} logged out", user_id); + disconnect_reason = DisconnectReason::Graceful; } else { disconnect_reason = DisconnectReason::Graceful; } @@ -1534,8 +1530,7 @@ async fn handle_socket( // Broadcast departure with reason let _ = channel_state.tx.send(ServerMessage::MemberLeft { - user_id: Some(user_id), - guest_session_id: None, + user_id, reason: disconnect_reason, }); } @@ -1554,18 +1549,14 @@ async fn get_members_with_avatars( // This handles the priority chain: forced > custom > selected realm > selected server > realm default > server default let mut result = Vec::with_capacity(members.len()); for member in members { - let avatar = if let Some(user_id) = member.user_id { - // Use the new effective avatar resolution which handles all priority levels - avatars::get_effective_avatar_render_data(pool, user_id, realm_id) - .await - .ok() - .flatten() - .map(|(render_data, _source)| render_data) - .unwrap_or_default() - } else { - // Guest users don't have avatars - AvatarRenderData::default() - }; + // All members now have a user_id (guests are regular users with the 'guest' tag) + // Use the effective avatar resolution which handles all priority levels + let avatar = avatars::get_effective_avatar_render_data(pool, member.user_id, realm_id) + .await + .ok() + .flatten() + .map(|(render_data, _source)| render_data) + .unwrap_or_default(); result.push(ChannelMemberWithAvatar { member, avatar }); } diff --git a/crates/chattyness-user-ui/src/auth/rls.rs b/crates/chattyness-user-ui/src/auth/rls.rs index 5c307f8..52f4e5c 100644 --- a/crates/chattyness-user-ui/src/auth/rls.rs +++ b/crates/chattyness-user-ui/src/auth/rls.rs @@ -19,7 +19,7 @@ use tower::{Layer, Service}; use tower_sessions::Session; use uuid::Uuid; -use super::session::{SESSION_GUEST_ID_KEY, SESSION_USER_ID_KEY}; +use super::session::SESSION_USER_ID_KEY; use chattyness_error::ErrorResponse; // ============================================================================= @@ -39,9 +39,6 @@ impl Drop for RlsConnectionInner { let _ = sqlx::query("SELECT public.set_current_user_id(NULL)") .execute(&mut *conn) .await; - let _ = sqlx::query("SELECT public.set_current_guest_session_id(NULL)") - .execute(&mut *conn) - .await; drop(conn); drop(pool); }); @@ -218,9 +215,9 @@ where let session = request.extensions().get::().cloned(); Box::pin(async move { - let (user_id, guest_session_id) = get_session_ids(session).await; + let user_id = get_session_user_id(session).await; - match acquire_rls_connection(&pool, user_id, guest_session_id).await { + match acquire_rls_connection(&pool, user_id).await { Ok(rls_conn) => { request.extensions_mut().insert(rls_conn); inner.call(request).await @@ -234,43 +231,30 @@ where } } -async fn get_session_ids(session: Option) -> (Option, Option) { +/// Get the user ID from the session if present. +async fn get_session_user_id(session: Option) -> Option { let Some(session) = session else { - return (None, None); + return None; }; - let user_id = session + session .get::(SESSION_USER_ID_KEY) .await .ok() - .flatten(); - let guest_session_id = session - .get::(SESSION_GUEST_ID_KEY) - .await - .ok() - .flatten(); - - (user_id, guest_session_id) + .flatten() } +/// Acquire an RLS connection with the user context set. +/// Guests are now regular users with the 'guest' tag, so they use the same user_id path. async fn acquire_rls_connection( pool: &PgPool, user_id: Option, - guest_session_id: Option, ) -> Result { let mut conn = pool.acquire().await?; - if user_id.is_some() { + if let Some(id) = user_id { sqlx::query("SELECT public.set_current_user_id($1)") - .bind(user_id) - .execute(&mut *conn) - .await?; - } else if guest_session_id.is_some() { - sqlx::query("SELECT public.set_current_user_id(NULL)") - .execute(&mut *conn) - .await?; - sqlx::query("SELECT public.set_current_guest_session_id($1)") - .bind(guest_session_id) + .bind(id) .execute(&mut *conn) .await?; } else { diff --git a/crates/chattyness-user-ui/src/auth/session.rs b/crates/chattyness-user-ui/src/auth/session.rs index 3b6bfac..cb8e882 100644 --- a/crates/chattyness-user-ui/src/auth/session.rs +++ b/crates/chattyness-user-ui/src/auth/session.rs @@ -19,9 +19,6 @@ pub const SESSION_CURRENT_REALM_KEY: &str = "current_realm_id"; /// Session original destination key (for password reset redirect). pub const SESSION_ORIGINAL_DEST_KEY: &str = "original_destination"; -/// Session guest ID key (for guest sessions). -pub const SESSION_GUEST_ID_KEY: &str = "guest_id"; - /// Create the session management layer. pub async fn create_session_layer( pool: PgPool, diff --git a/crates/chattyness-user-ui/src/components/avatar_canvas.rs b/crates/chattyness-user-ui/src/components/avatar_canvas.rs index 65cc3c5..1ff8b87 100644 --- a/crates/chattyness-user-ui/src/components/avatar_canvas.rs +++ b/crates/chattyness-user-ui/src/components/avatar_canvas.rs @@ -456,8 +456,9 @@ impl CanvasLayout { } /// Get a unique key for a member (for Leptos For keying). -pub fn member_key(m: &ChannelMemberWithAvatar) -> (Option, Option) { - (m.member.user_id, m.member.guest_session_id) +/// Note: Guests are now regular users with the 'guest' tag, so user_id is always present. +pub fn member_key(m: &ChannelMemberWithAvatar) -> Uuid { + m.member.user_id } /// Individual avatar canvas component. @@ -789,11 +790,7 @@ pub fn AvatarCanvas( // Compute data-member-id reactively let data_member_id = move || { let m = member.get(); - m.member - .user_id - .map(|u| u.to_string()) - .or_else(|| m.member.guest_session_id.map(|g| g.to_string())) - .unwrap_or_default() + m.member.user_id.to_string() }; view! { diff --git a/crates/chattyness-user-ui/src/components/chat_types.rs b/crates/chattyness-user-ui/src/components/chat_types.rs index d5bfbb1..8ed38cd 100644 --- a/crates/chattyness-user-ui/src/components/chat_types.rs +++ b/crates/chattyness-user-ui/src/components/chat_types.rs @@ -11,11 +11,12 @@ pub const MAX_MESSAGE_LOG_SIZE: usize = 2000; pub const DEFAULT_BUBBLE_TIMEOUT_MS: i64 = 60_000; /// A chat message for display and logging. +/// Note: Guests are now regular users with the 'guest' tag, so all messages have a user_id. +/// System messages use Uuid::nil() as the user_id. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ChatMessage { pub message_id: Uuid, - pub user_id: Option, - pub guest_session_id: Option, + pub user_id: Uuid, pub display_name: String, pub content: String, /// Emotion name (e.g., "happy", "sad", "neutral"). @@ -70,15 +71,8 @@ impl MessageLog { } /// Get the latest message from a specific user. - pub fn latest_from_user( - &self, - user_id: Option, - guest_id: Option, - ) -> Option<&ChatMessage> { - self.messages - .iter() - .rev() - .find(|m| m.user_id == user_id && m.guest_session_id == guest_id) + pub fn latest_from_user(&self, user_id: Uuid) -> Option<&ChatMessage> { + self.messages.iter().rev().find(|m| m.user_id == user_id) } /// Get all messages. diff --git a/crates/chattyness-user-ui/src/components/scene_viewer.rs b/crates/chattyness-user-ui/src/components/scene_viewer.rs index 8b30d46..c7e13d2 100644 --- a/crates/chattyness-user-ui/src/components/scene_viewer.rs +++ b/crates/chattyness-user-ui/src/components/scene_viewer.rs @@ -37,7 +37,7 @@ pub fn RealmSceneViewer( scene: Scene, realm_slug: String, #[prop(into)] members: Signal>, - #[prop(into)] active_bubbles: Signal, Option), ActiveBubble>>, + #[prop(into)] active_bubbles: Signal>, #[prop(into)] loose_props: Signal>, #[prop(into)] on_move: Callback<(f64, f64)>, #[prop(into)] on_prop_click: Callback, @@ -51,11 +51,9 @@ pub fn RealmSceneViewer( #[prop(optional, into)] fading_members: Option>>, /// Current user's user_id (for context menu filtering). + /// Note: Guests are now regular users with the 'guest' tag. #[prop(optional, into)] current_user_id: Option>>, - /// Current user's guest_session_id (for context menu filtering). - #[prop(optional, into)] - current_guest_session_id: Option>>, /// Whether the current user is a guest (guests cannot use context menu). #[prop(optional, into)] is_guest: Option>, @@ -183,7 +181,6 @@ pub fn RealmSceneViewer( #[cfg(feature = "hydrate")] let on_overlay_contextmenu = { let current_user_id = current_user_id.clone(); - let current_guest_session_id = current_guest_session_id.clone(); move |ev: web_sys::MouseEvent| { use wasm_bindgen::JsCast; @@ -194,7 +191,6 @@ pub fn RealmSceneViewer( // Get current user identity for filtering let my_user_id = current_user_id.map(|s| s.get()).flatten(); - let my_guest_session_id = current_guest_session_id.map(|s| s.get()).flatten(); // Get click position let client_x = ev.client_x() as f64; @@ -215,22 +211,17 @@ pub fn RealmSceneViewer( if let Some(member_id_str) = canvas.get_attribute("data-member-id") { // Check if click hits a non-transparent pixel if hit_test_canvas(&canvas, client_x, client_y) { - // Parse the member ID to determine if it's a user_id or guest_session_id + // Parse the member ID (now always user_id since guests are users) if let Ok(member_id) = member_id_str.parse::() { // Check if this is the current user's avatar - let is_current_user = my_user_id == Some(member_id) - || my_guest_session_id == Some(member_id); + let is_current_user = my_user_id == Some(member_id); if !is_current_user { // Find the display name for this member let display_name = members .get() .iter() - .find(|m| { - m.member.user_id == Some(member_id) - || m.member.guest_session_id - == Some(member_id) - }) + .find(|m| m.member.user_id == member_id) .map(|m| m.member.display_name.clone()); if let Some(name) = display_name { diff --git a/crates/chattyness-user-ui/src/components/ws_client.rs b/crates/chattyness-user-ui/src/components/ws_client.rs index 5ca9ef0..7485e03 100644 --- a/crates/chattyness-user-ui/src/components/ws_client.rs +++ b/crates/chattyness-user-ui/src/components/ws_client.rs @@ -58,12 +58,11 @@ pub type WsSender = Box; pub type WsSenderStorage = StoredValue, LocalStorage>; /// Information about the current channel member (received on Welcome). +/// Note: Guests are now regular users with the 'guest' tag, so all members have a user_id. #[derive(Clone, Debug)] pub struct ChannelMemberInfo { - /// The user's user_id (if authenticated user). - pub user_id: Option, - /// The user's guest_session_id (if guest). - pub guest_session_id: Option, + /// The user's user_id. + pub user_id: uuid::Uuid, /// The user's display name. pub display_name: String, /// Whether this user is a guest (has the 'guest' tag). @@ -99,6 +98,12 @@ pub struct SummonInfo { pub summoned_by: String, } +/// Close function type for WebSocket (takes close code and reason). +pub type WsCloser = Box; + +/// Local stored value type for the closer (non-Send, WASM-compatible). +pub type WsCloserStorage = StoredValue, LocalStorage>; + /// Result of a moderator command. #[derive(Clone, Debug)] pub struct ModCommandResultInfo { @@ -124,6 +129,7 @@ pub struct MemberIdentityInfo { /// Returns a tuple of: /// - `Signal` - The current connection state /// - `WsSenderStorage` - A stored sender function to send messages +/// - `WsCloserStorage` - A stored close function to close the WebSocket with a specific code #[cfg(feature = "hydrate")] pub fn use_channel_websocket( realm_slug: Signal, @@ -141,9 +147,11 @@ pub fn use_channel_websocket( on_summoned: Option>, on_mod_command_result: Option>, on_member_identity_updated: Option>, -) -> (Signal, WsSenderStorage) { +) -> (Signal, WsSenderStorage, WsCloserStorage) { use std::cell::RefCell; use std::rc::Rc; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; use wasm_bindgen::{JsCast, closure::Closure}; use web_sys::{CloseEvent, ErrorEvent, MessageEvent, WebSocket}; @@ -155,6 +163,8 @@ pub fn use_channel_websocket( // Flag to track intentional closes (teleport, scene change) - guarantees local state // even if close code doesn't arrive correctly due to browser/server quirks let is_intentional_close: Rc> = Rc::new(RefCell::new(false)); + // Flag to prevent accessing disposed reactive values after component unmount + let is_disposed: Arc = Arc::new(AtomicBool::new(false)); // Create a stored sender function (using new_local for WASM single-threaded environment) let ws_ref_for_send = ws_ref.clone(); @@ -175,6 +185,17 @@ pub fn use_channel_websocket( let ws_ref_clone = ws_ref.clone(); let members_clone = members.clone(); let is_intentional_close_for_cleanup = is_intentional_close.clone(); + let is_disposed_for_effect = is_disposed.clone(); + + // Clone for closer callback (must be done before Effect captures ws_ref and is_intentional_close) + let ws_ref_for_close = ws_ref.clone(); + let is_intentional_close_for_closer = is_intentional_close.clone(); + + // Set disposed flag on cleanup to prevent accessing disposed reactive values + let is_disposed_for_cleanup = is_disposed.clone(); + on_cleanup(move || { + is_disposed_for_cleanup.store(true, Ordering::Relaxed); + }); Effect::new(move |_| { let slug = realm_slug.get(); @@ -263,7 +284,13 @@ pub fn use_channel_websocket( let heartbeat_started_clone = heartbeat_started.clone(); // For tracking current user ID to ignore self MemberLeft during reconnection let current_user_id_for_msg = current_user_id.clone(); + // For checking if component is disposed + let is_disposed_for_msg = is_disposed_for_effect.clone(); let onmessage = Closure::wrap(Box::new(move |e: MessageEvent| { + // Skip if component has been disposed + if is_disposed_for_msg.load(Ordering::Relaxed) { + return; + } if let Ok(text) = e.data().dyn_into::() { let text: String = text.into(); #[cfg(debug_assertions)] @@ -278,7 +305,7 @@ pub fn use_channel_websocket( } = msg { // Track current user ID for MemberLeft filtering - *current_user_id_for_msg.borrow_mut() = member.user_id; + *current_user_id_for_msg.borrow_mut() = Some(member.user_id); if !*heartbeat_started_clone.borrow() { *heartbeat_started_clone.borrow_mut() = true; @@ -312,7 +339,6 @@ pub fn use_channel_websocket( if let Some(ref callback) = on_welcome_clone { let info = ChannelMemberInfo { user_id: member.user_id, - guest_session_id: member.guest_session_id, display_name: member.display_name.clone(), is_guest: member.is_guest, }; @@ -345,7 +371,12 @@ pub fn use_channel_websocket( let set_ws_state_err = set_ws_state; let ws_state_for_err = ws_state; let reconnect_trigger_for_error = reconnect_trigger; + let is_disposed_for_err = is_disposed_for_effect.clone(); let onerror = Closure::wrap(Box::new(move |e: ErrorEvent| { + // Skip if component has been disposed + if is_disposed_for_err.load(Ordering::Relaxed) { + return; + } #[cfg(debug_assertions)] web_sys::console::error_1(&format!("[WS] Error: {:?}", e.message()).into()); @@ -389,7 +420,12 @@ pub fn use_channel_websocket( let set_ws_state_close = set_ws_state; let reconnect_trigger_for_close = reconnect_trigger; let is_intentional_close_for_onclose = is_intentional_close.clone(); + let is_disposed_for_close = is_disposed_for_effect.clone(); let onclose = Closure::wrap(Box::new(move |e: CloseEvent| { + // Skip if component has been disposed + if is_disposed_for_close.load(Ordering::Relaxed) { + return; + } let code = e.code(); #[cfg(debug_assertions)] web_sys::console::log_1( @@ -408,8 +444,11 @@ pub fn use_channel_websocket( reconnect_trigger.update(|v| *v = v.wrapping_add(1)); }) .forget(); - } else if code == close_codes::SCENE_CHANGE || *is_intentional_close_for_onclose.borrow() { - // Intentional close (scene change/teleport) - don't show disconnection + } else if code == close_codes::SCENE_CHANGE + || code == close_codes::LOGOUT + || *is_intentional_close_for_onclose.borrow() + { + // Intentional close (scene change/teleport/logout) - don't show disconnection // Check both code AND flag for defense-in-depth (flag is guaranteed local state) #[cfg(debug_assertions)] web_sys::console::log_1(&"[WS] Intentional close, not setting Disconnected".into()); @@ -426,7 +465,21 @@ pub fn use_channel_websocket( *ws_ref_clone.borrow_mut() = Some(ws); }); - (Signal::derive(move || ws_state.get()), sender) + // Create closer function for explicit WebSocket closure (e.g., logout) + // Uses clones created before the Effect closure captured the originals + let closer: WsCloserStorage = StoredValue::new_local(Some(Box::new(move |code: u16, reason: String| { + if let Some(ws) = ws_ref_for_close.borrow().as_ref() { + // Set intentional close flag BEFORE closing + *is_intentional_close_for_closer.borrow_mut() = true; + #[cfg(debug_assertions)] + web_sys::console::log_1( + &format!("[WS] Closing with code={}, reason={}", code, reason).into(), + ); + let _ = ws.close_with_code_and_reason(code, &reason); + } + }))); + + (Signal::derive(move || ws_state.get()), sender, closer) } /// Handle a message received from the server. @@ -447,128 +500,118 @@ fn handle_server_message( on_member_identity_updated: &Option>, current_user_id: &std::rc::Rc>>, ) { - let mut members_vec = members.borrow_mut(); + // Process message and collect any callbacks to run AFTER releasing the borrow + enum PostAction { + None, + UpdateMembers(Vec), + UpdateMembersAndFade(Vec, FadingMember), + UpdateMembersAndIdentity(Vec, MemberIdentityInfo), + ChatMessage(ChatMessage), + LoosePropsSync(Vec), + PropDropped(LooseProp), + PropPickedUp(uuid::Uuid), + Error(WsError), + TeleportApproved(TeleportInfo), + Summoned(SummonInfo), + ModCommandResult(ModCommandResultInfo), + } - match msg { - ServerMessage::Welcome { - member: _, - members: initial_members, - config: _, // Config is handled in the caller for heartbeat setup - } => { - *members_vec = initial_members; - on_update.run(members_vec.clone()); - } - ServerMessage::MemberJoined { member } => { - // Remove if exists (rejoin case), then add - members_vec.retain(|m| { - m.member.user_id != member.member.user_id - || m.member.guest_session_id != member.member.guest_session_id - }); - members_vec.push(member); - on_update.run(members_vec.clone()); - } - ServerMessage::MemberLeft { - user_id, - guest_session_id, - reason, - } => { - // Check if this is our own MemberLeft due to timeout - ignore it during reconnection - // so we don't see our own avatar fade out - let own_user_id = *current_user_id.borrow(); - let is_self = own_user_id.is_some() && user_id == own_user_id; - if is_self && reason == DisconnectReason::Timeout { - #[cfg(debug_assertions)] - web_sys::console::log_1( - &"[WS] Ignoring self MemberLeft during reconnection".into(), - ); - return; + let action = { + let mut members_vec = members.borrow_mut(); + let own_user_id = *current_user_id.borrow(); + + match msg { + ServerMessage::Welcome { + member: _, + members: initial_members, + config: _, // Config is handled in the caller for heartbeat setup + } => { + *members_vec = initial_members; + PostAction::UpdateMembers(members_vec.clone()) } + ServerMessage::MemberJoined { member } => { + // Remove if exists (rejoin case), then add + members_vec.retain(|m| m.member.user_id != member.member.user_id); + members_vec.push(member); + PostAction::UpdateMembers(members_vec.clone()) + } + ServerMessage::MemberLeft { user_id, reason } => { + // Check if this is our own MemberLeft due to timeout - ignore it during reconnection + // so we don't see our own avatar fade out + let is_self = own_user_id.is_some_and(|id| user_id == id); + if is_self && reason == DisconnectReason::Timeout { + #[cfg(debug_assertions)] + web_sys::console::log_1( + &"[WS] Ignoring self MemberLeft during reconnection".into(), + ); + return; + } - // Find the member before removing - let leaving_member = members_vec - .iter() - .find(|m| { - m.member.user_id == user_id && m.member.guest_session_id == guest_session_id - }) - .cloned(); + // Find the member before removing + let leaving_member = members_vec + .iter() + .find(|m| m.member.user_id == user_id) + .cloned(); - // Always remove from active members list - members_vec.retain(|m| { - m.member.user_id != user_id || m.member.guest_session_id != guest_session_id - }); - on_update.run(members_vec.clone()); + // Always remove from active members list + members_vec.retain(|m| m.member.user_id != user_id); + let updated = members_vec.clone(); - // For timeout disconnects, trigger fading animation - if reason == DisconnectReason::Timeout { - if let Some(member) = leaving_member { - let fading = FadingMember { - member, - fade_start: js_sys::Date::now() as i64, - fade_duration: FADE_DURATION_MS, - }; - on_member_fading.run(fading); + // For timeout disconnects, trigger fading animation + if reason == DisconnectReason::Timeout { + if let Some(member) = leaving_member { + let fading = FadingMember { + member, + fade_start: js_sys::Date::now() as i64, + fade_duration: FADE_DURATION_MS, + }; + PostAction::UpdateMembersAndFade(updated, fading) + } else { + PostAction::UpdateMembers(updated) + } + } else { + PostAction::UpdateMembers(updated) } } - } - ServerMessage::PositionUpdated { - user_id, - guest_session_id, - x, - y, - } => { - if let Some(m) = members_vec.iter_mut().find(|m| { - m.member.user_id == user_id && m.member.guest_session_id == guest_session_id - }) { - m.member.position_x = x; - m.member.position_y = y; + ServerMessage::PositionUpdated { user_id, x, y } => { + if let Some(m) = members_vec + .iter_mut() + .find(|m| m.member.user_id == user_id) + { + m.member.position_x = x; + m.member.position_y = y; + } + PostAction::UpdateMembers(members_vec.clone()) } - on_update.run(members_vec.clone()); - } - ServerMessage::EmotionUpdated { - user_id, - guest_session_id, - emotion, - emotion_layer, - } => { - if let Some(m) = members_vec.iter_mut().find(|m| { - m.member.user_id == user_id && m.member.guest_session_id == guest_session_id - }) { - // Parse emotion name to EmotionState - m.member.current_emotion = emotion - .parse::() - .unwrap_or_default(); - m.avatar.emotion_layer = emotion_layer; + ServerMessage::EmotionUpdated { + user_id, + emotion, + emotion_layer, + } => { + if let Some(m) = members_vec + .iter_mut() + .find(|m| m.member.user_id == user_id) + { + // Parse emotion name to EmotionState + m.member.current_emotion = emotion + .parse::() + .unwrap_or_default(); + m.avatar.emotion_layer = emotion_layer; + } + PostAction::UpdateMembers(members_vec.clone()) } - on_update.run(members_vec.clone()); - } - ServerMessage::Pong => { - // Heartbeat acknowledged - nothing to do - } - ServerMessage::Error { code, message } => { - // Always log errors to console (not just debug mode) - web_sys::console::error_1(&format!("[WS] Server error: {} - {}", code, message).into()); - // Call error callback if provided - if let Some(callback) = on_error { - callback.run(WsError { code, message }); + ServerMessage::Pong => { + // Heartbeat acknowledged - nothing to do + PostAction::None } - } - ServerMessage::ChatMessageReceived { - message_id, - user_id, - guest_session_id, - display_name, - content, - emotion, - x, - y, - timestamp, - is_whisper, - is_same_scene, - } => { - let chat_msg = ChatMessage { + ServerMessage::Error { code, message } => { + // Always log errors to console (not just debug mode) + web_sys::console::error_1(&format!("[WS] Server error: {} - {}", code, message).into()); + PostAction::Error(WsError { code, message }) + } + ServerMessage::ChatMessageReceived { message_id, user_id, - guest_session_id, display_name, content, emotion, @@ -577,119 +620,171 @@ fn handle_server_message( timestamp, is_whisper, is_same_scene, - is_system: false, - }; - on_chat_message.run(chat_msg); - } - ServerMessage::LoosePropsSync { props } => { - on_loose_props_sync.run(props); - } - ServerMessage::PropDropped { prop } => { - on_prop_dropped.run(prop); - } - ServerMessage::PropPickedUp { prop_id, .. } => { - on_prop_picked_up.run(prop_id); - } - ServerMessage::PropExpired { prop_id } => { - // Treat expired props the same as picked up (remove from display) - on_prop_picked_up.run(prop_id); - } - ServerMessage::AvatarUpdated { - user_id, - guest_session_id, - avatar, - } => { - // Find member and update their avatar layers - if let Some(m) = members_vec.iter_mut().find(|m| { - m.member.user_id == user_id && m.member.guest_session_id == guest_session_id - }) { - m.avatar.skin_layer = avatar.skin_layer.clone(); - m.avatar.clothes_layer = avatar.clothes_layer.clone(); - m.avatar.accessories_layer = avatar.accessories_layer.clone(); - m.avatar.emotion_layer = avatar.emotion_layer.clone(); + } => { + PostAction::ChatMessage(ChatMessage { + message_id, + user_id, + display_name, + content, + emotion, + x, + y, + timestamp, + is_whisper, + is_same_scene, + is_system: false, + }) } - on_update.run(members_vec.clone()); - } - ServerMessage::TeleportApproved { - scene_id, - scene_slug, - } => { - if let Some(callback) = on_teleport_approved { - callback.run(TeleportInfo { + ServerMessage::LoosePropsSync { props } => { + PostAction::LoosePropsSync(props) + } + ServerMessage::PropDropped { prop } => { + PostAction::PropDropped(prop) + } + ServerMessage::PropPickedUp { prop_id, .. } => { + PostAction::PropPickedUp(prop_id) + } + ServerMessage::PropExpired { prop_id } => { + // Treat expired props the same as picked up (remove from display) + PostAction::PropPickedUp(prop_id) + } + ServerMessage::AvatarUpdated { user_id, avatar } => { + // Find member and update their avatar layers + if let Some(m) = members_vec + .iter_mut() + .find(|m| m.member.user_id == user_id) + { + m.avatar.skin_layer = avatar.skin_layer.clone(); + m.avatar.clothes_layer = avatar.clothes_layer.clone(); + m.avatar.accessories_layer = avatar.accessories_layer.clone(); + m.avatar.emotion_layer = avatar.emotion_layer.clone(); + } + PostAction::UpdateMembers(members_vec.clone()) + } + ServerMessage::TeleportApproved { + scene_id, + scene_slug, + } => { + PostAction::TeleportApproved(TeleportInfo { scene_id, scene_slug, - }); + }) } - } - ServerMessage::Summoned { - scene_id, - scene_slug, - summoned_by, - } => { - if let Some(callback) = on_summoned { - callback.run(SummonInfo { + ServerMessage::Summoned { + scene_id, + scene_slug, + summoned_by, + } => { + PostAction::Summoned(SummonInfo { scene_id, scene_slug, summoned_by, - }); + }) + } + ServerMessage::ModCommandResult { success, message } => { + PostAction::ModCommandResult(ModCommandResultInfo { success, message }) + } + ServerMessage::MemberIdentityUpdated { + user_id, + display_name, + is_guest, + } => { + // Update the internal members list so subsequent updates don't overwrite + if let Some(member) = members_vec + .iter_mut() + .find(|m| m.member.user_id == user_id) + { + member.member.display_name = display_name.clone(); + member.member.is_guest = is_guest; + } + PostAction::UpdateMembersAndIdentity( + members_vec.clone(), + MemberIdentityInfo { + user_id, + display_name, + is_guest, + }, + ) + } + ServerMessage::AvatarForced { + user_id, + avatar, + reason: _, + forced_by: _, + } => { + // Update the forced user's avatar + if let Some(m) = members_vec.iter_mut().find(|m| m.member.user_id == user_id) { + m.avatar.skin_layer = avatar.skin_layer.clone(); + m.avatar.clothes_layer = avatar.clothes_layer.clone(); + m.avatar.accessories_layer = avatar.accessories_layer.clone(); + m.avatar.emotion_layer = avatar.emotion_layer.clone(); + } + PostAction::UpdateMembers(members_vec.clone()) + } + ServerMessage::AvatarCleared { + user_id, + avatar, + cleared_by: _, + } => { + // Restore the user's original avatar + if let Some(m) = members_vec.iter_mut().find(|m| m.member.user_id == user_id) { + m.avatar.skin_layer = avatar.skin_layer.clone(); + m.avatar.clothes_layer = avatar.clothes_layer.clone(); + m.avatar.accessories_layer = avatar.accessories_layer.clone(); + m.avatar.emotion_layer = avatar.emotion_layer.clone(); + } + PostAction::UpdateMembers(members_vec.clone()) } } - ServerMessage::ModCommandResult { success, message } => { - if let Some(callback) = on_mod_command_result { - callback.run(ModCommandResultInfo { success, message }); - } - } - ServerMessage::MemberIdentityUpdated { - user_id, - display_name, - is_guest, - } => { - // Update the internal members list so subsequent updates don't overwrite - if let Some(member) = members_vec - .iter_mut() - .find(|m| m.member.user_id == Some(user_id)) - { - member.member.display_name = display_name.clone(); - member.member.is_guest = is_guest; - } - on_update.run(members_vec.clone()); + }; // members_vec borrow is dropped here + // Now run callbacks without holding any borrows + match action { + PostAction::None => {} + PostAction::UpdateMembers(members) => { + on_update.run(members); + } + PostAction::UpdateMembersAndFade(members, fading) => { + on_update.run(members); + on_member_fading.run(fading); + } + PostAction::UpdateMembersAndIdentity(members, info) => { + on_update.run(members); if let Some(callback) = on_member_identity_updated { - callback.run(MemberIdentityInfo { - user_id, - display_name, - is_guest, - }); + callback.run(info); } } - ServerMessage::AvatarForced { - user_id, - avatar, - reason: _, - forced_by: _, - } => { - // Update the forced user's avatar - if let Some(m) = members_vec.iter_mut().find(|m| m.member.user_id == Some(user_id)) { - m.avatar.skin_layer = avatar.skin_layer.clone(); - m.avatar.clothes_layer = avatar.clothes_layer.clone(); - m.avatar.accessories_layer = avatar.accessories_layer.clone(); - m.avatar.emotion_layer = avatar.emotion_layer.clone(); - } - on_update.run(members_vec.clone()); + PostAction::ChatMessage(msg) => { + on_chat_message.run(msg); } - ServerMessage::AvatarCleared { - user_id, - avatar, - cleared_by: _, - } => { - // Restore the user's original avatar - if let Some(m) = members_vec.iter_mut().find(|m| m.member.user_id == Some(user_id)) { - m.avatar.skin_layer = avatar.skin_layer.clone(); - m.avatar.clothes_layer = avatar.clothes_layer.clone(); - m.avatar.accessories_layer = avatar.accessories_layer.clone(); - m.avatar.emotion_layer = avatar.emotion_layer.clone(); + PostAction::LoosePropsSync(props) => { + on_loose_props_sync.run(props); + } + PostAction::PropDropped(prop) => { + on_prop_dropped.run(prop); + } + PostAction::PropPickedUp(prop_id) => { + on_prop_picked_up.run(prop_id); + } + PostAction::Error(err) => { + if let Some(callback) = on_error { + callback.run(err); + } + } + PostAction::TeleportApproved(info) => { + if let Some(callback) = on_teleport_approved { + callback.run(info); + } + } + PostAction::Summoned(info) => { + if let Some(callback) = on_summoned { + callback.run(info); + } + } + PostAction::ModCommandResult(info) => { + if let Some(callback) = on_mod_command_result { + callback.run(info); } - on_update.run(members_vec.clone()); } } } @@ -712,8 +807,9 @@ pub fn use_channel_websocket( _on_summoned: Option>, _on_mod_command_result: Option>, _on_member_identity_updated: Option>, -) -> (Signal, WsSenderStorage) { +) -> (Signal, WsSenderStorage, WsCloserStorage) { let (ws_state, _) = signal(WsState::Disconnected); let sender: WsSenderStorage = StoredValue::new_local(None); - (Signal::derive(move || ws_state.get()), sender) + let closer: WsCloserStorage = StoredValue::new_local(None); + (Signal::derive(move || ws_state.get()), sender, closer) } diff --git a/crates/chattyness-user-ui/src/pages/realm.rs b/crates/chattyness-user-ui/src/pages/realm.rs index 85af451..ba56c06 100644 --- a/crates/chattyness-user-ui/src/pages/realm.rs +++ b/crates/chattyness-user-ui/src/pages/realm.rs @@ -31,7 +31,7 @@ use chattyness_db::models::{ RealmWithUserRole, Scene, SceneSummary, }; #[cfg(feature = "hydrate")] -use chattyness_db::ws_messages::ClientMessage; +use chattyness_db::ws_messages::{close_codes, ClientMessage}; #[cfg(not(feature = "hydrate"))] use crate::components::ws_client::WsSender; @@ -65,8 +65,8 @@ pub fn RealmPage() -> impl IntoView { // Chat message state - use StoredValue for WASM compatibility (single-threaded) let message_log: StoredValue = StoredValue::new_local(MessageLog::new()); - let (active_bubbles, set_active_bubbles) = - signal(HashMap::<(Option, Option), ActiveBubble>::new()); + // Bubble key is now just user_id since guests are regular users with the 'guest' tag + let (active_bubbles, set_active_bubbles) = signal(HashMap::::new()); // Inventory popup state let (inventory_open, set_inventory_open) = signal(false); @@ -112,8 +112,8 @@ pub fn RealmPage() -> impl IntoView { let (current_position, set_current_position) = signal((400.0_f64, 300.0_f64)); // Current user identity (received from WebSocket Welcome message) + // Note: Guests are now regular users with the 'guest' tag, so everyone has a user_id let (current_user_id, set_current_user_id) = signal(Option::::None); - let (current_guest_session_id, set_current_guest_session_id) = signal(Option::::None); // Whether the current user is a guest (has the 'guest' tag) let (is_guest, set_is_guest) = signal(false); @@ -246,7 +246,6 @@ pub fn RealmPage() -> impl IntoView { fading.retain(|f| { !new_members.iter().any(|m| { m.member.user_id == f.member.member.user_id - && m.member.guest_session_id == f.member.member.guest_session_id }) }); }); @@ -275,7 +274,7 @@ pub fn RealmPage() -> impl IntoView { if msg.is_same_scene { // Same scene whisper: show as italic bubble (handled by bubble rendering) - let key = (msg.user_id, msg.guest_session_id); + let key = msg.user_id; let expires_at = msg.timestamp + DEFAULT_BUBBLE_TIMEOUT_MS; set_active_bubbles.update(|bubbles| { bubbles.insert( @@ -292,7 +291,7 @@ pub fn RealmPage() -> impl IntoView { } } else { // Regular broadcast: show as bubble - let key = (msg.user_id, msg.guest_session_id); + let key = msg.user_id; let expires_at = msg.timestamp + DEFAULT_BUBBLE_TIMEOUT_MS; set_active_bubbles.update(|bubbles| { bubbles.insert( @@ -333,7 +332,6 @@ pub fn RealmPage() -> impl IntoView { // Remove any existing entry for this user (shouldn't happen, but be safe) members.retain(|m| { m.member.member.user_id != fading.member.member.user_id - || m.member.member.guest_session_id != fading.member.member.guest_session_id }); members.push(fading); }); @@ -342,8 +340,7 @@ pub fn RealmPage() -> impl IntoView { // Callback to capture current user identity from Welcome message #[cfg(feature = "hydrate")] let on_welcome = Callback::new(move |info: ChannelMemberInfo| { - set_current_user_id.set(info.user_id); - set_current_guest_session_id.set(info.guest_session_id); + set_current_user_id.set(Some(info.user_id)); set_current_display_name.set(info.display_name.clone()); set_is_guest.set(info.is_guest); }); @@ -373,8 +370,7 @@ pub fn RealmPage() -> impl IntoView { // Log teleport to message log let teleport_msg = ChatMessage { message_id: Uuid::new_v4(), - user_id: None, - guest_session_id: None, + user_id: Uuid::nil(), // System message display_name: "[SYSTEM]".to_string(), content: format!("Teleported to scene: {}", info.scene_slug), emotion: "neutral".to_string(), @@ -453,8 +449,7 @@ pub fn RealmPage() -> impl IntoView { // Log summon to message log let summon_msg = ChatMessage { message_id: Uuid::new_v4(), - user_id: None, - guest_session_id: None, + user_id: Uuid::nil(), // System/mod message display_name: "[MOD]".to_string(), content: format!("Summoned by {} to scene: {}", info.summoned_by, info.scene_slug), emotion: "neutral".to_string(), @@ -543,8 +538,7 @@ pub fn RealmPage() -> impl IntoView { let status = if info.success { "OK" } else { "FAILED" }; let mod_msg = ChatMessage { message_id: Uuid::new_v4(), - user_id: None, - guest_session_id: None, + user_id: Uuid::nil(), // System/mod message display_name: "[MOD]".to_string(), content: format!("[{}] {}", status, info.message), emotion: "neutral".to_string(), @@ -573,7 +567,7 @@ pub fn RealmPage() -> impl IntoView { set_members.update(|members| { if let Some(member) = members .iter_mut() - .find(|m| m.member.user_id == Some(info.user_id)) + .find(|m| m.member.user_id == info.user_id) { member.member.display_name = info.display_name.clone(); } @@ -581,7 +575,7 @@ pub fn RealmPage() -> impl IntoView { }); #[cfg(feature = "hydrate")] - let (ws_state, ws_sender) = use_channel_websocket( + let (ws_state, ws_sender, ws_close) = use_channel_websocket( slug, Signal::derive(move || channel_id.get()), reconnect_trigger, @@ -826,8 +820,13 @@ pub fn RealmPage() -> impl IntoView { Rc::new(RefCell::new(None)); let closure_holder_clone = closure_holder.clone(); + // Holder for keyup closure (for hotkey help dismissal) + let keyup_closure_holder: Rc>>> = + Rc::new(RefCell::new(None)); + let keyup_closure_holder_clone = keyup_closure_holder.clone(); + Effect::new(move |_| { - // Cleanup previous closure if any + // Cleanup previous keydown closure if any if let Some(old_closure) = closure_holder_clone.borrow_mut().take() { if let Some(window) = web_sys::window() { let _ = window.remove_event_listener_with_callback( @@ -837,6 +836,16 @@ pub fn RealmPage() -> impl IntoView { } } + // Cleanup previous keyup closure if any + if let Some(old_closure) = keyup_closure_holder_clone.borrow_mut().take() { + if let Some(window) = web_sys::window() { + let _ = window.remove_event_listener_with_callback( + "keyup", + old_closure.as_ref().unchecked_ref(), + ); + } + } + let current_slug = slug.get(); if current_slug.is_empty() { return; @@ -1071,8 +1080,44 @@ pub fn RealmPage() -> impl IntoView { ); } - // Forget the keyup closure (it lives for the duration of the page) - keyup_closure.forget(); + // Store the keyup closure for cleanup + *keyup_closure_holder_clone.borrow_mut() = Some(keyup_closure); + }); + + // Cleanup event listeners when component unmounts + // We need to store JS function references for cleanup since Rc isn't Send+Sync + let keydown_fn: StoredValue, LocalStorage> = StoredValue::new_local(None); + let keyup_fn: StoredValue, LocalStorage> = StoredValue::new_local(None); + + // Store references to the JS functions for cleanup + Effect::new({ + let closure_holder = closure_holder.clone(); + let keyup_closure_holder = keyup_closure_holder.clone(); + move |_| { + if let Some(ref closure) = *closure_holder.borrow() { + let func: &js_sys::Function = closure.as_ref().unchecked_ref(); + keydown_fn.set_value(Some(func.clone())); + } + if let Some(ref closure) = *keyup_closure_holder.borrow() { + let func: &js_sys::Function = closure.as_ref().unchecked_ref(); + keyup_fn.set_value(Some(func.clone())); + } + } + }); + + on_cleanup(move || { + if let Some(window) = web_sys::window() { + keydown_fn.with_value(|func| { + if let Some(f) = func { + let _ = window.remove_event_listener_with_callback("keydown", f); + } + }); + keyup_fn.with_value(|func| { + if let Some(f) = func { + let _ = window.remove_event_listener_with_callback("keyup", f); + } + }); + } }); // Save position on page unload (beforeunload event) @@ -1109,19 +1154,33 @@ pub fn RealmPage() -> impl IntoView { set_chat_focused.set(focused); }); - // Create logout callback (WebSocket disconnects automatically) + // Create logout callback - explicitly close WebSocket before calling logout API let on_logout = Callback::new(move |_: ()| { #[cfg(feature = "hydrate")] { use gloo_net::http::Request; + use gloo_timers::callback::Timeout; let navigate = navigate.clone(); - spawn_local(async move { - // WebSocket close handles channel leave automatically - let _: Result = - Request::post("/api/auth/logout").send().await; - navigate("/", Default::default()); + // 1. Close WebSocket explicitly with LOGOUT code + ws_close.with_value(|closer| { + if let Some(close_fn) = closer { + close_fn(close_codes::LOGOUT, "logout".to_string()); + } }); + + // 2. Small delay to ensure close message is sent, then call logout API + Timeout::new(100, move || { + spawn_local(async move { + // 3. Call logout API + let _: Result = + Request::post("/api/auth/logout").send().await; + + // 4. Navigate to home + navigate("/", Default::default()); + }); + }) + .forget(); } }); @@ -1279,7 +1338,6 @@ pub fn RealmPage() -> impl IntoView { }) fading_members=Signal::derive(move || fading_members.get()) current_user_id=Signal::derive(move || current_user_id.get()) - current_guest_session_id=Signal::derive(move || current_guest_session_id.get()) is_guest=Signal::derive(move || is_guest.get()) on_whisper_request=on_whisper_request_cb /> diff --git a/db/schema/functions/001_helpers.sql b/db/schema/functions/001_helpers.sql index 8e25392..825a650 100644 --- a/db/schema/functions/001_helpers.sql +++ b/db/schema/functions/001_helpers.sql @@ -68,24 +68,6 @@ EXCEPTION END; $$ LANGUAGE plpgsql STABLE; --- Set current guest session ID for RLS -CREATE OR REPLACE FUNCTION public.set_current_guest_session_id(guest_session_id UUID) -RETURNS VOID AS $$ -BEGIN - PERFORM set_config('app.current_guest_session_id', guest_session_id::TEXT, false); -END; -$$ LANGUAGE plpgsql; - --- Get current guest session ID for RLS -CREATE OR REPLACE FUNCTION public.current_guest_session_id() -RETURNS UUID AS $$ -BEGIN - RETURN NULLIF(current_setting('app.current_guest_session_id', true), '')::UUID; -EXCEPTION - WHEN OTHERS THEN RETURN NULL; -END; -$$ LANGUAGE plpgsql STABLE; - -- Check if current user is a server admin CREATE OR REPLACE FUNCTION public.is_server_admin() RETURNS BOOLEAN AS $$ @@ -318,4 +300,34 @@ COMMENT ON FUNCTION scene.clear_stale_instance_members(DOUBLE PRECISION) IS GRANT EXECUTE ON FUNCTION scene.clear_all_instance_members() TO chattyness_app; GRANT EXECUTE ON FUNCTION scene.clear_stale_instance_members(DOUBLE PRECISION) TO chattyness_app; +-- ============================================================================= +-- Guest Cleanup Functions +-- ============================================================================= + +-- Clean up stale guest accounts that haven't been active in 7 days +-- Guests are users with the 'guest' tag in auth.users +-- Uses SECURITY DEFINER to bypass RLS +CREATE OR REPLACE FUNCTION auth.cleanup_stale_guests() +RETURNS INTEGER AS $$ +DECLARE + deleted_count INTEGER; +BEGIN + WITH deleted AS ( + DELETE FROM auth.users + WHERE 'guest' = ANY(tags) + AND last_seen_at < now() - interval '7 days' + RETURNING id + ) + SELECT count(*) INTO deleted_count FROM deleted; + + RETURN deleted_count; +END; +$$ LANGUAGE plpgsql SECURITY DEFINER; + +COMMENT ON FUNCTION auth.cleanup_stale_guests() IS + 'Removes guest accounts (users with guest tag) inactive for 7+ days. Run via cron.'; + +-- Grant execute to chattyness_app +GRANT EXECUTE ON FUNCTION auth.cleanup_stale_guests() TO chattyness_app; + COMMIT; diff --git a/db/schema/policies/001_rls.sql b/db/schema/policies/001_rls.sql index 559ebf8..b216a6d 100644 --- a/db/schema/policies/001_rls.sql +++ b/db/schema/policies/001_rls.sql @@ -259,16 +259,6 @@ CREATE POLICY auth_sessions_admin ON auth.sessions GRANT SELECT, INSERT, UPDATE, DELETE ON auth.sessions TO chattyness_app; --- auth.guest_sessions -ALTER TABLE auth.guest_sessions ENABLE ROW LEVEL SECURITY; - -CREATE POLICY auth_guest_sessions_all ON auth.guest_sessions - FOR ALL TO chattyness_app - USING (true) - WITH CHECK (true); - -GRANT SELECT, INSERT, UPDATE, DELETE ON auth.guest_sessions TO chattyness_app; - -- auth.tower_sessions ALTER TABLE auth.tower_sessions ENABLE ROW LEVEL SECURITY; @@ -727,14 +717,8 @@ CREATE POLICY scene_instance_members_select ON scene.instance_members CREATE POLICY scene_instance_members_own ON scene.instance_members FOR ALL TO chattyness_app - USING ( - user_id = public.current_user_id() - OR guest_session_id = public.current_guest_session_id() - ) - WITH CHECK ( - user_id = public.current_user_id() - OR guest_session_id = public.current_guest_session_id() - ); + USING (user_id = public.current_user_id()) + WITH CHECK (user_id = public.current_user_id()); GRANT SELECT, INSERT, UPDATE, DELETE ON scene.instance_members TO chattyness_app; @@ -929,16 +913,12 @@ CREATE POLICY chat_messages_select ON chat.messages CREATE POLICY chat_messages_insert ON chat.messages FOR INSERT TO chattyness_app - WITH CHECK ( - user_id = public.current_user_id() - OR guest_session_id = public.current_guest_session_id() - ); + WITH CHECK (user_id = public.current_user_id()); CREATE POLICY chat_messages_update ON chat.messages FOR UPDATE TO chattyness_app USING ( user_id = public.current_user_id() - OR guest_session_id = public.current_guest_session_id() OR public.is_server_moderator() ); diff --git a/db/schema/tables/030_realm.sql b/db/schema/tables/030_realm.sql index 9379305..352612f 100644 --- a/db/schema/tables/030_realm.sql +++ b/db/schema/tables/030_realm.sql @@ -102,35 +102,6 @@ ALTER TABLE realm.realms ADD CONSTRAINT fk_realm_realms_default_scene FOREIGN KEY (default_scene_id) REFERENCES realm.scenes(id) ON DELETE SET NULL; --- ============================================================================= --- Guest Sessions (created here since it references realm tables) --- ============================================================================= --- Note: current_instance_id FK is added in 045_scene.sql after scene.instances exists - -CREATE TABLE auth.guest_sessions ( - id UUID PRIMARY KEY DEFAULT gen_random_uuid(), - - guest_name public.display_name NOT NULL, - token_hash TEXT NOT NULL, - - user_agent TEXT, - ip_address INET, - - current_realm_id UUID REFERENCES realm.realms(id) ON DELETE SET NULL, - current_instance_id UUID, -- FK added in 045_scene.sql - - expires_at TIMESTAMPTZ NOT NULL, - last_activity_at TIMESTAMPTZ NOT NULL DEFAULT now(), - created_at TIMESTAMPTZ NOT NULL DEFAULT now(), - - CONSTRAINT uq_auth_guest_sessions_token UNIQUE (token_hash) -); - -COMMENT ON TABLE auth.guest_sessions IS 'Anonymous guest sessions'; - -CREATE INDEX idx_auth_guest_sessions_expires ON auth.guest_sessions (expires_at); -CREATE INDEX idx_auth_guest_sessions_ip ON auth.guest_sessions (ip_address); - -- ============================================================================= -- Realm Memberships -- ============================================================================= diff --git a/db/schema/tables/045_scene.sql b/db/schema/tables/045_scene.sql index 8d1bf87..1343c0e 100644 --- a/db/schema/tables/045_scene.sql +++ b/db/schema/tables/045_scene.sql @@ -40,30 +40,19 @@ CREATE INDEX idx_scene_instances_scene ON scene.instances (scene_id); CREATE INDEX idx_scene_instances_type ON scene.instances (scene_id, instance_type); CREATE INDEX idx_scene_instances_expires ON scene.instances (expires_at) WHERE expires_at IS NOT NULL; --- ============================================================================= --- Add FK from auth.guest_sessions to scene.instances --- ============================================================================= --- guest_sessions.current_instance_id was added without FK in 030_realm.sql --- Now we can add the constraint since scene.instances exists --- ============================================================================= - -ALTER TABLE auth.guest_sessions - ADD CONSTRAINT fk_auth_guest_sessions_instance - FOREIGN KEY (current_instance_id) REFERENCES scene.instances(id) ON DELETE SET NULL; - -- ============================================================================= -- Instance Members (renamed from realm.channel_members) -- ============================================================================= -- Users currently present in an instance with their positions. -- Note: instance_id is actually scene_id in this system (scenes are used directly as instances). +-- Guests are regular users with the 'guest' tag in auth.users. -- ============================================================================= CREATE TABLE scene.instance_members ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), instance_id UUID NOT NULL REFERENCES realm.scenes(id) ON DELETE CASCADE, - user_id UUID REFERENCES auth.users(id) ON DELETE CASCADE, - guest_session_id UUID REFERENCES auth.guest_sessions(id) ON DELETE CASCADE, + user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE, position public.virtual_point NOT NULL DEFAULT ST_SetSRID(ST_MakePoint(400, 300), 0), @@ -74,19 +63,13 @@ CREATE TABLE scene.instance_members ( joined_at TIMESTAMPTZ NOT NULL DEFAULT now(), last_moved_at TIMESTAMPTZ NOT NULL DEFAULT now(), - CONSTRAINT chk_scene_instance_members_user_or_guest CHECK ( - (user_id IS NOT NULL AND guest_session_id IS NULL) OR - (user_id IS NULL AND guest_session_id IS NOT NULL) - ), - CONSTRAINT uq_scene_instance_members_user UNIQUE (instance_id, user_id), - CONSTRAINT uq_scene_instance_members_guest UNIQUE (instance_id, guest_session_id) + CONSTRAINT uq_scene_instance_members_user UNIQUE (instance_id, user_id) ); -COMMENT ON TABLE scene.instance_members IS 'Users in an instance with positions'; +COMMENT ON TABLE scene.instance_members IS 'Users in an instance with positions (guests are users with guest tag)'; CREATE INDEX idx_scene_instance_members_instance ON scene.instance_members (instance_id); -CREATE INDEX idx_scene_instance_members_user ON scene.instance_members (user_id) WHERE user_id IS NOT NULL; -CREATE INDEX idx_scene_instance_members_guest ON scene.instance_members (guest_session_id) WHERE guest_session_id IS NOT NULL; +CREATE INDEX idx_scene_instance_members_user ON scene.instance_members (user_id); CREATE INDEX idx_scene_instance_members_position ON scene.instance_members USING GIST (position); -- ============================================================================= diff --git a/db/schema/tables/050_chat.sql b/db/schema/tables/050_chat.sql index 3cb95a7..5222fb2 100644 --- a/db/schema/tables/050_chat.sql +++ b/db/schema/tables/050_chat.sql @@ -28,9 +28,8 @@ CREATE TABLE chat.messages ( instance_id UUID NOT NULL REFERENCES scene.instances(id) ON DELETE CASCADE, - -- Sender (either user or guest) - user_id UUID REFERENCES auth.users(id) ON DELETE SET NULL, - guest_session_id UUID REFERENCES auth.guest_sessions(id) ON DELETE SET NULL, + -- Sender (all users including guests - guests have 'guest' tag in auth.users) + user_id UUID NOT NULL REFERENCES auth.users(id) ON DELETE SET NULL, -- Cached sender info (in case account deleted) sender_name public.display_name NOT NULL, @@ -51,13 +50,7 @@ CREATE TABLE chat.messages ( deleted_at TIMESTAMPTZ, -- Timestamps - created_at TIMESTAMPTZ NOT NULL DEFAULT now(), - - -- Either user_id or guest_session_id must be set - CONSTRAINT chk_chat_messages_sender CHECK ( - (user_id IS NOT NULL AND guest_session_id IS NULL) OR - (user_id IS NULL AND guest_session_id IS NOT NULL) - ) + created_at TIMESTAMPTZ NOT NULL DEFAULT now() ); COMMENT ON TABLE chat.messages IS 'Instance messages (design supports future time-based partitioning)';