Example fixes for 3792

[fhanik]

Harden the RFC 8705 mTLS token endpoint

Seven patches closing six security weaknesses and one regression, each backed by a
failing test in MtlsTokenEndpointHardeningMockMvcTests /
MtlsDisabledTokenEndpointMockMvcTests.

1. Why test code changed

Changing tests alongside a fix is normally a smell — it's how a red test gets argued
into green without the defect being fixed. Three test files change here. None of them
weakens an assertion about behaviour under test, and the distinction matters enough to
itemise.

1.1 A pre-existing test asserted the absence of the rule being added

ClientAdminEndpointsValidatorTests.validateTlsClientAuthClaimConfig_acceptsSubTemplateAtExactlyMaxLength
fed the validator a 256-character all-{ template and asserted assertThatNoException().

That test's purpose is a ReDoS timing bound — its own comment says so, and its real
assertion is elapsedMillis < 100. The pathological input it uses happens to contain no
closing brace, so it also contains no placeholder. This patch rejects placeholder-free
templates, which means that input is now invalid for a second, unrelated reason.

The test was updated to assert the rejection arrives quickly, and renamed to
validateTlsClientAuthClaimConfig_rejectsPlaceholderlessSubTemplateAtMaxLengthQuickly.
The timing bound — the thing the test exists to protect — is unchanged. What changed is
the expected outcome for one input, because the patch deliberately changes it. A test
that pins behaviour a patch intentionally alters has to move with the patch; the
alternative is keeping a rule the test forbids.

No other author test changed. MtlsClaimsEnhancerTest, TlsClientAuthenticationTest,
ClientDetailsAuthenticationProviderTests and UaaTokenEndpointTests are untouched —
including UaaTokenEndpointTests, which is why doDelegateGet keeps its two-argument
signature and the servlet request is resolved from RequestContextHolder instead.

1.2 One of my own assertions over-specified the fix

B1's refusal branch asserted error == "invalid_client". The security property is
"the mTLS endpoint must not issue a certificate-bound user token" — but that assertion
had quietly encoded a second requirement: "and it must be refused as a
client-authentication failure."

A grant type the endpoint does not serve is invalid_grant, not invalid_client. My
assertion had prejudged which layer the fix belonged in. It now accepts any 4xx carrying
an OAuth error code, and still fails on a 5xx or a body with no error code — so the thing
it actually guards against (an ugly failure) is intact, while the implementation choice
is no longer constrained by the test.

This is my bug, introduced before the fix existed, and it is the only assertion in the
suite that was loosened.

1.3 New servlet filter needs wiring into the MockMvc chain

DefaultTestContext builds MockMvc with exactly three filters: zone-path rewriting,
zone session, and the Spring Security chain. A filter registered in
SpringServletXmlFiltersConfiguration does not appear there automatically —
TokenEndpointDocs already has to add rawPeerCertificateCaptureFilter by hand for the
same reason, with a comment saying so.

MtlsDisabledTokenEndpointMockMvcTests now builds its own MockMvc including
mtlsEndpointAvailabilityFilter. That is harness wiring, not an assertion: without it
the filter under test never executes, and E1 would pass or fail for reasons unrelated to
the patch.

1.4 What did not change

The assertions expressing the seven findings are byte-identical to the versions that were
red before this commit. A3, C5, D2, D3, E1 and E4 assert exactly what they asserted then.
If a patch is wrong, its test still fails.

2. Backwards compatibility

Every patch is gated on a condition that is false for all pre-existing traffic. Two are
gated on the request path, three on per-client mTLS configuration, one is deny-only on a
single path, and one strictly widens what the API accepts.

2.1 Patches gated on the mTLS request path

ClientDetailsAuthenticationProvider's new guard and UaaTokenEndpoint's grant
restriction both begin with a path test:

if (isTlsClientAuthPath(authentication.getDetails()) && !tlsClientAuthConfigured) { ... }

if (request == null || !RawPeerCertificateCaptureFilter.isMtlsTokenPath(request.getServletPath())) {
    return;
}

isMtlsTokenPath matches /oauth/mtls/token and paths beneath it, and nothing else. For
/oauth/token, /oauth/token/alias/{entityId}, /oauth/authorize, the SCIM endpoints,
the login UI and every other route, both patches are a no-op before they read anything.
client_secret_basic, client_secret_post, private_key_jwt, none, PKCE, SAML
bearer, JWT bearer and refresh flows are not reachable from either branch.

The else if (isTlsClientAuthPath(...)) branch removed from the empty-credentials block
was itself reachable only on that same path, and only for clients with no
tls-client-auth-ca — exactly the case the new guard now rejects earlier. Nothing
outside /oauth/mtls/token can reach either.

2.2 Patches gated on per-client mTLS configuration

validateTlsClientAuth's new catch is reached only from the branch guarded by
TlsClientAuthConfiguration.isConfigured(...) — a client with no tls-client-auth-ca
never enters it. The converted exception keeps e.getMessage() verbatim, so even for
mTLS clients the HTTP response body is unchanged on the path that already returned 401;
only the path that returned 500 moves.

ClientAdminEndpointsValidator.validateTlsClientAuthClaimConfig reads only keys prefixed
tls-client-auth-. A client carrying none of them reaches no new code:
requireAtLeastOnePlaceholder runs only for a non-blank template that exists, and the
RESERVED_CLAIM_NAMES check only for entries in tls-client-auth-claim-mappings.

MtlsClaimsEnhancer.enhance already returned an empty map unless the request carried a
certificate and the authentication method was tls_client_auth and the client had
TLS config. The reserved-name skip and the placeholder check sit inside that path.

These three do change behaviour for mTLS clients configured with a placeholder-free
template or a mapping onto a reserved claim — that is the point. tls_client_auth ships
for the first time in this PR, so no released deployment has such a client to break.

RESERVED_CLAIM_NAMES is a static final constant on TlsClientAuthConfiguration, not
an instance field. UaaClientDetails and TlsClientAuthConfiguration serialise exactly
as before; no stored client JSON changes shape, and no migration is implied.

2.3 The new filter is deny-only and scoped to one path

MtlsEndpointAvailabilityFilter runs on every request, which is worth being explicit
about. Its entire body is:

if (!mtlsEnabled && RawPeerCertificateCaptureFilter.isMtlsTokenPath(httpRequest.getServletPath())) {
    ((HttpServletResponse) response).sendError(HttpServletResponse.SC_NOT_FOUND);
    return;
}
chain.doFilter(request, response);

It has no branch that grants anything. When uaa.mtls-enabled is true it is a
pass-through for all paths. When false it affects only /oauth/mtls/token, which today
answers 403 "Could not verify the provided CSRF token" from the uiSecurity catch-all
— no working flow depends on that response. Cost on every other request is one string
comparison.

Order -290 places it after ZonePathContextRewritingFilter
(Ordered.HIGHEST_PRECEDENCE + 1), so /z/{subdomain}/oauth/mtls/token is matched
identically to a direct request, and before Spring Security (-100), so the path never
reaches a filter chain. It does not displace rawPeerCertificateCaptureFilter (-300)
or clientCertificateMapperFilter (-200); the relative order of those two, and their
relationship to the security filter, is unchanged.

2.4 The one patch that touches every client widens what is accepted

Removing the token-endpoint-auth-method rejection from checkMtlsClientConfigAllowed
is the only change on a code path every client create and update traverses. It deletes a
validation; it adds none. A client that was accepted before is still accepted, and
clients carrying that key — silently stored and ignored by UAA for years, and rejected
only by this PR — can be created and updated again. Nothing in the codebase reads the key.

2.5 Not changed

Certificate validation itself is untouched: PKIX path building,
validateEndEntityConstraints, the XFCC trusted-proxy checks,
RawPeerCertificateCaptureFilter, MtlsPathGuardedFilter, the Tomcat/BCJSSE connector
customiser and required-claims enforcement are all as the author wrote them. So are
UaaTokenServices, ClientCredentialsTokenGranter, the OIDC discovery changes and the
ClientAuthentication constants.

3. The patches

#	Weakness	Patch
A3	/oauth/mtls/token served any client with a valid client_secret, certificate or not — while OIDC discovery advertises it as mtls_endpoint_aliases.token_endpoint	ClientDetailsAuthenticationProvider rejects any client without tls-client-auth-ca on that path, whatever credentials it presents
B1	A certificate-authenticated client could run the password grant there, producing one token carrying both cnf.x5t#S256 (RFC 8705 §3 sender-constraint) and user_id/user_name/email — two unrelated identities, with the returned refresh token usable at the same endpoint	UaaTokenEndpoint serves only client_credentials on that path, rejected as invalid_grant, without echoing the submitted grant type
C5	InvalidClientDetailsException is a UaaException → OAuth2Exception → RuntimeException, so it escaped ClientBasicAuthenticationFilter (which catches only AuthenticationException): the same untrusted certificate returned 401 with a client_id parameter and 500 plus an ERROR stack trace with Authorization: Basic clientId: — an unauthenticated log-flooding vector	Converted to BadCredentialsException at the throw site, message preserved
D2	A tls-client-auth-sub-template with no {placeholder} renders to itself, and UaaTokenServices re-applies sub after its defaults — so a constant template set sub to any fixed string, including a real user's id	Validator requires at least one placeholder in sub and aud templates; MtlsClaimsEnhancer.renderTemplate drops placeholder-free templates as defense in depth for the bootstrap path, which bypasses admin-API validation
D3	NON_ADDITIONAL_ROOT_CLAIMS protects UAA's own claims but not amr, acr, auth_time, client_auth_method or cnf, and the validator applied no allowlist to claim names — so a certificate subject field could assert, in a signed token, how the caller authenticated	New TlsClientAuthConfiguration.RESERVED_CLAIM_NAMES; validator rejects mappings onto them, enhancer skips them
E1	With uaa.mtls-enabled=false the security chain is absent but the @RequestMapping is not, so the path fell through to the browser login chain and was stopped by CSRF rather than by any gate	New MtlsEndpointAvailabilityFilter returns 404 for that path when the feature is off
E4	checkMtlsClientConfigAllowed rejected every client create/update carrying token-endpoint-auth-method — not a UAA property, not read anywhere, and reachable via hyphenated BOSH manifest keys	Check removed

4. Known gaps not addressed

Deliberately out of scope, to keep the commit to its tests:

• GET /oauth/mtls/token?grant_type=... is still permitted by the inherited
  allowQueryStringForTokens default.
• clientCertificateMapperFilter still reflectively instantiates a package-private
  third-party class at startup even when the feature is disabled.
• validateTlsClientAuth still returns a bare false, so "no certificate presented" and
  "trusted certificate, wrong required-claims" produce byte-identical denials — A4
  and D4 both pin tls_client_auth: certificate validation failed.