Separate submitted and response salts in UI filters

[snoopdave]

Roller's UI CSRF protection uses two salt filters: one checks the token
submitted with a request, the other generates the token for the next response.
This change gives each filter a single responsibility and corrects the order in
which they run.

What changed

• Validate and consume the salt value submitted with the request.
• Run validation before the response-salt filter; mint the fresh response salt
  only after a successful validation, and terminate rejected POSTs without a
  replacement token.
• Defer multipart *.rol validation to an interceptor that runs immediately
  after the upload interceptor, and reject non-Struts multipart requests that
  lack a submitted token.
• Remove the salt.ignored.urls setting and its isIgnoredURL() helper, which
  did not match the shipped values correctly.

Tests

• A request without a valid submitted salt is rejected; one valid token
  succeeds once and a second use of the same token fails.
• Filter ordering validates before response-token generation.
• Successful POSTs that render forms receive a new response token.
• Multipart actions accept one valid token and reject missing, invalid, and
  already-used tokens, exercised on the supported Tomcat deployment and the
  repository Jetty test setup.

[mraible]

The filter order on master was a complete CSRF bypass (LoadSaltFilter ran first and ValidateSaltFilter accepted the freshly minted salt request attribute), so this fix matters and the shape is right. Two regressions need fixing before merge, because making salts genuinely single-use exposes flows that only worked thanks to the bypass:

1. A multipart action with a chain result (bookmarksImport!save → bookmarks) is validated twice; the second pass finds the salt already consumed and throws after the import has committed. (inline)
2. Comments.jsp reuses one #comments_salt for every inline AJAX save, so the second save now fails silently. (inline)

Also worth stating in the description: salts now expire for real (cache.salt.timeout 3600s, cache.salt.size 5000 LRU, one salt minted per /roller-ui request and per tiles forward), so a form left open for an hour, or open on a busy multi-user site, is rejected on submit with the text lost. That's the intended consequence of closing the bypass, but it's a user-visible change from master.

[mraible]

This runs again on every chained invocation of the same request. bookmarksImport!save is multipart and has <result name="success" type="chain">bookmarks</result>, so the second pass finds the salt already removed from SaltCache and throws Security Violation after the OPML import has been flushed. Simplest fix: after a successful consumeSubmittedSalt, set a request attribute (e.g. request.setAttribute("salt.validated", Boolean.TRUE)) and skip validation when it's present. Good candidate for a ValidateSaltInterceptorTest case.

Related: when Struts can't parse the multipart body (over struts.multipart.maxSize), MultiPartRequestWrapper has no fields, getParameter("salt") is null and this throws a 500 instead of letting the fileUpload interceptor's size error reach the form as it does today. Checking ((MultiPartRequestWrapper) request).hasErrors() first would keep that UX.

[mraible]

Comments.jsp:429 reads #comments_salt once and sends it on every commentdata AJAX POST. With the salt removed on first use, editing a second comment without reloading fails here with a 500, and the $.ajax call has no error handler, so the save just silently doesn't happen. CommentDataServlet could return a fresh salt in its JSON (the response passes through LoadSaltFilter, so request.getAttribute("salt") is available) and the JS update #comments_salt from it.

[mraible]

Now that this is the only path, cache.salt.size (5000) and cache.salt.timeout (3600s) are user-visible limits: LoadSaltFilter mints a salt on every /roller-ui request and on every tiles FORWARD, so entries get evicted quickly on a multi-user site, and a form left open for over an hour is rejected with the entry text lost. Not a blocker, but please call it out in the description; raising the defaults would soften it.

[mraible]

Edge case: Struts only wraps requests whose full Content-Type matches its MULTIPART_FORM_DATA_REGEX (boundary ≤ 70 chars, boundary before charset). A multipart POST this check accepts but Struts doesn't wrap skips validation here and then fails in the interceptor because getParameter("salt") is null. Browsers never send those, so probably fine, just noting that deferring on media type alone isn't quite the same decision Struts makes.

[mraible]

cwd-relative, so this passes only when run from app/; the other two tests in this class use getResourceAsStream, and surefire sets project.build.directory for this module.

[mraible]

The comment in ValidateSaltFilter says multipart params only exist after fileUpload, but Struts wraps the request in MultiPartRequestWrapper in StrutsPrepareAndExecuteFilter before any interceptor runs; fileUpload only copies file items into action params. So this ref could sit right after exception (validate before anything else does work), and SaltConfigurationTest:51 shouldn't pin it as adjacent to fileUpload.

[mraible]

Nit: this block is duplicated in the interceptor; a SaltValidator.requireSubmittedSalt(request) that throws would keep both rejection paths in sync.

[mraible]

This builds the chain in validate-then-load order and then asserts that order, so it can't fail if web.xml is swapped back; SaltConfigurationTest already covers the ordering.

[mraible]

Asserting the literal salt.ignored.urls is absent from roller.properties means a migration note like # salt.ignored.urls is no longer supported breaks the build. The behaviour is already covered by the filter no longer reading the property.