feat(websocket): realtime transport backend — entity-change nudges + channel tokens - #43431
Conversation
Backend foundation for the realtime websocket transport (step 6): on each task
completion, TaskManager.publish_entity_change emits a best-effort pub/sub nudge
to a public, per-entity-type channel (entity-changes:task) carrying only
non-sensitive fields ({entity_type, id, status}) — no payload. It's published
once to the type channel (not fanned per subscriber), so a widely-shared entity
is one message. A browser transport forwards it; clients re-fetch any
authz-sensitive data through the authorized REST API (status_changes / chart
re-request) where TaskFilter/RLS apply. Strictly best-effort — never disrupts
completion signalling; the interval poll is the backstop.
Per review: the public per-entity-type pub/sub must disclose no sensitive info
and stay general across all entity types (dashboards, charts, datasets, RLS,
alerts) — status is task-specific and does not generalize. Drop status from the
nudge; it now carries only {entity_type, id}. Per-type channels stay
(entity-changes:task, later :dashboard/:chart/...) so a list view subscribes
only to its type. Status/sensitive detail moves to the per-principal tier or the
authorized re-fetch.
…ion auth) Reinstate websocket connection auth as a feature-agnostic service (replacing the GAQ async-token handshake removed in step 4). superset/websocket/channel.py derives a deterministic per-principal channel (user:<id> / guest:<hmac>, reusing the guest identity from superset.tasks.guest) and mints a JWT cookie the superset-websocket server verifies to bind a socket to its channel. Deterministic per-principal channels (vs the legacy per-session-random) let the task layer publish a user's events to their channel. New WEBSOCKET_* config (ENABLED, URL, JWT_SECRET/cookie), gated cookie registration in init, exposed to bootstrap conf.
|
Bito Automatic Review Skipped - Branch Excluded |
|
/review |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The tests/unit_tests/websocket/__init__.py was created empty (touch); RAT requires the Apache header.
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
|
|
||
| CHANGE_ME_SECRET_KEY = "CHANGE_ME_TO_A_COMPLEX_RANDOM_SECRET" # noqa: S105 | ||
| CHANGE_ME_GUEST_TOKEN_JWT_SECRET = "test-guest-secret-change-me" # noqa: S105 | ||
| CHANGE_ME_WEBSOCKET_JWT_SECRET = "test-ws-secret-change-me" # noqa: S105 |
There was a problem hiding this comment.
Suggestion: This introduces a known, fixed signing key for websocket authentication. Any deployment that enables the websocket transport without explicitly overriding WEBSOCKET_JWT_SECRET allows anyone who knows this public default to forge a JWT for arbitrary user:<id> channels and receive sensitive per-principal events. Do not use a known fallback secret for an authentication token; fail startup when a strong deployment-specific key is not configured. [security]
Severity Level: Critical 🚨
- ❌ Forged sockets can subscribe to arbitrary user channels.
- ❌ Sensitive per-principal chart events may be exposed.
- ⚠️ Production configuration permits startup with the known fallback.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/constants.py
**Line:** 32:32
**Comment:**
*Security: This introduces a known, fixed signing key for websocket authentication. Any deployment that enables the websocket transport without explicitly overriding `WEBSOCKET_JWT_SECRET` allows anyone who knows this public default to forge a JWT for arbitrary `user:<id>` channels and receive sensitive per-principal events. Do not use a known fallback secret for an authentication token; fail startup when a strong deployment-specific key is not configured.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Addressed in 100e208 via a startup guard, check_websocket_secret() (initialization/init.py, called in the check sequence): when WEBSOCKET_ENABLED is on and WEBSOCKET_JWT_SECRET is the placeholder or shorter than 32 bytes, it warns and sys.exit(1) outside debug/testing — so an enabled production deployment cannot run with the known default. This mirrors the established check_guest_token_secret / check_secret_key convention (a CHANGE_ME_* placeholder plus a startup check), rather than introducing a different pattern. Covered by check_websocket_secret_test.py.
| if request.cookies.get(cookie_name): | ||
| return response |
There was a problem hiding this comment.
Suggestion: The cookie is accepted solely because a value with this name exists; its JWT is never checked against the current principal or expiration. After logout/login, the browser can keep the previous user's token, so the websocket server binds the new session to a stale channel and may expose the previous user's per-principal events to the newly logged-in user. Decode and validate the existing token's channel against channel_id, or replace/delete it whenever the principal changes. [security]
Severity Level: Critical 🚨
- ❌ Cross-user websocket sessions can receive private events.
- ❌ Dashboard chart-data completion details may leak.
- ⚠️ Logout/login does not rotate the channel cookie.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/websocket/channel.py
**Line:** 92:93
**Comment:**
*Security: The cookie is accepted solely because a value with this name exists; its JWT is never checked against the current principal or expiration. After logout/login, the browser can keep the previous user's token, so the websocket server binds the new session to a stale channel and may expose the previous user's per-principal events to the newly logged-in user. Decode and validate the existing token's channel against `channel_id`, or replace/delete it whenever the principal changes.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Addressed in 100e208. register_ws_channel_cookie no longer trusts a cookie just because it exists: _cookie_channel() decodes/verifies the existing token and the cookie is re-minted whenever its channel claim ≠ the current principal (missing, invalid, expired, or a different user after logout→login), and delete_cookie is called when there is no principal (anonymous / just logged out — the after_request runs after the session is cleared, so get_channel_id() returns None). Covered by test_cookie_reminted_when_principal_changes and test_cookie_cleared_for_anonymous.
| # sockets. Set a strong random WEBSOCKET_JWT_SECRET (>= 32 bytes) in production. | ||
| WEBSOCKET_ENABLED = False | ||
| WEBSOCKET_URL = "ws://127.0.0.1:8080/" | ||
| WEBSOCKET_JWT_SECRET = CHANGE_ME_WEBSOCKET_JWT_SECRET |
There was a problem hiding this comment.
Suggestion: When WEBSOCKET_ENABLED is set to True, this default remains the publicly known CHANGE_ME_WEBSOCKET_JWT_SECRET, and the initialization path does not reject it. Attackers who know this value can forge channel JWTs and subscribe sockets to another principal's private channel. Add startup validation that refuses enabled deployments using the placeholder, or require an explicitly configured secret. [security]
Severity Level: Critical 🚨
- ❌ Forged JWTs can access another user's private channel.
- ❌ Dashboard chart-data events can become cross-user readable.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 2949:2949
**Comment:**
*Security: When `WEBSOCKET_ENABLED` is set to `True`, this default remains the publicly known `CHANGE_ME_WEBSOCKET_JWT_SECRET`, and the initialization path does not reject it. Attackers who know this value can forge channel JWTs and subscribe sockets to another principal's private channel. Add startup validation that refuses enabled deployments using the placeholder, or require an explicitly configured secret.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Addressed in 100e208 via a startup guard, check_websocket_secret() (initialization/init.py, called in the check sequence): when WEBSOCKET_ENABLED is on and WEBSOCKET_JWT_SECRET is the placeholder or shorter than 32 bytes, it warns and sys.exit(1) outside debug/testing — so an enabled production deployment cannot run with the known default. This mirrors the established check_guest_token_secret / check_secret_key convention (a CHANGE_ME_* placeholder plus a startup check), rather than introducing a different pattern. Covered by check_websocket_secret_test.py.
| self.configure_ssh_manager() | ||
| self.configure_stats_manager() | ||
| self.configure_task_manager() | ||
| self.configure_websocket() |
There was a problem hiding this comment.
Suggestion: The websocket cookie hook is now enabled for every configured websocket deployment, but the hook retains an existing token without checking whether its channel still matches the current principal. After a user logs out and another user logs in in the same browser, the old bearer cookie remains valid for up to the token expiration and the new websocket connection can continue using the previous user's channel, causing missed events or exposure of that user's private events. Clear or re-mint the cookie when the authenticated principal changes, and invalidate it on logout. [security]
Severity Level: Critical 🚨
- ❌ User switching can expose prior user websocket events.
- ❌ Dashboard chart completion details may reach the wrong principal.
- ⚠️ Impact requires websocket transport deployment and logout/login reuse.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/initialization/__init__.py
**Line:** 1049:1049
**Comment:**
*Security: The websocket cookie hook is now enabled for every configured websocket deployment, but the hook retains an existing token without checking whether its channel still matches the current principal. After a user logs out and another user logs in in the same browser, the old bearer cookie remains valid for up to the token expiration and the new websocket connection can continue using the previous user's channel, causing missed events or exposure of that user's private events. Clear or re-mint the cookie when the authenticated principal changes, and invalidate it on logout.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Addressed in 100e208. register_ws_channel_cookie no longer trusts a cookie just because it exists: _cookie_channel() decodes/verifies the existing token and the cookie is re-minted whenever its channel claim ≠ the current principal (missing, invalid, expired, or a different user after logout→login), and delete_cookie is called when there is no principal (anonymous / just logged out — the after_request runs after the session is cleared, so get_channel_id() returns None). Covered by test_cookie_reminted_when_principal_changes and test_cookie_cleared_for_anonymous.
…ak secret Address two Critical review findings on the channel-token service: - The cookie hook only minted when absent, so after a logout/login in the same browser the previous user's cookie (channel user:<oldid>) persisted and could bind the new session to the prior user's channel. Now re-mint whenever the cookie's channel claim doesn't match the current principal (missing, invalid/ expired, or a different user), and delete it when there is no principal (anonymous / logged out). - WEBSOCKET_ENABLED with the default/placeholder or <32-byte WEBSOCKET_JWT_SECRET let an attacker forge channel tokens. Add check_websocket_secret startup guard (mirrors check_guest_token_secret): warn always, refuse to start outside debug/testing. Tests for cookie rotation/clear + the secret guard.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## gaq-to-gtf #43431 +/- ##
===========================================
Coverage 78.83% 78.83%
===========================================
Files 2879 2880 +1
Lines 164342 164407 +65
Branches 37985 37994 +9
===========================================
+ Hits 129561 129617 +56
- Misses 32341 32347 +6
- Partials 2440 2443 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
SUMMARY
Step 6 of the GAQ→GTF epic (#43407): realtime websocket transport — backend foundation. Step 4 removed the GAQ firehose +
async-tokenJWT that the oldsuperset-websocketserver relied on (async chart-data went polling-only). This PR lays the backend foundation for a generalized, GTF-native realtime push transport. The Node server rewrite and the frontend socket client follow in subsequent slices (see Remaining below).The transport has two tiers, each with the authorization that fits it:
{entity_type, id}, no status or payload — to a per-type channel (entity-changes:task, later:dashboard,:chart,:dataset, …). A list view subscribes only to its type and re-fetches the actual (authz-scoped) data through the authorized REST API on a nudge. The contract is general across all entity types; id existence is intentionally public, so this channel never carries authz-sensitive data.user:<id>/guest:<hmac>); the server delivers a channel's events only to that principal's sockets — the same isolation master's async-events transport had, so this path can carry sensitive completion detail. (The publish side + delivery are wired in the Node/frontend slices.)What's in this PR (backend foundation)
TaskManager.publish_entity_change(task_uuid)— best-effort pub/sub of the opaque nudge toentity-changes:taskon task completion (emitted alongside the existing guaranteed completion signal; strictly best-effort, never disrupts it — the interval poll remains the backstop).superset/websocket/channel.py— a feature-agnostic channel-token service (replacing the GAQasync-tokenhandshake removed in step 4):get_channel_id()(per-principal, reusing the guest identity fromsuperset.tasks.guest),mint_channel_token(), andregister_ws_channel_cookie()(anafter_requesthttponlyJWT cookie the websocket server verifies). Deterministic per-principal channels (vs the legacy per-session-random) let the task layer publish a user's events to their channel.WEBSOCKET_*config (WEBSOCKET_ENABLED,WEBSOCKET_URL,WEBSOCKET_JWT_SECRET,WEBSOCKET_JWT_COOKIE_NAME/_SECURE/_SAMESITE/_DOMAIN/_EXPIRATION_SECONDS), a gatedconfigure_websocket()in initialization, andWEBSOCKET_ENABLED/WEBSOCKET_URLexposed to the frontend bootstrap.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend only; no user-visible change until the Node/frontend slices land.
TESTING INSTRUCTIONS
Automated:
pytest tests/unit_tests/tasks/test_manager.py tests/unit_tests/websocket/. WithDISTRIBUTED_COORDINATION_CONFIGset, completing a GTF task publishes{entity_type: "task", id}onentity-changes:task(best-effort). WithWEBSOCKET_ENABLED=True+ aWEBSOCKET_JWT_SECRET, authenticated responses carry thesuperset-ws-tokencookie whosechannelclaim isuser:<id>(orguest:<hmac>).ADDITIONAL INFORMATION
WEBSOCKET_*config)Remaining step-6 slices (this PR or immediate follow-ups): per-principal chart-data publish (task completion → submitter's channel); rewrite
superset-websocket/src/index.ts(subscribe the publicentity-changes:*broadcast + route per-principal channel events by the JWTchannelclaim; drop theasync-events-full/result_urlfirehose model); frontend socket client inasyncEvent.ts(accelerate chart-data completion + trigger list re-fetches; interval poll stays the fallback); config/docker/docs. Shipping the websocket server on the official image is step 8 (#43407 tracker).Targets the
gaq-to-gtffeature branch (part of #43407), notmaster.