feat(jans-lock): log audit status

[yurem]

Prepare

• Read PR guidelines
• Read license information

---

Description

Target issue

#12599

closes #12599

Implementation Details

---

Test and Document the changes

• Static code analysis has been run locally and issues have been fixed
• Relevant unit and integration tests have been added/updated
• Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

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.

• I confirm that there is no impact on the docs due to the code changes in this PR.

Summary by CodeRabbit

• New Features

  • Audit logs now record explicit success/failure for operations.
  • Centralized audit processing for health, log, telemetry, and policy requests.

• Improvements

  • Simplified request interfaces for audit endpoints.
  • Audit entries are written after operations complete, improving accuracy.
  • More reliable client IP capture for audit records and clearer error/reporting flows.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds success-status tracking to audit logging by changing log(...) signatures to accept a boolean success, sets success on AuditLogEntry before logging, moves audits to finally blocks to record actual outcomes, and centralizes audit request processing in AuditRestWebServiceImpl.

Changes

Cohort / File(s)	Summary
Audit Logger Core API io/jans/lock/cedarling/service/app/audit/ApplicationCedarlingAuditLogger.java, io/jans/lock/service/app/audit/ApplicationAuditLogger.java	Public log() signature changed to log(AuditLogEntry, boolean success); implementations set auditLogEntry.setSuccess(success) before delegating to existing logging paths. Removed an unused PreDestroy import in cedarling variant.
Filter & Processing Layer io/jans/lock/cedarling/service/filter/CedarlingAuthorizationProcessingFilter.java, io/jans/lock/service/filter/AuthorizationProcessingFilter.java	Added @Context HttpServletRequest field for IP extraction; audit action updated (cedarling variant) and callers now invoke log(auditLogEntry, success) instead of two-step set+log.
Configuration & Well-Known Endpoints io/jans/lock/service/ws/rs/ConfigurationRestWebService.java, io/jans/lock/service/ws/rs/WellKnownConfiguration.java	Injected HttpServletRequest via @Context; moved audit logging into try/finally and pass actual success boolean to log(auditLogEntry, success).
Audit REST API (interface) io/jans/lock/service/ws/rs/audit/AuditRestWebService.java	Removed HttpServletResponse parameter from six endpoint method signatures (health, bulk health, log, bulk log, telemetry, bulk telemetry); signatures simplified to (HttpServletRequest, SecurityContext).
Audit REST Implementation io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java	Major refactor: dropped HttpServletResponse from handlers, added centralized processAuditRequest workflow, introduced dependencies (AppConfiguration, DataMapperService, JsonService, AuditForwarderService, AuditService, StatService, ApplicationAuditLogger), unified parsing/forwarding/persistence/stat reporting and response building.
Base Resource Utility io/jans/lock/service/ws/rs/base/BaseResource.java	uriInfo visibility tightened to private; added protected boolean getResponseResult(Response) helper to map response to success boolean.
Policy & Stat REST Implementations io/jans/lock/service/ws/rs/policy/PolicyRestWebServiceImpl.java, io/jans/lock/service/ws/rs/stat/StatRestWebServiceImpl.java	Wrapped logic in try/finally to capture response and call applicationAuditLogger.log(auditLogEntry, getResponseResult(response)); added request-based IP extraction, input validation, and response-local variable usage to ensure accurate success reporting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Pay extra attention to: AuditRestWebServiceImpl.java (centralized workflow, forwarding vs persistence, dependency wiring), all changed public method signatures (ensure callers updated), and correctness of success boolean propagation via getResponseResult() in try/finally placements.

Possibly related PRs

• feat(jans-lock): audit server actions #12588: Introduces the same audit logging signature changes and related filter updates that this PR follows up on.
• Lock logger modes #12571: Related refactor of the lock server audit/logging subsystem and centralized processing logic; overlaps with routing/persistence changes in this PR.

Suggested reviewers

