fix(sso): refuse a SAML LogoutRequest that names another user

[marevol]

The problem

/sso/logout is anonymous and, because SAML requires tomcat.sameSiteCookies=none,
is reachable cross-site with the victim's session cookie attached. With the
shipped default saml.security.want_messages_signed=false, java-saml accepts a
LogoutRequest that carries no signature, and every other check it makes is
conditional on an attribute the sender simply omits:

check	why it does not fire
signature	LogoutRequest.java:479 — only when getWantMessagesSigned()
NotOnOrAfter	:452 — guarded by hasAttribute
Destination	:462 — guarded by hasAttribute
Issuer vs saml.idp.entityid	:474 — if (issuer != null && …), and getIssuer() returns null when the element is absent (:905-917). saml-schema-protocol-2.0.xsd:31 declares <element ref="saml:Issuer" minOccurs="0"/>, so omitting it still passes XML validation

The NameID is the one element java-saml insists on (:694-696 throws NO_NAMEID),
and its value was never compared with anything. So ending an authenticated
session required no knowledge of the deployment at all — not even the IdP entity
ID. Auth.processSLO(false, …) then invalidates the session at Auth.java:1242.

Impact is a forced logout (denial of service), not account takeover.

The change

getLogoutResponse() now compares the LogoutRequest's NameID with the session
user's before letting java-saml invalidate the session. On a mismatch the
session is kept and the IdP still gets an ordinary LogoutResponse — an error
would tell an unauthenticated sender whether it guessed a live session, and
would leave a confused-but-legitimate IdP unable to finish its own logout.

SamlUser.getName() is the NameID: it is what logout(FessUserBean) already
passes as the nameId of the LogoutRequest Fess sends.

The SAMLResponse branch is untouched.

Why the comparison is not equals

The risk is asymmetric — a false match only restores the previous behaviour,
while a false mismatch silently breaks a working single logout.

• Both sides are trimmed. They are read from the text content of two
  different XML documents, and java-saml trims neither unless
  saml.parsing.trim_name_ids is on, which Fess leaves off. An IdP that
  pretty-prints its LogoutRequest but not its assertion would otherwise look
  like a different user on every logout.
• Case is ignored. An IdP that normalises an email or UPN differently
  between its assertion and its LogoutRequest is a real deployment. It costs
  nothing: a sender who does not know the NameID fails at any case, and one who
  does gains nothing from changing it.

Fail-open by design

Nothing short of a clear mismatch changes behaviour: nobody logged in, a user
who did not come from SAML, a malformed message, an EncryptedID with no SP key
to open it. All of them mean "cannot tell", which keeps today's behaviour.

The message is parsed through new LogoutRequest(settings, ServletUtils.makeHttpRequest(request))
with the same argument list LogoutRequest.isValid() uses, not by hand — hand
parsing would put an XML parser, and therefore an XXE surface, in front of an
unauthenticated sender. Constructing a LogoutRequest touches no replay cache
(only isValid() registers an ID), so parsing twice does not make java-saml
reject its own copy.

The NameID is bounded to 64 characters and stripped of control characters before
it reaches the log, the way SpnegoAuthenticator.sanitizeForLog already treats a
client-supplied realm — it is written before anything authenticated the
message, and it is XML text content, so it can carry a newline.

What this does NOT fix

This narrows the exposure, it does not close it. Still ended by a crafted
request:

• a SAML session whose NameID the sender already knows (with the default
  emailAddress format, that may just be the victim's email address)
• any session that did not come from SAML (local admin, LDAP), since there is no
  NameID to compare

saml.security.want_messages_signed=true remains the real fix. The class javadoc
and the unsigned_logoutrequest_accepted comment are updated to say exactly
this, since both previously asserted that the NameID is never compared.

Verification

• mvn -o clean test -Dtest=SamlAuthenticatorTest → Tests run: 45, Failures: 0, Errors: 0, Skipped: 0 (was 36)
• mvn -o test -Dtest='*Sso*,*Saml*' → Tests run: 167, Failures: 0, Errors: 0, Skipped: 0
• mvn -o javadoc:jar → BUILD SUCCESS (the one remaining warning is the pre-existing, unrelated ChunkBoundaryFinder.java:882)
• mvn -o formatter:format license:format → no further changes

12 mutations were applied to the production code and each was confirmed to fail a
test before being restored: restoring processSLO(false, …), dropping the
SAMLRequest guard, dropping .trim(), equalsIgnoreCase → equals, dropping
the instanceof SamlUser check, removing the exception swallowing, dropping the
blank-NameID guards, removing the WARN, inverting the comparison, forcing the
NameID reader to null, and making sanitizeForLog return its input unchanged.
None passed.

Worth knowing before merge

• The check applies to signed LogoutRequests too. With
  want_messages_signed=true the message is authenticated, yet a NameID mismatch
  still keeps the session. That is deliberate — honouring it would log out the
  wrong user — but it means a correctly-secured deployment is also exposed to the
  false-mismatch failure mode. There is no kill switch; a mismatch is at least
  no longer silent, since the WARN names both values.
• The production wiring of getSavedUserBean() is not covered by a test. In
  the unit container FessLoginAssist resolves as a component def but fails to
  auto-inject its @Resource UserBhv (no user index), so the tests override a
  protected seam — the same idiom MeHandlerTest uses. ViewHelper and
  MeHandler rely on the same call in production. One manual SLO round trip
  against a real IdP before release would be worth it.

Related

• docs(sso): correct the SAML single logout warning and surface the AuthnRequest cap #3257 and docs(sso): document saml.request.id.ttl and correct two SAML inaccuracies fess-docs#487 correct the comments and docs that understated this exposure (different regions of the same file; merges cleanly).