From bf1293d13b684bc1cd2e4cba529a0b8b6329f3dc Mon Sep 17 00:00:00 2001 From: Mateusz Piekarski <137193315+piekarski-mateusz@users.noreply.github.com> Date: Tue, 27 Jan 2026 09:33:43 +0100 Subject: [PATCH] fix: ouath2proxy logout fix (#5522) --- keep-ui/shared/api/ApiClient.ts | 8 +- .../shared/api/__tests__/ApiClient.test.ts | 176 ++++++++++++++++ .../lib/hooks/__tests__/useSignOut.test.ts | 195 ++++++++++++++++++ keep-ui/shared/lib/hooks/useSignOut.ts | 8 + 4 files changed, 386 insertions(+), 1 deletion(-) create mode 100644 keep-ui/shared/api/__tests__/ApiClient.test.ts create mode 100644 keep-ui/shared/lib/hooks/__tests__/useSignOut.test.ts diff --git a/keep-ui/shared/api/ApiClient.ts b/keep-ui/shared/api/ApiClient.ts index 19261bd5d1..44a8e0aed8 100644 --- a/keep-ui/shared/api/ApiClient.ts +++ b/keep-ui/shared/api/ApiClient.ts @@ -6,6 +6,7 @@ import { getApiURL } from "@/utils/apiUrl"; import * as Sentry from "@sentry/nextjs"; import { signOut as signOutClient } from "next-auth/react"; import { GuestSession } from "@/types/auth"; +import { AuthType } from "@/utils/authenticationType"; const READ_ONLY_ALLOWED_METHODS = ["GET", "OPTIONS"]; const READ_ONLY_ALWAYS_ALLOWED_URLS = [ @@ -77,7 +78,12 @@ export class ApiClient { if (response.status === 401) { // on server, middleware will handle the sign out if (!this.isServer) { - await signOutClient(); + // For OAUTH2PROXY auth, redirect to oauth2-proxy's sign_out endpoint + if (this.config?.AUTH_TYPE === AuthType.OAUTH2PROXY) { + window.location.href = "/oauth2/sign_out"; + } else { + await signOutClient(); + } } throw new KeepApiError( `${data.message || data.detail}`, diff --git a/keep-ui/shared/api/__tests__/ApiClient.test.ts b/keep-ui/shared/api/__tests__/ApiClient.test.ts new file mode 100644 index 0000000000..67252ae958 --- /dev/null +++ b/keep-ui/shared/api/__tests__/ApiClient.test.ts @@ -0,0 +1,176 @@ +import { ApiClient } from "../ApiClient"; +import { signOut as signOutClient } from "next-auth/react"; +import { AuthType } from "@/utils/authenticationType"; +import { Session } from "next-auth"; +import { InternalConfig } from "@/types/internal-config"; + +// Mock dependencies +jest.mock("next-auth/react", () => ({ + signOut: jest.fn(), +})); + +jest.mock("@sentry/nextjs", () => ({ + captureException: jest.fn(), +})); + +// Helper to create mock Response objects for Jest/Node environment +function createMockResponse( + body: object, + status: number, + contentType = "application/json" +): Response { + return { + ok: status >= 200 && status < 300, + status, + headers: { + get: (name: string) => (name.toLowerCase() === "content-type" ? contentType : null), + }, + json: async () => body, + text: async () => JSON.stringify(body), + } as unknown as Response; +} + +describe("ApiClient", () => { + let locationHref = ""; + + const mockSession = { + user: { id: "1", name: "Test User", email: "test@test.com" }, + accessToken: "test-token", + tenantId: "test-tenant", + userRole: "admin", + expires: "2099-01-01", + } as Session; + + const createConfig = (authType: AuthType): InternalConfig => + ({ + AUTH_TYPE: authType, + }) as unknown as InternalConfig; + + beforeEach(() => { + jest.clearAllMocks(); + global.fetch = jest.fn(); + locationHref = ""; + + // Mock window.location.href using Object.defineProperty + Object.defineProperty(window, "location", { + value: { + href: "", + origin: "http://localhost:3000", + }, + writable: true, + configurable: true, + }); + + Object.defineProperty(window.location, "href", { + get: () => locationHref, + set: (value: string) => { + locationHref = value; + }, + configurable: true, + }); + }); + + describe("handleResponse with 401 status", () => { + it("should redirect to /oauth2/sign_out for OAUTH2PROXY auth type on 401", async () => { + const client = new ApiClient(mockSession, createConfig(AuthType.OAUTH2PROXY)); + + const mockResponse = createMockResponse( + { message: "Unauthorized", detail: "Token expired" }, + 401 + ); + + await expect( + client.handleResponse(mockResponse, "/test-url") + ).rejects.toThrow(); + + expect(locationHref).toBe("/oauth2/sign_out"); + expect(signOutClient).not.toHaveBeenCalled(); + }); + + it("should call NextAuth signOut for DB auth type on 401", async () => { + const client = new ApiClient(mockSession, createConfig(AuthType.DB)); + + const mockResponse = createMockResponse( + { message: "Unauthorized", detail: "Token expired" }, + 401 + ); + + await expect( + client.handleResponse(mockResponse, "/test-url") + ).rejects.toThrow(); + + expect(signOutClient).toHaveBeenCalled(); + expect(locationHref).toBe(""); + }); + + it("should call NextAuth signOut for AUTH0 auth type on 401", async () => { + const client = new ApiClient(mockSession, createConfig(AuthType.AUTH0)); + + const mockResponse = createMockResponse( + { message: "Unauthorized", detail: "Token expired" }, + 401 + ); + + await expect( + client.handleResponse(mockResponse, "/test-url") + ).rejects.toThrow(); + + expect(signOutClient).toHaveBeenCalled(); + expect(locationHref).toBe(""); + }); + + it("should call NextAuth signOut for KEYCLOAK auth type on 401", async () => { + const client = new ApiClient(mockSession, createConfig(AuthType.KEYCLOAK)); + + const mockResponse = createMockResponse( + { message: "Unauthorized", detail: "Token expired" }, + 401 + ); + + await expect( + client.handleResponse(mockResponse, "/test-url") + ).rejects.toThrow(); + + expect(signOutClient).toHaveBeenCalled(); + expect(locationHref).toBe(""); + }); + + it("should not sign out on server side (isServer=true)", async () => { + // Temporarily mock typeof window to simulate server + const originalWindow = global.window; + // @ts-ignore + delete global.window; + + const client = new ApiClient(mockSession, createConfig(AuthType.OAUTH2PROXY)); + + // Restore window for test assertions + global.window = originalWindow; + + const mockResponse = createMockResponse( + { message: "Unauthorized", detail: "Token expired" }, + 401 + ); + + await expect( + client.handleResponse(mockResponse, "/test-url") + ).rejects.toThrow(); + + // On server side, neither redirect nor signOut should be called + expect(signOutClient).not.toHaveBeenCalled(); + }); + }); + + describe("handleResponse with successful response", () => { + it("should return JSON data for successful response", async () => { + const client = new ApiClient(mockSession, createConfig(AuthType.DB)); + const responseData = { id: 1, name: "test" }; + + const mockResponse = createMockResponse(responseData, 200); + + const result = await client.handleResponse(mockResponse, "/test-url"); + + expect(result).toEqual(responseData); + expect(signOutClient).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/keep-ui/shared/lib/hooks/__tests__/useSignOut.test.ts b/keep-ui/shared/lib/hooks/__tests__/useSignOut.test.ts new file mode 100644 index 0000000000..4d91ba4015 --- /dev/null +++ b/keep-ui/shared/lib/hooks/__tests__/useSignOut.test.ts @@ -0,0 +1,195 @@ +import { renderHook, act } from "@testing-library/react"; +import { useSignOut } from "../useSignOut"; +import { signOut } from "next-auth/react"; +import { useConfig } from "@/utils/hooks/useConfig"; +import { AuthType } from "@/utils/authenticationType"; + +// Mock dependencies +jest.mock("next-auth/react", () => ({ + signOut: jest.fn(), +})); + +jest.mock("@/utils/hooks/useConfig"); + +jest.mock("@sentry/nextjs", () => ({ + setUser: jest.fn(), +})); + +jest.mock("posthog-js", () => ({ + reset: jest.fn(), +})); + +describe("useSignOut", () => { + let locationHref = ""; + + beforeEach(() => { + jest.clearAllMocks(); + locationHref = ""; + + // Mock window.location.href using Object.defineProperty + Object.defineProperty(window, "location", { + value: { + href: "", + }, + writable: true, + configurable: true, + }); + + Object.defineProperty(window.location, "href", { + get: () => locationHref, + set: (value: string) => { + locationHref = value; + }, + configurable: true, + }); + }); + + it("should not sign out when config is not loaded", () => { + (useConfig as jest.Mock).mockReturnValue({ data: null }); + + const { result } = renderHook(() => useSignOut()); + + act(() => { + result.current(); + }); + + expect(signOut).not.toHaveBeenCalled(); + expect(locationHref).toBe(""); + }); + + it("should redirect to /oauth2/sign_out for OAUTH2PROXY auth type", () => { + (useConfig as jest.Mock).mockReturnValue({ + data: { + AUTH_TYPE: AuthType.OAUTH2PROXY, + SENTRY_DISABLED: "true", + POSTHOG_DISABLED: "true", + }, + }); + + const { result } = renderHook(() => useSignOut()); + + act(() => { + result.current(); + }); + + expect(locationHref).toBe("/oauth2/sign_out"); + expect(signOut).not.toHaveBeenCalled(); + }); + + it("should call NextAuth signOut for DB auth type", () => { + (useConfig as jest.Mock).mockReturnValue({ + data: { + AUTH_TYPE: AuthType.DB, + SENTRY_DISABLED: "true", + POSTHOG_DISABLED: "true", + }, + }); + + const { result } = renderHook(() => useSignOut()); + + act(() => { + result.current(); + }); + + expect(signOut).toHaveBeenCalled(); + expect(locationHref).toBe(""); + }); + + it("should call NextAuth signOut for AUTH0 auth type", () => { + (useConfig as jest.Mock).mockReturnValue({ + data: { + AUTH_TYPE: AuthType.AUTH0, + SENTRY_DISABLED: "true", + POSTHOG_DISABLED: "true", + }, + }); + + const { result } = renderHook(() => useSignOut()); + + act(() => { + result.current(); + }); + + expect(signOut).toHaveBeenCalled(); + expect(locationHref).toBe(""); + }); + + it("should call NextAuth signOut for KEYCLOAK auth type", () => { + (useConfig as jest.Mock).mockReturnValue({ + data: { + AUTH_TYPE: AuthType.KEYCLOAK, + SENTRY_DISABLED: "true", + POSTHOG_DISABLED: "true", + }, + }); + + const { result } = renderHook(() => useSignOut()); + + act(() => { + result.current(); + }); + + expect(signOut).toHaveBeenCalled(); + expect(locationHref).toBe(""); + }); + + it("should call NextAuth signOut for NOAUTH auth type", () => { + (useConfig as jest.Mock).mockReturnValue({ + data: { + AUTH_TYPE: AuthType.NOAUTH, + SENTRY_DISABLED: "true", + POSTHOG_DISABLED: "true", + }, + }); + + const { result } = renderHook(() => useSignOut()); + + act(() => { + result.current(); + }); + + expect(signOut).toHaveBeenCalled(); + expect(locationHref).toBe(""); + }); + + it("should reset Sentry user when SENTRY_DISABLED is not true", () => { + const Sentry = require("@sentry/nextjs"); + + (useConfig as jest.Mock).mockReturnValue({ + data: { + AUTH_TYPE: AuthType.DB, + SENTRY_DISABLED: "false", + POSTHOG_DISABLED: "true", + }, + }); + + const { result } = renderHook(() => useSignOut()); + + act(() => { + result.current(); + }); + + expect(Sentry.setUser).toHaveBeenCalledWith(null); + }); + + it("should reset PostHog when POSTHOG_DISABLED is not true", () => { + const posthog = require("posthog-js"); + + (useConfig as jest.Mock).mockReturnValue({ + data: { + AUTH_TYPE: AuthType.DB, + SENTRY_DISABLED: "true", + POSTHOG_DISABLED: "false", + }, + }); + + const { result } = renderHook(() => useSignOut()); + + act(() => { + result.current(); + }); + + expect(posthog.reset).toHaveBeenCalled(); + }); +}); + diff --git a/keep-ui/shared/lib/hooks/useSignOut.ts b/keep-ui/shared/lib/hooks/useSignOut.ts index 7695c2cfc0..6922ad44d6 100644 --- a/keep-ui/shared/lib/hooks/useSignOut.ts +++ b/keep-ui/shared/lib/hooks/useSignOut.ts @@ -5,6 +5,7 @@ import { signOut } from "next-auth/react"; import * as Sentry from "@sentry/nextjs"; import posthog from "posthog-js"; import { useConfig } from "@/utils/hooks/useConfig"; +import { AuthType } from "@/utils/authenticationType"; export function useSignOut() { const { data: configData } = useConfig(); @@ -22,6 +23,13 @@ export function useSignOut() { posthog.reset(); } + // For OAUTH2PROXY auth, redirect to oauth2-proxy's sign_out endpoint + // This properly clears the oauth2-proxy session + if (configData?.AUTH_TYPE === AuthType.OAUTH2PROXY) { + window.location.href = "/oauth2/sign_out"; + return; + } + signOut(); }, [configData]); }