#340 Part 3: Complete Audit Flow integration and create Audit Interface

[ginaxu1]

Summary

This PR centralizes audit logging for POLICY_CHECK and CONSENT_CHECK events in Orchestration Engine, implements PROVIDER_FETCH events, and establishes a clean Auditor interface in shared/audit/ for easy migration when audit-service moves to its own repository.

Key Changes:

• Audit Interface: Created Auditor interface in shared/audit/interface.go for clean abstraction
• Centralized Audit Logging: All audit events (ORCHESTRATION_REQUEST_RECEIVED, POLICY_CHECK, CONSENT_CHECK, PROVIDER_FETCH) are logged from Orchestration Engine for consistent traceID correlation
• POLICY_CHECK Events: Logged by OE after PDP API call (one record per API call, status reflects result)
• CONSENT_CHECK Events: Logged by OE after CE API call (one record per API call, status reflects result)
• Removed REQUEST/RESPONSE Variants: All events are single events (no REQUEST/RESPONSE pairs)
• Complete Traceability: Full audit trail from OE with all events linked by trace ID from the initial request context
• Reusable Audit Service: Moved OpenDIF-specific event types to sample enums.yaml config, keeping DefaultEnums generic

Example Audit Flow

A single OE request now generates a complete sequence of audit events linked by one traceID:

1. ORCHESTRATION_REQUEST_RECEIVED (traceID: abc-123)

   • Logged by Orchestration Engine when GraphQL request is received
   • Includes: application ID, GraphQL query
   • Status: SUCCESS

2. POLICY_CHECK (traceID: abc-123)

   • Logged by Orchestration Engine after PDP API call
   • One audit record per API call
   • Includes: application ID, required fields, authorization status, consent requirements, unauthorized/expired fields
   • Status: SUCCESS if API call succeeds, FAILURE if API call fails or unauthorized/expired
   • Target: policy-decision-point

3. CONSENT_CHECK (traceID: abc-123)

   • Logged by Orchestration Engine after CE API call
   • One audit record per API call
   • Includes: application ID, owner ID/email, consent ID, consent status, consent portal URL, fields count
   • Status: SUCCESS if API call succeeds, FAILURE if API call fails
   • Target: consent-engine

4. PROVIDER_FETCH (traceID: abc-123, target: provider-1, provider-2, etc.)

   • Logged by Orchestration Engine after receiving response from each provider
   • One audit record per API call
   • Includes: application ID, schema ID, service key, requested fields, GraphQL query, response status, errors, data keys
   • Status: SUCCESS or FAILURE based on provider response
   • Target: Provider service key

Example: For a request with 2 providers:

• Total events: 4 (1 orchestration + 1 policy + 1 consent + 2 providers)
• All linked by: traceID: abc-123

All events can be retrieved using: GET /api/audit-logs?traceId=abc-123

Changes

Shared Audit Package (shared/audit/)

• New Audit Interface: Created Auditor interface in interface.go for clean abstraction
  • Provides LogEvent() and IsEnabled() methods
  • Makes it easy to swap implementations when audit-service moves to its own repository
  • AuditClient alias maintained for backward compatibility
• Updated Middleware: Updated middleware.go to use Auditor interface instead of concrete AuditClient
• Updated Initialization: Updated init.go to use Auditor interface
• Client Implementation: Client struct implements Auditor interface (already had required methods)

Event Types

The following event types are now fully implemented:

Event Type	Logged By	Description	Status Values	Target Service
ORCHESTRATION_REQUEST_RECEIVED	Orchestration Engine	GraphQL request received	SUCCESS	-
POLICY_CHECK	Orchestration Engine	Authorization decision result (logged after PDP API call, one record per API call)	SUCCESS, FAILURE	policy-decision-point
CONSENT_CHECK	Orchestration Engine	Consent check result (logged after CE API call, one record per API call)	SUCCESS, FAILURE	consent-engine
PROVIDER_FETCH	Orchestration Engine	Provider fetch result (one record per API call)	SUCCESS, FAILURE	Provider service key

Configuration

Environment Variables

Orchestration Engine supports audit configuration:

