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

fix(auth): logout invalidates session before audit (CWE-613)

Browse Source 
Reorder AuthSessionController.logout so HttpSession.invalidate runs before
AuthService.logout, and wrap the audit call in try/catch so an exception (e.g.
the user was deleted between login and logout, making the audit-time
findByEmail throw) cannot leave the session row alive in spring_session.
The user's intent — "log me out" — is honoured even when audit fails.
Addresses PR #612 / Nora B2.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-05-17 22:38:57 +02:00
parent 17b29edd14
commit ea65611690
2 changed files with 25 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
backend/src/main/java/org/raddatz/familienarchiv/auth/AuthSessionController.java
View File

@@ -4,6 +4,7 @@ import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.raddatz.familienarchiv.user.AppUser;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.Authentication;
@@ -18,6 +19,7 @@ import org.springframework.web.bind.annotation.*;
@RestController
@RequestMapping("/api/auth")
@RequiredArgsConstructor
@Slf4j
public class AuthSessionController {
private final AuthService authService;
@@ -53,17 +55,26 @@ public class AuthSessionController {
@PostMapping("/logout")
public ResponseEntity<Void> logout(Authentication authentication, HttpServletRequest httpRequest) {
String email = authentication.getName();
String ip = resolveClientIp(httpRequest);
String ua = resolveUserAgent(httpRequest);
authService.logout(authentication.getName(), ip, ua);
// CWE-613 defense: invalidate the session first — that is the contract the user
// is relying on when they click "Log out." Audit is best-effort and must not
// bubble up: if the user record was deleted while the session was live, the
// audit lookup throws, but the session row in spring_session must still die.
HttpSession session = httpRequest.getSession(false);
if (session != null) {
session.invalidate();
}
SecurityContextHolder.clearContext();
try {
authService.logout(email, ip, ua);
} catch (Exception ex) {
log.warn("Audit logout failed for {}; session was already invalidated", email, ex);
}
return ResponseEntity.noContent().build();
}

12
backend/src/test/java/org/raddatz/familienarchiv/auth/AuthSessionControllerTest.java
View File

@@ -129,4 +129,16 @@ class AuthSessionControllerTest {
mockMvc.perform(post("/api/auth/logout"))
.andExpect(status().isUnauthorized());
}
@Test
void logout_returns_204_even_when_audit_throws() throws Exception {
// CWE-613 defense: the session MUST be invalidated even if the audit lookup
// explodes (e.g. user deleted between login and logout). Audit is best-effort.
doThrow(new RuntimeException("audit DB down"))
.when(authService).logout(anyString(), anyString(), anyString());
mockMvc.perform(post("/api/auth/logout")
.with(user("ghost@test.de")))
.andExpect(status().isNoContent());
}
}