From 32ead32e833edcf9e56206be9eb602dbc21082fc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 19:16:05 +0000 Subject: [PATCH 01/14] feat: Add enabled prop to KnockProvider Co-authored-by: chris --- .../modules/core/context/KnockProvider.tsx | 30 +++- .../test/core/KnockProvider.test.tsx | 162 ++++++++++++++++++ 2 files changed, 191 insertions(+), 1 deletion(-) diff --git a/packages/react-core/src/modules/core/context/KnockProvider.tsx b/packages/react-core/src/modules/core/context/KnockProvider.tsx index 08447588..177c5c0c 100644 --- a/packages/react-core/src/modules/core/context/KnockProvider.tsx +++ b/packages/react-core/src/modules/core/context/KnockProvider.tsx @@ -26,6 +26,13 @@ export type KnockProviderProps = { i18n?: I18nContent; logLevel?: LogLevel; branch?: string; + /** + * Controls whether the KnockProvider should authenticate and initialize the Knock client. + * When set to false, the provider will skip authentication and just render children. + * This is useful for preventing auth errors when user credentials are not yet available. + * @default true + */ + enabled?: boolean; } & ( | { /** @@ -55,7 +62,11 @@ export type KnockProviderProps = { } ); -export const KnockProvider: React.FC> = ({ +const AuthenticatedKnockProvider: React.FC< + PropsWithChildren< + Omit & { enabled: true | undefined } + > +> = ({ apiKey, host, logLevel, @@ -104,6 +115,23 @@ export const KnockProvider: React.FC> = ({ ); }; +export const KnockProvider: React.FC> = ({ + enabled = true, + children, + ...props +}) => { + // When disabled, skip authentication and just render children + if (!enabled) { + return <>{children}; + } + + return ( + + {children} + + ); +}; + export const useKnockClient = (): Knock => { const context = React.useContext(KnockContext); if (!context) { diff --git a/packages/react-core/test/core/KnockProvider.test.tsx b/packages/react-core/test/core/KnockProvider.test.tsx index 7cfc4376..6f6ffdc4 100644 --- a/packages/react-core/test/core/KnockProvider.test.tsx +++ b/packages/react-core/test/core/KnockProvider.test.tsx @@ -331,4 +331,166 @@ describe("KnockProvider", () => { ); }); }); + + describe("enabled prop", () => { + test("renders children without authentication when enabled is false", () => { + const TestChild = () =>
Child content
; + + const { getByTestId } = render( + + + , + ); + + expect(getByTestId("child")).toHaveTextContent("Child content"); + // Verify no API calls were made (no authentication) + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + }); + + test("authenticates normally when enabled is true (default)", () => { + const TestConsumer = () => { + const knock = useKnockClient(); + return
API Key: {knock.apiKey}
; + }; + + const { getByTestId } = render( + + + , + ); + + expect(getByTestId("consumer-msg")).toHaveTextContent( + "API Key: test_api_key", + ); + }); + + test("authenticates normally when enabled is explicitly true", () => { + const TestConsumer = () => { + const knock = useKnockClient(); + return
API Key: {knock.apiKey}
; + }; + + const { getByTestId } = render( + + + , + ); + + expect(getByTestId("consumer-msg")).toHaveTextContent( + "API Key: test_api_key", + ); + }); + + test("useKnockClient throws error when enabled is false", () => { + const TestConsumer = () => { + const knock = useKnockClient(); + return
API Key: {knock.apiKey}
; + }; + + // Suppress console.error for this test since we expect an error + const originalError = console.error; + console.error = vi.fn(); + + expect(() => + render( + + + , + ), + ).toThrow("useKnockClient must be used within a KnockProvider"); + + console.error = originalError; + }); + + test("toggling enabled from false to true initializes authentication", () => { + const TestChild = () =>
Child content
; + + const { rerender } = render( + + + , + ); + + // Verify no API calls when disabled + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + + // Enable the provider + rerender( + + + , + ); + + // Verify authentication happened + expect(mockApiClient.makeRequest).toHaveBeenCalledWith( + expect.objectContaining({ + method: "PUT", + url: "/v1/users/test_user_id", + data: { name: "John" }, + }), + ); + }); + + test("toggling enabled from true to false stops providing context", () => { + const TestConsumer = () => { + try { + const knock = useKnockClient(); + return
API Key: {knock.apiKey}
; + } catch { + return
No context available
; + } + }; + + const { getByTestId, rerender } = render( + + + , + ); + + // Initially should work + expect(getByTestId("consumer-msg")).toHaveTextContent( + "API Key: test_api_key", + ); + + // Disable the provider + rerender( + + + , + ); + + // Now context should not be available + expect(getByTestId("error-msg")).toHaveTextContent( + "No context available", + ); + }); + }); }); From 37dc85c083989b69b0289a3cc6d22551a62942e1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 19:25:42 +0000 Subject: [PATCH 02/14] Checkpoint before follow-up message Co-authored-by: chris --- .../modules/core/context/KnockProvider.tsx | 7 +++-- .../test/core/KnockProvider.test.tsx | 29 ++++++++++++++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/react-core/src/modules/core/context/KnockProvider.tsx b/packages/react-core/src/modules/core/context/KnockProvider.tsx index 177c5c0c..9cfe7e1c 100644 --- a/packages/react-core/src/modules/core/context/KnockProvider.tsx +++ b/packages/react-core/src/modules/core/context/KnockProvider.tsx @@ -118,15 +118,16 @@ const AuthenticatedKnockProvider: React.FC< export const KnockProvider: React.FC> = ({ enabled = true, children, + i18n, ...props }) => { - // When disabled, skip authentication and just render children + // When disabled, skip authentication but still provide i18n context if (!enabled) { - return <>{children}; + return {children}; } return ( - + {children} ); diff --git a/packages/react-core/test/core/KnockProvider.test.tsx b/packages/react-core/test/core/KnockProvider.test.tsx index 6f6ffdc4..474f4c1e 100644 --- a/packages/react-core/test/core/KnockProvider.test.tsx +++ b/packages/react-core/test/core/KnockProvider.test.tsx @@ -3,7 +3,7 @@ import { cleanup, render } from "@testing-library/react"; import { afterEach, describe, expect, test, vi } from "vitest"; import { createMockKnock } from "../../../client/test/test-utils/mocks"; -import { KnockProvider, useKnockClient } from "../../src"; +import { KnockProvider, useKnockClient, useTranslations } from "../../src"; const TEST_BRANCH_SLUG = "lorem-ipsum-dolor-branch"; @@ -492,5 +492,32 @@ describe("KnockProvider", () => { "No context available", ); }); + + test("i18n context is still available when enabled is false", () => { + const TestChild = () => { + const { t, locale } = useTranslations(); + return ( +
+ Locale: {locale}, Translation: {t("archiveNotification")} +
+ ); + }; + + const { getByTestId } = render( + + + , + ); + + // i18n should still work (with default locale and translations) + expect(getByTestId("i18n-test")).toHaveTextContent("Locale: en"); + expect(getByTestId("i18n-test")).toHaveTextContent( + "Translation: Archive this notification", + ); + }); }); }); From 9398db2e0aea62f0afe3bccb36eeb9ca5747966c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 19:30:07 +0000 Subject: [PATCH 03/14] Refactor KnockProvider to always instantiate Knock client This change ensures that the Knock client is always instantiated, but authentication is conditionally applied based on the `enabled` prop. This simplifies the provider's logic and allows child providers to function even when Knock is disabled. Co-authored-by: chris --- .../modules/core/context/KnockProvider.tsx | 60 ++++---- .../test/core/KnockProvider.test.tsx | 128 ++++++++++++------ 2 files changed, 116 insertions(+), 72 deletions(-) diff --git a/packages/react-core/src/modules/core/context/KnockProvider.tsx b/packages/react-core/src/modules/core/context/KnockProvider.tsx index 9cfe7e1c..a259b066 100644 --- a/packages/react-core/src/modules/core/context/KnockProvider.tsx +++ b/packages/react-core/src/modules/core/context/KnockProvider.tsx @@ -7,7 +7,6 @@ import * as React from "react"; import { PropsWithChildren } from "react"; import { I18nContent, KnockI18nProvider } from "../../i18n"; -import { useAuthenticatedKnockClient } from "../hooks"; export interface KnockProviderState { knock: Knock; @@ -62,11 +61,7 @@ export type KnockProviderProps = { } ); -const AuthenticatedKnockProvider: React.FC< - PropsWithChildren< - Omit & { enabled: true | undefined } - > -> = ({ +export const KnockProvider: React.FC> = ({ apiKey, host, logLevel, @@ -77,6 +72,7 @@ const AuthenticatedKnockProvider: React.FC< i18n, identificationStrategy, branch, + enabled = true, ...props }) => { const userIdOrUserWithProperties = props?.user || props?.userId; @@ -101,35 +97,37 @@ const AuthenticatedKnockProvider: React.FC< ], ); - const knock = useAuthenticatedKnockClient( - apiKey ?? "", - userIdOrUserWithProperties, - userToken, - authenticateOptions, - ); + // Create a Knock client instance - always instantiate, but conditionally authenticate + const knockClient = React.useMemo(() => { + const knock = new Knock(apiKey ?? "", { + host: authenticateOptions.host, + logLevel: authenticateOptions.logLevel, + branch: authenticateOptions.branch, + }); - return ( - - {children} - - ); -}; + // Only authenticate if enabled + if (enabled && userIdOrUserWithProperties) { + knock.authenticate(userIdOrUserWithProperties, userToken, { + onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, + timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, + identificationStrategy: authenticateOptions.identificationStrategy, + }); + } -export const KnockProvider: React.FC> = ({ - enabled = true, - children, - i18n, - ...props -}) => { - // When disabled, skip authentication but still provide i18n context - if (!enabled) { - return {children}; - } + return knock; + }, [apiKey, authenticateOptions, enabled, userIdOrUserWithProperties, userToken]); + + // Cleanup on unmount + React.useEffect(() => { + return () => { + knockClient.teardown(); + }; + }, [knockClient]); return ( - - {children} - + + {children} + ); }; diff --git a/packages/react-core/test/core/KnockProvider.test.tsx b/packages/react-core/test/core/KnockProvider.test.tsx index 474f4c1e..4c770afc 100644 --- a/packages/react-core/test/core/KnockProvider.test.tsx +++ b/packages/react-core/test/core/KnockProvider.test.tsx @@ -3,7 +3,12 @@ import { cleanup, render } from "@testing-library/react"; import { afterEach, describe, expect, test, vi } from "vitest"; import { createMockKnock } from "../../../client/test/test-utils/mocks"; -import { KnockProvider, useKnockClient, useTranslations } from "../../src"; +import { + KnockFeedProvider, + KnockProvider, + useKnockClient, + useTranslations, +} from "../../src"; const TEST_BRANCH_SLUG = "lorem-ipsum-dolor-branch"; @@ -29,6 +34,9 @@ mockApiClient.makeRequest.mockImplementation(async ({ method, url, data }) => { afterEach(() => { cleanup(); vi.clearAllMocks(); + // Reset the knock instance state to prevent test pollution + knock.userId = undefined; + knock.userToken = undefined; }); describe("KnockProvider", () => { @@ -389,45 +397,53 @@ describe("KnockProvider", () => { ); }); - test("useKnockClient throws error when enabled is false", () => { + test("useKnockClient returns unauthenticated client when enabled is false", () => { const TestConsumer = () => { const knock = useKnockClient(); - return
API Key: {knock.apiKey}
; + return ( +
+ API Key: {knock.apiKey}, Authenticated: {String(knock.isAuthenticated())} +
+ ); }; - // Suppress console.error for this test since we expect an error - const originalError = console.error; - console.error = vi.fn(); - - expect(() => - render( - - - , - ), - ).toThrow("useKnockClient must be used within a KnockProvider"); - - console.error = originalError; + const { getByTestId } = render( + + + , + ); + + // Client should exist but not be authenticated + expect(getByTestId("consumer-msg")).toHaveTextContent("API Key: test_api_key"); + expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: false"); }); test("toggling enabled from false to true initializes authentication", () => { - const TestChild = () =>
Child content
; + const TestConsumer = () => { + const knock = useKnockClient(); + return ( +
+ Authenticated: {String(knock.isAuthenticated())} +
+ ); + }; - const { rerender } = render( + const { getByTestId, rerender } = render( - + , ); - // Verify no API calls when disabled + // Should not be authenticated initially + expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: false"); expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); // Enable the provider @@ -437,11 +453,12 @@ describe("KnockProvider", () => { user={{ id: "test_user_id", name: "John" }} enabled={true} > - + , ); - // Verify authentication happened + // Should now be authenticated + expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: true"); expect(mockApiClient.makeRequest).toHaveBeenCalledWith( expect.objectContaining({ method: "PUT", @@ -451,14 +468,14 @@ describe("KnockProvider", () => { ); }); - test("toggling enabled from true to false stops providing context", () => { + test("toggling enabled from true to false creates new unauthenticated client", () => { const TestConsumer = () => { - try { - const knock = useKnockClient(); - return
API Key: {knock.apiKey}
; - } catch { - return
No context available
; - } + const knock = useKnockClient(); + return ( +
+ API Key: {knock.apiKey}, Authenticated: {String(knock.isAuthenticated())} +
+ ); }; const { getByTestId, rerender } = render( @@ -471,10 +488,12 @@ describe("KnockProvider", () => { , ); - // Initially should work - expect(getByTestId("consumer-msg")).toHaveTextContent( - "API Key: test_api_key", - ); + // Initially authenticated + expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: true"); + + // Clear the authenticated state for the mock (simulating a new instance) + knock.userId = undefined; + knock.userToken = undefined; // Disable the provider rerender( @@ -487,10 +506,9 @@ describe("KnockProvider", () => { , ); - // Now context should not be available - expect(getByTestId("error-msg")).toHaveTextContent( - "No context available", - ); + // Client still exists but not authenticated (because we created a new instance) + expect(getByTestId("consumer-msg")).toHaveTextContent("API Key: test_api_key"); + expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: false"); }); test("i18n context is still available when enabled is false", () => { @@ -519,5 +537,33 @@ describe("KnockProvider", () => { "Translation: Archive this notification", ); }); + + test("child providers can render when enabled is false", () => { + const TestChild = () => { + const knock = useKnockClient(); + return ( +
+ Authenticated: {String(knock.isAuthenticated())} +
+ ); + }; + + const { getByTestId } = render( + + + + + , + ); + + // Child provider should render successfully + expect(getByTestId("feed-test")).toHaveTextContent("Authenticated: false"); + // No API requests should be made + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + }); }); }); From 54024ee50bc0b6888d46b9fd3154ee84dcf2fbc8 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 19:39:22 +0000 Subject: [PATCH 04/14] feat: Add resetAuthentication to Knock client and provider Co-authored-by: chris --- packages/client/src/knock.ts | 19 +++++ packages/client/test/knock.test.ts | 54 +++++++++++++ .../modules/core/context/KnockProvider.tsx | 81 ++++++++++++++++--- .../test/core/KnockProvider.test.tsx | 50 +++++++----- 4 files changed, 173 insertions(+), 31 deletions(-) diff --git a/packages/client/src/knock.ts b/packages/client/src/knock.ts index 7fb4fdb3..d83f1f9c 100644 --- a/packages/client/src/knock.ts +++ b/packages/client/src/knock.ts @@ -165,6 +165,25 @@ class Knock { return checkUserToken ? !!(this.userId && this.userToken) : !!this.userId; } + /* + Resets the authentication state and tears down any active connections. + This is useful when you want to clear the current user session without destroying the Knock instance. + */ + resetAuthentication() { + this.log("Resetting authentication state"); + + // Teardown any active feeds and connections + this.feeds.teardownInstances(); + this.teardown(); + + // Clear authentication state + this.userId = undefined; + this.userToken = undefined; + + // Reinitialize the API client without credentials + this.apiClient = this.createApiClient(); + } + // Used to teardown any connected instances teardown() { if (this.tokenExpirationTimer) { diff --git a/packages/client/test/knock.test.ts b/packages/client/test/knock.test.ts index ec9dd5d9..b53afd36 100644 --- a/packages/client/test/knock.test.ts +++ b/packages/client/test/knock.test.ts @@ -772,4 +772,58 @@ describe("Knock Client", () => { expect(knock.isAuthenticated(true)).toBe(true); }); }); + + describe("resetAuthentication", () => { + test("clears userId and userToken", () => { + const knock = new Knock("pk_test_12345"); + + knock.authenticate("user_123", "token_456"); + expect(knock.isAuthenticated()).toBe(true); + expect(knock.userId).toBe("user_123"); + expect(knock.userToken).toBe("token_456"); + + knock.resetAuthentication(); + + expect(knock.isAuthenticated()).toBe(false); + expect(knock.userId).toBeUndefined(); + expect(knock.userToken).toBeUndefined(); + }); + + test("tears down feed instances", () => { + const knock = new Knock("pk_test_12345"); + knock.authenticate("user_123", "token_456"); + + const teardownSpy = vi.spyOn(knock.feeds, "teardownInstances"); + + knock.resetAuthentication(); + + expect(teardownSpy).toHaveBeenCalled(); + }); + + test("calls teardown to clean up connections", () => { + const knock = new Knock("pk_test_12345"); + knock.authenticate("user_123", "token_456"); + + const teardownSpy = vi.spyOn(knock, "teardown"); + + knock.resetAuthentication(); + + expect(teardownSpy).toHaveBeenCalled(); + }); + + test("can re-authenticate after reset", () => { + const knock = new Knock("pk_test_12345"); + + knock.authenticate("user_123", "token_456"); + expect(knock.userId).toBe("user_123"); + + knock.resetAuthentication(); + expect(knock.isAuthenticated()).toBe(false); + + knock.authenticate("user_456", "token_789"); + expect(knock.isAuthenticated()).toBe(true); + expect(knock.userId).toBe("user_456"); + expect(knock.userToken).toBe("token_789"); + }); + }); }); diff --git a/packages/react-core/src/modules/core/context/KnockProvider.tsx b/packages/react-core/src/modules/core/context/KnockProvider.tsx index a259b066..c11f4ceb 100644 --- a/packages/react-core/src/modules/core/context/KnockProvider.tsx +++ b/packages/react-core/src/modules/core/context/KnockProvider.tsx @@ -97,25 +97,80 @@ export const KnockProvider: React.FC> = ({ ], ); - // Create a Knock client instance - always instantiate, but conditionally authenticate - const knockClient = React.useMemo(() => { - const knock = new Knock(apiKey ?? "", { - host: authenticateOptions.host, - logLevel: authenticateOptions.logLevel, - branch: authenticateOptions.branch, - }); - - // Only authenticate if enabled - if (enabled && userIdOrUserWithProperties) { - knock.authenticate(userIdOrUserWithProperties, userToken, { + // Create and manage Knock client instance + const [forceUpdate, setForceUpdate] = React.useState(0); + const knockClient = React.useMemo( + () => { + const knock = new Knock(apiKey ?? "", { + host: authenticateOptions.host, + logLevel: authenticateOptions.logLevel, + branch: authenticateOptions.branch, + }); + + // Authenticate synchronously on creation if enabled + if (enabled && userIdOrUserWithProperties) { + knock.authenticate(userIdOrUserWithProperties, userToken, { + onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, + timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, + identificationStrategy: authenticateOptions.identificationStrategy, + }); + } + + return knock; + }, + // We intentionally omit enabled, userIdOrUserWithProperties, and userToken + // to reuse the same instance when these change. Auth state changes are handled + // in the effect below via resetAuthentication()/authenticate() calls. + // eslint-disable-next-line react-hooks/exhaustive-deps + [apiKey, authenticateOptions, forceUpdate], + ); + + // Track previous enabled state to detect transitions + const prevEnabledRef = React.useRef(enabled); + + React.useEffect(() => { + const wasEnabled = prevEnabledRef.current; + const isEnabled = enabled; + + if (wasEnabled && !isEnabled && knockClient.isAuthenticated()) { + // Transitioning from enabled→disabled: reset auth and force update + knockClient.resetAuthentication(); + setForceUpdate((n) => n + 1); + } else if (!wasEnabled && isEnabled && userIdOrUserWithProperties) { + // Transitioning from disabled→enabled: authenticate + knockClient.authenticate(userIdOrUserWithProperties, userToken, { onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, identificationStrategy: authenticateOptions.identificationStrategy, }); + setForceUpdate((n) => n + 1); + } else if (isEnabled && userIdOrUserWithProperties) { + // If enabled and user/token changed, reauthenticate + const userId = + typeof userIdOrUserWithProperties === "string" + ? userIdOrUserWithProperties + : userIdOrUserWithProperties?.id; + + if ( + knockClient.userId !== userId || + knockClient.userToken !== userToken + ) { + knockClient.authenticate(userIdOrUserWithProperties, userToken, { + onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, + timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, + identificationStrategy: authenticateOptions.identificationStrategy, + }); + } } - return knock; - }, [apiKey, authenticateOptions, enabled, userIdOrUserWithProperties, userToken]); + prevEnabledRef.current = enabled; + }, [ + enabled, + knockClient, + userIdOrUserWithProperties, + userToken, + authenticateOptions, + ]); // Cleanup on unmount React.useEffect(() => { diff --git a/packages/react-core/test/core/KnockProvider.test.tsx b/packages/react-core/test/core/KnockProvider.test.tsx index 4c770afc..3e05b233 100644 --- a/packages/react-core/test/core/KnockProvider.test.tsx +++ b/packages/react-core/test/core/KnockProvider.test.tsx @@ -1,5 +1,5 @@ import "@testing-library/jest-dom/vitest"; -import { cleanup, render } from "@testing-library/react"; +import { cleanup, render, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, test, vi } from "vitest"; import { createMockKnock } from "../../../client/test/test-utils/mocks"; @@ -290,7 +290,7 @@ describe("KnockProvider", () => { ); }); - test("changing identification strategy from skip to inline enables identification", () => { + test("changing identification strategy from skip to inline enables identification", async () => { const TestConsumer = () => { const knock = useKnockClient(); return
User Id: {knock.userId}
; @@ -329,14 +329,16 @@ describe("KnockProvider", () => { , ); - // Verify identify is now called - expect(mockApiClient.makeRequest).toHaveBeenCalledWith( - expect.objectContaining({ - method: "PUT", - url: "/v1/users/test_user_id", - data: { name: "John Doe", email: "john@example.com" }, - }), - ); + // Wait for the effect to complete + await waitFor(() => { + expect(mockApiClient.makeRequest).toHaveBeenCalledWith( + expect.objectContaining({ + method: "PUT", + url: "/v1/users/test_user_id", + data: { name: "John Doe", email: "john@example.com" }, + }), + ); + }); }); }); @@ -422,7 +424,7 @@ describe("KnockProvider", () => { expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: false"); }); - test("toggling enabled from false to true initializes authentication", () => { + test("toggling enabled from false to true initializes authentication", async () => { const TestConsumer = () => { const knock = useKnockClient(); return ( @@ -457,8 +459,11 @@ describe("KnockProvider", () => { , ); - // Should now be authenticated - expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: true"); + // Wait for the effect to complete + await waitFor(() => { + expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: true"); + }); + expect(mockApiClient.makeRequest).toHaveBeenCalledWith( expect.objectContaining({ method: "PUT", @@ -468,7 +473,7 @@ describe("KnockProvider", () => { ); }); - test("toggling enabled from true to false creates new unauthenticated client", () => { + test("toggling enabled from true to false calls resetAuthentication", async () => { const TestConsumer = () => { const knock = useKnockClient(); return ( @@ -491,9 +496,13 @@ describe("KnockProvider", () => { // Initially authenticated expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: true"); - // Clear the authenticated state for the mock (simulating a new instance) - knock.userId = undefined; - knock.userToken = undefined; + // Spy on resetAuthentication and have it actually clear the auth state + const resetAuthSpy = vi.spyOn(knock, "resetAuthentication").mockImplementation(() => { + knock.userId = undefined; + knock.userToken = undefined; + knock.feeds.teardownInstances(); + knock.teardown(); + }); // Disable the provider rerender( @@ -506,7 +515,12 @@ describe("KnockProvider", () => { , ); - // Client still exists but not authenticated (because we created a new instance) + // Wait for the effect to complete + await waitFor(() => { + expect(resetAuthSpy).toHaveBeenCalled(); + }); + + // Client still exists but not authenticated expect(getByTestId("consumer-msg")).toHaveTextContent("API Key: test_api_key"); expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: false"); }); From 26d1904f627b770d8b09d4c2b1d884a7f6afdde1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 19:44:47 +0000 Subject: [PATCH 05/14] feat: Add authentication checks to feed methods Co-authored-by: chris --- packages/client/src/clients/feed/feed.ts | 65 ++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/packages/client/src/clients/feed/feed.ts b/packages/client/src/clients/feed/feed.ts index 21cff1c1..d0c7e59d 100644 --- a/packages/client/src/clients/feed/feed.ts +++ b/packages/client/src/clients/feed/feed.ts @@ -176,6 +176,11 @@ class Feed { } async markAsSeen(itemOrItems: FeedItemOrItems) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAsSeen - user not authenticated"); + return { entries: [] }; + } + const now = new Date().toISOString(); this.optimisticallyPerformStatusUpdate( itemOrItems, @@ -188,6 +193,11 @@ class Feed { } async markAllAsSeen() { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAllAsSeen - user not authenticated"); + return { entries: [] }; + } + // To mark all of the messages as seen we: // 1. Optimistically update *everything* we have in the store // 2. We decrement the `unseen_count` to zero optimistically @@ -230,6 +240,11 @@ class Feed { } async markAsUnseen(itemOrItems: FeedItemOrItems) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAsUnseen - user not authenticated"); + return { entries: [] }; + } + this.optimisticallyPerformStatusUpdate( itemOrItems, "unseen", @@ -241,6 +256,11 @@ class Feed { } async markAsRead(itemOrItems: FeedItemOrItems) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAsRead - user not authenticated"); + return { entries: [] }; + } + const now = new Date().toISOString(); this.optimisticallyPerformStatusUpdate( itemOrItems, @@ -253,6 +273,11 @@ class Feed { } async markAllAsRead() { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAllAsRead - user not authenticated"); + return { entries: [] }; + } + // To mark all of the messages as read we: // 1. Optimistically update *everything* we have in the store // 2. We decrement the `unread_count` to zero optimistically @@ -295,6 +320,11 @@ class Feed { } async markAsUnread(itemOrItems: FeedItemOrItems) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAsUnread - user not authenticated"); + return { entries: [] }; + } + this.optimisticallyPerformStatusUpdate( itemOrItems, "unread", @@ -309,6 +339,11 @@ class Feed { itemOrItems: FeedItemOrItems, metadata?: Record, ) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAsInteracted - user not authenticated"); + return { entries: [] }; + } + const now = new Date().toISOString(); this.optimisticallyPerformStatusUpdate( itemOrItems, @@ -332,6 +367,11 @@ class Feed { TODO: how do we handle rollbacks? */ async markAsArchived(itemOrItems: FeedItemOrItems) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAsArchived - user not authenticated"); + return { entries: [] }; + } + const state = this.store.getState(); const shouldOptimisticallyRemoveItems = @@ -403,6 +443,11 @@ class Feed { } async markAllAsArchived() { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAllAsArchived - user not authenticated"); + return { entries: [] }; + } + // Note: there is the potential for a race condition here because the bulk // update is an async method, so if a new message comes in during this window before // the update has been processed we'll effectively reset the `unseen_count` to be what it was. @@ -430,6 +475,11 @@ class Feed { } async markAllReadAsArchived() { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAllReadAsArchived - user not authenticated"); + return { entries: [] }; + } + // Note: there is the potential for a race condition here because the bulk // update is an async method, so if a new message comes in during this window before // the update has been processed we'll effectively reset the `unseen_count` to be what it was. @@ -472,6 +522,11 @@ class Feed { } async markAsUnarchived(itemOrItems: FeedItemOrItems) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping markAsUnarchived - user not authenticated"); + return { entries: [] }; + } + const state = this.store.getState(); const items = Array.isArray(itemOrItems) ? itemOrItems : [itemOrItems]; @@ -518,6 +573,11 @@ class Feed { /* Fetches the feed content, appending it to the store */ async fetch(options: FetchFeedOptions = {}) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping fetch - user not authenticated"); + return; + } + const { networkStatus, ...state } = this.store.getState(); // If the user is not authenticated, then do nothing @@ -608,6 +668,11 @@ class Feed { } async fetchNextPage(options: FetchFeedOptions = {}) { + if (!this.knock.isAuthenticated()) { + this.knock.log("[Feed] Skipping fetchNextPage - user not authenticated"); + return; + } + // Attempts to fetch the next page of results (if we have any) const { pageInfo } = this.store.getState(); From c1f0e2c6b203a05fea386bda8d8d9b92542891fe Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 19:50:17 +0000 Subject: [PATCH 06/14] Refactor: Improve logging and authentication logic This commit refactors the logging within the feed client to ensure consistency and improves the authentication logic in the Knock client and KnockProvider. Co-authored-by: chris --- packages/client/src/clients/feed/feed.ts | 16 ++++++++++++---- packages/client/src/knock.ts | 6 +++--- .../src/modules/core/context/KnockProvider.tsx | 6 ++++-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/client/src/clients/feed/feed.ts b/packages/client/src/clients/feed/feed.ts index d0c7e59d..49da2435 100644 --- a/packages/client/src/clients/feed/feed.ts +++ b/packages/client/src/clients/feed/feed.ts @@ -340,7 +340,9 @@ class Feed { metadata?: Record, ) { if (!this.knock.isAuthenticated()) { - this.knock.log("[Feed] Skipping markAsInteracted - user not authenticated"); + this.knock.log( + "[Feed] Skipping markAsInteracted - user not authenticated", + ); return { entries: [] }; } @@ -444,7 +446,9 @@ class Feed { async markAllAsArchived() { if (!this.knock.isAuthenticated()) { - this.knock.log("[Feed] Skipping markAllAsArchived - user not authenticated"); + this.knock.log( + "[Feed] Skipping markAllAsArchived - user not authenticated", + ); return { entries: [] }; } @@ -476,7 +480,9 @@ class Feed { async markAllReadAsArchived() { if (!this.knock.isAuthenticated()) { - this.knock.log("[Feed] Skipping markAllReadAsArchived - user not authenticated"); + this.knock.log( + "[Feed] Skipping markAllReadAsArchived - user not authenticated", + ); return { entries: [] }; } @@ -523,7 +529,9 @@ class Feed { async markAsUnarchived(itemOrItems: FeedItemOrItems) { if (!this.knock.isAuthenticated()) { - this.knock.log("[Feed] Skipping markAsUnarchived - user not authenticated"); + this.knock.log( + "[Feed] Skipping markAsUnarchived - user not authenticated", + ); return { entries: [] }; } diff --git a/packages/client/src/knock.ts b/packages/client/src/knock.ts index d83f1f9c..2d176a11 100644 --- a/packages/client/src/knock.ts +++ b/packages/client/src/knock.ts @@ -171,15 +171,15 @@ class Knock { */ resetAuthentication() { this.log("Resetting authentication state"); - + // Teardown any active feeds and connections this.feeds.teardownInstances(); this.teardown(); - + // Clear authentication state this.userId = undefined; this.userToken = undefined; - + // Reinitialize the API client without credentials this.apiClient = this.createApiClient(); } diff --git a/packages/react-core/src/modules/core/context/KnockProvider.tsx b/packages/react-core/src/modules/core/context/KnockProvider.tsx index c11f4ceb..eaa5ebc8 100644 --- a/packages/react-core/src/modules/core/context/KnockProvider.tsx +++ b/packages/react-core/src/modules/core/context/KnockProvider.tsx @@ -111,7 +111,8 @@ export const KnockProvider: React.FC> = ({ if (enabled && userIdOrUserWithProperties) { knock.authenticate(userIdOrUserWithProperties, userToken, { onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, - timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, + timeBeforeExpirationInMs: + authenticateOptions.timeBeforeExpirationInMs, identificationStrategy: authenticateOptions.identificationStrategy, }); } @@ -157,7 +158,8 @@ export const KnockProvider: React.FC> = ({ ) { knockClient.authenticate(userIdOrUserWithProperties, userToken, { onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, - timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, + timeBeforeExpirationInMs: + authenticateOptions.timeBeforeExpirationInMs, identificationStrategy: authenticateOptions.identificationStrategy, }); } From 35812bc69a45fec9b140ee56f5b48bcd6034561e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 19:57:24 +0000 Subject: [PATCH 07/14] Refactor: Use isAuthenticated instead of failIfNotAuthenticated Co-authored-by: chris --- packages/client/src/clients/guide/client.ts | 35 +++++++++++++++++-- .../client/test/clients/guide/guide.test.ts | 8 ++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index 9c089521..86bbc5db 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -350,7 +350,14 @@ export class KnockGuideClient { async fetch(opts?: { filters?: QueryFilterParams }) { this.knock.log("[Guide] .fetch"); - this.knock.failIfNotAuthenticated(); + + if (!this.knock.isAuthenticated()) { + this.knock.log("[Guide] Skipping fetch - user not authenticated"); + return { + status: "error" as const, + error: new Error("Not authenticated"), + }; + } const queryParams = this.buildQueryParams(opts?.filters); const queryKey = this.formatQueryKey(queryParams); @@ -400,7 +407,12 @@ export class KnockGuideClient { subscribe() { if (!this.socket) return; - this.knock.failIfNotAuthenticated(); + + if (!this.knock.isAuthenticated()) { + this.knock.log("[Guide] Skipping subscribe - user not authenticated"); + return; + } + this.knock.log("[Guide] Subscribing to real time updates"); // Ensure a live socket connection if not yet connected. @@ -846,6 +858,11 @@ export class KnockGuideClient { async markAsSeen(guide: GuideData, step: GuideStepData) { if (step.message.seen_at) return; + if (!this.knock.isAuthenticated()) { + this.knock.log("[Guide] Skipping markAsSeen - user not authenticated"); + return; + } + this.knock.log( `[Guide] Marking as seen (Guide key: ${guide.key}, Step ref:${step.ref})`, ); @@ -874,6 +891,13 @@ export class KnockGuideClient { step: GuideStepData, metadata?: GenericData, ) { + if (!this.knock.isAuthenticated()) { + this.knock.log( + "[Guide] Skipping markAsInteracted - user not authenticated", + ); + return; + } + this.knock.log( `[Guide] Marking as interacted (Guide key: ${guide.key}; Step ref:${step.ref})`, ); @@ -901,6 +925,13 @@ export class KnockGuideClient { async markAsArchived(guide: GuideData, step: GuideStepData) { if (step.message.archived_at) return; + if (!this.knock.isAuthenticated()) { + this.knock.log( + "[Guide] Skipping markAsArchived - user not authenticated", + ); + return; + } + this.knock.log( `[Guide] Marking as archived (Guide key: ${guide.key}, Step ref:${step.ref})`, ); diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index 999c9384..cb880259 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -271,7 +271,7 @@ describe("KnockGuideClient", () => { await client.fetch(); - expect(mockKnock.failIfNotAuthenticated).toHaveBeenCalled(); + expect(mockKnock.isAuthenticated).toHaveBeenCalled(); expect(mockKnock.user.getGuides).toHaveBeenCalledWith( channelId, expect.objectContaining({ @@ -288,7 +288,7 @@ describe("KnockGuideClient", () => { const client = new KnockGuideClient(mockKnock, channelId); await client.fetch(); - expect(mockKnock.failIfNotAuthenticated).toHaveBeenCalled(); + expect(mockKnock.isAuthenticated).toHaveBeenCalled(); expect(mockStore.setState).toHaveBeenCalledWith(expect.any(Function)); // Get the last setState call and execute its function @@ -343,7 +343,7 @@ describe("KnockGuideClient", () => { ); client.subscribe(); - expect(mockKnock.failIfNotAuthenticated).toHaveBeenCalled(); + expect(mockKnock.isAuthenticated).toHaveBeenCalled(); expect(mockSocket.channel).toHaveBeenCalledWith( `guides:${channelId}`, expect.objectContaining({ @@ -3050,7 +3050,7 @@ describe("KnockGuideClient", () => { await client.fetch({ filters: { type: "tooltip" } }); - expect(mockKnock.failIfNotAuthenticated).toHaveBeenCalled(); + expect(mockKnock.isAuthenticated).toHaveBeenCalled(); expect(mockKnock.user.getGuides).toHaveBeenCalledWith( channelId, expect.objectContaining({ From b2d62dbd582b829e8e5811fe62a9b4ecadd84efb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 20:02:05 +0000 Subject: [PATCH 08/14] feat: Add authentication checks to MS Teams and Slack clients Co-authored-by: chris --- packages/client/src/clients/ms-teams/index.ts | 28 +++++++++++++++++++ packages/client/src/clients/slack/index.ts | 19 +++++++++++++ 2 files changed, 47 insertions(+) diff --git a/packages/client/src/clients/ms-teams/index.ts b/packages/client/src/clients/ms-teams/index.ts index e0da5a5c..6d7eeeea 100644 --- a/packages/client/src/clients/ms-teams/index.ts +++ b/packages/client/src/clients/ms-teams/index.ts @@ -18,6 +18,13 @@ class MsTeamsClient { } async authCheck({ tenant: tenantId, knockChannelId }: AuthCheckInput) { + if (!this.instance.isAuthenticated()) { + this.instance.log( + "[MS Teams] Skipping authCheck - user not authenticated", + ); + return { status: "not_connected" }; + } + const result = await this.instance.client().makeRequest({ method: "GET", url: `/v1/providers/ms-teams/${knockChannelId}/auth_check`, @@ -36,6 +43,13 @@ class MsTeamsClient { async getTeams( input: GetMsTeamsTeamsInput, ): Promise { + if (!this.instance.isAuthenticated()) { + this.instance.log( + "[MS Teams] Skipping getTeams - user not authenticated", + ); + return { ms_teams_teams: [], skip_token: null }; + } + const { knockChannelId, tenant: tenantId } = input; const queryOptions = input.queryOptions || {}; @@ -62,6 +76,13 @@ class MsTeamsClient { async getChannels( input: GetMsTeamsChannelsInput, ): Promise { + if (!this.instance.isAuthenticated()) { + this.instance.log( + "[MS Teams] Skipping getChannels - user not authenticated", + ); + return { ms_teams_channels: [] }; + } + const { knockChannelId, teamId, tenant: tenantId } = input; const queryOptions = input.queryOptions || {}; @@ -88,6 +109,13 @@ class MsTeamsClient { tenant: tenantId, knockChannelId, }: RevokeAccessTokenInput) { + if (!this.instance.isAuthenticated()) { + this.instance.log( + "[MS Teams] Skipping revokeAccessToken - user not authenticated", + ); + return { status: "success" }; + } + const result = await this.instance.client().makeRequest({ method: "PUT", url: `/v1/providers/ms-teams/${knockChannelId}/revoke_access`, diff --git a/packages/client/src/clients/slack/index.ts b/packages/client/src/clients/slack/index.ts index 1ad308db..6b5d868c 100644 --- a/packages/client/src/clients/slack/index.ts +++ b/packages/client/src/clients/slack/index.ts @@ -13,6 +13,11 @@ class SlackClient { } async authCheck({ tenant, knockChannelId }: AuthCheckInput) { + if (!this.instance.isAuthenticated()) { + this.instance.log("[Slack] Skipping authCheck - user not authenticated"); + return { status: "not_connected" }; + } + const result = await this.instance.client().makeRequest({ method: "GET", url: `/v1/providers/slack/${knockChannelId}/auth_check`, @@ -31,6 +36,13 @@ class SlackClient { async getChannels( input: GetSlackChannelsInput, ): Promise { + if (!this.instance.isAuthenticated()) { + this.instance.log( + "[Slack] Skipping getChannels - user not authenticated", + ); + return { slack_channels: [], next_cursor: null }; + } + const { knockChannelId, tenant } = input; const queryOptions = input.queryOptions || {}; @@ -57,6 +69,13 @@ class SlackClient { } async revokeAccessToken({ tenant, knockChannelId }: RevokeAccessTokenInput) { + if (!this.instance.isAuthenticated()) { + this.instance.log( + "[Slack] Skipping revokeAccessToken - user not authenticated", + ); + return { status: "success" }; + } + const result = await this.instance.client().makeRequest({ method: "PUT", url: `/v1/providers/slack/${knockChannelId}/revoke_access`, From 11a145272292b3406d1f63a0d4acd6837f3becf3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 20:06:52 +0000 Subject: [PATCH 09/14] Refactor KnockProvider authentication logic Co-authored-by: chris --- .../modules/core/context/KnockProvider.tsx | 43 +++++-------------- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/packages/react-core/src/modules/core/context/KnockProvider.tsx b/packages/react-core/src/modules/core/context/KnockProvider.tsx index eaa5ebc8..fad5308a 100644 --- a/packages/react-core/src/modules/core/context/KnockProvider.tsx +++ b/packages/react-core/src/modules/core/context/KnockProvider.tsx @@ -126,46 +126,23 @@ export const KnockProvider: React.FC> = ({ [apiKey, authenticateOptions, forceUpdate], ); - // Track previous enabled state to detect transitions - const prevEnabledRef = React.useRef(enabled); - + // Handle authentication state based on enabled prop React.useEffect(() => { - const wasEnabled = prevEnabledRef.current; - const isEnabled = enabled; - - if (wasEnabled && !isEnabled && knockClient.isAuthenticated()) { - // Transitioning from enabled→disabled: reset auth and force update - knockClient.resetAuthentication(); - setForceUpdate((n) => n + 1); - } else if (!wasEnabled && isEnabled && userIdOrUserWithProperties) { - // Transitioning from disabled→enabled: authenticate + if (enabled && userIdOrUserWithProperties) { + // When enabled, authenticate (or re-authenticate if user/token changed) + // Note: authenticate() handles re-auth internally if userId/userToken changed knockClient.authenticate(userIdOrUserWithProperties, userToken, { onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, - timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, + timeBeforeExpirationInMs: + authenticateOptions.timeBeforeExpirationInMs, identificationStrategy: authenticateOptions.identificationStrategy, }); setForceUpdate((n) => n + 1); - } else if (isEnabled && userIdOrUserWithProperties) { - // If enabled and user/token changed, reauthenticate - const userId = - typeof userIdOrUserWithProperties === "string" - ? userIdOrUserWithProperties - : userIdOrUserWithProperties?.id; - - if ( - knockClient.userId !== userId || - knockClient.userToken !== userToken - ) { - knockClient.authenticate(userIdOrUserWithProperties, userToken, { - onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, - timeBeforeExpirationInMs: - authenticateOptions.timeBeforeExpirationInMs, - identificationStrategy: authenticateOptions.identificationStrategy, - }); - } + } else if (!enabled && knockClient.isAuthenticated()) { + // When disabled, reset authentication if currently authenticated + knockClient.resetAuthentication(); + setForceUpdate((n) => n + 1); } - - prevEnabledRef.current = enabled; }, [ enabled, knockClient, From 108ea8dfd434376218f5aa2f7ca09628d9808591 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 20:10:55 +0000 Subject: [PATCH 10/14] Fix: Correct KnockProvider authenticate options formatting Co-authored-by: chris --- packages/react-core/src/modules/core/context/KnockProvider.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/react-core/src/modules/core/context/KnockProvider.tsx b/packages/react-core/src/modules/core/context/KnockProvider.tsx index fad5308a..b789569d 100644 --- a/packages/react-core/src/modules/core/context/KnockProvider.tsx +++ b/packages/react-core/src/modules/core/context/KnockProvider.tsx @@ -133,8 +133,7 @@ export const KnockProvider: React.FC> = ({ // Note: authenticate() handles re-auth internally if userId/userToken changed knockClient.authenticate(userIdOrUserWithProperties, userToken, { onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, - timeBeforeExpirationInMs: - authenticateOptions.timeBeforeExpirationInMs, + timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, identificationStrategy: authenticateOptions.identificationStrategy, }); setForceUpdate((n) => n + 1); From d388aa59aacd382290799f3e137c4f19d8cffc7e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 20:17:47 +0000 Subject: [PATCH 11/14] Add authentication checks to client methods Co-authored-by: chris --- packages/client/src/clients/messages/index.ts | 28 +++++++++++++++++++ packages/client/src/clients/users/index.ts | 14 ++++++++-- .../client/test/clients/users/users.test.ts | 8 ++---- .../KnockExpoPushNotificationProvider.tsx | 13 ++++++++- .../push/KnockPushNotificationProvider.tsx | 14 ++++++++++ 5 files changed, 68 insertions(+), 9 deletions(-) diff --git a/packages/client/src/clients/messages/index.ts b/packages/client/src/clients/messages/index.ts index cf370181..2956908c 100644 --- a/packages/client/src/clients/messages/index.ts +++ b/packages/client/src/clients/messages/index.ts @@ -30,6 +30,13 @@ class MessageClient { status: MessageEngagementStatus, options?: UpdateMessageStatusOptions, ): Promise { + if (!this.knock.isAuthenticated()) { + this.knock.log( + "[Messages] Skipping updateStatus - user not authenticated", + ); + throw new Error("Not authenticated"); + } + // Metadata is only required for the "interacted" status const payload = status === "interacted" && options @@ -49,6 +56,13 @@ class MessageClient { messageId: string, status: Exclude, ): Promise { + if (!this.knock.isAuthenticated()) { + this.knock.log( + "[Messages] Skipping removeStatus - user not authenticated", + ); + throw new Error("Not authenticated"); + } + const result = await this.knock.client().makeRequest({ method: "DELETE", url: `/v1/messages/${messageId}/${status}`, @@ -62,6 +76,13 @@ class MessageClient { status: MessageEngagementStatus | "unseen" | "unread" | "unarchived", options?: UpdateMessageStatusOptions, ): Promise { + if (!this.knock.isAuthenticated()) { + this.knock.log( + "[Messages] Skipping batchUpdateStatuses - user not authenticated", + ); + return []; + } + // Metadata is only required for the "interacted" status const additionalPayload = status === "interacted" && options ? { metadata: options.metadata } : {}; @@ -80,6 +101,13 @@ class MessageClient { status, options, }: BulkUpdateMessagesInChannelProperties): Promise { + if (!this.knock.isAuthenticated()) { + this.knock.log( + "[Messages] Skipping bulkUpdateAllStatusesInChannel - user not authenticated", + ); + throw new Error("Not authenticated"); + } + const result = await this.knock.client().makeRequest({ method: "POST", url: `/v1/channels/${channelId}/messages/bulk/${status}`, diff --git a/packages/client/src/clients/users/index.ts b/packages/client/src/clients/users/index.ts index 9b6d9ca0..7851b105 100644 --- a/packages/client/src/clients/users/index.ts +++ b/packages/client/src/clients/users/index.ts @@ -89,7 +89,12 @@ class UserClient { } async getChannelData(params: GetChannelDataInput) { - this.instance.failIfNotAuthenticated(); + if (!this.instance.isAuthenticated()) { + this.instance.log( + "[User] Skipping getChannelData - user not authenticated", + ); + throw new Error("Not authenticated"); + } const result = await this.instance.client().makeRequest({ method: "GET", @@ -103,7 +108,12 @@ class UserClient { channelId, channelData, }: SetChannelDataInput) { - this.instance.failIfNotAuthenticated(); + if (!this.instance.isAuthenticated()) { + this.instance.log( + "[User] Skipping setChannelData - user not authenticated", + ); + throw new Error("Not authenticated"); + } const result = await this.instance.client().makeRequest({ method: "PUT", diff --git a/packages/client/test/clients/users/users.test.ts b/packages/client/test/clients/users/users.test.ts index 5d84b1a0..95ae9cb7 100644 --- a/packages/client/test/clients/users/users.test.ts +++ b/packages/client/test/clients/users/users.test.ts @@ -612,15 +612,11 @@ describe("User Client", () => { await expect( client.getChannelData({ channelId: "test" }), - ).rejects.toThrow( - "Not authenticated. Please call `authenticate` first.", - ); + ).rejects.toThrow("Not authenticated"); await expect( client.setChannelData({ channelId: "test", channelData: {} }), - ).rejects.toThrow( - "Not authenticated. Please call `authenticate` first.", - ); + ).rejects.toThrow("Not authenticated"); }); test("handles channel data operation errors", async () => { diff --git a/packages/expo/src/modules/push/KnockExpoPushNotificationProvider.tsx b/packages/expo/src/modules/push/KnockExpoPushNotificationProvider.tsx index 6aab7205..cd165217 100644 --- a/packages/expo/src/modules/push/KnockExpoPushNotificationProvider.tsx +++ b/packages/expo/src/modules/push/KnockExpoPushNotificationProvider.tsx @@ -208,6 +208,13 @@ const InternalKnockExpoPushNotificationProvider: React.FC< return; } + if (!knockClient.isAuthenticated()) { + knockClient.log( + "[Knock] Skipping status update - user not authenticated", + ); + return; + } + return knockClient.messages.updateStatus(messageId, status); }, [knockClient], @@ -219,7 +226,7 @@ const InternalKnockExpoPushNotificationProvider: React.FC< customNotificationHandler ?? defaultNotificationHandler, }); - if (autoRegister) { + if (autoRegister && knockClient.isAuthenticated()) { registerForPushNotifications() .then((token) => { if (token) { @@ -241,6 +248,10 @@ const InternalKnockExpoPushNotificationProvider: React.FC< _error, ); }); + } else if (autoRegister && !knockClient.isAuthenticated()) { + knockClient.log( + "[Knock] Skipping auto-register - user not authenticated", + ); } const notificationReceivedSubscription = diff --git a/packages/react-native/src/modules/push/KnockPushNotificationProvider.tsx b/packages/react-native/src/modules/push/KnockPushNotificationProvider.tsx index 625fd9ff..a030cfc2 100644 --- a/packages/react-native/src/modules/push/KnockPushNotificationProvider.tsx +++ b/packages/react-native/src/modules/push/KnockPushNotificationProvider.tsx @@ -45,6 +45,13 @@ export const KnockPushNotificationProvider: React.FC< // Acts like an upsert. Inserts or updates const registerPushTokenToChannel = useCallback( async (token: string, channelId: string): Promise => { + if (!knockClient.isAuthenticated()) { + knockClient.log( + "[Knock] Skipping registerPushTokenToChannel - user not authenticated", + ); + return; + } + const newDevice: Device = { token, locale: Intl.DateTimeFormat().resolvedOptions().locale, @@ -81,6 +88,13 @@ export const KnockPushNotificationProvider: React.FC< const unregisterPushTokenFromChannel = useCallback( async (token: string, channelId: string): Promise => { + if (!knockClient.isAuthenticated()) { + knockClient.log( + "[Knock] Skipping unregisterPushTokenFromChannel - user not authenticated", + ); + return; + } + return knockClient.user .getChannelData({ channelId: channelId }) .then((result: ChannelData) => { From 4c69069404684efb005dcf1279d15b34e8188780 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 20:23:57 +0000 Subject: [PATCH 12/14] Refactor KnockProvider authentication logic Co-authored-by: chris --- .../modules/core/context/KnockProvider.tsx | 38 ++------- .../test/core/KnockProvider.test.tsx | 79 ++++++++++++------- 2 files changed, 58 insertions(+), 59 deletions(-) diff --git a/packages/react-core/src/modules/core/context/KnockProvider.tsx b/packages/react-core/src/modules/core/context/KnockProvider.tsx index b789569d..2819fae3 100644 --- a/packages/react-core/src/modules/core/context/KnockProvider.tsx +++ b/packages/react-core/src/modules/core/context/KnockProvider.tsx @@ -97,34 +97,14 @@ export const KnockProvider: React.FC> = ({ ], ); - // Create and manage Knock client instance - const [forceUpdate, setForceUpdate] = React.useState(0); - const knockClient = React.useMemo( - () => { - const knock = new Knock(apiKey ?? "", { - host: authenticateOptions.host, - logLevel: authenticateOptions.logLevel, - branch: authenticateOptions.branch, - }); - - // Authenticate synchronously on creation if enabled - if (enabled && userIdOrUserWithProperties) { - knock.authenticate(userIdOrUserWithProperties, userToken, { - onUserTokenExpiring: authenticateOptions.onUserTokenExpiring, - timeBeforeExpirationInMs: - authenticateOptions.timeBeforeExpirationInMs, - identificationStrategy: authenticateOptions.identificationStrategy, - }); - } - - return knock; - }, - // We intentionally omit enabled, userIdOrUserWithProperties, and userToken - // to reuse the same instance when these change. Auth state changes are handled - // in the effect below via resetAuthentication()/authenticate() calls. - // eslint-disable-next-line react-hooks/exhaustive-deps - [apiKey, authenticateOptions, forceUpdate], - ); + // Create Knock client instance (without authentication) + const knockClient = React.useMemo(() => { + return new Knock(apiKey ?? "", { + host: authenticateOptions.host, + logLevel: authenticateOptions.logLevel, + branch: authenticateOptions.branch, + }); + }, [apiKey, authenticateOptions]); // Handle authentication state based on enabled prop React.useEffect(() => { @@ -136,11 +116,9 @@ export const KnockProvider: React.FC> = ({ timeBeforeExpirationInMs: authenticateOptions.timeBeforeExpirationInMs, identificationStrategy: authenticateOptions.identificationStrategy, }); - setForceUpdate((n) => n + 1); } else if (!enabled && knockClient.isAuthenticated()) { // When disabled, reset authentication if currently authenticated knockClient.resetAuthentication(); - setForceUpdate((n) => n + 1); } }, [ enabled, diff --git a/packages/react-core/test/core/KnockProvider.test.tsx b/packages/react-core/test/core/KnockProvider.test.tsx index 3e05b233..48f49335 100644 --- a/packages/react-core/test/core/KnockProvider.test.tsx +++ b/packages/react-core/test/core/KnockProvider.test.tsx @@ -343,6 +343,27 @@ describe("KnockProvider", () => { }); describe("enabled prop", () => { + test("does not double-authenticate on initial mount", () => { + const TestConsumer = () => { + const knock = useKnockClient(); + return
User: {knock.userId}
; + }; + + const authenticateSpy = vi.spyOn(knock, "authenticate"); + + render( + + + , + ); + + // Should only authenticate once, not twice + expect(authenticateSpy).toHaveBeenCalledTimes(1); + }); + test("renders children without authentication when enabled is false", () => { const TestChild = () =>
Child content
; @@ -425,6 +446,8 @@ describe("KnockProvider", () => { }); test("toggling enabled from false to true initializes authentication", async () => { + const authenticateSpy = vi.spyOn(knock, "authenticate"); + const TestConsumer = () => { const knock = useKnockClient(); return ( @@ -434,7 +457,7 @@ describe("KnockProvider", () => { ); }; - const { getByTestId, rerender } = render( + const { rerender } = render( { , ); - // Should not be authenticated initially - expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: false"); + // Should not have authenticated when disabled + expect(authenticateSpy).not.toHaveBeenCalled(); expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); // Enable the provider @@ -459,9 +482,13 @@ describe("KnockProvider", () => { , ); - // Wait for the effect to complete + // Wait for the effect to complete and authentication to happen await waitFor(() => { - expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: true"); + expect(authenticateSpy).toHaveBeenCalledWith( + { id: "test_user_id", name: "John" }, + undefined, + expect.any(Object), + ); }); expect(mockApiClient.makeRequest).toHaveBeenCalledWith( @@ -474,36 +501,32 @@ describe("KnockProvider", () => { }); test("toggling enabled from true to false calls resetAuthentication", async () => { - const TestConsumer = () => { - const knock = useKnockClient(); - return ( -
- API Key: {knock.apiKey}, Authenticated: {String(knock.isAuthenticated())} -
- ); - }; + const authenticateSpy = vi.spyOn(knock, "authenticate"); + const resetAuthSpy = vi.spyOn(knock, "resetAuthentication"); + + // Manually set authenticated state for this test + knock.userId = "test_user_id"; - const { getByTestId, rerender } = render( + const TestChild = () =>
Content
; + + const { rerender } = render( - + , ); - // Initially authenticated - expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: true"); - - // Spy on resetAuthentication and have it actually clear the auth state - const resetAuthSpy = vi.spyOn(knock, "resetAuthentication").mockImplementation(() => { - knock.userId = undefined; - knock.userToken = undefined; - knock.feeds.teardownInstances(); - knock.teardown(); + // Wait for initial authentication + await waitFor(() => { + expect(authenticateSpy).toHaveBeenCalled(); }); + expect(authenticateSpy).toHaveBeenCalledTimes(1); + expect(resetAuthSpy).not.toHaveBeenCalled(); + // Disable the provider rerender( { user={{ id: "test_user_id" }} enabled={false} > - + , ); - // Wait for the effect to complete + // Wait for resetAuthentication to be called await waitFor(() => { expect(resetAuthSpy).toHaveBeenCalled(); }); - // Client still exists but not authenticated - expect(getByTestId("consumer-msg")).toHaveTextContent("API Key: test_api_key"); - expect(getByTestId("consumer-msg")).toHaveTextContent("Authenticated: false"); + expect(resetAuthSpy).toHaveBeenCalledTimes(1); }); test("i18n context is still available when enabled is false", () => { From 0ded648f5c62860f79730e69e8722fd648cac24e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 20:34:56 +0000 Subject: [PATCH 13/14] feat: Add authentication guards to client methods This commit adds checks to ensure that client methods are only called when the user is authenticated. If the user is not authenticated, the method will log a message and return early, preventing unnecessary API calls. Co-authored-by: chris --- .../client/test/clients/feed/feed.test.ts | 236 ++++++++++++++++++ .../client/test/clients/guide/guide.test.ts | 138 ++++++++++ .../test/clients/messages/messages.test.ts | 94 +++++++ .../test/clients/ms-teams/ms-teams.test.ts | 97 +++++++ .../client/test/clients/slack/slack.test.ts | 74 ++++++ 5 files changed, 639 insertions(+) diff --git a/packages/client/test/clients/feed/feed.test.ts b/packages/client/test/clients/feed/feed.test.ts index f616fe8d..9c8002a3 100644 --- a/packages/client/test/clients/feed/feed.test.ts +++ b/packages/client/test/clients/feed/feed.test.ts @@ -21,6 +21,16 @@ describe("Feed", () => { }; }; + const getUnauthenticatedTestSetup = () => { + const { knock, mockApiClient } = createMockKnock(); + // Don't authenticate - leave knock unauthenticated + return { + knock, + mockApiClient, + cleanup: () => vi.clearAllMocks(), + }; + }; + describe("Basic Feed Tests", () => { test("can create a feed client", () => { const { knock, cleanup } = getTestSetup(); @@ -1582,4 +1592,230 @@ describe("Feed", () => { } }); }); + + describe("Authentication Guards", () => { + test("fetch skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedTestSetup(); + + try { + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + undefined, + ); + + const logSpy = vi.spyOn(knock, "log"); + + await feed.fetch(); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] Skipping fetch - user not authenticated", + ); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("markAsSeen skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedTestSetup(); + + try { + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + undefined, + ); + const feedItem = createUnreadFeedItem(); + const logSpy = vi.spyOn(knock, "log"); + + const result = await feed.markAsSeen(feedItem); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] Skipping markAsSeen - user not authenticated", + ); + expect(result).toEqual({ entries: [] }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("markAllAsSeen skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedTestSetup(); + + try { + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + undefined, + ); + const logSpy = vi.spyOn(knock, "log"); + + const result = await feed.markAllAsSeen(); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] Skipping markAllAsSeen - user not authenticated", + ); + expect(result).toEqual({ entries: [] }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("markAsRead skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedTestSetup(); + + try { + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + undefined, + ); + const feedItem = createUnreadFeedItem(); + const logSpy = vi.spyOn(knock, "log"); + + const result = await feed.markAsRead(feedItem); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] Skipping markAsRead - user not authenticated", + ); + expect(result).toEqual({ entries: [] }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("markAllAsRead skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedTestSetup(); + + try { + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + undefined, + ); + const logSpy = vi.spyOn(knock, "log"); + + const result = await feed.markAllAsRead(); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] Skipping markAllAsRead - user not authenticated", + ); + expect(result).toEqual({ entries: [] }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("markAsArchived skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedTestSetup(); + + try { + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + undefined, + ); + const feedItem = createUnreadFeedItem(); + const logSpy = vi.spyOn(knock, "log"); + + const result = await feed.markAsArchived(feedItem); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] Skipping markAsArchived - user not authenticated", + ); + expect(result).toEqual({ entries: [] }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("markAllAsArchived skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedTestSetup(); + + try { + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + undefined, + ); + const logSpy = vi.spyOn(knock, "log"); + + const result = await feed.markAllAsArchived(); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] Skipping markAllAsArchived - user not authenticated", + ); + expect(result).toEqual({ entries: [] }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("markAsInteracted skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedTestSetup(); + + try { + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + undefined, + ); + const feedItem = createUnreadFeedItem(); + const logSpy = vi.spyOn(knock, "log"); + + const result = await feed.markAsInteracted(feedItem); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] Skipping markAsInteracted - user not authenticated", + ); + expect(result).toEqual({ entries: [] }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("listenForUpdates skips websocket connection when not authenticated", () => { + const { knock, cleanup } = getUnauthenticatedTestSetup(); + + try { + const mockSocketManager = { + join: vi.fn(), + } as unknown as FeedSocketManager; + + const feed = new Feed( + knock, + "01234567-89ab-cdef-0123-456789abcdef", + {}, + mockSocketManager, + ); + + const logSpy = vi.spyOn(knock, "log"); + + feed.listenForUpdates(); + + expect(logSpy).toHaveBeenCalledWith( + "[Feed] User is not authenticated, skipping listening for updates", + ); + expect(mockSocketManager.join).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + }); }); diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index cb880259..d542bae0 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -3059,4 +3059,142 @@ describe("KnockGuideClient", () => { ); }); }); + + describe("Authentication Guards", () => { + test("fetch skips API call when not authenticated", async () => { + const unauthKnock = { + ...mockKnock, + isAuthenticated: vi.fn(() => false), + }; + + const client = new KnockGuideClient(unauthKnock, channelId); + const logSpy = vi.spyOn(unauthKnock, "log"); + + const result = await client.fetch(); + + expect(logSpy).toHaveBeenCalledWith( + "[Guide] Skipping fetch - user not authenticated", + ); + expect(result).toEqual({ + status: "error", + error: new Error("Not authenticated"), + }); + expect(mockKnock.user.getGuides).not.toHaveBeenCalled(); + }); + + test("subscribe skips websocket when not authenticated", () => { + const testSocket = { + channel: vi.fn(), + connect: vi.fn(), + isConnected: vi.fn(() => false), + }; + + const unauthKnock = { + ...mockKnock, + isAuthenticated: vi.fn(() => false), + client: () => ({ socket: testSocket }), + }; + + const logSpy = vi.spyOn(unauthKnock, "log"); + const client = new KnockGuideClient(unauthKnock, channelId); + + client.subscribe(); + + // Check that the skip message was logged + expect(logSpy).toHaveBeenCalledWith( + "[Guide] Skipping subscribe - user not authenticated", + ); + expect(testSocket.channel).not.toHaveBeenCalled(); + }); + + test("markAsSeen skips API call when not authenticated", async () => { + const unauthKnock = { + ...mockKnock, + isAuthenticated: vi.fn(() => false), + }; + + const mockGuide = { + key: "test-guide", + steps: [ + { + ref: "step-1", + message: { id: "msg_1", seen_at: null }, + content: {}, + }, + ], + }; + + const client = new KnockGuideClient(unauthKnock, channelId); + const logSpy = vi.spyOn(unauthKnock, "log"); + + const result = await client.markAsSeen(mockGuide, mockGuide.steps[0]); + + expect(logSpy).toHaveBeenCalledWith( + "[Guide] Skipping markAsSeen - user not authenticated", + ); + expect(result).toBeUndefined(); + expect(mockKnock.user.markGuideStepAs).not.toHaveBeenCalled(); + }); + + test("markAsInteracted skips API call when not authenticated", async () => { + const unauthKnock = { + ...mockKnock, + isAuthenticated: vi.fn(() => false), + }; + + const mockGuide = { + key: "test-guide", + steps: [ + { + ref: "step-1", + message: { id: "msg_1", interacted_at: null }, + content: {}, + }, + ], + }; + + const client = new KnockGuideClient(unauthKnock, channelId); + const logSpy = vi.spyOn(unauthKnock, "log"); + + const result = await client.markAsInteracted( + mockGuide, + mockGuide.steps[0], + ); + + expect(logSpy).toHaveBeenCalledWith( + "[Guide] Skipping markAsInteracted - user not authenticated", + ); + expect(result).toBeUndefined(); + expect(mockKnock.user.markGuideStepAs).not.toHaveBeenCalled(); + }); + + test("markAsArchived skips API call when not authenticated", async () => { + const unauthKnock = { + ...mockKnock, + isAuthenticated: vi.fn(() => false), + }; + + const mockGuide = { + key: "test-guide", + steps: [ + { + ref: "step-1", + message: { id: "msg_1", archived_at: null }, + content: {}, + }, + ], + }; + + const client = new KnockGuideClient(unauthKnock, channelId); + const logSpy = vi.spyOn(unauthKnock, "log"); + + const result = await client.markAsArchived(mockGuide, mockGuide.steps[0]); + + expect(logSpy).toHaveBeenCalledWith( + "[Guide] Skipping markAsArchived - user not authenticated", + ); + expect(result).toBeUndefined(); + expect(mockKnock.user.markGuideStepAs).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/client/test/clients/messages/messages.test.ts b/packages/client/test/clients/messages/messages.test.ts index 23ea8564..3987fc85 100644 --- a/packages/client/test/clients/messages/messages.test.ts +++ b/packages/client/test/clients/messages/messages.test.ts @@ -747,4 +747,98 @@ describe("MessageClient", () => { ); }); }); + + describe("Authentication Guards", () => { + const getUnauthenticatedSetup = () => { + const { knock, mockApiClient } = createMockKnock(); + // Don't authenticate + return { knock, mockApiClient, cleanup: () => vi.clearAllMocks() }; + }; + + test("updateStatus skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new MessageClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + await expect(client.updateStatus("msg_123", "read")).rejects.toThrow( + "Not authenticated", + ); + + expect(logSpy).toHaveBeenCalledWith( + "[Messages] Skipping updateStatus - user not authenticated", + ); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("removeStatus skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new MessageClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + await expect(client.removeStatus("msg_123", "seen")).rejects.toThrow( + "Not authenticated", + ); + + expect(logSpy).toHaveBeenCalledWith( + "[Messages] Skipping removeStatus - user not authenticated", + ); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("batchUpdateStatuses skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new MessageClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + const result = await client.batchUpdateStatuses( + ["msg_1", "msg_2"], + "read", + ); + + expect(logSpy).toHaveBeenCalledWith( + "[Messages] Skipping batchUpdateStatuses - user not authenticated", + ); + expect(result).toEqual([]); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("bulkUpdateAllStatusesInChannel skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new MessageClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + await expect( + client.bulkUpdateAllStatusesInChannel({ + channelId: "channel_123", + status: "read", + options: {}, + }), + ).rejects.toThrow("Not authenticated"); + + expect(logSpy).toHaveBeenCalledWith( + "[Messages] Skipping bulkUpdateAllStatusesInChannel - user not authenticated", + ); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + }); }); diff --git a/packages/client/test/clients/ms-teams/ms-teams.test.ts b/packages/client/test/clients/ms-teams/ms-teams.test.ts index 0aae31f5..9dc69941 100644 --- a/packages/client/test/clients/ms-teams/ms-teams.test.ts +++ b/packages/client/test/clients/ms-teams/ms-teams.test.ts @@ -630,4 +630,101 @@ describe("Microsoft Teams Client", () => { } }); }); + + describe("Authentication Guards", () => { + const getUnauthenticatedSetup = () => { + const { knock, mockApiClient } = createMockKnock(); + // Don't authenticate + return { knock, mockApiClient, cleanup: () => vi.clearAllMocks() }; + }; + + test("authCheck skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new MsTeamsClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + const result = await client.authCheck({ + tenant: "tenant_123", + knockChannelId: "channel_123", + }); + + expect(logSpy).toHaveBeenCalledWith( + "[MS Teams] Skipping authCheck - user not authenticated", + ); + expect(result).toEqual({ status: "not_connected" }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("getTeams skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new MsTeamsClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + const result = await client.getTeams({ + tenant: "tenant_123", + knockChannelId: "channel_123", + }); + + expect(logSpy).toHaveBeenCalledWith( + "[MS Teams] Skipping getTeams - user not authenticated", + ); + expect(result).toEqual({ ms_teams_teams: [], skip_token: null }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("getChannels skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new MsTeamsClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + const result = await client.getChannels({ + tenant: "tenant_123", + knockChannelId: "channel_123", + teamId: "team_123", + }); + + expect(logSpy).toHaveBeenCalledWith( + "[MS Teams] Skipping getChannels - user not authenticated", + ); + expect(result).toEqual({ ms_teams_channels: [] }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("revokeAccessToken skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new MsTeamsClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + const result = await client.revokeAccessToken({ + tenant: "tenant_123", + knockChannelId: "channel_123", + }); + + expect(logSpy).toHaveBeenCalledWith( + "[MS Teams] Skipping revokeAccessToken - user not authenticated", + ); + expect(result).toEqual({ status: "success" }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + }); }); diff --git a/packages/client/test/clients/slack/slack.test.ts b/packages/client/test/clients/slack/slack.test.ts index 0c5871c5..4b23b2a1 100644 --- a/packages/client/test/clients/slack/slack.test.ts +++ b/packages/client/test/clients/slack/slack.test.ts @@ -510,4 +510,78 @@ describe("Slack Client", () => { } }); }); + + describe("Authentication Guards", () => { + const getUnauthenticatedSetup = () => { + const { knock, mockApiClient } = createMockKnock(); + // Don't authenticate + return { knock, mockApiClient, cleanup: () => vi.clearAllMocks() }; + }; + + test("authCheck skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new SlackClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + const result = await client.authCheck({ + tenant: "tenant_123", + knockChannelId: "channel_123", + }); + + expect(logSpy).toHaveBeenCalledWith( + "[Slack] Skipping authCheck - user not authenticated", + ); + expect(result).toEqual({ status: "not_connected" }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("getChannels skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new SlackClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + const result = await client.getChannels({ + tenant: "tenant_123", + knockChannelId: "channel_123", + }); + + expect(logSpy).toHaveBeenCalledWith( + "[Slack] Skipping getChannels - user not authenticated", + ); + expect(result).toEqual({ slack_channels: [], next_cursor: null }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + + test("revokeAccessToken skips API call when not authenticated", async () => { + const { knock, mockApiClient, cleanup } = getUnauthenticatedSetup(); + + try { + const client = new SlackClient(knock); + const logSpy = vi.spyOn(knock, "log"); + + const result = await client.revokeAccessToken({ + tenant: "tenant_123", + knockChannelId: "channel_123", + }); + + expect(logSpy).toHaveBeenCalledWith( + "[Slack] Skipping revokeAccessToken - user not authenticated", + ); + expect(result).toEqual({ status: "success" }); + expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); + } finally { + cleanup(); + } + }); + }); }); From 0f9752cf5addfe0acac2483d6ad08dc09704d07a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Jan 2026 21:12:50 +0000 Subject: [PATCH 14/14] Fix: Update revokeAccessToken status to not_connected Co-authored-by: chris --- packages/client/src/clients/ms-teams/index.ts | 2 +- packages/client/src/clients/slack/index.ts | 2 +- packages/client/test/clients/ms-teams/ms-teams.test.ts | 2 +- packages/client/test/clients/slack/slack.test.ts | 2 +- .../expo/src/modules/push/KnockExpoPushNotificationProvider.tsx | 1 + 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/client/src/clients/ms-teams/index.ts b/packages/client/src/clients/ms-teams/index.ts index 6d7eeeea..974d124c 100644 --- a/packages/client/src/clients/ms-teams/index.ts +++ b/packages/client/src/clients/ms-teams/index.ts @@ -113,7 +113,7 @@ class MsTeamsClient { this.instance.log( "[MS Teams] Skipping revokeAccessToken - user not authenticated", ); - return { status: "success" }; + return { status: "not_connected" }; } const result = await this.instance.client().makeRequest({ diff --git a/packages/client/src/clients/slack/index.ts b/packages/client/src/clients/slack/index.ts index 6b5d868c..8c92c071 100644 --- a/packages/client/src/clients/slack/index.ts +++ b/packages/client/src/clients/slack/index.ts @@ -73,7 +73,7 @@ class SlackClient { this.instance.log( "[Slack] Skipping revokeAccessToken - user not authenticated", ); - return { status: "success" }; + return { status: "not_connected" }; } const result = await this.instance.client().makeRequest({ diff --git a/packages/client/test/clients/ms-teams/ms-teams.test.ts b/packages/client/test/clients/ms-teams/ms-teams.test.ts index 9dc69941..b5308959 100644 --- a/packages/client/test/clients/ms-teams/ms-teams.test.ts +++ b/packages/client/test/clients/ms-teams/ms-teams.test.ts @@ -720,7 +720,7 @@ describe("Microsoft Teams Client", () => { expect(logSpy).toHaveBeenCalledWith( "[MS Teams] Skipping revokeAccessToken - user not authenticated", ); - expect(result).toEqual({ status: "success" }); + expect(result).toEqual({ status: "not_connected" }); expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); } finally { cleanup(); diff --git a/packages/client/test/clients/slack/slack.test.ts b/packages/client/test/clients/slack/slack.test.ts index 4b23b2a1..e9b91148 100644 --- a/packages/client/test/clients/slack/slack.test.ts +++ b/packages/client/test/clients/slack/slack.test.ts @@ -577,7 +577,7 @@ describe("Slack Client", () => { expect(logSpy).toHaveBeenCalledWith( "[Slack] Skipping revokeAccessToken - user not authenticated", ); - expect(result).toEqual({ status: "success" }); + expect(result).toEqual({ status: "not_connected" }); expect(mockApiClient.makeRequest).not.toHaveBeenCalled(); } finally { cleanup(); diff --git a/packages/expo/src/modules/push/KnockExpoPushNotificationProvider.tsx b/packages/expo/src/modules/push/KnockExpoPushNotificationProvider.tsx index cd165217..5589ce25 100644 --- a/packages/expo/src/modules/push/KnockExpoPushNotificationProvider.tsx +++ b/packages/expo/src/modules/push/KnockExpoPushNotificationProvider.tsx @@ -288,6 +288,7 @@ const InternalKnockExpoPushNotificationProvider: React.FC< autoRegister, knockExpoChannelId, knockClient, + knockClient.userId, // Track userId to detect authentication state changes ]); return (