Skip to content

Separate submitted and response salts in UI filters - #167

Open
snoopdave wants to merge 1 commit into
masterfrom
salt-filter-request-handling
Open

Separate submitted and response salts in UI filters#167
snoopdave wants to merge 1 commit into
masterfrom
salt-filter-request-handling

Conversation

@snoopdave

Copy link
Copy Markdown
Contributor

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.

Validate the salt submitted with the request rather than a request attribute, and run validation before the response salt is generated. Move multipart validation into a Struts interceptor after the upload interceptor, since filters cannot read multipart fields. Drop the unused salt.ignored.urls bypass.

Claude-Session: https://claude.ai/code/session_01A1fhY1E2PCFU6UAPXu2WtV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant