diff --git a/platform-api/internal/service/mcp.go b/platform-api/internal/service/mcp.go index c4d708cbb5..dd498fb6a2 100644 --- a/platform-api/internal/service/mcp.go +++ b/platform-api/internal/service/mcp.go @@ -549,8 +549,16 @@ func (s *MCPProxyService) Delete(orgUUID, handle, deletedBy string) error { } // FetchServerInfo fetches server information from an MCP backend. -// When proxyId is provided, the URL and auth are fetched from the stored proxy configuration. -// When proxyId is not provided, url is required and auth is optional. +// +// The target and credentials are selected by which fields the caller supplies: +// - url alone, or url + auth: contact that URL with the caller's own credentials +// (the creation flow, where nothing is stored yet). +// - proxyId alone: contact the proxy's stored URL with its stored credentials. +// - proxyId + url: contact the caller's URL with the proxy's stored credentials, for +// validating an unsaved endpoint edit without re-entering a write-only secret. +// +// auth may never accompany proxyId: in any stored-credential flow the stored auth is +// authoritative, so an override is rejected rather than silently ignored. func (s *MCPProxyService) FetchServerInfo(orgUUID string, req *api.MCPServerInfoFetchRequest) (*api.MCPServerInfoFetchResponse, error) { if req == nil { return nil, apperror.ValidationFailed.New("A request body is required.") @@ -562,22 +570,15 @@ func (s *MCPProxyService) FetchServerInfo(orgUUID string, req *api.MCPServerInfo hasProxyID := req.ProxyId != nil && *req.ProxyId != "" hasURL := req.Url != nil && *req.Url != "" - // Exactly one of url / proxyId selects the target, matching the oneOf constraint on - // MCPServerInfoFetchRequest. Accepting both would leave which one wins implicit. - if hasProxyID && hasURL { - return nil, apperror.ValidationFailed.New("Provide either url or proxyId, not both.") - } if !hasProxyID && !hasURL { return nil, apperror.ValidationFailed.New("Either url or proxyId is required.") } - // Auth is only meaningful for the direct-url flow; in refetch mode the stored auth is - // authoritative, so a caller-supplied override is rejected rather than silently ignored. if hasProxyID && req.Auth != nil { return nil, apperror.ValidationFailed.New("The auth field is not allowed when proxyId is provided.") } if hasProxyID { - // ProxyId provided - fetch stored configuration (refetch flow) + // ProxyId provided - resolve stored configuration (refetch flow) // Auth override is NOT allowed in refetch - use exactly what's stored proxy, err := s.repo.GetByHandle(*req.ProxyId, orgUUID) if err != nil { @@ -595,6 +596,13 @@ func (s *MCPProxyService) FetchServerInfo(orgUUID string, req *api.MCPServerInfo url = proxy.Configuration.Upstream.Main.URL } + // A caller-supplied URL overrides the stored one, and is used verbatim — this is + // what lets the UI validate an unsaved endpoint edit without re-entering a + // write-only credential. + if hasURL { + url = *req.Url + } + // Use stored auth from proxy configuration. The stored value is a // {{ secret "handle" }} placeholder, not the plaintext credential — resolve it // through the secret store before using it as the actual upstream header value. diff --git a/platform-api/internal/service/mcp_test.go b/platform-api/internal/service/mcp_test.go index bbdf6448bc..5a1d4e722f 100644 --- a/platform-api/internal/service/mcp_test.go +++ b/platform-api/internal/service/mcp_test.go @@ -162,11 +162,10 @@ func TestFetchServerInfoRefetchUsesStoredURLVerbatim(t *testing.T) { } // TestFetchServerInfoRejectsAmbiguousTargets asserts the request-shape rules enforced by -// FetchServerInfo, matching the oneOf constraint on MCPServerInfoFetchRequest: exactly one -// of url/proxyId selects the target, and auth may not accompany proxyId (where the stored -// auth is authoritative) — no branch may silently ignore a supplied field. +// FetchServerInfo: at least one of url/proxyId must select the target, and auth may not +// accompany proxyId (where the stored auth is authoritative) — no branch may silently +// ignore a supplied field. func TestFetchServerInfoRejectsAmbiguousTargets(t *testing.T) { - url := "https://mcp.example.com/mcp" proxyID := "mcp-proxy-1" header, value := "X-API-Key", "secret" @@ -174,7 +173,6 @@ func TestFetchServerInfoRejectsAmbiguousTargets(t *testing.T) { name string req *api.MCPServerInfoFetchRequest }{ - {"both url and proxyId", &api.MCPServerInfoFetchRequest{Url: &url, ProxyId: &proxyID}}, {"neither url nor proxyId", &api.MCPServerInfoFetchRequest{}}, {"auth supplied with proxyId", &api.MCPServerInfoFetchRequest{ ProxyId: &proxyID, @@ -192,3 +190,64 @@ func TestFetchServerInfoRejectsAmbiguousTargets(t *testing.T) { }) } } + +// TestFetchServerInfoSuppliedURLWithStoredCredential covers the url+proxyId shape, which +// lets the UI validate an unsaved endpoint edit using the proxy's stored (write-only) +// credential. The supplied URL must override the stored one verbatim — no path +// manipulation, same as the proxyId-only flow — while the stored auth header still rides +// along, since that is the whole point of referencing the proxy. +func TestFetchServerInfoSuppliedURLWithStoredCredential(t *testing.T) { + var mu sync.Mutex + var paths []string + var authHeaders []string + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + paths = append(paths, r.URL.Path) + // Only the MCP calls themselves carry credentials; CheckURLReachability's + // preceding HEAD probe is a deliberately unauthenticated diagnostic. + if r.Method != http.MethodHead { + authHeaders = append(authHeaders, r.Header.Get("X-API-Key")) + } + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + w.Header().Set("mcp-session-id", "test-session") + _, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":1,"result":{"protocolVersion":"2025-06-18","serverInfo":{"name":"test","version":"1.0"}}}`)) + })) + defer srv.Close() + + proxy := &model.MCPProxy{ + Handle: "mcp-proxy-1", + Configuration: model.MCPProxyConfiguration{ + Upstream: model.UpstreamConfig{ + Main: &model.UpstreamEndpoint{ + URL: srv.URL + "/mcp", + // A literal (non-placeholder) stored value, so this exercises the + // request shape without needing the secret store. + Auth: &model.UpstreamAuth{Header: "X-API-Key", Value: "stored-secret"}, + }, + }, + }, + } + repo := &mockMCPProxyRepository{getByHandleResult: proxy} + service := NewMCPProxyService(repo, nil, nil, nil, nil, slog.Default(), &noopAuditRepo{}, &config.Server{}, newTestIdentityService()) + + proxyID := "mcp-proxy-1" + editedURL := srv.URL + "/api/v2/mcp" // an unsaved endpoint edit + _, err := service.FetchServerInfo("org-1", &api.MCPServerInfoFetchRequest{ + ProxyId: &proxyID, + Url: &editedURL, + }) + require.NoError(t, err) + + mu.Lock() + defer mu.Unlock() + require.NotEmpty(t, paths, "backend should have received at least one request") + for _, p := range paths { + assert.Equal(t, "/api/v2/mcp", p, "the supplied URL must override the stored one, verbatim") + } + require.NotEmpty(t, authHeaders, "backend should have received at least one MCP call") + for _, h := range authHeaders { + assert.Equal(t, "stored-secret", h, "the proxy's stored credential must still be sent") + } +} diff --git a/platform-api/resources/openapi.yaml b/platform-api/resources/openapi.yaml index 677f199f1b..46f76544e1 100644 --- a/platform-api/resources/openapi.yaml +++ b/platform-api/resources/openapi.yaml @@ -8396,37 +8396,37 @@ components: MCPServerInfoFetchRequest: type: object - oneOf: + anyOf: - required: [ "url" ] - required: [ "proxyId" ] - not: - required: [ "auth" ] + not: + required: [ "proxyId", "auth" ] description: | - Target MCP server to introspect. Provide exactly one of `url` (a direct backend URL, - optionally with `auth`) or `proxyId` (refetch using a stored proxy configuration) — never - both. `auth` must not be sent together with `proxyId`; the stored auth is always used in - the refetch flow. + Target MCP server to introspect, and the credentials to introspect it with. At least + one of `url`/`proxyId` must be provided properties: url: type: string format: uri description: | - Endpoint URL of the MCP server to fetch information from. Mutually exclusive - with `proxyId`; exactly one of the two must be provided. + Endpoint URL of the MCP server to fetch information from. Required unless + `proxyId` is given. When sent together with `proxyId` it overrides that proxy's + stored upstream URL, while the proxy's stored credentials are still used — this + validates an unsaved endpoint edit without re-sending a write-only secret. example: https://mcp.server.com/mcp proxyId: type: string description: | - MCP proxy handle (identifier) for refresh operations. When provided, the server - fetches URL and auth from the stored proxy configuration. Mutually exclusive - with `url`; exactly one of the two must be provided. + MCP proxy handle (identifier) for refresh operations. The stored credentials of + this proxy are used for the fetch, and its stored upstream URL too unless `url` + overrides it. Required unless `url` is given. example: my-mcp-proxy auth: $ref: '#/components/schemas/UpstreamAuth' description: | - Authentication configuration for the fetch request. Allowed only alongside - `url` (initial creation flow); sending it with `proxyId` is rejected, as the - stored auth is used in the refetch flow. + Authentication configuration for the fetch request. Allowed only when `proxyId` + is absent (initial creation flow); sending it with `proxyId` is rejected, as the + stored auth is used whenever a proxy is referenced. MCPServerInfoFetchResponse: type: object diff --git a/portals/ai-workspace/cypress/e2e/002-mcp-proxies/005-mcp-backend-connection-refetch.cy.js b/portals/ai-workspace/cypress/e2e/002-mcp-proxies/005-mcp-backend-connection-refetch.cy.js index 9dbddb43c0..ecbfd94277 100644 --- a/portals/ai-workspace/cypress/e2e/002-mcp-proxies/005-mcp-backend-connection-refetch.cy.js +++ b/portals/ai-workspace/cypress/e2e/002-mcp-proxies/005-mcp-backend-connection-refetch.cy.js @@ -19,18 +19,19 @@ /** * "Refetch Server Info" behaviour on the MCP Proxy — Backend Connection tab. * - * ExternalServersOverview's handleRefetch has two mutually-exclusive request - * shapes, chosen by whether the endpoint URL / auth header / auth value differ - * from what is currently persisted on the server: + * ExternalServersOverview's handleRefetch picks one of three request shapes, + * according to which credential the fetch has to run with: * - * - Unedited: POST /mcp-proxies/fetch-server-info { url, proxyId } — the + * - Unedited: POST /mcp-proxies/fetch-server-info { proxyId } — the * backend resolves the stored URL and auth (via the saved secret handle) * itself, so the browser never needs to re-send a credential it can't * read back (auth.value is writeOnly). - * - Edited: POST /mcp-proxies/fetch-server-info { url, auth: { type, - * header, value } } — validates the live, not-yet-saved values directly. - * proxyId and an auth override are mutually exclusive by API contract, so - * this omits proxyId. + * - Endpoint edited, credential untouched: { url, proxyId } — the backend + * fetches the unsaved URL using the stored secret, so an endpoint change + * can be verified without re-entering a credential the browser can't read. + * - Credential typed: { url, auth: { type, header, value } } — validates + * the live, not-yet-saved values directly. auth and proxyId are mutually + * exclusive by API contract, so this omits proxyId. * * Covers: * TC-98 Refetch with nothing edited → request uses proxyId, omits auth @@ -199,7 +200,7 @@ describe('AI Workspace — MCP proxy Backend Connection tab (Refetch Server Info cy.wait('@refetch').then((pi) => { const body = pi.request.body; expect(body.proxyId, 'refetch request carries proxyId').to.equal(createdServerId); - expect(body.url, 'refetch request still carries the current url').to.equal(ORIGINAL_URL); + expect(body.url, 'url is omitted — mutually exclusive with proxyId').to.be.undefined; expect(body.auth, 'no auth override sent alongside proxyId').to.be.undefined; }); @@ -297,7 +298,8 @@ describe('AI Workspace — MCP proxy Backend Connection tab (Refetch Server Info cy.wait('@refetchAfterSave').then((pi) => { const body = pi.request.body; expect(body.proxyId, 'refetch after save uses proxyId again').to.equal(createdServerId); - expect(body.url, 'refetch after save uses the newly saved url').to.equal(newUrl); + expect(body.url, 'url omitted — the backend resolves the newly saved url').to.be + .undefined; expect(body.auth, 'no auth override needed post-save').to.be.undefined; }); }); @@ -340,4 +342,37 @@ describe('AI Workspace — MCP proxy Backend Connection tab (Refetch Server Info }); }); }); + + // --------------------------------------------------------------------------- + // TC-102 + // --------------------------------------------------------------------------- + it('TC-102: refetching after a URL-only edit sends url + proxyId, reusing the stored credential', () => { + // An unsaved endpoint edit. The credential field is never focused, so it stays + // masked and the browser has no value to send; proxyId is what lets the backend + // supply it. + const editedPathUrl = 'https://sample.mcp.example.com/v2/mcp'; + + cy.get('[data-testid="backend-connection-endpoint-url"]') + .clear() + .type(editedPathUrl); + cy.get('[data-testid="backend-connection-auth-value"]') + .should('have.value', '******'); + + stubFetchServerInfo('refetchEditedUrl'); + + cy.get('[data-testid="backend-connection-refetch"]', { timeout: 15000 }) + .should('not.be.disabled') + .click(); + + cy.wait('@refetchEditedUrl').then((pi) => { + const body = pi.request.body; + expect(body.url, 'the unsaved, edited url is what gets fetched').to.equal(editedPathUrl); + expect(body.proxyId, 'proxyId supplies the stored credential').to.equal(createdServerId); + // The masked sentinel must never be sent as if it were a credential, and no + // plaintext is available client-side to send either. + expect(body.auth, 'auth is never combined with proxyId').to.be.undefined; + }); + + cy.contains('Connection verified', { timeout: 15000 }).should('be.visible'); + }); }); diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx index de7599718d..4b3be11b7c 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/externalServers/ExternalServersOverview.tsx @@ -682,23 +682,22 @@ export default function ExternalServersOverview(): JSX.Element { } const trimmedHeaderName = authHeaderName.trim(); + const storedHeaderName = (server.upstream?.main?.auth?.header ?? '').trim(); const endpointUnchanged = trimmedUrl === (server.upstream?.main?.url ?? '').trim(); - const headerNameUnchanged = - trimmedHeaderName === (server.upstream?.main?.auth?.header ?? '').trim(); - const credentialUnchanged = isCredentialMasked || !hasCredentialChanged; + const headerNameUnchanged = trimmedHeaderName === storedHeaderName; setIsRefetching(true); try { + // Which credential the fetch runs with is what picks the request shape. A live + // value the user just typed goes on the wire directly; otherwise the credential + // is write-only (masked as ******) and only the backend can resolve it, which it + // does from proxyId. auth never accompanies proxyId — whenever a proxy is + // referenced, its stored auth is authoritative and an override is rejected. let request: MCPServerInfoFetchRequest; - if (endpointUnchanged && headerNameUnchanged && credentialUnchanged) { - // Nothing edited yet — let the backend resolve the stored URL and auth (via the - // saved secret handle) from the persisted proxy config instead of us re-entering - // a credential we can never read back. - request = { url: trimmedUrl, proxyId: server.id }; - } else if (trimmedHeaderName && !isCredentialMasked && authHeaderValue.trim()) { - // Something was edited and we have a live credential value to validate with. - // proxyId + auth override together aren't allowed, so this omits proxyId. + if (trimmedHeaderName && !isCredentialMasked && authHeaderValue.trim()) { + // A new or rotated credential was typed — validate the endpoint against it + // directly, before anything is saved. request = { url: trimmedUrl, auth: { @@ -707,14 +706,28 @@ export default function ExternalServersOverview(): JSX.Element { value: authHeaderValue.trim(), }, }; - } else if (trimmedHeaderName && isCredentialMasked) { + } else if (!trimmedHeaderName) { + // No auth header configured, or the user is clearing it — validate the way the + // saved proxy would actually behave, unauthenticated. + request = { url: trimmedUrl }; + } else if (!headerNameUnchanged) { + // The header was renamed but the value is still masked. proxyId would resolve + // the stored header name too, so there is no shape that pairs the new name with + // the stored value — the user has to supply one or the other. showSnackbar( - 'Enter the authentication header value to refetch with the updated endpoint or header.', + 'Enter the authentication header value, or save your changes first, to refetch with the renamed header.', 'error' ); return; + } else if (endpointUnchanged) { + // Nothing to override — the backend resolves both the stored URL and the stored + // credential from the persisted config. + request = { proxyId: server.id }; } else { - request = { url: trimmedUrl }; + // The endpoint was edited but the credential was not. Send both: the backend + // fetches the unsaved URL using the stored secret, so the user can verify an + // endpoint change without re-entering a credential they can never read back. + request = { url: trimmedUrl, proxyId: server.id }; } const response = await mcpServerValidationApis.fetchMCPProxyServerInfo( diff --git a/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx b/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx index 28ef2fa2f0..a0c29920ba 100644 --- a/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx +++ b/portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsx @@ -345,8 +345,7 @@ export default function ServiceProviderNew() { const providerId = toProviderId(formState.name); // The API key is optional. A credential-bearing auth type with no key - // entered stores nothing to inject, so record it as 'other' — upstream - // authentication is then left to user-attached policies. Sending + // entered stores nothing to inject, so record it as 'none'. Sending // 'api-key' with an empty value creates a provider the gateway rejects // at deployment time. const hasCredential = Boolean(formState.upstreamAuthValue.trim()); @@ -358,7 +357,7 @@ export default function ServiceProviderNew() { ? { type: isNoCredentialsAuthType ? formState.upstreamAuthType - : 'other', + : 'none', header: '', value: '', } diff --git a/portals/ai-workspace/src/utils/types.ts b/portals/ai-workspace/src/utils/types.ts index d85327a33e..d359d5c606 100644 --- a/portals/ai-workspace/src/utils/types.ts +++ b/portals/ai-workspace/src/utils/types.ts @@ -837,19 +837,36 @@ export type MCPServerListResponse = ApiListResponse; /** * MCP Server Info Fetch Request + * + * At least one of `url`/`proxyId` is required, and `auth` may never accompany `proxyId` + * — whenever a proxy is referenced its stored auth is authoritative and an override is + * rejected. Modelled as a union so that invalid combination doesn't compile. The three + * valid shapes are: + * + * { url, auth? } — contact this URL with these (or no) credentials + * { proxyId } — contact the stored URL with the stored credentials + * { url, proxyId } — contact this URL with the stored credentials, so an unsaved + * endpoint edit can be validated without re-sending the secret */ -export interface MCPServerInfoFetchRequest { - url: string; - proxyId?: string; +interface MCPServerInfoFetchRequestBase { transportType?: string; headers?: Record; - auth?: { - type: string; - header: string; - value: string; - }; } +export type MCPServerInfoFetchRequest = MCPServerInfoFetchRequestBase & + ( + | { + url: string; + proxyId?: never; + auth?: { + type: string; + header: string; + value: string; + }; + } + | { proxyId: string; url?: string; auth?: never } + ); + /** * MCP Server Info Fetch Response */