• CHOREO_AUDIT_CONNECTION_SERVICEURL: Audit service base URL (e.g., http://localhost:3001)
  • If not set or empty, audit logging is disabled
  • Default: empty (audit disabled)
• ENABLE_AUDIT: Explicitly enable/disable audit logging
  • Values: true, 1, yes (case-insensitive) to enable
  • Default: enabled if CHOREO_AUDIT_CONNECTION_SERVICEURL is set

Upgrade Path

1. Deploy updated audit-service with new event types in enums.yaml:
   • POLICY_CHECK (one record per API call, logged by Orchestration Engine)
   • CONSENT_CHECK (one record per API call, logged by Orchestration Engine)
   • PROVIDER_FETCH (one record per API call, logged by Orchestration Engine)
2. Deploy updated Orchestration Engine with centralized audit logging for all events
3. Deploy updated Policy Decision Point and Consent Engine (audit logging removed, traceID propagation kept)
4. Set CHOREO_AUDIT_CONNECTION_SERVICEURL environment variable in Orchestration Engine
5. Verify audit events are being logged correctly using GET /api/audit-logs?traceId=<trace-id>

Related PRs

• 340 Refactor Audit Service for Generalized Auditing with Trace ID Support #384 - Refactor audit-service to be reusable, general with trace_id
• 317 Add Observability to PDP and OE #408 - Add shared audit client and integrate with OE and Portal Backend
• #340 Part 2: add Audit Service unit tests, db configuration, and delete legacy files #410 - Audit Service clean up and add unit tests

Related Issues

Closes #340

[gemini-code-assist]

Code Review

This pull request introduces comprehensive audit logging across the consent-engine, orchestration-engine, and policy-decision-point services. It adds new EventType enums to the audit-service configuration for ORCHESTRATION_REQUEST_RECEIVED, POLICY_CHECK_REQUEST, POLICY_CHECK_RESPONSE, CONSENT_CHECK_REQUEST, CONSENT_CHECK_RESPONSE, PROVIDER_FETCH_REQUEST, and PROVIDER_FETCH_RESPONSE. Each service now initializes a new AuditMiddleware to enable or disable audit logging based on configuration. The consent-engine and policy-decision-point both gain new audit.go middleware files (which a reviewer noted are duplicates and should be refactored into a shared package). This middleware handles extracting or generating trace IDs from HTTP headers and provides a global function for logging generalized audit events. The orchestration-engine's FederateQuery function and the consent-engine and policy-decision-point handlers are updated to log detailed request and response audit events for policy checks, consent checks, orchestration requests, and provider fetches. Review comments highlight several instances where json.Marshal errors are not handled consistently, leading to dropped audit events or compile errors due to unused variables, suggesting a refactor to ensure events are logged even with partial metadata. Additionally, the duplication of the audit.go middleware file across services was identified as a maintainability concern, recommending its relocation to a shared library.

I am having trouble creating individual review comments. Click here to see my feedback.

exchange/consent-engine/v1/handlers/internal_handler.go (184-188)

The variable jsonErr is declared via := but it's not used when json.Marshal succeeds. This will result in a Go compile error: jsonErr declared and not used. This issue is present in several of the new response-logging functions across the PR.

To fix this, you could restructure the code, for example:

var responseMetadataJSON []byte
responseMetadataBytes, err := json.Marshal(responseMetadata)
if err != nil {
    slog.Error("Failed to marshal response metadata for audit", "error", err)
    responseMetadataJSON = []byte("{}")
} else {
    responseMetadataJSON = responseMetadataBytes
}

exchange/orchestration-engine/federator/federator.go (712-716)

The variable jsonErr is declared via := but it's not used when json.Marshal succeeds. This will result in a Go compile error: jsonErr declared and not used. This same issue is present in logConsentCheckResponse (line 801) and logProviderFetchResponse (line 918) in this file.

Please refactor these to ensure the code compiles. For example:

var responseMetadataJSON []byte
responseMetadataBytes, err := json.Marshal(responseMetadata)
if err != nil {
    logger.Log.Error("Failed to marshal response metadata for audit", "error", err)
    responseMetadataJSON = []byte("{}")
} else {
    responseMetadataJSON = responseMetadataBytes
}

exchange/policy-decision-point/v1/handler.go (209-212)

The variable jsonErr is declared via := but it's not used when json.Marshal succeeds. This will result in a Go compile error: jsonErr declared and not used.

Please refactor this to ensure the code compiles. For example:

var responseMetadataJSON []byte
responseMetadataBytes, err := json.Marshal(responseMetadata)
if err != nil {
    // Consider logging this error
    responseMetadataJSON = []byte("{}")
} else {
    responseMetadataJSON = responseMetadataBytes
}

exchange/consent-engine/v1/middleware/audit.go (1-75)

This file is an exact copy of exchange/policy-decision-point/v1/middleware/audit.go. Duplicating this audit middleware logic across services increases maintenance overhead. A change in trace ID handling or global client initialization would need to be applied in multiple places.

This generic middleware for handling trace IDs and providing a global audit client would be better placed in a shared package, for example exchange/shared/middleware or even within the existing exchange/shared/audit package, to be reused by both consent-engine and policy-decision-point. This would follow the DRY (Don't Repeat Yourself) principle and improve long-term maintainability.

exchange/consent-engine/v1/handlers/internal_handler.go (131-134)

When json.Marshal fails, this function returns, causing the CONSENT_CHECK_REQUEST audit event to be dropped. This is inconsistent with the pattern in response-logging functions (e.g., logConsentCheckResponse), which proceed to log the event with empty metadata. For more robust auditing, it's better to log the event with empty metadata rather than dropping it entirely.

    if err != nil {
        slog.Error("Failed to marshal request metadata for audit", "error", err)
        requestMetadataJSON = []byte("{}")
    } else {
        // This branch is needed to avoid the "err declared and not used" compile error.
    }

exchange/orchestration-engine/federator/federator.go (617-621)

When json.Marshal fails, this function returns, causing the ORCHESTRATION_REQUEST_RECEIVED audit event to be dropped. This is inconsistent with the pattern in response-logging functions in this file, which are designed to log an event even if metadata marshaling fails. For more robust auditing, it's better to log the event with empty metadata rather than dropping it entirely. This applies to logPolicyCheckRequest, logConsentCheckRequest, and logProviderFetchRequest as well.

exchange/policy-decision-point/v1/handler.go (153-156)

When json.Marshal fails, this function returns, causing the POLICY_CHECK_REQUEST audit event to be dropped. The marshaling error is also not logged. For more robust auditing, it's better to log the event with empty metadata and also log the error, similar to other services.

[ginaxu1]

/gemini review

[gemini-code-assist]

Code Review

This is an excellent and comprehensive pull request that significantly improves the auditing capabilities across the platform. The move to a generalized, optional audit service with full trace ID propagation is a great architectural improvement. The replacement of the generic DATA_EXCHANGE event with more specific ones like ORCHESTRATION_REQUEST_RECEIVED and PROVIDER_FETCH adds valuable clarity to the audit trail. My review includes a few suggestions to further improve maintainability by reducing code duplication in the new middleware and enhancing the test mocks for better coverage.

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive audit logging system, which is a valuable addition for traceability. The implementation of a shared trace ID middleware is a solid architectural decision. However, the review has identified several critical issues that need to be addressed. There are significant inconsistencies between the implementation and the PR description, particularly regarding which service is responsible for logging POLICY_CHECK and CONSENT_CHECK events. The implementation in the Orchestration Engine currently handles this, while the description states this responsibility was moved to the PDP and CE services. Correspondingly, the audit logging logic is missing from the PDP and CE handlers. Additionally, there is a critical bug in the Orchestration Engine related to context metadata for auditing, as well as some dead and confusing code. These issues should be resolved before this PR is merged.

[gemini-code-assist]

Code Review

This pull request is a significant and well-executed refactoring of the audit logging mechanism. It successfully centralizes audit event logging within the Orchestration Engine, introduces a clean Auditor interface for better abstraction, and creates a reusable shared middleware for trace ID propagation, which are all excellent improvements. The addition of comprehensive unit tests for the mock repository and a new integration test for trace ID correlation greatly enhances the quality and reliability of the changes.

My review includes a critical comment regarding a potential context propagation issue in a new helper function that could break trace correlation under certain conditions. I've also provided a few medium-severity suggestions to improve test robustness and code clarity in the mock repository.

[gemini-code-assist]

Code Review

This is an excellent pull request that significantly improves the audit logging architecture. Centralizing the audit logic in the Orchestration Engine and introducing a clean Auditor interface are great steps towards a more maintainable and scalable system. The addition of the shared trace middleware to ensure trace ID propagation is also a fantastic improvement. The code is well-structured, and the inclusion of a comprehensive integration test for trace ID correlation is highly appreciated.

I have a couple of minor suggestions to further improve maintainability and documentation consistency, but overall, this is a very solid contribution. Great work!

[gemini-code-assist]

Code Review

This is a comprehensive pull request that successfully centralizes audit logging and introduces a clean Auditor interface. The changes significantly improve the audit flow's traceability and make the audit service more generic and reusable. I'm particularly impressed with the addition of the new integration tests, which provide excellent coverage for the end-to-end audit flow and trace ID correlation. The refactoring to move OpenDIF-specific enums out of the default configuration is also a great step towards making the service more modular.

I've found one high-severity issue related to duplicated trace ID handling logic that could lead to bugs. Please see my specific comment. Other than that, the changes are excellent.

[gemini-code-assist]

Code Review

This is an excellent pull request that significantly improves the audit logging capabilities. The introduction of the Auditor interface provides a clean abstraction, and centralizing the audit event logging in the Orchestration Engine ensures consistent trace ID correlation. The refactoring to make the audit service more generic and configurable via YAML is a great step towards reusability. The addition of comprehensive integration tests for the audit flow and trace ID correlation is a huge win for the project's stability.

I have a few suggestions to further improve the clarity of the audit logs by better separating request and response metadata, and to remove some redundant code. Overall, this is a very strong contribution.

[ginaxu1]

/gemini review

[sthanikan2000]

Only a small suggestion. Except that part LGTM