Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions AUDIT.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ Table **`audit_events`** (entity `AuditEventEntity`):
| `actorId` | UUID | From `Actor.userId()` when the interceptor can resolve the current user |
| `actorRole` | String | `Role.name()` when present |
| `principalName` | String | `request.getUserPrincipal().getName()` |
| `httpMethod` | String | e.g. GET, POST |
| `requestPath` | String | `request.getRequestURI()` |
| `queryString` | String | Raw query string; **sanitized before persist** (see below) |
| `handler` | String | For `HandlerMethod`: `SimpleClassName#methodName`; otherwise simple class name |
Expand Down Expand Up @@ -74,7 +73,7 @@ For each request the interceptor:

1. Tries `SecurityActorAdapter.currentUser()` → on failure (e.g. not authenticated), continues with `actorId` / `actorRole` unset rather than failing the HTTP request.
2. Sets identity fields: `actorId`, `actorRole`, `principalName`.
3. Sets request metadata: method, URI, query string, resolved handler name.
3. Sets request metadata: URI, query string, resolved handler name.
4. Sets `responseStatus`, `errorType` (from `ex`).
5. Sets **`caseId`** via `extractCaseId(request)`:
- Reads `HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE`.
Expand Down Expand Up @@ -176,7 +175,7 @@ Goal: avoid storing secrets in `queryString`.

- **Route:** `GET /ui/audit` → template `audit/list.html`.
- **Navigation:** Link in `fragments/header.html` (“Audit”).
- Table shows: time, actor role/id, method, path + query, status, caseId, handler, error type. Empty state copy mentions access as well as “no rows.”
- Table shows: time, actor role/id, event, description, status, caseId, details. Empty state copy mentions access as well as “no rows.”

