diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 473ff9ff..c3799ce0 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -50,6 +50,7 @@ spring-boot-starter-security = { module = "org.springframework.boot:spring-boot- spring-boot-starter-oauth2-resource-server = { module = "org.springframework.boot:spring-boot-starter-oauth2-resource-server" } spring-boot-docker-compose = { module = "org.springframework.boot:spring-boot-docker-compose" } spring-boot-starter-test = { module = "org.springframework.boot:spring-boot-starter-test" } +spring-security-test = { module = "org.springframework.security:spring-security-test" } spring-boot-testcontainers = { module = "org.springframework.boot:spring-boot-testcontainers" } # Spring AI spring-ai-starter-mcp-server-webmvc = { module = "org.springframework.ai:spring-ai-starter-mcp-server-webmvc" } @@ -102,6 +103,7 @@ spring-boot-dev = [ test = [ "spring-boot-starter-test", + "spring-security-test", "spring-boot-testcontainers", "spring-ai-spring-boot-testcontainers", "testcontainers-junit-jupiter", diff --git a/src/test/java/org/apache/solr/mcp/server/security/HttpSecurityFilterChainTest.java b/src/test/java/org/apache/solr/mcp/server/security/HttpSecurityFilterChainTest.java new file mode 100644 index 00000000..c8067ee1 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/security/HttpSecurityFilterChainTest.java @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.security; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import org.apache.solr.mcp.server.TestcontainersConfiguration; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.context.annotation.Import; +import org.springframework.http.HttpStatus; +import org.springframework.test.context.ActiveProfiles; +import org.testcontainers.junit.jupiter.Testcontainers; + +/** + * Pins the anonymous-access boundary of the {@code http} filter chain. + * + *
+ * {@link HttpSecurityConfiguration} deliberately splits the actuator: probes + * stay open so load balancers and orchestrators can reach them, while every + * other endpoint requires authentication — otherwise an unauthenticated caller + * could read the dependency tree from {@code /actuator/sbom/application} or + * scrape metrics that map the tool surface. + * + *
+ * That decision is a one-line {@code requestMatchers} rule. Widening it to + * {@code permitAll()} would expose all of the above and break no other test, so + * this asserts both halves: health open, everything else closed. + * + *
+ * No issuer is configured here, which is the point — with OAuth2 unwired the + * chain must still deny anonymous access rather than fall open. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@Import(TestcontainersConfiguration.class) +@ActiveProfiles("http") +@Tag("integration") +@Testcontainers(disabledWithoutDocker = true) +class HttpSecurityFilterChainTest { + + /** Open for liveness/readiness probes — the one anonymous actuator path. */ + private static final String HEALTH_PROBE = "/actuator/health"; + + /** Exposes the full dependency tree; must never be anonymous. */ + private static final String SBOM_ENDPOINT = "/actuator/sbom/application"; + + /** Maps the tool surface; must never be anonymous. */ + private static final String METRICS_ENDPOINT = "/actuator/metrics"; + + @LocalServerPort + private int port; + + private int statusOf(String path) throws Exception { + HttpRequest request = HttpRequest.newBuilder().uri(URI.create("http://localhost:" + port + path)).GET().build(); + return HttpClient.newHttpClient().send(request, HttpResponse.BodyHandlers.ofString()).statusCode(); + } + + @Test + void healthProbeIsAnonymouslyReachable() throws Exception { + assertEquals(HttpStatus.OK.value(), statusOf(HEALTH_PROBE), + HEALTH_PROBE + " must stay open for liveness and readiness probes"); + } + + /** + * Denial in this context is exactly {@code 403}, and the assertion pins that + * rather than accepting "401 or 403". + * + *
+ * This class configures no issuer, so {@link HttpSecurityConfiguration} skips + * the OAuth2 wiring and no {@code BearerTokenAuthenticationEntryPoint} is + * installed. Spring Security falls back to {@code Http403ForbiddenEntryPoint}, + * which rejects outright instead of issuing a challenge — measured as + * {@code 403} on every denied path here. + * + *
+ * Accepting either code would weaken the test in a way that matters: a chain + * that silently lost its bearer-token entry point, or gained one it should not + * have, would still pass. Wiring an issuer turns the same request into a + * {@code 401} carrying {@code WWW-Authenticate: Bearer} — a different + * configuration, and one this class deliberately does not exercise. + */ + private void assertDenied(String path, String why) throws Exception { + assertEquals(HttpStatus.FORBIDDEN.value(), statusOf(path), why); + } + + @Test + void sbomEndpointRequiresAuthentication() throws Exception { + assertDenied(SBOM_ENDPOINT, SBOM_ENDPOINT + " exposes the full dependency tree and must not be anonymous"); + } + + @Test + void metricsEndpointRequiresAuthentication() throws Exception { + assertDenied(METRICS_ENDPOINT, METRICS_ENDPOINT + " maps the tool surface and must not be anonymous"); + } +} diff --git a/src/test/java/org/apache/solr/mcp/server/security/McpInspectorCorsTest.java b/src/test/java/org/apache/solr/mcp/server/security/McpInspectorCorsTest.java new file mode 100644 index 00000000..88a2f967 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/security/McpInspectorCorsTest.java @@ -0,0 +1,126 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.security; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.List; +import org.apache.solr.mcp.server.TestcontainersConfiguration; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.context.annotation.Import; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.test.context.ActiveProfiles; +import org.testcontainers.junit.jupiter.Testcontainers; + +/** + * Pins the CORS contract the MCP Inspector depends on. + * + *
+ * The Inspector's UI runs at {@code http://localhost:6274} and is the default + * value of {@code mcp.cors.allowed-origins}. That default is a plain property: + * narrowing it, reordering it, or setting {@code MCP_CORS_ALLOWED_ORIGINS=*} + * silently stops the Inspector connecting, and no other test notices. + * + *
+ * The wildcard case is the trap. {@code setAllowedOrigins} is the strict API, + * so {@code *} combined with {@code allowCredentials(true)} does not open the + * server up — it rejects every origin, including the Inspector's, with + * no warning logged. An operator reaching for {@code *} to "allow everything" + * gets the opposite. + * + *
+ * This replays the exact preflight a browser sends on the Inspector's behalf
+ * and asserts the response permits the request.
+ */
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+@Import(TestcontainersConfiguration.class)
+@ActiveProfiles("http")
+@Tag("integration")
+@Testcontainers(disabledWithoutDocker = true)
+class McpInspectorCorsTest {
+
+ /** The MCP Inspector UI origin, and the shipped default allowlist entry. */
+ private static final String INSPECTOR_ORIGIN = "http://localhost:6274";
+
+ /** Single path the MCP Streamable HTTP transport routes through. */
+ private static final String MCP_ENDPOINT = "/mcp";
+
+ /**
+ * Methods the Streamable HTTP transport needs: POST sends messages, GET opens
+ * the stream, DELETE ends the session. Dropping any one breaks a different part
+ * of the transport.
+ */
+ private static final List
+ * {@code McpToolRegistrationTest#everyMcpEndpointIsPreAuthorized} asserts the
+ * annotation is declared on every MCP entry point. That is a static check — it
+ * cannot tell whether {@link MethodSecurityConfiguration} is wired such that
+ * the annotation has any runtime effect. If the profile gate or the
+ * {@code http.security.enabled} property condition stopped matching, every
+ * annotation would silently become a no-op and the static test would still
+ * pass.
+ *
+ *
+ * This test runs in the {@code http} profile with security left at its default
+ * (enabled) and invokes a secured method through the Spring proxy with an empty
+ * SecurityContext. Enforcement means an {@link AccessDeniedException}.
+ */
+@SpringBootTest
+@Import(TestcontainersConfiguration.class)
+@ActiveProfiles("http")
+@Tag("integration")
+@Testcontainers(disabledWithoutDocker = true)
+class MethodSecurityEnforcementTest {
+
+ @Autowired
+ private CollectionService collectionService;
+
+ /**
+ * With an entirely empty SecurityContext, Spring Security raises
+ * {@link AuthenticationCredentialsNotFoundException} (an
+ * {@code AuthenticationException}) rather than {@code AccessDeniedException} —
+ * the latter is for an authenticated principal lacking authority. Asserting the
+ * broad {@code SecurityException}-free supertype would pass for the wrong
+ * reason, so this pins the specific type.
+ */
+ @Test
+ void unauthenticatedCallToSecuredToolIsRejected() {
+ assertThrows(AuthenticationCredentialsNotFoundException.class, () -> collectionService.listCollections(),
+ "list-collections carries @PreAuthorize(\"isAuthenticated()\") and was called with no "
+ + "authentication, so method security must reject it. Succeeding means the annotation "
+ + "is decorative: @EnableMethodSecurity is not in effect for this context.");
+ }
+
+ /**
+ * The necessary counterpart to
+ * {@link #unauthenticatedCallToSecuredToolIsRejected()}.
+ *
+ *
+ * A rejection test on its own cannot distinguish "correctly denies anonymous
+ * callers" from "denies every caller". Both produce the same green result, so a
+ * gate that were wedged permanently shut would look identical to a working one.
+ * That gap is not hypothetical: a secured tool call was for a time believed
+ * broken — reported as denying even valid tokens — and no test existed that
+ * could have contradicted it. The claim turned out to be false, but only a live
+ * server proved so.
+ *
+ *
+ * {@code @WithMockUser} installs an authenticated principal before the method
+ * runs and clears it afterwards, so the {@code ThreadLocal} context cannot leak
+ * into {@link #unauthenticatedCallToSecuredToolIsRejected()} and make that test
+ * order-dependent.
+ */
+ @Test
+ @WithMockUser
+ void authenticatedCallToSecuredToolSucceeds() {
+ List