Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions platform-api/internal/service/mcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,3 +251,26 @@ func TestFetchServerInfoSuppliedURLWithStoredCredential(t *testing.T) {
assert.Equal(t, "stored-secret", h, "the proxy's stored credential must still be sent")
}
}

// TestFetchServerInfoUpstreamUnauthorizedReturnsValidationFailed verifies that when the upstream
// MCP server responds with 401 Unauthorized to the initialize request, FetchServerInfo returns an
// apperror.ValidationFailed (HTTP 400 Bad Request) rather than apperror.Unauthorized (HTTP 401).
// This prevents the frontend API client from treating the upstream credential failure as an
// expired workspace session and logging out the user.
func TestFetchServerInfoUpstreamUnauthorizedReturnsValidationFailed(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusUnauthorized)
_, _ = w.Write([]byte(`{"error":"Unauthorized"}`))
}))
defer srv.Close()

service := NewMCPProxyService(&mockMCPProxyRepository{}, nil, nil, nil, nil, slog.Default(), &noopAuditRepo{}, &config.Server{}, newTestIdentityService())

targetURL := srv.URL + "/mcp"
_, err := service.FetchServerInfo("org-1", &api.MCPServerInfoFetchRequest{
Url: &targetURL,
})
require.Error(t, err)
assert.True(t, apperror.ValidationFailed.Is(err), "expected apperror.ValidationFailed when upstream returns 401, got: %v", err)
assert.False(t, apperror.Unauthorized.Is(err), "should NOT be apperror.Unauthorized (which would log out workspace session)")
}
17 changes: 11 additions & 6 deletions platform-api/internal/utils/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,16 @@ func initializeMCPServer(url string, headerName string, headerValue string) (str

// Check HTTP status code
if resp.StatusCode == http.StatusUnauthorized {
return "", nil, apperror.Unauthorized.New().
return "", nil, apperror.ValidationFailed.New("The MCP server requires authentication credentials or the provided credentials are invalid.").
WithLogMessage("MCP server returned 401 Unauthorized to the initialize request")
}
if resp.StatusCode == http.StatusForbidden {
return "", nil, apperror.ValidationFailed.New("Access to the MCP server was forbidden (403).").
WithLogMessage("MCP server returned 403 Forbidden to the initialize request")
}
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
return "", nil, fmt.Errorf("initialize request failed with status %d: %s", resp.StatusCode, string(body))
return "", nil, apperror.ValidationFailed.New(fmt.Sprintf("MCP server initialize request failed with status %d.", resp.StatusCode)).
WithLogMessage(fmt.Sprintf("initialize request failed with status %d", resp.StatusCode))
}

// Check if response is event stream and parse it
Expand All @@ -315,10 +320,10 @@ func initializeMCPServer(url string, headerName string, headerValue string) (str
if err := json.Unmarshal(body, &initResult); err != nil {
// Only ignore unmarshal error if this was a valid event stream (parsed above)
if !isEventStreamResp {
return "", nil, fmt.Errorf("failed to parse initialize response: %w, body: %s", err, string(body))
return "", nil, fmt.Errorf("failed to parse initialize response: %w", err)
}
// For event stream, if unmarshal fails after successful parsing, that's still an error
return "", nil, fmt.Errorf("failed to parse initialize response from event stream: %w, body: %s", err, string(body))
return "", nil, fmt.Errorf("failed to parse initialize response from event stream: %w", err)
}

if initResult.Error != nil {
Expand Down Expand Up @@ -374,15 +379,15 @@ func postJSONRPCWithSession(url string, req any, sessionID string, headerName st

// Check HTTP status code
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
return nil, fmt.Errorf("request failed with status %d: %s", resp.StatusCode, string(body))
return nil, fmt.Errorf("request failed with status %d", resp.StatusCode)
}

// Check if response is event stream
if isEventStream(resp) {
// Extract JSON data from event stream
data, err := parseEventStream(body)
if err != nil {
return nil, fmt.Errorf("failed to parse event stream: %w, body: %s", err, string(body))
return nil, fmt.Errorf("failed to parse event stream: %w", err)
}
return data, nil
}
Expand Down