From 71bca83c9e53164ac305392be0f685d010606691 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 9 Apr 2026 19:27:20 -0400 Subject: [PATCH 01/17] generate and pass nonce for slack auth --- packages/react-core/src/index.ts | 1 + .../core/hooks/useAuthPostMessageListener.ts | 63 ++++++- .../src/modules/slack/hooks/index.ts | 2 +- .../src/modules/slack/hooks/useSlackAuth.ts | 11 ++ .../react-core/src/modules/slack/index.ts | 1 + .../hooks/useAuthPostMessageListener.test.ts | 169 +++++++++++++++++- .../test/slack/useSlackAuth.test.tsx | 58 ++++++ .../SlackAuthButton/SlackAuthButton.tsx | 2 + 8 files changed, 296 insertions(+), 11 deletions(-) diff --git a/packages/react-core/src/index.ts b/packages/react-core/src/index.ts index 69b69b96a..6994637ca 100644 --- a/packages/react-core/src/index.ts +++ b/packages/react-core/src/index.ts @@ -59,6 +59,7 @@ export { type KnockSlackProviderProps, type KnockSlackProviderState, type SlackChannelQueryOptions, + getSlackNonceStorageKey, useConnectedSlackChannels, useKnockSlackClient, useSlackAuth, diff --git a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts index 40451af5d..c87553a88 100644 --- a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts +++ b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts @@ -7,13 +7,45 @@ export interface UseAuthPostMessageListenerOptions { popupWindowRef: React.MutableRefObject; setConnectionStatus: (status: ConnectionStatus) => void; onAuthenticationComplete?: (authenticationResp: string) => void; + /** + * The sessionStorage key where the CSRF nonce was stored when the auth URL + * was built. When provided, the listener will verify the nonce returned in + * the postMessage payload matches the stored value. + */ + nonceStorageKey?: string; +} + +/** + * Extracts the message type from a postMessage event data payload. + * Supports both the legacy string format ("authComplete") and the new + * structured format ({ type: "authComplete", nonce: "..." }). + */ +function getMessageType(data: unknown): string | undefined { + if (typeof data === "string") return data; + if (typeof data === "object" && data !== null && "type" in data) { + return (data as { type: string }).type; + } + return undefined; +} + +/** + * Extracts the nonce from a structured postMessage event data payload. + * Returns undefined for legacy string-format messages. + */ +function getMessageNonce(data: unknown): string | undefined { + if (typeof data === "object" && data !== null && "nonce" in data) { + const nonce = (data as { nonce: unknown }).nonce; + return typeof nonce === "string" ? nonce : undefined; + } + return undefined; } /** * Hook that listens for postMessage events from OAuth popup windows. * * Handles "authComplete" and "authFailed" messages sent from the OAuth flow popup, - * validates the message origin, updates connection status, and closes the popup. + * validates the message origin, optionally verifies the CSRF nonce, updates + * connection status, and closes the popup. * * @param options - Configuration options for the postMessage listener * @@ -24,6 +56,7 @@ export interface UseAuthPostMessageListenerOptions { * popupWindowRef, * setConnectionStatus, * onAuthenticationComplete, + * nonceStorageKey: "knock:slack-auth-nonce:channel_123", * }); * ``` */ @@ -35,6 +68,7 @@ export function useAuthPostMessageListener( popupWindowRef, setConnectionStatus, onAuthenticationComplete, + nonceStorageKey, } = options; useEffect(() => { @@ -44,17 +78,35 @@ export function useAuthPostMessageListener( return; } - if (event.data === "authComplete") { + const messageType = getMessageType(event.data); + const returnedNonce = getMessageNonce(event.data); + + if (messageType === "authComplete") { + // Verify CSRF nonce when both a stored nonce and a returned nonce exist + if (nonceStorageKey && returnedNonce !== undefined) { + const storedNonce = sessionStorage.getItem(nonceStorageKey); + if (storedNonce && storedNonce !== returnedNonce) { + setConnectionStatus("error"); + popupWindowRef.current = null; + return; + } + sessionStorage.removeItem(nonceStorageKey); + } + setConnectionStatus("connected"); - onAuthenticationComplete?.(event.data); + onAuthenticationComplete?.(messageType); // Clear popup ref so polling stops and doesn't trigger callback again if (popupWindowRef.current && !popupWindowRef.current.closed) { popupWindowRef.current.close(); } popupWindowRef.current = null; - } else if (event.data === "authFailed") { + } else if (messageType === "authFailed") { + // Clean up stored nonce on failure + if (nonceStorageKey) { + sessionStorage.removeItem(nonceStorageKey); + } setConnectionStatus("error"); - onAuthenticationComplete?.(event.data); + onAuthenticationComplete?.(messageType); popupWindowRef.current = null; } }; @@ -66,5 +118,6 @@ export function useAuthPostMessageListener( onAuthenticationComplete, setConnectionStatus, popupWindowRef, + nonceStorageKey, ]); } diff --git a/packages/react-core/src/modules/slack/hooks/index.ts b/packages/react-core/src/modules/slack/hooks/index.ts index efb29ce33..cd1752081 100644 --- a/packages/react-core/src/modules/slack/hooks/index.ts +++ b/packages/react-core/src/modules/slack/hooks/index.ts @@ -1,4 +1,4 @@ export { default as useSlackConnectionStatus } from "./useSlackConnectionStatus"; export { default as useSlackChannels } from "./useSlackChannels"; export { default as useConnectedSlackChannels } from "./useConnectedSlackChannels"; -export { default as useSlackAuth } from "./useSlackAuth"; +export { default as useSlackAuth, getSlackNonceStorageKey } from "./useSlackAuth"; diff --git a/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts b/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts index 91f5c9eb1..6c191b235 100644 --- a/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts +++ b/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts @@ -12,6 +12,10 @@ const DEFAULT_SLACK_SCOPES = [ "groups:read", ]; +export function getSlackNonceStorageKey(channelId: string): string { + return `knock:slack-auth-nonce:${channelId}`; +} + type UseSlackAuthOutput = { buildSlackAuthUrl: () => string; disconnectFromSlack: () => void; @@ -83,8 +87,15 @@ function useSlackAuth( ]); const buildSlackAuthUrl = useCallback(() => { + const nonce = crypto.randomUUID(); + sessionStorage.setItem( + getSlackNonceStorageKey(knockSlackChannelId), + nonce, + ); + const rawParams = { state: JSON.stringify({ + nonce, redirect_url: redirectUrl, access_token_object: { object_id: tenantId, diff --git a/packages/react-core/src/modules/slack/index.ts b/packages/react-core/src/modules/slack/index.ts index 1e3d15438..47b6ac7a2 100644 --- a/packages/react-core/src/modules/slack/index.ts +++ b/packages/react-core/src/modules/slack/index.ts @@ -9,6 +9,7 @@ export { useSlackChannels, useConnectedSlackChannels, useSlackAuth, + getSlackNonceStorageKey, } from "./hooks"; export { type ContainerObject, diff --git a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts index c84e24b70..e617d9088 100644 --- a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts +++ b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts @@ -3,6 +3,23 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { useAuthPostMessageListener } from "../../../src/modules/core/hooks/useAuthPostMessageListener"; +const mockSessionStorage = (() => { + let store: Record = {}; + return { + getItem: vi.fn((key: string) => store[key] ?? null), + setItem: vi.fn((key: string, value: string) => { + store[key] = value; + }), + removeItem: vi.fn((key: string) => { + delete store[key]; + }), + clear: vi.fn(() => { + store = {}; + }), + }; +})(); +vi.stubGlobal("sessionStorage", mockSessionStorage); + describe("useAuthPostMessageListener", () => { const knockHost = "https://api.knock.app"; let popupWindowRef: { current: Window | null }; @@ -12,6 +29,7 @@ describe("useAuthPostMessageListener", () => { beforeEach(() => { vi.clearAllMocks(); + mockSessionStorage.clear(); // Create a mock popup window mockPopup = { @@ -24,7 +42,7 @@ describe("useAuthPostMessageListener", () => { onAuthenticationComplete = vi.fn(); }); - it("should handle authComplete message and update status", () => { + it("should handle authComplete string message and update status", () => { renderHook(() => useAuthPostMessageListener({ knockHost, @@ -34,7 +52,6 @@ describe("useAuthPostMessageListener", () => { }), ); - // Simulate postMessage event with authComplete const event = new MessageEvent("message", { data: "authComplete", origin: knockHost, @@ -47,7 +64,29 @@ describe("useAuthPostMessageListener", () => { expect(popupWindowRef.current).toBeNull(); }); - it("should handle authFailed message and update status", () => { + it("should handle authComplete object message and update status", () => { + renderHook(() => + useAuthPostMessageListener({ + knockHost, + popupWindowRef, + setConnectionStatus, + onAuthenticationComplete, + }), + ); + + const event = new MessageEvent("message", { + data: { type: "authComplete", nonce: "abc" }, + origin: knockHost, + }); + window.dispatchEvent(event); + + expect(setConnectionStatus).toHaveBeenCalledWith("connected"); + expect(onAuthenticationComplete).toHaveBeenCalledWith("authComplete"); + expect(mockPopup.close).toHaveBeenCalled(); + expect(popupWindowRef.current).toBeNull(); + }); + + it("should handle authFailed string message and update status", () => { renderHook(() => useAuthPostMessageListener({ knockHost, @@ -57,7 +96,6 @@ describe("useAuthPostMessageListener", () => { }), ); - // Simulate postMessage event with authFailed const event = new MessageEvent("message", { data: "authFailed", origin: knockHost, @@ -68,6 +106,26 @@ describe("useAuthPostMessageListener", () => { expect(popupWindowRef.current).toBeNull(); }); + it("should handle authFailed object message and update status", () => { + renderHook(() => + useAuthPostMessageListener({ + knockHost, + popupWindowRef, + setConnectionStatus, + onAuthenticationComplete, + }), + ); + + const event = new MessageEvent("message", { + data: { type: "authFailed" }, + origin: knockHost, + }); + window.dispatchEvent(event); + + expect(setConnectionStatus).toHaveBeenCalledWith("error"); + expect(popupWindowRef.current).toBeNull(); + }); + it("should ignore messages from different origins", () => { renderHook(() => useAuthPostMessageListener({ @@ -78,7 +136,6 @@ describe("useAuthPostMessageListener", () => { }), ); - // Simulate postMessage event from different origin const event = new MessageEvent("message", { data: "authComplete", origin: "https://evil.com", @@ -111,4 +168,106 @@ describe("useAuthPostMessageListener", () => { removeEventListenerSpy.mockRestore(); }); + + describe("CSRF nonce verification", () => { + const nonceStorageKey = "knock:slack-auth-nonce:channel_123"; + + it("should accept authComplete when nonce matches", () => { + mockSessionStorage.setItem(nonceStorageKey, "valid-nonce"); + + renderHook(() => + useAuthPostMessageListener({ + knockHost, + popupWindowRef, + setConnectionStatus, + onAuthenticationComplete, + nonceStorageKey, + }), + ); + + const event = new MessageEvent("message", { + data: { type: "authComplete", nonce: "valid-nonce" }, + origin: knockHost, + }); + window.dispatchEvent(event); + + expect(setConnectionStatus).toHaveBeenCalledWith("connected"); + expect(onAuthenticationComplete).toHaveBeenCalledWith("authComplete"); + expect(mockSessionStorage.removeItem).toHaveBeenCalledWith( + nonceStorageKey, + ); + }); + + it("should reject authComplete when nonce does not match", () => { + mockSessionStorage.setItem(nonceStorageKey, "expected-nonce"); + + renderHook(() => + useAuthPostMessageListener({ + knockHost, + popupWindowRef, + setConnectionStatus, + onAuthenticationComplete, + nonceStorageKey, + }), + ); + + const event = new MessageEvent("message", { + data: { type: "authComplete", nonce: "wrong-nonce" }, + origin: knockHost, + }); + window.dispatchEvent(event); + + expect(setConnectionStatus).toHaveBeenCalledWith("error"); + expect(onAuthenticationComplete).not.toHaveBeenCalled(); + }); + + it("should accept authComplete with legacy string format even when nonceStorageKey is set", () => { + mockSessionStorage.setItem(nonceStorageKey, "stored-nonce"); + + renderHook(() => + useAuthPostMessageListener({ + knockHost, + popupWindowRef, + setConnectionStatus, + onAuthenticationComplete, + nonceStorageKey, + }), + ); + + // Legacy API sends bare string — no nonce to verify, so allow it + const event = new MessageEvent("message", { + data: "authComplete", + origin: knockHost, + }); + window.dispatchEvent(event); + + expect(setConnectionStatus).toHaveBeenCalledWith("connected"); + expect(onAuthenticationComplete).toHaveBeenCalledWith("authComplete"); + }); + + it("should clean up stored nonce on authFailed", () => { + mockSessionStorage.setItem(nonceStorageKey, "stored-nonce"); + + renderHook(() => + useAuthPostMessageListener({ + knockHost, + popupWindowRef, + setConnectionStatus, + onAuthenticationComplete, + nonceStorageKey, + }), + ); + + const event = new MessageEvent("message", { + data: { type: "authFailed", nonce: "stored-nonce" }, + origin: knockHost, + }); + window.dispatchEvent(event); + + expect(setConnectionStatus).toHaveBeenCalledWith("error"); + expect(mockSessionStorage.removeItem).toHaveBeenCalledWith( + nonceStorageKey, + ); + }); + }); }); diff --git a/packages/react-core/test/slack/useSlackAuth.test.tsx b/packages/react-core/test/slack/useSlackAuth.test.tsx index 2b608ff4c..043533b32 100644 --- a/packages/react-core/test/slack/useSlackAuth.test.tsx +++ b/packages/react-core/test/slack/useSlackAuth.test.tsx @@ -2,9 +2,30 @@ import { act, renderHook } from "@testing-library/react"; import { beforeEach, describe, expect, test, vi } from "vitest"; import useSlackAuth from "../../src/modules/slack/hooks/useSlackAuth"; +import { getSlackNonceStorageKey } from "../../src/modules/slack/hooks/useSlackAuth"; const TEST_BRANCH_SLUG = "lorem-ipsum-branch"; +const mockRandomUUID = vi.fn(() => "test-nonce-uuid"); +vi.stubGlobal("crypto", { randomUUID: mockRandomUUID }); + +const mockSessionStorage = (() => { + const store: Record = {}; + return { + getItem: vi.fn((key: string) => store[key] ?? null), + setItem: vi.fn((key: string, value: string) => { + store[key] = value; + }), + removeItem: vi.fn((key: string) => { + delete store[key]; + }), + clear: vi.fn(() => { + for (const key in store) delete store[key]; + }), + }; +})(); +vi.stubGlobal("sessionStorage", mockSessionStorage); + const mockSetConnectionStatus = vi.fn(); const mockSetActionLabel = vi.fn(); @@ -48,6 +69,7 @@ describe("useSlackAuth", () => { "chat:write,chat:write.public,channels:read,groups:read", ); expect(state).toEqual({ + nonce: "test-nonce-uuid", redirect_url: "http://localhost:3000", access_token_object: { object_id: "test_tenant_id", @@ -60,6 +82,42 @@ describe("useSlackAuth", () => { }); }); + test("buildSlackAuthUrl stores nonce in sessionStorage", () => { + const { result } = renderHook(() => + useSlackAuth("test_client_id", "http://localhost:3000"), + ); + + result.current.buildSlackAuthUrl(); + + expect(mockSessionStorage.setItem).toHaveBeenCalledWith( + getSlackNonceStorageKey("test_channel_id"), + "test-nonce-uuid", + ); + }); + + test("buildSlackAuthUrl generates a new nonce each time", () => { + mockRandomUUID + .mockReturnValueOnce("nonce-1") + .mockReturnValueOnce("nonce-2"); + + const { result } = renderHook(() => + useSlackAuth("test_client_id", "http://localhost:3000"), + ); + + const url1 = new URL(result.current.buildSlackAuthUrl()); + const state1 = JSON.parse( + new URLSearchParams(url1.search).get("state") || "{}", + ); + + const url2 = new URL(result.current.buildSlackAuthUrl()); + const state2 = JSON.parse( + new URLSearchParams(url2.search).get("state") || "{}", + ); + + expect(state1.nonce).toBe("nonce-1"); + expect(state2.nonce).toBe("nonce-2"); + }); + test("buildSlackAuthUrl uses custom scopes when provided", () => { const { result } = renderHook(() => useSlackAuth("test_client_id", "http://localhost:3000", { diff --git a/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx b/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx index 0da638b33..a597f009f 100644 --- a/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx +++ b/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx @@ -5,6 +5,7 @@ import { useKnockSlackClient, useSlackAuth, useTranslations, + getSlackNonceStorageKey, } from "@knocklabs/react-core"; import { FunctionComponent, useCallback, useMemo } from "react"; @@ -64,6 +65,7 @@ export const SlackAuthButton: FunctionComponent = ({ popupWindowRef, setConnectionStatus, onAuthenticationComplete, + nonceStorageKey: getSlackNonceStorageKey(knockSlackChannelId), }); useAuthPolling({ From d2202a05a67f95c46ab7c584d27d6524f5dea434 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 11:25:45 -0400 Subject: [PATCH 02/17] yarn format --- packages/react-core/src/modules/slack/hooks/index.ts | 5 ++++- packages/react-core/src/modules/slack/hooks/useSlackAuth.ts | 5 +---- .../slack/components/SlackAuthButton/SlackAuthButton.tsx | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react-core/src/modules/slack/hooks/index.ts b/packages/react-core/src/modules/slack/hooks/index.ts index cd1752081..75d192629 100644 --- a/packages/react-core/src/modules/slack/hooks/index.ts +++ b/packages/react-core/src/modules/slack/hooks/index.ts @@ -1,4 +1,7 @@ export { default as useSlackConnectionStatus } from "./useSlackConnectionStatus"; export { default as useSlackChannels } from "./useSlackChannels"; export { default as useConnectedSlackChannels } from "./useConnectedSlackChannels"; -export { default as useSlackAuth, getSlackNonceStorageKey } from "./useSlackAuth"; +export { + default as useSlackAuth, + getSlackNonceStorageKey, +} from "./useSlackAuth"; diff --git a/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts b/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts index 6c191b235..79544bb33 100644 --- a/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts +++ b/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts @@ -88,10 +88,7 @@ function useSlackAuth( const buildSlackAuthUrl = useCallback(() => { const nonce = crypto.randomUUID(); - sessionStorage.setItem( - getSlackNonceStorageKey(knockSlackChannelId), - nonce, - ); + sessionStorage.setItem(getSlackNonceStorageKey(knockSlackChannelId), nonce); const rawParams = { state: JSON.stringify({ diff --git a/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx b/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx index a597f009f..24079ad95 100644 --- a/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx +++ b/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx @@ -1,11 +1,11 @@ import { + getSlackNonceStorageKey, useAuthPolling, useAuthPostMessageListener, useKnockClient, useKnockSlackClient, useSlackAuth, useTranslations, - getSlackNonceStorageKey, } from "@knocklabs/react-core"; import { FunctionComponent, useCallback, useMemo } from "react"; From 6cf103c3a4026dd910d4fab77ab97314fe28fc45 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 12:03:41 -0400 Subject: [PATCH 03/17] handle empty nonce from session storage, and also clean up in error scenario --- .../core/hooks/useAuthPostMessageListener.ts | 6 +++-- .../hooks/useAuthPostMessageListener.test.ts | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts index c87553a88..7f566b32c 100644 --- a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts +++ b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts @@ -82,10 +82,12 @@ export function useAuthPostMessageListener( const returnedNonce = getMessageNonce(event.data); if (messageType === "authComplete") { - // Verify CSRF nonce when both a stored nonce and a returned nonce exist + // Verify CSRF nonce when a nonceStorageKey is configured and the API + // returned a nonce if (nonceStorageKey && returnedNonce !== undefined) { const storedNonce = sessionStorage.getItem(nonceStorageKey); - if (storedNonce && storedNonce !== returnedNonce) { + if (!storedNonce || storedNonce !== returnedNonce) { + sessionStorage.removeItem(nonceStorageKey); setConnectionStatus("error"); popupWindowRef.current = null; return; diff --git a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts index e617d9088..4bd8f0eca 100644 --- a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts +++ b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts @@ -245,6 +245,28 @@ describe("useAuthPostMessageListener", () => { expect(onAuthenticationComplete).toHaveBeenCalledWith("authComplete"); }); + it("should reject authComplete when stored nonce is missing", () => { + // Do NOT store a nonce — simulates cleared storage or replay + renderHook(() => + useAuthPostMessageListener({ + knockHost, + popupWindowRef, + setConnectionStatus, + onAuthenticationComplete, + nonceStorageKey, + }), + ); + + const event = new MessageEvent("message", { + data: { type: "authComplete", nonce: "some-nonce" }, + origin: knockHost, + }); + window.dispatchEvent(event); + + expect(setConnectionStatus).toHaveBeenCalledWith("error"); + expect(onAuthenticationComplete).not.toHaveBeenCalled(); + }); + it("should clean up stored nonce on authFailed", () => { mockSessionStorage.setItem(nonceStorageKey, "stored-nonce"); From 83460f5435fa18ebc6f4af8e0d127bd771303cb5 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 12:31:58 -0400 Subject: [PATCH 04/17] cursor review feedback --- .../core/hooks/useAuthPostMessageListener.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts index 7f566b32c..1505e17fc 100644 --- a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts +++ b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts @@ -20,12 +20,11 @@ export interface UseAuthPostMessageListenerOptions { * Supports both the legacy string format ("authComplete") and the new * structured format ({ type: "authComplete", nonce: "..." }). */ -function getMessageType(data: unknown): string | undefined { - if (typeof data === "string") return data; +function getMessageType(data: unknown): string { if (typeof data === "object" && data !== null && "type" in data) { return (data as { type: string }).type; } - return undefined; + return data as string; } /** @@ -79,14 +78,13 @@ export function useAuthPostMessageListener( } const messageType = getMessageType(event.data); - const returnedNonce = getMessageNonce(event.data); if (messageType === "authComplete") { - // Verify CSRF nonce when a nonceStorageKey is configured and the API - // returned a nonce - if (nonceStorageKey && returnedNonce !== undefined) { + // Verify CSRF nonce when a nonceStorageKey is configured. + if (nonceStorageKey) { + const returnedNonce = getMessageNonce(event.data); const storedNonce = sessionStorage.getItem(nonceStorageKey); - if (!storedNonce || storedNonce !== returnedNonce) { + if (!storedNonce || !returnedNonce || storedNonce !== returnedNonce) { sessionStorage.removeItem(nonceStorageKey); setConnectionStatus("error"); popupWindowRef.current = null; From a422d49a4d207175ca77a92821bf6b5fd5e4e68a Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 12:36:54 -0400 Subject: [PATCH 05/17] test clean up --- .../hooks/useAuthPostMessageListener.test.ts | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts index 4bd8f0eca..f9ffcadbd 100644 --- a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts +++ b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts @@ -221,7 +221,7 @@ describe("useAuthPostMessageListener", () => { expect(onAuthenticationComplete).not.toHaveBeenCalled(); }); - it("should accept authComplete with legacy string format even when nonceStorageKey is set", () => { + it("should reject legacy string format when nonceStorageKey is set", () => { mockSessionStorage.setItem(nonceStorageKey, "stored-nonce"); renderHook(() => @@ -234,15 +234,39 @@ describe("useAuthPostMessageListener", () => { }), ); - // Legacy API sends bare string — no nonce to verify, so allow it + // Legacy string has no nonce — fails verification when nonceStorageKey is configured const event = new MessageEvent("message", { data: "authComplete", origin: knockHost, }); window.dispatchEvent(event); - expect(setConnectionStatus).toHaveBeenCalledWith("connected"); - expect(onAuthenticationComplete).toHaveBeenCalledWith("authComplete"); + expect(setConnectionStatus).toHaveBeenCalledWith("error"); + expect(onAuthenticationComplete).not.toHaveBeenCalled(); + }); + + it("should reject structured authComplete without nonce field", () => { + mockSessionStorage.setItem(nonceStorageKey, "stored-nonce"); + + renderHook(() => + useAuthPostMessageListener({ + knockHost, + popupWindowRef, + setConnectionStatus, + onAuthenticationComplete, + nonceStorageKey, + }), + ); + + // Structured object but missing nonce — should not bypass verification + const event = new MessageEvent("message", { + data: { type: "authComplete" }, + origin: knockHost, + }); + window.dispatchEvent(event); + + expect(setConnectionStatus).toHaveBeenCalledWith("error"); + expect(onAuthenticationComplete).not.toHaveBeenCalled(); }); it("should reject authComplete when stored nonce is missing", () => { From ac3a0e459635c342ea7c9ce5247799673865b970 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 12:53:59 -0400 Subject: [PATCH 06/17] cursor review feedback --- .../core/hooks/useAuthPostMessageListener.ts | 18 +- .../hooks/useAuthPostMessageListener.test.ts | 12 ++ slack-oauth-csrf-diagnosis.md | 180 ++++++++++++++++++ switchboard-csrf-nonce-prompt.md | 75 ++++++++ 4 files changed, 277 insertions(+), 8 deletions(-) create mode 100644 slack-oauth-csrf-diagnosis.md create mode 100644 switchboard-csrf-nonce-prompt.md diff --git a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts index 1505e17fc..cb6f0bb6b 100644 --- a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts +++ b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts @@ -71,6 +71,13 @@ export function useAuthPostMessageListener( } = options; useEffect(() => { + const closePopup = () => { + if (popupWindowRef.current && !popupWindowRef.current.closed) { + popupWindowRef.current.close(); + } + popupWindowRef.current = null; + }; + const receiveMessage = (event: MessageEvent) => { // Validate message origin for security if (event.origin !== knockHost) { @@ -87,7 +94,7 @@ export function useAuthPostMessageListener( if (!storedNonce || !returnedNonce || storedNonce !== returnedNonce) { sessionStorage.removeItem(nonceStorageKey); setConnectionStatus("error"); - popupWindowRef.current = null; + closePopup(); return; } sessionStorage.removeItem(nonceStorageKey); @@ -95,19 +102,14 @@ export function useAuthPostMessageListener( setConnectionStatus("connected"); onAuthenticationComplete?.(messageType); - // Clear popup ref so polling stops and doesn't trigger callback again - if (popupWindowRef.current && !popupWindowRef.current.closed) { - popupWindowRef.current.close(); - } - popupWindowRef.current = null; + closePopup(); } else if (messageType === "authFailed") { - // Clean up stored nonce on failure if (nonceStorageKey) { sessionStorage.removeItem(nonceStorageKey); } setConnectionStatus("error"); onAuthenticationComplete?.(messageType); - popupWindowRef.current = null; + closePopup(); } }; diff --git a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts index f9ffcadbd..fc13afa3b 100644 --- a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts +++ b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts @@ -103,6 +103,7 @@ describe("useAuthPostMessageListener", () => { window.dispatchEvent(event); expect(setConnectionStatus).toHaveBeenCalledWith("error"); + expect(mockPopup.close).toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); @@ -123,6 +124,7 @@ describe("useAuthPostMessageListener", () => { window.dispatchEvent(event); expect(setConnectionStatus).toHaveBeenCalledWith("error"); + expect(mockPopup.close).toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); @@ -219,6 +221,8 @@ describe("useAuthPostMessageListener", () => { expect(setConnectionStatus).toHaveBeenCalledWith("error"); expect(onAuthenticationComplete).not.toHaveBeenCalled(); + expect(mockPopup.close).toHaveBeenCalled(); + expect(popupWindowRef.current).toBeNull(); }); it("should reject legacy string format when nonceStorageKey is set", () => { @@ -243,6 +247,8 @@ describe("useAuthPostMessageListener", () => { expect(setConnectionStatus).toHaveBeenCalledWith("error"); expect(onAuthenticationComplete).not.toHaveBeenCalled(); + expect(mockPopup.close).toHaveBeenCalled(); + expect(popupWindowRef.current).toBeNull(); }); it("should reject structured authComplete without nonce field", () => { @@ -267,6 +273,8 @@ describe("useAuthPostMessageListener", () => { expect(setConnectionStatus).toHaveBeenCalledWith("error"); expect(onAuthenticationComplete).not.toHaveBeenCalled(); + expect(mockPopup.close).toHaveBeenCalled(); + expect(popupWindowRef.current).toBeNull(); }); it("should reject authComplete when stored nonce is missing", () => { @@ -289,6 +297,8 @@ describe("useAuthPostMessageListener", () => { expect(setConnectionStatus).toHaveBeenCalledWith("error"); expect(onAuthenticationComplete).not.toHaveBeenCalled(); + expect(mockPopup.close).toHaveBeenCalled(); + expect(popupWindowRef.current).toBeNull(); }); it("should clean up stored nonce on authFailed", () => { @@ -314,6 +324,8 @@ describe("useAuthPostMessageListener", () => { expect(mockSessionStorage.removeItem).toHaveBeenCalledWith( nonceStorageKey, ); + expect(mockPopup.close).toHaveBeenCalled(); + expect(popupWindowRef.current).toBeNull(); }); }); }); diff --git a/slack-oauth-csrf-diagnosis.md b/slack-oauth-csrf-diagnosis.md new file mode 100644 index 000000000..fefe61393 --- /dev/null +++ b/slack-oauth-csrf-diagnosis.md @@ -0,0 +1,180 @@ +# Slack OAuth CSRF Nonce Diagnosis + +## Problem + +A customer using a headless implementation of `useSlackAuth` was flagged during Slack's app review process for missing CSRF protection (nonce) in the `state` parameter of their OAuth URL. Investigation confirmed two issues: + +1. **No nonce is generated.** `buildSlackAuthUrl()` in `useSlackAuth.ts:85-111` constructs the `state` parameter with only data-passing fields (redirect_url, access_token_object, channel_id, public_key, user_token, branch_slug). There is no random, session-bound nonce for CSRF protection. + +2. **Even if a customer added a nonce, they couldn't verify it.** The `state` parameter is fully consumed by the Knock API at `/providers/slack/authenticate`. The `postMessage` sent back to the client is a bare string (`"authComplete"` or `"authFailed"`) — it carries no data from the original state, so the nonce is lost. + +This affects **both the `SlackAuthButton` component and headless `useSlackAuth` users**, and the identical pattern exists in `useMsTeamsAuth.ts:34-50` for MS Teams. + +--- + +## Current OAuth Flow + +``` +1. SDK (useSlackAuth.buildSlackAuthUrl) → builds URL with state = JSON of + {redirect_url, access_token_object, channel_id, public_key, user_token, branch_slug} +2. User authorizes in Slack popup +3. Slack redirects to api.knock.app/providers/slack/authenticate?code=...&state=... +4. Knock API consumes state, exchanges code for token, stores it +5. Knock API sends postMessage("authComplete") back to the parent window +6. SDK (useAuthPostMessageListener) receives "authComplete" string → sets status to "connected" + OR: SDK (useAuthPolling) polls authCheck endpoint every 2s → detects success +``` + +### Key Files (SDK) + +| File | Role | +|------|------| +| `packages/react-core/src/modules/slack/hooks/useSlackAuth.ts` | Builds auth URL with `state`, handles disconnect | +| `packages/react-core/src/modules/ms-teams/hooks/useMsTeamsAuth.ts` | Same pattern for MS Teams | +| `packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts` | Listens for postMessage from OAuth popup, validates origin | +| `packages/react-core/src/modules/core/hooks/useAuthPolling.ts` | Fallback polling (every 2s, up to 3 min) | +| `packages/react-core/src/modules/slack/context/KnockSlackProvider.tsx` | Context provider managing popup ref and connection status | +| `packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx` | Button component that opens popup, wires up listener + polling | +| `packages/react/src/modules/ms-teams/components/MsTeamsAuthButton/MsTeamsAuthButton.tsx` | Same for MS Teams | + +### Key Endpoint (API) + +| Endpoint | Repo | +|----------|------| +| `/providers/slack/authenticate` | [switchboard](https://github.com/knocklabs/switchboard/blob/b50704b460e62229ea5c9c66d14ed82f0a3bb326/lib/switchboard_web/router.ex#L327) | + +--- + +## Recommended Changes + +### 1. SDK Changes (this repo) + +#### A. Generate and store a nonce in `useSlackAuth.ts` + +```typescript +// In buildSlackAuthUrl callback: +const nonce = crypto.randomUUID(); +sessionStorage.setItem(`knock:slack-auth-nonce:${knockSlackChannelId}`, nonce); + +const rawParams = { + state: JSON.stringify({ + nonce, // <-- NEW + redirect_url: redirectUrl, + access_token_object: { ... }, + channel_id: knockSlackChannelId, + public_key: knock.apiKey, + user_token: knock.userToken, + branch_slug: knock.branch, + }), + client_id: slackClientId, + scope: combinedScopes.join(","), +}; +``` + +Same change in `useMsTeamsAuth.ts`. + +#### B. Update `useAuthPostMessageListener.ts` to verify the nonce + +The postMessage data format needs to change from a bare string to a structured object. The listener should: + +```typescript +const receiveMessage = (event: MessageEvent) => { + if (event.origin !== knockHost) return; + + // Support both old format (string) and new format (object with nonce) + const messageType = typeof event.data === "string" + ? event.data + : event.data?.type; + const returnedNonce = typeof event.data === "object" + ? event.data?.nonce + : undefined; + + if (messageType === "authComplete") { + // Verify nonce if one was stored (and one was returned) + if (returnedNonce !== undefined) { + const storedNonce = sessionStorage.getItem(`knock:${provider}-auth-nonce:${channelId}`); + if (storedNonce && storedNonce !== returnedNonce) { + setConnectionStatus("error"); + return; + } + sessionStorage.removeItem(`knock:${provider}-auth-nonce:${channelId}`); + } + + setConnectionStatus("connected"); + onAuthenticationComplete?.(messageType); + // ... close popup + } +}; +``` + +The listener needs to accept a `channelId` (or sessionStorage key) so it knows which nonce to look up. This means updating `UseAuthPostMessageListenerOptions` to include the relevant context. + +#### C. Expose the nonce for headless users + +For customers using `useSlackAuth` headlessly (not using `SlackAuthButton` or the postMessage listener), they need to be able to access the nonce to verify it themselves. Two options: + +- **Option 1**: Return it from `buildSlackAuthUrl` — change the return type from `string` to `{ url: string; nonce: string }` (breaking change) +- **Option 2** (recommended): Keep `buildSlackAuthUrl` returning a string, but add a separate `getAuthNonce()` function to the hook's return value, or document that customers can read it from `sessionStorage` at the known key + +#### D. Polling path (`useAuthPolling.ts`) + +The polling mechanism independently checks auth status via `authCheck()` — it doesn't go through the OAuth redirect. It doesn't need nonce verification directly, since it's just checking "did the token get stored?" However, we could optionally verify the nonce through the authCheck response if the API returns it. + +### 2. API Changes (switchboard repo) + +#### A. `/providers/slack/authenticate` — echo the nonce in postMessage + +This is the critical API change. The endpoint currently sends: + +```javascript +postMessage("authComplete", "*") // or "authFailed" +``` + +It needs to change to: + +```javascript +// Extract nonce from the state parameter +const state = JSON.parse(params["state"]) +const nonce = state["nonce"] + +// Send structured data instead of bare string +postMessage({ type: "authComplete", nonce: nonce }, redirect_origin) +``` + +This must be backward compatible: if no nonce is in the state, send the old format or send `{ type: "authComplete" }` with no nonce field. The SDK's updated listener handles both formats. + +#### B. Same change for `/providers/ms-teams/authenticate` + +The MS Teams callback endpoint needs the identical treatment. + +#### C. Tighten postMessage target origin (optional but recommended) + +If the API is currently using `"*"` as the target origin for `postMessage`, it should be tightened to the `redirect_url` origin from the state parameter. This prevents the auth result from being intercepted by other windows. + +--- + +## Summary of Changes by File + +| File | Repo | Change | +|------|------|--------| +| `react-core/.../useSlackAuth.ts` | SDK | Generate nonce, store in sessionStorage, include in state | +| `react-core/.../useMsTeamsAuth.ts` | SDK | Same as above | +| `react-core/.../useAuthPostMessageListener.ts` | SDK | Accept structured postMessage, verify nonce against sessionStorage | +| `react/.../SlackAuthButton.tsx` | SDK | Pass channelId to postMessage listener for nonce lookup | +| `react/.../MsTeamsAuthButton.tsx` | SDK | Same as above | +| `switchboard/.../slack/authenticate` | API | Extract nonce from state, include in postMessage response | +| `switchboard/.../ms-teams/authenticate` | API | Same as above | +| Tests for all above | Both | Update to cover nonce generation, verification, and backward compat | + +--- + +## Migration / Backward Compatibility + +The recommended rollout order: + +1. **Ship SDK first** with nonce generation + listener that accepts both string and object postMessage formats +2. **Ship API second** with nonce echo in postMessage (using object format) +3. Eventually deprecate string-only format + +- The SDK change is backward compatible: it adds a nonce to state, but old API versions that ignore unknown state fields won't break. +- The API change requires care: sending structured postMessage data means old SDK versions that check `event.data === "authComplete"` will fail — so the API should also keep sending the string format during a transition period, or the SDK should be released first with support for both formats. diff --git a/switchboard-csrf-nonce-prompt.md b/switchboard-csrf-nonce-prompt.md new file mode 100644 index 000000000..83e677d85 --- /dev/null +++ b/switchboard-csrf-nonce-prompt.md @@ -0,0 +1,75 @@ +# Switchboard: Add CSRF Nonce to Slack OAuth Callback + +## Context + +Our JavaScript SDK (`@knocklabs/react-core`) builds OAuth authorization URLs for Slack that include a `state` parameter. This `state` is a JSON-encoded object used to pass data through the OAuth flow: + +```json +{ + "redirect_url": "https://customer-app.com/callback", + "access_token_object": { "object_id": "tenant_123", "collection": "$tenants" }, + "channel_id": "knock_channel_abc", + "public_key": "pk_test_...", + "user_token": "jwt_...", + "branch_slug": "production" +} +``` + +Slack's app review process now requires a CSRF-preventing nonce in the `state` parameter. We are updating the SDK to generate a random `nonce` field and include it in the `state` JSON. The SDK stores this nonce in `sessionStorage` so it can verify it when the auth flow completes. + +However, the OAuth callback endpoint in Switchboard (`/providers/slack/authenticate`) consumes the `state` parameter server-side and communicates the result back to the client via `postMessage`. Currently the `postMessage` sends a bare string (`"authComplete"` or `"authFailed"`), which means the nonce is lost and the client can never verify it. + +**We need Switchboard to echo the nonce back to the client in the `postMessage` payload.** + +## What to Change + +### 1. Slack OAuth callback — `/providers/slack/authenticate` + +Reference: `lib/switchboard_web/router.ex` line 327 and whatever controller/handler it routes to. + +When processing the OAuth callback: + +1. Parse the `state` JSON from the query params (you likely already do this to extract `redirect_url`, `access_token_object`, etc.) +2. Extract the `nonce` field from the parsed state (it may not be present — old SDK versions won't send it) +3. When sending the `postMessage` back to the parent window, change the data format from a bare string to a structured object that includes the nonce: + +**Before:** +```javascript +window.opener.postMessage("authComplete", "*"); +// or +window.opener.postMessage("authFailed", "*"); +``` + +**After:** +```javascript +window.opener.postMessage({ type: "authComplete", nonce: "" }, ""); +// or +window.opener.postMessage({ type: "authFailed", nonce: "" }, ""); +``` + +If the `nonce` field is not present in the state (backward compat with old SDK versions), omit it from the payload or set it to `null`: +```javascript +window.opener.postMessage({ type: "authComplete", nonce: null }, ""); +``` + +### 2. Tighten `postMessage` target origin (recommended) + +If the `postMessage` calls currently use `"*"` as the target origin, tighten them to use the origin derived from the `redirect_url` in the state parameter. For example, if `redirect_url` is `https://customer-app.com/callback`, the target origin should be `https://customer-app.com`. This prevents the auth result from being interceptable by unrelated windows. + +If `redirect_url` is missing or unparseable, fall back to `"*"`. + +## Backward Compatibility + +- **Old SDK + new API**: The SDK currently checks `event.data === "authComplete"`. Sending `{ type: "authComplete", nonce: ... }` will break this check. The updated SDK (shipping first) will handle both formats. During the transition period where old SDK versions are still in the wild, consider one of: + - **(a)** Accept that old SDK versions will break on this change (they'll fall back to the polling mechanism, which still works independently — so auth will still succeed, just not via the faster postMessage path) + - **(b)** Send both formats: `postMessage("authComplete", ...)` followed by `postMessage({ type: "authComplete", nonce }, ...)` — the old SDK picks up the first, the new SDK picks up the second. This is the safest approach for a smooth rollout. +- **New SDK + old API**: The updated SDK will accept both bare strings and structured objects. If the API hasn't been updated yet, the SDK still works — it just can't verify the nonce (which is fine, it degrades gracefully). + +**Recommendation: use approach (b)** — send both formats — to avoid any disruption for customers on older SDK versions. + +## Testing + +- Verify that a Slack OAuth flow with the new SDK sends a `nonce` in the state and receives it back in the `postMessage` payload +- Verify that a Slack OAuth flow with an old SDK (no `nonce` in state) still works correctly (postMessage sends `nonce: null`) +- Verify that the `authFailed` path also includes the nonce +- If tightening the target origin: verify `postMessage` is received correctly when `redirect_url` is present, and that the `"*"` fallback works when it's absent From 5495a779befe0eed6a8ff91c05c7155b346ec89f Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 14:18:56 -0400 Subject: [PATCH 07/17] scope nonce storage key with user id also --- .../src/modules/slack/hooks/useSlackAuth.ts | 12 +++++++++--- packages/react-core/test/slack/useSlackAuth.test.tsx | 3 ++- .../components/SlackAuthButton/SlackAuthButton.tsx | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts b/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts index 79544bb33..b71e145fc 100644 --- a/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts +++ b/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts @@ -12,8 +12,11 @@ const DEFAULT_SLACK_SCOPES = [ "groups:read", ]; -export function getSlackNonceStorageKey(channelId: string): string { - return `knock:slack-auth-nonce:${channelId}`; +export function getSlackNonceStorageKey( + channelId: string, + userId: string, +): string { + return `knock:slack-auth-nonce:${channelId}:${userId}`; } type UseSlackAuthOutput = { @@ -88,7 +91,10 @@ function useSlackAuth( const buildSlackAuthUrl = useCallback(() => { const nonce = crypto.randomUUID(); - sessionStorage.setItem(getSlackNonceStorageKey(knockSlackChannelId), nonce); + sessionStorage.setItem( + getSlackNonceStorageKey(knockSlackChannelId, knock.userId!), + nonce, + ); const rawParams = { state: JSON.stringify({ diff --git a/packages/react-core/test/slack/useSlackAuth.test.tsx b/packages/react-core/test/slack/useSlackAuth.test.tsx index 043533b32..34e5b6173 100644 --- a/packages/react-core/test/slack/useSlackAuth.test.tsx +++ b/packages/react-core/test/slack/useSlackAuth.test.tsx @@ -47,6 +47,7 @@ vi.mock("../../src/modules/core", () => ({ slack: mockSlackClient, apiKey: "test_api_key", userToken: "test_user_token", + userId: "test_user_id", branch: TEST_BRANCH_SLUG, }), })); @@ -90,7 +91,7 @@ describe("useSlackAuth", () => { result.current.buildSlackAuthUrl(); expect(mockSessionStorage.setItem).toHaveBeenCalledWith( - getSlackNonceStorageKey("test_channel_id"), + getSlackNonceStorageKey("test_channel_id", "test_user_id"), "test-nonce-uuid", ); }); diff --git a/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx b/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx index 24079ad95..d3c34aecc 100644 --- a/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx +++ b/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx @@ -65,7 +65,7 @@ export const SlackAuthButton: FunctionComponent = ({ popupWindowRef, setConnectionStatus, onAuthenticationComplete, - nonceStorageKey: getSlackNonceStorageKey(knockSlackChannelId), + nonceStorageKey: getSlackNonceStorageKey(knockSlackChannelId, knock.userId!), }); useAuthPolling({ From 909ed2f9f6472955e95e3d53df5b63fe4660c912 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 14:19:39 -0400 Subject: [PATCH 08/17] remove prompt md's --- slack-oauth-csrf-diagnosis.md | 180 ------------------------------- switchboard-csrf-nonce-prompt.md | 75 ------------- 2 files changed, 255 deletions(-) delete mode 100644 slack-oauth-csrf-diagnosis.md delete mode 100644 switchboard-csrf-nonce-prompt.md diff --git a/slack-oauth-csrf-diagnosis.md b/slack-oauth-csrf-diagnosis.md deleted file mode 100644 index fefe61393..000000000 --- a/slack-oauth-csrf-diagnosis.md +++ /dev/null @@ -1,180 +0,0 @@ -# Slack OAuth CSRF Nonce Diagnosis - -## Problem - -A customer using a headless implementation of `useSlackAuth` was flagged during Slack's app review process for missing CSRF protection (nonce) in the `state` parameter of their OAuth URL. Investigation confirmed two issues: - -1. **No nonce is generated.** `buildSlackAuthUrl()` in `useSlackAuth.ts:85-111` constructs the `state` parameter with only data-passing fields (redirect_url, access_token_object, channel_id, public_key, user_token, branch_slug). There is no random, session-bound nonce for CSRF protection. - -2. **Even if a customer added a nonce, they couldn't verify it.** The `state` parameter is fully consumed by the Knock API at `/providers/slack/authenticate`. The `postMessage` sent back to the client is a bare string (`"authComplete"` or `"authFailed"`) — it carries no data from the original state, so the nonce is lost. - -This affects **both the `SlackAuthButton` component and headless `useSlackAuth` users**, and the identical pattern exists in `useMsTeamsAuth.ts:34-50` for MS Teams. - ---- - -## Current OAuth Flow - -``` -1. SDK (useSlackAuth.buildSlackAuthUrl) → builds URL with state = JSON of - {redirect_url, access_token_object, channel_id, public_key, user_token, branch_slug} -2. User authorizes in Slack popup -3. Slack redirects to api.knock.app/providers/slack/authenticate?code=...&state=... -4. Knock API consumes state, exchanges code for token, stores it -5. Knock API sends postMessage("authComplete") back to the parent window -6. SDK (useAuthPostMessageListener) receives "authComplete" string → sets status to "connected" - OR: SDK (useAuthPolling) polls authCheck endpoint every 2s → detects success -``` - -### Key Files (SDK) - -| File | Role | -|------|------| -| `packages/react-core/src/modules/slack/hooks/useSlackAuth.ts` | Builds auth URL with `state`, handles disconnect | -| `packages/react-core/src/modules/ms-teams/hooks/useMsTeamsAuth.ts` | Same pattern for MS Teams | -| `packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts` | Listens for postMessage from OAuth popup, validates origin | -| `packages/react-core/src/modules/core/hooks/useAuthPolling.ts` | Fallback polling (every 2s, up to 3 min) | -| `packages/react-core/src/modules/slack/context/KnockSlackProvider.tsx` | Context provider managing popup ref and connection status | -| `packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx` | Button component that opens popup, wires up listener + polling | -| `packages/react/src/modules/ms-teams/components/MsTeamsAuthButton/MsTeamsAuthButton.tsx` | Same for MS Teams | - -### Key Endpoint (API) - -| Endpoint | Repo | -|----------|------| -| `/providers/slack/authenticate` | [switchboard](https://github.com/knocklabs/switchboard/blob/b50704b460e62229ea5c9c66d14ed82f0a3bb326/lib/switchboard_web/router.ex#L327) | - ---- - -## Recommended Changes - -### 1. SDK Changes (this repo) - -#### A. Generate and store a nonce in `useSlackAuth.ts` - -```typescript -// In buildSlackAuthUrl callback: -const nonce = crypto.randomUUID(); -sessionStorage.setItem(`knock:slack-auth-nonce:${knockSlackChannelId}`, nonce); - -const rawParams = { - state: JSON.stringify({ - nonce, // <-- NEW - redirect_url: redirectUrl, - access_token_object: { ... }, - channel_id: knockSlackChannelId, - public_key: knock.apiKey, - user_token: knock.userToken, - branch_slug: knock.branch, - }), - client_id: slackClientId, - scope: combinedScopes.join(","), -}; -``` - -Same change in `useMsTeamsAuth.ts`. - -#### B. Update `useAuthPostMessageListener.ts` to verify the nonce - -The postMessage data format needs to change from a bare string to a structured object. The listener should: - -```typescript -const receiveMessage = (event: MessageEvent) => { - if (event.origin !== knockHost) return; - - // Support both old format (string) and new format (object with nonce) - const messageType = typeof event.data === "string" - ? event.data - : event.data?.type; - const returnedNonce = typeof event.data === "object" - ? event.data?.nonce - : undefined; - - if (messageType === "authComplete") { - // Verify nonce if one was stored (and one was returned) - if (returnedNonce !== undefined) { - const storedNonce = sessionStorage.getItem(`knock:${provider}-auth-nonce:${channelId}`); - if (storedNonce && storedNonce !== returnedNonce) { - setConnectionStatus("error"); - return; - } - sessionStorage.removeItem(`knock:${provider}-auth-nonce:${channelId}`); - } - - setConnectionStatus("connected"); - onAuthenticationComplete?.(messageType); - // ... close popup - } -}; -``` - -The listener needs to accept a `channelId` (or sessionStorage key) so it knows which nonce to look up. This means updating `UseAuthPostMessageListenerOptions` to include the relevant context. - -#### C. Expose the nonce for headless users - -For customers using `useSlackAuth` headlessly (not using `SlackAuthButton` or the postMessage listener), they need to be able to access the nonce to verify it themselves. Two options: - -- **Option 1**: Return it from `buildSlackAuthUrl` — change the return type from `string` to `{ url: string; nonce: string }` (breaking change) -- **Option 2** (recommended): Keep `buildSlackAuthUrl` returning a string, but add a separate `getAuthNonce()` function to the hook's return value, or document that customers can read it from `sessionStorage` at the known key - -#### D. Polling path (`useAuthPolling.ts`) - -The polling mechanism independently checks auth status via `authCheck()` — it doesn't go through the OAuth redirect. It doesn't need nonce verification directly, since it's just checking "did the token get stored?" However, we could optionally verify the nonce through the authCheck response if the API returns it. - -### 2. API Changes (switchboard repo) - -#### A. `/providers/slack/authenticate` — echo the nonce in postMessage - -This is the critical API change. The endpoint currently sends: - -```javascript -postMessage("authComplete", "*") // or "authFailed" -``` - -It needs to change to: - -```javascript -// Extract nonce from the state parameter -const state = JSON.parse(params["state"]) -const nonce = state["nonce"] - -// Send structured data instead of bare string -postMessage({ type: "authComplete", nonce: nonce }, redirect_origin) -``` - -This must be backward compatible: if no nonce is in the state, send the old format or send `{ type: "authComplete" }` with no nonce field. The SDK's updated listener handles both formats. - -#### B. Same change for `/providers/ms-teams/authenticate` - -The MS Teams callback endpoint needs the identical treatment. - -#### C. Tighten postMessage target origin (optional but recommended) - -If the API is currently using `"*"` as the target origin for `postMessage`, it should be tightened to the `redirect_url` origin from the state parameter. This prevents the auth result from being intercepted by other windows. - ---- - -## Summary of Changes by File - -| File | Repo | Change | -|------|------|--------| -| `react-core/.../useSlackAuth.ts` | SDK | Generate nonce, store in sessionStorage, include in state | -| `react-core/.../useMsTeamsAuth.ts` | SDK | Same as above | -| `react-core/.../useAuthPostMessageListener.ts` | SDK | Accept structured postMessage, verify nonce against sessionStorage | -| `react/.../SlackAuthButton.tsx` | SDK | Pass channelId to postMessage listener for nonce lookup | -| `react/.../MsTeamsAuthButton.tsx` | SDK | Same as above | -| `switchboard/.../slack/authenticate` | API | Extract nonce from state, include in postMessage response | -| `switchboard/.../ms-teams/authenticate` | API | Same as above | -| Tests for all above | Both | Update to cover nonce generation, verification, and backward compat | - ---- - -## Migration / Backward Compatibility - -The recommended rollout order: - -1. **Ship SDK first** with nonce generation + listener that accepts both string and object postMessage formats -2. **Ship API second** with nonce echo in postMessage (using object format) -3. Eventually deprecate string-only format - -- The SDK change is backward compatible: it adds a nonce to state, but old API versions that ignore unknown state fields won't break. -- The API change requires care: sending structured postMessage data means old SDK versions that check `event.data === "authComplete"` will fail — so the API should also keep sending the string format during a transition period, or the SDK should be released first with support for both formats. diff --git a/switchboard-csrf-nonce-prompt.md b/switchboard-csrf-nonce-prompt.md deleted file mode 100644 index 83e677d85..000000000 --- a/switchboard-csrf-nonce-prompt.md +++ /dev/null @@ -1,75 +0,0 @@ -# Switchboard: Add CSRF Nonce to Slack OAuth Callback - -## Context - -Our JavaScript SDK (`@knocklabs/react-core`) builds OAuth authorization URLs for Slack that include a `state` parameter. This `state` is a JSON-encoded object used to pass data through the OAuth flow: - -```json -{ - "redirect_url": "https://customer-app.com/callback", - "access_token_object": { "object_id": "tenant_123", "collection": "$tenants" }, - "channel_id": "knock_channel_abc", - "public_key": "pk_test_...", - "user_token": "jwt_...", - "branch_slug": "production" -} -``` - -Slack's app review process now requires a CSRF-preventing nonce in the `state` parameter. We are updating the SDK to generate a random `nonce` field and include it in the `state` JSON. The SDK stores this nonce in `sessionStorage` so it can verify it when the auth flow completes. - -However, the OAuth callback endpoint in Switchboard (`/providers/slack/authenticate`) consumes the `state` parameter server-side and communicates the result back to the client via `postMessage`. Currently the `postMessage` sends a bare string (`"authComplete"` or `"authFailed"`), which means the nonce is lost and the client can never verify it. - -**We need Switchboard to echo the nonce back to the client in the `postMessage` payload.** - -## What to Change - -### 1. Slack OAuth callback — `/providers/slack/authenticate` - -Reference: `lib/switchboard_web/router.ex` line 327 and whatever controller/handler it routes to. - -When processing the OAuth callback: - -1. Parse the `state` JSON from the query params (you likely already do this to extract `redirect_url`, `access_token_object`, etc.) -2. Extract the `nonce` field from the parsed state (it may not be present — old SDK versions won't send it) -3. When sending the `postMessage` back to the parent window, change the data format from a bare string to a structured object that includes the nonce: - -**Before:** -```javascript -window.opener.postMessage("authComplete", "*"); -// or -window.opener.postMessage("authFailed", "*"); -``` - -**After:** -```javascript -window.opener.postMessage({ type: "authComplete", nonce: "" }, ""); -// or -window.opener.postMessage({ type: "authFailed", nonce: "" }, ""); -``` - -If the `nonce` field is not present in the state (backward compat with old SDK versions), omit it from the payload or set it to `null`: -```javascript -window.opener.postMessage({ type: "authComplete", nonce: null }, ""); -``` - -### 2. Tighten `postMessage` target origin (recommended) - -If the `postMessage` calls currently use `"*"` as the target origin, tighten them to use the origin derived from the `redirect_url` in the state parameter. For example, if `redirect_url` is `https://customer-app.com/callback`, the target origin should be `https://customer-app.com`. This prevents the auth result from being interceptable by unrelated windows. - -If `redirect_url` is missing or unparseable, fall back to `"*"`. - -## Backward Compatibility - -- **Old SDK + new API**: The SDK currently checks `event.data === "authComplete"`. Sending `{ type: "authComplete", nonce: ... }` will break this check. The updated SDK (shipping first) will handle both formats. During the transition period where old SDK versions are still in the wild, consider one of: - - **(a)** Accept that old SDK versions will break on this change (they'll fall back to the polling mechanism, which still works independently — so auth will still succeed, just not via the faster postMessage path) - - **(b)** Send both formats: `postMessage("authComplete", ...)` followed by `postMessage({ type: "authComplete", nonce }, ...)` — the old SDK picks up the first, the new SDK picks up the second. This is the safest approach for a smooth rollout. -- **New SDK + old API**: The updated SDK will accept both bare strings and structured objects. If the API hasn't been updated yet, the SDK still works — it just can't verify the nonce (which is fine, it degrades gracefully). - -**Recommendation: use approach (b)** — send both formats — to avoid any disruption for customers on older SDK versions. - -## Testing - -- Verify that a Slack OAuth flow with the new SDK sends a `nonce` in the state and receives it back in the `postMessage` payload -- Verify that a Slack OAuth flow with an old SDK (no `nonce` in state) still works correctly (postMessage sends `nonce: null`) -- Verify that the `authFailed` path also includes the nonce -- If tightening the target origin: verify `postMessage` is received correctly when `redirect_url` is present, and that the `"*"` fallback works when it's absent From 71517e52da46199ddd27de662ffd6704563fdb2f Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 15:30:12 -0400 Subject: [PATCH 09/17] cursor review feedback --- packages/react-core/src/modules/slack/hooks/useSlackAuth.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts b/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts index b71e145fc..393f77a2c 100644 --- a/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts +++ b/packages/react-core/src/modules/slack/hooks/useSlackAuth.ts @@ -118,6 +118,7 @@ function useSlackAuth( tenantId, knockSlackChannelId, knock.apiKey, + knock.userId, knock.userToken, knock.branch, slackClientId, From 692c0ad9396c47ff1ef0931a82841d2ad4ba930d Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 15:32:28 -0400 Subject: [PATCH 10/17] yarn format --- .../slack/components/SlackAuthButton/SlackAuthButton.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx b/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx index d3c34aecc..f4cb9ba1f 100644 --- a/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx +++ b/packages/react/src/modules/slack/components/SlackAuthButton/SlackAuthButton.tsx @@ -65,7 +65,10 @@ export const SlackAuthButton: FunctionComponent = ({ popupWindowRef, setConnectionStatus, onAuthenticationComplete, - nonceStorageKey: getSlackNonceStorageKey(knockSlackChannelId, knock.userId!), + nonceStorageKey: getSlackNonceStorageKey( + knockSlackChannelId, + knock.userId!, + ), }); useAuthPolling({ From 17733f4da633742a45ea9286f0de9807aca6c851 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 15:32:58 -0400 Subject: [PATCH 11/17] changeset --- .changeset/true-plums-switch.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/true-plums-switch.md diff --git a/.changeset/true-plums-switch.md b/.changeset/true-plums-switch.md new file mode 100644 index 000000000..ce26765b0 --- /dev/null +++ b/.changeset/true-plums-switch.md @@ -0,0 +1,6 @@ +--- +"@knocklabs/react-core": patch +"@knocklabs/react": patch +--- + +[Slack] Add support for nonce verification in slack auth From 722836ca9d77f3dd00dfb0acb590da3779745634 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 15:46:14 -0400 Subject: [PATCH 12/17] review feedback --- .../src/modules/core/hooks/useAuthPostMessageListener.ts | 4 ++-- .../test/core/hooks/useAuthPostMessageListener.test.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts index cb6f0bb6b..c4a4a52b9 100644 --- a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts +++ b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts @@ -91,13 +91,13 @@ export function useAuthPostMessageListener( if (nonceStorageKey) { const returnedNonce = getMessageNonce(event.data); const storedNonce = sessionStorage.getItem(nonceStorageKey); + sessionStorage.removeItem(nonceStorageKey); if (!storedNonce || !returnedNonce || storedNonce !== returnedNonce) { - sessionStorage.removeItem(nonceStorageKey); setConnectionStatus("error"); + onAuthenticationComplete?.("authFailed"); closePopup(); return; } - sessionStorage.removeItem(nonceStorageKey); } setConnectionStatus("connected"); diff --git a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts index fc13afa3b..37b022ebd 100644 --- a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts +++ b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts @@ -220,7 +220,7 @@ describe("useAuthPostMessageListener", () => { window.dispatchEvent(event); expect(setConnectionStatus).toHaveBeenCalledWith("error"); - expect(onAuthenticationComplete).not.toHaveBeenCalled(); + expect(onAuthenticationComplete).toHaveBeenCalledWith("authFailed"); expect(mockPopup.close).toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); @@ -246,7 +246,7 @@ describe("useAuthPostMessageListener", () => { window.dispatchEvent(event); expect(setConnectionStatus).toHaveBeenCalledWith("error"); - expect(onAuthenticationComplete).not.toHaveBeenCalled(); + expect(onAuthenticationComplete).toHaveBeenCalledWith("authFailed"); expect(mockPopup.close).toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); @@ -272,7 +272,7 @@ describe("useAuthPostMessageListener", () => { window.dispatchEvent(event); expect(setConnectionStatus).toHaveBeenCalledWith("error"); - expect(onAuthenticationComplete).not.toHaveBeenCalled(); + expect(onAuthenticationComplete).toHaveBeenCalledWith("authFailed"); expect(mockPopup.close).toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); @@ -296,7 +296,7 @@ describe("useAuthPostMessageListener", () => { window.dispatchEvent(event); expect(setConnectionStatus).toHaveBeenCalledWith("error"); - expect(onAuthenticationComplete).not.toHaveBeenCalled(); + expect(onAuthenticationComplete).toHaveBeenCalledWith("authFailed"); expect(mockPopup.close).toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); From cf99f5bcde61eec74a1b292fcf7b8484b3bb6833 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 15:47:13 -0400 Subject: [PATCH 13/17] comment update --- .../src/modules/core/hooks/useAuthPostMessageListener.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts index c4a4a52b9..ed044b991 100644 --- a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts +++ b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts @@ -55,7 +55,7 @@ function getMessageNonce(data: unknown): string | undefined { * popupWindowRef, * setConnectionStatus, * onAuthenticationComplete, - * nonceStorageKey: "knock:slack-auth-nonce:channel_123", + * nonceStorageKey: "knock:slack-auth-nonce:channel_123:user_1", * }); * ``` */ From 6877dd035be7b0bc02905e02bef458bed2f2d259 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 15:57:23 -0400 Subject: [PATCH 14/17] update test nonce --- .../test/core/hooks/useAuthPostMessageListener.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts index 37b022ebd..5d67b6cbb 100644 --- a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts +++ b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts @@ -172,7 +172,7 @@ describe("useAuthPostMessageListener", () => { }); describe("CSRF nonce verification", () => { - const nonceStorageKey = "knock:slack-auth-nonce:channel_123"; + const nonceStorageKey = "knock:slack-auth-nonce:channel_123:user_123"; it("should accept authComplete when nonce matches", () => { mockSessionStorage.setItem(nonceStorageKey, "valid-nonce"); From 6ddf4bbaf67633e7ddbafa9a58251f30d67ae8b0 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 16:55:56 -0400 Subject: [PATCH 15/17] don't close popup --- .../src/modules/core/hooks/useAuthPostMessageListener.ts | 2 +- .../test/core/hooks/useAuthPostMessageListener.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts index ed044b991..3914284a6 100644 --- a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts +++ b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts @@ -109,7 +109,7 @@ export function useAuthPostMessageListener( } setConnectionStatus("error"); onAuthenticationComplete?.(messageType); - closePopup(); + popupWindowRef.current = null; } }; diff --git a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts index 5d67b6cbb..3a103a09d 100644 --- a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts +++ b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts @@ -103,7 +103,7 @@ describe("useAuthPostMessageListener", () => { window.dispatchEvent(event); expect(setConnectionStatus).toHaveBeenCalledWith("error"); - expect(mockPopup.close).toHaveBeenCalled(); + expect(mockPopup.close).not.toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); @@ -124,7 +124,7 @@ describe("useAuthPostMessageListener", () => { window.dispatchEvent(event); expect(setConnectionStatus).toHaveBeenCalledWith("error"); - expect(mockPopup.close).toHaveBeenCalled(); + expect(mockPopup.close).not.toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); @@ -324,7 +324,7 @@ describe("useAuthPostMessageListener", () => { expect(mockSessionStorage.removeItem).toHaveBeenCalledWith( nonceStorageKey, ); - expect(mockPopup.close).toHaveBeenCalled(); + expect(mockPopup.close).not.toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); }); }); From 3dd1becf39d2be8b16b00553c56b349c1409360d Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 19:26:05 -0400 Subject: [PATCH 16/17] bail out if stored nonce was already consumed --- .../src/modules/core/hooks/useAuthPostMessageListener.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts index 3914284a6..cc3cbfaa5 100644 --- a/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts +++ b/packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts @@ -92,7 +92,13 @@ export function useAuthPostMessageListener( const returnedNonce = getMessageNonce(event.data); const storedNonce = sessionStorage.getItem(nonceStorageKey); sessionStorage.removeItem(nonceStorageKey); - if (!storedNonce || !returnedNonce || storedNonce !== returnedNonce) { + + // If nonce already consumed by a prior handler invocation, then bail + // out from checking again. + if ( + !returnedNonce || + (storedNonce && storedNonce !== returnedNonce) + ) { setConnectionStatus("error"); onAuthenticationComplete?.("authFailed"); closePopup(); From 22f5bd02226a337b271eb197753373a081eaec8f Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 10 Apr 2026 19:31:45 -0400 Subject: [PATCH 17/17] fix tests --- .../hooks/useAuthPostMessageListener.test.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts index 3a103a09d..dfaed2a79 100644 --- a/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts +++ b/packages/react-core/test/core/hooks/useAuthPostMessageListener.test.ts @@ -1,5 +1,5 @@ -import { renderHook } from "@testing-library/react"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { cleanup, renderHook } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { useAuthPostMessageListener } from "../../../src/modules/core/hooks/useAuthPostMessageListener"; @@ -27,6 +27,10 @@ describe("useAuthPostMessageListener", () => { let onAuthenticationComplete: ReturnType | undefined; let mockPopup: { close: ReturnType; closed: boolean }; + afterEach(() => { + cleanup(); + }); + beforeEach(() => { vi.clearAllMocks(); mockSessionStorage.clear(); @@ -277,8 +281,9 @@ describe("useAuthPostMessageListener", () => { expect(popupWindowRef.current).toBeNull(); }); - it("should reject authComplete when stored nonce is missing", () => { - // Do NOT store a nonce — simulates cleared storage or replay + it("should treat missing stored nonce as already consumed and accept", () => { + // Do NOT store a nonce — simulates nonce already consumed by a prior + // handler invocation; the code intentionally passes through. renderHook(() => useAuthPostMessageListener({ knockHost, @@ -295,8 +300,8 @@ describe("useAuthPostMessageListener", () => { }); window.dispatchEvent(event); - expect(setConnectionStatus).toHaveBeenCalledWith("error"); - expect(onAuthenticationComplete).toHaveBeenCalledWith("authFailed"); + expect(setConnectionStatus).toHaveBeenCalledWith("connected"); + expect(onAuthenticationComplete).toHaveBeenCalledWith("authComplete"); expect(mockPopup.close).toHaveBeenCalled(); expect(popupWindowRef.current).toBeNull(); });