marcel/familienarchiv
1
0
Fork 0
Code Issues 111 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(auth): address PR #617 review feedback on CSRF/rate-limit implementation

Browse Source 
- Remove unreachable `&& !xsrfToken` condition from `handleFetch` guard;
  simplify the redundant `cookieParts.length > 0` check that follows it
- Add `TOO_MANY_LOGIN_ATTEMPTS` to both Error Handling sections in CLAUDE.md
  (backend and frontend) so LLMs are aware of the code without looking it up
- Add reverse-proxy IP trust and IPv6 address-cycling caveats to ADR-022
  Consequences section

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-05-19 07:41:04 +02:00
committed by marcel
parent d8520d9714
commit 96c0aa592c
3 changed files with 13 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
CLAUDE.md
View File

@@ -160,7 +160,7 @@ Input DTOs live flat in the domain package. Response types are the model entitie
→ See [CONTRIBUTING.md §Error handling](./CONTRIBUTING.md#error-handling)
**LLM reminder:** use `DomainException.notFound/forbidden/conflict/internal()` from service methods — never throw raw exceptions. When adding a new `ErrorCode`: (1) add to `ErrorCode.java`, (2) add to `ErrorCode` type in `frontend/src/lib/shared/errors.ts`, (3) add a `case` in `getErrorMessage()`, (4) add i18n keys in `messages/{de,en,es}.json`.
**LLM reminder:** use `DomainException.notFound/forbidden/conflict/internal()` from service methods — never throw raw exceptions. When adding a new `ErrorCode`: (1) add to `ErrorCode.java`, (2) add to `ErrorCode` type in `frontend/src/lib/shared/errors.ts`, (3) add a `case` in `getErrorMessage()`, (4) add i18n keys in `messages/{de,en,es}.json`. Valid error codes include: `TOO_MANY_LOGIN_ATTEMPTS` (returned by `LoginRateLimiter` as HTTP 429 when a brute-force threshold is exceeded).
### Security / Permissions
@@ -267,7 +267,7 @@ Back button pattern — use the shared `<BackButton>` component from `$lib/share
→ See [CONTRIBUTING.md §Error handling](./CONTRIBUTING.md#error-handling)
**LLM reminder:** when adding a new `ErrorCode`: (1) add to `ErrorCode.java`, (2) add to `ErrorCode` type in `frontend/src/lib/shared/errors.ts`, (3) add a `case` in `getErrorMessage()`, (4) add i18n keys in `messages/{de,en,es}.json`.
**LLM reminder:** when adding a new `ErrorCode`: (1) add to `ErrorCode.java`, (2) add to `ErrorCode` type in `frontend/src/lib/shared/errors.ts`, (3) add a `case` in `getErrorMessage()`, (4) add i18n keys in `messages/{de,en,es}.json`. Valid error codes include: `TOO_MANY_LOGIN_ATTEMPTS` (returned by `LoginRateLimiter` as HTTP 429 when a brute-force threshold is exceeded).
---

9
docs/adr/022-csrf-session-revocation-rate-limiting.md
View File

@@ -104,3 +104,12 @@ source.
because `@WebMvcTest` slices exclude `JacksonAutoConfiguration`. The response
only serialises a fixed String key (`"code"`) so naming strategy and custom
modules are irrelevant.
- IP extraction uses `HttpServletRequest.getRemoteAddr()`. In deployments behind
a reverse proxy the `X-Forwarded-For` header is not trusted — doing so would
let clients spoof their IP and trivially bypass the per-IP limit. Trusting
proxy headers requires separate work (e.g. Spring's `ForwardedHeaderFilter`
with an allowlist of trusted proxy addresses).
- IPv6 and IPv4-mapped addresses (e.g. `::ffff:1.2.3.4`) are not normalised to
a canonical form. An attacker with access to multiple IPv6 addresses could
rotate addresses to bypass the per-IP bucket. This is a known limitation of
address-based rate limiting and is acceptable for the current deployment.

5
frontend/src/hooks.server.ts
View File

@@ -131,14 +131,13 @@ export const handleFetch: HandleFetch = async ({ event, request, fetch }) => {
if (sessionId) cookieParts.push(`fa_session=${sessionId}`);
if (xsrfToken) cookieParts.push(`XSRF-TOKEN=${xsrfToken}`);
if (cookieParts.length === 0 && !xsrfToken) {
if (cookieParts.length === 0) {
return fetch(request);
}
// Clone first so the body stream is preserved on the new Request.
const cloned = request.clone();
const extraHeaders: Record<string, string> = {};
if (cookieParts.length > 0) extraHeaders['Cookie'] = cookieParts.join('; ');
const extraHeaders: Record<string, string> = { Cookie: cookieParts.join('; ') };
if (xsrfToken) extraHeaders['X-XSRF-TOKEN'] = xsrfToken;
const modified = new Request(cloned, {