Pac4jHttpSecurity: the security filter consumes url-encoded request bodies, so a route can receive an empty form #53

New issue

Closed

opened 2026-09-14 14:09:03 +00:00 by mph · 0 comments

mph commented

2026-09-14 14:09:03 +00:00

Owner

Found while investigating intermittent E2E failures in medeca-modul-poptavky (MEDECA-497).

The shape of the problem

The pac4j security filter reads the body of every application/x-www-form-urlencoded request, and then hands the route the same request whose body it has already consumed. When the server gives the body as a single-read socket stream, the route decodes an empty form — with no error.

The chain, each link read in source (versions as resolved by iw-support 0.1.23):

Pac4jHttpSecurity.contextBuilder builds the web context with a body extractor over the live request: new Http4sWebContext[F](req, _ => dispatcher.unsafeRunSync(req.bodyText.compile.string)) (server/http/src/main/scala/works/iterative/server/http/impl/pac4j/Pac4jHttpSecurity.scala:32-33). baseSecurityFilter passes it to SecurityFilterMiddleware.securityFilter (:75-99); secure wraps that (:116-130); Pac4jModuleRegistry.scala:24 calls secure(clients = clients).
With a client list, pac4j-core DefaultSecurityLogic calls clientFinder.find on every request (6.3.2 :99), and DefaultSecurityClientFinder.find calls context.getRequestParameter(clientNameParameter) (default force_client) whenever client names are set (6.0.3 :53, 6.3.2 :54).
http4s-pac4j 5.0.0 Http4sWebContext.getRequestParameter (:54-60) and getRequestParameters (:67-70) decode the body when the Content-Type is exactly application/x-www-form-urlencoded; for any other type they read the query string only. getRequestContent is a lazy val (:183-184).
DefaultSecurityGrantedAccessAdapter passes context.getRequest downstream (SecurityFilterMiddleware.scala:30-32) — the request whose body pac4j has read.
http4s-blaze-server 0.23.17 makes the body a strict, re-readable chunk only when the whole body arrived in the same buffer as the headers (Http1Stage.scala:215-235). Otherwise the body is a socket stream (streamingBody, :247-289), and a second run of it ends at once with no error (:285).
Chromium writes headers and body in one socket write only when together they are at most 1400 bytes (net/http/http_stream_parser.cc, kMaxMergedHeaderAndBodySize; read on Chromium main). A larger request goes out as two writes, so whether blaze's first read holds the body depends on timing.

Result: a url-encoded POST/PUT of more than about 1400 bytes (headers plus body) sometimes reaches the route with an empty form. In MEDECA-497 a template-preview POST (~544 B body) returned a preview built from an empty form in about 16% of CI executions; a 221 B create request never failed in the same 37 jobs.

Status: every link above is read in source. The split read has not yet been observed at runtime — a split-write probe is planned in MEDECA-497. Independent of that, reading a single-read stream twice is a defect in this module.

Why the consumer cannot fix it cleanly

Overriding val contextBuilder in a subclass does not work: the parent constructor builds callbackService and both logout services with it (Pac4jHttpSecurity.scala:43-71), so the override is still null at that point.
A subclass can override def baseSecurityFilter, but only by copying its body, which then hides every later change made here.
Every app that calls secure(...) with a client list has the exposure. The fix belongs in this module.

Constraints on the fix

