mirror of
https://github.com/arsvendg/Stirling-PDF.git
synced 2026-07-14 10:34:06 +02:00
MCP token rejection reason and stop logging the raw tokens (#6700)
- Surface the real reason an MCP token is rejected: the 401's WWW-Authenticate header now includes error_description (audience/issuer/expiry), and a present-but-rejected token logs the concrete OAuth2 reason. Tokenless 401s (the normal discovery handshake) stay at debug. - Add McpConfigValidator that sanity-checks MCP config at startup and logs actionable warnings (missing issuer-uri/resource-id, unrecognized auth mode, sub + require-existing-account, open access, scopes, allow/block overlap) so misconfig shows up in the logs before a client ever connects. - Align the audience-rejection message to mention both resource-id and accepted-audiences. - Harden audit writes: hash JWT-shaped or over-long principals (token:<sha256-prefix>) so the insert fits the column and never stores a raw bearer token, and stop logging the raw principal on persist failure. --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details.
This commit is contained in:
+35
-5
@@ -1,6 +1,10 @@
|
|||||||
package stirling.software.proprietary.config;
|
package stirling.software.proprietary.config;
|
||||||
|
|
||||||
|
import java.nio.charset.StandardCharsets;
|
||||||
|
import java.security.MessageDigest;
|
||||||
|
import java.security.NoSuchAlgorithmException;
|
||||||
import java.time.Instant;
|
import java.time.Instant;
|
||||||
|
import java.util.HexFormat;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
|
||||||
@@ -61,17 +65,43 @@ public class CustomAuditEventRepository implements AuditEventRepository {
|
|||||||
|
|
||||||
PersistentAuditEvent ent =
|
PersistentAuditEvent ent =
|
||||||
PersistentAuditEvent.builder()
|
PersistentAuditEvent.builder()
|
||||||
.principal(ev.getPrincipal())
|
.principal(safePrincipal(ev.getPrincipal()))
|
||||||
.type(ev.getType())
|
.type(ev.getType())
|
||||||
.data(auditEventData)
|
.data(auditEventData)
|
||||||
.timestamp(ev.getTimestamp())
|
.timestamp(ev.getTimestamp())
|
||||||
.build();
|
.build();
|
||||||
repo.save(ent);
|
repo.save(ent);
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
log.error(
|
log.error("Failed to persist audit event (fail-open); type={}", ev.getType(), e);
|
||||||
"Failed to persist audit event (fail-open); principal={}",
|
}
|
||||||
ev.getPrincipal(),
|
}
|
||||||
e);
|
|
||||||
|
/** Width of the {@code principal} column; longer values are hashed so the insert can't fail. */
|
||||||
|
private static final int PRINCIPAL_MAX_LENGTH = 255;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Hash JWT-shaped or over-long principals so the insert fits the column and stores no secret.
|
||||||
|
*/
|
||||||
|
static String safePrincipal(String principal) {
|
||||||
|
if (principal == null || principal.isBlank()) {
|
||||||
|
return "anonymous";
|
||||||
|
}
|
||||||
|
// Hash JWTs ("eyJ...") and any over-long value rather than store verbatim.
|
||||||
|
if (principal.startsWith("eyJ") || principal.length() > PRINCIPAL_MAX_LENGTH) {
|
||||||
|
return "token:" + sha256Prefix(principal);
|
||||||
|
}
|
||||||
|
return principal;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** First 8 bytes of SHA-256 as hex: stable, one-way, collision-safe enough. */
|
||||||
|
private static String sha256Prefix(String value) {
|
||||||
|
try {
|
||||||
|
byte[] digest =
|
||||||
|
MessageDigest.getInstance("SHA-256")
|
||||||
|
.digest(value.getBytes(StandardCharsets.UTF_8));
|
||||||
|
return HexFormat.of().formatHex(digest, 0, 8);
|
||||||
|
} catch (NoSuchAlgorithmException e) {
|
||||||
|
return "unhashable";
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-2
@@ -43,8 +43,8 @@ public class McpAudienceValidator implements OAuth2TokenValidator<Jwt> {
|
|||||||
return OAuth2TokenValidatorResult.failure(
|
return OAuth2TokenValidatorResult.failure(
|
||||||
new OAuth2Error(
|
new OAuth2Error(
|
||||||
"invalid_token",
|
"invalid_token",
|
||||||
"MCP server has no resource id configured; rejecting all tokens"
|
"MCP audience binding is not configured; rejecting all tokens until"
|
||||||
+ " until mcp.auth.resource-id is set.",
|
+ " mcp.auth.resource-id or mcp.auth.accepted-audiences is set.",
|
||||||
null));
|
null));
|
||||||
}
|
}
|
||||||
List<String> aud = token.getAudience();
|
List<String> aud = token.getAudience();
|
||||||
|
|||||||
+43
-5
@@ -4,15 +4,21 @@ import java.io.IOException;
|
|||||||
|
|
||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.security.core.AuthenticationException;
|
import org.springframework.security.core.AuthenticationException;
|
||||||
|
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
|
||||||
|
import org.springframework.security.oauth2.core.OAuth2Error;
|
||||||
import org.springframework.security.web.AuthenticationEntryPoint;
|
import org.springframework.security.web.AuthenticationEntryPoint;
|
||||||
|
|
||||||
import jakarta.servlet.http.HttpServletRequest;
|
import jakarta.servlet.http.HttpServletRequest;
|
||||||
import jakarta.servlet.http.HttpServletResponse;
|
import jakarta.servlet.http.HttpServletResponse;
|
||||||
|
|
||||||
|
import lombok.extern.slf4j.Slf4j;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Emits 401 + {@code WWW-Authenticate: Bearer resource_metadata="..."} (RFC 9728), preferring
|
* Emits 401 + {@code WWW-Authenticate: Bearer resource_metadata="..."} (RFC 9728) from
|
||||||
* X-Forwarded-* headers to build the public-facing metadata URL.
|
* X-Forwarded-* headers. A rejected token also logs the OAuth2 reason and echoes it as {@code
|
||||||
|
* error_description}.
|
||||||
*/
|
*/
|
||||||
|
@Slf4j
|
||||||
public class McpAuthenticationEntryPoint implements AuthenticationEntryPoint {
|
public class McpAuthenticationEntryPoint implements AuthenticationEntryPoint {
|
||||||
|
|
||||||
private final String metadataPath;
|
private final String metadataPath;
|
||||||
@@ -28,15 +34,47 @@ public class McpAuthenticationEntryPoint implements AuthenticationEntryPoint {
|
|||||||
HttpServletResponse response,
|
HttpServletResponse response,
|
||||||
AuthenticationException authException)
|
AuthenticationException authException)
|
||||||
throws IOException {
|
throws IOException {
|
||||||
|
// Tokenless 401 is the normal discovery handshake; only a rejected token is a real failure.
|
||||||
|
boolean tokenPresented = request.getHeader("Authorization") != null;
|
||||||
|
String reason = rejectionReason(authException);
|
||||||
|
if (tokenPresented) {
|
||||||
|
log.warn("MCP rejected bearer token: {}", reason != null ? reason : "invalid_token");
|
||||||
|
} else {
|
||||||
|
log.debug("MCP 401: no bearer token; returning protected-resource metadata pointer");
|
||||||
|
}
|
||||||
|
|
||||||
String scheme = firstForwarded(request, "X-Forwarded-Proto", request.getScheme());
|
String scheme = firstForwarded(request, "X-Forwarded-Proto", request.getScheme());
|
||||||
String authority = forwardedHost(request, scheme);
|
String authority = forwardedHost(request, scheme);
|
||||||
String metadataUrl = scheme + "://" + authority + metadataPath;
|
String metadataUrl = scheme + "://" + authority + metadataPath;
|
||||||
response.setHeader(
|
|
||||||
"WWW-Authenticate",
|
StringBuilder header = new StringBuilder("Bearer error=\"invalid_token\"");
|
||||||
"Bearer error=\"invalid_token\", resource_metadata=\"" + metadataUrl + "\"");
|
if (tokenPresented && reason != null) {
|
||||||
|
header.append(", error_description=\"").append(reason).append('"');
|
||||||
|
}
|
||||||
|
header.append(", resource_metadata=\"").append(metadataUrl).append('"');
|
||||||
|
response.setHeader("WWW-Authenticate", header.toString());
|
||||||
response.sendError(HttpStatus.UNAUTHORIZED.value(), "Unauthorized");
|
response.sendError(HttpStatus.UNAUTHORIZED.value(), "Unauthorized");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* OAuth2 error as {@code "code - description"}, sanitized for a header/log line; null if none.
|
||||||
|
*/
|
||||||
|
private static String rejectionReason(AuthenticationException ex) {
|
||||||
|
if (!(ex instanceof OAuth2AuthenticationException oae)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
OAuth2Error error = oae.getError();
|
||||||
|
if (error == null) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
String description = error.getDescription();
|
||||||
|
String combined =
|
||||||
|
(description == null || description.isBlank())
|
||||||
|
? error.getErrorCode()
|
||||||
|
: error.getErrorCode() + " - " + description;
|
||||||
|
return combined == null ? null : combined.replaceAll("[\\r\\n\"]", " ").trim();
|
||||||
|
}
|
||||||
|
|
||||||
/** host[:port] from forwarded headers when present, else the servlet host/port. */
|
/** host[:port] from forwarded headers when present, else the servlet host/port. */
|
||||||
private static String forwardedHost(HttpServletRequest request, String scheme) {
|
private static String forwardedHost(HttpServletRequest request, String scheme) {
|
||||||
String host = firstForwarded(request, "X-Forwarded-Host", null);
|
String host = firstForwarded(request, "X-Forwarded-Host", null);
|
||||||
|
|||||||
+195
@@ -0,0 +1,195 @@
|
|||||||
|
package stirling.software.proprietary.mcp.security;
|
||||||
|
|
||||||
|
import java.util.ArrayList;
|
||||||
|
import java.util.List;
|
||||||
|
import java.util.Locale;
|
||||||
|
|
||||||
|
import stirling.software.common.model.ApplicationProperties;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Startup sanity-checks for MCP config; {@link McpSecurityConfig} logs the findings at boot so a
|
||||||
|
* misconfigured /mcp endpoint shows up in the logs instead of as a later rejected-token 401.
|
||||||
|
*/
|
||||||
|
public final class McpConfigValidator {
|
||||||
|
|
||||||
|
public enum Severity {
|
||||||
|
WARN,
|
||||||
|
INFO
|
||||||
|
}
|
||||||
|
|
||||||
|
public record Finding(Severity severity, String message) {}
|
||||||
|
|
||||||
|
private McpConfigValidator() {}
|
||||||
|
|
||||||
|
/** Inspect the resolved MCP config and return ordered findings (most actionable first). */
|
||||||
|
public static List<Finding> validate(ApplicationProperties.Mcp mcp) {
|
||||||
|
List<Finding> findings = new ArrayList<>();
|
||||||
|
ApplicationProperties.Mcp.Auth auth = mcp.getAuth();
|
||||||
|
|
||||||
|
if ("apikey".equalsIgnoreCase(auth.getMode())) {
|
||||||
|
findings.add(
|
||||||
|
info(
|
||||||
|
"auth mode = apikey - clients send a Stirling API key via X-API-KEY (or"
|
||||||
|
+ " Authorization: Bearer <key>); no external IdP needed. The key"
|
||||||
|
+ " must belong to a provisioned, enabled account (Account -> API"
|
||||||
|
+ " Keys)."));
|
||||||
|
return findings;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Anything that isn't exactly "apikey" runs the OAuth chain (mirrors isApiKeyMode()).
|
||||||
|
String mode = auth.getMode();
|
||||||
|
if (mode != null && !mode.isBlank() && !"oauth".equalsIgnoreCase(mode.trim())) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"mcp.auth.mode='"
|
||||||
|
+ mode
|
||||||
|
+ "' is not recognized (expected 'oauth' or 'apikey'); it falls"
|
||||||
|
+ " back to the OAuth chain, which rejects every token unless"
|
||||||
|
+ " issuer-uri and resource-id are set. A near-miss like"
|
||||||
|
+ " 'api-key' is NOT treated as API-key mode."));
|
||||||
|
}
|
||||||
|
findings.add(info("auth mode = oauth - running as an OAuth2 resource server for /mcp."));
|
||||||
|
|
||||||
|
if (isBlank(auth.getIssuerUri())) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"mcp.auth.issuer-uri is not set: the JWT decoder fails closed and rejects"
|
||||||
|
+ " every token. Set it to your IdP issuer that publishes"
|
||||||
|
+ " /.well-known/openid-configuration (e.g."
|
||||||
|
+ " https://login.microsoftonline.com/<tenant>/v2.0)."));
|
||||||
|
} else if (!looksLikeUrl(auth.getIssuerUri())) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"mcp.auth.issuer-uri='"
|
||||||
|
+ auth.getIssuerUri()
|
||||||
|
+ "' does not look like an http(s) URL."));
|
||||||
|
}
|
||||||
|
|
||||||
|
boolean hasResourceId = !isBlank(auth.getResourceId());
|
||||||
|
boolean hasAcceptedAudiences =
|
||||||
|
auth.getAcceptedAudiences().stream().anyMatch(a -> !isBlank(a));
|
||||||
|
|
||||||
|
if (!hasResourceId && !hasAcceptedAudiences) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"neither mcp.auth.resource-id nor mcp.auth.accepted-audiences is set: the"
|
||||||
|
+ " audience validator fails closed and rejects every token (RFC"
|
||||||
|
+ " 8707). Set resource-id to this server's public /mcp URL, or"
|
||||||
|
+ " accepted-audiences to the audience your IdP actually mints."));
|
||||||
|
} else {
|
||||||
|
if (hasResourceId && !looksLikeUrl(auth.getResourceId())) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"mcp.auth.resource-id='"
|
||||||
|
+ auth.getResourceId()
|
||||||
|
+ "' is not an http(s) URL: the token aud must match it"
|
||||||
|
+ " exactly (scheme, host and port included)."));
|
||||||
|
} else if (hasResourceId && !auth.getResourceId().endsWith("/mcp")) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"mcp.auth.resource-id='"
|
||||||
|
+ auth.getResourceId()
|
||||||
|
+ "' does not end in /mcp: it must match the public URL"
|
||||||
|
+ " clients call and the audience your IdP puts in the"
|
||||||
|
+ " token."));
|
||||||
|
}
|
||||||
|
if (hasAcceptedAudiences) {
|
||||||
|
findings.add(
|
||||||
|
info(
|
||||||
|
"mcp.auth.accepted-audiences="
|
||||||
|
+ auth.getAcceptedAudiences()
|
||||||
|
+ " - tokens whose aud matches any of these are accepted, the"
|
||||||
|
+ " escape hatch for IdPs that can't mint a resource-specific"
|
||||||
|
+ " audience (e.g. an Entra ID app id, or Supabase's"
|
||||||
|
+ " aud=authenticated)."));
|
||||||
|
} else {
|
||||||
|
findings.add(
|
||||||
|
info(
|
||||||
|
"audience binding is strict (token aud must equal"
|
||||||
|
+ " mcp.auth.resource-id). If your IdP can't mint that - e.g."
|
||||||
|
+ " Entra ID issues aud=<client-id> - set"
|
||||||
|
+ " mcp.auth.accepted-audiences to the audience it actually"
|
||||||
|
+ " emits."));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (isBlank(auth.getJwksUri())) {
|
||||||
|
findings.add(
|
||||||
|
info(
|
||||||
|
"mcp.auth.jwks-uri not set - signing keys are auto-discovered from the"
|
||||||
|
+ " issuer's OpenID configuration."));
|
||||||
|
}
|
||||||
|
|
||||||
|
if ("sub".equalsIgnoreCase(auth.getUsernameClaim()) && auth.isRequireExistingAccount()) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"mcp.auth.username-claim='sub' with require-existing-account=true: many"
|
||||||
|
+ " IdPs (e.g. Entra ID, Google) set 'sub' to an opaque id that won't"
|
||||||
|
+ " match a Stirling username. Set mcp.auth.username-claim to 'email'"
|
||||||
|
+ " or 'preferred_username', or provision accounts keyed by sub."));
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!auth.isRequireExistingAccount()) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"mcp.auth.require-existing-account=false: any token your IdP signs can"
|
||||||
|
+ " invoke MCP tools even if its subject has no Stirling account. Set"
|
||||||
|
+ " it true unless you intend open access for every IdP-valid"
|
||||||
|
+ " token."));
|
||||||
|
}
|
||||||
|
|
||||||
|
if (mcp.isScopesEnabled()) {
|
||||||
|
findings.add(
|
||||||
|
info(
|
||||||
|
"mcp.scopes-enabled=true - the IdP must mint 'mcp.tools.read' and"
|
||||||
|
+ " 'mcp.tools.write' scopes or clients are rejected; set"
|
||||||
|
+ " mcp.scopes-enabled=false if it can only issue coarse tokens."));
|
||||||
|
}
|
||||||
|
|
||||||
|
List<String> allowed = mcp.getAllowedOperations();
|
||||||
|
List<String> blocked = mcp.getBlockedOperations();
|
||||||
|
if (allowed != null && !allowed.isEmpty()) {
|
||||||
|
findings.add(
|
||||||
|
info(
|
||||||
|
"mcp.allowed-operations is a strict allow-list of "
|
||||||
|
+ allowed.size()
|
||||||
|
+ " operation(s); every other tool is hidden, so a wrong or"
|
||||||
|
+ " typo'd id silently exposes nothing."));
|
||||||
|
List<String> shadowed =
|
||||||
|
blocked == null
|
||||||
|
? List.of()
|
||||||
|
: allowed.stream().filter(blocked::contains).toList();
|
||||||
|
if (!shadowed.isEmpty()) {
|
||||||
|
findings.add(
|
||||||
|
warn(
|
||||||
|
"mcp operation(s) "
|
||||||
|
+ shadowed
|
||||||
|
+ " are in both allowed-operations and blocked-operations;"
|
||||||
|
+ " blocked wins, so they are hidden."));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (findings.stream().noneMatch(f -> f.severity() == Severity.WARN)) {
|
||||||
|
findings.add(info("OAuth settings look complete."));
|
||||||
|
}
|
||||||
|
|
||||||
|
return findings;
|
||||||
|
}
|
||||||
|
|
||||||
|
private static boolean isBlank(String value) {
|
||||||
|
return value == null || value.isBlank();
|
||||||
|
}
|
||||||
|
|
||||||
|
private static boolean looksLikeUrl(String value) {
|
||||||
|
String lower = value.toLowerCase(Locale.ROOT);
|
||||||
|
return lower.startsWith("http://") || lower.startsWith("https://");
|
||||||
|
}
|
||||||
|
|
||||||
|
private static Finding warn(String message) {
|
||||||
|
return new Finding(Severity.WARN, message);
|
||||||
|
}
|
||||||
|
|
||||||
|
private static Finding info(String message) {
|
||||||
|
return new Finding(Severity.INFO, message);
|
||||||
|
}
|
||||||
|
}
|
||||||
+8
-18
@@ -77,24 +77,14 @@ public class McpSecurityConfig {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@PostConstruct
|
@PostConstruct
|
||||||
void warnIfMisconfigured() {
|
void validateConfigOnStartup() {
|
||||||
ApplicationProperties.Mcp mcp = applicationProperties.getMcp();
|
log.info("MCP server enabled - validating configuration:");
|
||||||
if (isApiKeyMode()) {
|
for (McpConfigValidator.Finding finding :
|
||||||
log.info(
|
McpConfigValidator.validate(applicationProperties.getMcp())) {
|
||||||
"MCP auth mode = apikey: clients authenticate with a Stirling per-user API key"
|
if (finding.severity() == McpConfigValidator.Severity.WARN) {
|
||||||
+ " (X-API-KEY header). No OAuth issuer required.");
|
log.warn("MCP config: {}", finding.message());
|
||||||
} else {
|
} else {
|
||||||
if (mcp.getAuth().getIssuerUri().isBlank()) {
|
log.info("MCP config: {}", finding.message());
|
||||||
log.warn(
|
|
||||||
"MCP enabled but mcp.auth.issuer-uri is blank - JWT decoder will reject"
|
|
||||||
+ " every token (fail-closed). Set mcp.auth.issuer-uri and"
|
|
||||||
+ " mcp.auth.resource-id before exposing /mcp to clients.");
|
|
||||||
}
|
|
||||||
if (mcp.getAuth().getResourceId().isBlank()) {
|
|
||||||
log.warn(
|
|
||||||
"MCP enabled but mcp.auth.resource-id is blank - audience validator will"
|
|
||||||
+ " reject every token. Set this to the public URL of the MCP"
|
|
||||||
+ " endpoint (RFC 8707).");
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+52
@@ -0,0 +1,52 @@
|
|||||||
|
package stirling.software.proprietary.config;
|
||||||
|
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertNotEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
|
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
|
||||||
|
class CustomAuditEventRepositoryTest {
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shortPrincipalPassesThroughUnchanged() {
|
||||||
|
assertEquals(
|
||||||
|
"[email protected]", CustomAuditEventRepository.safePrincipal("[email protected]"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void blankOrNullPrincipalBecomesAnonymous() {
|
||||||
|
assertEquals("anonymous", CustomAuditEventRepository.safePrincipal(null));
|
||||||
|
assertEquals("anonymous", CustomAuditEventRepository.safePrincipal(" "));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void tokenShapedPrincipalIsHashedNotStoredVerbatim() {
|
||||||
|
String jwt = "eyJhbGciOiJSUzI1NiJ9." + "x".repeat(1400);
|
||||||
|
|
||||||
|
String safe = CustomAuditEventRepository.safePrincipal(jwt);
|
||||||
|
|
||||||
|
assertNotEquals(jwt, safe);
|
||||||
|
assertFalse(safe.contains(jwt), "raw token must not be stored");
|
||||||
|
assertTrue(safe.startsWith("token:"));
|
||||||
|
assertTrue(safe.length() <= 255, "must fit the principal column");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void distinctTokensStayDistinguishable() {
|
||||||
|
String a = CustomAuditEventRepository.safePrincipal("eyJ" + "a".repeat(400));
|
||||||
|
String b = CustomAuditEventRepository.safePrincipal("eyJ" + "b".repeat(400));
|
||||||
|
|
||||||
|
assertNotEquals(a, b, "different tokens must map to different audit principals");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void sameTokenHashesStably() {
|
||||||
|
String token = "eyJ" + "c".repeat(400);
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
CustomAuditEventRepository.safePrincipal(token),
|
||||||
|
CustomAuditEventRepository.safePrincipal(token));
|
||||||
|
}
|
||||||
|
}
|
||||||
+28
@@ -11,6 +11,8 @@ import static org.mockito.Mockito.when;
|
|||||||
|
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.mockito.ArgumentCaptor;
|
import org.mockito.ArgumentCaptor;
|
||||||
|
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
|
||||||
|
import org.springframework.security.oauth2.core.OAuth2Error;
|
||||||
|
|
||||||
import jakarta.servlet.http.HttpServletRequest;
|
import jakarta.servlet.http.HttpServletRequest;
|
||||||
import jakarta.servlet.http.HttpServletResponse;
|
import jakarta.servlet.http.HttpServletResponse;
|
||||||
@@ -58,4 +60,30 @@ class McpAuthenticationEntryPointTest {
|
|||||||
verify(resp).setHeader(eq("WWW-Authenticate"), header.capture());
|
verify(resp).setHeader(eq("WWW-Authenticate"), header.capture());
|
||||||
assertTrue(header.getValue().contains("http://localhost:8080" + META), header.getValue());
|
assertTrue(header.getValue().contains("http://localhost:8080" + META), header.getValue());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void surfacesRejectionReasonWhenTokenPresented() throws Exception {
|
||||||
|
HttpServletRequest req = mock(HttpServletRequest.class);
|
||||||
|
when(req.getScheme()).thenReturn("https");
|
||||||
|
when(req.getServerName()).thenReturn("mcp.example.com");
|
||||||
|
when(req.getServerPort()).thenReturn(443);
|
||||||
|
when(req.getHeader("Authorization")).thenReturn("Bearer bad.token");
|
||||||
|
HttpServletResponse resp = mock(HttpServletResponse.class);
|
||||||
|
|
||||||
|
OAuth2Error error =
|
||||||
|
new OAuth2Error(
|
||||||
|
"invalid_token",
|
||||||
|
"Token audience does not include this server's resource id"
|
||||||
|
+ " (https://mcp.example.com/mcp).",
|
||||||
|
null);
|
||||||
|
|
||||||
|
entryPoint.commence(req, resp, new OAuth2AuthenticationException(error));
|
||||||
|
|
||||||
|
ArgumentCaptor<String> header = ArgumentCaptor.forClass(String.class);
|
||||||
|
verify(resp).setHeader(eq("WWW-Authenticate"), header.capture());
|
||||||
|
String www = header.getValue();
|
||||||
|
assertTrue(
|
||||||
|
www.contains("error_description=\"invalid_token - Token audience does not include"),
|
||||||
|
"must surface the rejection reason, got: " + www);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+151
@@ -0,0 +1,151 @@
|
|||||||
|
package stirling.software.proprietary.mcp.security;
|
||||||
|
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
|
|
||||||
|
import java.util.List;
|
||||||
|
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
|
||||||
|
import stirling.software.common.model.ApplicationProperties;
|
||||||
|
|
||||||
|
class McpConfigValidatorTest {
|
||||||
|
|
||||||
|
private static ApplicationProperties.Mcp newMcp() {
|
||||||
|
return new ApplicationProperties.Mcp();
|
||||||
|
}
|
||||||
|
|
||||||
|
private static boolean hasWarn(List<McpConfigValidator.Finding> findings, String needle) {
|
||||||
|
return findings.stream()
|
||||||
|
.anyMatch(
|
||||||
|
f ->
|
||||||
|
f.severity() == McpConfigValidator.Severity.WARN
|
||||||
|
&& f.message().contains(needle));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void apiKeyModeSkipsOAuthChecks() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setMode("apikey");
|
||||||
|
|
||||||
|
List<McpConfigValidator.Finding> findings = McpConfigValidator.validate(mcp);
|
||||||
|
|
||||||
|
assertEquals(1, findings.size());
|
||||||
|
assertEquals(McpConfigValidator.Severity.INFO, findings.get(0).severity());
|
||||||
|
assertTrue(findings.get(0).message().contains("apikey"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void blankIssuerAndResourceProduceWarnings() {
|
||||||
|
// Defaults: oauth mode, blank issuer-uri and resource-id.
|
||||||
|
List<McpConfigValidator.Finding> findings = McpConfigValidator.validate(newMcp());
|
||||||
|
|
||||||
|
assertTrue(hasWarn(findings, "issuer-uri"), "blank issuer must warn");
|
||||||
|
assertTrue(hasWarn(findings, "resource-id"), "blank resource-id must warn");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void subClaimWithRequireAccountWarns() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setIssuerUri("https://issuer.example.com");
|
||||||
|
mcp.getAuth().setResourceId("https://host.example.com/mcp");
|
||||||
|
// Defaults username-claim=sub, require-existing-account=true.
|
||||||
|
|
||||||
|
assertTrue(hasWarn(McpConfigValidator.validate(mcp), "username-claim='sub'"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void completeConfigReportsReadyWithNoWarnings() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setIssuerUri("https://issuer.example.com");
|
||||||
|
mcp.getAuth().setResourceId("https://host.example.com/mcp");
|
||||||
|
mcp.getAuth().setUsernameClaim("email");
|
||||||
|
mcp.setScopesEnabled(false);
|
||||||
|
|
||||||
|
List<McpConfigValidator.Finding> findings = McpConfigValidator.validate(mcp);
|
||||||
|
|
||||||
|
assertTrue(
|
||||||
|
findings.stream().noneMatch(f -> f.severity() == McpConfigValidator.Severity.WARN),
|
||||||
|
"complete config must have no warnings");
|
||||||
|
assertTrue(findings.stream().anyMatch(f -> f.message().contains("look complete")));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void acceptedAudiencesCoverBlankResourceId() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setIssuerUri("https://issuer.example.com");
|
||||||
|
mcp.getAuth().setResourceId("");
|
||||||
|
mcp.getAuth().setAcceptedAudiences(List.of("authenticated"));
|
||||||
|
mcp.getAuth().setUsernameClaim("email");
|
||||||
|
mcp.setScopesEnabled(false);
|
||||||
|
|
||||||
|
List<McpConfigValidator.Finding> findings = McpConfigValidator.validate(mcp);
|
||||||
|
|
||||||
|
assertFalse(
|
||||||
|
hasWarn(findings, "fails closed"),
|
||||||
|
"accepted-audiences must satisfy audience binding without a resource id");
|
||||||
|
assertTrue(
|
||||||
|
findings.stream().anyMatch(f -> f.message().contains("accepted-audiences=")),
|
||||||
|
"configured accepted-audiences should be surfaced");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void strictAudienceHintsAtAcceptedAudiencesEscapeHatch() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setIssuerUri("https://issuer.example.com");
|
||||||
|
mcp.getAuth().setResourceId("https://host.example.com/mcp");
|
||||||
|
mcp.getAuth().setUsernameClaim("email");
|
||||||
|
mcp.setScopesEnabled(false);
|
||||||
|
|
||||||
|
List<McpConfigValidator.Finding> findings = McpConfigValidator.validate(mcp);
|
||||||
|
|
||||||
|
assertTrue(
|
||||||
|
findings.stream().anyMatch(f -> f.message().contains("accepted-audiences")),
|
||||||
|
"should point coarse-audience IdPs at accepted-audiences");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void unrecognizedModeWarnsAboutOAuthFallback() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setMode("api-key"); // near-miss typo that silently runs the OAuth chain
|
||||||
|
|
||||||
|
assertTrue(hasWarn(McpConfigValidator.validate(mcp), "is not recognized"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void requireExistingAccountFalseWarnsAboutOpenAccess() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setIssuerUri("https://issuer.example.com");
|
||||||
|
mcp.getAuth().setResourceId("https://host.example.com/mcp");
|
||||||
|
mcp.getAuth().setUsernameClaim("email");
|
||||||
|
mcp.getAuth().setRequireExistingAccount(false);
|
||||||
|
|
||||||
|
assertTrue(hasWarn(McpConfigValidator.validate(mcp), "require-existing-account=false"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void nonUrlResourceIdWarns() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setIssuerUri("https://issuer.example.com");
|
||||||
|
mcp.getAuth().setResourceId("localhost:8080/mcp"); // missing scheme
|
||||||
|
|
||||||
|
assertTrue(hasWarn(McpConfigValidator.validate(mcp), "is not an http(s) URL"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void allowListIsFlaggedAndOverlapWithBlockListWarns() {
|
||||||
|
ApplicationProperties.Mcp mcp = newMcp();
|
||||||
|
mcp.getAuth().setIssuerUri("https://issuer.example.com");
|
||||||
|
mcp.getAuth().setResourceId("https://host.example.com/mcp");
|
||||||
|
mcp.setAllowedOperations(List.of("merge-pdfs", "split-pdf"));
|
||||||
|
mcp.setBlockedOperations(List.of("split-pdf"));
|
||||||
|
|
||||||
|
List<McpConfigValidator.Finding> findings = McpConfigValidator.validate(mcp);
|
||||||
|
|
||||||
|
assertTrue(
|
||||||
|
findings.stream().anyMatch(f -> f.message().contains("strict allow-list")),
|
||||||
|
"an allow-list should be surfaced");
|
||||||
|
assertTrue(hasWarn(findings, "blocked wins"), "allowed+blocked overlap should warn");
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user