sadpandajoe commented on code in PR #45404:
URL: https://github.com/apache/superset/pull/45404#discussion_r4410071145
##########
superset-frontend/src/middleware/realtime.ts:
##########
@@ -267,16 +301,132 @@ const teardownSocket = (): void => {
}
};
+interface Ticket {
+ token: string;
+ expiresInMs: number;
+}
+
+/** Fetch a fresh connection ticket for the current session. */
+const fetchTicket = async (): Promise<Ticket> => {
+ const { json } = await SupersetClient.get({ endpoint: TICKET_ENDPOINT });
Review Comment:
For an anonymous visitor on a public dashboard (`PUBLIC_ROLE_LIKE` set with
`WEBSOCKET_ENABLE` on), the bootstrap still enables realtime, so this ticket
request returns 401. Since it doesn't pass `ignoreUnauthorized: true`,
`SupersetClient` runs its unauthorized handler and navigates to `/login` before
`isUnauthorizedError` here ever sees the rejection, so opening the dashboard
bounces the visitor to the login page instead of just leaving realtime off.
Should this request pass `ignoreUnauthorized: true`, or skip the ticket fetch
when there is no signed-in user?
##########
superset-websocket/src/index.ts:
##########
@@ -715,72 +738,243 @@ export const readSocketIdentity = (
};
};
+/** An inbound `auth` frame: `{type: 'auth', token, tab_id?}`. */
+interface AuthFrame {
+ token: string;
+ tabId?: string;
+}
+
/**
- * WebSocket `connection` event handler. `identity` is the one already verified
- * during the HTTP upgrade (see `httpUpgrade`), so each connection's JWT is
- * verified exactly once.
+ * Parse a raw inbound frame into an `auth` frame, or `null` when it is
anything
+ * else (malformed JSON, another message type, or a missing token). An invalid
or
+ * absent `tab_id` is dropped rather than failing the frame, so the socket
falls
+ * back to principal-grain delivery.
*/
-export const wsConnection = (
- ws: WebSocket,
- identity: SocketIdentity,
- tabId?: string,
-) => {
- const { channel } = identity;
-
- // Refuse the connection if a configured connection limit has been reached,
- // before tracking it against the internal registries. The cap is keyed on
the
- // principal channel (which counts all of a principal's tabs); the per-tab
- // channel is a delivery refinement and is not separately limit-checked.
- const limitReason = connectionLimitReason(channel);
- if (limitReason) {
- statsd.increment('ws_connection_rejected');
- logger.warn(`Refusing connection on channel ${channel}: ${limitReason}`);
- ws.close(CONNECTION_LIMIT_CLOSE_CODE, limitReason);
- return;
+const parseAuthFrame = (raw: WebSocket.RawData): AuthFrame | null => {
+ let parsed: unknown;
+ try {
+ parsed = JSON.parse(raw.toString());
+ } catch {
+ return null;
}
+ if (!parsed || typeof parsed !== 'object') return null;
+ const {
+ type,
+ token,
+ tab_id: tabId,
+ } = parsed as {
+ type?: unknown;
+ token?: unknown;
+ tab_id?: unknown;
+ };
+ if (type !== 'auth' || typeof token !== 'string' || token.length === 0) {
+ return null;
+ }
+ return {
+ token,
+ tabId: typeof tabId === 'string' && isValidTabId(tabId) ? tabId :
undefined,
+ };
+};
- // When the browser advertised a tab id at connect, also register the socket
- // under a per-tab channel derived from its authorized principal channel
- // (`<channel>:<tabId>`). The tab id is client-supplied but always prefixed
by
- // the authorized principal channel, so it can never address another
- // principal's sockets. Task-status can then target one tab, while the
- // principal channel still reaches all of a principal's tabs.
- const tabChannel = tabId ? `${channel}:${tabId}` : undefined;
-
- const socketInstance: SocketInstance = {
- ws,
- channel,
- tabChannel,
- identity,
- pongTs: Date.now(),
+const sendAuthOk = (ws: WebSocket, identity: SocketIdentity): void => {
+ const message: OutboundMessage = {
+ topic: AUTH_REPLY_TOPIC,
+ payload: {
+ status: 'ok',
+ expires_in: Math.max(
+ 0,
+ Math.floor((identity.tokenExpiresAtMs - Date.now()) / 1000),
+ ),
+ },
};
+ ws.send(JSON.stringify(message));
Review Comment:
This `ws.send` skips the `maxSocketBufferBytes` check that `sendToSocket`
applies. An authenticated client can replay its still-valid `auth` frame over
and over without reading replies, and every one queues another ack in the
socket's outbound buffer with no cap or termination when no Pub/Sub traffic is
flowing. Should the ack go through the bounded send path (or close the socket
when `bufferedAmount` is over the limit) like other outbound messages?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
