diff --git a/platform-api/internal/apperror/catalog.go b/platform-api/internal/apperror/catalog.go index 9f8af379d..cc7ae25da 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 2ca871488..776d3c909 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 d46f6f851..0a7c43ac4 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 33f8d6386..fa03ec9ea 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,34 @@ 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.StatusBadRequest { + t.Errorf("HTTPStatus = %d, want %d", appErr.HTTPStatus, http.StatusBadRequest) + } +} diff --git a/portals/ai-workspace/src/apis/platformApis.ts b/portals/ai-workspace/src/apis/platformApis.ts index 3d9edd986..e01c69129 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 a47ee3396..9e519051c 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 a6d63667a..013b509a9 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 3c2b38412..c45b8fd12 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}`); }