From 85956b08addd33ef8ee035b0b46d782b2f0bf1e0 Mon Sep 17 00:00:00 2001 From: Dilan Induwara <153802063+Induwara04@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:47:19 +0530 Subject: [PATCH 1/2] Fix Unauthorized mcp issue --- platform-api/internal/apperror/catalog.go | 8 ++++- platform-api/internal/apperror/codes.go | 5 ++- platform-api/internal/utils/mcp.go | 6 +++- platform-api/internal/utils/mcp_test.go | 36 +++++++++++++++++++ portals/ai-workspace/src/apis/platformApis.ts | 6 ++-- portals/ai-workspace/src/auth/logout.ts | 10 +++++- .../src/clients/choreoApiClient.ts | 6 ++-- .../src/contexts/ChoreoUserContext.tsx | 3 +- 8 files changed, 71 insertions(+), 9 deletions(-) diff --git a/platform-api/internal/apperror/catalog.go b/platform-api/internal/apperror/catalog.go index 9f8af379da..cc7ae25dae 100644 --- a/platform-api/internal/apperror/catalog.go +++ b/platform-api/internal/apperror/catalog.go @@ -130,11 +130,17 @@ var ( DeploymentInvalidStatus = def(CodeDeploymentInvalidStatus, http.StatusBadRequest, "The specified deployment status filter is invalid.") ) -// MCP proxy entries. +// MCP proxy entries. MCPProxyUpstreamUnauthorized covers an upstream MCP +// server rejecting the credentials we introspect it with. It must NOT reuse +// Unauthorized: that entry means "the caller's own credentials are invalid", +// and clients act on it by tearing down their session — which an upstream's +// 401 must never trigger. A remote peer's status is data, not our status. var ( MCPProxyNotFound = def(CodeMCPProxyNotFound, http.StatusNotFound, "The specified MCP proxy could not be found.") MCPProxyExists = def(CodeMCPProxyExists, http.StatusConflict, "An MCP proxy with this ID already exists.") MCPProxyDeploymentValidationFailed = def(CodeMCPProxyDeploymentValidationFailed, http.StatusBadRequest, "%s") + MCPProxyUpstreamUnauthorized = def(CodeMCPProxyUpstreamUnauthorized, http.StatusBadRequest, + "The MCP server rejected the supplied credentials.") ) // Organization / project / application entries. diff --git a/platform-api/internal/apperror/codes.go b/platform-api/internal/apperror/codes.go index 2ca8714885..776d3c9090 100644 --- a/platform-api/internal/apperror/codes.go +++ b/platform-api/internal/apperror/codes.go @@ -102,11 +102,14 @@ const ( CodeRESTAPIAPIKeyForbidden = "REST_API_API_KEY_FORBIDDEN" ) -// MCP proxy domain codes. +// MCP proxy domain codes. MCP_PROXY_UPSTREAM_UNAUTHORIZED reports that the +// *remote* MCP server rejected our credentials — deliberately not a 401, so a +// client cannot confuse it with its own session expiring (see catalog.go). const ( CodeMCPProxyNotFound = "MCP_PROXY_NOT_FOUND" CodeMCPProxyExists = "MCP_PROXY_EXISTS" CodeMCPProxyDeploymentValidationFailed = "MCP_PROXY_DEPLOYMENT_VALIDATION_FAILED" + CodeMCPProxyUpstreamUnauthorized = "MCP_PROXY_UPSTREAM_UNAUTHORIZED" ) // Organization domain codes. diff --git a/platform-api/internal/utils/mcp.go b/platform-api/internal/utils/mcp.go index d46f6f8518..0a7c43ac4e 100644 --- a/platform-api/internal/utils/mcp.go +++ b/platform-api/internal/utils/mcp.go @@ -293,8 +293,12 @@ func initializeMCPServer(url string, headerName string, headerValue string) (str } // Check HTTP status code + // The upstream's 401 is reported as MCP_PROXY_UPSTREAM_UNAUTHORIZED (400), + // never as our own Unauthorized: clients treat a 401 from this API as their + // session expiring and force a logout, so relaying a remote peer's 401 + // verbatim would let any auth-requiring MCP server sign the user out. if resp.StatusCode == http.StatusUnauthorized { - return "", nil, apperror.Unauthorized.New(). + return "", nil, apperror.MCPProxyUpstreamUnauthorized.New(). WithLogMessage("MCP server returned 401 Unauthorized to the initialize request") } if resp.StatusCode < 200 || resp.StatusCode >= 300 { diff --git a/platform-api/internal/utils/mcp_test.go b/platform-api/internal/utils/mcp_test.go index 33f8d63867..0c6c974359 100644 --- a/platform-api/internal/utils/mcp_test.go +++ b/platform-api/internal/utils/mcp_test.go @@ -18,8 +18,12 @@ package utils import ( + "errors" + "net/http" + "net/http/httptest" "testing" + "github.com/wso2/api-platform/platform-api/internal/apperror" "github.com/wso2/api-platform/platform-api/internal/constants" "github.com/wso2/api-platform/platform-api/internal/model" @@ -93,3 +97,35 @@ func TestBuildMCPDeploymentYAML(t *testing.T) { t.Errorf("SpecVersion = %q", deploymentStruct.Spec.SpecVersion) } } + +// TestFetchMCPServerInfoUpstream401IsNotOurUnauthorized pins the status-code +// separation between "the remote MCP server rejected our credentials" and "the +// caller's own session is invalid". Clients (the AI Workspace) react to a 401 +// from this API by tearing down the session and redirecting to /login, so +// relaying an upstream's 401 verbatim let any auth-requiring MCP server sign +// the user out. The condition must surface as MCP_PROXY_UPSTREAM_UNAUTHORIZED +// with a non-401 status instead. +func TestFetchMCPServerInfoUpstream401IsNotOurUnauthorized(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + defer srv.Close() + + _, err := FetchMCPServerInfo(srv.URL, "", "") + if err == nil { + t.Fatal("expected an error when the MCP server rejects the initialize request") + } + + // errors.As, not a type assertion: FetchMCPServerInfo wraps the failure. + var appErr *apperror.Error + if !errors.As(err, &appErr) { + t.Fatalf("expected a catalog error, got %T: %v", err, err) + } + if appErr.Code != apperror.CodeMCPProxyUpstreamUnauthorized { + t.Errorf("Code = %q, want %q", appErr.Code, apperror.CodeMCPProxyUpstreamUnauthorized) + } + if appErr.HTTPStatus == http.StatusUnauthorized { + t.Error("an upstream MCP server's 401 must not reach the caller as a 401 — " + + "clients treat that as their own session expiring") + } +} diff --git a/portals/ai-workspace/src/apis/platformApis.ts b/portals/ai-workspace/src/apis/platformApis.ts index 3d9edd986f..e01c69129c 100644 --- a/portals/ai-workspace/src/apis/platformApis.ts +++ b/portals/ai-workspace/src/apis/platformApis.ts @@ -82,12 +82,14 @@ const mutatingHeaders = (): Record => ({ * instead of string-matching the message. */ const parseApiError = async (res: Response): Promise => { - handleUnauthorizedResponse(res); let body: unknown; try { body = await res.json(); } catch { /* body not JSON */ } - return buildApiError(res.status, body, `HTTP ${res.status}`); + const err = buildApiError(res.status, body, `HTTP ${res.status}`); + // Pass the code so only a genuine UNAUTHORIZED tears down the session. + handleUnauthorizedResponse(res, err.code); + return err; }; // ============================================================================ diff --git a/portals/ai-workspace/src/auth/logout.ts b/portals/ai-workspace/src/auth/logout.ts index a47ee3396b..9e519051c2 100644 --- a/portals/ai-workspace/src/auth/logout.ts +++ b/portals/ai-workspace/src/auth/logout.ts @@ -60,9 +60,17 @@ let unauthorizedRedirectStarted = false; * Start the session-expiry flow once when an authenticated API call returns * 401. Several requests can fail together when a session expires, so the * guard prevents duplicate logout calls and competing redirects. + * + * `errorCode` is the platform API's error `code` from the response body, when + * the caller has already parsed it. Only the unified `UNAUTHORIZED` code means + * *our* session is dead; any other code on a 401 describes something else the + * request touched (e.g. an upstream server rejecting credentials), and must not + * sign the user out. A 401 with no parseable code is treated as session expiry, + * which is the safe default. */ -export const handleUnauthorizedResponse = (response: Response): boolean => { +export const handleUnauthorizedResponse = (response: Response, errorCode?: string): boolean => { if (response.status !== 401) return false; + if (errorCode !== undefined && errorCode !== 'UNAUTHORIZED') return false; if (!unauthorizedRedirectStarted) { unauthorizedRedirectStarted = true; diff --git a/portals/ai-workspace/src/clients/choreoApiClient.ts b/portals/ai-workspace/src/clients/choreoApiClient.ts index a6d63667a8..013b509a93 100644 --- a/portals/ai-workspace/src/clients/choreoApiClient.ts +++ b/portals/ai-workspace/src/clients/choreoApiClient.ts @@ -102,12 +102,13 @@ export const request = async (config: ApiRequestConfig): Promise => { }); if (!res.ok) { - handleUnauthorizedResponse(res); let data: unknown; try { data = await res.json(); } catch { /* body not JSON */ } const err = buildApiError(res.status, data, `HTTP ${res.status}`); + // Pass the code so only a genuine UNAUTHORIZED tears down the session. + handleUnauthorizedResponse(res, err.code); logger.error( `[platformApiClient] ${method} ${url} → ${res.status} [${err.code ?? 'UNKNOWN'}]: ${err.message}` + (err.trackingId ? ` (trackingId: ${err.trackingId})` : ''), @@ -152,12 +153,13 @@ const sendForm = async ( const res = await fetch(url, { method, credentials: 'include', headers, body: form }); if (!res.ok) { - handleUnauthorizedResponse(res); let data: unknown; try { data = await res.json(); } catch { /* body not JSON */ } const err = buildApiError(res.status, data, `HTTP ${res.status}`); + // Pass the code so only a genuine UNAUTHORIZED tears down the session. + handleUnauthorizedResponse(res, err.code); logger.error( `[platformApiClient] ${method} ${url} → ${res.status} [${err.code ?? 'UNKNOWN'}]: ${err.message}` + (err.trackingId ? ` (trackingId: ${err.trackingId})` : ''), diff --git a/portals/ai-workspace/src/contexts/ChoreoUserContext.tsx b/portals/ai-workspace/src/contexts/ChoreoUserContext.tsx index 3c2b384122..c45b8fd128 100644 --- a/portals/ai-workspace/src/contexts/ChoreoUserContext.tsx +++ b/portals/ai-workspace/src/contexts/ChoreoUserContext.tsx @@ -99,12 +99,13 @@ async function fetchPlatformOrganization(): Promise { }); if (!res.ok) { - handleUnauthorizedResponse(res); if (res.status === 404) { logger.warn('[ChoreoUserContext] No organization found — register one at /register-org'); return []; } const body = await res.json().catch(() => ({})); + // Pass the code so only a genuine UNAUTHORIZED tears down the session. + handleUnauthorizedResponse(res, body?.code); throw new Error(body?.message ?? `GET /organizations failed: HTTP ${res.status}`); } From fdfa06d6981b10d9fdaebf40fc48eef93870ab7b Mon Sep 17 00:00:00 2001 From: Dilan Induwara <153802063+Induwara04@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:02:20 +0530 Subject: [PATCH 2/2] resolve comments --- platform-api/internal/utils/mcp_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/platform-api/internal/utils/mcp_test.go b/platform-api/internal/utils/mcp_test.go index 0c6c974359..fa03ec9ea8 100644 --- a/platform-api/internal/utils/mcp_test.go +++ b/platform-api/internal/utils/mcp_test.go @@ -124,8 +124,7 @@ func TestFetchMCPServerInfoUpstream401IsNotOurUnauthorized(t *testing.T) { if appErr.Code != apperror.CodeMCPProxyUpstreamUnauthorized { t.Errorf("Code = %q, want %q", appErr.Code, apperror.CodeMCPProxyUpstreamUnauthorized) } - if appErr.HTTPStatus == http.StatusUnauthorized { - t.Error("an upstream MCP server's 401 must not reach the caller as a 401 — " + - "clients treat that as their own session expiring") + if appErr.HTTPStatus != http.StatusBadRequest { + t.Errorf("HTTPStatus = %d, want %d", appErr.HTTPStatus, http.StatusBadRequest) } }