Skip to content

Commit df9dbc5

Browse files
FrooodleFrooodle
authored
MCP token rejection reason and stop logging the raw tokens (Stirling-Tools#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.
1 parent de9242c commit df9dbc5

8 files changed

Lines changed: 514 additions & 30 deletions

File tree

‎app/proprietary/src/main/java/stirling/software/proprietary/config/CustomAuditEventRepository.java‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package stirling.software.proprietary.config;
22

3+
import java.nio.charset.StandardCharsets;
4+
import java.security.MessageDigest;
5+
import java.security.NoSuchAlgorithmException;
36
import java.time.Instant;
7+
import java.util.HexFormat;
48
import java.util.List;
59
import java.util.Map;
610

@@ -61,17 +65,43 @@ public void add(AuditEvent ev) {
6165

6266
PersistentAuditEvent ent =
6367
PersistentAuditEvent.builder()
64-
.principal(ev.getPrincipal())
68+
.principal(safePrincipal(ev.getPrincipal()))
6569
.type(ev.getType())
6670
.data(auditEventData)
6771
.timestamp(ev.getTimestamp())
6872
.build();
6973
repo.save(ent);
7074
} catch (Exception e) {
71-
log.error(
72-
"Failed to persist audit event (fail-open); principal={}",
73-
ev.getPrincipal(),
74-
e);
75+
log.error("Failed to persist audit event (fail-open); type={}", ev.getType(), e);
76+
}
77+
}
78+
79+
/** Width of the {@code principal} column; longer values are hashed so the insert can't fail. */
80+
private static final int PRINCIPAL_MAX_LENGTH = 255;
81+
82+
/**
83+
* Hash JWT-shaped or over-long principals so the insert fits the column and stores no secret.
84+
*/
85+
static String safePrincipal(String principal) {
86+
if (principal == null || principal.isBlank()) {
87+
return "anonymous";
88+
}
89+
// Hash JWTs ("eyJ...") and any over-long value rather than store verbatim.
90+
if (principal.startsWith("eyJ") || principal.length() > PRINCIPAL_MAX_LENGTH) {
91+
return "token:" + sha256Prefix(principal);
92+
}
93+
return principal;
94+
}
95+
96+
/** First 8 bytes of SHA-256 as hex: stable, one-way, collision-safe enough. */
97+
private static String sha256Prefix(String value) {
98+
try {
99+
byte[] digest =
100+
MessageDigest.getInstance("SHA-256")
101+
.digest(value.getBytes(StandardCharsets.UTF_8));
102+
return HexFormat.of().formatHex(digest, 0, 8);
103+
} catch (NoSuchAlgorithmException e) {
104+
return "unhashable";
75105
}
76106
}
77107
}

‎app/proprietary/src/main/java/stirling/software/proprietary/mcp/security/McpAudienceValidator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ public OAuth2TokenValidatorResult validate(Jwt token) {
4343
return OAuth2TokenValidatorResult.failure(
4444
new OAuth2Error(
4545
"invalid_token",
46-
"MCP server has no resource id configured; rejecting all tokens"
47-
+ " until mcp.auth.resource-id is set.",
46+
"MCP audience binding is not configured; rejecting all tokens until"
47+
+ " mcp.auth.resource-id or mcp.auth.accepted-audiences is set.",
4848
null));
4949
}
5050
List<String> aud = token.getAudience();

‎app/proprietary/src/main/java/stirling/software/proprietary/mcp/security/McpAuthenticationEntryPoint.java‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,21 @@
44

55
import org.springframework.http.HttpStatus;
66
import org.springframework.security.core.AuthenticationException;
7+
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
8+
import org.springframework.security.oauth2.core.OAuth2Error;
79
import org.springframework.security.web.AuthenticationEntryPoint;
810

911
import jakarta.servlet.http.HttpServletRequest;
1012
import jakarta.servlet.http.HttpServletResponse;
1113

