Merge with Python Services - #24
Conversation
… external ML service
… recognition data, including faces, embeddings, and associated metadata.
…ecognition improvements
| @RequestParam("file") MultipartFile file) { | ||
|
|
||
| try { | ||
| log.info("Call record analysis requested: {}", file.getOriginalFilename()); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
In general, to fix log injection you must sanitize or validate all user-controlled values before logging them. For plain-text logs, the main goal is to remove or neutralize characters that can break log structure (newlines, carriage returns, tabs, other control characters) or otherwise create confusing entries. You can do this either by strict validation (only allow safe characters and log nothing or a placeholder otherwise) or by sanitizing (replacing unsafe characters with safe alternatives like spaces).
For this specific case, the best fix with minimal functional impact is to sanitize the filename before logging it. We can introduce a small private helper method in CallAnalysisController that strips out newline (\n), carriage return (\r), and other ISO control characters from the string (or replaces them with a space), and then use this method in the logging call. This preserves the information content of the filename for debugging while preventing an attacker from injecting additional log lines. The change is local to CallAnalysisController.java: we add the helper method inside the class and change line 47 to log sanitizeForLog(file.getOriginalFilename()) instead of the raw value. No new imports are required, since we can use String.replace and String.replaceAll (or chars().filter) from the standard library.
| @@ -34,6 +34,21 @@ | ||
| private final CallAnalysisService callAnalysisService; | ||
|
|
||
| /** | ||
| * Sanitize a string for safe logging by removing control characters | ||
| * that could be used for log injection (such as newlines). | ||
| */ | ||
| private String sanitizeForLog(String value) { | ||
| if (value == null) { | ||
| return null; | ||
| } | ||
| // Remove CR, LF and other ISO control characters | ||
| return value | ||
| .replace("\r", "") | ||
| .replace("\n", "") | ||
| .replaceAll("\\p{Cntrl}", ""); | ||
| } | ||
|
|
||
| /** | ||
| * Analyze a single call record PDF. | ||
| * | ||
| * @param file PDF file containing call records | ||
| @@ -44,7 +59,7 @@ | ||
| @RequestParam("file") MultipartFile file) { | ||
|
|
||
| try { | ||
| log.info("Call record analysis requested: {}", file.getOriginalFilename()); | ||
| log.info("Call record analysis requested: {}", sanitizeForLog(file.getOriginalFilename())); | ||
|
|
||
| // Validate file | ||
| if (file.isEmpty()) { |
| @RequestParam(value = "risk_level", required = false, defaultValue = "medium") String riskLevel) { | ||
|
|
||
| try { | ||
| log.info("Criminal registration requested: {} ({})", name, nic); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
In general, to fix log injection you should sanitize or validate any user-provided data before logging it. Common mitigations are: (1) enforce a strict pattern for expected fields (e.g., only letters, spaces, and a few punctuation characters in a name), or (2) remove or neutralize control characters and line breaks (e.g., replace \r and \n with spaces) before logging.
For this specific case, the best fix that preserves existing functionality is to sanitize the name (and, for completeness, also the nic parameter that is logged in the same statement) before logging. We can introduce a small private helper method within FacialRecognitionController that strips carriage return and newline characters from any string, and then use this helper in the log call. This keeps the logged information essentially the same for normal users (names and NICs rarely contain newlines) while preventing attackers from injecting additional log lines.
Concretely:
- Add a private method
sanitizeForLog(String input)insideFacialRecognitionController(anywhere in the class, e.g., near other helpers likegetCurrentUserIdif present). - This method should return
nullif the input isnull, otherwise return a version of the string with\rand\nremoved (or replaced with a space), e.g.input.replaceAll("[\r\n]", ""). - Update the
log.infostatement inregisterCriminalto logsanitizeForLog(name)andsanitizeForLog(nic)instead of the raw values. - No new imports are required; we can use standard
Stringmethods.
| @@ -81,6 +81,19 @@ | ||
| } | ||
|
|
||
| /** | ||
| * Sanitizes user-provided strings before logging to prevent log injection. | ||
| * | ||
| * @param input the original string, possibly containing control characters | ||
| * @return a string with newline characters removed, or null if input is null | ||
| */ | ||
| private String sanitizeForLog(String input) { | ||
| if (input == null) { | ||
| return null; | ||
| } | ||
| return input.replaceAll("[\\r\\n]", ""); | ||
| } | ||
|
|
||
| /** | ||
| * Register a new criminal with their photo for facial recognition. | ||
| * Requires authentication. | ||
| * | ||
| @@ -100,7 +113,9 @@ | ||
| @RequestParam(value = "risk_level", required = false, defaultValue = "medium") String riskLevel) { | ||
|
|
||
| try { | ||
| log.info("Criminal registration requested: {} ({})", name, nic); | ||
| log.info("Criminal registration requested: {} ({})", | ||
| sanitizeForLog(name), | ||
| sanitizeForLog(nic)); | ||
|
|
||
| // Validate photo | ||
| if (photo.isEmpty()) { |
| @RequestParam(value = "risk_level", required = false, defaultValue = "medium") String riskLevel) { | ||
|
|
||
| try { | ||
| log.info("Criminal registration requested: {} ({})", name, nic); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
In general, to prevent log injection when logging user-controlled data, you either (1) validate inputs to a restricted character set before use, or (2) sanitize them specifically for the logging context, most importantly removing or neutralizing newline and carriage-return characters (and optionally other control characters). This ensures that a single log event cannot be split into multiple lines or visually altered by crafted input.
For this specific controller method in FacialRecognitionController.java, the minimal, behavior-preserving fix is to sanitize the name and nic values only for the purpose of logging, without changing what is stored or forwarded to the ML service. We can do this by creating sanitized local variables right before the log call, replacing \r and \n with spaces (or another safe character), and using those sanitized versions in the log.info call. This requires no new imports and doesn’t change the function’s external behavior.
Concretely:
- In
registerCriminal(...), before thelog.info("Criminal registration requested: {} ({})", name, nic);line, defineString safeName = name == null ? null : name.replace('\n', ' ').replace('\r', ' ');and similarlyString safeNic = nic == null ? null : nic.replace('\n', ' ').replace('\r', ' ');. - Change the logging statement to use
safeNameandsafeNicinstead ofnameandnic. - Leave all other logic, including the values passed to
facialRecognitionService.registerCriminal(...), unchanged.
| @@ -100,7 +100,9 @@ | ||
| @RequestParam(value = "risk_level", required = false, defaultValue = "medium") String riskLevel) { | ||
|
|
||
| try { | ||
| log.info("Criminal registration requested: {} ({})", name, nic); | ||
| String safeName = name == null ? null : name.replace('\n', ' ').replace('\r', ' '); | ||
| String safeNic = nic == null ? null : nic.replace('\n', ' ').replace('\r', ' '); | ||
| log.info("Criminal registration requested: {} ({})", safeName, safeNic); | ||
|
|
||
| // Validate photo | ||
| if (photo.isEmpty()) { |
| } else { | ||
| throw new Exception("Failed to retrieve analysis results"); | ||
| public JsonNode analyzeCallRecord(MultipartFile file) { | ||
| log.info("Forwarding call record analysis to ML service: {}", file.getOriginalFilename()); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
In general, to fix log injection you should sanitize any user-controlled data before including it in log messages. For plain-text logs, a common approach is to remove or replace newline characters (\r, \n) and optionally other non-printable characters, so an attacker cannot split or visually manipulate log entries.
For this specific case, the best minimal-impact fix is to sanitize the filename returned by file.getOriginalFilename() before logging it. We can do this inline without changing behavior elsewhere: obtain the original filename, replace any \r and \n characters with a safe replacement (e.g. a space or an underscore), and then log the sanitized version. This preserves the information content (the name is still recognizable) while preventing line breaks and simple log forgeries. No new imports are needed; we can use String.replace or replaceAll from the standard library.
Concretely, in src/main/java/com/crimeLink/analyzer/service/CallAnalysisService.java, around line 48, change the logging statement from using file.getOriginalFilename() directly to something like:
String originalFilename = file.getOriginalFilename();
String safeFilename = originalFilename == null ? "unknown" : originalFilename.replace('\n', '_').replace('\r', '_');
log.info("Forwarding call record analysis to ML service: {}", safeFilename);This remains fully compatible with the existing functionality while mitigating log injection risk.
| @@ -45,7 +45,11 @@ | ||
| * @return JSON response with analysis results | ||
| */ | ||
| public JsonNode analyzeCallRecord(MultipartFile file) { | ||
| log.info("Forwarding call record analysis to ML service: {}", file.getOriginalFilename()); | ||
| String originalFilename = file.getOriginalFilename(); | ||
| String safeFilename = originalFilename == null | ||
| ? "unknown" | ||
| : originalFilename.replace('\n', '_').replace('\r', '_'); | ||
| log.info("Forwarding call record analysis to ML service: {}", safeFilename); | ||
|
|
||
| String url = callAnalysisServiceUrl + "/analyze"; | ||
|
|
| */ | ||
| public JsonNode registerCriminal(MultipartFile photo, String criminalId, String name, | ||
| String nic, String riskLevel) { | ||
| log.info("Forwarding criminal registration to ML service: {} ({})", name, nic); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
In general, to fix log injection you should sanitize or validate any user-provided strings before including them in log messages. A simple and effective approach for text logs is to strip or replace newline (\n) and carriage return (\r) characters (and optionally other control characters) from user input before logging.
For this specific case, the best minimal fix is to sanitize the name and nic values right before they are logged in FacialRecognitionService.registerCriminal. We can introduce a small private helper method inside FacialRecognitionService that removes \r and \n characters from any string, and then use this method when calling log.info. This keeps existing behavior for all other uses of name and nic (e.g., sending them to the ML service) unchanged, while ensuring that the log sink only receives sanitized values.
Concretely:
- In
src/main/java/com/crimeLink/analyzer/service/FacialRecognitionService.java, add aprivate String sanitizeForLog(String value)method near the top of the class (after the constructor or before method declarations). - Change the logging line in
registerCriminalfrom loggingnameandnicdirectly to loggingsanitizeForLog(name)andsanitizeForLog(nic). - No external dependencies are required, as we can rely on simple
String.replaceoperations.
No changes are needed in FacialRecognitionController.java, because the risk is at the log sink in the service.
| @@ -39,6 +39,18 @@ | ||
| } | ||
|
|
||
| /** | ||
| * Sanitize a string value before logging to prevent log injection. | ||
| * Currently strips carriage return and newline characters. | ||
| */ | ||
| private String sanitizeForLog(String value) { | ||
| if (value == null) { | ||
| return null; | ||
| } | ||
| return value.replace('\n', ' ') | ||
| .replace('\r', ' '); | ||
| } | ||
|
|
||
| /** | ||
| * Analyze a suspect image for facial recognition matches. | ||
| * Forwards the request to Python ML service and returns the response. | ||
| * | ||
| @@ -113,7 +125,8 @@ | ||
| */ | ||
| public JsonNode registerCriminal(MultipartFile photo, String criminalId, String name, | ||
| String nic, String riskLevel) { | ||
| log.info("Forwarding criminal registration to ML service: {} ({})", name, nic); | ||
| log.info("Forwarding criminal registration to ML service: {} ({})", | ||
| sanitizeForLog(name), sanitizeForLog(nic)); | ||
|
|
||
| String url = facialRecognitionServiceUrl + "/register"; | ||
|
|
| */ | ||
| public JsonNode registerCriminal(MultipartFile photo, String criminalId, String name, | ||
| String nic, String riskLevel) { | ||
| log.info("Forwarding criminal registration to ML service: {} ({})", name, nic); |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
In general, to fix log injection issues, any user-controlled data written to logs should be validated and/or sanitized prior to logging, particularly removing or replacing newline and other control characters that could visually break or forge log records. You can either enforce a strict input format (e.g., only alphanumerics and specific separators) or normalize the string by replacing disallowed characters with safe substitutes.
For this specific case, the best minimal change that preserves existing behavior is to sanitize the nic value right before logging, without altering what is sent to the ML service. We can do this by creating a local, sanitized version of nic (e.g., replace all \r and \n with spaces) and using that in the log.info calls, while leaving the original nic value untouched for business logic. This avoids changing imports or interfaces and keeps the rest of the code intact.
Concretely:
- In
FacialRecognitionService.registerCriminal(...)around line 116, introduce a local variableString safeNic = nic == null ? null : nic.replace('\n', ' ').replace('\r', ' ');and logsafeNicinstead ofnic. - Optionally, the same pattern could be used in the controller for its own log line, but CodeQL’s flagged sink is in the service; per instructions we will only touch the shown snippets and keep behavior otherwise unchanged.
No extra methods or classes are required; this uses only core String methods, so no new imports are needed.
| @@ -113,7 +113,8 @@ | ||
| */ | ||
| public JsonNode registerCriminal(MultipartFile photo, String criminalId, String name, | ||
| String nic, String riskLevel) { | ||
| log.info("Forwarding criminal registration to ML service: {} ({})", name, nic); | ||
| String safeNic = nic == null ? null : nic.replace('\n', ' ').replace('\r', ' '); | ||
| log.info("Forwarding criminal registration to ML service: {} ({})", name, safeNic); | ||
|
|
||
| String url = facialRecognitionServiceUrl + "/register"; | ||
|
|
There was a problem hiding this comment.
Pull request overview
This pull request integrates Python ML microservices for Call Analysis and Facial Recognition functionality into the Spring Boot monolith. The changes implement a hybrid architecture where Spring Boot acts as an API gateway, handling authentication and routing while delegating ML inference to Python FastAPI services.
Changes:
- Added new service integration layer (FacialRecognitionService, CallAnalysisService) to communicate with Python ML microservices via RestTemplate
- Refactored CallAnalysisController to remove database persistence and forward requests directly to Python service, removing previous async analysis pattern
- Added new FacialRecognitionController with endpoints for image analysis, criminal registration, and history retrieval
- Removed database persistence layer for call analysis (CallAnalysisRepository, CallAnalysisRecord entity, CallAnalysisResultDTO)
- Added database schema for facial recognition feature with tables for criminals, suspect photos, and audit logs
- Configured RestTemplate bean with appropriate timeouts (10s connect, 30s read) for ML service communication
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| FacialRecognitionService.java | New service layer acting as proxy to Python facial recognition ML service, handling image analysis and criminal registration |
| CallAnalysisService.java | Refactored to proxy Python call analysis service, replaced async database-backed approach with synchronous forwarding |
| FacialRecognitionController.java | New REST API controller providing endpoints for facial recognition operations with basic file validation |
| CallAnalysisController.java | Refactored from async database-backed analysis to synchronous ML service forwarding, added batch analysis support |
| RestTemplateConfig.java | Configuration bean for RestTemplate with ML service-appropriate timeouts |
| SecurityConfig.java | Added public access to ML service health check endpoints |
| facial_recognition_tables.sql | Comprehensive database schema for criminals, photos, face embeddings, and audit logs |
| CallAnalysisRepository.java | Deleted - no longer needed as analysis is handled by Python service |
| CallAnalysisRecord.java | Deleted - entity removed as persistence moved to Python service |
| CallAnalysisResultDTO.java | Deleted - replaced with generic JsonNode responses from ML service |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Validate files | ||
| if (files.length == 0) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "No files provided")); | ||
| } | ||
|
|
||
| return ResponseEntity.ok(result); | ||
| for (MultipartFile file : files) { | ||
| if (file.isEmpty()) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "One or more files are empty")); | ||
| } | ||
| String contentType = file.getContentType(); | ||
| if (contentType == null || !contentType.equals("application/pdf")) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "Invalid file type: " + file.getOriginalFilename() + ". Only PDF files are allowed.")); | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent validation between single and batch endpoints: The single file analyze endpoint validates content type using 'equals("application/pdf")' while validating file emptiness, but the batch endpoint has the same validation duplicated for each file. Consider extracting this validation logic into a private helper method to reduce duplication and ensure consistency.
| CONSTRAINT only_one_primary_per_criminal | ||
| EXCLUDE USING gist (criminal_id WITH =) | ||
| WHERE (is_primary = true) | ||
| ); |
There was a problem hiding this comment.
Database constraint syntax issue: The EXCLUDE constraint 'only_one_primary_per_criminal' uses 'EXCLUDE USING gist' which requires the 'btree_gist' extension to be enabled in PostgreSQL. This extension is not automatically available and must be explicitly created using 'CREATE EXTENSION btree_gist;' before this table can be created. Consider either adding this extension creation to the schema file, documenting this requirement, or using an alternative approach like a unique partial index or a trigger to enforce the constraint.
| }); | ||
|
|
||
| HttpEntity<MultiValueMap<String, Object>> requestEntity = new HttpEntity<>(body, headers); | ||
| @Value("${ml.call-analysis.url:http://localhost:5001}") |
There was a problem hiding this comment.
Configuration property key mismatch: The application.properties file uses 'python.call-analysis.url' and 'python.facial-recognition.url', but the services are using '@value' annotations with 'ml.call-analysis.url' and 'ml.facial-recognition.url'. This means the services will always fall back to the default values (localhost:5001 and localhost:5002) and won't pick up the configured values from application.properties. Either update the application.properties keys to 'ml.call-analysis.url' and 'ml.facial-recognition.url', or update the @value annotations in the services to match the existing keys.
| @Value("${ml.call-analysis.url:http://localhost:5001}") | |
| @Value("${python.call-analysis.url:http://localhost:5001}") |
| private final RestTemplate restTemplate; | ||
| private final ObjectMapper objectMapper; | ||
|
|
||
| @Value("${ml.facial-recognition.url:http://localhost:5002}") |
There was a problem hiding this comment.
Configuration property key mismatch: The application.properties file uses 'python.facial-recognition.url', but this service is using '@value' annotation with 'ml.facial-recognition.url'. This means the service will always fall back to the default value (localhost:5002) and won't pick up the configured value from application.properties. Either update the application.properties key to 'ml.facial-recognition.url', or update the @value annotation to match the existing 'python.facial-recognition.url' key.
| @Value("${ml.facial-recognition.url:http://localhost:5002}") | |
| @Value("${python.facial-recognition.url:http://localhost:5002}") |
| @RestController | ||
| @RequestMapping("/api/investigator/call-analysis") | ||
| @PreAuthorize("hasAnyRole('Investigator', 'OIC', 'Admin')") | ||
| @RequestMapping("/api/call-analysis") | ||
| @RequiredArgsConstructor | ||
| @Slf4j | ||
| public class CallAnalysisController { | ||
|
|
||
| @Autowired | ||
| private CallAnalysisService callAnalysisService; | ||
| private final CallAnalysisService callAnalysisService; | ||
|
|
||
| /** | ||
| * Upload PDF file for call record analysis | ||
| * Analyze a single call record PDF. | ||
| * | ||
| * @param file PDF file containing call records | ||
| * @return Analysis ID for tracking results | ||
| * @return Analysis results with crime indicators, entity graph, etc. | ||
| */ | ||
| @PostMapping("/upload") | ||
| public ResponseEntity<Map<String, Object>> uploadCallRecords(@RequestParam("file") MultipartFile file) { | ||
| @PostMapping("/analyze") | ||
| public ResponseEntity<?> analyzeCallRecord( | ||
| @RequestParam("file") MultipartFile file) { | ||
|
|
||
| try { | ||
| log.info("Call record analysis requested: {}", file.getOriginalFilename()); | ||
|
|
||
| // Validate file | ||
| if (file.isEmpty()) { | ||
| Map<String, Object> error = new HashMap<>(); | ||
| error.put("error", "No file provided"); | ||
| return ResponseEntity.badRequest().body(error); | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "No file provided")); | ||
| } | ||
|
|
||
| if (!file.getOriginalFilename().toLowerCase().endsWith(".pdf")) { | ||
| Map<String, Object> error = new HashMap<>(); | ||
| error.put("error", "Only PDF files are supported"); | ||
| return ResponseEntity.badRequest().body(error); | ||
| String contentType = file.getContentType(); | ||
| if (contentType == null || !contentType.equals("application/pdf")) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "Invalid file type. Please upload a PDF file.")); | ||
| } | ||
|
|
||
| // Send to Python service | ||
| String analysisId = callAnalysisService.analyzeCallRecords(file); | ||
|
|
||
| Map<String, Object> response = new HashMap<>(); | ||
| response.put("analysis_id", analysisId); | ||
| response.put("status", "processing"); | ||
| response.put("message", "Analysis started successfully"); | ||
|
|
||
| return ResponseEntity.ok(response); | ||
| // Forward to ML service | ||
| JsonNode result = callAnalysisService.analyzeCallRecord(file); | ||
|
|
||
| return ResponseEntity.ok(result); | ||
|
|
||
| } catch (Exception e) { | ||
| Map<String, Object> error = new HashMap<>(); | ||
| error.put("error", "Failed to process file: " + e.getMessage()); | ||
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(error); | ||
| } catch (RuntimeException e) { | ||
| log.error("Call record analysis failed: {}", e.getMessage()); | ||
| return ResponseEntity.internalServerError() | ||
| .body(Map.of("error", e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get analysis results by ID | ||
| * @param analysisId Analysis ID returned from upload | ||
| * @return Complete analysis results including network graph and criminal matches | ||
| * Analyze multiple call record PDFs in batch. | ||
| * | ||
| * @param files Array of PDF files containing call records | ||
| * @return Batch analysis results | ||
| */ | ||
| @GetMapping("/results/{analysisId}") | ||
| public ResponseEntity<CallAnalysisResultDTO> getAnalysisResults(@PathVariable String analysisId) { | ||
| @PostMapping("/analyze/batch") | ||
| public ResponseEntity<?> analyzeBatch( | ||
| @RequestParam("files") MultipartFile[] files) { | ||
|
|
||
| try { | ||
| CallAnalysisResultDTO result = callAnalysisService.getAnalysisResults(analysisId); | ||
| log.info("Batch call analysis requested: {} files", files.length); | ||
|
|
||
| if (result == null) { | ||
| return ResponseEntity.notFound().build(); | ||
| // Validate files | ||
| if (files.length == 0) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "No files provided")); | ||
| } | ||
|
|
||
| return ResponseEntity.ok(result); | ||
| for (MultipartFile file : files) { | ||
| if (file.isEmpty()) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "One or more files are empty")); | ||
| } | ||
| String contentType = file.getContentType(); | ||
| if (contentType == null || !contentType.equals("application/pdf")) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "Invalid file type: " + file.getOriginalFilename() + ". Only PDF files are allowed.")); | ||
| } | ||
| } | ||
|
|
||
| } catch (Exception e) { | ||
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build(); | ||
| } | ||
| } | ||
| // Forward to ML service | ||
| List<MultipartFile> fileList = Arrays.asList(files); | ||
| JsonNode result = callAnalysisService.analyzeBatch(fileList); | ||
|
|
||
| return ResponseEntity.ok(result); | ||
|
|
||
| /** | ||
| * Get all analysis history | ||
| * @return List of all analyses | ||
| */ | ||
| @GetMapping("/history") | ||
| public ResponseEntity<Map<String, Object>> getAnalysisHistory() { | ||
| try { | ||
| Map<String, Object> history = callAnalysisService.getAllAnalyses(); | ||
| return ResponseEntity.ok(history); | ||
| } catch (Exception e) { | ||
| Map<String, Object> error = new HashMap<>(); | ||
| error.put("error", "Failed to retrieve history: " + e.getMessage()); | ||
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(error); | ||
| } catch (RuntimeException e) { | ||
| log.error("Batch call analysis failed: {}", e.getMessage()); | ||
| return ResponseEntity.internalServerError() | ||
| .body(Map.of("error", e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check Python service health | ||
| * @return Health status of call analysis service | ||
| * Health check endpoint for the call analysis ML service. | ||
| * Public endpoint for monitoring. | ||
| * | ||
| * @return Health status | ||
| */ | ||
| @GetMapping("/health") | ||
| public ResponseEntity<Map<String, Object>> checkServiceHealth() { | ||
| try { | ||
| Map<String, Object> health = callAnalysisService.checkPythonServiceHealth(); | ||
| public ResponseEntity<?> checkHealth() { | ||
| JsonNode health = callAnalysisService.checkHealth(); | ||
|
|
||
| String status = health.has("status") ? health.get("status").asText() : "unknown"; | ||
|
|
||
| if ("healthy".equals(status) || "ok".equals(status)) { | ||
| return ResponseEntity.ok(health); | ||
| } catch (Exception e) { | ||
| Map<String, Object> error = new HashMap<>(); | ||
| error.put("status", "unhealthy"); | ||
| error.put("error", e.getMessage()); | ||
| return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).body(error); | ||
| } else { | ||
| return ResponseEntity.status(503).body(health); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing authentication/authorization checks: Unlike the old implementation which used '@PreAuthorize' to restrict access to specific roles (Investigator, OIC, Admin), the new controller endpoints have no role-based access control. The '/api/call-analysis/analyze' and '/api/call-analysis/analyze/batch' endpoints can be accessed by any authenticated user. Consider adding appropriate authorization checks to ensure only authorized roles can upload and analyze sensitive call records.
| @RestController | ||
| @RequestMapping("/api/facial") | ||
| @RequiredArgsConstructor | ||
| @Slf4j | ||
| public class FacialRecognitionController { | ||
|
|
||
| private final FacialRecognitionService facialRecognitionService; | ||
|
|
||
| /** | ||
| * Analyze a suspect image for facial recognition matches. | ||
| * Requires authentication - user ID is extracted from JWT token. | ||
| * | ||
| * @param image The image file to analyze (multipart) | ||
| * @param threshold Similarity threshold (0-100), default 45 | ||
| * @param caseId Optional case ID for linking to investigation | ||
| * @return Analysis results with matched criminals | ||
| */ | ||
| @PostMapping("/analyze") | ||
| public ResponseEntity<?> analyzeImage( | ||
| @RequestParam("image") MultipartFile image, | ||
| @RequestParam(value = "threshold", required = false, defaultValue = "45") Float threshold, | ||
| @RequestParam(value = "case_id", required = false) String caseId) { | ||
|
|
||
| try { | ||
| // Get authenticated user ID from security context | ||
| String userId = getCurrentUserId(); | ||
| log.info("Facial recognition analysis requested by user: {}", userId); | ||
|
|
||
| // Validate image | ||
| if (image.isEmpty()) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "No image provided")); | ||
| } | ||
|
|
||
| // Validate file type | ||
| String contentType = image.getContentType(); | ||
| if (contentType == null || !contentType.startsWith("image/")) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "Invalid file type. Please upload an image.")); | ||
| } | ||
|
|
||
| // Forward to ML service | ||
| JsonNode result = facialRecognitionService.analyzeImage(image, threshold, userId, caseId); | ||
|
|
||
| return ResponseEntity.ok(result); | ||
|
|
||
| } catch (RuntimeException e) { | ||
| log.error("Facial recognition analysis failed: {}", e.getMessage()); | ||
| return ResponseEntity.internalServerError() | ||
| .body(Map.of("error", e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Register a new criminal with their photo for facial recognition. | ||
| * Requires authentication. | ||
| * | ||
| * @param photo Photo of the criminal | ||
| * @param criminalId Optional existing criminal ID to link | ||
| * @param name Criminal's name | ||
| * @param nic National ID Card number | ||
| * @param riskLevel Risk level (high, medium, low) | ||
| * @return Registration result with criminal details | ||
| */ | ||
| @PostMapping("/register") | ||
| public ResponseEntity<?> registerCriminal( | ||
| @RequestParam("photo") MultipartFile photo, | ||
| @RequestParam(value = "criminal_id", required = false) String criminalId, | ||
| @RequestParam("name") String name, | ||
| @RequestParam("nic") String nic, | ||
| @RequestParam(value = "risk_level", required = false, defaultValue = "medium") String riskLevel) { | ||
|
|
||
| try { | ||
| log.info("Criminal registration requested: {} ({})", name, nic); | ||
|
|
||
| // Validate photo | ||
| if (photo.isEmpty()) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "No photo provided")); | ||
| } | ||
|
|
||
| String contentType = photo.getContentType(); | ||
| if (contentType == null || !contentType.startsWith("image/")) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "Invalid file type. Please upload an image.")); | ||
| } | ||
|
|
||
| // Forward to ML service | ||
| JsonNode result = facialRecognitionService.registerCriminal( | ||
| photo, criminalId, name, nic, riskLevel); | ||
|
|
||
| return ResponseEntity.ok(result); | ||
|
|
||
| } catch (RuntimeException e) { | ||
| log.error("Criminal registration failed: {}", e.getMessage()); | ||
| return ResponseEntity.internalServerError() | ||
| .body(Map.of("error", e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get list of all registered criminals with face embeddings. | ||
| * | ||
| * @return List of criminals | ||
| */ | ||
| @GetMapping("/criminals") | ||
| public ResponseEntity<?> getCriminals() { | ||
| try { | ||
| JsonNode result = facialRecognitionService.getCriminals(); | ||
| return ResponseEntity.ok(result); | ||
| } catch (RuntimeException e) { | ||
| log.error("Failed to fetch criminals: {}", e.getMessage()); | ||
| return ResponseEntity.internalServerError() | ||
| .body(Map.of("error", e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get facial recognition history/audit logs. | ||
| * | ||
| * @param limit Maximum number of records (default 50) | ||
| * @return Recognition history | ||
| */ | ||
| @GetMapping("/history") | ||
| public ResponseEntity<?> getRecognitionHistory( | ||
| @RequestParam(value = "limit", required = false, defaultValue = "50") Integer limit) { | ||
| try { | ||
| JsonNode result = facialRecognitionService.getRecognitionHistory(limit); | ||
| return ResponseEntity.ok(result); | ||
| } catch (RuntimeException e) { | ||
| log.error("Failed to fetch history: {}", e.getMessage()); | ||
| return ResponseEntity.internalServerError() | ||
| .body(Map.of("error", e.getMessage())); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Health check endpoint for the facial recognition ML service. | ||
| * Public endpoint for monitoring. | ||
| * | ||
| * @return Health status | ||
| */ | ||
| @GetMapping("/health") | ||
| public ResponseEntity<?> checkHealth() { | ||
| JsonNode health = facialRecognitionService.checkHealth(); | ||
|
|
||
| String status = health.has("status") ? health.get("status").asText() : "unknown"; | ||
|
|
||
| if ("healthy".equals(status)) { | ||
| return ResponseEntity.ok(health); | ||
| } else { | ||
| return ResponseEntity.status(503).body(health); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Extract current user ID from security context. | ||
| */ | ||
| private String getCurrentUserId() { | ||
| Authentication auth = SecurityContextHolder.getContext().getAuthentication(); | ||
| if (auth != null && auth.isAuthenticated() && !"anonymousUser".equals(auth.getPrincipal())) { | ||
| return auth.getName(); | ||
| } | ||
| return "unknown"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing authentication/authorization checks: The facial recognition endpoints have no role-based access control. Sensitive operations like analyzing images for criminal matches and registering new criminals can be performed by any authenticated user. Consider adding appropriate authorization checks using '@PreAuthorize' or SecurityConfig rules to ensure only authorized roles (e.g., Investigator, OIC, Admin) can access these endpoints.
| @PostMapping("/analyze") | ||
| public ResponseEntity<?> analyzeImage( | ||
| @RequestParam("image") MultipartFile image, | ||
| @RequestParam(value = "threshold", required = false, defaultValue = "45") Float threshold, | ||
| @RequestParam(value = "case_id", required = false) String caseId) { | ||
|
|
||
| try { | ||
| // Get authenticated user ID from security context | ||
| String userId = getCurrentUserId(); | ||
| log.info("Facial recognition analysis requested by user: {}", userId); | ||
|
|
||
| // Validate image | ||
| if (image.isEmpty()) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "No image provided")); | ||
| } | ||
|
|
||
| // Validate file type | ||
| String contentType = image.getContentType(); | ||
| if (contentType == null || !contentType.startsWith("image/")) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "Invalid file type. Please upload an image.")); | ||
| } |
There was a problem hiding this comment.
Missing file size validation: The controller accepts image files without validating their size, which could lead to memory issues or denial of service if large files are uploaded. Consider adding a maximum file size check (e.g., using '@RequestParam' with size limits or manual validation) before processing the file.
| @PostMapping("/analyze") | ||
| public ResponseEntity<?> analyzeCallRecord( | ||
| @RequestParam("file") MultipartFile file) { | ||
|
|
||
| try { | ||
| log.info("Call record analysis requested: {}", file.getOriginalFilename()); | ||
|
|
||
| // Validate file | ||
| if (file.isEmpty()) { | ||
| Map<String, Object> error = new HashMap<>(); | ||
| error.put("error", "No file provided"); | ||
| return ResponseEntity.badRequest().body(error); | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "No file provided")); | ||
| } | ||
|
|
||
| if (!file.getOriginalFilename().toLowerCase().endsWith(".pdf")) { | ||
| Map<String, Object> error = new HashMap<>(); | ||
| error.put("error", "Only PDF files are supported"); | ||
| return ResponseEntity.badRequest().body(error); | ||
| String contentType = file.getContentType(); | ||
| if (contentType == null || !contentType.equals("application/pdf")) { | ||
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "Invalid file type. Please upload a PDF file.")); | ||
| } |
There was a problem hiding this comment.
Missing file size validation: The controller accepts PDF files without validating their size, which could lead to memory issues or denial of service if large files are uploaded. Consider adding a maximum file size check before processing the files.
| return ResponseEntity.badRequest() | ||
| .body(Map.of("error", "Invalid file type. Please upload an image.")); | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing input validation for required parameters: The registerCriminal method accepts 'name' and 'nic' as required parameters but doesn't validate that they are not null or empty before forwarding to the ML service. If null or empty values are passed, they will only be caught by the Python service, leading to unnecessary network calls and unclear error messages. Consider adding null/empty checks and returning appropriate error responses.
| // Validate required text fields | |
| if (name == null || name.trim().isEmpty()) { | |
| return ResponseEntity.badRequest() | |
| .body(Map.of("error", "Name is required")); | |
| } | |
| if (nic == null || nic.trim().isEmpty()) { | |
| return ResponseEntity.badRequest() | |
| .body(Map.of("error", "NIC is required")); | |
| } |
| body.add("image", new ByteArrayResource(image.getBytes()) { | ||
| @Override | ||
| public String getFilename() { | ||
| return image.getOriginalFilename(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Potential resource leak: The MultipartFile.getBytes() method is called without handling potential IOException during the ByteArrayResource creation. If an IOException occurs while reading the file bytes, the exception is caught but the file resource may not be properly cleaned up. Consider wrapping the file processing in a try-with-resources block or ensuring proper cleanup in the exception handler.
Merge with Python Services ( Call + Face )