**Security (HTTP layer):** `SecurityConfig` requires authentication for any request not on the permit-all list; there is **no extra** `@PreAuthorize` on audit endpoints—**fine-grained rules are entirely in `AuditService.listEvents`**.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ public AuditEventDTO toDTO(AuditEventEntity entity) {
.actorId(entity.getActorId())
.actorRole(entity.getActorRole())
.principalName(entity.getPrincipalName())
.httpMethod(entity.getHttpMethod())
.requestPath(entity.getRequestPath())
.queryString(entity.getQueryString())
.handler(entity.getHandler())
.responseStatus(entity.getResponseStatus())
.errorType(entity.getErrorType())
.caseId(entity.getCaseId())
.statusChange(entity.getStatusChange())
.eventName(entity.getEventName())
.description(entity.getDescription())
.clientIp(entity.getClientIp())
.userAgent(entity.getUserAgent())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,13 @@
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;
import org.springframework.web.servlet.mvc.method.annotation.SseEmitter;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

import java.io.IOException;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -28,8 +33,10 @@
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.stream.Collectors;

@Slf4j
@Service
@RequiredArgsConstructor
public class AuditService {
Expand All @@ -39,6 +46,8 @@ public class AuditService {
private final CaseRepository caseRepository;
private final ObjectMapper objectMapper = new ObjectMapper();

private final List<SseEmitter> emitters = new CopyOnWriteArrayList<>();

private static final String REDACTED = "[REDACTED]";
private static final Set<String> SENSITIVE_KEYS = Set.of(
"password",
Expand Down Expand Up @@ -69,6 +78,66 @@ public void record(AuditEventEntity event) {
}
event.setQueryString(sanitizeAuditPayload(event.getQueryString()));
auditEventRepository.save(event);

TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
@Override
public void afterCommit() {
try {
broadcast(event);
} catch (Throwable t) {
log.warn("Failed to broadcast audit event {} after commit: {}", event.getId(), t.getMessage(), t);
}
}
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

private void broadcast(AuditEventEntity event) {
if (emitters.isEmpty()) return;

AuditEventDTO dto = auditEventMapper.toDTO(event);
List<SseEmitter> failedEmitters = new ArrayList<>();

for (SseEmitter emitter : emitters) {
try {
emitter.send(SseEmitter.event()
.name("audit-event")
.data(dto));
} catch (IOException | IllegalStateException e) {
failedEmitters.add(emitter);
}
}
emitters.removeAll(failedEmitters);
}

public SseEmitter createEmitter() {
SseEmitter emitter = new SseEmitter(30 * 60 * 1000L); // 30 minute timeout
emitters.add(emitter);

emitter.onCompletion(() -> {
log.debug("SSE emitter completed");
emitters.remove(emitter);
});
emitter.onTimeout(() -> {
log.debug("SSE emitter timed out");
emitters.remove(emitter);
});
emitter.onError((ex) -> {
log.debug("SSE emitter error: {}", ex.getMessage());
emitters.remove(emitter);
});

// Send an initial event to confirm connection
try {
emitter.send(SseEmitter.event()
.name("connected")
.data("Audit stream connected"));
log.debug("SSE emitter created and initial event sent");
} catch (IOException e) {
log.error("Failed to send initial SSE event", e);
emitters.remove(emitter);
}

return emitter;
}

private String sanitizeAuditPayload(String payload) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,17 @@ private boolean isPatient(Actor actor) {

private void recordStatusChange(Actor actor, UUID caseId, CaseStatus from, CaseStatus to) {
String statusChange = (from != null ? from.name() : "NEW") + " -> " + to.name();
String eventName = from == null ? "CASE_CREATED" : "STATUS_CHANGED";
String description = from == null ? "Case was created" : "Status changed from " + from + " to " + to;

AuditEventEntity event = AuditEventEntity.builder()
.caseId(caseId)
.statusChange(statusChange)
.eventName(eventName)
.description(description)
.actorId(actor != null ? actor.userId() : null)
.actorRole(actor != null && actor.role() != null ? actor.role().name() : null)
.occurredAt(Instant.now())
.build();
auditService.record(event);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,20 @@ public void afterCompletion(int status) {
auditService.record(AuditEventEntity.builder()
.caseId(caseEntity.getId())
.statusChange(statusChange)
.eventName("STATUS_CHANGED")
.description("Status changed to COMMUNICATION after document upload")
.actorId(actor.userId())
.actorRole(actor.role() != null ? actor.role().name() : null)
.occurredAt(Instant.now())
.build());
} else {
auditService.record(AuditEventEntity.builder()
.caseId(caseEntity.getId())
.eventName("DOCUMENT_UPLOADED")
.description("Document uploaded: " + entity.getFileName())
.actorId(actor.userId())
.actorRole(actor.role() != null ? actor.role().name() : null)
.occurredAt(Instant.now())
.build());
}
Comment on lines +118 to 127

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

DOCUMENT_UPLOADED audit entry includes raw filename — consider length/PII handling.

entity.getFileName() comes directly from the uploaded MultipartFile.getOriginalFilename() and is written verbatim into description. Since:

  • Filenames can legitimately contain PII or sensitive content (e.g., patient-1234-ssn.pdf), and
  • The queryString sanitization path in AuditService.record() (per AUDIT.md) only scrubs the queryString field, leaving description untouched,

this can leak sensitive data into the audit table and onto the audit UI. Consider either truncating/escaping the filename, or documenting that audit description is a non-sanitized field so operators can reason about it.

Additionally, no upper bound on description length means a pathologically long filename could exceed the default column length once a DDL is generated with a length constraint. A @Column(length = ...) on AuditEventEntity.description with matching truncation here would make behavior predictable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/application/service/DocumentService.java`
around lines 118 - 127, The audit entry writes the raw uploaded filename into
AuditEventEntity.description via the DocumentService call to
auditService.record(...) using entity.getFileName(), which risks leaking PII and
unbounded length; update DocumentService to sanitize/redact and bound the
filename before building the AuditEventEntity (e.g., replace sensitive segments
or mask characters and/or truncate to a safe max length) and ensure
AuditEventEntity.description has a matching column length constraint so the
stored description cannot exceed DB limits; reference the builder usage around
AuditEventEntity.builder() and the entity.getFileName() call when applying these
changes.

return documentMapper.toDTO(saved);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.authentication.HttpStatusEntryPoint;
import org.springframework.http.HttpStatus;

@Configuration
@EnableWebSecurity
Expand Down Expand Up @@ -52,6 +55,12 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http, CustomOAuth2Us
.invalidateHttpSession(true)
.deleteCookies("JSESSIONID")
)
.exceptionHandling(ex -> ex
.defaultAuthenticationEntryPointFor(
new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED),
request -> request.getRequestURI().startsWith("/api/")
)
)
.userDetailsService(localUserDetailsService);
return http.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public class AuditEventEntity {
private String actorRole;
private String principalName;

private String httpMethod;
private String requestPath;
private String queryString;
private String handler;
Expand All @@ -51,6 +50,8 @@ public class AuditEventEntity {
private UUID caseId;

private String statusChange;
private String eventName;
private String description;

private String clientIp;
private String userAgent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ public AuditInterceptor(AuditService auditService, SecurityActorAdapter security

@Override
public void afterCompletion(HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex) {
String method = request.getMethod();
boolean isMutation = "POST".equalsIgnoreCase(method) ||
"PUT".equalsIgnoreCase(method) ||
"DELETE".equalsIgnoreCase(method) ||
"PATCH".equalsIgnoreCase(method);

// Only record mutations or errors to reduce noise
if (!isMutation && ex == null) {
return;
}
Comment on lines +31 to +40

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Non-mutating requests that fail without throwing will silently skip audit.

afterCompletion receives ex only for exceptions that propagate out of the handler. Failures handled by @ExceptionHandler/@ControllerAdvice (e.g., AccessDeniedException, AuthenticationException, validation errors mapped to 4xx) typically arrive here with ex == null and a non-2xx response.getStatus(). With the new guard, a GET that is rejected with 401/403/500 via a controller advice will not be audited, which weakens audit coverage for exactly the reads security auditors usually care about.

Consider also auditing non-mutating requests when the response status indicates an error:

Proposed fix
-        // Only record mutations or errors to reduce noise
-        if (!isMutation && ex == null) {
-            return;
-        }
+        // Only record mutations or errors to reduce noise
+        int status = response != null ? response.getStatus() : 0;
+        boolean isError = ex != null || status >= 400;
+        if (!isMutation && !isError) {
+            return;
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/infrastructure/web/AuditInterceptor.java`
around lines 31 - 40, The current guard in afterCompletion uses isMutation and
ex to skip auditing non-mutations, which misses non-mutating requests that
failed without throwing; update the condition in
AuditInterceptor.afterCompletion to also consider HTTP response status by
checking HttpServletResponse.getStatus() (e.g., treat status >= 400 as an error)
so that when ex == null but response.getStatus() >= 400 you still proceed with
audit logging; refer to the isMutation boolean, the ex parameter and
response.getStatus() when modifying the early-return logic.


Actor actor = null;
try {
actor = securityActorAdapter.currentUser();
Expand All @@ -43,10 +54,11 @@ public void afterCompletion(HttpServletRequest request, HttpServletResponse resp
}
event.setPrincipalName(request.getUserPrincipal() != null ? request.getUserPrincipal().getName() : null);

event.setHttpMethod(request.getMethod());
event.setRequestPath(request.getRequestURI());
event.setQueryString(request.getQueryString());
event.setHandler(handlerName(handler));
event.setEventName("WEB_ACTION");
event.setDescription(method + " " + request.getRequestURI());

event.setResponseStatus(response != null ? response.getStatus() : null);
event.setErrorType(ex != null ? ex.getClass().getSimpleName() : null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public class AuditEventDTO {
private String actorRole;
private String principalName;

private String httpMethod;
private String requestPath;
private String queryString;
private String handler;
Expand All @@ -32,6 +31,8 @@ public class AuditEventDTO {
private UUID caseId;

private String statusChange;
private String eventName;
private String description;

private String clientIp;
private String userAgent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.format.annotation.DateTimeFormat;
import org.example.projektarendehantering.common.Actor;
import org.example.projektarendehantering.common.Role;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.servlet.mvc.method.annotation.SseEmitter;

import java.time.Instant;
import java.util.UUID;
Expand Down Expand Up @@ -42,4 +46,13 @@ public ResponseEntity<Page<AuditEventDTO>> list(

return ResponseEntity.ok(auditService.listEvents(securityActorAdapter.currentUser(), from, to, caseId, pageable));
}

@GetMapping(value = "/stream", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
public SseEmitter stream() {
Actor actor = securityActorAdapter.currentUser();
if (actor.role() != Role.MANAGER) {
throw new org.example.projektarendehantering.common.NotAuthorizedException("Only managers can view the audit stream");
}
return auditService.createEmitter();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.server.ResponseStatusException;

import org.springframework.http.converter.HttpMessageNotReadableException;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.context.request.async.AsyncRequestNotUsableException;
import org.springframework.web.servlet.resource.NoResourceFoundException;
import java.util.Map;

@Slf4j
Expand Down Expand Up @@ -100,6 +104,39 @@ public Object handleResponseStatusException(ResponseStatusException e, HttpServl
return "error";
}

@ExceptionHandler(NoResourceFoundException.class)
@ResponseStatus(HttpStatus.NOT_FOUND)
public void handleNotFound(NoResourceFoundException e) {
}

@ExceptionHandler(AsyncRequestNotUsableException.class)
public void handleAsyncDisconnect(AsyncRequestNotUsableException e) {
log.debug("SSE client disconnected: {}", e.getMessage());
}

@ExceptionHandler({HttpMessageNotReadableException.class, MethodArgumentNotValidException.class})
public Object handleBadRequestException(Exception e, HttpServletRequest request, HttpServletResponse response, Model model) {
HttpStatus status = HttpStatus.BAD_REQUEST;
String message = "Invalid request payload";
if (e instanceof MethodArgumentNotValidException) {
message = "Validation failed";
}

if (isRestRequest(request)) {
return ResponseEntity.status(status)
.body(Map.of(
"errorCode", "BAD_REQUEST",
"message", message,
"status", status.value()
));
}
response.setStatus(status.value());
model.addAttribute("status", status.value() + " " + status.getReasonPhrase());
model.addAttribute("message", message);
model.addAttribute("errorCode", "BAD_REQUEST");
return "error";
}

@ExceptionHandler(Exception.class)
public Object handleGeneralException(Exception e, HttpServletRequest request, HttpServletResponse response, Model model) {
log.error("Unhandled exception for request {}: {}", request.getRequestURI(), e.getMessage(), e);
Expand Down
Loading