Skip to content

Commit f9ebd6a

Browse files
committed
feat: add strict observer-to-primary promotion controls and immediate logout promotion
Observer-to-primary promotion protections: - Block auto-promotion during active primary grace periods - Prevent creating multiple primary sessions simultaneously - Validate transfer source is actual current primary - Check for duplicate primaries before promotion Immediate promotion on logout: - Trigger validateSinglePrimary() immediately when primary disconnects - Smart grace period bypass: allow promotion within 2 seconds of disconnect - Provides instant promotion on logout while protecting against network blips Enhanced validation and logging: - Log session additions/removals with counts - Display session IDs in validation logs for debugging - Track grace period timing for smart bypass decisions
1 parent ffc4a2a commit f9ebd6a

File tree

1 file changed

+129
-11
lines changed

1 file changed

+129
-11
lines changed

session_manager.go

Lines changed: 129 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,20 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe
255255
// and assign primary status atomically to prevent race conditions
256256
primaryExists := sm.primarySessionID != "" && sm.sessions[sm.primarySessionID] != nil
257257

258+
// Check if there's an active grace period for a primary session (different from this session)
259+
hasActivePrimaryGracePeriod := false
260+
if sm.lastPrimaryID != "" && sm.lastPrimaryID != session.ID {
261+
if graceTime, exists := sm.reconnectGrace[sm.lastPrimaryID]; exists {
262+
if time.Now().Before(graceTime) {
263+
if reconnectInfo, hasInfo := sm.reconnectInfo[sm.lastPrimaryID]; hasInfo {
264+
if reconnectInfo.Mode == SessionModePrimary {
265+
hasActivePrimaryGracePeriod = true
266+
}
267+
}
268+
}
269+
}
270+
}
271+
258272
// Check if this session was recently demoted via transfer
259273
isBlacklisted := sm.isSessionBlacklisted(session.ID)
260274

@@ -263,6 +277,7 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe
263277
Str("nickname", session.Nickname).
264278
Str("currentPrimarySessionID", sm.primarySessionID).
265279
Bool("primaryExists", primaryExists).
280+
Bool("hasActivePrimaryGracePeriod", hasActivePrimaryGracePeriod).
266281
Int("totalSessions", len(sm.sessions)).
267282
Bool("wasWithinGracePeriod", wasWithinGracePeriod).
268283
Bool("wasPreviouslyPrimary", wasPreviouslyPrimary).
@@ -271,10 +286,10 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe
271286
Msg("AddSession state analysis")
272287

273288
// Become primary only if:
274-
// 1. Was previously primary (within grace) AND no current primary, OR
275-
// 2. There's no primary at all AND not recently transferred away
276-
// Never allow primary promotion if already restored within grace period
277-
shouldBecomePrimary := !wasWithinGracePeriod && ((wasPreviouslyPrimary && !primaryExists) || (!primaryExists && !isBlacklisted))
289+
// 1. Was previously primary (within grace) AND no current primary AND no other session has grace period, OR
290+
// 2. There's no primary at all AND not recently transferred away AND no active grace period
291+
// Never allow primary promotion if already restored within grace period or another session has grace period
292+
shouldBecomePrimary := !wasWithinGracePeriod && !hasActivePrimaryGracePeriod && ((wasPreviouslyPrimary && !primaryExists) || (!primaryExists && !isBlacklisted))
278293