pac4j must still see url-encoded form parameters. docs/CSRF_GUIDE.md:195-202 documents a hidden-field pac4jCsrfToken that travels in the url-encoded body, and at least one consumer (cmi-portaly) uses it. DefaultSavedRequestHandler.save (pac4j-core 6.3.2 :32-35) also builds the post-login replay from getRequestParameters. A security-filter context that never reads the body is therefore not acceptable.
Buffer only url-encoded bodies. Every other Content-Type — multipart/form-data, application/octet-stream (e.g. the streaming upload in FileStoreEndpointsModulePlatformSpecific.scala:27-29), JSON — must reach the route as the original, unread stream. Do not use an unconditional BodyCache.
Cover at least http4s-pac4j's condition. Also buffer application/x-www-form-urlencoded with a charset parameter; buffering that superset is safe.
Size limit. Configurable in Pac4jSecurityConfig, default 2 MiB (2097152 bytes — the same default as Tomcat's maxPostSize, which also applies only to bodies parsed into parameters). Count the encoded body bytes. Reject with 413 when Content-Length exceeds the limit, before reading; for a body without Content-Length, stop reading at limit + 1 byte and reject. Today there is no limit on this path at all.
The callback and logout services must keep working (an OIDC form_post callback needs the body). They read it once and nothing reads it after them; confirm that they need no change.

Suggested direction

Inside baseSecurityFilter (or a small middleware that secure applies before the pac4j filter): for a url-encoded request, read the body once within the limit, rebuild the request with a strict body holding those bytes, and build the Http4sWebContext from the rebuilt request. Because SecurityFilterMiddleware hands context.getRequest to the route, the route then decodes the same bytes. Every other request passes through untouched.

Tests

A url-encoded request whose body stream can be consumed only once (e.g. backed by a Ref, emitting its bytes once and nothing on a second run), through secure(clients = Some(...)), to a route that decodes UrlForm: the route sees every field. This test must fail on the current code.
Same request: pac4j still reads a form parameter from the body (e.g. a pac4jCsrfToken hidden field is accepted by the CSRF check).
A multipart body and an octet-stream body reach the route unread (the single-read stream is still unconsumed when the route runs).
A url-encoded body over the limit gives 413, with and without Content-Length; a body exactly at the limit passes.
application/x-www-form-urlencoded; charset=UTF-8 is buffered too.

Delivery

PR into release/0.1.x, release 0.1.24 (the consumer pins 0.1.23), then forward-port to main — the same pattern as #45 / #46.
The consumer side (the supportVersion bump and E2E verification) is tracked in medeca-modul-poptavky MEDECA-497.

Note for consumers

htmx does not swap a 4xx response by default, so a 413 on an htmx request shows the user nothing. Consumers whose forms can approach the limit should make the rejection visible.

Reference: medeca-modul-poptavky, project-management/issues/MEDECA-497/analysis.md, Defect 1 > Hypothesis 1 (on branch MEDECA-497, local worktree ~/Devel/projects/medeca-modul-poptavky-MEDECA-497; not pushed yet).

Found while investigating intermittent E2E failures in `medeca-modul-poptavky` (MEDECA-497). ## The shape of the problem The pac4j security filter reads the body of every `application/x-www-form-urlencoded` request, and then hands the route the same request whose body it has already consumed. When the server gives the body as a single-read socket stream, the route decodes an empty form — with no error. The chain, each link read in source (versions as resolved by iw-support 0.1.23): 1. `Pac4jHttpSecurity.contextBuilder` builds the web context with a body extractor over the live request: `new Http4sWebContext[F](req, _ => dispatcher.unsafeRunSync(req.bodyText.compile.string))` (`server/http/src/main/scala/works/iterative/server/http/impl/pac4j/Pac4jHttpSecurity.scala:32-33`). `baseSecurityFilter` passes it to `SecurityFilterMiddleware.securityFilter` (`:75-99`); `secure` wraps that (`:116-130`); `Pac4jModuleRegistry.scala:24` calls `secure(clients = clients)`. 2. With a client list, pac4j-core `DefaultSecurityLogic` calls `clientFinder.find` on every request (6.3.2 `:99`), and `DefaultSecurityClientFinder.find` calls `context.getRequestParameter(clientNameParameter)` (default `force_client`) whenever client names are set (6.0.3 `:53`, 6.3.2 `:54`). 3. http4s-pac4j 5.0.0 `Http4sWebContext.getRequestParameter` (`:54-60`) and `getRequestParameters` (`:67-70`) decode the **body** when the Content-Type is exactly `application/x-www-form-urlencoded`; for any other type they read the query string only. `getRequestContent` is a lazy val (`:183-184`). 4. `DefaultSecurityGrantedAccessAdapter` passes `context.getRequest` downstream (`SecurityFilterMiddleware.scala:30-32`) — the request whose body pac4j has read. 5. http4s-blaze-server 0.23.17 makes the body a strict, re-readable chunk only when the whole body arrived in the same buffer as the headers (`Http1Stage.scala:215-235`). Otherwise the body is a socket stream (`streamingBody`, `:247-289`), and a second run of it ends at once with no error (`:285`). 6. Chromium writes headers and body in one socket write only when together they are at most 1400 bytes (`net/http/http_stream_parser.cc`, `kMaxMergedHeaderAndBodySize`; read on Chromium `main`). A larger request goes out as two writes, so whether blaze's first read holds the body depends on timing. Result: a url-encoded POST/PUT of more than about 1400 bytes (headers plus body) sometimes reaches the route with an empty form. In MEDECA-497 a template-preview POST (~544 B body) returned a preview built from an empty form in about 16% of CI executions; a 221 B create request never failed in the same 37 jobs. Status: every link above is read in source. The split read has not yet been observed at runtime — a split-write probe is planned in MEDECA-497. Independent of that, reading a single-read stream twice is a defect in this module. ## Why the consumer cannot fix it cleanly - Overriding `val contextBuilder` in a subclass does not work: the parent constructor builds `callbackService` and both logout services with it (`Pac4jHttpSecurity.scala:43-71`), so the override is still `null` at that point. - A subclass can override `def baseSecurityFilter`, but only by copying its body, which then hides every later change made here. - Every app that calls `secure(...)` with a client list has the exposure. The fix belongs in this module. ## Constraints on the fix 1. **pac4j must still see url-encoded form parameters.** `docs/CSRF_GUIDE.md:195-202` documents a hidden-field `pac4jCsrfToken` that travels in the url-encoded body, and at least one consumer (cmi-portaly) uses it. `DefaultSavedRequestHandler.save` (pac4j-core 6.3.2 `:32-35`) also builds the post-login replay from `getRequestParameters`. A security-filter context that never reads the body is therefore not acceptable. 2. **Buffer only url-encoded bodies.** Every other Content-Type — `multipart/form-data`, `application/octet-stream` (e.g. the streaming upload in `FileStoreEndpointsModulePlatformSpecific.scala:27-29`), JSON — must reach the route as the original, unread stream. Do not use an unconditional `BodyCache`. 3. **Cover at least http4s-pac4j's condition.** Also buffer `application/x-www-form-urlencoded` with a charset parameter; buffering that superset is safe. 4. **Size limit.** Configurable in `Pac4jSecurityConfig`, default 2 MiB (2097152 bytes — the same default as Tomcat's `maxPostSize`, which also applies only to bodies parsed into parameters). Count the encoded body bytes. Reject with 413 when `Content-Length` exceeds the limit, before reading; for a body without `Content-Length`, stop reading at limit + 1 byte and reject. Today there is no limit on this path at all. 5. The callback and logout services must keep working (an OIDC `form_post` callback needs the body). They read it once and nothing reads it after them; confirm that they need no change. ## Suggested direction Inside `baseSecurityFilter` (or a small middleware that `secure` applies before the pac4j filter): for a url-encoded request, read the body once within the limit, rebuild the request with a strict body holding those bytes, and build the `Http4sWebContext` from the rebuilt request. Because `SecurityFilterMiddleware` hands `context.getRequest` to the route, the route then decodes the same bytes. Every other request passes through untouched. ## Tests - A url-encoded request whose body stream can be consumed only once (e.g. backed by a `Ref`, emitting its bytes once and nothing on a second run), through `secure(clients = Some(...))`, to a route that decodes `UrlForm`: the route sees every field. This test must fail on the current code. - Same request: pac4j still reads a form parameter from the body (e.g. a `pac4jCsrfToken` hidden field is accepted by the CSRF check). - A multipart body and an octet-stream body reach the route unread (the single-read stream is still unconsumed when the route runs). - A url-encoded body over the limit gives 413, with and without `Content-Length`; a body exactly at the limit passes. - `application/x-www-form-urlencoded; charset=UTF-8` is buffered too. ## Delivery - PR into `release/0.1.x`, release **0.1.24** (the consumer pins 0.1.23), then forward-port to `main` — the same pattern as #45 / #46. - The consumer side (the `supportVersion` bump and E2E verification) is tracked in `medeca-modul-poptavky` MEDECA-497. ## Note for consumers htmx does not swap a 4xx response by default, so a 413 on an htmx request shows the user nothing. Consumers whose forms can approach the limit should make the rejection visible. Reference: `medeca-modul-poptavky`, `project-management/issues/MEDECA-497/analysis.md`, Defect 1 > Hypothesis 1 (on branch `MEDECA-497`, local worktree `~/Devel/projects/medeca-modul-poptavky-MEDECA-497`; not pushed yet).