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
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
*
* <p>
* {@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.
*
* <p>
* 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.
*
* <p>
* 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".
*
* <p>
* 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.
*
* <p>
* 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");
}
}
Original file line number Diff line number Diff line change
@@ -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.
*
* <p>
* 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.
*
* <p>
* 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 <em>every</em> origin, including the Inspector's, with
* no warning logged. An operator reaching for {@code *} to "allow everything"
* gets the opposite.
*
* <p>
* 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<HttpMethod> TRANSPORT_METHODS = List.of(HttpMethod.GET, HttpMethod.POST,
HttpMethod.DELETE);

@LocalServerPort
private int port;

private HttpResponse<String> preflight(String origin, HttpMethod method, String requestHeaders) throws Exception {
HttpRequest.Builder builder = HttpRequest.newBuilder(URI.create("http://localhost:" + port + MCP_ENDPOINT))
.method(HttpMethod.OPTIONS.name(), HttpRequest.BodyPublishers.noBody())
.header(HttpHeaders.ORIGIN, origin).header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, method.name());
if (requestHeaders != null) {
builder.header(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS, requestHeaders);
}
return HttpClient.newHttpClient().send(builder.build(), HttpResponse.BodyHandlers.ofString());
}

@Test
void inspectorPreflightIsAllowed() throws Exception {
HttpResponse<String> response = preflight(INSPECTOR_ORIGIN, HttpMethod.POST,
HttpHeaders.CONTENT_TYPE + "," + HttpHeaders.AUTHORIZATION);

assertEquals(HttpStatus.OK.value(), response.statusCode(),
"The MCP Inspector cannot connect unless its origin passes preflight. Check that "
+ "mcp.cors.allowed-origins still contains " + INSPECTOR_ORIGIN);
assertEquals(INSPECTOR_ORIGIN,
response.headers().firstValue(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN).orElse(null),
"The specific origin must be echoed back; a wildcard is invalid alongside credentials");
assertEquals(Boolean.TRUE.toString(),
response.headers().firstValue(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS).orElse(null),
"The Inspector sends the bearer token as a credentialed request");
}

@Test
void inspectorTransportMethodsAreAllowed() throws Exception {
String allowed = preflight(INSPECTOR_ORIGIN, HttpMethod.POST, null).headers()
.firstValue(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS).orElse("");

for (HttpMethod method : TRANSPORT_METHODS) {
assertTrue(allowed.contains(method.name()),
() -> "MCP Streamable HTTP needs " + method + "; Allow-Methods was: " + allowed);
}
}

@Test
void unknownOriginIsRejected() throws Exception {
assertEquals(HttpStatus.FORBIDDEN.value(),
preflight("http://not-the-inspector.example", HttpMethod.POST, null).statusCode(),
"Origins outside the allowlist must be refused, otherwise the allowlist is decorative");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* 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.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.util.List;
import org.apache.solr.mcp.server.TestcontainersConfiguration;
import org.apache.solr.mcp.server.collection.CollectionService;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.annotation.Import;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
import org.springframework.security.test.context.support.WithMockUser;
import org.springframework.test.context.ActiveProfiles;
import org.testcontainers.junit.jupiter.Testcontainers;

/**
* Probe: is {@code @PreAuthorize} actually enforced, or merely present?
*
* <p>
* {@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.
*
* <p>
* 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()}.
*
* <p>
* 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.
*
* <p>
* {@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<String> collections = assertDoesNotThrow(() -> collectionService.listCollections(),
"list-collections is gated by @PreAuthorize(\"isAuthenticated()\") and was called with an "
+ "authenticated principal, so method security must permit it. An AccessDeniedException "
+ "here means the gate rejects everyone, not just anonymous callers — the annotation "
+ "would be denying valid callers rather than enforcing a boundary.");

assertNotNull(collections, "an authorized list-collections call must return a result, not null");
}
}