codeant-ai-for-open-source[bot] commented on code in PR #45957:
URL: https://github.com/apache/superset/pull/45957#discussion_r4446675624
##########
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);
+
+ vi.useRealTimers();
+ });
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `5aa688b`.
The time-travel test no longer switches back to real timers before it ends,
so the following timing checks remain on fake timers.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset-embedded-sdk/src/index.test.ts:
##########
@@ -516,25 +516,115 @@ 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", () => {
+ beforeEach(() => {
+ mockSwitchboard = makeMockSwitchboard();
+ switchboards = [mockSwitchboard];
+ ports = [{ close: vi.fn() }];
+ getImpls = [() => ({ type: "handshake" })];
+ });
+
+ 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 fake guest token", async () => {
+ const mockFetch = vi.fn().mockResolvedValue("invalid-token");
+
+ await expect(
+ embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ })
+ ).rejects.toThrow();
+ });
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `5aa688b`.
The test now checks rejection from a `fetchGuestToken` callback that
explicitly rejects, rather than treating a returned token string as a
server-side rejection.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset-embedded-sdk/src/index.test.ts:
##########
@@ -516,25 +516,115 @@ 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", () => {
+ beforeEach(() => {
+ mockSwitchboard = makeMockSwitchboard();
+ switchboards = [mockSwitchboard];
+ ports = [{ close: vi.fn() }];
+ getImpls = [() => ({ type: "handshake" })];
+ });
+
+ 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 fake guest token", async () => {
+ const mockFetch = vi.fn().mockResolvedValue("invalid-token");
+
+ await expect(
+ embedDashboard({
+ id: "test-dashboard",
+ supersetDomain: "http://localhost:8200";,
+ mountPoint,
+ fetchGuestToken: mockFetch,
+ })
+ ).rejects.toThrow();
+ });
+
+ it("handles token expiration gracefully", async () => {
+ let callCount = 0;
+ const mockFetch = vi.fn().mockImplementation(() => {
+ callCount++;
+ if (callCount === 1) return Promise.reject(new Error("Token
expired"));
+ return Promise.resolve("new-valid-token");
+ });
- 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();
+ });
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `5aa688b`.
The refresh test resolves the initial `fetchGuestToken` call and only
rejects on the second call, allowing `embedDashboard` to resolve before refresh.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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]