279294
if wasWithinGracePeriod {
280295
sm.logger.Debug().
@@ -354,6 +369,12 @@ func (sm *SessionManager) AddSession(session *Session, clientSettings *SessionSe
354369
// This ensures that primary existence checks work correctly during restoration
355370
sm.sessions[session.ID] = session
356371

372+
sm.logger.Info().
373+
Str("sessionID", session.ID).
374+
Str("mode", string(session.Mode)).
375+
Int("totalSessions", len(sm.sessions)).
376+
Msg("Session added to manager")
377+
357378
// Ensure session has auto-generated nickname if needed
358379
sm.ensureNickname(session)
359380

@@ -373,12 +394,21 @@ func (sm *SessionManager) RemoveSession(sessionID string) {
373394

374395
session, exists := sm.sessions[sessionID]
375396
if !exists {
397+
sm.logger.Debug().
398+
Str("sessionID", sessionID).
399+
Msg("RemoveSession called but session not found in map")
376400
return
377401
}
378402

379403
wasPrimary := session.Mode == SessionModePrimary
380404
delete(sm.sessions, sessionID)
381405

406+
sm.logger.Info().
407+
Str("sessionID", sessionID).
408+
Bool("wasPrimary", wasPrimary).
409+
Int("remainingSessions", len(sm.sessions)).
410+
Msg("Session removed from manager")
411+
382412
// Remove from queue if present
383413
sm.removeFromQueue(sessionID)
384414

@@ -428,10 +458,18 @@ func (sm *SessionManager) RemoveSession(sessionID string) {
428458
sm.logger.Info().
429459
Str("sessionID", sessionID).
430460
Dur("gracePeriod", time.Duration(gracePeriod)*time.Second).
431-
Msg("Primary session removed, grace period active - auto-promotion will occur after grace expires")
432-
433-
// NOTE: Do NOT call validateSinglePrimary() here - let grace period expire naturally
434-
// The cleanupInactiveSessions() function will handle promotion after grace period expires
461+
Int("remainingSessions", len(sm.sessions)).
462+
Msg("Primary session removed, grace period active")
463+
464+
// Immediate promotion check: if there are observers waiting, trigger validation
465+
// This allows immediate promotion while still respecting grace period protection
466+
if len(sm.sessions) > 0 {
467+
sm.logger.Debug().
468+
Str("removedPrimaryID", sessionID).
469+
Int("remainingSessions", len(sm.sessions)).
470+
Msg("Triggering immediate validation for potential promotion")
471+
sm.validateSinglePrimary()
472+
}
435473
}
436474

437475
// Notify remaining sessions
@@ -704,6 +742,20 @@ func (sm *SessionManager) TransferPrimary(fromID, toID string) error {
704742
sm.mu.Lock()
705743
defer sm.mu.Unlock()
706744

745+
// SECURITY: Verify fromID is the actual current primary
746+
if sm.primarySessionID != fromID {
747+
return fmt.Errorf("transfer denied: %s is not the current primary (current primary: %s)", fromID, sm.primarySessionID)
748+
}
749+
750+
fromSession, exists := sm.sessions[fromID]
751+
if !exists {
752+
return ErrSessionNotFound
753+
}
754+
755+
if fromSession.Mode != SessionModePrimary {
756+
return errors.New("transfer denied: from session is not in primary mode")
757+
}
758+
707759
// Use centralized transfer method
708760
err := sm.transferPrimaryRole(fromID, toID, "direct_transfer", "manual transfer request")
709761
if err != nil {
@@ -899,20 +951,64 @@ func (sm *SessionManager) validateSinglePrimary() {
899951
sm.primarySessionID = ""
900952
}
901953

954+
// Check if there's an active grace period for any primary session
955+
// BUT: if grace period just started (within 2 seconds), allow immediate promotion
956+
hasActivePrimaryGracePeriod := false
957+
for sessionID, graceTime := range sm.reconnectGrace {
958+
if time.Now().Before(graceTime) {
959+
if reconnectInfo, hasInfo := sm.reconnectInfo[sessionID]; hasInfo {
960+
if reconnectInfo.Mode == SessionModePrimary {
961+
// Calculate how long ago the grace period started
962+
gracePeriod := 10
963+
if currentSessionSettings != nil && currentSessionSettings.ReconnectGrace > 0 {
964+
gracePeriod = currentSessionSettings.ReconnectGrace
965+
}
966+
graceStartTime := graceTime.Add(-time.Duration(gracePeriod) * time.Second)
967+
timeSinceGraceStart := time.Since(graceStartTime)
968+
969+
// If grace period just started (within 2 seconds), allow immediate promotion
970+
// This enables instant promotion on logout while still protecting against network blips
971+
if timeSinceGraceStart > 2*time.Second {
972+
hasActivePrimaryGracePeriod = true
973+
sm.logger.Debug().
974+
Str("gracePrimaryID", sessionID).
975+
Dur("remainingGrace", time.Until(graceTime)).
976+
Dur("timeSinceGraceStart", timeSinceGraceStart).
977+
Msg("Active grace period detected for primary session - blocking auto-promotion")
978+
} else {
979+
sm.logger.Debug().
980+
Str("gracePrimaryID", sessionID).
981+
Dur("timeSinceGraceStart", timeSinceGraceStart).
982+
Msg("Grace period just started - allowing immediate promotion")
983+
}
984+
break
985+
}
986+
}
987+
}
988+
}
989+
990+
// Build session IDs list for debugging
991+
sessionIDs := make([]string, 0, len(sm.sessions))
992+
for id := range sm.sessions {
993+
sessionIDs = append(sessionIDs, id)
994+
}
995+
902996
sm.logger.Debug().
903997
Int("primarySessionCount", len(primarySessions)).
904998
Str("primarySessionID", sm.primarySessionID).
905999
Int("totalSessions", len(sm.sessions)).
1000+
Strs("sessionIDs", sessionIDs).
1001+
Bool("hasActivePrimaryGracePeriod", hasActivePrimaryGracePeriod).
9061002
Msg("validateSinglePrimary state check")
9071003

908-
// Auto-promote if there are NO primary sessions at all
909-
if len(primarySessions) == 0 && sm.primarySessionID == "" && len(sm.sessions) > 0 {
1004+
// Auto-promote if there are NO primary sessions at all AND no active grace period
1005+
if len(primarySessions) == 0 && sm.primarySessionID == "" && len(sm.sessions) > 0 && !hasActivePrimaryGracePeriod {
9101006
// Find a session to promote to primary
9111007
nextSessionID := sm.findNextSessionToPromote()
9121008
if nextSessionID != "" {
9131009
sm.logger.Info().
9141010
Str("promotedSessionID", nextSessionID).
915-
Msg("Auto-promoting observer to primary - no primary sessions exist")
1011+
Msg("Auto-promoting observer to primary - no primary sessions exist and no grace period active")
9161012

9171013
// Use the centralized promotion logic
9181014
err := sm.transferPrimaryRole("", nextSessionID, "emergency_auto_promotion", "no primary sessions detected")
@@ -931,6 +1027,7 @@ func (sm *SessionManager) validateSinglePrimary() {
9311027
Int("primarySessions", len(primarySessions)).
9321028
Str("primarySessionID", sm.primarySessionID).
9331029
Bool("hasSessions", len(sm.sessions) > 0).
1030+
Bool("hasActivePrimaryGracePeriod", hasActivePrimaryGracePeriod).
9341031
Msg("Emergency auto-promotion conditions not met")
9351032
}
9361033
}
@@ -944,6 +1041,15 @@ func (sm *SessionManager) transferPrimaryRole(fromSessionID, toSessionID, transf
9441041
return ErrSessionNotFound
9451042
}
9461043

1044+
// SECURITY: Prevent promoting a session that's already primary
1045+
if toSession.Mode == SessionModePrimary {
1046+
sm.logger.Warn().
1047+
Str("sessionID", toSessionID).
1048+
Str("transferType", transferType).
1049+
Msg("Attempted to promote session that is already primary")
1050+
return errors.New("target session is already primary")
1051+
}
1052+
9471053
var fromSession *Session
9481054
var fromExists bool
9491055
if fromSessionID != "" {
@@ -967,6 +1073,18 @@ func (sm *SessionManager) transferPrimaryRole(fromSessionID, toSessionID, transf
9671073
Msg("Demoted existing primary session")
9681074
}
9691075

1076+
// SECURITY: Before promoting, verify there are no other primary sessions
1077+
for id, sess := range sm.sessions {
1078+
if id != toSessionID && sess.Mode == SessionModePrimary {
1079+
sm.logger.Error().
1080+
Str("existingPrimaryID", id).
1081+
Str("targetPromotionID", toSessionID).
1082+
Str("transferType", transferType).
1083+
Msg("CRITICAL: Attempted to create second primary - blocking promotion")
1084+
return fmt.Errorf("cannot promote: another primary session exists (%s)", id)
1085+
}
1086+
}
1087+
9701088
// Promote target session
9711089
toSession.Mode = SessionModePrimary
9721090
toSession.hidRPCAvailable = false // Force re-handshake

0 commit comments

Comments
 (0)