diff --git a/cloud.go b/cloud.go index 8657376b..ea6d934b 100644 --- a/cloud.go +++ b/cloud.go @@ -512,12 +512,8 @@ func handleSessionRequest( _ = wsjson.Write(context.Background(), c, gin.H{"error": "session manager not initialized"}) return fmt.Errorf("session manager not initialized") } - scopedLogger.Debug().Msg("About to call AddSession") + err = sessionManager.AddSession(session, req.SessionSettings) - scopedLogger.Debug(). - Bool("addSessionSucceeded", err == nil). - Str("error", fmt.Sprintf("%v", err)). - Msg("AddSession returned") if err != nil { scopedLogger.Warn().Err(err).Msg("failed to add session to session manager") if err == ErrMaxSessionsReached { @@ -527,7 +523,6 @@ func handleSessionRequest( } return err } - scopedLogger.Debug().Msg("AddSession completed successfully, continuing") if session.HasPermission(PermissionPaste) { cancelKeyboardMacro() diff --git a/jsonrpc.go b/jsonrpc.go index 168915eb..33c7974a 100644 --- a/jsonrpc.go +++ b/jsonrpc.go @@ -192,12 +192,10 @@ func onRPCMessage(message webrtc.DataChannelMessage, session *Session) { if err := RequirePermission(session, PermissionSessionApprove); err != nil { handlerErr = err } else if sessionID, ok := request.Params["sessionId"].(string); ok { - if targetSession := sessionManager.GetSession(sessionID); targetSession != nil && targetSession.Mode == SessionModePending { - targetSession.Mode = SessionModeObserver - sessionManager.broadcastSessionListUpdate() + handlerErr = sessionManager.ApproveSession(sessionID) + if handlerErr == nil { + go sessionManager.broadcastSessionListUpdate() result = map[string]interface{}{"status": "approved"} - } else { - handlerErr = errors.New("session not found or not pending") } } else { handlerErr = errors.New("invalid sessionId parameter") @@ -206,14 +204,18 @@ func onRPCMessage(message webrtc.DataChannelMessage, session *Session) { if err := RequirePermission(session, PermissionSessionApprove); err != nil { handlerErr = err } else if sessionID, ok := request.Params["sessionId"].(string); ok { - if targetSession := sessionManager.GetSession(sessionID); targetSession != nil && targetSession.Mode == SessionModePending { - writeJSONRPCEvent("sessionAccessDenied", map[string]interface{}{ - "message": "Access denied by primary session", - }, targetSession) - sessionManager.broadcastSessionListUpdate() + handlerErr = sessionManager.DenySession(sessionID) + if handlerErr == nil { + // Notify the denied session + if targetSession := sessionManager.GetSession(sessionID); targetSession != nil { + go func() { + writeJSONRPCEvent("sessionAccessDenied", map[string]interface{}{ + "message": "Access denied by primary session", + }, targetSession) + sessionManager.broadcastSessionListUpdate() + }() + } result = map[string]interface{}{"status": "denied"} - } else { - handlerErr = errors.New("session not found or not pending") } } else { handlerErr = errors.New("invalid sessionId parameter") @@ -251,24 +253,35 @@ func onRPCMessage(message webrtc.DataChannelMessage, session *Session) { } else if targetSession := sessionManager.GetSession(sessionID); targetSession != nil { // Users can update their own nickname, or admins can update any if targetSession.ID == session.ID || session.HasPermission(PermissionSessionManage) { - targetSession.Nickname = nickname - - // If session is pending and approval is required, send the approval request now that we have a nickname - if targetSession.Mode == SessionModePending && currentSessionSettings != nil && currentSessionSettings.RequireApproval { - if primary := sessionManager.GetPrimarySession(); primary != nil { - go func() { - writeJSONRPCEvent("newSessionPending", map[string]interface{}{ - "sessionId": targetSession.ID, - "source": targetSession.Source, - "identity": targetSession.Identity, - "nickname": targetSession.Nickname, - }, primary) - }() + // Check nickname uniqueness + allSessions := sessionManager.GetAllSessions() + for _, existingSession := range allSessions { + if existingSession.ID != sessionID && existingSession.Nickname == nickname { + handlerErr = fmt.Errorf("nickname '%s' is already in use by another session", nickname) + break } } - sessionManager.broadcastSessionListUpdate() - result = map[string]interface{}{"status": "updated"} + if handlerErr == nil { + targetSession.Nickname = nickname + + // If session is pending and approval is required, send the approval request now that we have a nickname + if targetSession.Mode == SessionModePending && currentSessionSettings != nil && currentSessionSettings.RequireApproval { + if primary := sessionManager.GetPrimarySession(); primary != nil { + go func() { + writeJSONRPCEvent("newSessionPending", map[string]interface{}{ + "sessionId": targetSession.ID, + "source": targetSession.Source, + "identity": targetSession.Identity, + "nickname": targetSession.Nickname, + }, primary) + }() + } + } + + sessionManager.broadcastSessionListUpdate() + result = map[string]interface{}{"status": "updated"} + } } else { handlerErr = errors.New("permission denied: can only update own nickname") } diff --git a/session_manager.go b/session_manager.go index d1cf6c38..f50bc32a 100644 --- a/session_manager.go +++ b/session_manager.go @@ -149,6 +149,15 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe sm.mu.Lock() defer sm.mu.Unlock() + // Check nickname uniqueness (only for non-empty nicknames) + if session.Nickname != "" { + for id, existingSession := range sm.sessions { + if id != session.ID && existingSession.Nickname == session.Nickname { + return fmt.Errorf("nickname '%s' is already in use by another session", session.Nickname) + } + } + } + wasWithinGracePeriod := false wasPreviouslyPrimary := false wasPreviouslyPending := false @@ -195,12 +204,14 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe // If this was the primary, try to restore primary status if existing.Mode == SessionModePrimary { isBlacklisted := sm.isSessionBlacklisted(session.ID) - if sm.lastPrimaryID == session.ID && !isBlacklisted { + // SECURITY: Prevent dual-primary window - only restore if no other primary exists + primaryExists := sm.primarySessionID != "" && sm.sessions[sm.primarySessionID] != nil + if sm.lastPrimaryID == session.ID && !isBlacklisted && !primaryExists { sm.primarySessionID = session.ID sm.lastPrimaryID = "" delete(sm.reconnectGrace, session.ID) } else { - // Grace period expired or another session took over + // Grace period expired, another session took over, or primary already exists session.Mode = SessionModeObserver } } @@ -781,6 +792,23 @@ func (sm *SessionManager) ApprovePrimaryRequest(currentPrimaryID, requesterID st return errors.New("not the primary session") } + // SECURITY: Verify requester session exists and is in Queued mode + requesterSession, exists := sm.sessions[requesterID] + if !exists { + sm.logger.Error(). + Str("requesterID", requesterID). + Msg("Requester session not found") + return errors.New("requester session not found") + } + + if requesterSession.Mode != SessionModeQueued { + sm.logger.Error(). + Str("requesterID", requesterID). + Str("actualMode", string(requesterSession.Mode)). + Msg("Requester session is not in queued mode") + return fmt.Errorf("requester session is not in queued mode (current mode: %s)", requesterSession.Mode) + } + // Remove requester from queue sm.removeFromQueue(requesterID) @@ -838,6 +866,51 @@ func (sm *SessionManager) DenyPrimaryRequest(currentPrimaryID, requesterID strin return nil } +// ApproveSession approves a pending session (thread-safe) +func (sm *SessionManager) ApproveSession(sessionID string) error { + sm.mu.Lock() + defer sm.mu.Unlock() + + session, exists := sm.sessions[sessionID] + if !exists { + return ErrSessionNotFound + } + + if session.Mode != SessionModePending { + return errors.New("session is not in pending mode") + } + + // Promote session to observer + session.Mode = SessionModeObserver + + sm.logger.Info(). + Str("sessionID", sessionID). + Msg("Session approved and promoted to observer") + + return nil +} + +// DenySession denies a pending session (thread-safe) +func (sm *SessionManager) DenySession(sessionID string) error { + sm.mu.Lock() + defer sm.mu.Unlock() + + session, exists := sm.sessions[sessionID] + if !exists { + return ErrSessionNotFound + } + + if session.Mode != SessionModePending { + return errors.New("session is not in pending mode") + } + + sm.logger.Info(). + Str("sessionID", sessionID). + Msg("Session denied - notifying session") + + return nil +} + // ForEachSession executes a function for each active session func (sm *SessionManager) ForEachSession(fn func(*Session)) { sm.mu.RLock()