fix(sse): cancel active streams on user logout#6033
fix(sse): cancel active streams on user logout#6033clubanderson wants to merge 1 commit intofix/6022-6024-6026-securityfrom
Conversation
Fixes #6029. SSE streaming endpoints (streamClusters in sse.go) ran inside SetBodyStreamWriter callbacks that blocked for up to sseOverallDeadline (~30s), so when a user logged out any in-flight SSE streams continued to emit cluster_data events until the deadline fired or the client disconnected. Mirrors the exec session cancellation pattern added in #6031 for /ws/exec sessions (#6024): - Add a per-user SSE session registry in sse.go with registerSSESession / unregisterSSESession / CancelUserSSEStreams, guarded by a plain sync.Mutex. - In streamClusters, capture the authenticated user ID before the deferred SetBodyStreamWriter callback runs (the fiber.Ctx may be reused by then) and register the stream context's cancel func in the registry, with a deferred unregister for normal stream end. - In auth.Logout, after CancelUserExecSessions, call CancelUserSSEStreams(claims.UserID) so logout tears down any live SSE streams the user had open. - Add sse_test.go with the four lifecycle tests mirroring exec_test.go: CancelsRegisteredContexts, OtherUserUnaffected, UnregisterSSESession_RemovesEntry, and NoSessions (no panic on empty map). streamDemoSSE is left unchanged — demo streams are instant, they do not block, so they need no cancellation path. Signed-off-by: Andrew Anderson <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
Fixes #6029. Stacked on #6031 (fix/6022-6024-6026-security).
Problem
SSE streaming endpoints (
streamClustersinpkg/api/handlers/sse.go) run insideSetBodyStreamWritercallbacks that block until either the client disconnects orsseOverallDeadline(~30s) fires. When a user logged out, any in-flight SSE streams kept emittingcluster_dataevents until that deadline — the token was revoked but the already-established stream had no active cancellation path.Fix
Mirrors the exec-session cancellation pattern added in #6031 for
/ws/execsessions (#6024).sse.go:sseSessions map[uuid.UUID]map[int64]context.CancelFuncguarded by a plainsync.MutexregisterSSESession/unregisterSSESession/CancelUserSSEStreamsstreamClusterscaptures the authenticated user ID before entering the deferredSetBodyStreamWritercallback (thefiber.Ctxmay be reused by then, soc.Localsis not safe to read inside), then registers the stream context'scancelfunc in the registry with a deferred unregister for normal stream end. All 17 SSE handlers insse.goroute throughstreamClusters, so one registration site covers every streaming endpoint.auth.LogoutcallsCancelUserSSEStreams(claims.UserID)immediately after the existingCancelUserExecSessionscall, tearing down any live SSE streams for the user on logout.streamDemoSSEis intentionally left unchanged — demo streams are instant (no blocking, no goroutines), so they need no cancellation path.Tests
New
pkg/api/handlers/sse_test.gomirroringexec_test.go:TestCancelUserSSEStreams_CancelsRegisteredContexts— register two contexts, call cancel, verify bothDone()and the per-user map entry is clearedTestCancelUserSSEStreams_OtherUserUnaffected— register two users, cancel one, verify the other's context is still aliveTestUnregisterSSESession_RemovesEntry— verify per-session removal drops only the target entry and the per-user slot is removed when emptyTestCancelUserSSEStreams_NoSessions— must not panic on empty mapAll pass:
Verification
go build ./...— cleango test ./pkg/api/handlers/... -count=1— passesgo vet ./...— cleancd web && npm run build— clean (post-build safety checks pass)Code quality
sync.Mutex(not RWMutex) — matches the exec-session patternuuid.UUIDfor user ID throughout — matches the rest of the codebase