5.0.1: case-insensitive trustedHosts + PR review nits

[devondragon]

Folds in the still-open feedback from the #318 / #319 reviews ahead of the 5.0.1 release.

Fixes

• trustedHosts case-insensitive match (AppUrlResolver) — the one substantive open bug. A mixed-case configured entry or X-Forwarded-Host (e.g. App.Example.Com) previously failed to match app.example.com, so the CWE-640 path ignored a legitimately trusted forwarded host and fell back to the container server name. Normalises both sides to lower case (RFC 4343) + adds a test.

Review nits

• Role.java: condense the multi-line EAGER rationale to one line (CLAUDE.md style).
• UserRepositoryEntityGraphTest: fix stale class Javadoc (Role.privileges is EAGER again) and name the collection in the plain-finder test comment.
• Soften "single query" → "single round trip (bounded, typically one query)" in UserRepository Javadoc, MIGRATION.md, and the 5.0.1 CHANGELOG entry — the EntityGraph test tolerates a small statement bound across JPA providers.

Already-resolved (verified, no change needed)

Passwordless message consistency, serialVersionUID on all event classes, trailing-slash strip, X-Forwarded comma handling, null/blank privilege guard, and log sanitization (sanitizeForLog) were all already fixed in #318. The CodeQL log-injection alerts are stale (sites are sanitized) and can be dismissed.

Test Plan

• ./gradlew clean test green; new matchesAllowListedForwardedHostCaseInsensitively passes.

[Copilot]

Pull request overview

Updates the security email app-URL resolution to treat user.security.trustedHosts matching as case-insensitive (so trusted forwarded hosts are reliably honored), and folds in several documentation/comment nits ahead of the 5.0.1 release.

Changes:

• Normalize trusted-host allow-list entries and forwarded-host comparisons to lower case (RFC 4343) and add a regression test.
• Refine repository/entity documentation around LAZY/EAGER behavior and clarify the “single round trip” entity-graph semantics.
• Update 5.0.1 release notes / migration guidance to match the above.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/main/java/com/digitalsanctuary/spring/user/util/AppUrlResolver.java	Normalizes trusted-host matching to be case-insensitive while preserving existing URL construction behavior.
src/test/java/com/digitalsanctuary/spring/user/util/AppUrlResolverTest.java	Adds a case-insensitive allow-list/forwarded-host regression test.
src/main/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepository.java	Clarifies Javadoc to “single round trip (bounded, typically single query)”.
src/test/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepositoryEntityGraphTest.java	Updates class/test comments to reflect Role.privileges being EAGER again and clarifies LAZY target (User.roles).
src/main/java/com/digitalsanctuary/spring/user/persistence/model/Role.java	Condenses EAGER rationale comment for Role.privileges.
MIGRATION.md	Updates migration text to “single round trip” wording and keeps 5.0.1 privilege-fetch note accurate.
CHANGELOG.md	Updates 5.0.1 notes: “single round trip” wording and adds the trustedHosts case-insensitivity fix entry.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[claude]

Code Review — PR #320: 5.0.1: case-insensitive trustedHosts + PR review nits

Note: This PR is already merged. Feedback is retrospective and intended for future reference.

---

Overview

This PR bundles one substantive bug fix with several documentation/comment nits carried over from the #318/#319 review cycle. The scope is well-controlled and the changes are tightly focused.

---

Bug Fix: AppUrlResolver — Case-Insensitive Host Matching

Verdict: Correct and well-executed.

// Constructor — normalise allow-list at construction time (good: pay cost once)
this.trustedHosts = trustedHosts == null ? List.of()
        : trustedHosts.stream().map(s -> s.trim().toLowerCase(Locale.ROOT)).toList();

// Comparison — normalise the incoming header for comparison only
boolean useForwarded = fwdHost != null && !fwdHost.isEmpty()
        && trustedHosts.contains(stripPort(fwdHost).toLowerCase(Locale.ROOT));

• Locale.ROOT is exactly right for hostname normalisation. Using Locale.getDefault() would be a subtle bug on Turkish/Azerbaijani systems where "I".toLowerCase() → "ı", not "i". This is the correct choice per RFC 4343.
• Case is preserved in the returned URL, which is also correct — only the comparison is normalised, not the host used to construct the URL. The test asserts this explicitly ("https://TRUSTED.example.com"), which is a nice self-documenting catch.
• stripPort is called before toLowerCase — the right order, since port numbers are numeric and stripping first prevents any unlikely confusion on edge-case host strings.
• IPv6 allow-list entries ([::1]) are unaffected (hex digits a–f map identically under Locale.ROOT).

Minor Observation

The test name matchesAllowListedForwardedHostCaseInsensitively doesn't follow the should[ExpectedBehavior]When[Condition] pattern specified in CLAUDE.md. The existing tests in this file also use the looser form (honorsForwardedHostOnlyWhenAllowListed), so this is a pre-existing inconsistency rather than something introduced here — but it's worth standardising across the file in a future pass. A consistent name would be: shouldMatchForwardedHostCaseInsensitivelyWhenAllowListEntryIsMixedCase.

---

Documentation / Javadoc Nits

Verdict: All correct.

• "single query" → "single round trip (bounded, typically single query)" in UserRepository, MIGRATION.md, and CHANGELOG.md — this is a meaningful improvement. The original wording was technically wrong for JPA implementations that may issue more than one statement (e.g. Hibernate 6+ collection batch fetching), and the new phrasing is both accurate and still useful to consumers.
• Role.java comment condensation — the long multi-line block is now a single crisp line pointing to MIGRATION.md for detail. Correct per CLAUDE.md conventions ("no unnecessary comments … one short line max").
• UserRepositoryEntityGraphTest Javadoc fix — the class-level doc now correctly states that only User.roles is LAZY (not Role.privileges), matching the 5.0.1 reality. The inline test comment also names the collection being checked, improving readability.

---

Security Consideration

The original case-sensitive comparison was a defence-in-depth gap on the CWE-640 path: a legitimately trusted host configured as App.Example.Com (or received from a proxy with non-normalised casing) would fail the trustedHosts.contains() check, log a warning, and silently fall back to the container's server name for the password-reset URL. This wasn't exploitable in itself (the fallback is safe), but it broke legitimate deployments and could mask misconfiguration. The fix closes the gap correctly.

---

Summary

Area	Assessment
Bug fix correctness	✅ Correct — Locale.ROOT, right normalisation order, case preserved in output
Test coverage	✅ New test covers mixed-case allow-list and mixed-case header
Javadoc accuracy	✅ "single round trip" is more honest than "single query"
Comment style	✅ Condensed to CLAUDE.md standard
Test naming convention	⚠️ Minor: doesn't follow should…When… pattern (pre-existing issue in the file)

Clean PR — no blocking issues.