fix(security): harden RequestCache so browser auto-probes cannot hijack the post-login redirect

[devondragon]

Problem

Clicking a deep link to a protected page while logged out (e.g. an email link) runs the user through login, then redirects to an error page at /apple-touch-icon.png?continue# instead of the original URL. Reproduced in production on two consuming apps (MagicMenu PRD logs, 2026-07-23).

Mechanism: Spring Security's default RequestCache saves any request that triggers authentication. Safari/iOS automatically probes /apple-touch-icon.png (and -precomposed) at the site root while rendering any page — including the login page. When that path is protected (the common case under defaultAction=deny unless every icon variant is whitelisted), the unauthenticated probe overwrites the saved request for the page the user actually clicked. SavedRequestAwareAuthenticationSuccessHandler then redirects to the icon URL after login. Spring Security's built-in matcher only excludes favicon.*, XHR, and application/json — touch icons, HTMX partials, and other asset fetches all slip through.

Fix

Two complementary changes:

1. Hardened, consumer-overridable RequestCache (the root-cause fix)

• New RequestCache bean in UserSecurityBeansAutoConfiguration (@ConditionalOnMissingBean(RequestCache.class), same pattern as the other security beans): an HttpSessionRequestCache whose matcher only saves plausible user navigations —
  • GET requests
  • that explicitly accept text/html (Accept: */* is ignored — real navigations always list text/html)
  • and are not XHR (X-Requested-With: XMLHttpRequest), HTMX (HX-Request), static-asset fetches (png/ico/css/js/fonts/etc.), or well-known auto-probed paths (/apple-touch-icon*, /favicon*, /.well-known/)
• Wired into buildSecurityFilterChain via http.requestCache(...). No public signature changes — consumers delegating to buildSecurityFilterChain(HttpSecurity, SessionRegistry) are unaffected.
• Consumers who want Spring Security's default behavior back just define their own RequestCache bean.

2. Auto-unprotect the always-public probe paths (convenience/cleanliness)

• getUnprotectedURIsList() now auto-adds /apple-touch-icon*, /favicon*, and /.well-known/** for every consumer, mirroring the RequestCache exclusion set. Without this, under defaultAction=deny these probes 302 to login unless each app remembers to whitelist every variant. Follows the existing precedent in the same method that auto-unprotects the MFA entry-point URIs.

Tests

• RequestCacheHardeningTest (7 tests), written red-first against the unfixed behavior: real navigation still saved; icon probe on the same session no longer overwrites it (the reported bug); icon paths never saved regardless of Accept header; non-HTML / HTMX / XHR / POST never saved.
• WebSecurityAuthorizationDenyTest.AutoUnprotectedProbePaths (5 tests): these paths are absent from application-test.yml's unprotectedURIs, so a 404 (reached the dispatcher) rather than a 302 (login redirect) proves the auto-unprotect applied.
• Full suite: 1003 tests, 0 failures, 0 skipped.

Consuming-app companions

• MagicMenu app-layer fix: devondragon/MagicMenu#361
• Reference app whitelist update: SpringUserFrameworkDemoApp (separate PR)

[claude]

Review: harden RequestCache against browser auto-probe hijacking

Nice root-cause fix for a nasty, hard-to-reproduce bug — the Safari touch-icon-probe race with HttpSessionRequestCache is a subtle interaction, and restricting the saved-request matcher to plausible navigations (GET + explicit text/html + not XHR/HTMX + not asset/probe path) is the right, minimal-footprint way to close it. It also follows the established @ConditionalOnMissingBean pattern from UserSecurityBeansAutoConfiguration exactly, so consumers keep a clean override hook. The javadoc is thorough and the new RequestCacheHardeningTest is well-targeted (red-first against the actual repro, not just unit-testing the matcher in isolation).

Correctness

• MediaTypeRequestMatcher uses sortBySpecificityAndQuality, so a concrete text/html in the Accept header always sorts ahead of */* regardless of quality-value ordering — the acceptsHtml/ignoredMediaTypes(ALL) combo behaves correctly even for unusual Accept header orderings. Good.
• isStaticAssetOrAutoProbe correctly strips the context path before matching, and the lastDot > lastSlash check correctly avoids misclassifying a dotted path segment (e.g. /user.service/page) as a file extension.
• Logic checks out for the scenarios the PR describes and the new tests.

Minor issues

1. Import order (UserSecurityBeansAutoConfiguration.java:30): import org.springframework.http.MediaType; is appended after the org.springframework.security.* imports, breaking alphabetical order (CLAUDE.md: "Imports: Alphabetical, no wildcards"). Should sort before org.springframework.security....
2. Inline fully-qualified type (UserSecurityBeansAutoConfiguration.java:243): isStaticAssetOrAutoProbe(jakarta.servlet.http.HttpServletRequest request) uses a fully-qualified name instead of an import, inconsistent with the rest of the file/codebase style.
3. STATIC_ASSET_EXTENSIONS includes json and xml — these are content-negotiated data formats, not really "static assets" like icons/fonts. If a consuming app ever deep-links a user to a content-negotiated, auth-protected endpoint ending in .json/.xml (e.g. /report.json), the saved request would silently be dropped. Probably fine in practice (the acceptsHtml check would likely already exclude most such requests), but worth a one-line comment noting the rationale, or reconsidering whether these two extensions belong in a "static asset" set at all.

Test coverage

• RequestCacheHardeningTest covers the core repro well, but doesn't test the static-asset-extension branch directly (e.g. GET /styles.css or /script.js with Accept: text/html) or the /favicon//.well-known/ prefixes — only /apple-touch-icon.png is exercised for the probe-path branch.
• More notably: CoreBeanOverrideTest in the same package already establishes a "default present / consumer bean wins" test pattern for every other @ConditionalOnMissingBean-guarded bean in UserSecurityBeansAutoConfiguration (PasswordEncoder, SessionRegistry, RoleHierarchy, DaoAuthenticationProvider, AppUrlResolver). This PR doesn't extend that suite to cover requestCache() — there's no test proving a consumer-supplied RequestCache bean actually wins and suppresses the library default, even though the PR description calls this out as the primary escape hatch ("Consumers who want Spring Security's default behavior back just define their own RequestCache bean"). Given the existing pattern, adding a consumerRequestCacheWins-style case there would close the gap cheaply.

Docs

• CONFIG.md/MIGRATION.md aren't updated to mention the new overridable RequestCache bean. Not strictly required (the class-level Javadoc documents it), but consistent with how other overridable beans are surfaced to consumers reading the docs rather than the source.

Security

• No concerns — this is a net security improvement (strictly narrows what gets saved/replayed after login, reducing the surface for saved-request based open-redirect-style issues), and preserves the @ConditionalOnMissingBean escape hatch so it can't silently break a consumer relying on custom request-cache behavior.

Overall: solid, well-tested fix. The two "minor issues" are trivial nits; the test-coverage gap against CoreBeanOverrideTest's established pattern is the one thing I'd actually ask to close before merge.

[claude]

Review: harden RequestCache against browser auto-probes

Nice root-cause fix — hardening the RequestCache matcher (rather than just special-casing icon paths) correctly handles the whole class of "browser auto-fetch overwrites the saved deep link" bugs (touch icons, favicons, future probes), not just the one reported in prod. The dual fix (hardened cache + auto-unprotecting the probe paths) is well-reasoned, backward compatible (@ConditionalOnMissingBean, no signature changes), and the PR description/tests make the reasoning easy to follow.

Code quality

• Import ordering (UserSecurityBeansAutoConfiguration.java): import org.springframework.http.MediaType; is appended after the org.springframework.security.* imports instead of sorted alphabetically (it should sit before org.springframework.security...). CLAUDE.md calls out "Imports: Alphabetical, no wildcards" — worth fixing for consistency.
• isStaticAssetOrAutoProbe(jakarta.servlet.http.HttpServletRequest request) uses the fully-qualified type inline rather than an import; the rest of the file imports its types, so this reads as an inconsistency rather than an intentional choice.
• Class javadoc references {@link org.springframework.security.web.savedrequest.RequestCache} fully-qualified even though RequestCache is already imported — could just be {@link RequestCache}.

Minor/edge-case observations (not blockers)

• STATIC_ASSET_EXTENSIONS includes json and xml. Since the matcher already requires GET + an explicit text/html Accept header, this mostly only matters if a real page URL happens to end in .json/.xml (e.g., a plain browser navigation to /report.xml) — such a deep link would silently not be saved. Probably fine given how rare that path shape is for real pages, but worth a one-line comment noting the trade-off since it's not obvious from the code alone.
• RequestCacheHardeningTest sets @TestPropertySource(properties = {"user.security.defaultAction=deny"}), but application-test.yml already defaults user.security.defaultAction: deny. Since @TestPropertySource changes the Spring context cache key, this forces a distinct context load even though the property is redundant — could drop it to reuse the cached @SecurityTest context and shave test-suite time.

Correctness

• The matcher logic (GET AND accepts text/html explicitly AND NOT-XHR AND NOT-HTMX AND NOT-static/probe path) looks correct and matches the described bug mechanism. Treating Accept: */* as non-HTML (via setIgnoredMediaTypes(Set.of(MediaType.ALL))) is the key insight that distinguishes real navigations from auto-probes, and is tested explicitly (iconPathIsNeverSavedRegardlessOfAcceptHeader).
• Ant-pattern additions (/apple-touch-icon*, /favicon*, /.well-known/**) correctly stay within a single path segment for the first two (matches the intent) while /.well-known/** intentionally spans segments — consistent with the RequestCache exclusion set, good symmetry between the two fixes.
• No circular-dependency risk from injecting RequestCache into WebSecurityConfig via constructor — Spring resolves the auto-configuration bean before it's needed since it's plain field injection, not an ordering-sensitive @Bean method call.

Test coverage

• Good coverage: the new RequestCacheHardeningTest reproduces the exact reported bug (icon probe on the same session overwriting a previously-saved navigation) rather than just testing the matcher in isolation, which is the right level to catch regressions.
• WebSecurityAuthorizationDenyTest.AutoUnprotectedProbePaths correctly asserts via 404-vs-302 rather than needing a stub controller, consistent with the existing pattern in that test class.
• Consider (optional) a quick check that defaultAction=allow mode isn't affected/doesn't need the same treatment — probably a non-issue since allow mode already permits unlisted paths, but calling it out would make the coverage story complete.

Security

• No concerns — this is a hardening change that reduces (not increases) what gets saved/replayed, and the auto-unprotected paths are inherently public, non-sensitive probes. No new attack surface introduced (e.g., the matcher doesn't trust any request-supplied value for a decision beyond header presence checks).

Overall: solid, well-tested, well-documented fix. The findings above are minor style/nit items — nothing blocking.

[devondragon]

Addressed code-review findings (self-review pass)

M1 (Medium) — fixed. LoginSuccessService extends SavedRequestAwareAuthenticationSuccessHandler, which held its own private default HttpSessionRequestCache and was never given the injected RequestCache bean. The fix worked only because both sides were coincidentally an HttpSessionRequestCache on the same session attribute — the filter chain (save) used the bean, the success handler (read) used its own default. A consumer overriding the RequestCache bean (per the advertised @ConditionalOnMissingBean contract) would have their request saved to one store and read from another, silently breaking the post-login redirect. Now LoginSuccessService is constructor-injected with the effective RequestCache and calls super.setRequestCache(...), so save and read share one instance. Added an M1 regression test asserting the read side consults the injected cache and redirects to its saved URL.

N5/N6/N7 — addressed. Added a no-Accept-header RequestCache test (the one matcher branch not previously exercised); documented the fail-open URL-encoding assumption in isStaticAssetOrAutoProbe; tidied import grouping.

L2 / L3 — intentionally kept, documented as behavior trade-offs. Protected .json/.xml top-level navigations aren't saved (overwhelmingly API/data fetches; .pdf is deliberately still saved). POST-triggered logins aren't saved (POST-replay-as-GET was already broken). Both are acceptable and called out here for release notes.

Full suite after changes: 1005 tests, 0 failures.

[claude]

Review: harden RequestCache against browser auto-probe hijacking

Nice root-cause fix — the /apple-touch-icon.png?continue bug is a great catch, and rewiring LoginSuccessService to read from the same injected RequestCache the filter chain writes to closes a real "write to one store, read from another" trap for consumers who override the cache. The @ConditionalOnMissingBean pattern is applied consistently with the rest of UserSecurityBeansAutoConfiguration, and test coverage is solid (red-first regression test for the exact reported bug, explicit-Accept/HTMX/XHR/POST negative cases, and a unit test proving LoginSuccessService consults the injected cache rather than falling back to its own default).

Code quality / conventions

• Import order nit — UserSecurityBeansAutoConfiguration.java adds import org.springframework.http.MediaType; between org.springframework.core.session.SessionRegistryImpl and org.springframework.security.core.userdetails.UserDetailsService, breaking alphabetical order. CLAUDE.md calls out "Imports: Alphabetical, no wildcards" as a convention — worth a quick fix even though nothing enforces it at build time.
• A few fully-qualified names are used inline in the new tests instead of imports (org.junit.jupiter.api.Nested, org.mockito.Mockito.atLeastOnce(...), SecurityMockMvcRequestPostProcessors.csrf()). Minor, but inconsistent with the rest of the codebase's style.

Design considerations

• /.well-known/** auto-unprotect has no per-item opt-out. getUnprotectedURIsList() now unconditionally adds /apple-touch-icon*, /favicon*, and /.well-known/** for every consumer, mirroring the existing MFA entry-point precedent. That's reasonable for the icon/favicon cases, but /.well-known/ is occasionally used by apps for non-standard, intentionally-protected paths. Unlike the RequestCache bean (fully overridable via @ConditionalOnMissingBean), there's no lightweight way for a consumer to opt out of just this addition short of overriding the whole WebSecurityConfig/filter chain. Given this is a behavior change for existing consumers under defaultAction=deny, it'd be worth a line in the PR/changelog (and ideally CONFIG.md/MIGRATION.md) calling it out explicitly.
• Related: neither the new RequestCache bean nor the new always-unprotected paths are mentioned in CONFIG.md. Since this changes default authorization behavior (new public paths) and introduces a new overridable bean, it seems worth documenting alongside the other user.security.* bean-override hooks.

Low-severity / informational

• MediaTypeRequestMatcher's ignored-media-type check short-circuits per media type in the resolved, quality-sorted Accept list. The heuristic assumes text/html will be evaluated before any */* entry, which holds for how real browsers order their Accept header, but it's a soft assumption rather than a structural guarantee (e.g. a client sending Accept: */*, text/html with equal weights could defeat the heuristic). Not worth blocking on, just noting as a known edge in the heuristic.

Security

• No open-redirect concerns: the saved-request URL is derived from the incoming request itself (not a user-supplied parameter), so the heuristic changes only which requests get cached, not how the redirect target is constructed.
• Making /.well-known/**, /favicon*, and /apple-touch-icon* always public is consistent with their intended use (RFC 8615 metadata, browser icon probes) and reduces the attack surface previously created by having every probe hit the login redirect.

Overall this looks correct, well-tested, and backwards compatible (no public signature changes, fully overridable via bean definition). The items above are minor — mostly documentation/style — rather than blockers.

[devondragon]

Additional quality pass — least-privilege on the auto-unprotect (N4)

Since the auto-unprotect widens the permitAll surface for every consumer (who never opted in and won't audit the framework internals), I scoped the patterns to exactly the browser-probed paths rather than a broad prefix:

before	after	rationale
/apple-touch-icon*	/apple-touch-icon*.png	every Safari probe (plain, precomposed, sized, sized-precomposed) ends in .png; keeps a route like /apple-touch-icon-admin protected
/favicon*	/favicon.*	the browser auto-probe is /favicon.ico; the dot anchor keeps a route like /favicon-report protected; sized favicons are <link>-referenced static assets, not auto-probes
/.well-known/**	unchanged	RFC 8615 well-known URIs are public by design

Added negative tests asserting /apple-touch-icon-admin and /favicon-report still require authentication under deny. This also aligns the framework with the tighter *.png pattern the MagicMenu consumer already uses. The save-side RequestCache heuristic stays intentionally broader — misclassifying there only fails open (a request isn't saved), it never grants access.

Deliberately left as-is (correct trade-offs, documented for release notes)

• L2 — protected .json/.xml/.js/.css top-level navigations aren't saved. Protected routes here are HTML pages; JSON/XML are /api/** (XHR/HTMX, already excluded) or public feeds. .pdf is kept saveable so real downloads replay.
• L3 — POST-triggered logins aren't saved. Spring replays saved requests as GET anyway (body lost), so this is neutral-to-better.

Full suite: 1007 tests, 0 failures.

[claude]

Review

Nice, well-scoped fix for a genuinely nasty bug — a browser-native probe silently hijacking the post-login redirect is the kind of thing that's very hard to catch without production logs, and the reproduction/root-cause writeup in the PR description is excellent.

What it does

1. Adds a hardened, @ConditionalOnMissingBean-guarded RequestCache bean (UserSecurityBeansAutoConfiguration.requestCache()) that only saves GET requests explicitly accepting text/html, excluding XHR/HTMX/static-asset/auto-probe requests, and wires it into the filter chain.
2. Fixes LoginSuccessService to read from that same injected RequestCache (previously it silently used its own private default — a real correctness bug for anyone who overrides the cache bean).
3. Auto-unprotects /apple-touch-icon*.png, /favicon.*, and /.well-known/** in getUnprotectedURIsList(), following the existing MFA-entry-point precedent.
4. 12 new focused tests covering the exact reported scenario plus edge cases.

Strengths

• The MediaTypeRequestMatcher + ignoredMediaTypes(MediaType.ALL) trick is the correct way to distinguish a real browser navigation (Accept: text/html,...) from a bare Accept: */* probe — verified the resolution semantics (icon probes and missing-Accept requests both resolve to [*/*], which is ignored, so they correctly fail the "accepts html" check).
• The LoginSuccessService fix (reading from the injected RequestCache rather than a private default) is arguably the most important part of this PR — without it, a consumer who overrides the RequestCache bean today gets a silent redirect-target bug, independent of the icon-probe issue. Good catch.
• Ant/path-pattern anchoring is careful: /apple-touch-icon*.png and /favicon.* are anchored to their extensions specifically so lookalike routes (/apple-touch-icon-admin, /favicon-report) stay protected — and there's a test for exactly that ("least privilege") case.
• Good test coverage of the actual regression scenario (same-session icon probe overwriting a real saved request), not just unit-level matcher checks.

Minor issues / nits

Import ordering (UserSecurityBeansAutoConfiguration.java) — CLAUDE.md specifies alphabetical imports, and the rest of the codebase follows a consistent grouping (java.* → org.springframework.* alphabetized → com.digitalsanctuary.* → jakarta.* → lombok.*, see LoginSuccessService.java/WebSecurityConfig.java for the pattern). In this file:

• import org.springframework.http.MediaType; is inserted mid-way through the org.springframework.security.* block instead of before it.
• import jakarta.servlet.http.HttpServletRequest; is placed before com.digitalsanctuary.* instead of after, breaking from the convention used elsewhere in the same PR (LoginSuccessService.java).

Duplicated HTMX header constant — HtmxAwareAuthenticationEntryPoint.HX_REQUEST_HEADER (package-private, same package com.digitalsanctuary.spring.user.security) already defines "HX-Request". The new matcher hardcodes the literal string again instead of reusing it. Also worth noting a small semantic drift: HtmxAwareAuthenticationEntryPoint.isHtmxRequest only treats the header as HTMX when its value is "true", while RequestHeaderRequestMatcher("HX-Request") (single-arg) matches on mere presence of the header regardless of value. In practice htmx.js always sends "true", so this is unlikely to bite, but the two "is this HTMX?" checks in the codebase now disagree slightly.

STATIC_ASSET_EXTENSIONS includes json/xml — these aren't really "static assets" the way images/fonts/css are; a real page a user deep-links to that happens to end in .json or .xml (uncommon, but possible with some routing schemes) would never have its saved request preserved. In practice this is mostly redundant given the Accept: text/html requirement already filters these out, so low risk — just flagging the extension list is broader than the "static asset" name implies.

Test naming — the "(M1 regression)" suffix in one @DisplayName references an internal milestone tag that won't mean anything to future readers of the test report. Consider dropping it or replacing with plain language (the docstring above it already explains the "why" well).

Optional: docs — CONFIG.md isn't updated to mention the newly auto-unprotected paths. Not strictly required (the MFA auto-unprotect precedent isn't documented there either), but since this changes default permitAll surface for every consumer, a one-line mention alongside the existing WebAuthn/dev-login notes could help consumers who go looking for why these paths are suddenly reachable without auth.

Security

• Widening permitAll to /apple-touch-icon*.png, /favicon.*, /.well-known/** is safe — these are conventionally-public static assets/metadata in virtually every web app, and the patterns are deliberately anchored to avoid accidentally exposing lookalike application routes (tested).
• The RequestCache hardening is a net security/correctness improvement — no open-redirect concern since the saved request is always the URL the user actually requested, not attacker-controlled input.
• No secrets, no new attack surface introduced by the matcher logic itself.

Verdict

Solid, well-tested fix with a good root-cause writeup. Only nits above (import order, a small duplication opportunity, and a naming nit) — nothing blocking.

[claude]

Review: harden RequestCache against browser auto-probe hijacking

Solid, well-targeted fix for a real production bug, with a clear root-cause explanation and good test coverage. A few notes below, nothing blocking.

Strengths

• Correct root-cause fix: restricting the RequestCache matcher to GET + explicit text/html Accept + not-XHR/HTMX + not-static-asset/auto-probe directly addresses the mechanism described in the PR body, rather than just patching the symptom (the icon-probe overwriting the saved request).
• Consumer-overridable via @ConditionalOnMissingBean(RequestCache.class), consistent with the existing pattern for the other beans in UserSecurityBeansAutoConfiguration.
• The LoginSuccessService fix is important and easy to miss: making it read from the injected RequestCache instead of its own private default is exactly right -- without it, a consumer overriding RequestCache would have the write side and read side silently pointing at different caches. The regression test for this (onAuthenticationSuccess_readsSavedRequestFromInjectedCache) is a good catch.
• Good breadth of tests: real navigation still saved, icon probe does not overwrite it, Accept-header edge cases, HTMX/XHR/POST exclusions, and the auto-unprotect least-privilege tests (/apple-touch-icon-admin, /favicon-report stay protected).

Issues

1. shouldAllowFaviconIco does not actually test the new behavior it claims to. src/test/resources/application-test.yml:91 already lists /favicon.ico in unprotectedURIs: unprotectedURIs: /,/index.html,/favicon.ico,/css/*,.... But the new nested test class Javadoc says the tested paths are "intentionally absent" from application-test.ymls unprotectedURIs, so a 404 proves the auto-unprotect applied. That claim does not hold for /favicon.ico specifically -- the test would pass identically with or without the new /favicon.* auto-add line, since /favicon.ico was already whitelisted. Consider testing a path only covered by the new pattern (e.g. /favicon.svg or /favicon.png) to actually exercise unprotectedURIs.add("/favicon.*").

2. Import ordering breaks the projects own alphabetical convention in UserSecurityBeansAutoConfiguration.java, which CLAUDE.md calls out explicitly ("Imports: Alphabetical, no wildcards"). import org.springframework.http.MediaType; is inserted between two org.springframework.security.* imports instead of before them, and import jakarta.servlet.http.HttpServletRequest; is placed before com.digitalsanctuary.* imports, whereas other files in this repo (e.g. SessionInvalidationService, StepUpService) place jakarta.* after com.digitalsanctuary.*. Minor, but worth a quick IDE re-organize-imports pass.

3. Minor style nit: a couple of new test additions use fully-qualified inline references instead of imports (org.junit.jupiter.api.Nested in WebSecurityAuthorizationDenyTest, org.mockito.Mockito.atLeastOnce() in LoginSuccessServiceTest, SecurityMockMvcRequestPostProcessors.csrf() in RequestCacheHardeningTest). Not wrong, just inconsistent with the rest of the codebases import style.

4. Debug logging is now slightly incomplete: WebSecurityConfig (around the unprotectedURIs debug log) still logs getUnprotectedURIsArray(), the config-only list, rather than the final unprotectedURIs used in the filter chain -- so the new auto-added probe paths will not show up in the debug trace meant to reflect what is actually unprotected.

5. Scope nit: the README.md version-string bump (5.1.0 to 5.1.1) is unrelated to this security fix, and per CLAUDE.md the release process manages versioning automatically -- worth splitting into its own doc-sync commit/PR, or dropping if it is meant to land via the release process instead.

Design notes (not defects, just worth flagging for awareness)

• The RequestCache exclusion heuristic (isStaticAssetOrAutoProbe, prefix-based: path.startsWith("/favicon") / "/apple-touch-icon") is intentionally broader than the unprotectedURIs Ant patterns (suffix-anchored: /favicon.*, /apple-touch-icon*.png). This is called out in the comments and is a reasonable fail-open tradeoff, but it does mean a hypothetical real page named e.g. /favicon-report would remain protected (good) yet could never become a post-login redirect target (since it is excluded from being saved) -- an edge case, but worth being aware of if a consumer ever has such a route.
• MediaTypeRequestMatcher(MediaType.TEXT_HTML) plus setIgnoredMediaTypes(Set.of(MediaType.ALL)) correctly treats a bare Accept: */* (or a missing Accept header, which HeaderContentNegotiationStrategy resolves to */*) as "not a confirmed HTML navigation" -- good, and matches the noAcceptHeaderIsNotSaved test.

Nice fix overall -- the mechanism write-up and regression tests give good confidence this addresses the reported production issue.

[claude]

Review: harden RequestCache against browser auto-probe hijacking

Nice root-cause fix, going after the RequestCache matcher instead of just special-casing /apple-touch-icon.png is the right call, and the writeup/tests make the mechanism easy to verify. A few notes below, nothing blocking.

What it does

• New consumer-overridable RequestCache bean (UserSecurityBeansAutoConfiguration.requestCache()) that only saves GET requests explicitly accepting text/html, excluding XHR/HTMX/static-asset/well-known-probe requests.
• Wires that bean into http.requestCache(...) in WebSecurityConfig.
• Fixes a related latent bug: LoginSuccessService (which reads the saved request via SavedRequestAwareAuthenticationSuccessHandler) now takes the same injected RequestCache via constructor + setRequestCache(...), so the read side and write side are guaranteed to agree. Previously a consumer overriding RequestCache would have had it written to their custom cache but read from the handlers private default HttpSessionRequestCache, silently breaking the redirect. Good catch, and it has its own regression test (LoginSuccessServiceTest.onAuthenticationSuccess_readsSavedRequestFromInjectedCache).
• Auto-unprotects /apple-touch-icon*.png, /favicon.*, /.well-known/** so the probes dont 302 to login, mirroring the existing MFA entry-point auto-unprotect precedent.

Code quality / style

• Import order in UserSecurityBeansAutoConfiguration.java: org.springframework.http.MediaType (new import) lands after the org.springframework.security.* block instead of before it, and jakarta.servlet.http.HttpServletRequest is inserted among the org.springframework.security.web.util.matcher.* imports instead of grouped with the other jakarta./com.digitalsanctuary. imports at the end. CLAUDE.md calls for strictly alphabetical imports, worth a quick reorder.
• Minor nit: super.setRequestCache(requestCache) in the new LoginSuccessService constructor. The super. qualifier isnt needed here (no shadowing), setRequestCache(requestCache) would read the same. Purely cosmetic.

Potential subtlety (not a bug, but worth a second look)

• The RequestCaches isStaticAssetOrAutoProbe treats any path starting with /favicon or /apple-touch-icon as a probe (unanchored prefix match), while the getUnprotectedURIsList() patterns are anchored to .ico/.png extensions specifically so a route like /favicon-report or /apple-touch-icon-admin stays protected. Thats intentionally documented as a fail-open heuristic, but the practical effect is: a legitimate deep link to /apple-touch-icon-admin (still protected, still requires login) will never be saved by the RequestCache, so post-login the user lands on the default success URL instead of that page, a small UX regression for the edge case the least-privilege tests (shouldStillProtectAppleTouchIconPrefixedRoute, shouldStillProtectFaviconPrefixedRoute) were specifically added to protect. Might be worth tightening isStaticAssetOrAutoProbes prefix check to the same extension-anchored logic used for the unprotected-URI patterns, so the two exclusion sets are actually equivalent as the javadoc claims (mirrors the exclusion set).

Test coverage

• Solid: RequestCacheHardeningTest directly reproduces the reported bug (icon probe on the same session overwriting the saved navigation) and covers the individual matcher branches (Accept header, HTMX, XHR, POST). WebSecurityAuthorizationDenyTest.AutoUnprotectedProbePaths verifies both the auto-unprotect and that it doesnt over-widen (prefix-sharing routes stay protected).
• The UserAPIUnitTest.disabledWarnings() change (filtering by thread name to avoid cross-test pollution under parallel execution) looks like an unrelated flaky-test fix bundled into this PR. Reasonable to keep if it was blocking CI, but might be worth calling out explicitly in the PR description (or splitting into its own commit/PR) since its orthogonal to the RequestCache fix.

Security

• Widening permitAll to /apple-touch-icon*.png, /favicon.*, /.well-known/** is safe. These are inherently public identity/well-known files, and the anchoring keeps prefix-sharing routes protected (verified by tests).
• No new attack surface from the RequestCache matcher itself, misclassification there only affects whether a request is saved for replay, not authorization, as the in-code comment correctly notes.

Other

• No public signature changes to buildSecurityFilterChain(HttpSecurity, SessionRegistry), and @ConditionalOnMissingBean(RequestCache.class) preserves the consumer-override escape hatch, consistent with the rest of UserSecurityBeansAutoConfiguration.
• README version bump (5.1.0 -> 5.1.1) matches gradle.properties (5.1.1-SNAPSHOT), so no conflict with the dont-manually-bump-versions release policy.

Overall this is a well-scoped, well-tested fix for a real production bug. The import-order nit is quick to fix; the prefix-matching subtlety is minor and optional.