From 894328d6b1224eafaa24c9f3be6be91f7b1e8737 Mon Sep 17 00:00:00 2001 From: Serguei Gorokhov Date: Fri, 4 Sep 2026 10:06:12 +0300 Subject: [PATCH] feat: wire dependency resolution into the MCP-application-proxy mint site ApplicationMcpProxyController's enhancement chain never included ResolveResourceDependenciesFn -- an application invoked over /v1/deployments/{id}/mcp got no dependency grants however it declared and however consented it was (documented v1 limitation, P-6). Adding the function to the chain as-is is not a one-line change: the chain is typed over ObjectNode, not RequestObject, and this controller mints its per-request key *after* the chain runs (inside McpUpstreamAuthInjector, at header-injection time) -- so a naive wiring would NPE on a null proxyApiKeyData on every call. ResolveResourceDependenciesFn is genericized ( extends BaseRequestFunction) so the one implementation joins both RequestObject-typed chains (the three conversation mint sites) and the ObjectNode-typed MCP chain -- the request body was never read by this function to begin with. McpProxyController gains a preAssignsPerRequestKey() hook (gated on the app's own mcp.forwardPerRequestKey, default true): when set, the per-request key is created before the enhancement chain runs (so the chain can bake a grant into it) and persisted to Redis only after the chain succeeds -- the same create/chain/assign order DeploymentPostController already uses. The body-handling dispatch moves from the Vert.x event loop onto the task executor to match, since the chain now does blocking consent/permission/Redis work. McpUpstreamAuthInjector's key mint is made idempotent: handleProxyRequest re-enters on every 429 retry, connection failure, and same-origin 307/308 redirect (the common Starlette/FastMCP trailing-slash case), and the old unconditional mint would replace the grant-bearing key with a fresh, grant-free one on the very first such re-entry -- and orphan the previous Redis-backed key, since only the current key is invalidated on completion. It now reuses an already-assigned key instead. A required dependency with nowhere to bake a grant into (forwardPerRequestKey disabled) is treated exactly like an unconsented one -- still hard-fails the call rather than silently succeeding for an app that can never receive the access it declared as required. Known edge cases, not fixed here (recorded for the team): a required dependency combined with forwardPerRequestKey disabled is a permanent, unvalidated 403 with no write-time signal -- a write-time cross-field check belongs in ResourceDependencyValidator as separate follow-up. tools/call allow-list enforcement (isToolCallAllowed) is gated on a client-controlled Content-Type header, a pre-existing gap this PR does not introduce but now places a second security-relevant function (this one) behind the same gate; the grant-baking side is fail-closed under that bypass. Spec: documentation repo, offline-access-delegation/implementation-specs/pr5b-mcp-mint-site.md Co-Authored-By: Claude Code --- .../ApplicationMcpProxyController.java | 10 +- .../controller/DeploymentPostController.java | 2 +- .../server/controller/McpProxyController.java | 29 ++- .../controller/ResponsesController.java | 2 +- .../anthropic/MessagesBaseController.java | 2 +- .../ResolveResourceDependenciesFn.java | 19 +- .../server/util/McpUpstreamAuthInjector.java | 18 +- .../ResourceDependencyResolutionApiTest.java | 205 ++++++++++++++++++ .../ResolveResourceDependenciesFnTest.java | 51 ++++- 9 files changed, 322 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ApplicationMcpProxyController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ApplicationMcpProxyController.java index b85e07ff8..09b8ff6b4 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ApplicationMcpProxyController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ApplicationMcpProxyController.java @@ -12,6 +12,7 @@ import com.epam.aidial.core.server.Proxy; import com.epam.aidial.core.server.ProxyContext; import com.epam.aidial.core.server.function.BaseRequestFunction; +import com.epam.aidial.core.server.function.ResolveResourceDependenciesFn; import com.epam.aidial.core.server.function.enhancement.InjectApplicationPropsToMcpRequest; import com.epam.aidial.core.server.service.ApplicationSchemaService; import com.epam.aidial.core.server.util.McpUpstreamAuthInjector; @@ -37,7 +38,9 @@ public class ApplicationMcpProxyController extends McpProxyController { public ApplicationMcpProxyController(Proxy proxy, ProxyContext context, String toolSetId) { super(proxy, context, toolSetId); this.applicationSchemaService = proxy.getApplicationSchemaService(); - this.enhancementFunctions = List.of(new InjectApplicationPropsToMcpRequest(proxy, context)); + this.enhancementFunctions = List.of( + new InjectApplicationPropsToMcpRequest(proxy, context), + new ResolveResourceDependenciesFn<>(proxy, context)); this.authInjector = new McpUpstreamAuthInjector(proxy); } @@ -91,6 +94,11 @@ protected String getUpstreamEndpoint(Deployment deployment) { return application.getMcp().getEndpoint(); } + @Override + protected boolean preAssignsPerRequestKey() { + return application.getMcp().isForwardPerRequestKey(); + } + @Override protected void injectProxyRequestHeaders(HttpClientRequest proxyRequest, MultiMap excludeHeaders) { excludeHeaders.add(HEADER_APPLICATION_PROPERTIES, "whatever"); diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/DeploymentPostController.java b/server/src/main/java/com/epam/aidial/core/server/controller/DeploymentPostController.java index 5d9f41870..a6a401628 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/DeploymentPostController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/DeploymentPostController.java @@ -80,7 +80,7 @@ private List> buildEnhancementFunctions() { new CollectRequestApplicationFilesFn(proxy, context), new BuildUpstreamCacheFn(proxy, context, InterfaceType.OPENAI_CHAT_COMPLETIONS), new CollectDeploymentsFn(proxy, context), - new ResolveResourceDependenciesFn(proxy, context)); + new ResolveResourceDependenciesFn<>(proxy, context)); } @ApiOperation( diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/McpProxyController.java b/server/src/main/java/com/epam/aidial/core/server/controller/McpProxyController.java index 035118911..30a48fb67 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/McpProxyController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/McpProxyController.java @@ -165,6 +165,13 @@ private void sendRequest(String absoluteUri) { private void handleRequestBody(Buffer requestBody) { context.setRequestBody(requestBody); + boolean preAssign = preAssignsPerRequestKey(); + if (preAssign) { + // The enhancement chain writes into this key (ResolveResourceDependenciesFn bakes + // dependency grants into it), so it must exist before the chain runs and be persisted + // only after — grants written after assignPerRequestApiKey would never reach Redis. + createProxyApiKeyData(); + } String contentType = context.getRequest().getHeader(HttpHeaders.CONTENT_TYPE); if (Strings.CI.contains(contentType, HEADER_CONTENT_TYPE_APPLICATION_JSON)) { @@ -190,6 +197,9 @@ private void handleRequestBody(Buffer requestBody) { return; } } + if (preAssign) { + apiKeyStore.assignPerRequestApiKey(context.getProxyApiKeyData()); + } sendRequest(); } @@ -228,12 +238,20 @@ private void handleProxyRequest(HttpClientRequest proxyRequest) { protected void injectProxyRequestHeaders(HttpClientRequest proxyRequest, MultiMap excludeHeaders) { } - protected String assignPerRequestKey() { + /** + * Whether this controller mints the upstream's per-request key before the request + * enhancement chain runs, so chain functions can write into it (D-25). Default {@code false}: + * the base MCP path leaves the mint to {@link com.epam.aidial.core.server.util.McpUpstreamAuthInjector} + * at header-injection time, which is where it has always happened. + */ + protected boolean preAssignsPerRequestKey() { + return false; + } + + private void createProxyApiKeyData() { ApiKeyData proxyApiKeyData = new ApiKeyData(); context.setProxyApiKeyData(proxyApiKeyData); ApiKeyData.initFromContext(proxyApiKeyData, context); - apiKeyStore.assignPerRequestApiKey(proxyApiKeyData); - return proxyApiKeyData.getPerRequestKey(); } /** @@ -446,7 +464,10 @@ private Future handleRateLimitSuccess(Deployment deployment) { context.setTraceOperation("Send request to %s deployment".formatted(deployment.getName())); context.getRequest().body() .onFailure(this::handleRequestBodyError) - .onSuccess(this::handleRequestBody); + .onSuccess(body -> taskExecutor.submit(() -> { + handleRequestBody(body); + return null; + }).onFailure(this::handleError)); return null; }); } diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ResponsesController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ResponsesController.java index 866ac8ca0..f26ce9b1b 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ResponsesController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ResponsesController.java @@ -74,7 +74,7 @@ public ResponsesController(Proxy proxy, ProxyContext context) { new CollectRequestApplicationFilesFn(proxy, context), new BuildUpstreamCacheFn(proxy, context, InterfaceType.OPENAI_RESPONSES), new CollectDeploymentsFn(proxy, context), - new ResolveResourceDependenciesFn(proxy, context)); + new ResolveResourceDependenciesFn<>(proxy, context)); } @Override diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/anthropic/MessagesBaseController.java b/server/src/main/java/com/epam/aidial/core/server/controller/anthropic/MessagesBaseController.java index 4bc676833..d5aafee03 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/anthropic/MessagesBaseController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/anthropic/MessagesBaseController.java @@ -74,7 +74,7 @@ protected List> buildEnhancementFunctions() { new EnhanceDeploymentRequestFn(proxy, context), new CollectRequestApplicationFilesFn(proxy, context), new CollectDeploymentsFn(proxy, context), - new ResolveResourceDependenciesFn(proxy, context)); + new ResolveResourceDependenciesFn<>(proxy, context)); } public Future handle() { diff --git a/server/src/main/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFn.java b/server/src/main/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFn.java index a24fdaefb..30730ff5e 100644 --- a/server/src/main/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFn.java +++ b/server/src/main/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFn.java @@ -9,7 +9,6 @@ import com.epam.aidial.core.server.data.AuthBucket; import com.epam.aidial.core.server.data.consent.Consent; import com.epam.aidial.core.server.data.permission.PerRequestSharedData; -import com.epam.aidial.core.server.function.request.RequestObject; import com.epam.aidial.core.server.log.ResourceDependencyAuditLog; import com.epam.aidial.core.server.security.AccessService; import com.epam.aidial.core.server.service.ResourceDependencyValidator; @@ -49,14 +48,20 @@ * {@code AccessService.lookupOriginatingUserPermissions} — own bucket as the human, shared, * public — and never through rules that read the calling key's own grants (D-24). */ -public class ResolveResourceDependenciesFn extends BaseRequestFunction { +public class ResolveResourceDependenciesFn extends BaseRequestFunction { public ResolveResourceDependenciesFn(Proxy proxy, ProxyContext context) { super(proxy, context); } + /** + * The request body is not an input here — resolution reads only {@code context}. The type + * parameter exists solely so this function can join chains of either element type + * ({@code RequestObject} on the conversation mint sites, {@code ObjectNode} on the MCP + * proxy); see D-25a. + */ @Override - public Boolean apply(RequestObject request) { + public Boolean apply(T ignored) { if (!(context.getDeployment() instanceof Application application)) { // Interceptor hop or a non-application deployment — dependencies resolve for the // application being called, nothing else. @@ -72,9 +77,15 @@ public Boolean apply(RequestObject request) { private void resolve(Application application, List declaration) { String applicationId = application.getName(); + // Nowhere to bake a grant into — e.g. an MCP application with forwardPerRequestKey + // disabled never gets a per-request key at all, so it can never receive one regardless of + // consent or reach. Treated exactly like "not consented": every declared dependency is + // unresolved, so a required one still hard-fails the call instead of silently succeeding + // for an app that was promised access it can never actually receive. + boolean hasKeyToBakeInto = context.getProxyApiKeyData() != null; // The consent record is content-bound to the whole declaration: any change since the grant // re-requires it, and until then nothing resolves. Checked before any resolution work. - boolean consented = proxy.getConsentService().isAdminConsented(applicationId, declaration); + boolean consented = hasKeyToBakeInto && proxy.getConsentService().isAdminConsented(applicationId, declaration); AuthBucket userBucket = BucketBuilder.buildBucket(context); List resolvedTargets = new ArrayList<>(); diff --git a/server/src/main/java/com/epam/aidial/core/server/util/McpUpstreamAuthInjector.java b/server/src/main/java/com/epam/aidial/core/server/util/McpUpstreamAuthInjector.java index 1d733b341..6b7b333c5 100644 --- a/server/src/main/java/com/epam/aidial/core/server/util/McpUpstreamAuthInjector.java +++ b/server/src/main/java/com/epam/aidial/core/server/util/McpUpstreamAuthInjector.java @@ -49,7 +49,7 @@ public void inject(BiConsumer headers, ToolSet toolSet, headers.accept(authHeader.getHeaderName(), authHeader.getHeaderValue()); } if (toolSet.isForwardPerRequestKey()) { - headers.accept(Proxy.HEADER_API_KEY, assignPerRequestKey(context)); + headers.accept(Proxy.HEADER_API_KEY, perRequestKey(context)); } } @@ -61,7 +61,7 @@ public void inject(BiConsumer headers, Application app, ProxyCon headers.accept(HEADER_APPLICATION_ID, app.getName()); Application.Mcp mcp = app.getMcp(); if (mcp.isForwardPerRequestKey()) { - headers.accept(Proxy.HEADER_API_KEY, assignPerRequestKey(context)); + headers.accept(Proxy.HEADER_API_KEY, perRequestKey(context)); } if (mcp.getConfigDelivery() == Application.McpConfigDelivery.HEADER) { applicationSchemaService.consumeMetadataProperties(app, (properties, appendHeader) -> { @@ -73,7 +73,19 @@ public void inject(BiConsumer headers, Application app, ProxyCon } } - private String assignPerRequestKey(ProxyContext context) { + /** + * Returns the per-request key the upstream should receive, minting one only if this request + * does not already have one. Re-entry is normal on this path: handleProxyRequest runs again + * on every 429 retry, connection/send failure, and same-origin 307/308 redirect. Minting + * afresh each time would orphan the previous Redis-backed key — only the current + * proxyApiKeyData is invalidated on completion — and would hand the upstream a key that does + * not carry the grants baked into the one built before the enhancement chain. + */ + private String perRequestKey(ProxyContext context) { + ApiKeyData assigned = context.getProxyApiKeyData(); + if (assigned != null && assigned.getPerRequestKey() != null) { + return assigned.getPerRequestKey(); + } ApiKeyData keyData = new ApiKeyData(); context.setProxyApiKeyData(keyData); ApiKeyData.initFromContext(keyData, context); diff --git a/server/src/test/java/com/epam/aidial/core/server/ResourceDependencyResolutionApiTest.java b/server/src/test/java/com/epam/aidial/core/server/ResourceDependencyResolutionApiTest.java index 9088faeff..b60def101 100644 --- a/server/src/test/java/com/epam/aidial/core/server/ResourceDependencyResolutionApiTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/ResourceDependencyResolutionApiTest.java @@ -11,10 +11,12 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicReference; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; /** * The v1 happy path, end to end on the chat-completions mint site: an admin-authored app @@ -180,4 +182,207 @@ void testChainedHopIsSandboxedFromTheCallersOwnGrants() { assertEquals(403, targetStatus.get(), "a grant already held by the calling app must not satisfy the called app's reach check"); } + + // ---- the MCP-application-proxy mint site (PR5b) ---- + + @Test + void testMcpMintSiteBakesConsentedDependencyGrantIntoThePerRequestKey() { + Response response = send(HttpMethod.GET, "/v1/bucket", null, "", "authorization", "user"); + String userBucket = new JsonObject(response.body()).getString("bucket"); + String appUrl = "applications/public/dep-mcp-app"; + + verify(send(HttpMethod.PUT, "/v1/applications/public/dep-mcp-app", null, """ + { + "display_name": "Dependency MCP App", + "mcp": {"endpoint": "http://localhost:4845/mcp", "transport": "HTTP"}, + "resource_dependencies": [ + {"kind": "dial.resourceLink", "link_id": "lnk_skills", + "target": {"path": "prompts/{current-user}/mcp-demo/"}, "access": ["write"], "required": true} + ] + } + """, "authorization", "admin", "If-None-Match", "*"), 200); + verify(send(HttpMethod.POST, "/v1/consent/" + appUrl + "/admin-consent", null, "", + "authorization", "admin"), 200); + + AtomicReference inScopeStatus = new AtomicReference<>(); + AtomicReference outOfScopeStatus = new AtomicReference<>(); + try (TestWebServer server = new TestWebServer(4845)) { + server.map(HttpMethod.POST, "/mcp", request -> { + String perRequestKey = request.getHeader(Proxy.HEADER_API_KEY); + inScopeStatus.set(send(HttpMethod.PUT, + "/v1/prompts/%s/mcp-demo/skill-by-app".formatted(userBucket), null, + "{\"id\":\"skill-by-app\",\"folderId\":\"mcp-demo/\",\"name\":\"skill-by-app\",\"content\":\"app content\"}", + "api-key", perRequestKey).status()); + outOfScopeStatus.set(send(HttpMethod.PUT, + "/v1/prompts/%s/outside-scope".formatted(userBucket), null, + "{\"id\":\"skill-by-app\",\"folderId\":\"mcp-demo/\",\"name\":\"skill-by-app\",\"content\":\"app content\"}", + "api-key", perRequestKey).status()); + return TestWebServer.createResponse(200, "{\"result\":\"ok\"}", "Content-Type", "application/json"); + }); + + Response completion = send(HttpMethod.POST, "/v1/deployments/" + appUrl + "/mcp", null, """ + {"payload":"foo"} + """, "authorization", "user", "Content-type", "application/json"); + assertEquals(200, completion.status(), () -> "Body: " + completion.body()); + } + + // Before this PR: no grant at all (the function was absent from the chain). After: baked. + assertEquals(200, inScopeStatus.get(), "the consented dependency target must be writable by the app's key"); + assertEquals(403, outOfScopeStatus.get(), "anything outside the declared target must stay off-limits"); + } + + @Test + void testMcpMintSiteSurvivesAnUpstreamTrailingSlashRedirect() { + // The D-25b pin: handleProxyRequest re-enters on a same-origin redirect. Without the + // idempotent mint (McpUpstreamAuthInjector.perRequestKey), the second hit would carry a + // fresh, grant-free key instead of the one built before the enhancement chain ran. + Response response = send(HttpMethod.GET, "/v1/bucket", null, "", "authorization", "user"); + String userBucket = new JsonObject(response.body()).getString("bucket"); + String appUrl = "applications/public/dep-mcp-redirect-app"; + + verify(send(HttpMethod.PUT, "/v1/applications/public/dep-mcp-redirect-app", null, """ + { + "display_name": "Dependency MCP Redirect App", + "mcp": {"endpoint": "http://localhost:4844/mcp", "transport": "HTTP"}, + "resource_dependencies": [ + {"kind": "dial.resourceLink", "link_id": "lnk_skills", + "target": {"path": "prompts/{current-user}/mcp-demo/"}, "access": ["write"], "required": true} + ] + } + """, "authorization", "admin", "If-None-Match", "*"), 200); + verify(send(HttpMethod.POST, "/v1/consent/" + appUrl + "/admin-consent", null, "", + "authorization", "admin"), 200); + + List observedKeys = new CopyOnWriteArrayList<>(); + AtomicReference targetStatus = new AtomicReference<>(); + try (TestWebServer server = new TestWebServer(4844)) { + server.map(HttpMethod.POST, "/mcp", request -> { + observedKeys.add(request.getHeader(Proxy.HEADER_API_KEY)); + return TestWebServer.createResponse(308, "", "Location", "/mcp/"); + }); + server.map(HttpMethod.POST, "/mcp/", request -> { + String perRequestKey = request.getHeader(Proxy.HEADER_API_KEY); + observedKeys.add(perRequestKey); + targetStatus.set(send(HttpMethod.PUT, + "/v1/prompts/%s/mcp-demo/skill-by-app".formatted(userBucket), null, + "{\"id\":\"skill-by-app\",\"folderId\":\"mcp-demo/\",\"name\":\"skill-by-app\",\"content\":\"app content\"}", + "api-key", perRequestKey).status()); + return TestWebServer.createResponse(200, "{\"result\":\"ok\"}", "Content-Type", "application/json"); + }); + + Response completion = send(HttpMethod.POST, "/v1/deployments/" + appUrl + "/mcp", null, """ + {"payload":"foo"} + """, "authorization", "user", "Content-type", "application/json"); + assertEquals(200, completion.status(), () -> "Body: " + completion.body()); + } + + assertEquals(2, observedKeys.size(), "expected exactly two upstream hits: the redirect and the target"); + assertEquals(observedKeys.get(0), observedKeys.get(1), + "the same per-request key must be forwarded on both the redirected and the final request"); + assertEquals(200, targetStatus.get(), "the grant baked before the chain ran must still be usable after the redirect"); + } + + @Test + void testMcpMintSiteFailsTheCallWhenRequiredDependencyIsUnresolvable() { + // The regression pin for the mint-timing inversion: a null-proxyApiKeyData NPE would have + // surfaced as 400 "Invalid JSON request body"; the correct outcome is a clean 403 from the + // resolver itself. + String appUrl = "applications/public/dep-mcp-fail-app"; + verify(send(HttpMethod.PUT, "/v1/applications/public/dep-mcp-fail-app", null, """ + { + "display_name": "Dependency MCP Fail App", + "mcp": {"endpoint": "http://localhost:4843/mcp", "transport": "HTTP"}, + "resource_dependencies": [ + {"kind": "dial.resourceLink", "link_id": "lnk_skills", + "target": {"path": "prompts/{current-user}/mcp-demo/"}, "access": ["write"], "required": true} + ] + } + """, "authorization", "admin", "If-None-Match", "*"), 200); + // No admin consent granted. + + Response completion = send(HttpMethod.POST, "/v1/deployments/" + appUrl + "/mcp", null, """ + {"payload":"foo"} + """, "authorization", "user", "Content-type", "application/json"); + + assertEquals(403, completion.status(), () -> "Body: " + completion.body()); + } + + @Test + void testMcpMintSiteGrantsNothingWhenTheAppDoesNotForwardThePerRequestKey() { + String appUrl = "applications/public/dep-mcp-noforward-app"; + verify(send(HttpMethod.PUT, "/v1/applications/public/dep-mcp-noforward-app", null, """ + { + "display_name": "Dependency MCP No-Forward App", + "mcp": {"endpoint": "http://localhost:4842/mcp", "transport": "HTTP", "forwardPerRequestKey": false}, + "resource_dependencies": [ + {"kind": "dial.resourceLink", "link_id": "lnk_skills", + "target": {"path": "prompts/{current-user}/mcp-demo/"}, "access": ["write"], "required": false} + ] + } + """, "authorization", "admin", "If-None-Match", "*"), 200); + verify(send(HttpMethod.POST, "/v1/consent/" + appUrl + "/admin-consent", null, "", + "authorization", "admin"), 200); + + AtomicReference observedKey = new AtomicReference<>(); + try (TestWebServer server = new TestWebServer(4842)) { + server.map(HttpMethod.POST, "/mcp", request -> { + observedKey.set(request.getHeader(Proxy.HEADER_API_KEY)); + return TestWebServer.createResponse(200, "{\"result\":\"ok\"}", "Content-Type", "application/json"); + }); + + Response completion = send(HttpMethod.POST, "/v1/deployments/" + appUrl + "/mcp", null, """ + {"payload":"foo"} + """, "authorization", "user", "Content-type", "application/json"); + assertEquals(200, completion.status(), () -> "Body: " + completion.body()); + } + + assertNull(observedKey.get(), "no Api-Key header must reach the upstream when forwardPerRequestKey is false"); + } + + @Test + void testMcpChainedHopResolvesItsOwnDeclaration() { + // The demo shape: an orchestrator's own per-request key calls the CRUD app over MCP. The + // CRUD app's own declaration resolves against the originating human at that depth — PR4b's + // behaviour, reachable from this mint site for the first time. + Response response = send(HttpMethod.GET, "/v1/bucket", null, "", "authorization", "user"); + String userBucket = new JsonObject(response.body()).getString("bucket"); + String appUrl = "applications/public/dep-mcp-chained-app"; + + verify(send(HttpMethod.PUT, "/v1/applications/public/dep-mcp-chained-app", null, """ + { + "display_name": "Dependency MCP Chained App", + "mcp": {"endpoint": "http://localhost:4841/mcp", "transport": "HTTP"}, + "resource_dependencies": [ + {"kind": "dial.resourceLink", "link_id": "lnk_skills", + "target": {"path": "prompts/{current-user}/mcp-demo/"}, "access": ["write"], "required": true} + ] + } + """, "authorization", "admin", "If-None-Match", "*"), 200); + verify(send(HttpMethod.POST, "/v1/consent/" + appUrl + "/admin-consent", null, "", + "authorization", "admin"), 200); + + // The key an orchestrator would hold when delegating to the declaring app over MCP. + ApiKeyData orchestratorKey = createAppKey("user", Map.of()); + orchestratorKey.setExecutionPath(List.of("orchestrator")); + apiKeyStore.assignPerRequestApiKey(orchestratorKey); + + AtomicReference targetStatus = new AtomicReference<>(); + try (TestWebServer server = new TestWebServer(4841)) { + server.map(HttpMethod.POST, "/mcp", request -> { + targetStatus.set(send(HttpMethod.PUT, + "/v1/prompts/%s/mcp-demo/skill-by-app".formatted(userBucket), null, + "{\"id\":\"skill-by-app\",\"folderId\":\"mcp-demo/\",\"name\":\"skill-by-app\",\"content\":\"app content\"}", + "api-key", request.getHeader(Proxy.HEADER_API_KEY)).status()); + return TestWebServer.createResponse(200, "{\"result\":\"ok\"}", "Content-Type", "application/json"); + }); + + Response completion = send(HttpMethod.POST, "/v1/deployments/" + appUrl + "/mcp", null, """ + {"payload":"foo"} + """, "api-key", orchestratorKey.getPerRequestKey(), "Content-type", "application/json"); + assertEquals(200, completion.status(), () -> "Body: " + completion.body()); + } + + assertEquals(200, targetStatus.get(), + "a chained MCP hop must resolve its own declaration against the originating user"); + } } diff --git a/server/src/test/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFnTest.java b/server/src/test/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFnTest.java index 37fbc25f9..b82e2096b 100644 --- a/server/src/test/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFnTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFnTest.java @@ -66,7 +66,7 @@ public class ResolveResourceDependenciesFnTest { private EncryptionService encryptionService; @InjectMocks - private ResolveResourceDependenciesFn fn; + private ResolveResourceDependenciesFn fn; private final ApiKeyData proxyApiKeyData = new ApiKeyData(); private Application application; @@ -118,6 +118,35 @@ void apply_isNoOpWithoutDeclaration() { verify(consentService, never()).isAdminConsented(anyString(), any()); } + @Test + void apply_isNoOpWithoutProxyApiKeyDataToBakeInto() { + // An MCP application with forwardPerRequestKey disabled never gets a per-request key at + // all, so context.getProxyApiKeyData() is null when this function runs — nowhere to bake + // a grant into regardless of consent or reach. Treated as unconsented: no NPE, and (since + // this record is optional) no failure either — see the next test for the required case. + when(context.getDeployment()).thenReturn(application); + application.setResourceDependencies(List.of(dependency("skills/{current-user}/", false))); + when(context.getProxyApiKeyData()).thenReturn(null); + + assertFalse(fn.apply(request)); + verify(consentService, never()).isAdminConsented(anyString(), any()); + } + + @Test + void apply_failsRequiredDependencyWhenNoKeyToBakeInto() { + // The bug a naive null-guard would reintroduce: an app that promises a required dependency + // must not have that promise silently ignored just because there is nowhere to bake the + // grant — an app that can never receive its required access must fail loudly, not pretend + // to work. Same "never half-works silently" rule the resolver applies everywhere else. + when(context.getDeployment()).thenReturn(application); + application.setResourceDependencies(List.of(dependency("skills/{current-user}/", true))); + when(context.getProxyApiKeyData()).thenReturn(null); + + HttpException error = assertThrows(HttpException.class, () -> fn.apply(request)); + assertEquals(HttpStatus.FORBIDDEN, error.getStatus()); + verify(consentService, never()).isAdminConsented(anyString(), any()); + } + @Test void apply_resolvesOnChainedHop() { // The guard this replaces was not an identity limitation: extractedClaims and originalKey @@ -176,6 +205,26 @@ void apply_bakesGrantForConsentedReachablePlaceholderTarget() { proxyApiKeyData.getPerRequestSharedResources().get(userSkillsFolder().getUrl()).permissions()); } + @Test + void apply_isTypeAgnostic() { + // The unit-level pin for D-25a: the argument is genuinely unread, so the same + // implementation joins a chain of any element type — RequestObject on the conversation + // mint sites, ObjectNode on the MCP proxy. + when(context.getDeployment()).thenReturn(application); + application.setResourceDependencies(List.of(dependency("skills/{current-user}/", false))); + when(consentService.isAdminConsented(eq("app"), any())).thenReturn(true); + when(accessService.lookupOriginatingUserPermissions(any(), eq(context))) + .thenReturn(Map.of(userSkillsFolder(), Set.of(ResourceAccessType.READ, ResourceAccessType.WRITE))); + + ResolveResourceDependenciesFn objectNodeFn = + new ResolveResourceDependenciesFn<>(proxy, context); + + assertFalse(objectNodeFn.apply(com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode())); + + assertEquals(Set.of(ResourceAccessType.READ, ResourceAccessType.WRITE), + proxyApiKeyData.getPerRequestSharedResources().get(userSkillsFolder().getUrl()).permissions()); + } + @Test void apply_combinesGrantsForRecordsWithTheSameTarget() { // Two records targeting the same URL with different rights combine, never overwrite.