From fa5a025c935d60e8172c281df31aa75d5052c73d Mon Sep 17 00:00:00 2001 From: Zakaria El Orche Date: Wed, 9 Sep 2026 10:22:40 +0000 Subject: [PATCH] test(ext-mcp): migrate McpAuthPolicyTest to flash-testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First migration, chosen because it is the case that drove the harness design: a Flash app plus a FakeOidcProvider, with tokens audience-bound to the app's own port, so the port has to be readable after boot. Drops the racy free-port dance, the hand-rolled HttpClient plumbing and the per-test teardown; failures now report the response body. 120 to 106 code lines, and what remains is tool-policy assertions rather than fixture code. The two boot-rejection tests keep building their app directly — a harness whose job is to boot an app is the wrong tool for asserting that booting fails — but port(0) removes freePort() from those too. Co-Authored-By: Claude Opus 5 --- flash-extensions/flash-ext-mcp/pom.xml | 5 + .../flash/ext/mcp/McpAuthPolicyTest.java | 199 +++++++++--------- 2 files changed, 100 insertions(+), 104 deletions(-) diff --git a/flash-extensions/flash-ext-mcp/pom.xml b/flash-extensions/flash-ext-mcp/pom.xml index e5effe7..db7f5ac 100644 --- a/flash-extensions/flash-ext-mcp/pom.xml +++ b/flash-extensions/flash-ext-mcp/pom.xml @@ -38,6 +38,11 @@ org.junit.jupiter junit-jupiter + + dev.relism + flash-testing + test + diff --git a/flash-extensions/flash-ext-mcp/src/test/java/dev/relism/flash/ext/mcp/McpAuthPolicyTest.java b/flash-extensions/flash-ext-mcp/src/test/java/dev/relism/flash/ext/mcp/McpAuthPolicyTest.java index fb31b9c..af42bdb 100644 --- a/flash-extensions/flash-ext-mcp/src/test/java/dev/relism/flash/ext/mcp/McpAuthPolicyTest.java +++ b/flash-extensions/flash-ext-mcp/src/test/java/dev/relism/flash/ext/mcp/McpAuthPolicyTest.java @@ -3,16 +3,14 @@ package dev.relism.flash.ext.mcp; import dev.relism.flash.ext.oidc.OidcConfig; import dev.relism.flash.ext.oidc.OidcExtension; import dev.relism.flash.extension.FlashApp; +import dev.relism.flash.extension.FlashConfiguration; +import dev.relism.flash.testing.FlashResponse; +import dev.relism.flash.testing.FlashTest; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; -import java.net.ServerSocket; -import java.net.URI; -import java.net.http.HttpClient; -import java.net.http.HttpRequest; -import java.net.http.HttpResponse; - -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -26,125 +24,118 @@ class McpAuthPolicyTest { private static final String SECURED_TOOLS = "dev.relism.flash.ext.mcp.authfixtures.secured"; private static final String AUTHENTICATED_ONLY_TOOLS = "dev.relism.flash.ext.mcp.authfixtures.authenticatedonly"; - private FlashApp app; - private FakeOidcProvider provider; + private static final FakeOidcProvider provider = newProvider(); - @AfterEach - void tearDown() { - if (app != null) app.stop(); - if (provider != null) provider.close(); - } - - @Test - void rolesAllowed_deniesWithoutRole_allowsWithRole() throws Exception { - int port = bootSecuredApp(SECURED_TOOLS); - String resourceId = "http://127.0.0.1:" + port + "/mcp"; - - String noRole = provider.signToken("user-1", resourceId, null); - HttpResponse denied = callTool(port, "admin_only", noRole); - assertEquals(200, denied.statusCode()); - assertTrue(denied.body().contains("\"isError\":true"), denied.body()); - assertTrue(denied.body().contains("missing required role"), denied.body()); - - String withRole = provider.signToken("user-1", resourceId, null, "admin"); - HttpResponse allowed = callTool(port, "admin_only", withRole); - assertEquals(200, allowed.statusCode()); - assertTrue(allowed.body().contains("\"isError\":false"), allowed.body()); - assertTrue(allowed.body().contains("ok"), allowed.body()); - } - - @Test - void scopesAllowed_deniesWithoutScope_allowsWithScope() throws Exception { - int port = bootSecuredApp(SECURED_TOOLS); - String resourceId = "http://127.0.0.1:" + port + "/mcp"; - - String noScope = provider.signToken("user-1", resourceId, "read"); - HttpResponse denied = callTool(port, "write_only", noScope); - assertEquals(200, denied.statusCode()); - assertTrue(denied.body().contains("\"isError\":true"), denied.body()); - assertTrue(denied.body().contains("missing required scope"), denied.body()); - - String withScope = provider.signToken("user-1", resourceId, "read write"); - HttpResponse allowed = callTool(port, "write_only", withScope); - assertEquals(200, allowed.statusCode()); - assertTrue(allowed.body().contains("\"isError\":false"), allowed.body()); - assertTrue(allowed.body().contains("written"), allowed.body()); - } - - @Test - void unannotatedTool_unaffectedByOtherToolsPolicies() throws Exception { - int port = bootSecuredApp(SECURED_TOOLS); - String resourceId = "http://127.0.0.1:" + port + "/mcp"; - - String plain = provider.signToken("user-1", resourceId, null); - HttpResponse resp = callTool(port, "open", plain); - assertEquals(200, resp.statusCode()); - assertTrue(resp.body().contains("\"isError\":false"), resp.body()); - assertTrue(resp.body().contains("open"), resp.body()); - } - - @Test - void toolAnnotated_butSecurityNone_failsAtBoot() throws Exception { - provider = new FakeOidcProvider(); - int port = freePort(); - app = FlashApp.create(port); + @RegisterExtension + static FlashTest secured = FlashTest.of(app -> { app.install(new OidcExtension(OidcConfig.builder( provider.issuer(), "mcp-client", "secret", "/auth/callback").build())); app.install(new McpExtension(McpConfig.builder("secure-server") .toolsPackage(SECURED_TOOLS) - .security(McpSecurity.NONE) + .security(McpSecurity.REQUIRED) .build())); + }); - IllegalStateException e = assertThrows(IllegalStateException.class, () -> app.start()); - assertTrue(e.getMessage().contains("no active OAuth2 protection"), e.getMessage()); + /** Tokens are audience-bound to this server, so the port has to be read back after boot. */ + private static String resourceId() { + return "http://127.0.0.1:" + secured.port() + "/mcp"; + } + + @AfterAll + static void closeProvider() { + provider.close(); + } + + // ── Tool policy ────────────────────────────────────────────────────────── + + @Test + void rolesAllowed_deniesWithoutRole_allowsWithRole() throws Exception { + callTool("admin_only", provider.signToken("user-1", resourceId(), null)) + .expectStatus(200) + .expectBodyContains("\"isError\":true") + .expectBodyContains("missing required role"); + + callTool("admin_only", provider.signToken("user-1", resourceId(), null, "admin")) + .expectStatus(200) + .expectBodyContains("\"isError\":false") + .expectBodyContains("ok"); } @Test - void bareAuthenticated_hasNoEffect_failsAtBoot() throws Exception { - provider = new FakeOidcProvider(); - int port = freePort(); - app = FlashApp.create(port); - app.install(new OidcExtension(OidcConfig.builder( - provider.issuer(), "mcp-client", "secret", "/auth/callback").build())); - app.install(new McpExtension(McpConfig.builder("secure-server") - .toolsPackage(AUTHENTICATED_ONLY_TOOLS) - .security(McpSecurity.REQUIRED) - .build())); + void scopesAllowed_deniesWithoutScope_allowsWithScope() throws Exception { + callTool("write_only", provider.signToken("user-1", resourceId(), "read")) + .expectStatus(200) + .expectBodyContains("\"isError\":true") + .expectBodyContains("missing required scope"); - IllegalStateException e = assertThrows(IllegalStateException.class, () -> app.start()); - assertTrue(e.getMessage().contains("no effect"), e.getMessage()); + callTool("write_only", provider.signToken("user-1", resourceId(), "read write")) + .expectStatus(200) + .expectBodyContains("\"isError\":false") + .expectBodyContains("written"); } - // ── Helpers ────────────────────────────────────────────────────────────── + @Test + void unannotatedTool_unaffectedByOtherToolsPolicies() throws Exception { + callTool("open", provider.signToken("user-1", resourceId(), null)) + .expectStatus(200) + .expectBodyContains("\"isError\":false") + .expectBodyContains("open"); + } - private int bootSecuredApp(String toolsPackage) throws Exception { - provider = new FakeOidcProvider(); - int port = freePort(); + private static FlashResponse callTool(String toolName, String token) { + return secured.request() + .header("Accept", "application/json") + .header("Authorization", "Bearer " + token) + .json("{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/call\",\"params\":{\"name\":\"" + + toolName + "\"}}") + .post("/mcp"); + } - app = FlashApp.create(port); + // ── Boot-time rejection ────────────────────────────────────────────────── + // These assert that start() throws, so they build the app directly rather than through + // FlashTest — a harness whose job is to boot an app is the wrong tool for asserting that + // booting fails. Port 0 still removes the old free-port dance. + + private FlashApp bootFailure; + + @AfterEach + void releaseBootFailureListener() { + if (bootFailure != null) bootFailure.stop().join(); + } + + @Test + void toolAnnotated_butSecurityNone_failsAtBoot() { + bootFailure = mcpApp(SECURED_TOOLS, McpSecurity.NONE); + + IllegalStateException error = assertThrows(IllegalStateException.class, bootFailure::start); + assertTrue(error.getMessage().contains("no active OAuth2 protection"), error.getMessage()); + } + + @Test + void bareAuthenticated_hasNoEffect_failsAtBoot() { + bootFailure = mcpApp(AUTHENTICATED_ONLY_TOOLS, McpSecurity.REQUIRED); + + IllegalStateException error = assertThrows(IllegalStateException.class, bootFailure::start); + assertTrue(error.getMessage().contains("no effect"), error.getMessage()); + } + + private static FlashApp mcpApp(String toolsPackage, McpSecurity security) { + FlashApp app = FlashApp.create(FlashConfiguration.builder() + .port(0).host("127.0.0.1").shutdownDrainTimeoutMs(250).build()); app.install(new OidcExtension(OidcConfig.builder( provider.issuer(), "mcp-client", "secret", "/auth/callback").build())); app.install(new McpExtension(McpConfig.builder("secure-server") .toolsPackage(toolsPackage) - .security(McpSecurity.REQUIRED) + .security(security) .build())); - app.start(); - return port; + return app; } - private static HttpResponse callTool(int port, String toolName, String token) throws Exception { - String body = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/call\",\"params\":{\"name\":\"" + toolName + "\"}}"; - HttpRequest.Builder req = HttpRequest.newBuilder(URI.create("http://127.0.0.1:" + port + "/mcp")) - .header("Content-Type", "application/json") - .header("Accept", "application/json") - .header("Authorization", "Bearer " + token) - .POST(HttpRequest.BodyPublishers.ofString(body)); - return HttpClient.newHttpClient().send(req.build(), HttpResponse.BodyHandlers.ofString()); - } - - private static int freePort() throws Exception { - try (ServerSocket s = new ServerSocket(0)) { - return s.getLocalPort(); + private static FakeOidcProvider newProvider() { + try { + return new FakeOidcProvider(); + } catch (Exception failure) { + throw new IllegalStateException("Could not start the fake OIDC provider", failure); } } }