• yuriyz
• yuriyzz
• pujavs

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat(jans-lock): log audit status' clearly and concisely summarizes the main change—implementing audit status logging based on operation outcomes.
Description check	✅ Passed	The PR description follows the template with all required sections completed: target issue (#12599), implementation details referenced, testing/documentation checklist marked, and documentation impact confirmed.
Linked Issues check	✅ Passed	All code changes directly support issue #12599: audit logger methods now accept a success parameter [ApplicationCedarlingAuditLogger, ApplicationAuditLogger, AuditRestWebServiceImpl, StatRestWebServiceImpl] and pass actual operation outcomes to reflect true/false status.
Out of Scope Changes check	✅ Passed	All changes are scoped to audit logging refactoring: method signatures updated to include success flags, IP extraction improved using HttpServletRequest context, audit logging relocated to finally blocks, and helper methods added to support the centralized audit workflow.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch lock_audit_status

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d1ae82a and bae97d1.

📒 Files selected for processing (6)

• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/ConfigurationRestWebService.java (3 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java (3 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/base/BaseResource.java (2 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/policy/PolicyRestWebServiceImpl.java (1 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/stat/StatRestWebService.java (3 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/stat/StatRestWebServiceImpl.java (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (4)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/ConfigurationRestWebService.java (1)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/stat/StatRestWebServiceImpl.java (1)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java (2)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

jans-lock/lock-server/service/src/main/java/io/jans/lock/util/ServerUtil.java (1)

• ServerUtil (43-113)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/policy/PolicyRestWebServiceImpl.java (1)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

🔇 Additional comments (14)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/base/BaseResource.java (2)

43-43: Nice tightening up of the encapsulation, partner!

Making uriInfo private while keeping the public getter is solid OOP practice. You better think twice before leaving fields package-private when you got getters! This change keeps the contract clean.

---

221-227: Let's get this party rollin' with success detection!

The logic here is spot-on! Returning false for null and checking the SUCCESSFUL family correctly identifies 2xx responses. This helper fits perfectly with the audit logging flow throughout the PR.

One tiny thought: getStatusInfo() could theoretically return null in edge cases, but Jakarta REST implementations make this extremely unlikely in practice. The current code is pragmatic and clean.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/stat/StatRestWebService.java (1)

48-49: I'm here to drop some knowledge on these signature updates!

Both statGet and statPost now accept @Context HttpServletRequest request as the first parameter, which is exactly what we need to extract the client IP for audit logging. The pattern is consistent across both methods, and using @Context injection is the proper Jakarta way!

This is a breaking interface change, but it's necessary to support the audit logging requirements from issue #12599.

Also applies to: 64-65

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/stat/StatRestWebServiceImpl.java (2)

74-81: Let's get this party rollin' with proper audit logging!

Like a wise ol' pigeon, I can see you've structured the audit flow correctly here! The try-finally block ensures that audit logging happens whether stat() succeeds or throws an exception. When an exception occurs, response stays null and getResponseResult(response) returns false, which correctly records the failure.

This pattern aligns perfectly with the PR objective to reflect actual operation outcomes!

---

93-100: I'm ready to have some fun with this consistent pattern!

The try-finally audit structure here matches statGet perfectly – consistency is key! The flow correctly captures whether the operation succeeded or failed, which is exactly what issue #12599 asked for.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/ConfigurationRestWebService.java (3)

67-67: Listen up, my friend! This @context injection is clean as a whistle!

The @Context HttpServletRequest parameter is rock-solid – JAX-RS guarantees it's never null during request processing, and since it's context-injected (not a client parameter), this change is completely non-breaking for API consumers. Nice move getting that request context for audit IP extraction!

---

68-69: Wise choice on that IP extraction, friend!

Using InetAddressUtility.getIpAddress(request) is the right call here. That utility checks all the proxy headers (X-Forwarded-For, Proxy-Client-IP, etc.) and handles comma-separated IPs correctly before falling back to remoteAddr. Your audit logs will capture the real client IP even behind load balancers and proxies. You're rollin' smart!

---

71-80: The null-safety concern in the review is incorrect – no NPE risk exists here.

Listen up, friend! I just dove into the implementation, and here's what I found: getLockConfiguration() ALWAYS creates and returns a non-null ObjectNode. The method constructs it with objectMapper.createObjectNode() and populates it with configuration data – there's no code path that returns null. So calling .toString() on line 74 is perfectly safe, no NPE risk whatsoever!

The try-finally audit logging pattern with the success flag? That's still solid as a rock. But this particular concern about null? That one doesn't hold water.

Likely an incorrect or invalid review comment.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java (6)

69-90: Nice setup, my friend!

All these freshly injected dependencies are properly wired for the centralized audit processing workflow. The CDI pattern is spot-on and everything looks clean!

---

246-310: Centralized processing for the win!

This wise ol' pigeon approves of your clean centralized audit request processor. The overloaded methods provide nice flexibility, error handling catches null JSON early, and cache headers are properly set. The separation between forwarding and persistence based on config mode is spot-on!

---

355-374: Documentation vibes are on point!

The updated javadoc clearly explains what this method does and how it handles non-array input. Nice work keepin' the code readable for the next developer who comes along!

---

376-441: Solid persistence logic, my friend!

The switch statement comprehensively covers all six endpoint types with proper JSON parsing and error handling. The distinction between BAD_REQUEST for parse failures and INTERNAL_SERVER_ERROR for persistence issues is the right call.

One small observation: mutating the ResponseBuilder status as a side effect while also returning a string message is a bit unconventional, but it works and keeps things consistent across the codebase.

---

103-103: You fixed that NPE trap, partner!

This wise ol' pigeon is happy to see you're now using the request parameter correctly in all six endpoints instead of getHttpRequest(). That previous NPE risk is gone and your audit logging will work smooth in all contexts—container or unit tests!

Based on past review comments.

Also applies to: 129-129, 157-157, 184-184, 210-210, 233-233

---

100-114: The original review comment is incorrect. AuditRestWebServiceImpl extends BaseResource, which defines the getResponseResult(Response response) method (BaseResource.java, line 221). The method explicitly handles null responses by returning false. The code will compile without errors and won't throw null pointer exceptions.

Likely an incorrect or invalid review comment.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[mo-auto]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[coderabbitai]

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/stat/StatRestWebServiceImpl.java (1)

69-97: This pigeon sees duplicate nests being built!

Both statGet and statPost have identical audit logging logic – creating the entry, wrapping in try-finally, logging with the response result. This duplication made it easier for the incorrect SSA_READ action type to slip through in both places!

Consider extracting the common audit logging pattern into a helper method to keep things DRY and reduce the chance of inconsistencies.

Here's one way to refactor this:

private Response statWithAudit(String months, String startMonth, String endMonth, String format, AuditActionType actionType) {
    AuditLogEntry auditLogEntry = new AuditLogEntry(InetAddressUtility.getIpAddress(getHttpRequest()), actionType);
    
    Response response = null;
    try {
        response = stat(months, startMonth, endMonth, format);
    } finally {
        applicationAuditLogger.log(auditLogEntry, getResponseResult(response));
    }
    
    return response;
}

@Override
public Response statGet(@QueryParam("month") String months,
                        @QueryParam("start-month") String startMonth,
                        @QueryParam("end-month") String endMonth,
                        @QueryParam("format") String format) {
    return statWithAudit(months, startMonth, endMonth, format, AuditActionType.STAT_READ);
}

@Override
public Response statPost(@FormParam("month") String months,
                         @FormParam("start-month") String startMonth,
                         @FormParam("end-month") String endMonth,
                         @FormParam("format") String format) {
    return statWithAudit(months, startMonth, endMonth, format, AuditActionType.STAT_READ);
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ffb47a and d1ae82a.

📒 Files selected for processing (11)

• jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/app/audit/ApplicationCedarlingAuditLogger.java (1 hunks)
• jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/filter/CedarlingAuthorizationProcessingFilter.java (2 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/app/audit/ApplicationAuditLogger.java (1 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/filter/AuthorizationProcessingFilter.java (2 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/ConfigurationRestWebService.java (2 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/WellKnownConfiguration.java (1 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebService.java (6 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java (3 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/base/BaseResource.java (1 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/policy/PolicyRestWebServiceImpl.java (1 hunks)
• jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/stat/StatRestWebServiceImpl.java (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (6)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/WellKnownConfiguration.java (1)

jans-lock/lock-server/service/src/main/java/io/jans/lock/util/ServerUtil.java (1)

• ServerUtil (43-113)

jans-lock/lock-server/cedarling/src/main/java/io/jans/lock/cedarling/service/filter/CedarlingAuthorizationProcessingFilter.java (1)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/policy/PolicyRestWebServiceImpl.java (1)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/audit/AuditRestWebServiceImpl.java (2)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

jans-lock/lock-server/service/src/main/java/io/jans/lock/util/ServerUtil.java (1)

• ServerUtil (43-113)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/filter/AuthorizationProcessingFilter.java (1)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/ConfigurationRestWebService.java (1)

jans-core/util/src/main/java/io/jans/net/InetAddressUtility.java (1)

• InetAddressUtility (21-121)

🔇 Additional comments (4)

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/stat/StatRestWebServiceImpl.java (1)

71-78: Nice work on this try-finally pattern, my friend!

The audit logging implementation is solid! By capturing the response and logging in the finally block, you ensure the audit gets recorded whether the operation succeeds or fails. This is exactly what we need to track the actual outcome, just like a pigeon never forgets where the good crumbs are!

The getResponseResult(response) call properly extracts the success status from the response for audit logging, aligning perfectly with the PR objective.

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/ConfigurationRestWebService.java (1)

54-55: Squawk! Formatting looks fine, friend!

Just a minor indentation tweak here – no functional changes to worry about. Your ApplicationAuditLogger field is still properly injected and ready to roll!

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/ws/rs/WellKnownConfiguration.java (1)

68-78: Sweet move on the success tracking

Listen up, my friend! This wise ol’ pigeon digs the way you default success to false, only flipping it after the JSON print sails through, so the audit trail actually reflects reality. Keep those good vibes flying!

jans-lock/lock-server/service/src/main/java/io/jans/lock/service/app/audit/ApplicationAuditLogger.java (1)

42-46: Audit entries now tell the full story

Listen up, my friend! This wise ol’ pigeon approves—grabbing the success flag right here keeps every audit entry honest before it hits the logger. That’s the kind of clarity that keeps the party groovin’.