From 461b4c2095c3d0593f3c8a5d9f7a31c5f34cae0d Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Tue, 3 Nov 2026 12:09:00 +0000 Subject: [PATCH 1/6] feat: surface OAuth validation on Catalog cards Signed-off-by: Vishu Bhatnagar --- .secrets.baseline | 10 + e2e/oauth-connection-validation.spec.ts | 348 ++++++++++-------- .../server-catalog/CatalogResults.tsx | 55 ++- src/pages/ServerCatalog.test.tsx | 75 +++- src/pages/ServerCatalog.tsx | 225 +++++++---- 5 files changed, 484 insertions(+), 229 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 6e0b4c1a..c418068c 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -164,6 +164,16 @@ "type": "Secret Keyword", "verified_result": null } + ], + "src/pages/ServerCatalog.tsx": [ + { + "hashed_secret": "d9b1022fb31deaf437aef6eac8b6dd7f0aa4c019", + "is_secret": false, + "is_verified": false, + "line_number": 1377, + "type": "Secret Keyword", + "verified_result": null + } ] }, "version": "0.13.1+ibm.64.dss", diff --git a/e2e/oauth-connection-validation.spec.ts b/e2e/oauth-connection-validation.spec.ts index 7bde38da..c0ed188a 100644 --- a/e2e/oauth-connection-validation.spec.ts +++ b/e2e/oauth-connection-validation.spec.ts @@ -1,162 +1,212 @@ import { test, expect } from "./fixtures/auth"; import { APP } from "./utils/paths"; -test.describe("Servers post-authorization connection validation", () => { - test("authorization exposes a manual test, scope failure can be reauthorized and retested", async ({ - page, - context, - apiMock, - }) => { - await apiMock.mockPermissions(); - let authorized = false; - let scopeFailure = true; - let tests = 0; - let imports = 0; - const gateway = { - id: "oauth-validation", - name: "Validation server", - url: "https://saved.example/mcp", - transport: "STREAMABLEHTTP", - enabled: true, - reachable: true, - visibility: "public", - authType: "oauth", - oauthConfig: { grant_type: "authorization_code" }, - createdAt: "2026-11-02T00:00:00Z", - updatedAt: "2026-11-02T00:00:00Z", - }; - await page.route("**/v1/mcp-servers?*", (route) => - route.fulfill({ json: { gateways: [gateway], nextCursor: null } }), - ); - await page.route(`**/v1/mcp-servers/${gateway.id}`, (route) => - route.fulfill({ json: gateway }), - ); - await page.route("**/oauth/status?*", (route) => - route.fulfill({ - json: { - [gateway.id]: { - oauth_enabled: true, - grant_type: "authorization_code", - user_token_status: { status: authorized ? "valid" : "missing", authorized }, +for (const catalog of [false, true]) { + const surface = catalog ? "Catalog" : "Servers"; + test.describe(`${surface} post-authorization connection validation`, () => { + test("authorization exposes a manual test, scope failure can be reauthorized and retested", async ({ + page, + context, + apiMock, + }) => { + await apiMock.mockPermissions(); + let authorized = false; + let scopeFailure = true; + let tests = 0; + let imports = 0; + const gateway = { + id: "oauth-validation", + name: "Validation server", + url: "https://saved.example/mcp", + transport: "STREAMABLEHTTP", + enabled: true, + reachable: true, + visibility: "public", + authType: "oauth", + oauthConfig: { grant_type: "authorization_code" }, + createdAt: "2026-11-02T00:00:00Z", + updatedAt: "2026-11-02T00:00:00Z", + }; + await page.route("**/v1/mcp-servers?*", (route) => + route.fulfill({ json: { gateways: [gateway], nextCursor: null } }), + ); + await page.route(`**/v1/mcp-servers/${gateway.id}`, (route) => + route.fulfill({ json: gateway }), + ); + await page.route("**/v1/catalog?*", (route) => + route.fulfill({ + json: { + servers: [ + { + id: "catalog-validation", + name: gateway.name, + url: "https://template.example/mcp", + description: "OAuth test server", + auth_type: "OAuth", + category: "Development", + provider: "Example", + is_registered: true, + gateway_id: gateway.id, + requires_oauth_config: false, + }, + ], + total: 1, }, - }, - }), - ); - await page.route("**/oauth/authorize-nonce", (route) => - route.fulfill({ json: { nonce: "validation-nonce" } }), - ); - await context.route("**/oauth/authorize/**", (route) => - route.fulfill({ contentType: "text/html", body: "Provider" }), - ); - await page.route(`**/oauth/fetch-tools/${gateway.id}`, (route) => { - imports++; - return route.fulfill({ json: { success: true } }); - }); - await page.route(`**/v1/mcp-servers/${gateway.id}/state?*`, (route) => - route.fulfill({ json: { status: "success" } }), - ); - await page.route("**/v1/mcp-servers/test-handshake", (route) => { - tests++; - expect(route.request().postDataJSON()).toEqual({ - baseUrl: gateway.url, - gatewayId: gateway.id, - validationMode: "post_authorization", + }), + ); + await page.route("**/oauth/status?*", (route) => + route.fulfill({ + json: { + [gateway.id]: { + oauth_enabled: true, + grant_type: "authorization_code", + user_token_status: { status: authorized ? "valid" : "missing", authorized }, + }, + }, + }), + ); + await page.route("**/oauth/authorize-nonce", (route) => + route.fulfill({ json: { nonce: "validation-nonce" } }), + ); + await context.route("**/oauth/authorize/**", (route) => + route.fulfill({ contentType: "text/html", body: "Provider" }), + ); + await page.route(`**/oauth/fetch-tools/${gateway.id}`, (route) => { + imports++; + return route.fulfill({ json: { success: true } }); }); - return route.fulfill({ - json: { - success: !scopeFailure, - latencyMs: 3, - stages: { token: "ok", negotiation: "ok", tools_list: scopeFailure ? "failed" : "ok" }, - errorCode: scopeFailure ? "insufficient_scope" : null, - reauthorizationRequired: scopeFailure, - componentCounts: scopeFailure ? null : { tools: 0 }, - }, + await page.route(`**/v1/mcp-servers/${gateway.id}/state?*`, (route) => + route.fulfill({ json: { status: "success" } }), + ); + await page.route("**/v1/mcp-servers/test-handshake", (route) => { + tests++; + expect(route.request().postDataJSON()).toEqual({ + baseUrl: gateway.url, + gatewayId: gateway.id, + validationMode: "post_authorization", + }); + return route.fulfill({ + json: { + success: !scopeFailure, + latencyMs: 3, + stages: { token: "ok", negotiation: "ok", tools_list: scopeFailure ? "failed" : "ok" }, + errorCode: scopeFailure ? "insufficient_scope" : null, + reauthorizationRequired: scopeFailure, + componentCounts: scopeFailure ? null : { tools: 0 }, + }, + }); }); + await page.goto(catalog ? APP.SERVER_CATALOG : APP.SERVERS); + const popupPromise = context.waitForEvent("page"); + if (catalog) { + await page.getByRole("button", { name: "Actions for Validation server" }).click(); + await page.getByRole("menuitem", { name: "Authorize", exact: true }).click(); + } + if (!catalog) await page.getByRole("button", { name: "Authorize Validation server" }).click(); + const popup = await popupPromise; + await popup.waitForURL(/\/oauth\/authorize\//); + authorized = true; + await popup.evaluate(() => { + window.opener.postMessage({ type: "oauth_callback", status: "success" }, "*"); + window.close(); + }); + const button = page.getByRole("button", { + name: /^(Test connection|Retest) Validation server$/, + }); + await expect(button).toBeVisible(); + expect(tests).toBe(0); + await expect.poll(() => imports).toBe(1); + await button.click(); + await expect( + page.getByText( + "The stored token lacks the required scope. Review the scopes and authorize again.", + ), + ).toBeVisible(); + expect(imports).toBe(1); + const retryPopupPromise = context.waitForEvent("page"); + await page.getByRole("button", { name: "Authorize again" }).click(); + const retryPopup = await retryPopupPromise; + await retryPopup.waitForURL(/\/oauth\/authorize\//); + await expect(page.getByText("Connection validation failed")).toHaveCount(0); + await retryPopup.evaluate(() => { + window.opener.postMessage({ type: "oauth_callback", status: "success" }, "*"); + window.close(); + }); + await expect(button).toBeVisible(); + scopeFailure = false; + await button.click(); + await expect(page.getByText("Stored authorization verified")).toBeVisible(); + await expect(page.getByText("Tools access: Passed")).toBeVisible(); + await expect(page.getByRole("status").getByText("0 tools", { exact: true })).toBeVisible(); + expect(tests).toBe(2); }); - await page.goto(APP.SERVERS); - const popupPromise = context.waitForEvent("page"); - await page.getByRole("button", { name: "Authorize Validation server" }).click(); - const popup = await popupPromise; - await popup.waitForURL(/\/oauth\/authorize\//); - authorized = true; - await popup.evaluate(() => { - window.opener.postMessage({ type: "oauth_callback", status: "success" }, "*"); - window.close(); - }); - const button = page.getByRole("button", { - name: /^(Test connection|Retest) Validation server$/, - }); - await expect(button).toBeVisible(); - expect(tests).toBe(0); - await expect.poll(() => imports).toBe(1); - await button.click(); - await expect( - page.getByText( - "The stored token lacks the required scope. Review the scopes and authorize again.", - ), - ).toBeVisible(); - expect(imports).toBe(1); - const retryPopupPromise = context.waitForEvent("page"); - await page.getByRole("button", { name: "Authorize again" }).click(); - const retryPopup = await retryPopupPromise; - await retryPopup.waitForURL(/\/oauth\/authorize\//); - await expect(page.getByText("Connection validation failed")).toHaveCount(0); - await retryPopup.evaluate(() => { - window.opener.postMessage({ type: "oauth_callback", status: "success" }, "*"); - window.close(); - }); - await expect(button).toBeVisible(); - scopeFailure = false; - await button.click(); - await expect(page.getByText("Stored authorization verified")).toBeVisible(); - await expect(page.getByText("Tools access: Passed")).toBeVisible(); - await expect(page.getByRole("status").getByText("0 tools", { exact: true })).toBeVisible(); - expect(tests).toBe(2); - }); - test("read-only users cannot run stored-token validation", async ({ page, apiMock }) => { - await apiMock.mockPermissions({ permissions: ["gateways.read"] }); - const gateway = { - id: "read-only-oauth", - name: "Read only OAuth", - url: "https://saved.example/mcp", - enabled: true, - reachable: true, - authType: "oauth", - oauthConfig: { grant_type: "authorization_code" }, - visibility: "public", - }; - await page.route("**/v1/mcp-servers?*", (route) => - route.fulfill({ json: { gateways: [gateway] } }), - ); - await page.route(`**/v1/mcp-servers/${gateway.id}`, (route) => - route.fulfill({ json: gateway }), - ); - await page.route("**/oauth/status?*", (route) => - route.fulfill({ - json: { - [gateway.id]: { - oauth_enabled: true, - grant_type: "authorization_code", - user_token_status: { status: "valid", authorized: true }, + test("read-only users cannot run stored-token validation", async ({ page, apiMock }) => { + await apiMock.mockPermissions({ permissions: ["gateways.read"] }); + const gateway = { + id: "read-only-oauth", + name: "Read only OAuth", + url: "https://saved.example/mcp", + enabled: true, + reachable: true, + authType: "oauth", + oauthConfig: { grant_type: "authorization_code" }, + visibility: "public", + }; + await page.route("**/v1/mcp-servers?*", (route) => + route.fulfill({ json: { gateways: [gateway] } }), + ); + await page.route(`**/v1/mcp-servers/${gateway.id}`, (route) => + route.fulfill({ json: gateway }), + ); + await page.route("**/v1/catalog?*", (route) => + route.fulfill({ + json: { + servers: [ + { + id: "read-only", + name: gateway.name, + url: gateway.url, + auth_type: "OAuth", + is_registered: true, + gateway_id: gateway.id, + }, + ], + total: 1, }, - }, - }), - ); - const statusResponse = page.waitForResponse( - (response) => response.url().includes("/oauth/status?") && response.ok(), - ); - await page.goto(APP.SERVERS); - await statusResponse; - await expect(page.getByText(gateway.name, { exact: true })).toBeVisible(); - await expect( - page.getByRole("button", { name: `${gateway.name} status: Active. Show details` }), - ).toBeVisible(); - await expect( - page.getByRole("button", { name: "Test connection Read only OAuth", exact: true }), - ).toHaveCount(0); + }), + ); + await page.route("**/oauth/status?*", (route) => + route.fulfill({ + json: { + [gateway.id]: { + oauth_enabled: true, + grant_type: "authorization_code", + user_token_status: { status: "valid", authorized: true }, + }, + }, + }), + ); + const statusResponse = page.waitForResponse( + (response) => response.url().includes("/oauth/status?") && response.ok(), + ); + await page.goto(catalog ? APP.SERVER_CATALOG : APP.SERVERS); + await statusResponse; + await expect(page.getByText(gateway.name, { exact: true })).toBeVisible(); + if (!catalog) + await expect( + page.getByRole("button", { name: `${gateway.name} status: Active. Show details` }), + ).toBeVisible(); + await expect( + page.getByRole("button", { name: "Test connection Read only OAuth", exact: true }), + ).toHaveCount(0); + if (catalog) { + await page.getByRole("button", { name: `Actions for ${gateway.name}` }).click(); + await expect(page.getByRole("menuitem", { name: "Test connection" })).toHaveCount(0); + } + }); }); -}); +} for (const [code, guidance] of [ [ diff --git a/src/components/server-catalog/CatalogResults.tsx b/src/components/server-catalog/CatalogResults.tsx index d272d164..cedb305e 100644 --- a/src/components/server-catalog/CatalogResults.tsx +++ b/src/components/server-catalog/CatalogResults.tsx @@ -118,6 +118,9 @@ function CatalogCard({ onView, onAdd, onTest, + canTestOAuthServer, + canTestOAuth = true, + renderOAuthValidation, onAuthorize, onDisconnect, isAdding, @@ -134,6 +137,9 @@ function CatalogCard({ onView: (trigger: HTMLElement) => void; onAdd: () => void; onTest: () => void; + canTestOAuthServer?: (server: CatalogServer) => boolean; + canTestOAuth?: boolean; + renderOAuthValidation?: (server: CatalogServer) => ReactNode; onAuthorize: () => void; onDisconnect: () => void; isAdding: boolean; @@ -279,19 +285,32 @@ function CatalogCard({ > {intl.formatMessage({ id: "mcpServer.catalog.viewDetails" })} - {canTest && ( - - {intl.formatMessage({ id: "mcpServer.catalog.test" })} - - )} + {canTest && + (!isCatalogOAuthServer(server) || + oauthStatuses?.[server.gateway_id ?? ""]?.state === "not_applicable" || + canTestOAuth) && ( + + {intl.formatMessage({ id: "mcpServer.catalog.test" })} + + )} {oauthState.canAuthorize && server.gateway_id && ( )} + {renderOAuthValidation?.(server)} @@ -481,6 +501,9 @@ export function CatalogResults({ onAdd, addingServerIds, onTest, + canTestOAuthServer, + canTestOAuth = true, + renderOAuthValidation, onAuthorize, onDisconnect, testingServerIds = EMPTY_PENDING_IDS, @@ -498,6 +521,9 @@ export function CatalogResults({ onAdd: (server: CatalogServer) => void; addingServerIds: ReadonlySet; onTest: (server: CatalogServer) => void; + canTestOAuthServer?: (server: CatalogServer) => boolean; + canTestOAuth?: boolean; + renderOAuthValidation?: (server: CatalogServer) => ReactNode; onAuthorize: (server: CatalogServer) => void; onDisconnect: (server: CatalogServer) => void; testingServerIds?: ReadonlySet; @@ -529,6 +555,9 @@ export function CatalogResults({ onView={(trigger) => onView(server, trigger)} onAdd={() => onAdd(server)} onTest={() => onTest(server)} + canTestOAuthServer={canTestOAuthServer} + canTestOAuth={canTestOAuth} + renderOAuthValidation={renderOAuthValidation} onAuthorize={() => onAuthorize(server)} onDisconnect={() => onDisconnect(server)} isAdding={addingServerIds.has(server.id)} diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index f18a5b67..e0c674c8 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -1,5 +1,5 @@ import type { ReactElement } from "react"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; @@ -9,7 +9,7 @@ import { registerCatalogServer, testCatalogServer, } from "@/api/catalog"; -import { ApiError } from "@/api/client"; +import { api, ApiError } from "@/api/client"; import { getOAuthStatuses } from "@/api/oauth"; import { OAuthAuthorizationError, @@ -58,7 +58,8 @@ vi.mock("@/api/oauth", async (importOriginal) => ({ ...(await importOriginal()), getOAuthStatuses: vi.fn(), })); -vi.mock("@/api/servers", () => ({ +vi.mock("@/api/servers", async (importOriginal) => ({ + ...(await importOriginal()), // Real error classes so `instanceof` checks in the page keep working // against rejections created from the mocked module surface. OAuthAuthorizationError: class OAuthAuthorizationError extends Error { @@ -84,6 +85,7 @@ vi.mock("@/api/servers", () => ({ } }, serversApi: { + testOAuthConnection: vi.fn(), openOAuthAuthorizationPopup: vi.fn(), triggerOAuthAuthorization: vi.fn(), toggleEnabled: vi.fn(), @@ -217,6 +219,7 @@ function renderWithRouter(ui: ReactElement, path = "/app/server-catalog") { } describe("ServerCatalog", () => { + afterEach(() => vi.restoreAllMocks()); beforeEach(() => { window.history.replaceState({}, "", "/app/"); mockUseQuery.mockImplementation((path) => @@ -256,6 +259,72 @@ describe("ServerCatalog", () => { mockFetchToolsAfterOAuth.mockResolvedValue({ success: true, message: "Tools fetched" }); }); + it("validates the current saved OAuth gateway URL on explicit Test only", async () => { + const user = userEvent.setup(); + const catalogServer = { + ...oauthServer, + is_registered: true, + gateway_id: "gateway-github", + requires_oauth_config: false, + }; + mockUseQuery.mockReturnValue( + queryResult({ data: { ...response, servers: [catalogServer], total: 1 } }), + ); + mockGetOAuthStatuses.mockResolvedValue({ + statuses: { + "gateway-github": { + oauth_enabled: true, + grant_type: "authorization_code", + user_token_status: { status: "valid", authorized: true }, + }, + }, + failures: {}, + }); + const gateway = { + id: "gateway-github", + name: "GitHub", + url: "https://saved.example/mcp", + enabled: true, + authType: "oauth", + oauthConfig: { grant_type: "authorization_code" }, + }; + const get = vi.spyOn(api, "get").mockResolvedValue(gateway); + const testConnection = vi.spyOn(serversApi, "testOAuthConnection"); + testConnection.mockResolvedValue({ + success: true, + latencyMs: 1, + stages: { token: "ok", negotiation: "ok", tools_list: "ok" }, + componentCounts: { tools: 0 }, + }); + renderWithRouter(); + const button = await screen.findByRole("button", { name: "Test connection" }); + expect(testConnection).not.toHaveBeenCalled(); + await user.click(button); + expect(await screen.findByText("Stored authorization verified")).toBeVisible(); + expect(get).toHaveBeenCalledTimes(2); + expect(testConnection).toHaveBeenCalledExactlyOnceWith( + { gatewayId: "gateway-github", baseUrl: gateway.url }, + expect.any(AbortSignal), + ); + expect(mockFetchToolsAfterOAuth).not.toHaveBeenCalled(); + expect(mockToggleEnabled).not.toHaveBeenCalled(); + expect(mockRegisterCatalogServer).not.toHaveBeenCalled(); + expect(mockTestCatalogServer).not.toHaveBeenCalled(); + }); + + it("does not expose OAuth Test to read-only callers", async () => { + const user = userEvent.setup(); + const catalogServer = { ...oauthServer, is_registered: true, gateway_id: "gateway-github" }; + mockUseQuery.mockReturnValue( + queryResult({ data: { ...response, servers: [catalogServer], total: 1 } }), + ); + authState.hasPermission.mockImplementation((permission) => permission === "gateways.read"); + renderWithRouter(); + await user.click(await screen.findByRole("button", { name: "Actions for GitHub" })); + expect(screen.queryByRole("menuitem", { name: "Test connection" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Test connection" })).not.toBeInTheDocument(); + }); + it("uses the catalog GET endpoint and shared loader", () => { mockUseQuery.mockReturnValue(queryResult({ data: undefined, isLoading: true })); diff --git a/src/pages/ServerCatalog.tsx b/src/pages/ServerCatalog.tsx index 570a007b..05c72b41 100644 --- a/src/pages/ServerCatalog.tsx +++ b/src/pages/ServerCatalog.tsx @@ -9,7 +9,7 @@ import { testCatalogServer, type GatewayImpactPreview, } from "@/api/catalog"; -import { ApiError } from "@/api/client"; +import { api, ApiError } from "@/api/client"; import { OAuthAuthorizationError, OAuthCancelledError, @@ -38,6 +38,14 @@ import type { CatalogServerRegisterBody, } from "@/generated/types"; import { useDebouncedValue } from "@/hooks/useDebouncedValue"; +import type { MCPServer } from "@/types/server"; +import { useOAuthConnectionValidation } from "@/hooks/useOAuthConnectionValidation"; +import { OAuthConnectionValidationPanel } from "@/components/servers/OAuthConnectionValidationPanel"; +import { + canValidateOAuthConnection, + oauthConfigurationKey, + oauthErrorMessage, +} from "@/lib/oauthConnectionValidation"; import { useOAuthStatuses } from "@/hooks/useOAuthStatuses"; import { useQuery } from "@/hooks/useQuery"; import { useRouter } from "@/router"; @@ -358,15 +366,13 @@ function CatalogPageLayout({ ); } -// Surface the provider's own description when it sent one. A blocked popup -// gets its own message (retrying won't help). Everything else, including a -// provider denial with only a raw error code, gets the localized text. -function oauthErrorMessage(intl: IntlShape, error: unknown): string { +// Preserve popup-blocker guidance and use safe localized provider error mappings. +function catalogOAuthErrorMessage(intl: IntlShape, error: unknown): string { if (error instanceof OAuthPopupBlockedError) { return intl.formatMessage({ id: "mcpServer.catalog.oauth.popupBlocked" }); } return ( - (error instanceof OAuthAuthorizationError && error.description) || + (error instanceof OAuthAuthorizationError && oauthErrorMessage(intl, error.errorCode)) || intl.formatMessage({ id: "mcpServer.catalog.oauth.authorizationError" }) ); } @@ -437,6 +443,49 @@ export function ServerCatalog() { reload: reloadOAuthStatuses, retry: retryOAuthStatus, } = useOAuthStatuses(oauthGatewayIds, { enabled: canReadOAuthStatuses }); + const [savedOAuthGateways, setSavedOAuthGateways] = useState>({}); + const oauthGatewayIdsKey = JSON.stringify(oauthGatewayIds); + useEffect(() => { + if (!canReadOAuthStatuses) { + setSavedOAuthGateways({}); + return; + } + const controller = new AbortController(); + const ids = JSON.parse(oauthGatewayIdsKey) as string[]; + setSavedOAuthGateways((previous) => + Object.fromEntries(Object.entries(previous).filter(([id]) => ids.includes(id))), + ); + ids.forEach((id) => { + void api + .get(`/v1/mcp-servers/${encodeURIComponent(id)}`, undefined, controller.signal) + .then((gateway) => { + if (!controller.signal.aborted) + setSavedOAuthGateways((previous) => ({ ...previous, [id]: gateway })); + }) + .catch(() => undefined); + }); + return () => controller.abort(); + }, [oauthGatewayIdsKey, canReadOAuthStatuses]); + const validationTargets = Object.fromEntries( + oauthGatewayIds.map((id) => [ + id, + savedOAuthGateways[id] ? oauthConfigurationKey(savedOAuthGateways[id]) : id, + ]), + ); + const { + entries: validationEntries, + test: testOAuthConnection, + reset: resetValidation, + } = useOAuthConnectionValidation(canReadOAuthStatuses && canUpdateServer, validationTargets); + const canTestOAuthServer = (server: CatalogServer) => { + const gateway = server.gateway_id ? savedOAuthGateways[server.gateway_id] : undefined; + return ( + canReadOAuthStatuses && + canUpdateServer && + !!gateway && + canValidateOAuthConnection(gateway, oauthStatuses[gateway.id]) + ); + }; const canTest = canReadOAuthStatuses; const canDisconnect = !permissionsLoading && hasPermission("gateways.delete"); const { filters, updateQuery, toggleFilterOption, clearFilterSection, clearAllFilters } = @@ -523,12 +572,18 @@ export function ServerCatalog() { const refreshGatewayData = useCallback( async (gatewayId: string, server: Pick) => { + const gatewayRefresh = api + .get(`/v1/mcp-servers/${encodeURIComponent(gatewayId)}`) + .then((gateway) => + setSavedOAuthGateways((previous) => ({ ...previous, [gatewayId]: gateway })), + ); const componentRefreshPromise = canUpdateServer ? serversApi.fetchToolsAfterOAuth(gatewayId) : Promise.resolve(undefined); const [, componentRefresh] = await Promise.allSettled([ refreshOAuthStatus(gatewayId), componentRefreshPromise, + gatewayRefresh, ]); if (componentRefresh.status === "rejected") { showRegistrationNotification({ @@ -746,7 +801,7 @@ export function ServerCatalog() { setOAuthFormNotification({ id: `oauth:${oauthServer.id}`, type: "info", - message: oauthErrorMessage(intl, error), + message: catalogOAuthErrorMessage(intl, error), }); return false; } @@ -771,6 +826,7 @@ export function ServerCatalog() { setOAuthAuthorizing(true); setOAuthFormNotification(undefined); try { + resetValidation(gatewayId); await serversApi.triggerOAuthAuthorization(gatewayId, authWindow); if (canUpdateServer) await serversApi.toggleEnabled(gatewayId, true); await refreshGatewayData(gatewayId, oauthServer); @@ -787,7 +843,7 @@ export function ServerCatalog() { setOAuthFormNotification({ id: `oauth:${oauthServer.id}`, type: "info", - message: oauthErrorMessage(intl, error), + message: catalogOAuthErrorMessage(intl, error), }); return false; } finally { @@ -799,6 +855,7 @@ export function ServerCatalog() { canUpdateServer, oauthServer, pendingOAuthGatewayId, + resetValidation, refreshGatewayData, refreshCatalogSilently, registerServer, @@ -811,6 +868,7 @@ export function ServerCatalog() { if (!server.gateway_id || !beginAdding(server.id)) return; dismissRegistrationNotification(`oauth:${server.id}`); try { + resetValidation(server.gateway_id); await serversApi.triggerOAuthAuthorization(server.gateway_id); if (canUpdateServer) await serversApi.toggleEnabled(server.gateway_id, true); await refreshGatewayData(server.gateway_id, server); @@ -825,7 +883,7 @@ export function ServerCatalog() { showRegistrationNotification({ id: `oauth:${server.id}`, type: "info", - message: oauthErrorMessage(intl, error), + message: catalogOAuthErrorMessage(intl, error), }); } finally { endAdding(server.id); @@ -833,6 +891,7 @@ export function ServerCatalog() { }, [ beginAdding, + resetValidation, canUpdateServer, dismissRegistrationNotification, endAdding, @@ -844,52 +903,50 @@ export function ServerCatalog() { ], ); - const handleTest = useCallback( - async (server: CatalogServer) => { - if ( - server.requires_oauth_config || - isAdding(server.id) || - isDisconnecting(server.id) || - !beginTesting(server.id) - ) { - return; - } + const handleTest = async (server: CatalogServer) => { + if ( + !canTest || + server.requires_oauth_config || + isAdding(server.id) || + isDisconnecting(server.id) || + !beginTesting(server.id) + ) { + return; + } - dismissRegistrationNotification(`test:${server.id}`); + dismissRegistrationNotification(`test:${server.id}`); - try { - const result = await testCatalogServer(server.url); - if (!result) { - showRegistrationNotification( - { - id: `test:${server.id}`, - type: "error", - message: intl.formatMessage( - { id: "mcpServer.catalog.testError" }, - { name: server.name }, - ), - }, - true, - ); - return; + if ( + isCatalogOAuthServer(server) && + oauthStatuses[server.gateway_id ?? ""]?.state !== "not_applicable" + ) { + if (!server.gateway_id || !canTestOAuthServer(server)) { + endTesting(server.id); + return; + } + const id = server.gateway_id; + await testOAuthConnection(id, async (signal) => { + const gateway = await api.get( + `/v1/mcp-servers/${encodeURIComponent(id)}`, + undefined, + signal, + ); + if ( + !gateway.enabled || + gateway.authType !== "oauth" || + gateway.oauthConfig?.grant_type !== "authorization_code" + ) { + throw new ApiError(422, null, "Gateway is not eligible for OAuth validation"); } + return { gatewayId: id, baseUrl: gateway.url }; + }); + endTesting(server.id); + return; + } - const statusCode = result.statusCode ?? 0; - const succeeded = statusCode >= 200 && statusCode < 300; - showRegistrationNotification( - { - id: `test:${server.id}`, - type: succeeded ? "success" : "error", - message: intl.formatMessage( - { - id: succeeded ? "mcpServer.catalog.testSuccess" : "mcpServer.catalog.testFailure", - }, - { name: server.name, statusCode, latencyMs: result.latencyMs ?? 0 }, - ), - }, - true, - ); - } catch { + try { + const result = await testCatalogServer(server.url); + if (!result) { showRegistrationNotification( { id: `test:${server.id}`, @@ -901,20 +958,37 @@ export function ServerCatalog() { }, true, ); - } finally { - endTesting(server.id); + return; } - }, - [ - beginTesting, - dismissRegistrationNotification, - endTesting, - intl, - isAdding, - isDisconnecting, - showRegistrationNotification, - ], - ); + + const statusCode = result.statusCode ?? 0; + const succeeded = statusCode >= 200 && statusCode < 300; + showRegistrationNotification( + { + id: `test:${server.id}`, + type: succeeded ? "success" : "error", + message: intl.formatMessage( + { + id: succeeded ? "mcpServer.catalog.testSuccess" : "mcpServer.catalog.testFailure", + }, + { name: server.name, statusCode, latencyMs: result.latencyMs ?? 0 }, + ), + }, + true, + ); + } catch { + showRegistrationNotification( + { + id: `test:${server.id}`, + type: "error", + message: intl.formatMessage({ id: "mcpServer.catalog.testError" }, { name: server.name }), + }, + true, + ); + } finally { + endTesting(server.id); + } + }; const handleDisconnect = useCallback( (server: CatalogServer) => { @@ -1065,6 +1139,7 @@ export function ServerCatalog() { let pollController: AbortController | null = null; try { + resetValidation(gatewayId); const response = await disconnectCatalogGateway(gatewayId); if (response.status === 202) { setDisconnectServer(null); @@ -1138,6 +1213,7 @@ export function ServerCatalog() { } }, [ beginDisconnecting, + resetValidation, disconnectServer, endDisconnecting, intl, @@ -1298,6 +1374,27 @@ export function ServerCatalog() { onAdd={handleAdd} addingServerIds={addingServerIds} onTest={(server) => void handleTest(server)} + canTestOAuthServer={canTestOAuthServer} + canTestOAuth={canReadOAuthStatuses && canUpdateServer} + renderOAuthValidation={(server) => { + if ( + !server.gateway_id || + !canTestOAuthServer(server) || + isAdding(server.id) || + isDisconnecting(server.id) + ) + return null; + const id = server.gateway_id; + return ( + void handleTest(server)} + onCancel={() => resetValidation(id)} + onAuthorize={() => void handleAuthorize(server)} + /> + ); + }} onAuthorize={(server) => void handleAuthorize(server)} onDisconnect={handleDisconnect} testingServerIds={testingServerIds} From ba9a487f107b1cacbd4962a26d57789ad616c3ef Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Wed, 4 Nov 2026 11:16:31 +0000 Subject: [PATCH 2/6] test: assert safe OAuth callback guidance in Catalog Signed-off-by: Vishu Bhatnagar --- .secrets.baseline | 2 +- src/pages/ServerCatalog.test.tsx | 43 ++++++++++---------------------- 2 files changed, 14 insertions(+), 31 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index c418068c..a6ef8e67 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -170,7 +170,7 @@ "hashed_secret": "d9b1022fb31deaf437aef6eac8b6dd7f0aa4c019", "is_secret": false, "is_verified": false, - "line_number": 1377, + "line_number": 1378, "type": "Secret Keyword", "verified_result": null } diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index e0c674c8..1f70d01e 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -60,30 +60,6 @@ vi.mock("@/api/oauth", async (importOriginal) => ({ })); vi.mock("@/api/servers", async (importOriginal) => ({ ...(await importOriginal()), - // Real error classes so `instanceof` checks in the page keep working - // against rejections created from the mocked module surface. - OAuthAuthorizationError: class OAuthAuthorizationError extends Error { - code?: string; - description?: string; - constructor(code?: string, description?: string) { - super(description || code || "OAuth authorization failed"); - this.name = "OAuthAuthorizationError"; - this.code = code; - this.description = description; - } - }, - OAuthCancelledError: class OAuthCancelledError extends Error { - constructor(message = "OAuth authorization was cancelled") { - super(message); - this.name = "OAuthCancelledError"; - } - }, - OAuthPopupBlockedError: class OAuthPopupBlockedError extends Error { - constructor() { - super("Failed to open OAuth authorization window"); - this.name = "OAuthPopupBlockedError"; - } - }, serversApi: { testOAuthConnection: vi.fn(), openOAuthAuthorizationPopup: vi.fn(), @@ -715,7 +691,7 @@ describe("ServerCatalog", () => { expect(screen.getByRole("button", { name: "Actions for GitHub" })).toHaveFocus(); }); - it("shows the provider's reason in the dialog when authorization fails", async () => { + it("shows localized provider guidance and hides raw descriptions", async () => { const user = userEvent.setup(); mockTriggerOAuthAuthorization.mockRejectedValue( new OAuthAuthorizationError("access_denied", "Provider denied the request"), @@ -738,9 +714,12 @@ describe("ServerCatalog", () => { ); await user.click(within(form).getByRole("button", { name: "Save" })); - // Real failures still surface so the user can react; only the - // deliberate popup dismissal is silent. - expect(await within(form).findByText("Provider denied the request")).toBeVisible(); + expect( + await within(form).findByText( + "The provider denied authorization. Authorize this server again.", + ), + ).toBeVisible(); + expect(within(form).queryByText("Provider denied the request")).not.toBeInTheDocument(); expect(within(form).getByRole("button", { name: "Save" })).toBeInTheDocument(); }); @@ -766,7 +745,9 @@ describe("ServerCatalog", () => { await user.click(within(form).getByRole("button", { name: "Save" })); expect( - await within(form).findByText("OAuth authorization could not be completed. Try again."), + await within(form).findByText( + "OAuth connection failed. Check the configuration and try again.", + ), ).toBeVisible(); expect(within(form).queryByText("OAuth authorization failed")).not.toBeInTheDocument(); expect(within(form).getByRole("button", { name: "Save" })).toBeInTheDocument(); @@ -794,7 +775,9 @@ describe("ServerCatalog", () => { await user.click(within(form).getByRole("button", { name: "Save" })); expect( - await within(form).findByText("OAuth authorization could not be completed. Try again."), + await within(form).findByText( + "The provider denied authorization. Authorize this server again.", + ), ).toBeVisible(); expect(within(form).queryByText("OAuth authorization failed")).not.toBeInTheDocument(); expect(within(form).getByRole("button", { name: "Save" })).toBeInTheDocument(); From 1c63cf9edb62e704e86fe40e29f7f2ffb96f2021 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Fri, 6 Nov 2026 11:20:03 +0000 Subject: [PATCH 3/6] test: match named OAuth validation actions in Catalog Signed-off-by: Vishu Bhatnagar --- src/pages/ServerCatalog.test.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index 1f70d01e..5a2ecdc2 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -273,7 +273,7 @@ describe("ServerCatalog", () => { componentCounts: { tools: 0 }, }); renderWithRouter(); - const button = await screen.findByRole("button", { name: "Test connection" }); + const button = await screen.findByRole("button", { name: "Test connection GitHub" }); expect(testConnection).not.toHaveBeenCalled(); await user.click(button); expect(await screen.findByText("Stored authorization verified")).toBeVisible(); @@ -298,7 +298,9 @@ describe("ServerCatalog", () => { renderWithRouter(); await user.click(await screen.findByRole("button", { name: "Actions for GitHub" })); expect(screen.queryByRole("menuitem", { name: "Test connection" })).not.toBeInTheDocument(); - expect(screen.queryByRole("button", { name: "Test connection" })).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Test connection GitHub" }), + ).not.toBeInTheDocument(); }); it("uses the catalog GET endpoint and shared loader", () => { From 7160b124784308f615d618fd95f6022d2b7e8993 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Fri, 6 Nov 2026 13:28:13 +0000 Subject: [PATCH 4/6] fix: address Catalog OAuth validation review feedback Signed-off-by: Vishu Bhatnagar --- .secrets.baseline | 12 +- .../server-catalog/CatalogResults.test.tsx | 12 +- .../server-catalog/CatalogResults.tsx | 61 ++---- src/lib/catalogConnectionValidation.test.ts | 51 +++++ src/lib/catalogConnectionValidation.ts | 34 ++++ src/lib/oauthConnectionValidation.test.ts | 43 ++++ src/lib/oauthConnectionValidation.ts | 13 ++ src/pages/ServerCatalog.test.tsx | 24 ++- src/pages/ServerCatalog.tsx | 191 ++++++++++-------- src/pages/Servers.tsx | 11 +- 10 files changed, 294 insertions(+), 158 deletions(-) create mode 100644 src/lib/catalogConnectionValidation.test.ts create mode 100644 src/lib/catalogConnectionValidation.ts diff --git a/.secrets.baseline b/.secrets.baseline index a6ef8e67..74e4224f 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "(?x)(package-lock\\.json$)|^\\.secrets\\.baseline$|src/i18n/locales/*|openapi.json|^.secrets.baseline$", "lines": null }, - "generated_at": "2026-11-02T13:55:29Z", + "generated_at": "2026-11-06T13:27:54Z", "plugins_used": [ { "name": "AWSKeyDetector" @@ -164,16 +164,6 @@ "type": "Secret Keyword", "verified_result": null } - ], - "src/pages/ServerCatalog.tsx": [ - { - "hashed_secret": "d9b1022fb31deaf437aef6eac8b6dd7f0aa4c019", - "is_secret": false, - "is_verified": false, - "line_number": 1378, - "type": "Secret Keyword", - "verified_result": null - } ] }, "version": "0.13.1+ibm.64.dss", diff --git a/src/components/server-catalog/CatalogResults.test.tsx b/src/components/server-catalog/CatalogResults.test.tsx index 5505de43..d32d5b13 100644 --- a/src/components/server-catalog/CatalogResults.test.tsx +++ b/src/components/server-catalog/CatalogResults.test.tsx @@ -36,7 +36,7 @@ function catalogResults( onDisconnect={vi.fn()} testingServerIds={testingServerIds} disconnectingServerIds={disconnectingServerIds} - canTest={false} + getTestAction={() => ({ kind: "generic", availability: "hidden" })} canDisconnect={false} /> ); @@ -136,7 +136,7 @@ describe("CatalogResults", () => { onTest={vi.fn()} onAuthorize={onAuthorize} onDisconnect={vi.fn()} - canTest={false} + getTestAction={() => ({ kind: "generic", availability: "hidden" })} canDisconnect={false} oauthStatuses={{ "gateway-github": { @@ -185,7 +185,7 @@ describe("CatalogResults", () => { onDisconnect={onDisconnect} testingServerIds={new Set()} disconnectingServerIds={new Set()} - canTest + getTestAction={() => ({ kind: "oauth", availability: "disabled" })} canDisconnect oauthStatuses={{ "gateway-github": { @@ -245,7 +245,7 @@ describe("CatalogResults", () => { onTest={vi.fn()} onAuthorize={vi.fn()} onDisconnect={vi.fn()} - canTest={false} + getTestAction={() => ({ kind: "generic", availability: "hidden" })} canDisconnect={false} oauthStatuses={{ "gateway-valid": { @@ -296,7 +296,7 @@ describe("CatalogResults", () => { onTest={vi.fn()} onAuthorize={onAuthorize} onDisconnect={vi.fn()} - canTest={false} + getTestAction={() => ({ kind: "generic", availability: "hidden" })} canDisconnect={false} />, ); @@ -326,7 +326,7 @@ describe("CatalogResults", () => { onTest={vi.fn()} onAuthorize={vi.fn()} onDisconnect={vi.fn()} - canTest={false} + getTestAction={() => ({ kind: "generic", availability: "hidden" })} canDisconnect={false} />, ); diff --git a/src/components/server-catalog/CatalogResults.tsx b/src/components/server-catalog/CatalogResults.tsx index cedb305e..17f51147 100644 --- a/src/components/server-catalog/CatalogResults.tsx +++ b/src/components/server-catalog/CatalogResults.tsx @@ -25,6 +25,7 @@ import { } from "@/components/ui/dropdown-menu"; import { StatusIndicator } from "@/components/ui/status-indicator"; import type { CatalogServer } from "@/generated/types"; +import type { CatalogTestAction } from "@/lib/catalogConnectionValidation"; import { useDebouncedValue } from "@/hooks/useDebouncedValue"; import { getAuthTypeGroupId, @@ -118,15 +119,13 @@ function CatalogCard({ onView, onAdd, onTest, - canTestOAuthServer, - canTestOAuth = true, + getTestAction, renderOAuthValidation, onAuthorize, onDisconnect, isAdding, isTesting, isDisconnecting, - canTest, canDisconnect, oauthStatuses, addError, @@ -137,15 +136,13 @@ function CatalogCard({ onView: (trigger: HTMLElement) => void; onAdd: () => void; onTest: () => void; - canTestOAuthServer?: (server: CatalogServer) => boolean; - canTestOAuth?: boolean; + getTestAction: (server: CatalogServer) => CatalogTestAction; renderOAuthValidation?: (server: CatalogServer) => ReactNode; onAuthorize: () => void; onDisconnect: () => void; isAdding: boolean; isTesting: boolean; isDisconnecting: boolean; - canTest: boolean; canDisconnect: boolean; oauthStatuses?: Readonly>; /** Why the last add attempt failed, resolved to the server's reason or the fallback. */ @@ -169,6 +166,7 @@ function CatalogCard({ server, server.gateway_id ? oauthStatuses?.[server.gateway_id] : undefined, ); + const testAction = getTestAction(server); const authTypeGroupId = getAuthTypeGroupId(server.auth_type); const requiresAuthentication = authTypeGroupId === "apiKey" || authTypeGroupId === "oauth"; const StatusIcon = STATUS_ICON[oauthState.severity]; @@ -285,32 +283,19 @@ function CatalogCard({ > {intl.formatMessage({ id: "mcpServer.catalog.viewDetails" })} - {canTest && - (!isCatalogOAuthServer(server) || - oauthStatuses?.[server.gateway_id ?? ""]?.state === "not_applicable" || - canTestOAuth) && ( - - {intl.formatMessage({ id: "mcpServer.catalog.test" })} - - )} + {testAction.availability !== "hidden" && ( + + {intl.formatMessage({ id: "mcpServer.catalog.test" })} + + )} {oauthState.canAuthorize && server.gateway_id && ( void; addingServerIds: ReadonlySet; onTest: (server: CatalogServer) => void; - canTestOAuthServer?: (server: CatalogServer) => boolean; - canTestOAuth?: boolean; + getTestAction: (server: CatalogServer) => CatalogTestAction; renderOAuthValidation?: (server: CatalogServer) => ReactNode; onAuthorize: (server: CatalogServer) => void; onDisconnect: (server: CatalogServer) => void; testingServerIds?: ReadonlySet; disconnectingServerIds?: ReadonlySet; - canTest: boolean; canDisconnect: boolean; oauthStatuses?: Readonly>; addErrors?: Readonly>; @@ -555,15 +536,13 @@ export function CatalogResults({ onView={(trigger) => onView(server, trigger)} onAdd={() => onAdd(server)} onTest={() => onTest(server)} - canTestOAuthServer={canTestOAuthServer} - canTestOAuth={canTestOAuth} + getTestAction={getTestAction} renderOAuthValidation={renderOAuthValidation} onAuthorize={() => onAuthorize(server)} onDisconnect={() => onDisconnect(server)} isAdding={addingServerIds.has(server.id)} isTesting={testingServerIds.has(server.id)} isDisconnecting={disconnectingServerIds.has(server.id)} - canTest={canTest} canDisconnect={canDisconnect} oauthStatuses={oauthStatuses} addError={addErrors[server.id]} diff --git a/src/lib/catalogConnectionValidation.test.ts b/src/lib/catalogConnectionValidation.test.ts new file mode 100644 index 00000000..9a52cfd8 --- /dev/null +++ b/src/lib/catalogConnectionValidation.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; +import type { CatalogServer } from "@/generated/types"; +import { catalogTestAction } from "./catalogConnectionValidation"; + +const server = { auth_type: "OAuth", is_registered: true, gateway_id: "gateway" } as CatalogServer; +const options = { canTest: true, canUpdate: true, canValidate: true, pending: false }; + +describe("Catalog connection test policy", () => { + it.each([ + ["no read permission", { canTest: false }, "hidden"], + ["read-only OAuth caller", { canUpdate: false }, "hidden"], + ["ineligible gateway or token", { canValidate: false }, "disabled"], + ["another action pending", { pending: true }, "disabled"], + ["eligible gateway", {}, "enabled"], + ] as const)("handles %s", (_name, changes, availability) => { + expect(catalogTestAction(server, { ...options, ...changes })).toEqual({ + kind: "oauth", + availability, + }); + }); + + it("keeps client-credentials OAuth on the generic test without update permission", () => { + expect( + catalogTestAction(server, { + ...options, + canUpdate: false, + canValidate: false, + status: { state: "not_applicable", status: { oauth_enabled: true } }, + }), + ).toEqual({ kind: "generic", availability: "enabled" }); + }); + + it("keeps non-OAuth tests available independently of OAuth eligibility", () => { + expect( + catalogTestAction( + { ...server, auth_type: "Open" }, + { + ...options, + canUpdate: false, + canValidate: false, + }, + ), + ).toEqual({ kind: "generic", availability: "enabled" }); + }); + + it("disables tests until required OAuth configuration is saved", () => { + expect( + catalogTestAction({ ...server, requires_oauth_config: true }, options).availability, + ).toBe("disabled"); + }); +}); diff --git a/src/lib/catalogConnectionValidation.ts b/src/lib/catalogConnectionValidation.ts new file mode 100644 index 00000000..b43f6628 --- /dev/null +++ b/src/lib/catalogConnectionValidation.ts @@ -0,0 +1,34 @@ +import type { CatalogServer } from "@/generated/types"; +import type { OAuthStatusEntry } from "@/api/oauth"; +import { isCatalogOAuthServer } from "@/utils/catalogAuthTypes"; + +export interface CatalogTestAction { + kind: "oauth" | "generic"; + availability: "hidden" | "disabled" | "enabled"; +} + +// Keep menu visibility, disabled state and click handling on the same policy. +export function catalogTestAction( + server: CatalogServer, + options: { + status?: OAuthStatusEntry; + canTest: boolean; + canUpdate: boolean; + canValidate: boolean; + pending: boolean; + }, +): CatalogTestAction { + const kind = + isCatalogOAuthServer(server) && options.status?.state !== "not_applicable" + ? "oauth" + : "generic"; + const availability = + !options.canTest || (kind === "oauth" && !options.canUpdate) + ? "hidden" + : server.requires_oauth_config || + options.pending || + (kind === "oauth" && !options.canValidate) + ? "disabled" + : "enabled"; + return { kind, availability }; +} diff --git a/src/lib/oauthConnectionValidation.test.ts b/src/lib/oauthConnectionValidation.test.ts index ee54aef9..75be12eb 100644 --- a/src/lib/oauthConnectionValidation.test.ts +++ b/src/lib/oauthConnectionValidation.test.ts @@ -1,5 +1,7 @@ import { describe, it, expect } from "vitest"; +import { OAuthAuthorizationError } from "@/api/servers"; import type { MCPServer } from "@/types/server"; +import type { OAuthStatusEntry } from "@/api/oauth"; import { createIntl, createIntlCache } from "react-intl"; import en from "@/i18n/locales/en-US/mcpServer.json"; import es from "@/i18n/locales/es-ES/mcpServer.json"; @@ -8,6 +10,7 @@ import { canValidateOAuthConnection, oauthConfigurationKey, oauthErrorMessage, + oauthAuthorizationErrorMessage, } from "./oauthConnectionValidation"; const gateway = { @@ -45,6 +48,20 @@ describe("OAuth validation eligibility and configuration", () => { ).toBe(false); expect(canValidateOAuthConnection(gateway)).toBe(false); }); + + it.each([ + ["valid", true], + ["near_expiry", true], + ["expired", false], + ["missing", false], + ] as const)("allows stored-token validation for %s: %s", (tokenStatus, expected) => { + const status: OAuthStatusEntry = { + state: "ready", + tokenStatus, + status: { oauth_enabled: true }, + }; + expect(canValidateOAuthConnection({ ...gateway, authType: "OAuth" }, status)).toBe(expected); + }); }); describe("OAuth error guidance", () => { @@ -83,3 +100,29 @@ describe("OAuth error guidance", () => { }, ); }); + +describe("OAuth authorization error details", () => { + const intl = createIntl({ locale: "en-US", messages: en }, createIntlCache()); + + it.each(["access_denied", "unlisted_provider_error"])( + "preserves provider context for %s with localized guidance", + (code) => { + const error = new OAuthAuthorizationError(code, "Contact your organization administrator"); + expect(oauthAuthorizationErrorMessage(intl, error)).toBe( + `${oauthErrorMessage(intl, code)} Provider message: Contact your organization administrator`, + ); + }, + ); + + it("omits empty and duplicate descriptions", () => { + const guidance = oauthErrorMessage(intl, "access_denied"); + for (const description of [undefined, "", guidance]) { + expect( + oauthAuthorizationErrorMessage( + intl, + new OAuthAuthorizationError("access_denied", description), + ), + ).toBe(guidance); + } + }); +}); diff --git a/src/lib/oauthConnectionValidation.ts b/src/lib/oauthConnectionValidation.ts index ba56ab3e..eb44fb60 100644 --- a/src/lib/oauthConnectionValidation.ts +++ b/src/lib/oauthConnectionValidation.ts @@ -56,6 +56,19 @@ export function oauthErrorMessage(intl: IntlShape, code?: string): string { }); } +export function oauthAuthorizationErrorMessage( + intl: IntlShape, + error: { errorCode?: string; description?: string }, +): string { + const guidance = oauthErrorMessage(intl, error.errorCode); + return error.description && error.description !== guidance + ? intl.formatMessage( + { id: "mcpServer.oauthValidation.providerError" }, + { guidance, description: error.description }, + ) + : guidance; +} + export function hasValidationStages(result: OAuthConnectionValidationResponse): boolean { return VALIDATION_STAGES.every((stage) => ["ok", "failed", "skipped", "unsupported"].includes(result?.stages?.[stage] ?? ""), diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index 5a2ecdc2..603b7c9b 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -229,13 +229,18 @@ describe("ServerCatalog", () => { mockTriggerOAuthAuthorization.mockReset(); mockToggleEnabled.mockReset(); mockFetchToolsAfterOAuth.mockReset(); + vi.mocked(serversApi.testOAuthConnection).mockReset(); mockOpenOAuthAuthorizationPopup.mockReturnValue({ close: vi.fn() } as unknown as Window); mockTriggerOAuthAuthorization.mockResolvedValue({ type: "oauth_callback", status: "success" }); mockToggleEnabled.mockResolvedValue({ status: "success", message: "Activated" }); mockFetchToolsAfterOAuth.mockResolvedValue({ success: true, message: "Tools fetched" }); }); - it("validates the current saved OAuth gateway URL on explicit Test only", async () => { + it.each([ + ["oauth", true], + ["OAuth", true], + ["oauth", false], + ])("validates the freshly saved %s gateway only when enabled=%s", async (authType, enabled) => { const user = userEvent.setup(); const catalogServer = { ...oauthServer, @@ -261,7 +266,7 @@ describe("ServerCatalog", () => { name: "GitHub", url: "https://saved.example/mcp", enabled: true, - authType: "oauth", + authType, oauthConfig: { grant_type: "authorization_code" }, }; const get = vi.spyOn(api, "get").mockResolvedValue(gateway); @@ -275,7 +280,17 @@ describe("ServerCatalog", () => { renderWithRouter(); const button = await screen.findByRole("button", { name: "Test connection GitHub" }); expect(testConnection).not.toHaveBeenCalled(); + get.mockResolvedValue({ ...gateway, enabled }); await user.click(button); + if (!enabled) { + expect( + await screen.findByText( + "The saved server is not eligible for this test or the request is invalid. Reload and check its configuration.", + ), + ).toBeVisible(); + expect(testConnection).not.toHaveBeenCalled(); + return; + } expect(await screen.findByText("Stored authorization verified")).toBeVisible(); expect(get).toHaveBeenCalledTimes(2); expect(testConnection).toHaveBeenCalledExactlyOnceWith( @@ -693,7 +708,7 @@ describe("ServerCatalog", () => { expect(screen.getByRole("button", { name: "Actions for GitHub" })).toHaveFocus(); }); - it("shows localized provider guidance and hides raw descriptions", async () => { + it("shows localized guidance with the provider description", async () => { const user = userEvent.setup(); mockTriggerOAuthAuthorization.mockRejectedValue( new OAuthAuthorizationError("access_denied", "Provider denied the request"), @@ -718,10 +733,9 @@ describe("ServerCatalog", () => { expect( await within(form).findByText( - "The provider denied authorization. Authorize this server again.", + "The provider denied authorization. Authorize this server again. Provider message: Provider denied the request", ), ).toBeVisible(); - expect(within(form).queryByText("Provider denied the request")).not.toBeInTheDocument(); expect(within(form).getByRole("button", { name: "Save" })).toBeInTheDocument(); }); diff --git a/src/pages/ServerCatalog.tsx b/src/pages/ServerCatalog.tsx index 05c72b41..1692cf55 100644 --- a/src/pages/ServerCatalog.tsx +++ b/src/pages/ServerCatalog.tsx @@ -44,8 +44,9 @@ import { OAuthConnectionValidationPanel } from "@/components/servers/OAuthConnec import { canValidateOAuthConnection, oauthConfigurationKey, - oauthErrorMessage, + oauthAuthorizationErrorMessage, } from "@/lib/oauthConnectionValidation"; +import { catalogTestAction } from "@/lib/catalogConnectionValidation"; import { useOAuthStatuses } from "@/hooks/useOAuthStatuses"; import { useQuery } from "@/hooks/useQuery"; import { useRouter } from "@/router"; @@ -371,10 +372,10 @@ function catalogOAuthErrorMessage(intl: IntlShape, error: unknown): string { if (error instanceof OAuthPopupBlockedError) { return intl.formatMessage({ id: "mcpServer.catalog.oauth.popupBlocked" }); } - return ( - (error instanceof OAuthAuthorizationError && oauthErrorMessage(intl, error.errorCode)) || - intl.formatMessage({ id: "mcpServer.catalog.oauth.authorizationError" }) - ); + if (error instanceof OAuthAuthorizationError) { + return oauthAuthorizationErrorMessage(intl, error); + } + return intl.formatMessage({ id: "mcpServer.catalog.oauth.authorizationError" }); } export function ServerCatalog() { @@ -477,16 +478,37 @@ export function ServerCatalog() { test: testOAuthConnection, reset: resetValidation, } = useOAuthConnectionValidation(canReadOAuthStatuses && canUpdateServer, validationTargets); - const canTestOAuthServer = (server: CatalogServer) => { - const gateway = server.gateway_id ? savedOAuthGateways[server.gateway_id] : undefined; - return ( - canReadOAuthStatuses && - canUpdateServer && - !!gateway && - canValidateOAuthConnection(gateway, oauthStatuses[gateway.id]) - ); - }; - const canTest = canReadOAuthStatuses; + const canTestOAuthServer = useCallback( + (server: CatalogServer) => { + const gateway = server.gateway_id ? savedOAuthGateways[server.gateway_id] : undefined; + return ( + canReadOAuthStatuses && + canUpdateServer && + !!gateway && + canValidateOAuthConnection(gateway, oauthStatuses[gateway.id]) + ); + }, + [canReadOAuthStatuses, canUpdateServer, savedOAuthGateways, oauthStatuses], + ); + const getTestAction = useCallback( + (server: CatalogServer) => + catalogTestAction(server, { + status: oauthStatuses[server.gateway_id ?? ""], + canTest: canReadOAuthStatuses, + canUpdate: canUpdateServer, + canValidate: canTestOAuthServer(server), + pending: isAdding(server.id) || isTesting(server.id) || isDisconnecting(server.id), + }), + [ + oauthStatuses, + canReadOAuthStatuses, + canUpdateServer, + canTestOAuthServer, + isAdding, + isTesting, + isDisconnecting, + ], + ); const canDisconnect = !permissionsLoading && hasPermission("gateways.delete"); const { filters, updateQuery, toggleFilterOption, clearFilterSection, clearAllFilters } = useCatalogFilters(); @@ -903,50 +925,65 @@ export function ServerCatalog() { ], ); - const handleTest = async (server: CatalogServer) => { - if ( - !canTest || - server.requires_oauth_config || - isAdding(server.id) || - isDisconnecting(server.id) || - !beginTesting(server.id) - ) { - return; - } + const handleTest = useCallback( + async (server: CatalogServer) => { + const action = getTestAction(server); + if (action.availability !== "enabled" || !beginTesting(server.id)) { + return; + } - dismissRegistrationNotification(`test:${server.id}`); + dismissRegistrationNotification(`test:${server.id}`); - if ( - isCatalogOAuthServer(server) && - oauthStatuses[server.gateway_id ?? ""]?.state !== "not_applicable" - ) { - if (!server.gateway_id || !canTestOAuthServer(server)) { + if (action.kind === "oauth") { + const id = server.gateway_id!; + await testOAuthConnection(id, async (signal) => { + const gateway = await api.get( + `/v1/mcp-servers/${encodeURIComponent(id)}`, + undefined, + signal, + ); + if (!canValidateOAuthConnection(gateway, oauthStatuses[id])) { + throw new ApiError(422, null, "Gateway is not eligible for OAuth validation"); + } + return { gatewayId: id, baseUrl: gateway.url }; + }); endTesting(server.id); return; } - const id = server.gateway_id; - await testOAuthConnection(id, async (signal) => { - const gateway = await api.get( - `/v1/mcp-servers/${encodeURIComponent(id)}`, - undefined, - signal, - ); - if ( - !gateway.enabled || - gateway.authType !== "oauth" || - gateway.oauthConfig?.grant_type !== "authorization_code" - ) { - throw new ApiError(422, null, "Gateway is not eligible for OAuth validation"); + + try { + const result = await testCatalogServer(server.url); + if (!result) { + showRegistrationNotification( + { + id: `test:${server.id}`, + type: "error", + message: intl.formatMessage( + { id: "mcpServer.catalog.testError" }, + { name: server.name }, + ), + }, + true, + ); + return; } - return { gatewayId: id, baseUrl: gateway.url }; - }); - endTesting(server.id); - return; - } - try { - const result = await testCatalogServer(server.url); - if (!result) { + const statusCode = result.statusCode ?? 0; + const succeeded = statusCode >= 200 && statusCode < 300; + showRegistrationNotification( + { + id: `test:${server.id}`, + type: succeeded ? "success" : "error", + message: intl.formatMessage( + { + id: succeeded ? "mcpServer.catalog.testSuccess" : "mcpServer.catalog.testFailure", + }, + { name: server.name, statusCode, latencyMs: result.latencyMs ?? 0 }, + ), + }, + true, + ); + } catch { showRegistrationNotification( { id: `test:${server.id}`, @@ -958,37 +995,21 @@ export function ServerCatalog() { }, true, ); - return; + } finally { + endTesting(server.id); } - - const statusCode = result.statusCode ?? 0; - const succeeded = statusCode >= 200 && statusCode < 300; - showRegistrationNotification( - { - id: `test:${server.id}`, - type: succeeded ? "success" : "error", - message: intl.formatMessage( - { - id: succeeded ? "mcpServer.catalog.testSuccess" : "mcpServer.catalog.testFailure", - }, - { name: server.name, statusCode, latencyMs: result.latencyMs ?? 0 }, - ), - }, - true, - ); - } catch { - showRegistrationNotification( - { - id: `test:${server.id}`, - type: "error", - message: intl.formatMessage({ id: "mcpServer.catalog.testError" }, { name: server.name }), - }, - true, - ); - } finally { - endTesting(server.id); - } - }; + }, + [ + getTestAction, + beginTesting, + dismissRegistrationNotification, + testOAuthConnection, + oauthStatuses, + endTesting, + showRegistrationNotification, + intl, + ], + ); const handleDisconnect = useCallback( (server: CatalogServer) => { @@ -1373,9 +1394,8 @@ export function ServerCatalog() { onView={handleView} onAdd={handleAdd} addingServerIds={addingServerIds} - onTest={(server) => void handleTest(server)} - canTestOAuthServer={canTestOAuthServer} - canTestOAuth={canReadOAuthStatuses && canUpdateServer} + onTest={handleTest} + getTestAction={getTestAction} renderOAuthValidation={(server) => { if ( !server.gateway_id || @@ -1399,7 +1419,6 @@ export function ServerCatalog() { onDisconnect={handleDisconnect} testingServerIds={testingServerIds} disconnectingServerIds={disconnectingServerIds} - canTest={canTest} canDisconnect={canDisconnect} oauthStatuses={oauthStatuses} addErrors={addErrors} diff --git a/src/pages/Servers.tsx b/src/pages/Servers.tsx index 26394ea6..50cdd023 100644 --- a/src/pages/Servers.tsx +++ b/src/pages/Servers.tsx @@ -26,7 +26,7 @@ import { OAuthConnectionValidationPanel } from "@/components/servers/OAuthConnec import { canValidateOAuthConnection, oauthConfigurationKey, - oauthErrorMessage, + oauthAuthorizationErrorMessage, } from "@/lib/oauthConnectionValidation"; import { isOAuthServer } from "@/lib/serverStatus"; @@ -327,14 +327,7 @@ export function Servers() { if (!(err instanceof OAuthCancelledError)) setOAuthDialogServerId(id); let reason: string; if (err instanceof OAuthAuthorizationError) { - const guidance = oauthErrorMessage(intl, err.errorCode); - reason = - err.description && err.description !== guidance - ? intl.formatMessage( - { id: "mcpServer.oauthValidation.providerError" }, - { guidance, description: err.description }, - ) - : guidance; + reason = oauthAuthorizationErrorMessage(intl, err); } else { reason = err instanceof OAuthCancelledError ? "" : sanitizeError(err); } From 72cf010250f925a0aff909ba193ea22325995b47 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Fri, 6 Nov 2026 13:38:21 +0000 Subject: [PATCH 5/6] test: control Catalog OAuth validation completion Signed-off-by: Vishu Bhatnagar --- src/pages/ServerCatalog.test.tsx | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index 603b7c9b..8a7baa74 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -269,18 +269,18 @@ describe("ServerCatalog", () => { authType, oauthConfig: { grant_type: "authorization_code" }, }; - const get = vi.spyOn(api, "get").mockResolvedValue(gateway); + const savedGateway = deferred(); + const validation = deferred>>(); + const get = vi + .spyOn(api, "get") + .mockReturnValueOnce(savedGateway.promise) + .mockResolvedValue({ ...gateway, enabled }); const testConnection = vi.spyOn(serversApi, "testOAuthConnection"); - testConnection.mockResolvedValue({ - success: true, - latencyMs: 1, - stages: { token: "ok", negotiation: "ok", tools_list: "ok" }, - componentCounts: { tools: 0 }, - }); + testConnection.mockReturnValue(validation.promise); renderWithRouter(); - const button = await screen.findByRole("button", { name: "Test connection GitHub" }); + await act(async () => savedGateway.resolve(gateway)); + const button = screen.getByRole("button", { name: "Test connection GitHub" }); expect(testConnection).not.toHaveBeenCalled(); - get.mockResolvedValue({ ...gateway, enabled }); await user.click(button); if (!enabled) { expect( @@ -291,12 +291,23 @@ describe("ServerCatalog", () => { expect(testConnection).not.toHaveBeenCalled(); return; } - expect(await screen.findByText("Stored authorization verified")).toBeVisible(); expect(get).toHaveBeenCalledTimes(2); expect(testConnection).toHaveBeenCalledExactlyOnceWith( { gatewayId: "gateway-github", baseUrl: gateway.url }, expect.any(AbortSignal), ); + expect( + screen.getByRole("button", { name: "Testing stored authorization… GitHub" }), + ).toHaveAttribute("aria-disabled", "true"); + await act(async () => { + validation.resolve({ + success: true, + latencyMs: 1, + stages: { token: "ok", negotiation: "ok", tools_list: "ok" }, + componentCounts: { tools: 0 }, + }); + }); + expect(screen.getByText("Stored authorization verified")).toBeVisible(); expect(mockFetchToolsAfterOAuth).not.toHaveBeenCalled(); expect(mockToggleEnabled).not.toHaveBeenCalled(); expect(mockRegisterCatalogServer).not.toHaveBeenCalled(); From 3d27caf304de2fdaf7d7ef717961b278b9c5e77c Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Fri, 6 Nov 2026 15:12:38 +0000 Subject: [PATCH 6/6] fix: reuse cached Catalog OAuth gateway requests Signed-off-by: Vishu Bhatnagar --- src/pages/ServerCatalog.test.tsx | 72 ++++++++++++++++++++++++++++++++ src/pages/ServerCatalog.tsx | 38 ++++++++++++----- 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index 8a7baa74..81062adf 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -329,6 +329,78 @@ describe("ServerCatalog", () => { ).not.toBeInTheDocument(); }); + it("reuses cached and pending gateway requests when the catalog ID list changes", async () => { + const first = { ...oauthServer, is_registered: true, gateway_id: "gateway-first" }; + const second = { ...first, id: "second", name: "Second", gateway_id: "gateway-second" }; + const third = { ...first, id: "third", name: "Third", gateway_id: "gateway-third" }; + const gateway = { id: first.gateway_id, enabled: true, authType: "oauth" }; + const pending = deferred(); + const secondRequest = deferred(); + const get = vi + .spyOn(api, "get") + .mockReturnValueOnce(pending.promise) + .mockReturnValueOnce(secondRequest.promise); + const show = (servers: CatalogServer[]) => + mockUseQuery.mockReturnValue(queryResult({ data: { ...response, servers } })); + + show([first]); + const { rerender, unmount } = renderWithRouter(); + expect(get).toHaveBeenCalledTimes(1); + const firstSignal = get.mock.calls[0][2] as AbortSignal; + + show([first, second]); + rerender(withProviders()); + expect(get).toHaveBeenCalledTimes(2); + expect(firstSignal.aborted).toBe(false); + + await act(async () => pending.resolve(gateway)); + expect(get).toHaveBeenCalledTimes(2); + await act(async () => secondRequest.resolve({ ...gateway, id: second.gateway_id })); + show([second]); + rerender(withProviders()); + show([first, second]); + rerender(withProviders()); + expect(get).toHaveBeenCalledTimes(2); + + const lastRequest = deferred(); + get.mockReturnValue(lastRequest.promise); + show([first, second, third]); + rerender(withProviders()); + expect(get).toHaveBeenCalledTimes(3); + const thirdSignal = get.mock.calls[2][2] as AbortSignal; + expect(thirdSignal.aborted).toBe(false); + unmount(); + expect(thirdSignal.aborted).toBe(true); + await act(async () => lastRequest.resolve({ ...gateway, id: third.gateway_id })); + }); + + it("clears cached gateways and aborts pending requests when read permission is revoked", async () => { + const first = { ...oauthServer, is_registered: true, gateway_id: "gateway-first" }; + const second = { ...first, id: "second", name: "Second", gateway_id: "gateway-second" }; + const gateway = { id: first.gateway_id, enabled: true, authType: "oauth" }; + const firstRequest = deferred(); + const pending = deferred(); + const get = vi + .spyOn(api, "get") + .mockReturnValueOnce(firstRequest.promise) + .mockReturnValue(pending.promise); + mockUseQuery.mockReturnValue(queryResult({ data: { ...response, servers: [first, second] } })); + const { rerender } = renderWithRouter(); + await act(async () => firstRequest.resolve(gateway)); + expect(get).toHaveBeenCalledTimes(2); + const signal = get.mock.calls[1][2] as AbortSignal; + authState.hasPermission.mockReturnValue(false); + rerender(withProviders()); + expect(signal.aborted).toBe(true); + await act(async () => pending.resolve({ ...gateway, id: second.gateway_id })); + const reloaded = deferred(); + get.mockReturnValue(reloaded.promise); + authState.hasPermission.mockReturnValue(true); + rerender(withProviders()); + expect(get).toHaveBeenCalledTimes(4); + await act(async () => reloaded.resolve(gateway)); + }); + it("uses the catalog GET endpoint and shared loader", () => { mockUseQuery.mockReturnValue(queryResult({ data: undefined, isLoading: true })); diff --git a/src/pages/ServerCatalog.tsx b/src/pages/ServerCatalog.tsx index 1692cf55..7640669c 100644 --- a/src/pages/ServerCatalog.tsx +++ b/src/pages/ServerCatalog.tsx @@ -445,28 +445,43 @@ export function ServerCatalog() { retry: retryOAuthStatus, } = useOAuthStatuses(oauthGatewayIds, { enabled: canReadOAuthStatuses }); const [savedOAuthGateways, setSavedOAuthGateways] = useState>({}); + const gatewayRequests = useRef(new Map()); + const abortGatewayRequests = useCallback(() => { + gatewayRequests.current.forEach((controller) => controller.abort()); + gatewayRequests.current.clear(); + }, []); + useEffect(() => abortGatewayRequests, [abortGatewayRequests]); const oauthGatewayIdsKey = JSON.stringify(oauthGatewayIds); useEffect(() => { if (!canReadOAuthStatuses) { - setSavedOAuthGateways({}); + abortGatewayRequests(); + setSavedOAuthGateways((previous) => (Object.keys(previous).length ? {} : previous)); return; } - const controller = new AbortController(); const ids = JSON.parse(oauthGatewayIdsKey) as string[]; - setSavedOAuthGateways((previous) => - Object.fromEntries(Object.entries(previous).filter(([id]) => ids.includes(id))), - ); + // Keep cached and in-flight entries across ID-list changes. Explicit tests + // and authorization refreshes still fetch the current saved configuration. ids.forEach((id) => { + if (savedOAuthGateways[id] || gatewayRequests.current.has(id)) return; + const controller = new AbortController(); + gatewayRequests.current.set(id, controller); void api .get(`/v1/mcp-servers/${encodeURIComponent(id)}`, undefined, controller.signal) .then((gateway) => { - if (!controller.signal.aborted) - setSavedOAuthGateways((previous) => ({ ...previous, [id]: gateway })); + if (!controller.signal.aborted) { + setSavedOAuthGateways((previous) => + previous[id] ? previous : { ...previous, [id]: gateway }, + ); + } }) - .catch(() => undefined); + .catch(() => undefined) + .finally(() => { + if (gatewayRequests.current.get(id) === controller) { + gatewayRequests.current.delete(id); + } + }); }); - return () => controller.abort(); - }, [oauthGatewayIdsKey, canReadOAuthStatuses]); + }, [oauthGatewayIdsKey, canReadOAuthStatuses, savedOAuthGateways, abortGatewayRequests]); const validationTargets = Object.fromEntries( oauthGatewayIds.map((id) => [ id, @@ -1402,8 +1417,9 @@ export function ServerCatalog() { !canTestOAuthServer(server) || isAdding(server.id) || isDisconnecting(server.id) - ) + ) { return null; + } const id = server.gateway_id; return (