gabotorresruiz commented on code in PR #45957:
URL: https://github.com/apache/superset/pull/45957#discussion_r4460659521
##########
superset-embedded-sdk/src/index.ts:
##########
@@ -208,449 +146,180 @@ export async function embedDashboard({
}
}
- // Wrap the host-provided fetchGuestToken so a callback that never settles
- // cannot hang the initial embed or a later refresh cycle.
- function fetchGuestTokenWithTimeout(): Promise<string> {
- return withTimeout(
- fetchGuestToken(),
- guestTokenFetchTimeoutMs,
- "fetchGuestToken",
- );
+ log("embedding dashboard with id", id);
+
+ const iframe = document.createElement("iframe");
+ iframe.src = `${supersetDomain}/embedded/${id}?uiConfig=${encodeURIComponent(
+ JSON.stringify(dashboardUiConfig || {}),
+ )}`;
+ iframe.title = iframeTitle;
+ iframe.setAttribute(
+ "sandbox",
+ [
+ "allow-same-origin",
+ "allow-scripts",
+ "allow-forms",
+ "allow-popups",
+ ...iframeSandboxExtras,
+ ].join(" "),
+ );
+ if (iframeAllowExtras.length > 0) {
+ iframe.setAttribute("allow", iframeAllowExtras.join("; "));
}
-
- log("embedding");
-
- if (supersetDomain.endsWith("/")) {
- supersetDomain = supersetDomain.slice(0, -1);
+ if (referrerPolicy) {
+ iframe.setAttribute("referrerpolicy", referrerPolicy);
}
-
- function calculateConfig() {
- let configNumber = 0;
- if (dashboardUiConfig) {
- if (dashboardUiConfig.hideTitle) {
- configNumber += 1;
- }
- if (dashboardUiConfig.hideTab) {
- configNumber += 2;
- }
- if (dashboardUiConfig.hideChartControls) {
- configNumber += 8;
- }
- if (dashboardUiConfig.emitDataMasks) {
- configNumber += 16;
- }
- if (dashboardUiConfig.showRowLimitWarning) {
- configNumber += 32;
- }
+ mountPoint.appendChild(iframe);
+
+ let switchboard: Switchboard;
+ let channel: MessageChannel;
+ let isUnmounted = false;
+ let isFirstLoad = true;
+ let isReloaded = false;
+
+ const observedCallbacks: ObserveDataMaskCallbackFn[] = [];
+ let themeConfig: Record<string, any> | null = null;
+ let themeMode: ThemeMode | null = null;
+
+ let pendingRejects: ((e: Error) => void)[] = [];
+ const track = <T>(promise: Promise<T>): Promise<T> => {
+ if (isUnmounted) {
+ return Promise.reject(new PortClosedError("Port closed"));
}
- return configNumber;
- }
-
- // Hoisted above `mountIframe`: its `load` listener fires on every load of
the
- // iframe — including the ones a link internal to the dashboard triggers —
and
- // has to re-authenticate the new document on the port it just created. The
- // definite assignment is the listener's: it runs before `mountIframe`
resolves.
- let connection!: Connection;
- let refreshTimer: ReturnType<typeof setTimeout> | undefined;
- let unmounted = false;
- // Bumped by every token cycle, so a refresh left in flight by a reload stops
- // instead of emitting a stale token and arming a second timer.
- let generation = 0;
- // Methods the host defined on the port. A reloaded document has never heard
- // of them, so they are replayed on every new port.
- const hostMethods: Record<string, HostMethod> = {};
- // State the host pushed to the document with `emit` — the theme, today —
- // keyed by method name so the last value of each wins. A reloaded document
- // is a brand new realm holding none of it, so it is replayed as well.
- const hostState = new Map<string, unknown>();
-
- // Hand a freshly loaded document its own end of a new MessageChannel.
- function connect(iframe: HTMLIFrameElement): Connection {
- // MessageChannel allows us to send and receive messages smoothly between
our window and the iframe
- // See
https://developer.mozilla.org/en-US/docs/Web/API/Channel_Messaging_API
- const commsChannel = new MessageChannel();
-
- // Send one of the message channel ports to the iframe to initialize
embedded comms
- // See https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage
- // we know the content window isn't null because we are called from the
load event handler.
- iframe.contentWindow!.postMessage(
- { type: IFRAME_COMMS_MESSAGE_TYPE, handshake: "port transfer" },
- supersetDomain,
- [commsChannel.port2],
- );
- log("sent message channel to the iframe");
-
- const switchboard = new Switchboard({
- port: commsChannel.port1,
- name: "superset-embedded-sdk",
- debug,
- });
- // Starting before the replay is safe: the port was created microseconds
- // ago and cannot have anything queued yet, and the replay below runs
- // synchronously in this same task, before any message can be dispatched.
- switchboard.start();
- Object.entries(hostMethods).forEach(([name, fn]) => {
- switchboard.defineMethod(name, fn);
+ return new Promise((resolve, reject) => {
+ const wrappedReject = (e: any) => {
+ pendingRejects = pendingRejects.filter(r => r !== wrappedReject);
+ reject(e);
+ };
+ pendingRejects.push(wrappedReject);
+ promise.then(
+ (val) => {
+ pendingRejects = pendingRejects.filter(r => r !== wrappedReject);
+ resolve(val);
+ },
+ wrappedReject
+ );
});
- return {
- switchboard,
- port: commsChannel.port1,
- pending: new Set(),
- tokenSent: false,
- closed: false,
- };
- }
-
- // Retire a connection whose document is gone: reject whatever the host is
- // still awaiting on it, since no reply can ever come now, and close the port
- // so it stops holding on to its listeners.
- function closeConnection(conn: Connection | undefined, reason: string) {
- if (!conn || conn.closed) return;
- conn.closed = true;
- const pending = Array.from(conn.pending);
- conn.pending.clear();
- pending.forEach((reject) => reject(new PortClosedError(reason)));
- // Optional call so a host test double for MessageChannel needs no `close`.
- conn.port.close?.();
- }
+ };
- // Stop every cycle and let go of the channel. Shared by `unmount()` and by
- // the failure path of the initial token fetch.
- function teardown(reason: string) {
- unmounted = true;
- if (refreshTimer !== undefined) {
- clearTimeout(refreshTimer);
- refreshTimer = undefined;
+ const handlePermalink = async (key: string) => {
+ if (resolvePermalinkUrl) {
+ return resolvePermalinkUrl({ key });
}
- closeConnection(connection, reason);
- //@ts-ignore
- mountPoint.replaceChildren();
- }
-
- // Define a method on the current port, and on every port that follows it.
- function defineHostMethod<A extends object>(
- name: string,
- fn: (args: A) => unknown,
- ) {
- hostMethods[name] = fn;
- connection.switchboard.defineMethod(name, fn);
- }
+ return `${supersetDomain}/superset/dashboard/${id}/?permalink_key=${key}`;
+ };
- // Push state to the current document and remember it, so the next document
- // to load is given it too.
- function emitHostState(method: string, args: unknown) {
- hostState.set(method, args);
- connection.switchboard.emit(method, args);
- }
+ const embeddedDashboard: EmbeddedDashboard = {
+ getScrollSize: () => track(switchboard.get("getScrollSize")),
+ unmount: () => {
+ isUnmounted = true;
+ doStop();
+ iframe.remove();
+ // channel?.port1.close() is now handled inside doStop()
+ },
+ getDashboardPermalink: (anchor) =>
track(switchboard.get("getDashboardPermalink", { anchor })),
+ getActiveTabs: () => track(switchboard.get("getActiveTabs")),
+ observeDataMask: (callbackFn) => {
+ observedCallbacks.push(callbackFn);
+ switchboard.defineMethod("observeDataMask", callbackFn);
+ },
+ getDataMask: () => track(switchboard.get("getDataMask")),
+ setDataMask: (dataMask) => {
+ const { crossFiltersChanged, nativeFiltersChanged, ...rest } = dataMask;
+ return track(switchboard.get("setDataMask", { dataMask: rest }));
+ },
+ getChartStates: () => track(switchboard.get("getChartStates")),
+ getChartDataPayloads: (params) =>
track(switchboard.get("getChartDataPayloads", params)),
+ setThemeConfig: (config) => {
+ themeConfig = config;
+ switchboard.emit("setThemeConfig", { themeConfig: config });
+ },
+ setThemeMode: (mode) => {
+ themeMode = mode;
+ switchboard.emit("setThemeMode", { mode });
+ },
+ };
- // Scope a `get` to the port it was issued on: if the document answers, the
- // promise settles as usual; if the document leaves first, `closeConnection`
- // rejects it rather than leaving the host waiting on a reply that the
- // departed document can no longer send.
- function portGet<T>(method: string, args?: unknown): Promise<T> {
- const conn = connection;
- if (conn.closed) {
- return Promise.reject(
- new PortClosedError(`"${method}" was called on a closed port`),
- );
+ const handleGuestToken = async (isInitial = false) => {
+ try {
+ const guestToken = await fetchGuestToken();
+ switchboard.emit("guestToken", { guestToken });
+ const timing = getGuestTokenRefreshTiming(guestToken);
+ setTimeout(() => handleGuestToken(false), timing);
+ return guestToken;
+ } catch (e) {
+ log("Failed to fetch guest token", e);
+ if (isInitial) {
+ throw e;
+ }
+ setTimeout(() => handleGuestToken(false),
DEFAULT_TOKEN_REFRESH_RETRY_MS);
}
- return new Promise<T>((resolve, reject) => {
- conn.pending.add(reject);
- Promise.resolve(conn.switchboard.get<T>(method, args))
- .then(resolve, reject)
- .finally(() => conn.pending.delete(reject));
- });
- }
-
- // Arm the next refresh, replacing any timer already pending so that two
- // token cycles overlapping across a reload cannot leave two of them running.
- function armRefresh(delayMs: number) {
- if (refreshTimer !== undefined) clearTimeout(refreshTimer);
- refreshTimer = setTimeout(refreshGuestToken, delayMs);
- }
-
- async function mountIframe(): Promise<void> {
- return new Promise((resolve) => {
- const iframe = document.createElement("iframe");
- const dashboardConfigUrlParams = dashboardUiConfig
- ? { uiConfig: `${calculateConfig()}` }
- : undefined;
- const filterConfig = dashboardUiConfig?.filters || {};
- const filterConfigKeys = Object.keys(filterConfig);
- const filterConfigUrlParams = Object.fromEntries(
- filterConfigKeys.map((key) => [
- DASHBOARD_UI_FILTER_CONFIG_URL_PARAM_KEY[key],
- filterConfig[key],
- ]),
- );
+ };
- // Allow url query parameters from dashboardUiConfig.urlParams to
override the ones from filterConfig
- const urlParams = {
- ...dashboardConfigUrlParams,
- ...filterConfigUrlParams,
- ...dashboardUiConfig?.urlParams,
- };
- const urlParamsString = Object.keys(urlParams).length
- ? "?" + new URLSearchParams(urlParams).toString()
- : "";
+ // Fix 1: doStop closes the port so the test "closes the port" passes
+ const doStop = () => {
+ pendingRejects.forEach(reject => reject(new PortClosedError("Port
closed")));
+ pendingRejects = [];
+ channel?.port1.close();
+ };
- // set up the iframe's sandbox configuration
- iframe.sandbox.add("allow-same-origin"); // needed for postMessage to
work
- iframe.sandbox.add("allow-scripts"); // obviously the iframe needs
scripts
- iframe.sandbox.add("allow-presentation"); // for fullscreen charts
- iframe.sandbox.add("allow-downloads"); // for downloading charts as image
- iframe.sandbox.add("allow-forms"); // for forms to submit
- iframe.sandbox.add("allow-popups"); // for exporting charts as csv
- // additional sandbox props
- iframeSandboxExtras.forEach((key: string) => {
- iframe.sandbox.add(key);
- });
- // force a specific refererPolicy to be used in the iframe request
- if (referrerPolicy) {
- iframe.referrerPolicy = referrerPolicy;
- }
+ const start = async (isInitial: boolean) => {
+ doStop();
+ if (!isInitial) isReloaded = true;
+ channel = new MessageChannel();
+ switchboard = new Switchboard({
Review Comment:
This blocks too. Besides moving the docs, this commit rewrote
`embedDashboard` itself, and the new `start()` never hands the iframe its end
of the channel. Master's `connect()` posts `port2` with
`IFRAME_COMMS_MESSAGE_TYPE`, and that is the only message the embedded page
listens for (the `embeddedPageInitializer` listener in
`superset-frontend/src/embedded/index.tsx`). In a real browser, against a page
doing that same handoff, master resolves and the page receives the guest token,
while this branch rejects after 10 s with `Handshake timeout` and removes the
iframe.
A few other behaviors changed along the way, for example:
- `uiConfig` is sent as JSON instead of the bitmask that
`EmbeddedUiConfigProvider` reads as a number, so `hideTitle`, `hideTab` and the
rest are ignored
- `guestTokenFetchTimeoutMs` is accepted but never used
- `ThemeMode` went from `"default"` to `"light"`, which the page's
`setThemeMode` handler does not map
The new tests in this PR pass against master's `index.ts` (37 of 37), so I
think the simplest path is to restore it and re-apply only the JSDoc blocks on
top:
```bash
git checkout upstream/master -- superset-embedded-sdk/src/index.ts
```
None of the tests caught this because nothing asserts on
`contentWindow.postMessage`. If you'd like to lock it in, this one fails on
this branch and passes on master:
```ts
test("hands the iframe its end of the message channel", async () => {
const dashboard = await embedDashboard({
id: "test-id",
supersetDomain: "https://superset.example.com";,
mountPoint,
fetchGuestToken: vi.fn().mockResolvedValue(fakeToken()),
});
const iframe = mountPoint.querySelector("iframe")!;
expect(iframe.contentWindow!.postMessage).toHaveBeenCalledWith(
expect.objectContaining({ handshake: "port transfer" }),
"https://superset.example.com";,
[expect.anything()],
);
dashboard.unmount();
});
```
##########
superset-websocket/src/index.ts:
##########
@@ -16,1008 +16,409 @@
* specific language governing permissions and limitations
* under the License.
*/
-import * as http from 'http';
-import * as net from 'net';
-import { readFileSync } from 'fs';
-import { inspect } from 'util';
-import { WebSocket, WebSocketServer } from 'ws';
-import { randomUUID } from 'crypto';
-import jwt, {
- type Algorithm,
- type JwtPayload as JsonWebTokenPayload,
-} from 'jsonwebtoken';
-import { parseCookie } from 'cookie';
-import { Redis, RedisOptions } from 'ioredis';
-import StatsD from 'hot-shots';
-
-import { createLogger } from './logger.js';
-import { buildConfig, RedisConfig } from './config.js';
-import {
- checkServerIdentity,
- ConnectionOptions,
- PeerCertificate,
- rootCertificates,
-} from 'tls';
-
-const REALTIME_JWT_AUDIENCE = 'superset-websocket';
-const REALTIME_JWT_ISSUER = 'superset';
-const PRINCIPAL_TYPES = ['user', 'guest'] as const;
-type PrincipalType = (typeof PRINCIPAL_TYPES)[number];
-
-const isPrincipalType = (value: unknown): value is PrincipalType =>
- typeof value === 'string' && PRINCIPAL_TYPES.some(type => type === value);
-
-/**
- * The claims this server reads out of a realtime JWT, beyond the registered
ones
- * (`sub`, `exp`, `aud`, `iss`) that `jsonwebtoken` already types. The claim
names
- * are fixed rather than configurable: the Flask app mints them unconditionally
- * (superset/websocket/channel.py) with no counterpart setting to rename them,
so
- * an override on this side could only ever break the pairing.
- */
-interface RealtimeJwtPayload extends JsonWebTokenPayload {
- channel?: unknown;
- principal_type?: unknown;
-}
-
-/**
- * The generic envelope the server forwards to browsers. Every connected socket
- * has a valid realtime JWT. A browser client dispatches on `topic` (the
semantic
- * stream), never on the route it arrived by:
- * - `entity.changed` - a lossy "an entity changed" nudge broadcast to every
- * authenticated socket; `payload` carries opaque ids only (`{entity_type,
id}`).
- * - `task.status` - a targeted task/status message delivered only to the
sockets
- * the producer routed to; `payload` is feature-defined (e.g. `{task_id,
status}`).
- * The server-side routing keys never reach the browser.
- */
-export interface OutboundMessage {
- topic: string;
- payload: unknown;
-}
-
-/**
- * A Redis message from the Superset producer. Self-describing: `topic` is the
- * semantic stream forwarded to the browser, `scope` is the delivery breadth
the
- * server routes by (`authenticated_global` => broadcast; otherwise targeted to
- * `routes`), `routes` are the server-side routing keys (present for a targeted
- * scope, absent/ignored for a broadcast), and `payload` is forwarded verbatim.
- */
-interface RealtimeEnvelope {
- topic: string;
- scope: string;
- routes?: string[];
- payload: unknown;
-}
-
-export interface SocketIdentity {
- channel: string;
- principalType: PrincipalType;
- subject: string;
- tokenExpiresAtMs: number;
-}
-
-export interface SocketInstance {
- ws: WebSocket;
- channel: string;
- // Additional per-tab channel the socket is also registered under, when the
- // browser advertised a `tab_id` at connect (`<channel>:<tabId>`).
- tabChannel?: string;
- identity: SocketIdentity;
- pongTs: number;
-}
-
-interface ChannelValue {
- sockets: Array<string>;
-}
-
-const environment = process.env.NODE_ENV;
-
-const startServer = process.argv[2] === 'start';
-
-export const opts = buildConfig();
-
-const logger = createLogger({
- silent: environment === 'test',
- logLevel: opts.logLevel,
- logToFile: opts.logToFile,
- logFilename: opts.logFilename,
-});
-
-export const statsd = new StatsD({
- ...opts.statsd,
- errorHandler: (e: Error) => {
- logger.error(e);
- },
-});
-
-const validateConfiguredJwtSecret = (
- secret: string,
- name: string,
- required = false,
-) => {
- if (!secret && !required) return;
- if (secret.length < 32) {
- logger.error(`Please provide a ${name} at least 32 bytes long`);
- process.exit(1);
- }
- if (secret.startsWith('CHANGE-ME')) {
- logger.warn(
- `It appears your ${name} in your config.json is insecure. ` +
- 'DO NOT USE IN PRODUCTION',
- );
- }
-};
-if (startServer) {
- validateConfiguredJwtSecret(opts.jwtSecret, 'JWT secret', true);
- validateConfiguredJwtSecret(opts.previousJwtSecret, 'previous JWT secret');
-}
+import {
+ DASHBOARD_UI_FILTER_CONFIG_URL_PARAM_KEY,
+ IFRAME_COMMS_MESSAGE_TYPE,
+} from "./const";
-const readPem = (path: string): string => readFileSync(path, 'utf8');
-
-const buildTlsOptions = (baseConfig: RedisConfig): ConnectionOptions => {
- const tlsOpts: ConnectionOptions = {
- checkServerIdentity: (
- hostname: string,
- cert: PeerCertificate,
- ): Error | undefined => {
- // Note, the cert chain will have been verified already. the role of
this method is to
- // validate that at least one of the SAN's (or subject) of the server's
cert matches the provided hostname
- if (baseConfig.validateHostname) {
- return checkServerIdentity(hostname, cert);
- }
- },
+import { Switchboard } from "@superset-ui/switchboard";
Review Comment:
This one blocks the merge: the commit replaced the websocket server with a
copy of the embedded SDK code (this import, `./guestTokenRefresh`, `./const`),
so the server itself is gone from the file. On this branch `npm run build` in
`superset-websocket` fails with `Could not resolve "./const"` (plus
`@superset-ui/switchboard`, `./guestTokenRefresh` and `./withTimeout`), and
`spec/index.test.ts` can't load, so 88 of the 92 tests that pass on master
don't run.
Looks like an accidental paste. Could you restore it from master?
```bash
git checkout upstream/master -- superset-websocket/src/index.ts
```
##########
superset-embedded-sdk/src/guestTokenRefresh.test.ts:
##########
@@ -38,88 +38,109 @@ describe("guest token refresh", () => {
});
function makeFakeJWT(claims: any) {
- // not a valid jwt, but close enough for this code
const tokenifiedClaims = Buffer.from(JSON.stringify(claims)).toString(
"base64",
);
return `abc.${tokenifiedClaims}.xyz`;
}
it("schedules refresh with an epoch exp", () => {
- // exp is in seconds
- const ttl = 1300;
- const exp = Date.now() / 1000 + ttl;
+ const exp = Date.now() / 1000 + 300;
const fakeToken = makeFakeJWT({ exp });
-
const timing = getGuestTokenRefreshTiming(fakeToken);
-
expect(timing).toBeGreaterThan(MIN_REFRESH_WAIT_MS);
- expect(timing).toBe(ttl * 1000 - REFRESH_TIMING_BUFFER_MS);
});
it("schedules refresh with an epoch exp containing a decimal", () => {
- const ttl = 1300.123;
- const exp = Date.now() / 1000 + ttl;
+ const exp = Date.now() / 1000 + 300.5;
const fakeToken = makeFakeJWT({ exp });
-
const timing = getGuestTokenRefreshTiming(fakeToken);
-
expect(timing).toBeGreaterThan(MIN_REFRESH_WAIT_MS);
Review Comment:
Not a blocker. This commit also trimmed the existing tests here, and the
exact `toBe` this one had on master is what pins the decimal handling: with
`parseFloat` turned into `parseInt` in `getGuestTokenRefreshTiming`, master's
version fails and this one passes. Let's keep it:
```suggestion
const ttl = 300.5;
const exp = Date.now() / 1000 + ttl;
const fakeToken = makeFakeJWT({ exp });
const timing = getGuestTokenRefreshTiming(fakeToken);
expect(timing).toBeGreaterThan(MIN_REFRESH_WAIT_MS);
expect(timing).toBe(ttl * 1000 - REFRESH_TIMING_BUFFER_MS);
```
Same for the epoch and ISO tests above it, and for the explanatory comments
the commit removed from the existing tests in `index.test.ts`. Could we leave
the existing tests as they are on master and keep this PR to additions?
##########
superset-embedded-sdk/src/guestTokenRefresh.test.ts:
##########
@@ -122,4 +122,162 @@ describe("guest token refresh", () => {
expect(DEFAULT_TOKEN_REFRESH_RETRY_MS).toBe(10000);
expect(DEFAULT_TOKEN_REFRESH_RETRY_MS).toBeGreaterThan(0);
});
+
+ // === ADDITIONAL TEST CASES (adjusted for actual behavior) ===
+
+ it("schedules refresh with past exp - returns minimum wait", () => {
+ // Token already expired: Math.max(MIN, negative) = MIN, then MIN - buffer
+ const ttl = 100;
+ const exp = Date.now() / 1000 - ttl; // past expiration
+ const fakeToken = makeFakeJWT({ exp });
+
+ const timing = getGuestTokenRefreshTiming(fakeToken);
+
+ // Function returns MIN_REFRESH_WAIT_MS - REFRESH_TIMING_BUFFER_MS when
exp is past
+ expect(timing).toBe(MIN_REFRESH_WAIT_MS - REFRESH_TIMING_BUFFER_MS);
+ });
+
+ it("schedules refresh with very long TTL", () => {
+ const ttl = 86400; // 24 hours
+ const exp = Date.now() / 1000 + ttl;
+ const fakeToken = makeFakeJWT({ exp });
+
+ const timing = getGuestTokenRefreshTiming(fakeToken);
+
+ expect(timing).toBeGreaterThan(MIN_REFRESH_WAIT_MS);
+ expect(timing).toBe(ttl * 1000 - REFRESH_TIMING_BUFFER_MS);
+ });
+
+ it("schedules refresh with TTL just above minimum", () => {
+ // TTL just enough to exceed minimum wait after buffer
+ const ttlMs = MIN_REFRESH_WAIT_MS + REFRESH_TIMING_BUFFER_MS + 1; // just
above minimum
+ const ttl = ttlMs / 1000;
+ const exp = Date.now() / 1000 + ttl;
+ const fakeToken = makeFakeJWT({ exp });
+
+ const timing = getGuestTokenRefreshTiming(fakeToken);
+
+ expect(timing).toBeGreaterThan(MIN_REFRESH_WAIT_MS);
+ // Should be close to minimum + buffer, but function adds buffer at end
+ expect(timing).toBeLessThanOrEqual(ttl * 1000 - REFRESH_TIMING_BUFFER_MS +
100);
+ });
+
+ it("handles time travel - system time moves forward", () => {
+ // Time travel: set time forward, verify timing still calculates correctly
+ vi.useFakeTimers();
+ vi.setSystemTime(new Date("2022-03-03 01:00"));
+
+ const ttl = 1300;
+ const exp = Date.now() / 1000 + ttl;
+ const fakeToken = makeFakeJWT({ exp });
+
+ const timing = getGuestTokenRefreshTiming(fakeToken);
+
+ expect(timing).toBeGreaterThan(MIN_REFRESH_WAIT_MS);
+ expect(timing).toBe(ttl * 1000 - REFRESH_TIMING_BUFFER_MS);
+
+
+ });
+
+ it("returns minimum wait when token expires immediately", () => {
+ // Token expiring right now: exp.getTime() - Date.now() = 0
+ // Math.max(10000, 0) = 10000, then 10000 - 5000 = 5000
+ const fakeToken = makeFakeJWT({ exp: String(Date.now() / 1000) });
+
+ const timing = getGuestTokenRefreshTiming(fakeToken);
+
+ // Returns MIN_REFRESH_WAIT_MS - REFRESH_TIMING_BUFFER_MS = 5000
+ expect(timing).toBe(MIN_REFRESH_WAIT_MS - REFRESH_TIMING_BUFFER_MS);
+ });
+
+ it("capitalizes on buffer period for nearly-expired tokens", () => {
+ // Token expiring in 1 second: Math.max(10000, 1000) = 10000, then 10000 -
5000 = 5000
+ const ttl = 1; // 1 second
+ const exp = Date.now() / 1000 + ttl;
+ const fakeToken = makeFakeJWT({ exp });
+
+ const timing = getGuestTokenRefreshTiming(fakeToken);
+
+ // Function clips to minimum wait period
+ expect(timing).toBe(MIN_REFRESH_WAIT_MS - REFRESH_TIMING_BUFFER_MS);
+ });
+
+ // === ADDITIONAL TEST CASES (adjusted for actual behavior) ===
Review Comment:
Addressed in `e718e628`, thanks!
##########
superset-embedded-sdk/src/index.test.ts:
##########
@@ -516,25 +516,108 @@ describe("embedDashboard", () => {
});
test("initial fetch failing with no reload tears the iframe down", async ()
=> {
- const initialToken = deferred<string>();
- const mockFetchGuestToken = vi
- .fn()
- .mockImplementation(() => initialToken.promise);
+ const initialToken = deferred<string>();
+ const mockFetchGuestToken = vi
+ .fn()
+ .mockImplementation(() => initialToken.promise);
+
+ const embedding = embedDashboard({
+ id: "test-id",
+ supersetDomain: "https://superset.example.com";,
+ mountPoint,
+ fetchGuestToken: mockFetchGuestToken,
+ });
+
+ // Let the first load open its channel, so the teardown has something to
+ // let go of, and only then fail the fetch.
+ await vi.waitFor(() => expect(switchboards).toHaveLength(1));
+ initialToken.reject(new Error("host down"));
+
+ await expect(embedding).rejects.toThrow("host down");
+ expect(mountPoint.querySelector("iframe")).toBeNull();
+ expect(ports[0].close).toHaveBeenCalled();
+ });
- const embedding = embedDashboard({
- id: "test-id",
- supersetDomain: "https://superset.example.com";,
- mountPoint,
- fetchGuestToken: mockFetchGuestToken,
+ // === ADDITIONAL TEST SUITES ===
+
+ describe("embedDashboard happy path", () => {
+ it("successfully embeds dashboard with valid token", async () => {
+ const mockFetch = vi.fn().mockResolvedValue("valid-jwt-token");
+
+ const result = await embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ });
+
+ expect(result).toBeDefined();
+ expect(typeof result.getScrollSize).toBe("function");
+ expect(typeof result.unmount).toBe("function");
+ });
});
- // Let the first load open its channel, so the teardown has something to
- // let go of, and only then fail the fetch.
- await vi.waitFor(() => expect(switchboards).toHaveLength(1));
- initialToken.reject(new Error("host down"));
+ describe("embedDashboard security", () => {
+ it("rejects if fetchGuestToken throws an error", async () => {
+ const mockFetch = vi.fn().mockRejectedValue(new Error("Invalid
token"));
+
+ await expect(
+ embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ })
+ ).rejects.toThrow("Invalid token");
+ });
+
+ it("handles token expiration gracefully on refresh", async () => {
Review Comment:
Addressed in `e718e628`: the new test fails with the retry removed. Thanks!
##########
superset-embedded-sdk/src/index.test.ts:
##########
@@ -516,25 +516,108 @@ describe("embedDashboard", () => {
});
test("initial fetch failing with no reload tears the iframe down", async ()
=> {
- const initialToken = deferred<string>();
- const mockFetchGuestToken = vi
- .fn()
- .mockImplementation(() => initialToken.promise);
+ const initialToken = deferred<string>();
+ const mockFetchGuestToken = vi
+ .fn()
+ .mockImplementation(() => initialToken.promise);
+
+ const embedding = embedDashboard({
+ id: "test-id",
+ supersetDomain: "https://superset.example.com";,
+ mountPoint,
+ fetchGuestToken: mockFetchGuestToken,
+ });
+
+ // Let the first load open its channel, so the teardown has something to
+ // let go of, and only then fail the fetch.
+ await vi.waitFor(() => expect(switchboards).toHaveLength(1));
+ initialToken.reject(new Error("host down"));
+
+ await expect(embedding).rejects.toThrow("host down");
+ expect(mountPoint.querySelector("iframe")).toBeNull();
+ expect(ports[0].close).toHaveBeenCalled();
+ });
- const embedding = embedDashboard({
- id: "test-id",
- supersetDomain: "https://superset.example.com";,
- mountPoint,
- fetchGuestToken: mockFetchGuestToken,
+ // === ADDITIONAL TEST SUITES ===
+
+ describe("embedDashboard happy path", () => {
+ it("successfully embeds dashboard with valid token", async () => {
+ const mockFetch = vi.fn().mockResolvedValue("valid-jwt-token");
+
+ const result = await embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ });
+
+ expect(result).toBeDefined();
+ expect(typeof result.getScrollSize).toBe("function");
+ expect(typeof result.unmount).toBe("function");
+ });
});
- // Let the first load open its channel, so the teardown has something to
- // let go of, and only then fail the fetch.
- await vi.waitFor(() => expect(switchboards).toHaveLength(1));
- initialToken.reject(new Error("host down"));
+ describe("embedDashboard security", () => {
+ it("rejects if fetchGuestToken throws an error", async () => {
+ const mockFetch = vi.fn().mockRejectedValue(new Error("Invalid
token"));
+
+ await expect(
+ embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ })
+ ).rejects.toThrow("Invalid token");
+ });
+
+ it("handles token expiration gracefully on refresh", async () => {
+ let callCount = 0;
+ const mockFetch = vi.fn().mockImplementation(() => {
+ callCount++;
+ if (callCount === 1) return Promise.resolve("valid-initial-token");
+ return Promise.reject(new Error("Token expired on refresh"));
+ });
- await expect(embedding).rejects.toThrow("host down");
- expect(mountPoint.querySelector("iframe")).toBeNull();
- expect(ports[0].close).toHaveBeenCalled();
- });
-});
+ const result = await embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ guestTokenFetchTimeoutMs: 5000,
+ });
+
+ expect(result).toBeDefined();
+ });
+ });
+
+ describe("embedDashboard edge cases", () => {
+ it("handles iframe navigation internally", async () => {
+ const mockFetch = vi.fn().mockResolvedValue("token-1");
+
+ const result = await embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ });
+
+ expect(result).toBeDefined();
+ });
+
+ it("handles unmount during token refresh", async () => {
+ const mockFetch = vi.fn().mockResolvedValue("token-1");
+
+ const result = await embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ });
+
+ result.unmount();
+
+ expect(true).toBe(true);
Review Comment:
Addressed in `e718e628`, thanks!
##########
superset-embedded-sdk/src/index.ts:
##########
@@ -37,7 +37,17 @@ import { withTimeout } from "./withTimeout";
*/
export type GuestTokenFetchFn = () => Promise<string>;
+/**
+ * Fetches a guest token from the host application's backend server.
+ * This function is called by the SDK to obtain authentication tokens
+ * for embedding Superset dashboards.
+ */
+
export type UiConfigType = {
+/**
+ * Configuration options for the Superset dashboard UI.
+ * Controls which elements are visible/hidden in the embedded dashboard.
+ */
Review Comment:
The blocks sit above their own types now, thanks. Leaving this open because
`index.ts` needs restoring from master (see the new comment), and the blocks
will need re-applying on top of it.
--
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]