14+
import lombok.extern.slf4j.Slf4j;
15+
1216
/**
13-
* Emits 401 + {@code WWW-Authenticate: Bearer resource_metadata="..."} (RFC 9728), preferring
14-
* X-Forwarded-* headers to build the public-facing metadata URL.
17+
* Emits 401 + {@code WWW-Authenticate: Bearer resource_metadata="..."} (RFC 9728) from
18+
* X-Forwarded-* headers. A rejected token also logs the OAuth2 reason and echoes it as {@code
19+
* error_description}.
1520
*/
21+
@Slf4j
1622
public class McpAuthenticationEntryPoint implements AuthenticationEntryPoint {
1723

1824
private final String metadataPath;
@@ -28,15 +34,47 @@ public void commence(
2834
HttpServletResponse response,
2935
AuthenticationException authException)
3036
throws IOException {
37+
// Tokenless 401 is the normal discovery handshake; only a rejected token is a real failure.
38+
boolean tokenPresented = request.getHeader("Authorization") != null;
39+
String reason = rejectionReason(authException);
40+
if (tokenPresented) {
41+
log.warn("MCP rejected bearer token: {}", reason != null ? reason : "invalid_token");
42+
} else {
43+
log.debug("MCP 401: no bearer token; returning protected-resource metadata pointer");
44+
}
45+
3146
String scheme = firstForwarded(request, "X-Forwarded-Proto", request.getScheme());
3247
String authority = forwardedHost(request, scheme);
3348
String metadataUrl = scheme + "://" + authority + metadataPath;
34-
response.setHeader(
35-
"WWW-Authenticate",
36-
"Bearer error=\"invalid_token\", resource_metadata=\"" + metadataUrl + "\"");
49+
50+
StringBuilder header = new StringBuilder("Bearer error=\"invalid_token\"");
51+
if (tokenPresented && reason != null) {
52+
header.append(", error_description=\"").append(reason).append('"');
53+
}
54+
header.append(", resource_metadata=\"").append(metadataUrl).append('"');
55+
response.setHeader("WWW-Authenticate", header.toString());
3756
response.sendError(HttpStatus.UNAUTHORIZED.value(), "Unauthorized");
3857
}
3958

59+
/**
60+
* OAuth2 error as {@code "code - description"}, sanitized for a header/log line; null if none.
61+
*/
62+
private static String rejectionReason(AuthenticationException ex) {
63+
if (!(ex instanceof OAuth2AuthenticationException oae)) {
64+
return null;
65+
}
66+
OAuth2Error error = oae.getError();
67+
if (error == null) {
68+
return null;
69+
}
70+
String description = error.getDescription();
71+
String combined =
72+
(description == null || description.isBlank())
73+
? error.getErrorCode()
74+
: error.getErrorCode() + " - " + description;
75+
return combined == null ? null : combined.replaceAll("[\\r\\n\"]", " ").trim();
76+
}
77+
4078
/** host[:port] from forwarded headers when present, else the servlet host/port. */
4179
private static String forwardedHost(HttpServletRequest request, String scheme) {
4280
String host = firstForwarded(request, "X-Forwarded-Host", null);

‎app/proprietary/src/main/java/stirling/software/proprietary/mcp/security/McpConfigValidator.java‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
package stirling.software.proprietary.mcp.security;
2+
3+
import java.util.ArrayList;
4+
import java.util.List;
5+
import java.util.Locale;
6+
7+
import stirling.software.common.model.ApplicationProperties;
8+
9+
/**
10+
* Startup sanity-checks for MCP config; {@link McpSecurityConfig} logs the findings at boot so a
11+
* misconfigured /mcp endpoint shows up in the logs instead of as a later rejected-token 401.
12+
*/
13+
public final class McpConfigValidator {
14+
15+
public enum Severity {
16+
WARN,
17+
INFO
18+
}
19+
20+
public record Finding(Severity severity, String message) {}
21+
22+
private McpConfigValidator() {}
23+
24+
/** Inspect the resolved MCP config and return ordered findings (most actionable first). */
25+
public static List<Finding> validate(ApplicationProperties.Mcp mcp) {
26+
List<Finding> findings = new ArrayList<>();
27+
ApplicationProperties.Mcp.Auth auth = mcp.getAuth();
28+
29+
if ("apikey".equalsIgnoreCase(auth.getMode())) {
30+
findings.add(
31+
info(
32+
"auth mode = apikey - clients send a Stirling API key via X-API-KEY (or"
33+
+ " Authorization: Bearer <key>); no external IdP needed. The key"
34+
+ " must belong to a provisioned, enabled account (Account -> API"
35+
+ " Keys)."));
36+
return findings;
37+
}
38+
39+
// Anything that isn't exactly "apikey" runs the OAuth chain (mirrors isApiKeyMode()).
40+
String mode = auth.getMode();
41+
if (mode != null && !mode.isBlank() && !"oauth".equalsIgnoreCase(mode.trim())) {
42+
findings.add(
43+
warn(
44+
"mcp.auth.mode='"
45+
+ mode
46+
+ "' is not recognized (expected 'oauth' or 'apikey'); it falls"
47+
+ " back to the OAuth chain, which rejects every token unless"
48+
+ " issuer-uri and resource-id are set. A near-miss like"
49+
+ " 'api-key' is NOT treated as API-key mode."));
50+
}
51+
findings.add(info("auth mode = oauth - running as an OAuth2 resource server for /mcp."));
52+
53+
if (isBlank(auth.getIssuerUri())) {
54+
findings.add(
55+
warn(
56+
"mcp.auth.issuer-uri is not set: the JWT decoder fails closed and rejects"
57+
+ " every token. Set it to your IdP issuer that publishes"
58+
+ " /.well-known/openid-configuration (e.g."
59+
+ " https://login.microsoftonline.com/<tenant>/v2.0)."));
60+
} else if (!looksLikeUrl(auth.getIssuerUri())) {
61+
findings.add(
62+
warn(
63+
"mcp.auth.issuer-uri='"
64+
+ auth.getIssuerUri()
65+
+ "' does not look like an http(s) URL."));
66+
}
67+
68+
boolean hasResourceId = !isBlank(auth.getResourceId());
69+
boolean hasAcceptedAudiences =
70+
auth.getAcceptedAudiences().stream().anyMatch(a -> !isBlank(a));
71+
72+
if (!hasResourceId && !hasAcceptedAudiences) {
73+
findings.add(
74+
warn(
75+
"neither mcp.auth.resource-id nor mcp.auth.accepted-audiences is set: the"
76+
+ " audience validator fails closed and rejects every token (RFC"
77+
+ " 8707). Set resource-id to this server's public /mcp URL, or"
78+
+ " accepted-audiences to the audience your IdP actually mints."));
79+
} else {
80+
if (hasResourceId && !looksLikeUrl(auth.getResourceId())) {
81+
findings.add(
82+
warn(
83+
"mcp.auth.resource-id='"
84+
+ auth.getResourceId()
85+
+ "' is not an http(s) URL: the token aud must match it"
86+
+ " exactly (scheme, host and port included)."));
87+
} else if (hasResourceId && !auth.getResourceId().endsWith("/mcp")) {
88+
findings.add(
89+
warn(
90+
"mcp.auth.resource-id='"
91+
+ auth.getResourceId()
92+
+ "' does not end in /mcp: it must match the public URL"
93+
+ " clients call and the audience your IdP puts in the"
94+
+ " token."));
95+
}
96+
if (hasAcceptedAudiences) {
97+
findings.add(
98+
info(
99+
"mcp.auth.accepted-audiences="
100+
+ auth.getAcceptedAudiences()
101+
+ " - tokens whose aud matches any of these are accepted, the"
102+
+ " escape hatch for IdPs that can't mint a resource-specific"
103+
+ " audience (e.g. an Entra ID app id, or Supabase's"
104+
+ " aud=authenticated)."));
105+
} else {
106+
findings.add(
107+
info(
108+
"audience binding is strict (token aud must equal"
109+
+ " mcp.auth.resource-id). If your IdP can't mint that - e.g."
110+
+ " Entra ID issues aud=<client-id> - set"
111+
+ " mcp.auth.accepted-audiences to the audience it actually"
112+
+ " emits."));
113+
}
114+
}
115+
116+
if (isBlank(auth.getJwksUri())) {
117+
findings.add(
118+
info(
119+
"mcp.auth.jwks-uri not set - signing keys are auto-discovered from the"
120+
+ " issuer's OpenID configuration."));
121+
}
122+
123+
if ("sub".equalsIgnoreCase(auth.getUsernameClaim()) && auth.isRequireExistingAccount()) {
124+
findings.add(
125+
warn(
126+
"mcp.auth.username-claim='sub' with require-existing-account=true: many"
127+
+ " IdPs (e.g. Entra ID, Google) set 'sub' to an opaque id that won't"
128+
+ " match a Stirling username. Set mcp.auth.username-claim to 'email'"
129+
+ " or 'preferred_username', or provision accounts keyed by sub."));
130+
}
131+
132+
if (!auth.isRequireExistingAccount()) {
133+
findings.add(
134+
warn(
135+
"mcp.auth.require-existing-account=false: any token your IdP signs can"
136+
+ " invoke MCP tools even if its subject has no Stirling account. Set"
137+
+ " it true unless you intend open access for every IdP-valid"
138+
+ " token."));
139+
}
140+
141+
if (mcp.isScopesEnabled()) {
142+
findings.add(
143+
info(
144+
"mcp.scopes-enabled=true - the IdP must mint 'mcp.tools.read' and"
145+
+ " 'mcp.tools.write' scopes or clients are rejected; set"
146+
+ " mcp.scopes-enabled=false if it can only issue coarse tokens."));
147+
}
148+
149+
List<String> allowed = mcp.getAllowedOperations();
150+
List<String> blocked = mcp.getBlockedOperations();
151+
if (allowed != null && !allowed.isEmpty()) {
152+
findings.add(
153+
info(
154+
"mcp.allowed-operations is a strict allow-list of "
155+
+ allowed.size()
156+
+ " operation(s); every other tool is hidden, so a wrong or"
157+
+ " typo'd id silently exposes nothing."));
158+
List<String> shadowed =
159+
blocked == null
160+
? List.of()
161+
: allowed.stream().filter(blocked::contains).toList();
162+
if (!shadowed.isEmpty()) {
163+
findings.add(
164+
warn(
165+
"mcp operation(s) "
166+
+ shadowed
167+
+ " are in both allowed-operations and blocked-operations;"
168+
+ " blocked wins, so they are hidden."));
169+
}
170+
}
171+
172+
if (findings.stream().noneMatch(f -> f.severity() == Severity.WARN)) {
173+
findings.add(info("OAuth settings look complete."));
174+
}
175+
176+
return findings;
177+
}
178+
179+
private static boolean isBlank(String value) {
180+
return value == null || value.isBlank();
181+
}
182+
183+
private static boolean looksLikeUrl(String value) {
184+
String lower = value.toLowerCase(Locale.ROOT);
185+
return lower.startsWith("http://") || lower.startsWith("https://");
186+
}
187+
188+
private static Finding warn(String message) {
189+
return new Finding(Severity.WARN, message);
190+
}
191+
192+
private static Finding info(String message) {
193+
return new Finding(Severity.INFO, message);
194+
}
195+
}

‎app/proprietary/src/main/java/stirling/software/proprietary/mcp/security/McpSecurityConfig.java‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -77,24 +77,14 @@ private void applyCors(HttpSecurity http) throws Exception {
7777
}
7878

7979
@PostConstruct
80-
void warnIfMisconfigured() {
81-
ApplicationProperties.Mcp mcp = applicationProperties.getMcp();
82-
if (isApiKeyMode()) {
83-
log.info(
84-
"MCP auth mode = apikey: clients authenticate with a Stirling per-user API key"
85-
+ " (X-API-KEY header). No OAuth issuer required.");
86-
} else {
87-
if (mcp.getAuth().getIssuerUri().isBlank()) {
88-
log.warn(
89-
"MCP enabled but mcp.auth.issuer-uri is blank - JWT decoder will reject"
90-
+ " every token (fail-closed). Set mcp.auth.issuer-uri and"
91-
+ " mcp.auth.resource-id before exposing /mcp to clients.");
92-
}
93-
if (mcp.getAuth().getResourceId().isBlank()) {
94-
log.warn(
95-
"MCP enabled but mcp.auth.resource-id is blank - audience validator will"
96-
+ " reject every token. Set this to the public URL of the MCP"
97-
+ " endpoint (RFC 8707).");
80+
void validateConfigOnStartup() {
81+
log.info("MCP server enabled - validating configuration:");
82+
for (McpConfigValidator.Finding finding :
83+
McpConfigValidator.validate(applicationProperties.getMcp())) {
84+
if (finding.severity() == McpConfigValidator.Severity.WARN) {
85+
log.warn("MCP config: {}", finding.message());
86+
} else {
87+
log.info("MCP config: {}", finding.message());
9888
}
9989
}
10090
}

0 commit comments

Comments
 (0)