fix(gdpr): address final PR review feedback

- Add JavaDoc warning in GdprDeletionService about contributor
  transaction safety (external API calls could cause partial deletion)
- Consolidate ConsentAuditService to use shared UserUtils.getClientIP()
- Fix session creation in GdprAPI audit logging (use getSession(false))
- Add rate limiting documentation to export and delete endpoints
- Add defensive parsing in FileAuditLogQueryService for unescaped pipes

Author: devondragon

src/main/java/com/digitalsanctuary/spring/user/api/GdprAPI.java

@@ -77,6 +77,11 @@ public class GdprAPI {
      *   <li>Data from registered GdprDataContributors</li>
      * </ul>
      *
+     * <p><b>Rate Limiting:</b> This endpoint performs resource-intensive operations
+     * (file I/O, JSON parsing, data aggregation). Consider implementing rate limiting
+     * at the infrastructure level (e.g., API gateway, Spring Security rate limiter,
+     * or reverse proxy) to prevent abuse.
+     *
      * @param userDetails the authenticated user
      * @param request the HTTP request
      * @return the complete data export as JSON
@@ -117,6 +122,10 @@ public ResponseEntity<JSONResponse> exportUserData(@AuthenticationPrincipal DSUs
      * <p>If configured, exports user data before deletion and includes
      * it in the response. After deletion, the user is logged out.
      *
+     * <p><b>Rate Limiting:</b> This endpoint performs destructive, resource-intensive
+     * operations. Consider implementing rate limiting at the infrastructure level
+     * to prevent abuse or accidental repeated deletion attempts.
+     *
      * @param userDetails the authenticated user
      * @param request the HTTP request
      * @return deletion result, optionally including exported data
@@ -306,7 +315,7 @@ private void logAuditEvent(String action, String status, String message, User us
         AuditEvent event = AuditEvent.builder()
                 .source(this)
                 .user(user)
-                .sessionId(request.getSession().getId())
+                .sessionId(request.getSession(false) != null ? request.getSession(false).getId() : null)
                 .ipAddress(UserUtils.getClientIP(request))
                 .userAgent(request.getHeader("User-Agent"))
                 .action(action)

src/main/java/com/digitalsanctuary/spring/user/audit/FileAuditLogQueryService.java

@@ -159,6 +159,11 @@ private Path getLogFilePath() {
     /**
      * Parses a single line from the audit log file.
      *
+     * <p><b>Note:</b> This parser assumes the audit log writer properly escapes
+     * pipe characters in message content. If audit messages contain unescaped pipes,
+     * parsing may be corrupted. Consider migrating to a structured format (JSON lines)
+     * for production deployments with untrusted input.
+     *
      * @param line the line to parse
      * @return the parsed AuditEventDTO, or null if parsing fails
      */
@@ -169,10 +174,27 @@ private AuditEventDTO parseLine(String line) {
 
         String[] parts = line.split("\\|", -1); // -1 to keep trailing empty strings
         if (parts.length < 10) {
-            log.debug("FileAuditLogQueryService.parseLine: Invalid line format, expected 10 fields: {}", line);
+            log.debug("FileAuditLogQueryService.parseLine: Invalid line format, expected 10 fields but got {}: {}",
+                    parts.length, line);
             return null;
         }
 
+        // Defensive: If more than 10 fields exist due to unescaped pipes in message,
+        // join the extra parts back into the message field
+        if (parts.length > 10) {
+            log.debug("FileAuditLogQueryService.parseLine: Line has {} fields (expected 10), " +
+                    "likely due to unescaped pipes in message content", parts.length);
+            // Join parts[7] through parts[parts.length-3] as the message
+            StringBuilder messageBuilder = new StringBuilder(parts[7]);
+            for (int i = 8; i < parts.length - 2; i++) {
+                messageBuilder.append("|").append(parts[i]);
+            }
+            parts[7] = messageBuilder.toString();
+            // Shift the last two fields (userAgent and extraData) to their expected positions
+            parts[8] = parts[parts.length - 2];
+            parts[9] = parts[parts.length - 1];
+        }
+
         try {
             return AuditEventDTO.builder()
                     .timestamp(parseTimestamp(parts[0]))

src/main/java/com/digitalsanctuary/spring/user/gdpr/ConsentAuditService.java

@@ -13,6 +13,7 @@
 import com.digitalsanctuary.spring.user.audit.AuditLogQueryService;
 import com.digitalsanctuary.spring.user.event.ConsentChangedEvent;
 import com.digitalsanctuary.spring.user.persistence.model.User;
+import com.digitalsanctuary.spring.user.util.UserUtils;
 import jakarta.servlet.http.HttpServletRequest;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -393,14 +394,7 @@ private ConsentRecord parseConsentRecord(AuditEventDTO event) {
      * Gets the client IP address from the request.
      */
     private String getClientIP(HttpServletRequest request) {
-        if (request == null) {
-            return null;
-        }
-        String xForwardedFor = request.getHeader("X-Forwarded-For");
-        if (xForwardedFor != null && !xForwardedFor.isEmpty()) {
-            return xForwardedFor.split(",")[0].trim();
-        }
-        return request.getRemoteAddr();
+        return UserUtils.getClientIP(request);
     }
 
     /**

src/main/java/com/digitalsanctuary/spring/user/gdpr/GdprDeletionService.java

@@ -178,6 +178,13 @@ protected DeletionResult executeUserDeletion(User user, GdprExportDTO exportedDa
 
     /**
      * Notifies all GdprDataContributors to prepare for deletion.
+     * <p>
+     * <b>IMPORTANT:</b> Contributors MUST only delete data within the same transactional context
+     * (i.e., database records in the same database). Avoid calling external APIs or services
+     * that could succeed while the main transaction fails, leading to partial deletion.
+     * <p>
+     * If external data needs cleanup, consider implementing async event listeners for
+     * {@link com.digitalsanctuary.spring.user.event.UserDeletedEvent} instead.
      */
     private void prepareContributorsForDeletion(User user) {
         if (dataContributors == null || dataContributors.isEmpty()) {