feat(jans-config-api): resolved audit api fix#13230
Conversation
Signed-off-by: imran <imranishaq7071@gmail.com>
📝 WalkthroughWalkthroughAdds new rate-limiting and external logger control schemas to the OpenAPI spec, and extends the audit log endpoint to accept ISO‑8601 date-time formats in addition to the legacy dd-MM-yyyy format. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java (1)
236-269:⚠️ Potential issue | 🔴 CriticalCritical: Date validation exceptions are swallowed — invalid dates silently return unfiltered results.
isValidlogEntry(line 259) callsvalidateDate, which callsthrowBadRequestExceptionon invalid input. However, this exception is caught by thecatch (Exception ex)on line 264, which logs the error and returns the unfiltered log entries. The user never sees the 400 response — they get a 200 with all entries instead.Move validation before the loop (and outside this try-catch), and parse
startDate/endDateonce, passing the resultingLocalDatevalues into the filtering logic.Proposed structural fix
private List<String> filterLogByDate(List<String> logEntries, String startDate, String endDate) { ... if (logEntries == null || logEntries.isEmpty() || (StringUtils.isBlank(startDate) && StringUtils.isBlank(endDate))) { return logEntries; } + + String datePattern = (StringUtils.isNotBlank(getAuditDateFormat()) ? getAuditDateFormat() + : AUDIT_FILE_DATE_FORMAT); + DateTimeFormatter formatter = DateTimeFormatter.ofPattern(datePattern); + + // Validate and parse dates once, outside the try-catch that swallows exceptions + validateDate(startDate, endDate, formatter); + LocalDate startLocalDate = StringUtils.isNotBlank(startDate) ? parseUserDate(startDate, formatter) : null; + LocalDate endLocalDate = StringUtils.isNotBlank(endDate) ? parseUserDate(endDate, formatter) : null; + List<String> filteredLogEntries = new ArrayList<>(); try { - String datePattern = (StringUtils.isNotBlank(getAuditDateFormat()) ? getAuditDateFormat() - : AUDIT_FILE_DATE_FORMAT); - DateTimeFormatter formatter = DateTimeFormatter.ofPattern(datePattern); - for (int i = 0; i < logEntries.size(); i++) { String line = logEntries.get(i); if (StringUtils.isNotBlank(line) && line.length() > datePattern.length()) { String timestampPart = line.substring(0, datePattern.length()); LocalDate logEntryLocalDate = getDate(timestampPart, formatter); - if (isValidlogEntry(formatter, startDate, endDate, logEntryLocalDate)) { + if (isInRange(logEntryLocalDate, startLocalDate, endLocalDate)) { filteredLogEntries.add(line); } } } } catch (Exception ex) { log.error("Error while filtering log file ...", ex); return logEntries; } return filteredLogEntries; }This also eliminates the O(N) repeated parsing of
startDate/endDate(currently 4parseUserDatecalls per log entry).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java` around lines 236 - 269, The current filterLogByDate swallows validation exceptions because validateDate/throwBadRequestException are invoked indirectly inside isValidlogEntry during the per-line loop; move user-date validation and parsing out of the loop and before the try-catch so bad user input produces the intended 400 instead of returning unfiltered results. Specifically, in filterLogByDate: validate and parse startDate and endDate once (using existing validateDate/parseUserDate or getDate/DateTimeFormatter) to LocalDate variables before iterating, remove repeated parse calls inside isValidlogEntry, and change isValidlogEntry (or add an overload) to accept the pre-parsed LocalDate values (or null) so the loop only calls getDate for the log entry timestamp and compares against the pre-parsed dates; ensure the try-catch no longer wraps the validation so exceptions from validateDate propagate to the caller.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java`:
- Around line 52-57: The three private DateTimeFormatter constants
(ISO_OFFSET_DATE_TIME, ISO_LOCAL_DATE_TIME, ISO_LOCAL_DATE) are redundant
aliases and should be removed; update parseUserDate to reference
DateTimeFormatter.ISO_OFFSET_DATE_TIME, DateTimeFormatter.ISO_LOCAL_DATE_TIME
and DateTimeFormatter.ISO_LOCAL_DATE directly (replace uses of the private
constants), then delete the now-unused fields (and clean up any unused imports).
This keeps parseUserDate functionality identical while removing indirection.
- Around line 401-411: The code duplicates parsing when fallbackFormatter is the
same as the legacy AUDIT_FILE_DATE_FORMAT; in AuditLogResource, before doing the
final legacy parse (creating DateTimeFormatter legacy =
DateTimeFormatter.ofPattern(AUDIT_FILE_DATE_FORMAT) and calling
LocalDate.parse), check whether fallbackFormatter is null or not equal to that
legacy formatter (e.g., compare fallbackFormatter.equals(legacy) or compare
patterns) and only perform the legacy parse when they differ, so you skip the
redundant parse attempt when fallbackFormatter already represents
AUDIT_FILE_DATE_FORMAT.
---
Outside diff comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java`:
- Around line 236-269: The current filterLogByDate swallows validation
exceptions because validateDate/throwBadRequestException are invoked indirectly
inside isValidlogEntry during the per-line loop; move user-date validation and
parsing out of the loop and before the try-catch so bad user input produces the
intended 400 instead of returning unfiltered results. Specifically, in
filterLogByDate: validate and parse startDate and endDate once (using existing
validateDate/parseUserDate or getDate/DateTimeFormatter) to LocalDate variables
before iterating, remove repeated parse calls inside isValidlogEntry, and change
isValidlogEntry (or add an overload) to accept the pre-parsed LocalDate values
(or null) so the loop only calls getDate for the log entry timestamp and
compares against the pre-parsed dates; ensure the try-catch no longer wraps the
validation so exceptions from validateDate propagate to the caller.
jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java
Outdated
Show resolved
Hide resolved
jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java
Outdated
Show resolved
Hide resolved
|
Signed-off-by: imran <imranishaq7071@gmail.com>
…nssenProject/jans into jans-config-api-audit-api-fix
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java (1)
247-256:⚠️ Potential issue | 🟠 MajorDate validation and parsing repeated O(N) times — pre-compute before the loop
isValidlogEntrycallsvalidateDate(which itself callsparseUserDatetwice) and then callsparseUserDatetwice more for the actual comparisons — fourparseUserDateinvocations per log line, each trying up to five parse strategies with exception-handling overhead. BecausestartDateandendDateare constant across all iterations, this work should be done once before the loop.♻️ Proposed refactor
Parse and validate the boundary dates once in
filterLogByDate, then pass the resolvedLocalDateobjects into the filtering predicate:DateTimeFormatter formatter = DateTimeFormatter.ofPattern(datePattern); + // Parse and validate boundary dates once — they are invariant across iterations + LocalDate startLocalDate = null; + LocalDate endLocalDate = null; + if (StringUtils.isNotBlank(startDate) || StringUtils.isNotBlank(endDate)) { + validateDate(startDate, endDate, formatter); // throws if invalid + if (StringUtils.isNotBlank(startDate)) { + startLocalDate = parseUserDate(startDate, formatter); + } + if (StringUtils.isNotBlank(endDate)) { + endLocalDate = parseUserDate(endDate, formatter); + } + } + for (int i = 0; i < logEntries.size(); i++) { String line = logEntries.get(i); if (StringUtils.isNotBlank(line) && line.length() > datePattern.length()) { String timestampPart = line.substring(0, datePattern.length()); LocalDate logEntryLocalDate = getDate(timestampPart, formatter); - if (isValidlogEntry(formatter, startDate, endDate, logEntryLocalDate)) { + if (isInDateRange(startLocalDate, endLocalDate, logEntryLocalDate)) { filteredLogEntries.add(line); } } }
isValidlogEntrycan then be simplified to a pure range-checkisInDateRange(LocalDate start, LocalDate end, LocalDate entry)with no parsing or validation.Also applies to: 301-339
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java` around lines 247 - 256, Pre-compute and validate the boundary dates once in filterLogByDate instead of repeatedly inside isValidlogEntry: call validateDate/parseUserDate for startDate and endDate once (using the same formatter) to produce resolved LocalDate objects, then change isValidlogEntry into a pure range-check method (e.g., isInDateRange(LocalDate parsedStart, LocalDate parsedEnd, LocalDate entryDate)) that only checks entryDate against those precomputed boundaries; keep getDate(formatter) per log line to obtain entryDate and add to filteredLogEntries when isInDateRange returns true, removing all repeated parseUserDate/validateDate calls inside the loop and simplifying methods validateDate/parseUserDate usage accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java`:
- Around line 92-93: The `@Schema.description` is being used for the query
parameters start_date and end_date in AuditLogResource (method parameters
startDate and endDate) which sets the schema model description instead of the
parameter description; change the annotations to use `@Parameter`(description =
"...") on the startDate and endDate parameters (and remove/move the description
out of `@Schema`), mirroring how the other params are annotated, and update the
method Javadoc `@param` lines that document startDate and endDate to list both
accepted formats (dd-MM-yyyy and ISO-8601) so the API docs and generated OpenAPI
spec show the parameter-level descriptions correctly.
- Around line 402-407: The equality check legacy.equals(fallbackFormatter) is
invalid because DateTimeFormatter doesn't override equals; change the guard to
detect when fallbackFormatter already represents the legacy pattern (or is null)
before attempting the duplicate parse: e.g., create legacy =
DateTimeFormatter.ofPattern(AUDIT_FILE_DATE_FORMAT) as before, then replace the
equals check with something like if (fallbackFormatter != null &&
fallbackFormatter.toString().contains(AUDIT_FILE_DATE_FORMAT)) { throw new
DateTimeParseException(...); } (or alternatively check fallbackFormatter ==
null) so you don't unconditionally call LocalDate.parse(trimmed, legacy). Ensure
you reference AUDIT_FILE_DATE_FORMAT, fallbackFormatter, legacy, and the
LocalDate.parse usage when making the change.
---
Outside diff comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java`:
- Around line 247-256: Pre-compute and validate the boundary dates once in
filterLogByDate instead of repeatedly inside isValidlogEntry: call
validateDate/parseUserDate for startDate and endDate once (using the same
formatter) to produce resolved LocalDate objects, then change isValidlogEntry
into a pure range-check method (e.g., isInDateRange(LocalDate parsedStart,
LocalDate parsedEnd, LocalDate entryDate)) that only checks entryDate against
those precomputed boundaries; keep getDate(formatter) per log line to obtain
entryDate and add to filteredLogEntries when isInDateRange returns true,
removing all repeated parseUserDate/validateDate calls inside the loop and
simplifying methods validateDate/parseUserDate usage accordingly.
jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java
Outdated
Show resolved
Hide resolved
jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java
Outdated
Show resolved
Hide resolved
Signed-off-by: imran <imranishaq7071@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java (1)
229-262:⚠️ Potential issue | 🟠 MajorValidation exceptions are silently swallowed by the broad
catch (Exception ex)at line 257, causing requests with malformed or invalid dates to return HTTP 200 instead of HTTP 400.
validateDate()callsthrowBadRequestException()(BaseResource.java line 130), which throws aBadRequestException. This exception propagates up throughisValidlogEntry()and is caught by thecatch (Exception ex)block, which logs an error and returns all log entries unchanged. Callers receive HTTP 200 regardless of whetherstartDate/endDateare malformed or whetherstartDate > endDate(the validation at lines 292–294). That validation is dead code from the API consumer's perspective.Additionally, there is unnecessary redundant date parsing in the loop.
isValidlogEntrycallsvalidateDate, which callsparseUserDatetwice (forstartDateandendDate).isValidlogEntrythen callsparseUserDateagain for both dates. With up to five sequential formatter attempts per call inparseUserDate(ISO_OFFSET_DATE_TIME, ISO_LOCAL_DATE_TIME, ISO_LOCAL_DATE, fallback formatter, legacy dd-MM-yyyy at lines 375–403), this results in 4N parse attempts for an N-line log file, even though both dates are constant across all iterations.Move date validation and parsing before the loop and outside the
try-catchblock, so validation errors propagate as HTTP 400. RefactorisValidlogEntryto accept pre-parsedLocalDateinstances:♻️ Proposed refactor
List<String> filteredLogEntries = new ArrayList<>(); + // Validate and parse dates once, outside the loop and before the catch block, + // so validation errors propagate to the caller as 400 Bad Request. + String datePattern = (StringUtils.isNotBlank(getAuditDateFormat()) ? getAuditDateFormat() + : AUDIT_FILE_DATE_FORMAT); + DateTimeFormatter formatter = DateTimeFormatter.ofPattern(datePattern); + validateDate(startDate, endDate, formatter); + LocalDate startLocal = StringUtils.isNotBlank(startDate) ? parseUserDate(startDate, formatter) : null; + LocalDate endLocal = StringUtils.isNotBlank(endDate) ? parseUserDate(endDate, formatter) : null; try { - String datePattern = (StringUtils.isNotBlank(getAuditDateFormat()) ? getAuditDateFormat() - : AUDIT_FILE_DATE_FORMAT); - log.debug("datePattern:{}", datePattern); - DateTimeFormatter formatter = DateTimeFormatter.ofPattern(datePattern); + log.debug("datePattern:{}", datePattern); for (int i = 0; i < logEntries.size(); i++) { String line = logEntries.get(i); if (StringUtils.isNotBlank(line) && line.length() > datePattern.length()) { String timestampPart = line.substring(0, datePattern.length()); LocalDate logEntryLocalDate = getDate(timestampPart, formatter); - if (isValidlogEntry(formatter, startDate, endDate, logEntryLocalDate)) { + if (isValidlogEntry(startLocal, endLocal, logEntryLocalDate)) { filteredLogEntries.add(line); } } }And simplify
isValidlogEntryto acceptLocalDateparameters directly, removing the redundant validation and parsing calls:- private boolean isValidlogEntry(DateTimeFormatter formatter, String startDate, String endDate, - LocalDate logEntryLocalDate) { + private boolean isValidlogEntry(LocalDate startLocalDate, LocalDate endLocalDate, + LocalDate logEntryLocalDate) { ... - validateDate(startDate, endDate, formatter); - LocalDate startLocalDate = null; - if (StringUtils.isNotBlank(startDate)) { - startLocalDate = parseUserDate(startDate, formatter); - } - LocalDate endLocalDate = null; - if (StringUtils.isNotBlank(endDate)) { - endLocalDate = parseUserDate(endDate, formatter); - } if (logEntryLocalDate != null) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java` around lines 229 - 262, filterLogByDate currently swallows validation BadRequestExceptions and reparses constant dates per log line; move date validation/parsing out of the loop and let validation errors propagate so callers get HTTP 400. Specifically: in filterLogByDate, call validateDate/parseUserDate once before iterating and obtain two LocalDate objects (or nulls) using the chosen formatter, remove the broad catch(Exception) that hides BadRequestException (or rethrow BadRequestException if you must catch other exceptions), and refactor isValidlogEntry to accept pre-parsed LocalDate parameters instead of calling validateDate/parseUserDate internally to eliminate redundant parsing per line; keep existing datePattern/formatter logic but perform it once before the loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java`:
- Around line 229-262: filterLogByDate currently swallows validation
BadRequestExceptions and reparses constant dates per log line; move date
validation/parsing out of the loop and let validation errors propagate so
callers get HTTP 400. Specifically: in filterLogByDate, call
validateDate/parseUserDate once before iterating and obtain two LocalDate
objects (or nulls) using the chosen formatter, remove the broad catch(Exception)
that hides BadRequestException (or rethrow BadRequestException if you must catch
other exceptions), and refactor isValidlogEntry to accept pre-parsed LocalDate
parameters instead of calling validateDate/parseUserDate internally to eliminate
redundant parsing per line; keep existing datePattern/formatter logic but
perform it once before the loop.
---
Duplicate comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AuditLogResource.java`:
- Around line 69-70: Update the stale Javadoc for AuditLogResource so the `@param`
tags for startDate and endDate reflect the new accepted ISO-8601 formats rather
than only "dd-MM-yyyy": locate the Javadoc on the AuditLogResource class/method
that documents parameters startDate and endDate and change the descriptions to
mention the supported ISO-8601 date and datetime formats (e.g., yyyy-MM-dd and
yyyy-MM-dd'T'HH:mm:ss or the specific ISO variants your parsing accepts) so the
documentation matches the implementation.
- Around line 394-403: The current code in AuditLogResource attempts the same
legacy parse unconditionally, causing a duplicate parse when fallbackFormatter
was built from AUDIT_FILE_DATE_FORMAT; change the final unconditional
LocalDate.parse(...) to only run the legacy dd-MM-yyyy parse when
fallbackFormatter == null, and otherwise propagate the original
DateTimeParseException (or return the method's error value) so you don't retry
the exact same pattern; reference fallbackFormatter, AUDIT_FILE_DATE_FORMAT, and
LocalDate.parse in your change.



Description
Accepts ISO-8601 date-time (e.g.
yyyy-MM-ddTHH:mm:ssZ) forstart_dateandend_dateon GET /api/v1/audit in addition to existingdd-MM-yyyy.Existing behaviour is unchanged; validation and OpenAPI spec updated.
Target issue
Support date-time search in /api/v1/audit: accept ISO-8601 (e.g. yyyy-MM-ddTHH:mm:ssZ) for start_date/end_date in addition to dd-MM-yyyy.
closes #13222
Implementation Details
Test and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.Summary by CodeRabbit
New Features
Improvements