• Joined on 2026-03-17
marcel commented on pull request marcel/familienarchiv#617 2026-05-18 14:11:58 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

🎨 Leonie Voss — UI/UX Designer & Accessibility Strategist

Verdict: Approved

The login page changes are clean and accessible. The rate-limited error state is handled correctly with…

marcel commented on pull request marcel/familienarchiv#617 2026-05-18 14:11:43 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

⚙️ Tobias Wendt — DevOps & Platform Engineer

Verdict: Approved

Clean addition. No new infrastructure, no new Docker services, no new environment variables. Exactly what I want to see…

marcel commented on pull request marcel/familienarchiv#617 2026-05-18 14:11:31 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

🧪 Sara Holt — QA Engineer

Verdict: ⚠️ Approved with concerns

The breadth of test updates is impressive — 40 files, every mutating MockMvc call updated with .with(csrf()). But two…

marcel commented on pull request marcel/familienarchiv#617 2026-05-18 14:11:15 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

🏛️ Markus Keller — Application Architect

Verdict: ⚠️ Approved with concerns

Architecture is sound: the three features are cohesive, correctly placed in the auth package, and the ADR…

marcel commented on pull request marcel/familienarchiv#617 2026-05-18 14:10:56 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ⚠️ Approved with concerns

Solid implementation with excellent test coverage. One blocker on code style and a handful of smaller…

marcel commented on pull request marcel/familienarchiv#617 2026-05-18 14:10:39 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

🔐 Nora "NullX" Steiner — Application Security Engineer

Verdict: ⚠️ Approved with concerns

Strong overall. The CSRF architecture is correct for a SPA, session revocation is properly…

marcel commented on pull request marcel/familienarchiv#617 2026-05-18 14:04:51 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

Review concerns addressed

marcel pushed to feat/issue-524-csrf-session-rate-limit at marcel/familienarchiv 2026-05-18 14:04:18 +02:00
a23fa4c668 fix(login): add role=alert to error divs; fix clock icon color to red
05ab8b13a0 docs(arch): update auth sequence diagram to Phase 2 (CSRF, rate limit, revocation)
1052295a6e docs(adr): add ADR-022 for CSRF, session revocation, and rate limiting
c3d1bea623 refactor(security): extract static ERROR_WRITER; update ADR ref to ADR-022
97585a9cd4 test(security): add CSRF rejection test to DocumentControllerTest
Compare 7 commits »
marcel commented on pull request marcel/familienarchiv#618 2026-05-18 13:20:44 +02:00
security(import): validate PDF magic bytes before S3 upload

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Verdict: ⚠️ Approved with concerns

Blockers

list-none removes the disclosure triangle — accessibility failure

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 13:20:26 +02:00
security(import): validate PDF magic bytes before S3 upload

🧪 Sara Holt — QA Engineer

Verdict: ⚠️ Approved with concerns

What Was Added

4 new backend tests + 1 helper method + 3 new frontend component tests. The happy path, skipped count,…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 13:20:11 +02:00
security(import): validate PDF magic bytes before S3 upload

🔒 Nora "NullX" Steiner — Application Security Engineer

Verdict: Approved

Security Assessment

CWE-434 (Unrestricted File Upload) — addressed correctly

The magic byte…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 13:19:55 +02:00
security(import): validate PDF magic bytes before S3 upload

📋 Elicit — Requirements Engineer

Verdict: ⚠️ Approved with concerns

Requirements Coverage

The core requirement (reject files that don't begin with the PDF magic bytes %PDF) is…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 13:19:40 +02:00
security(import): validate PDF magic bytes before S3 upload

🔧 Tobias Wendt — DevOps & Platform Engineer

Verdict: Approved

What I Checked

No infrastructure changes — correct This PR touches zero Docker Compose configuration, CI…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 13:19:32 +02:00
security(import): validate PDF magic bytes before S3 upload

🏛️ Markus Keller — Application Architect

Verdict: Approved

What I Checked

Module boundaries — clean All changes stay inside the importing package. SkippedFile,…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 13:19:18 +02:00
security(import): validate PDF magic bytes before S3 upload

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ⚠️ Approved with concerns

Blockers

AdminControllerTest.java — duplicate import

import java.util.List;
import
marcel commented on issue marcel/familienarchiv#529 2026-05-18 13:15:52 +02:00
security(import): validate PDF magic bytes in MassImportService before S3 upload

Implementation complete — PR #618

All acceptance criteria addressed. Two atomic commits:

e124c68cfeat(import): validate PDF magic bytes before S3 upload

  • Added SkippedFile
marcel created pull request marcel/familienarchiv#618 2026-05-18 13:15:32 +02:00
security(import): validate PDF magic bytes before S3 upload
marcel created branch worktree-feat+issue-529-pdf-magic-bytes in marcel/familienarchiv 2026-05-18 13:15:17 +02:00
marcel pushed to worktree-feat+issue-529-pdf-magic-bytes at marcel/familienarchiv 2026-05-18 13:15:17 +02:00
22589e4729 feat(admin): surface skipped file count in ImportStatusCard
e124c68cf4 feat(import): validate PDF magic bytes before S3 upload
Compare 2 commits »
marcel commented on pull request marcel/familienarchiv#617 2026-05-18 13:14:42 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Verdict: ⚠️ Approved with concerns

The rate-limited login error state is a good addition. One accessibility inconsistency between…