Skip to content

fix: scope job-status WebSocket events to the job owner - #475

Open
NaitikVerma6776 wants to merge 1 commit into
CCExtractor:mainfrom
NaitikVerma6776:fix/473-websocket-job-status-scoping
Open

NaitikVerma6776 wants to merge 1 commit into
CCExtractor:mainfrom
NaitikVerma6776:fix/473-websocket-job-status-scoping

Conversation

@NaitikVerma6776

Copy link
Copy Markdown
Contributor

Summary

  • Bind authenticated WebSocket connections to the session user UUID (ignore spoofable clientID query param).
  • Attach ClientID to queued jobs and job-status events; deliver only to matching sockets.
  • Add regression tests for two-user fan-out, session-over-query binding, and ClientID on broadcast.

Fixes #473

Test plan

  • go test ./controllers/ -run Test_deliverJobStatus_|Test_AuthenticatedWebSocketHandler_|Test_AddJob_IncludesClientID|Test_AddTaskHandler_WithDueDate|Test_AddTaskHandler_WithoutDueDate|Test_EditTaskHandler_WithDependencies
  • Two browsers / users on /home: User A adds a task; User B must not receive WS frames or success toasts
  • User A still gets queued / in-progress / success and refetch/toast as before
  • Confirm WS upgrade still requires a valid session

Broadcasting every job status to all authenticated sockets caused other
users to toast and refetch. Bind connections to the session UUID and
only deliver events for matching ClientID.

Fixes CCExtractor#473
@github-actions

Copy link
Copy Markdown

Thank you for opening this PR!

Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools.

Please take a moment to:

  • Check the "Files changed" tab
  • Leave comments on any lines for functions, comments, etc. that are important, non-obvious, or may need attention
  • Clarify decisions you made or areas you might be unsure about and/or any future updates being considered.
  • Finally, submit all the comments!

More information on how to conduct a self review:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request

This helps make the review process smoother and gives us a clearer understanding of your thought process.

Once you've added your self-review, we'll continue from our side. Thank you!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Job-status WebSocket events appear to be broadcast to every connected client

1 participant