Introduce a new API for debugging purposes#1033
Open
LinukaAr wants to merge 49 commits intowso2:masterfrom
Open
Introduce a new API for debugging purposes#1033LinukaAr wants to merge 49 commits intowso2:masterfrom
LinukaAr wants to merge 49 commits intowso2:masterfrom
Conversation
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
KD23243
reviewed
Feb 27, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/factories/DebugServiceFactory.java (1)
28-36: Guard the factory with the OSGi service check.Right now the class always constructs
DebugService, so a missingDebugRequestCoordinatoris only discovered on the first request. The sibling API factories fail fast in a static initializer instead.Based on learnings, factory classes should use eager static initialization blocks to fail fast if required OSGi services are not available, ensuring consistency across API service factories.♻️ Proposed refactor
+import org.wso2.carbon.identity.api.server.debug.common.DebugServiceHolder; import org.wso2.carbon.identity.api.server.debug.v1.core.DebugService; @@ - private static final DebugService SERVICE = new DebugService(); + private static final DebugService SERVICE; + + static { + if (DebugServiceHolder.getDebugRequestCoordinator() == null) { + throw new IllegalStateException("DebugRequestCoordinator is not available from OSGi context."); + } + SERVICE = new DebugService(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/factories/DebugServiceFactory.java` around lines 28 - 36, Replace the lazy instantiation in DebugServiceFactory with an eager static initializer that resolves and validates required OSGi services (e.g., DebugRequestCoordinator) at class load time and throws a clear runtime exception if missing; specifically, move creation of the DebugService instance into a static block that looks up the DebugRequestCoordinator (or other required services) and constructs the DebugService only when all dependencies are present, so getDebugService() returns a pre-validated SERVICE and the class fails fast on missing OSGi services.components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.java (1)
106-115: Guard the coordinator response before dereferencing it.Please double-check the framework contract here: if
handleResourceDebugRequest()can returnnull,response.getData()throws outsideexecuteWithServerErrorHandling, so the API skips its structured 500 path. Also,put(TIMESTAMP, ...)clobbers any framework-provided timestamp instead of preserving it.🛡️ Proposed hardening
DebugRequestCoordinator coordinator = getCoordinatorOrThrow(); DebugResponse response = executeWithServerErrorHandling( () -> coordinator.handleResourceDebugRequest(debugRequest), "Error processing start debug session request.", "Error occurred while processing debug request."); + if (response == null) { + throw new DebugFrameworkServerException( + DebugFrameworkConstants.ErrorMessages.ERROR_CODE_SERVER_ERROR.getCode(), + DebugFrameworkConstants.ErrorMessages.ERROR_CODE_SERVER_ERROR.getMessage(), + "Debug framework returned an empty response."); + } Map<String, Object> resultMap = response.getData() != null ? new HashMap<>(response.getData()) : new HashMap<>(); - resultMap.put(DebugConstants.ResponseKeys.TIMESTAMP, System.currentTimeMillis()); + resultMap.putIfAbsent(DebugConstants.ResponseKeys.TIMESTAMP, System.currentTimeMillis()); resultMap.putIfAbsent(DebugConstants.ResponseKeys.STATUS, deriveStatus(resultMap));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.java` around lines 106 - 115, The code dereferences response without guarding for null and always overwrites any existing timestamp; update the block after executeWithServerErrorHandling to first check if response is null and treat that as an error/empty response (e.g., use an empty map or throw via the same server error handling path), then build resultMap from response.getData() only when response != null; also change the timestamp insertion from resultMap.put(DebugConstants.ResponseKeys.TIMESTAMP, ...) to resultMap.putIfAbsent(DebugConstants.ResponseKeys.TIMESTAMP, System.currentTimeMillis()) so an existing framework timestamp is preserved; keep use of deriveStatus(resultMap) to set STATUS if absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/constants/DebugConstants.java`:
- Around line 60-63: The ResourceType constants use uppercase values but the
published API (debug.yaml) expects lowercase; update the constants
ResourceType.IDP and ResourceType.FRAUD_DETECTION to use "idp" and
"fraud_detection" respectively so runtime validation/routing matches the
documented API (and scan for any comparisons against these constants to ensure
behavior remains consistent).
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.java`:
- Around line 59-63: The code in DebugService uses
debugConnectionRequest.getConnectionId() without trimming, so
whitespace-only-trimmed IDs pass validation but the raw value is sent
downstream; update the block that populates properties (where
DebugConstants.RequestKeys.CONNECTION_ID is set) to call trim() on
debugConnectionRequest.getConnectionId() (after a null check) and only put the
trimmed value into the properties map so downstream lookups receive the
normalized ID.
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.java`:
- Around line 212-237: The code normalizes session/state/requested IDs into
debugId and then unconditionally writes it back to metadata as
DebugConstants.ResponseKeys.SESSION_ID, which mutates semantics; instead remove
the unconditional metadata.put(DebugConstants.ResponseKeys.SESSION_ID, ...) and
preserve original keys when populating metadata from frameworkResponse (i.e.,
let the for-loop copy SESSION_ID or STATE as they originally appear), and if you
need to include the fallback requestedDebugId, write it under a distinct key
(e.g., "requestedDebugId") rather than overwriting SESSION_ID; update references
around debugId, frameworkResponse, requestedDebugId, metadata and
response.setDebugId accordingly.
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yaml`:
- Around line 19-21: The security requirements currently list OAuth2 with an
empty scopes array (OAuth2: []), which means no scopes are enforced; update the
OAuth2 entries under the security section to list the actual scope keys (e.g.,
use the scope names like read, write, or the real scope identifiers used in the
OpenAPI "scopes" map) so the generated DebugApi.java/@Authorization.scopes
reflect and enforce the documented view/update scopes; locate the OAuth2 entries
in this file (and the other occurrences at the referenced ranges) and replace
the empty arrays with the correct scope names that match the "scopes"
definitions in the OpenAPI securitySchemes.
- Around line 179-188: The OpenAPI schema for DebugConnectionRequest currently
permits empty bodies; make connectionId required so requests lacking it are
rejected at validation. Update the DebugConnectionRequest object to include a
required array containing "connectionId" and ensure the existing connectionId
property remains unchanged; reference DebugConnectionRequest and its
connectionId property when making the change.
In `@components/org.wso2.carbon.identity.api.server.debug/pom.xml`:
- Around line 23-31: The module org.wso2.carbon.identity.api.server.debug
declares version 1.5.16-SNAPSHOT while its parent identity-api-server in the
reactor uses 1.3.233-SNAPSHOT; update the <version> element for
org.wso2.carbon.identity.api.server.debug to match the reactor/root project
version (use the same snapshot value as the parent POM) so parent resolution and
${project.version} inheritance are consistent and submodules resolve to the
reactor-built version.
- Around line 36-38: The property mavan.findbugsplugin.exclude.file currently
points to ../../../findbugs-exclude-filter.xml which climbs one level too many
and misses the repo-root file; update the value to
../../findbugs-exclude-filter.xml (i.e., change the property in the POM where
mavan.findbugsplugin.exclude.file is defined) so the module resolves the
findbugs-exclude-filter.xml at the repository root.
---
Nitpick comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.java`:
- Around line 106-115: The code dereferences response without guarding for null
and always overwrites any existing timestamp; update the block after
executeWithServerErrorHandling to first check if response is null and treat that
as an error/empty response (e.g., use an empty map or throw via the same server
error handling path), then build resultMap from response.getData() only when
response != null; also change the timestamp insertion from
resultMap.put(DebugConstants.ResponseKeys.TIMESTAMP, ...) to
resultMap.putIfAbsent(DebugConstants.ResponseKeys.TIMESTAMP,
System.currentTimeMillis()) so an existing framework timestamp is preserved;
keep use of deriveStatus(resultMap) to set STATUS if absent.
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/factories/DebugServiceFactory.java`:
- Around line 28-36: Replace the lazy instantiation in DebugServiceFactory with
an eager static initializer that resolves and validates required OSGi services
(e.g., DebugRequestCoordinator) at class load time and throws a clear runtime
exception if missing; specifically, move creation of the DebugService instance
into a static block that looks up the DebugRequestCoordinator (or other required
services) and constructs the DebugService only when all dependencies are
present, so getDebugService() returns a pre-validated SERVICE and the class
fails fast on missing OSGi services.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: deba01d4-0d7f-4231-9ceb-1c829fcea629
⛔ Files ignored due to path filters (8)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/DebugApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/DebugApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/factories/DebugApiServiceFactory.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/DebugConnectionRequest.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/DebugConnectionResponse.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/DebugConnectionResponseMetadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/DebugResult.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/Error.javais excluded by!**/gen/**
📒 Files selected for processing (10)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.common/pom.xmlcomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.common/src/main/java/org/wso2/carbon/identity/api/server/debug/common/DebugServiceHolder.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xmlcomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/constants/DebugConstants.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/factories/DebugServiceFactory.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yamlcomponents/org.wso2.carbon.identity.api.server.debug/pom.xmlpom.xml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.java (1)
193-203:⚠️ Potential issue | 🟠 MajorDon’t expose raw exception messages in the response description.
If
getDescription()is empty, this sendsexception.getMessage()back to the caller. That is user-visible API surface and can leak framework/internal wording; fall back to the generic description instead.🛡️ Proposed fix
private String resolveClientErrorDescription(DebugFrameworkClientException exception, String fallback) { if (exception == null) { return fallback; } if (exception.getDescription() != null && !exception.getDescription().trim().isEmpty()) { return exception.getDescription(); } - if (exception.getMessage() != null && !exception.getMessage().trim().isEmpty()) { - return exception.getMessage(); - } return fallback; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.java` around lines 193 - 203, The method resolveClientErrorDescription currently returns exception.getMessage() when getDescription() is empty, which can leak internal text; update resolveClientErrorDescription(DebugFrameworkClientException exception, String fallback) so that after null checks it never returns exception.getMessage() to the caller—if exception.getDescription() is null/blank, return the provided fallback instead (optionally log the exception message internally but do not expose it in the API response). Ensure the method still handles null exception and blank description cases and references resolveClientErrorDescription and DebugFrameworkClientException for locating the change.
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml (1)
114-149: Remove or activate commented-out plugin configuration.The commented-out
openapi-generator-maven-pluginblock appears to be unused scaffolding. If code generation is needed, uncomment and configure it properly; otherwise, remove the dead code to keep the POM clean.Additionally, note that the commented config uses
dateLibrary=java17which conflicts with the<source>1.8</source>/<target>1.8</target>compiler settings if this were to be activated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml` around lines 114 - 149, The POM contains a commented-out openapi-generator-maven-plugin block (openapi-generator-maven-plugin and cxf-wso2-openapi-generator) which should be either removed or re-enabled and corrected; either delete the entire commented plugin block to remove dead scaffolding, or uncomment and configure it properly (ensure dateLibrary=java17 is consistent with your Java target by updating the Maven compiler <source>/<target> from 1.8 to 17 or changing dateLibrary to a Java 8-compatible value) and verify the plugin versions and paths (inputSpec debug.yaml, generatorName org.wso2.carbon.codegen.CxfWso2Generator) before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml`:
- Around line 37-39: The pom property name contains a typo: change the property
key "mavan.findbugsplugin.exclude.file" to "maven.findbugsplugin.exclude.file"
so the FindBugs/SpotBugs exclude filter is picked up; update the properties
block where "mavan.findbugsplugin.exclude.file" is declared to use the corrected
"maven.findbugsplugin.exclude.file" identifier.
- Around line 81-90: The two debug artifacts
(org.wso2.carbon.identity.debug.framework and
org.wso2.carbon.identity.debug.idp) currently hardcode version 7.10.35; remove
the explicit <version> entries from these dependencies so they inherit the
version from the root POM, and add/update entries in the root POM's
<dependencyManagement> to declare these artifacts with version
${carbon.identity.framework.version}; ensure the property
${carbon.identity.framework.version} is defined in the root POM so both
dependencies pick up the centralized version.
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.java`:
- Around line 111-114: The code in DebugService currently unconditionally
overwrites any coordinator-provided timestamp by calling
resultMap.put(DebugConstants.ResponseKeys.TIMESTAMP,
System.currentTimeMillis()); change this to preserve existing timestamps by
using resultMap.putIfAbsent(DebugConstants.ResponseKeys.TIMESTAMP,
System.currentTimeMillis()) so the framework-provided timestamp is kept if
present while still supplying a fallback; leave deriveStatus(resultMap) logic
as-is.
- Around line 74-77: processGetResult currently forwards raw debugId to
getDebugResult causing mismatches with the trimmed id used at start; modify
processGetResult to normalize the input by trimming whitespace (and optionally
collapsing surrounding control chars) and validate it prior to lookup (reject
empty or whitespace-only ids with a BadRequest/DebugFrameworkClientException),
then pass the normalized id into getDebugResult; refer to the processGetResult
and getDebugResult methods and ensure behavior mirrors the start flow that trims
connectionId.
---
Duplicate comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.java`:
- Around line 193-203: The method resolveClientErrorDescription currently
returns exception.getMessage() when getDescription() is empty, which can leak
internal text; update
resolveClientErrorDescription(DebugFrameworkClientException exception, String
fallback) so that after null checks it never returns exception.getMessage() to
the caller—if exception.getDescription() is null/blank, return the provided
fallback instead (optionally log the exception message internally but do not
expose it in the API response). Ensure the method still handles null exception
and blank description cases and references resolveClientErrorDescription and
DebugFrameworkClientException for locating the change.
---
Nitpick comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml`:
- Around line 114-149: The POM contains a commented-out
openapi-generator-maven-plugin block (openapi-generator-maven-plugin and
cxf-wso2-openapi-generator) which should be either removed or re-enabled and
corrected; either delete the entire commented plugin block to remove dead
scaffolding, or uncomment and configure it properly (ensure dateLibrary=java17
is consistent with your Java target by updating the Maven compiler
<source>/<target> from 1.8 to 17 or changing dateLibrary to a Java 8-compatible
value) and verify the plugin versions and paths (inputSpec debug.yaml,
generatorName org.wso2.carbon.codegen.CxfWso2Generator) before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 546529a2-4c80-4536-a945-d5e65ab110d6
📒 Files selected for processing (6)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.common/pom.xmlcomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xmlcomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/constants/DebugConstants.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.javacomponents/org.wso2.carbon.identity.api.server.debug/pom.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- components/org.wso2.carbon.identity.api.server.debug/pom.xml
Comment on lines
+81
to
+90
| <dependency> | ||
| <groupId>org.wso2.carbon.identity.framework</groupId> | ||
| <artifactId>org.wso2.carbon.identity.debug.framework</artifactId> | ||
| <version>7.10.35</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.wso2.carbon.identity.framework</groupId> | ||
| <artifactId>org.wso2.carbon.identity.debug.idp</artifactId> | ||
| <version>7.10.35</version> | ||
| </dependency> |
Contributor
There was a problem hiding this comment.
Hardcoded dependency versions for debug framework artifacts.
Same issue as in the common module: both org.wso2.carbon.identity.debug.framework and org.wso2.carbon.identity.debug.idp have hardcoded version 7.10.35 instead of using centralized version management from the root POM.
♻️ Suggested fix
<dependency>
<groupId>org.wso2.carbon.identity.framework</groupId>
<artifactId>org.wso2.carbon.identity.debug.framework</artifactId>
- <version>7.10.35</version>
</dependency>
<dependency>
<groupId>org.wso2.carbon.identity.framework</groupId>
<artifactId>org.wso2.carbon.identity.debug.idp</artifactId>
- <version>7.10.35</version>
</dependency>Ensure both artifacts are added to the root POM's <dependencyManagement> section with ${carbon.identity.framework.version}.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml`
around lines 81 - 90, The two debug artifacts
(org.wso2.carbon.identity.debug.framework and
org.wso2.carbon.identity.debug.idp) currently hardcode version 7.10.35; remove
the explicit <version> entries from these dependencies so they inherit the
version from the root POM, and add/update entries in the root POM's
<dependencyManagement> to declare these artifacts with version
${carbon.identity.framework.version}; ensure the property
${carbon.identity.framework.version} is defined in the root POM so both
dependencies pick up the centralized version.
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new Debug REST API module (v1) to start debug sessions for supported resource types and retrieve results by debug session ID.
Changes:
- Added a new Maven module
org.wso2.carbon.identity.api.server.debugwithcommonandv1sub-modules. - Added OpenAPI definition (
debug.yaml) and generated JAX-RS stubs/models for/debug/{resourceType}and/debug/{debugId}/result. - Implemented API/service layer to delegate to the debug framework and return structured error responses.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Registers the new debug API component as a reactor module. |
| components/org.wso2.carbon.identity.api.server.debug/pom.xml | Defines the new debug API parent module and aggregates common + v1. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yaml | Adds the OpenAPI contract for the Debug API. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.java | Implements the REST endpoints and error mapping to the API models. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/factories/DebugServiceFactory.java | Provides a shared DebugService instance for API impls. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.java | Bridges API requests to the underlying debug framework coordinator. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/constants/DebugConstants.java | Introduces API-level constants for request/response keys and error codes. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/Error.java | Generated error response model for consistent API errors. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/DebugResult.java | Generated model for debug result retrieval responses. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/DebugConnectionResponseMetadata.java | Generated metadata model (e.g., authorization URL). |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/DebugConnectionResponse.java | Generated response model for starting debug sessions. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/model/DebugConnectionRequest.java | Generated request model for starting debug sessions. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/factories/DebugApiServiceFactory.java | Generated factory providing the API service implementation. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/DebugApiService.java | Generated service interface for debug endpoints. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/DebugApi.java | Generated JAX-RS resource wiring requests to the service. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml | Defines dependencies/build for the debug v1 API bundle. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.common/src/main/java/org/wso2/carbon/identity/api/server/debug/common/DebugServiceHolder.java | Adds OSGi service lookup for DebugRequestCoordinator. |
| components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.common/pom.xml | Defines the common module packaging and dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
components/org.wso2.carbon.identity.api.server.debug/pom.xml (1)
36-38:⚠️ Potential issue | 🟠 MajorFix the shared FindBugs exclusion path.
From
components/org.wso2.carbon.identity.api.server.debug,../../../findbugs-exclude-filter.xmlgoes one directory above the repository root, so this module will miss the shared exclusions.🛠️ Proposed fix
- <mavan.findbugsplugin.exclude.file>${project.basedir}/../../../findbugs-exclude-filter.xml</mavan.findbugsplugin.exclude.file> + <mavan.findbugsplugin.exclude.file>${project.basedir}/../../findbugs-exclude-filter.xml</mavan.findbugsplugin.exclude.file>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.api.server.debug/pom.xml` around lines 36 - 38, The mavan.findbugsplugin.exclude.file property in the pom.xml points too far up (../../../...) and misses the repository's shared findbugs-exclude-filter.xml; update the property value (mavan.findbugsplugin.exclude.file) to the correct relative path that references the shared exclusions (e.g. change ../../../findbugs-exclude-filter.xml to ../../findbugs-exclude-filter.xml) so the module picks up the repository-wide findbugs-exclude-filter.xml.
🧹 Nitpick comments (1)
findbugs-exclude-filter.xml (1)
157-163: Scope this suppression to the flagged methods.The justification here only covers
resolveConnectionStatusandresolveResultStatus, but the filter suppressesIMPROPER_UNICODEfor the wholeDebugApiServiceImplclass. That makes future real findings in this REST resource invisible.🛠️ Narrower exclusion
- <FindBugsFilter> - <Match> - <Class name="org.wso2.carbon.identity.api.server.debug.v1.impl.DebugApiServiceImpl" /> - <Bug pattern="IMPROPER_UNICODE" /> - </Match> - </FindBugsFilter> + <FindBugsFilter> + <Match> + <Class name="org.wso2.carbon.identity.api.server.debug.v1.impl.DebugApiServiceImpl" /> + <Method name="resolveConnectionStatus" /> + <Bug pattern="IMPROPER_UNICODE" /> + </Match> + </FindBugsFilter> + <FindBugsFilter> + <Match> + <Class name="org.wso2.carbon.identity.api.server.debug.v1.impl.DebugApiServiceImpl" /> + <Method name="resolveResultStatus" /> + <Bug pattern="IMPROPER_UNICODE" /> + </Match> + </FindBugsFilter>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@findbugs-exclude-filter.xml` around lines 157 - 163, The suppression currently silences IMPROPER_UNICODE for the entire DebugApiServiceImpl class; narrow it to only the flagged methods by replacing the Class-level Match with Method-level matches for resolveConnectionStatus and resolveResultStatus (use <Method name="resolveConnectionStatus" /> and <Method name="resolveResultStatus" /> or include their exact signatures if available) so the filter only excludes IMPROPER_UNICODE for those methods in DebugApiServiceImpl and leaves other findings in that class visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.java`:
- Around line 186-199: Normalize and validate the incoming resourceType by
trimming (and optionally lowercasing) before performing comparisons and before
passing it downstream: replace direct uses of the raw resourceType with a
normalized variable (e.g., normalizedResourceType = resourceType.trim()) in
DebugService, use normalizedResourceType when checking against
DebugConstants.ResourceType.IDP and DebugConstants.ResourceType.FRAUD_DETECTION,
and ensure the same normalizedResourceType is passed into
handleGenericDebugRequest so downstream logic receives the trimmed value.
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yaml`:
- Around line 42-50: The OpenAPI parameter "resourceType" currently allows any
non-empty string; update its schema in debug.yaml (parameter name
"resourceType") to declare an enum with the allowed values
["idp","fraud_detection"] so the spec validates inputs up front; modify the
schema block for resourceType to include an enum array containing those two
strings (and keep other constraints like type/minLength/maxLength as
appropriate).
- Around line 147-149: The OAuth2 `authorizationCode` block in the OpenAPI spec
uses hardcoded https://localhost:9443 URLs; change the `authorizationUrl` and
`tokenUrl` values to environment-agnostic paths (for example `/oauth2/authorize`
and `/oauth2/token`) so Swagger UI and generated clients resolve them against
the OpenAPI `servers[].url` (or replace with fully qualified production URLs as
appropriate); update the `authorizationCode` -> `authorizationUrl` and
`authorizationCode` -> `tokenUrl` keys accordingly.
---
Duplicate comments:
In `@components/org.wso2.carbon.identity.api.server.debug/pom.xml`:
- Around line 36-38: The mavan.findbugsplugin.exclude.file property in the
pom.xml points too far up (../../../...) and misses the repository's shared
findbugs-exclude-filter.xml; update the property value
(mavan.findbugsplugin.exclude.file) to the correct relative path that references
the shared exclusions (e.g. change ../../../findbugs-exclude-filter.xml to
../../findbugs-exclude-filter.xml) so the module picks up the repository-wide
findbugs-exclude-filter.xml.
---
Nitpick comments:
In `@findbugs-exclude-filter.xml`:
- Around line 157-163: The suppression currently silences IMPROPER_UNICODE for
the entire DebugApiServiceImpl class; narrow it to only the flagged methods by
replacing the Class-level Match with Method-level matches for
resolveConnectionStatus and resolveResultStatus (use <Method
name="resolveConnectionStatus" /> and <Method name="resolveResultStatus" /> or
include their exact signatures if available) so the filter only excludes
IMPROPER_UNICODE for those methods in DebugApiServiceImpl and leaves other
findings in that class visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6ff8cb5-1bdc-465f-9245-0c275c0e406b
⛔ Files ignored due to path filters (1)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/gen/java/org/wso2/carbon/identity/api/server/debug/v1/factories/DebugApiServiceFactory.javais excluded by!**/gen/**
📒 Files selected for processing (6)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xmlcomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yamlcomponents/org.wso2.carbon.identity.api.server.debug/pom.xmlfindbugs-exclude-filter.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml
- components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/impl/DebugApiServiceImpl.java
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yaml (1)
102-107: Consider adding length constraints todebugIdfor consistency.Unlike
resourceType(which hasminLength: 1,maxLength: 50), thedebugIdparameter has no length constraints. Adding similar bounds would provide defense-in-depth at the spec layer, consistent with other parameters.📏 Suggested schema addition
- name: "debugId" in: "path" description: "The debug session identifier." required: true schema: type: "string" + minLength: 1 + maxLength: 255🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yaml` around lines 102 - 107, The debugId path parameter currently lacks length constraints; add minLength: 1 and maxLength: 50 to its schema (matching the resourceType bounds) so the debugId parameter enforces the same length limits at the spec layer; update the debugId parameter's schema block to include these properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yaml`:
- Around line 102-107: The debugId path parameter currently lacks length
constraints; add minLength: 1 and maxLength: 50 to its schema (matching the
resourceType bounds) so the debugId parameter enforces the same length limits at
the spec layer; update the debugId parameter's schema block to include these
properties.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc09fc87-7aac-4954-a502-071143b095c1
📒 Files selected for processing (3)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/java/org/wso2/carbon/identity/api/server/debug/v1/core/DebugService.javacomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/src/main/resources/debug.yamlpom.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- pom.xml
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml (1)
81-90:⚠️ Potential issue | 🟠 MajorAvoid pinning these framework artifact versions in the child POM.
Lines 84 and 89 bypass parent dependency management for
org.wso2.carbon.identity.debug.frameworkandorg.wso2.carbon.identity.debug.idp. That makes this module easy to drift from the rest of the reactor during framework upgrades.♻️ Proposed fix
<dependency> <groupId>org.wso2.carbon.identity.framework</groupId> <artifactId>org.wso2.carbon.identity.debug.framework</artifactId> - <version>7.10.35</version> </dependency> <dependency> <groupId>org.wso2.carbon.identity.framework</groupId> <artifactId>org.wso2.carbon.identity.debug.idp</artifactId> - <version>7.10.35</version> </dependency>Run this to confirm these artifacts are version-managed centrally and to locate any remaining local pins:
#!/bin/bash set -euo pipefail rg -n -C2 --glob 'pom.xml' \ '<dependencyManagement>|<artifactId>org\.wso2\.carbon\.identity\.debug\.(framework|idp)</artifactId>|<carbon\.identity\.framework\.version>|<version>7\.10\.35</version>'Expected result: the root/parent POM manages these artifacts, and this module is not the place pinning
7.10.35.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml` around lines 81 - 90, The POM currently pins versions for the dependencies with artifactId org.wso2.carbon.identity.debug.framework and org.wso2.carbon.identity.debug.idp; remove the explicit <version>7.10.35</version> entries so these artifacts inherit versions from the parent dependencyManagement, or alternatively move them into the parent dependencyManagement if not already present; locate the dependency blocks for org.wso2.carbon.identity.debug.framework and org.wso2.carbon.identity.debug.idp and delete the <version> element(s) so the module no longer overrides the centrally managed framework version.
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml (1)
114-165: Enable automated regeneration of API stubs from the OpenAPI spec.The generated API sources (
src/gen/java) are committed to version control, anddebug.yamlis maintained in the repository. However, the only code generator configuration in this module—theopenapi-generator-maven-plugin—is commented out. This creates a reproducibility gap: there is no build-time mechanism to regenerate stubs fromdebug.yaml, so the spec and checked-in sources can drift without detection.Either:
- Uncomment and enable the generator plugin so regeneration runs during builds, or
- Document and enforce the regeneration workflow (e.g., via CI/CD or contributor guidelines) if generated sources are intentionally managed elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml` around lines 114 - 165, The pom currently has the openapi-generator-maven-plugin block commented out which prevents build-time regeneration of the API stubs from debug.yaml; restore and enable the plugin block (openapi-generator-maven-plugin with generatorName org.wso2.carbon.codegen.CxfWso2Generator and inputSpec pointing to src/main/resources/debug.yaml) so it executes during the generate-sources phase (goal generate) and outputs to src/gen/java, or alternatively add documentation/CI steps that enforce regeneration of src/gen/java from debug.yaml (or both) and keep the plugin configuration (and generator dependency cxf-wso2-openapi-generator) present for reproducibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml`:
- Around line 81-90: The POM currently pins versions for the dependencies with
artifactId org.wso2.carbon.identity.debug.framework and
org.wso2.carbon.identity.debug.idp; remove the explicit
<version>7.10.35</version> entries so these artifacts inherit versions from the
parent dependencyManagement, or alternatively move them into the parent
dependencyManagement if not already present; locate the dependency blocks for
org.wso2.carbon.identity.debug.framework and org.wso2.carbon.identity.debug.idp
and delete the <version> element(s) so the module no longer overrides the
centrally managed framework version.
---
Nitpick comments:
In
`@components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xml`:
- Around line 114-165: The pom currently has the openapi-generator-maven-plugin
block commented out which prevents build-time regeneration of the API stubs from
debug.yaml; restore and enable the plugin block (openapi-generator-maven-plugin
with generatorName org.wso2.carbon.codegen.CxfWso2Generator and inputSpec
pointing to src/main/resources/debug.yaml) so it executes during the
generate-sources phase (goal generate) and outputs to src/gen/java, or
alternatively add documentation/CI steps that enforce regeneration of
src/gen/java from debug.yaml (or both) and keep the plugin configuration (and
generator dependency cxf-wso2-openapi-generator) present for reproducibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a933f68d-56d2-4c05-bbe3-a82d08693338
📒 Files selected for processing (3)
components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.common/pom.xmlcomponents/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.v1/pom.xmlcomponents/org.wso2.carbon.identity.api.server.debug/pom.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- components/org.wso2.carbon.identity.api.server.debug/org.wso2.carbon.identity.api.server.debug.common/pom.xml
- components/org.wso2.carbon.identity.api.server.debug/pom.xml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR introduces a generic Debug REST API framework that enables developers and administrators to test and validate various system configurations (Identity Providers, fraud detection rules, etc.)
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.Security checks
Related PRs
Related Issues
Summary by CodeRabbit