Add ciba configs to the application#1069
Merged
sahandilshan merged 1 commit intowso2:masterfrom Feb 23, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Client Initiated Backchannel Authentication (CIBA) support across API models, OpenAPI schemas, constants, metadata generation, service wiring, and build dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MetadataAPI as ServerApplicationMetadataService
participant CibaService as CibaAuthServiceImpl
participant OIDCConfig as OpenIDConnectConfiguration
Client->>MetadataAPI: getOIDCMetadata()
MetadataAPI->>CibaService: getSupportedNotificationChannels()
CibaService-->>MetadataAPI: supported channels list
MetadataAPI->>MetadataAPI: build CIBAMetadata (map channels → CIBANotificationChannel)
MetadataAPI->>OIDCConfig: cibaMetadata(CIBAMetadata)
MetadataAPI-->>Client: OIDC metadata (includes CIBA)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
Comment on lines
+325
to
+328
| private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, | ||
| CIBAAuthenticationRequestConfiguration cibaAuthReq) { | ||
|
|
||
| if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { |
There was a problem hiding this comment.
Log Improvement Suggestion No: 1
Suggested change
| private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, | |
| CIBAAuthenticationRequestConfiguration cibaAuthReq) { | |
| if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { | |
| private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, | |
| CIBAAuthenticationRequestConfiguration cibaAuthReq) { | |
| if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { | |
| log.debug("Updating CIBA authentication request configurations for application"); |
Comment on lines
+336
to
+339
| } else { | ||
| consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY); | ||
| } | ||
| consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel()); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 2
Suggested change
| } else { | |
| consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY); | |
| } | |
| consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel()); | |
| } else { | |
| consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY); | |
| } | |
| consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel()); | |
| log.info("CIBA authentication request configurations updated successfully for application"); |
...tion/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java
Show resolved
Hide resolved
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 | ||
| #### Log Improvement Suggestion No: 2 | ||
| #### Log Improvement Suggestion No: 4 |
![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: 1
🧹 Nitpick comments (2)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml (1)
5001-5020: Consider adding aminimumconstraint oncibaAuthReqExpiryTimeand documenting the relationship between channels.Two optional improvements:
- A
minimum: 1oncibaAuthReqExpiryTimewould prevent nonsensical zero or negative expiry values at the schema validation level.- Consider noting (in
cibaDefaultNotificationChannel's description) that the default channel should be one of the values listed incibaNotificationChannels, since OpenAPI cannot enforce this cross-field constraint.Proposed diff
cibaAuthReqExpiryTime: type: integer format: int64 + minimum: 1 description: "CIBA authentication request expiry time in seconds." example: 600 cibaNotificationChannels: type: array items: type: string description: "List of allowed notification channels." example: - "email" - "sms" cibaDefaultNotificationChannel: type: string - description: "Default notification channel." + description: "Default notification channel. Must be one of the values in cibaNotificationChannels." example: "email"🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml` around lines 5001 - 5020, Add validation and clarify the default-channel relationship in the CIBAAuthenticationRequestConfiguration schema: add "minimum: 1" to the cibaAuthReqExpiryTime property to prevent zero/negative values, and update the description of cibaDefaultNotificationChannel to state that the default must be one of the entries in cibaNotificationChannels (e.g., "Default notification channel; must be one of the values listed in cibaNotificationChannels"). Reference the CIBAAuthenticationRequestConfiguration object and the properties cibaAuthReqExpiryTime, cibaNotificationChannels, and cibaDefaultNotificationChannel when making these changes.components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java (1)
237-252: Consider trimming channel names after split to guard against whitespace in stored values.If the comma-separated string in the DTO ever contains spaces (e.g.,
"email, sms"), the resulting list entries will have leading/trailing whitespace. While the current write path inApiModelToOAuthConsumerAppdoesn't introduce spaces, the DTO value could originate from other sources.♻️ Optional: trim after split
- return Arrays.asList(oAuthConsumerAppDTO.getCibaNotificationChannels().split(",")); + return Arrays.stream(oAuthConsumerAppDTO.getCibaNotificationChannels().split(",")) + .map(String::trim) + .collect(java.util.stream.Collectors.toList());🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java` around lines 237 - 252, The getCibaNotificationChannels method should trim whitespace and discard empty entries when splitting the comma-delimited string; update getCibaNotificationChannels(OAuthConsumerAppDTO) to split the DTO value, map each token with .trim(), filter out empty strings, and collect into a List<String> (keeping the existing blank-check and returning Collections.emptyList() when appropriate); ensure buildCIBAAuthenticationRequestConfiguration still calls getCibaNotificationChannels unchanged so downstream consumers receive the trimmed list.
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java`:
- Around line 325-341: The method updateCIBAAuthenticationRequestConfigurations
currently risks NPEs; first add a null guard for the cibaAuthReq parameter
(return early if cibaAuthReq == null) and only proceed if
consumerAppDTO.getGrantTypes() is non-null/non-empty (e.g., use
CollectionUtils.isNotEmpty(consumerAppDTO.getGrantTypes()) or check for null
before calling contains) when checking CIBA_GRANT_TYPE, then replace the manual
StringBuilder loop with String.join(",",
cibaAuthReq.getCibaNotificationChannels()) to set
consumerAppDTO.setCibaNotificationChannels(...) while preserving the
else-to-empty-string behavior and still set cibaAuthReqExpiryTime and
cibaDefaultNotificationChannel.
---
Nitpick comments:
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java`:
- Around line 237-252: The getCibaNotificationChannels method should trim
whitespace and discard empty entries when splitting the comma-delimited string;
update getCibaNotificationChannels(OAuthConsumerAppDTO) to split the DTO value,
map each token with .trim(), filter out empty strings, and collect into a
List<String> (keeping the existing blank-check and returning
Collections.emptyList() when appropriate); ensure
buildCIBAAuthenticationRequestConfiguration still calls
getCibaNotificationChannels unchanged so downstream consumers receive the
trimmed list.
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml`:
- Around line 5001-5020: Add validation and clarify the default-channel
relationship in the CIBAAuthenticationRequestConfiguration schema: add "minimum:
1" to the cibaAuthReqExpiryTime property to prevent zero/negative values, and
update the description of cibaDefaultNotificationChannel to state that the
default must be one of the entries in cibaNotificationChannels (e.g., "Default
notification channel; must be one of the values listed in
cibaNotificationChannels"). Reference the CIBAAuthenticationRequestConfiguration
object and the properties cibaAuthReqExpiryTime, cibaNotificationChannels, and
cibaDefaultNotificationChannel when making these changes.
Comment on lines
+325
to
+341
| private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, | ||
| CIBAAuthenticationRequestConfiguration cibaAuthReq) { | ||
|
|
||
| if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { | ||
| consumerAppDTO.setCibaAuthReqExpiryTime(cibaAuthReq.getCibaAuthReqExpiryTime()); | ||
| List<String> cibaNotificationChannels = cibaAuthReq.getCibaNotificationChannels(); | ||
| StringBuilder sb = new StringBuilder(); | ||
| if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) { | ||
| cibaNotificationChannels.forEach(channel -> sb.append(channel).append(",")); | ||
| sb.deleteCharAt(sb.length() - 1); | ||
| consumerAppDTO.setCibaNotificationChannels(sb.toString()); | ||
| } else { | ||
| consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY); | ||
| } | ||
| consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel()); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
NullPointerException risks: missing null guards on both consumerAppDTO.getGrantTypes() and cibaAuthReq.
Two NPE paths:
consumerAppDTO.getGrantTypes()can benull—getGrantTypes()(line 100) returnsnullwhen grant types are empty, so.contains(...)on line 328 will throw NPE.cibaAuthReqis never null-checked — every otherupdate*method in this class guards against anullconfiguration parameter (e.g.,updateSubjectTokenConfigurationschecksif (subjectToken != null)). If the caller omits the CIBA block from the request JSON,oidcModel.getCibaAuthenticationRequest()will benull, and line 329 will throw NPE.
Additionally, the comma-separated string construction can be simplified with String.join.
🐛 Proposed fix
private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO,
CIBAAuthenticationRequestConfiguration cibaAuthReq) {
- if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) {
+ if (cibaAuthReq == null || StringUtils.isBlank(consumerAppDTO.getGrantTypes())
+ || !consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) {
+ return;
+ }
+
- consumerAppDTO.setCibaAuthReqExpiryTime(cibaAuthReq.getCibaAuthReqExpiryTime());
- List<String> cibaNotificationChannels = cibaAuthReq.getCibaNotificationChannels();
- StringBuilder sb = new StringBuilder();
- if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) {
- cibaNotificationChannels.forEach(channel -> sb.append(channel).append(","));
- sb.deleteCharAt(sb.length() - 1);
- consumerAppDTO.setCibaNotificationChannels(sb.toString());
- } else {
- consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY);
- }
- consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel());
+ consumerAppDTO.setCibaAuthReqExpiryTime(cibaAuthReq.getCibaAuthReqExpiryTime());
+ List<String> cibaNotificationChannels = cibaAuthReq.getCibaNotificationChannels();
+ if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) {
+ consumerAppDTO.setCibaNotificationChannels(String.join(",", cibaNotificationChannels));
+ } else {
+ consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY);
}
+ consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, | |
| CIBAAuthenticationRequestConfiguration cibaAuthReq) { | |
| if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { | |
| consumerAppDTO.setCibaAuthReqExpiryTime(cibaAuthReq.getCibaAuthReqExpiryTime()); | |
| List<String> cibaNotificationChannels = cibaAuthReq.getCibaNotificationChannels(); | |
| StringBuilder sb = new StringBuilder(); | |
| if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) { | |
| cibaNotificationChannels.forEach(channel -> sb.append(channel).append(",")); | |
| sb.deleteCharAt(sb.length() - 1); | |
| consumerAppDTO.setCibaNotificationChannels(sb.toString()); | |
| } else { | |
| consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY); | |
| } | |
| consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel()); | |
| } | |
| } | |
| private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, | |
| CIBAAuthenticationRequestConfiguration cibaAuthReq) { | |
| if (cibaAuthReq == null || StringUtils.isBlank(consumerAppDTO.getGrantTypes()) | |
| || !consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { | |
| return; | |
| } | |
| consumerAppDTO.setCibaAuthReqExpiryTime(cibaAuthReq.getCibaAuthReqExpiryTime()); | |
| List<String> cibaNotificationChannels = cibaAuthReq.getCibaNotificationChannels(); | |
| if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) { | |
| consumerAppDTO.setCibaNotificationChannels(String.join(",", cibaNotificationChannels)); | |
| } else { | |
| consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY); | |
| } | |
| consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel()); | |
| } |
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java`
around lines 325 - 341, The method updateCIBAAuthenticationRequestConfigurations
currently risks NPEs; first add a null guard for the cibaAuthReq parameter
(return early if cibaAuthReq == null) and only proceed if
consumerAppDTO.getGrantTypes() is non-null/non-empty (e.g., use
CollectionUtils.isNotEmpty(consumerAppDTO.getGrantTypes()) or check for null
before calling contains) when checking CIBA_GRANT_TYPE, then replace the manual
StringBuilder loop with String.join(",",
cibaAuthReq.getCibaNotificationChannels()) to set
consumerAppDTO.setCibaNotificationChannels(...) while preserving the
else-to-empty-string behavior and still set cibaAuthReqExpiryTime and
cibaDefaultNotificationChannel.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds Client Initiated Backchannel Authentication (CIBA) configuration support to the application management API. It introduces a new CIBAAuthenticationRequestConfiguration schema and integrates it into the OpenIDConnect configuration, allowing applications to configure CIBA-specific settings including authentication request expiry time, notification channels, and default notification channel.
Changes:
- Added CIBA configuration schema to the OpenAPI specification (applications.yaml)
- Implemented bidirectional conversion between API models and OAuth consumer DTOs for CIBA settings
- Added CIBA_GRANT_TYPE constant for grant type checking
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| applications.yaml | Added CIBAAuthenticationRequestConfiguration schema definition with three properties: expiry time, notification channels list, and default channel |
| CIBAAuthenticationRequestConfiguration.java | Generated model class for CIBA configuration with standard getters/setters, equals, hashCode, and toString methods |
| OpenIDConnectConfiguration.java | Added cibaAuthenticationRequest field to integrate CIBA configuration into OIDC settings |
| OAuthConsumerAppToApiModel.java | Implemented conversion from OAuthConsumerAppDTO to CIBA API model, parsing comma-separated notification channels |
| ApiModelToOAuthConsumerApp.java | Implemented conversion from CIBA API model to OAuthConsumerAppDTO, joining notification channels with commas |
| ApplicationManagementEndpointConstants.java | Added CIBA_GRANT_TYPE constant for identifying CIBA grant type |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+325
to
+341
| private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, | ||
| CIBAAuthenticationRequestConfiguration cibaAuthReq) { | ||
|
|
||
| if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { | ||
| consumerAppDTO.setCibaAuthReqExpiryTime(cibaAuthReq.getCibaAuthReqExpiryTime()); | ||
| List<String> cibaNotificationChannels = cibaAuthReq.getCibaNotificationChannels(); | ||
| StringBuilder sb = new StringBuilder(); | ||
| if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) { | ||
| cibaNotificationChannels.forEach(channel -> sb.append(channel).append(",")); | ||
| sb.deleteCharAt(sb.length() - 1); | ||
| consumerAppDTO.setCibaNotificationChannels(sb.toString()); | ||
| } else { | ||
| consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY); | ||
| } | ||
| consumerAppDTO.setCibaDefaultNotificationChannel(cibaAuthReq.getCibaDefaultNotificationChannel()); | ||
| } | ||
| } |
There was a problem hiding this comment.
The method lacks a null check for the cibaAuthReq parameter before accessing it. When cibaAuthReq is null, calling cibaAuthReq.getCibaAuthReqExpiryTime() on line 329 will throw a NullPointerException. All similar update methods in this class (e.g., updateSubjectTokenConfigurations, updateSubjectConfigurations, updateRequestObjectConfiguration) check if the configuration parameter is not null before accessing it. This method should follow the same pattern and add a null check at the beginning.
Comment on lines
+331
to
+335
| StringBuilder sb = new StringBuilder(); | ||
| if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) { | ||
| cibaNotificationChannels.forEach(channel -> sb.append(channel).append(",")); | ||
| sb.deleteCharAt(sb.length() - 1); | ||
| consumerAppDTO.setCibaNotificationChannels(sb.toString()); |
There was a problem hiding this comment.
Using StringBuilder with forEach and manual deletion of the trailing comma is inefficient and less readable. The codebase already has a cleaner pattern using String.join() for joining list elements with a delimiter (see ApplicationTemplateApiModelToTemplate.java:53). Replace this StringBuilder logic with String.join(",", cibaNotificationChannels) for better maintainability and consistency with the rest of the codebase.
Suggested change
| StringBuilder sb = new StringBuilder(); | |
| if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) { | |
| cibaNotificationChannels.forEach(channel -> sb.append(channel).append(",")); | |
| sb.deleteCharAt(sb.length() - 1); | |
| consumerAppDTO.setCibaNotificationChannels(sb.toString()); | |
| if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) { | |
| consumerAppDTO.setCibaNotificationChannels(String.join(",", cibaNotificationChannels)); |
Contributor
Author
There was a problem hiding this comment.
Valid suggestions
| private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, | ||
| CIBAAuthenticationRequestConfiguration cibaAuthReq) { | ||
|
|
||
| if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { |
There was a problem hiding this comment.
The getGrantTypes() method can return null (line 100), which would cause a NullPointerException when calling consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE) on line 328. Before checking if the grant types contain CIBA_GRANT_TYPE, verify that getGrantTypes() returns a non-null value, or handle the potential null case appropriately.
Suggested change
| if (consumerAppDTO.getGrantTypes().contains(CIBA_GRANT_TYPE)) { | |
| List<String> grantTypes = consumerAppDTO.getGrantTypes(); | |
| if (cibaAuthReq != null && grantTypes != null && grantTypes.contains(CIBA_GRANT_TYPE)) { |
sahandilshan
force-pushed
the
ciba
branch
3 times, most recently
from
6cdff68 to
137a755
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.java (1)
36-59:⚠️ Potential issue | 🟠 MajorFail fast (or explicitly handle) missing CibaAuthService.
cibaAuthServiceis pulled from OSGi and used without a null check. If it’s required, add a fail-fast check like the other services; if optional, document and guard its usage inServerApplicationMetadataService.🛠️ Suggested guard
STSAdminServiceInterface sTSAdminServiceInterface = ApplicationManagementServiceHolder.getStsAdminService(); CibaAuthServiceImpl cibaAuthService = ApplicationManagementServiceHolder.getCibaAuthService(); + if (cibaAuthService == null) { + throw new IllegalStateException("CibaAuthServiceImpl is not available from OSGi context."); + } + if (applicationManagementService == null) { throw new IllegalStateException("ApplicationManagementService is not available from OSGi context."); }🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.java` around lines 36 - 59, The static initializer obtains cibaAuthService via ApplicationManagementServiceHolder.getCibaAuthService() but does not null-check it; either make it fail-fast like the other services by adding a null check and throwing an IllegalStateException with a clear message referencing CibaAuthServiceImpl, or (if CibaAuthServiceImpl is optional) document that in ServerApplicationMetadataService and update ServerApplicationMetadataService’s constructor and code paths to guard against a null cibaAuthService before use; ensure changes reference the cibaAuthService variable and the ServerApplicationMetadataService constructor used when creating SERVICE.
🧹 Nitpick comments (3)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java (1)
20-53: Use deterministic ordering for CIBA notification channel metadata.
HashMapiteration order is nondeterministic; if this map feeds metadata, clients can see unstable ordering. UsingLinkedHashMapkeeps insertion order consistent (matching the grant type map).♻️ Suggested change
-import java.util.HashMap;- private static final Map<String, String> CIBA_NOTIFICATION_CHANNELS_NAMES = new HashMap<>(); + private static final Map<String, String> CIBA_NOTIFICATION_CHANNELS_NAMES = new LinkedHashMap<>();Also applies to: 294-309
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java` around lines 20 - 53, In ApplicationManagementConstants, the CIBA_NOTIFICATION_CHANNELS_NAMES is declared as a HashMap causing nondeterministic iteration order; change its declaration to a LinkedHashMap (like OAUTH_GRANT_TYPE_NAMES) to preserve insertion order and ensure stable metadata ordering, and make the same change for the other similar map declared later in this class (the second CIBA notification channel metadata map referenced in the file) so both use LinkedHashMap.components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.java (1)
140-144: Avoid casting CibaAuthService to impl type.Hard-casting the OSGi service to
CibaAuthServiceImplrisksClassCastExceptionif a different implementation or proxy is registered. The codebase inconsistently uses interface types for most service holders (e.g.,ApplicationManagementService,AuthorizedAPIManagementService); this holder should follow the same pattern. Store and return theCibaAuthServiceinterface type instead, and update callers accordingly. Verify that theCibaAuthServiceinterface exposes all required methods (e.g.,getSupportedNotificationChannels()) before applying the refactor.Also applies to: 270-273
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.java` around lines 140 - 144, The CibaAuthService holder is casting the OSGi service to CibaAuthServiceImpl which can cause ClassCastException; change the holder(s) (the static inner class currently defining SERVICE) to store and expose the CibaAuthService interface instead of CibaAuthServiceImpl, remove the hard cast when assigning from PrivilegedCarbonContext.getThreadLocalCarbonContext().getOSGiService(CibaAuthService.class, null), and update all callers to use the CibaAuthService interface (ensuring the interface declares needed methods such as getSupportedNotificationChannels()); apply the same change to the other analogous holder instance referenced in the file.components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java (1)
89-100: Add@deprecatedJavadoc tag to document the replacement.The
@Deprecatedannotation has no accompanying@deprecatedJavadoc tag indicating what to use instead. This leaves callers without migration guidance.♻️ Suggested improvement
+ /** + * `@deprecated` Use {`@link` `#ServerApplicationMetadataService`(ApplicationManagementService, + * SAMLSSOConfigServiceImpl, OAuthAdminServiceImpl, STSAdminServiceInterface, CibaAuthServiceImpl)} instead. + */ `@Deprecated` public ServerApplicationMetadataService(ApplicationManagementService applicationManagementService,🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java` around lines 89 - 100, Add a Javadoc `@deprecated` tag to the deprecated constructor ServerApplicationMetadataService(ApplicationManagementService, SAMLSSOConfigServiceImpl, OAuthAdminServiceImpl, STSAdminServiceInterface) that documents the replacement API to use (for example the newer constructor or factory method) and a short migration hint; update the Javadoc above that constructor to include "@deprecated Use <replacement constructor or method name> instead" and optionally mention why and what dependencies to pass (e.g., Ciba via ApplicationManagementServiceHolder.getCibaAuthService()) so callers know how to migrate.
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java`:
- Around line 237-251: getCibaNotificationChannels currently splits the comma
string as-is which keeps whitespace and empty tokens; update
getCibaNotificationChannels to split the string, trim each token, filter out
blank/empty tokens (e.g., using StringUtils.isBlank or .trim() check), and
collect the remaining tokens into the List<String> returned to ensure
buildCIBAAuthenticationRequestConfiguration receives only valid, de-blanked
channel names from OAuthConsumerAppDTO.getCibaNotificationChannels().
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java`:
- Around line 326-335: The code calls
cibaAuthService.getSupportedNotificationChannels() without checking for null;
update ServerApplicationMetadataService to retrieve cibaAuthService via
ApplicationManagementServiceHolder.getCibaAuthService(), null-check it, and if
null throw the same explicit exception used elsewhere (e.g., as in
getWSTrustMetadata()/WSTrustInboundFunctions) instead of dereferencing; ensure
the exception type/message aligns with existing patterns and only proceed to
call getSupportedNotificationChannels() and populate
oidcMetaData.cibaMetadata(cibaMetadata) when cibaAuthService is non-null.
In `@pom.xml`:
- Around line 356-361: The POM uses an unavailable property value
identity.inbound.oauth2.version (7.4.10) causing Maven resolution failures;
update the property identity.inbound.oauth2.version to a published release
(choose 7.0.424 or 7.2.3) and ensure all dependencies that reference it—e.g.,
org.wso2.carbon.identity.inbound.auth.oauth2:org.wso2.carbon.identity.oauth.ciba
and the related oauth, common, dcr, rar artifacts—use that new property value so
Maven can resolve those modules.
---
Outside diff comments:
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.java`:
- Around line 36-59: The static initializer obtains cibaAuthService via
ApplicationManagementServiceHolder.getCibaAuthService() but does not null-check
it; either make it fail-fast like the other services by adding a null check and
throwing an IllegalStateException with a clear message referencing
CibaAuthServiceImpl, or (if CibaAuthServiceImpl is optional) document that in
ServerApplicationMetadataService and update ServerApplicationMetadataService’s
constructor and code paths to guard against a null cibaAuthService before use;
ensure changes reference the cibaAuthService variable and the
ServerApplicationMetadataService constructor used when creating SERVICE.
---
Nitpick comments:
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java`:
- Around line 20-53: In ApplicationManagementConstants, the
CIBA_NOTIFICATION_CHANNELS_NAMES is declared as a HashMap causing
nondeterministic iteration order; change its declaration to a LinkedHashMap
(like OAUTH_GRANT_TYPE_NAMES) to preserve insertion order and ensure stable
metadata ordering, and make the same change for the other similar map declared
later in this class (the second CIBA notification channel metadata map
referenced in the file) so both use LinkedHashMap.
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.java`:
- Around line 140-144: The CibaAuthService holder is casting the OSGi service to
CibaAuthServiceImpl which can cause ClassCastException; change the holder(s)
(the static inner class currently defining SERVICE) to store and expose the
CibaAuthService interface instead of CibaAuthServiceImpl, remove the hard cast
when assigning from
PrivilegedCarbonContext.getThreadLocalCarbonContext().getOSGiService(CibaAuthService.class,
null), and update all callers to use the CibaAuthService interface (ensuring the
interface declares needed methods such as getSupportedNotificationChannels());
apply the same change to the other analogous holder instance referenced in the
file.
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java`:
- Around line 89-100: Add a Javadoc `@deprecated` tag to the deprecated
constructor ServerApplicationMetadataService(ApplicationManagementService,
SAMLSSOConfigServiceImpl, OAuthAdminServiceImpl, STSAdminServiceInterface) that
documents the replacement API to use (for example the newer constructor or
factory method) and a short migration hint; update the Javadoc above that
constructor to include "@deprecated Use <replacement constructor or method name>
instead" and optionally mention why and what dependencies to pass (e.g., Ciba
via ApplicationManagementServiceHolder.getCibaAuthService()) so callers know how
to migrate.
Comment on lines
+237
to
+251
| private CIBAAuthenticationRequestConfiguration buildCIBAAuthenticationRequestConfiguration(OAuthConsumerAppDTO oAuthConsumerAppDTO) { | ||
|
|
||
| return new CIBAAuthenticationRequestConfiguration() | ||
| .cibaAuthReqExpiryTime(oAuthConsumerAppDTO.getCibaAuthReqExpiryTime()) | ||
| .cibaNotificationChannels(getCibaNotificationChannels(oAuthConsumerAppDTO)); | ||
| } | ||
|
|
||
| private List<String> getCibaNotificationChannels(OAuthConsumerAppDTO oAuthConsumerAppDTO) { | ||
|
|
||
| if (StringUtils.isBlank(oAuthConsumerAppDTO.getCibaNotificationChannels())) { | ||
| return Collections.emptyList(); | ||
| } else { | ||
| return Arrays.asList(oAuthConsumerAppDTO.getCibaNotificationChannels().split(",")); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Trim and de-blank CIBA notification channels after splitting.
split(",") preserves whitespace/empty tokens (e.g., "email, sms", "email,,sms"), which can leak invalid channel values to the API model.
🛠️ Suggested fix
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.stream.Collectors;- return Arrays.asList(oAuthConsumerAppDTO.getCibaNotificationChannels().split(","));
+ return Arrays.stream(oAuthConsumerAppDTO.getCibaNotificationChannels().split(","))
+ .map(String::trim)
+ .filter(StringUtils::isNotBlank)
+ .collect(Collectors.toList());🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java`
around lines 237 - 251, getCibaNotificationChannels currently splits the comma
string as-is which keeps whitespace and empty tokens; update
getCibaNotificationChannels to split the string, trim each token, filter out
blank/empty tokens (e.g., using StringUtils.isBlank or .trim() check), and
collect the remaining tokens into the List<String> returned to ensure
buildCIBAAuthenticationRequestConfiguration receives only valid, de-blanked
channel names from OAuthConsumerAppDTO.getCibaNotificationChannels().
Comment on lines
+326
to
+335
| List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels(); | ||
| CIBAMetadata cibaMetadata = new CIBAMetadata(); | ||
| for (String notificationChannel : supportedNotificationChannels) { | ||
| CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel(); | ||
| ciBANotificationChannel.setName(notificationChannel); | ||
| ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames().getOrDefault(notificationChannel, | ||
| notificationChannel)); | ||
| cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel); | ||
| } | ||
| oidcMetaData.cibaMetadata(cibaMetadata); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and examine the context around lines 326-335 and 349
fd -n "ServerApplicationMetadataService.java" --type f | head -5Repository: wso2/identity-api-server
Length of output: 298
🏁 Script executed:
# Search for CibaAuthService interface/implementation
rg -l "CibaAuthService" --type java | head -10Repository: wso2/identity-api-server
Length of output: 837
🏁 Script executed:
# Check ApplicationManagementServiceHolder and getCibaAuthService() method
rg -n "getCibaAuthService" --type java -A 5 -B 2Repository: wso2/identity-api-server
Length of output: 7212
🏁 Script executed:
# Search for sTSAdminServiceInterface null check handling
rg -n "sTSAdminServiceInterface" --type java -A 3 -B 3Repository: wso2/identity-api-server
Length of output: 17728
🏁 Script executed:
# Find getSupportedNotificationChannels method definition
rg -n "getSupportedNotificationChannels" --type java -A 8 -B 2Repository: wso2/identity-api-server
Length of output: 6714
🏁 Script executed:
# Find CibaAuthServiceHolder
rg -l "class CibaAuthServiceHolder" --type javaRepository: wso2/identity-api-server
Length of output: 323
🏁 Script executed:
# Check CibaAuthServiceHolder.SERVICE initialization
rg "CibaAuthServiceHolder" --type java -A 10 -B 5 | head -100Repository: wso2/identity-api-server
Length of output: 6939
🏁 Script executed:
# Examine the full method at line 326-335 with more context
sed -n '315,360p' "$(find . -name ServerApplicationMetadataService.java -path '*/core/*' | head -1)"Repository: wso2/identity-api-server
Length of output: 2532
🏁 Script executed:
# Check if getSupportedNotificationChannels can return null
rg "getSupportedNotificationChannels" --type java -B 5 -A 10Repository: wso2/identity-api-server
Length of output: 9315
🏁 Script executed:
# Verify CibaAuthService implementation to check if getSupportedNotificationChannels can return null
fd -type f "CibaAuthService.java" | head -5Repository: wso2/identity-api-server
Length of output: 238
🏁 Script executed:
# Get the full method signature and implementation of getSupportedNotificationChannels
rg -l "CibaAuthService" --type java | xargs grep -l "getSupportedNotificationChannels" | head -3Repository: wso2/identity-api-server
Length of output: 318
🏁 Script executed:
# Check CibaAuthService interface/impl for `@Nullable` or javadoc on getSupportedNotificationChannels
rg "interface CibaAuthService|class CibaAuthServiceImpl" --type java -A 50 | grep -A 30 "getSupportedNotificationChannels"Repository: wso2/identity-api-server
Length of output: 50
🏁 Script executed:
# Find CibaAuthService files
find . -name "*CibaAuthService*" -type f | head -10Repository: wso2/identity-api-server
Length of output: 50
🏁 Script executed:
# Search for CibaAuthService class definition
rg "class CibaAuthServiceImpl|interface CibaAuthService" -A 30Repository: wso2/identity-api-server
Length of output: 50
🏁 Script executed:
# Verify that getOSGiService can return null by checking PrivilegedCarbonContext usage
rg "getOSGiService" -A 2 -B 2 | head -50Repository: wso2/identity-api-server
Length of output: 11884
🏁 Script executed:
# Check if there are other null-checks for optional OSGi services in the codebase
rg "ApplicationManagementServiceHolder\.get.*Service\(\)" -B 2 -A 5 | head -80Repository: wso2/identity-api-server
Length of output: 23061
🏁 Script executed:
# Verify the exact line 326 and surrounding context in the current code
sed -n '320,340p' "$(find . -name ServerApplicationMetadataService.java -path '*/core/*' | head -1)"Repository: wso2/identity-api-server
Length of output: 1176
Null-dereference: cibaAuthService can be null when CIBA bundle is not deployed.
ApplicationManagementServiceHolder.getCibaAuthService() retrieves the service via PrivilegedCarbonContext.getOSGiService(...), which returns null when the CIBA bundle is unavailable. Line 326 calls cibaAuthService.getSupportedNotificationChannels() without guarding against null, causing NullPointerException when CIBA is not deployed.
However, based on the established pattern in the codebase (see WSTrustInboundFunctions.java and getWSTrustMetadata() at line 349), optional OSGi services should be null-checked and fail explicitly via exception, not silently skipped. Align the fix with this pattern:
🐛 Proposed fix — guard with exception
- List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels();
- CIBAMetadata cibaMetadata = new CIBAMetadata();
- for (String notificationChannel : supportedNotificationChannels) {
- CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel();
- ciBANotificationChannel.setName(notificationChannel);
- ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames().getOrDefault(notificationChannel,
- notificationChannel));
- cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel);
- }
- oidcMetaData.cibaMetadata(cibaMetadata);
+ if (cibaAuthService != null) {
+ List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels();
+ if (supportedNotificationChannels != null) {
+ CIBAMetadata cibaMetadata = new CIBAMetadata();
+ for (String notificationChannel : supportedNotificationChannels) {
+ CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel();
+ ciBANotificationChannel.setName(notificationChannel);
+ ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames()
+ .getOrDefault(notificationChannel, notificationChannel));
+ cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel);
+ }
+ oidcMetaData.cibaMetadata(cibaMetadata);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels(); | |
| CIBAMetadata cibaMetadata = new CIBAMetadata(); | |
| for (String notificationChannel : supportedNotificationChannels) { | |
| CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel(); | |
| ciBANotificationChannel.setName(notificationChannel); | |
| ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames().getOrDefault(notificationChannel, | |
| notificationChannel)); | |
| cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel); | |
| } | |
| oidcMetaData.cibaMetadata(cibaMetadata); | |
| if (cibaAuthService != null) { | |
| List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels(); | |
| if (supportedNotificationChannels != null) { | |
| CIBAMetadata cibaMetadata = new CIBAMetadata(); | |
| for (String notificationChannel : supportedNotificationChannels) { | |
| CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel(); | |
| ciBANotificationChannel.setName(notificationChannel); | |
| ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames() | |
| .getOrDefault(notificationChannel, notificationChannel)); | |
| cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel); | |
| } | |
| oidcMetaData.cibaMetadata(cibaMetadata); | |
| } | |
| } |
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java`
around lines 326 - 335, The code calls
cibaAuthService.getSupportedNotificationChannels() without checking for null;
update ServerApplicationMetadataService to retrieve cibaAuthService via
ApplicationManagementServiceHolder.getCibaAuthService(), null-check it, and if
null throw the same explicit exception used elsewhere (e.g., as in
getWSTrustMetadata()/WSTrustInboundFunctions) instead of dereferencing; ensure
the exception type/message aligns with existing patterns and only proceed to
call getSupportedNotificationChannels() and populate
oidcMetaData.cibaMetadata(cibaMetadata) when cibaAuthService is non-null.
Comment on lines
+356
to
+361
| <dependency> | ||
| <groupId>org.wso2.carbon.identity.inbound.auth.oauth2</groupId> | ||
| <artifactId>org.wso2.carbon.identity.oauth.ciba</artifactId> | ||
| <scope>provided</scope> | ||
| <version>${identity.inbound.oauth2.version}</version> | ||
| </dependency> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the pom.xml structure around the mentioned lines
fd pom.xml | head -5Repository: wso2/identity-api-server
Length of output: 715
🌐 Web query:
org.wso2.carbon.identity.oauth.ciba version 7.4.10 WSO2 Maven repository
💡 Result:
org.wso2.carbon.identity.inbound.auth.oauth2:org.wso2.carbon.identity.oauth.ciba does not have a 7.4.10 release published in the public Maven repositories (the published versions are in the 7.0.x line, e.g., 7.0.4xx/7.0.2xx, etc.).[1][2]
To consume the artifact from WSO2’s Maven repo, add WSO2 Public and then use an existing published version:
<repositories>
<repository>
<id>wso2-public</id>
<url>https://maven.wso2.org/nexus/content/groups/wso2-public/</url>
</repository>
</repositories>
<dependency>
<groupId>org.wso2.carbon.identity.inbound.auth.oauth2</groupId>
<artifactId>org.wso2.carbon.identity.oauth.ciba</artifactId>
<version><!-- use a published 7.0.x version --></version>
</dependency>[1] https://mvnrepository.com/artifact/org.wso2.carbon.identity.inbound.auth.oauth2/org.wso2.carbon.identity.oauth.ciba?sort=date
[2] https://mvnrepository.com/artifact/org.wso2.carbon.identity.inbound.auth.oauth2/org.wso2.carbon.identity.oauth.ciba
[3] https://stackoverflow.com/questions/32205269/org-wso2-carbon-identity-oauth-stub-artifact-not-found
🌐 Web query:
identity.inbound.oauth2 version 7.4.10 breaking changes WSO2
💡 Result:
I couldn’t find any WSO2 artifact/release named identity.inbound.oauth2 (or org.wso2.carbon.identity.inbound.auth.oauth2:org.wso2.carbon.identity.oauth) version 7.4.10.
What I can verify from WSO2’s public release streams:
- The
identity-inbound-auth-oauthGitHub repo has tags likev7.2.0–v7.2.3andv7.0.423–v7.0.427(Feb 2026), but nov7.4.10tag. (github.com) - The Maven coordinates most people mean by “inbound oauth2” (
org.wso2.carbon.identity.inbound.auth.oauth2:org.wso2.carbon.identity.oauth) show latest lines such as 7.0.424 (Feb 6, 2026), not 7.4.x. (mvnrepository.com)
Potential “breaking” behavior change you might be referring to (OAuth2 inbound area)
WSO2 published a security fix that stops issuing access tokens for disabled IdPs when JWT Bearer or SAML2 Bearer grant types are used; they explicitly warn this can cause “functional disruptions” if you relied on the old behavior. (security.docs.wso2.com)
What I need to answer precisely
Can you paste the exact Maven GAV (groupId:artifactId) or the repo name you’re upgrading, and where you saw 7.4.10? (It may be a different project, or a different WSO2 component name than the common inbound OAuth2 ones.)
Citations:
- 1: https://github.com/wso2-extensions/identity-inbound-auth-oauth/tags
- 2: https://mvnrepository.com/artifact/org.wso2.carbon.identity.inbound.auth.oauth2/org.wso2.carbon.identity.oauth?utm_source=openai
- 3: https://security.docs.wso2.com/en/latest/security-announcements/security-advisories/2025/WSO2-2025-4010/?utm_source=openai
🏁 Script executed:
# Check if we can see pom.xml content around lines 356-361 and 1034
fd pom.xml -x cat {} | head -400 | tail -100Repository: wso2/identity-api-server
Length of output: 5339
🏁 Script executed:
# Find the root pom.xml in this repository
git ls-files | grep "^pom.xml$"Repository: wso2/identity-api-server
Length of output: 75
🏁 Script executed:
# If root pom.xml exists, examine lines around 356-361 and 1034
cat -n pom.xml 2>/dev/null | sed -n '350,365p; 1030,1040p'Repository: wso2/identity-api-server
Length of output: 2021
🏁 Script executed:
# Search for identity.inbound.oauth2.version property
rg "identity.inbound.oauth2.version" -A 2 -B 2Repository: wso2/identity-api-server
Length of output: 2108
Correct identity.inbound.oauth2.version to an available release—7.4.10 does not exist.
The pom.xml sets identity.inbound.oauth2.version to 7.4.10, but this version is not published in WSO2's public Maven repositories. The org.wso2.carbon.identity.inbound.auth.oauth2:org.wso2.carbon.identity.oauth.ciba artifact and related OAuth2 modules only have releases in the 7.0.x (latest 7.0.424) and 7.2.x (latest 7.2.3) lines. This will cause immediate Maven resolution failures for all five artifacts using this property (oauth, ciba, common, dcr, rar). Use 7.0.424 or 7.2.3 instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pom.xml` around lines 356 - 361, The POM uses an unavailable property value
identity.inbound.oauth2.version (7.4.10) causing Maven resolution failures;
update the property identity.inbound.oauth2.version to a published release
(choose 7.0.424 or 7.2.3) and ensure all dependencies that reference it—e.g.,
org.wso2.carbon.identity.inbound.auth.oauth2:org.wso2.carbon.identity.oauth.ciba
and the related oauth, common, dcr, rar artifacts—use that new property value so
Maven can resolve those modules.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.java (1)
42-59:⚠️ Potential issue | 🟡 MinorAdd a null check (or an explanatory comment) for
cibaAuthService.Every other service fetched in this static initializer either has an explicit null check that throws
IllegalStateException, or a comment explaining why null is acceptable (e.g., line 56 forsTSAdminServiceInterface).cibaAuthServicehas neither. If the CIBA module is not deployed (consistent with the PR note that this merges after the upstream CIBA PR),getCibaAuthService()will returnnull, and passing it silently to the constructor risks an NPE downstream insideServerApplicationMetadataService.Either document that null is intentionally allowed (matching the STS pattern), or add a guard:
🛡️ Option A — CIBA is optional (matches STS pattern)
// Null check for STSAdminServiceInterface is not mandatory as per the previous implementation. + + // Null check for CibaAuthServiceImpl is not mandatory; CIBA support is optional and the + // service may not be available if the CIBA module is not deployed. SERVICE = new ServerApplicationMetadataService(applicationManagementService, samlSSOConfigService, oAuthAdminService, sTSAdminServiceInterface, cibaAuthService);🛡️ Option B — CIBA is required (fail-fast)
+ if (cibaAuthService == null) { + throw new IllegalStateException("CibaAuthServiceImpl is not available from OSGi context."); + } + SERVICE = new ServerApplicationMetadataService(applicationManagementService, samlSSOConfigService, oAuthAdminService, sTSAdminServiceInterface, cibaAuthService);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.
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.java` around lines 42 - 59, The static initializer currently fetches cibaAuthService via getCibaAuthService() but lacks the null handling present for other services; either add an explicit null check that throws IllegalStateException (fail-fast) or add a short explanatory comment stating that cibaAuthService may be null because CIBA is optional (matching the sTSAdminServiceInterface pattern). Locate the reference to cibaAuthService and getCibaAuthService() in the factory static block and implement one of the two options before constructing new ServerApplicationMetadataService(...), ensuring the constructor invocation (ServerApplicationMetadataService) and the SERVICE assignment reflect the chosen behavior.
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java (1)
52-52: ConsiderLinkedHashMapto match the established pattern and ensure predictable iteration order.
OAUTH_GRANT_TYPE_NAMES(line 51) usesLinkedHashMapto preserve insertion order. UsingHashMapforCIBA_NOTIFICATION_CHANNELS_NAMESbreaks that consistency and gives non-deterministic iteration order, which can affect UIs that render the channels as an ordered list.♻️ Proposed fix
- private static final Map<String, String> CIBA_NOTIFICATION_CHANNELS_NAMES = new HashMap<>(); + private static final Map<String, String> CIBA_NOTIFICATION_CHANNELS_NAMES = new LinkedHashMap<>();The
java.util.LinkedHashMapimport is already present on line 28.🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java` at line 52, Replace the CIBA_NOTIFICATION_CHANNELS_NAMES declaration to use LinkedHashMap instead of HashMap so its iteration order is deterministic and consistent with OAUTH_GRANT_TYPE_NAMES; locate the static field CIBA_NOTIFICATION_CHANNELS_NAMES in ApplicationManagementConstants and change its instantiation to new LinkedHashMap<>() (the LinkedHashMap import is already present).
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml`:
- Around line 5019-5034: Add a minimum constraint to the
CIBAAuthenticationRequestConfiguration.cibaAuthReqExpiryTime schema so
non-positive values fail validation: update the cibaAuthReqExpiryTime property
(in the CIBAAuthenticationRequestConfiguration object) to include minimum: 1
(keeping type: integer and format: int64) so values less than 1 are rejected by
the OpenAPI schema validation.
---
Outside diff comments:
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.java`:
- Around line 42-59: The static initializer currently fetches cibaAuthService
via getCibaAuthService() but lacks the null handling present for other services;
either add an explicit null check that throws IllegalStateException (fail-fast)
or add a short explanatory comment stating that cibaAuthService may be null
because CIBA is optional (matching the sTSAdminServiceInterface pattern). Locate
the reference to cibaAuthService and getCibaAuthService() in the factory static
block and implement one of the two options before constructing new
ServerApplicationMetadataService(...), ensuring the constructor invocation
(ServerApplicationMetadataService) and the SERVICE assignment reflect the chosen
behavior.
---
Duplicate comments:
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java`:
- Around line 237-252: getCibaNotificationChannels currently splits the raw CSV
from OAuthConsumerAppDTO.getCibaNotificationChannels() without trimming or
filtering, which allows whitespace or empty tokens through; update
getCibaNotificationChannels to split on comma, trim each token, filter out
empty/blank tokens (and return Collections.emptyList() if none remain) before
returning the list so CIBAAuthenticationRequestConfiguration receives only valid
channel names. Use a stream or loop to map(String::trim),
filter(StringUtils::isNotBlank) and collect to a List.
---
Nitpick comments:
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java`:
- Line 52: Replace the CIBA_NOTIFICATION_CHANNELS_NAMES declaration to use
LinkedHashMap instead of HashMap so its iteration order is deterministic and
consistent with OAUTH_GRANT_TYPE_NAMES; locate the static field
CIBA_NOTIFICATION_CHANNELS_NAMES in ApplicationManagementConstants and change
its instantiation to new LinkedHashMap<>() (the LinkedHashMap import is already
present).
Comment on lines
+5019
to
+5034
| CIBAAuthenticationRequestConfiguration: | ||
| type: object | ||
| properties: | ||
| cibaAuthReqExpiryTime: | ||
| type: integer | ||
| format: int64 | ||
| description: "CIBA authentication request expiry time in seconds." | ||
| example: 600 | ||
| cibaNotificationChannels: | ||
| type: array | ||
| items: | ||
| type: string | ||
| description: "List of allowed notification channels." | ||
| example: | ||
| - "email" | ||
| - "sms" |
Contributor
There was a problem hiding this comment.
Consider adding minimum: 1 to cibaAuthReqExpiryTime.
An expiry time of 0 or a negative value has no valid semantic for a CIBA auth request. Without a minimum constraint, invalid values will pass schema validation silently.
🛡️ Suggested fix
cibaAuthReqExpiryTime:
type: integer
format: int64
+ minimum: 1
description: "CIBA authentication request expiry time in seconds."
example: 600🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml`
around lines 5019 - 5034, Add a minimum constraint to the
CIBAAuthenticationRequestConfiguration.cibaAuthReqExpiryTime schema so
non-positive values fail validation: update the cibaAuthReqExpiryTime property
(in the CIBAAuthenticationRequestConfiguration object) to include minimum: 1
(keeping type: integer and format: int64) so values less than 1 are rejected by
the OpenAPI schema validation.
...o2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml
Outdated
Show resolved
Hide resolved
...src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/OIDCMetaData.java
Outdated
Show resolved
Hide resolved
...a/org/wso2/carbon/identity/api/server/application/management/v1/CIBANotificationChannel.java
Outdated
Show resolved
Hide resolved
...src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/CIBAMetadata.java
Outdated
Show resolved
Hide resolved
...on/identity/api/server/application/management/v1/CIBAAuthenticationRequestConfiguration.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (5)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java (1)
324-339:⚠️ Potential issue | 🔴 CriticalGuard against null grant types and missing CIBA config.
consumerAppDTO.getGrantTypes()can be null (when no grants are set) andcibaAuthReqcan be absent from the request, so this method can throw NPEs. Add early returns before dereferencing.🐛 Suggested fix
private void updateCIBAAuthenticationRequestConfigurations(OAuthConsumerAppDTO consumerAppDTO, CIBAAuthenticationRequestConfiguration cibaAuthReq) { - if (consumerAppDTO.getGrantTypes().contains(OAuthConstants.GrantTypes.CIBA)) { + if (cibaAuthReq == null || StringUtils.isBlank(consumerAppDTO.getGrantTypes()) + || !consumerAppDTO.getGrantTypes().contains(OAuthConstants.GrantTypes.CIBA)) { + return; + } + consumerAppDTO.setCibaAuthReqExpiryTime(cibaAuthReq.getAuthReqExpiryTime()); List<String> cibaNotificationChannels = cibaAuthReq.getNotificationChannels(); StringBuilder sb = new StringBuilder(); if (CollectionUtils.isNotEmpty(cibaNotificationChannels)) { cibaNotificationChannels.forEach(channel -> sb.append(channel).append(",")); sb.deleteCharAt(sb.length() - 1); consumerAppDTO.setCibaNotificationChannels(sb.toString()); } else { consumerAppDTO.setCibaNotificationChannels(StringUtils.EMPTY); } - } }#!/bin/bash # Verify grantTypes nullability and CIBA config availability in model/DTOs. rg -n "class OAuthConsumerAppDTO" --type java -C2 rg -n "String getGrantTypes" --type java -C2 rg -n "getCibaAuthenticationRequest" --type java -C2🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java` around lines 324 - 339, The updateCIBAAuthenticationRequestConfigurations method can NPE when consumerAppDTO.getGrantTypes() is null or when the passed cibaAuthReq is null or its getNotificationChannels() is null; add early-return guards at the start of updateCIBAAuthenticationRequestConfigurations to return if cibaAuthReq == null or consumerAppDTO.getGrantTypes() is null/empty, and when building the comma-separated string check that cibaAuthReq.getNotificationChannels() is non-null and non-empty before using StringBuilder (or avoid deleteCharAt by joining with a delimiter); reference method updateCIBAAuthenticationRequestConfigurations, OAuthConsumerAppDTO.getGrantTypes, CIBAAuthenticationRequestConfiguration.getNotificationChannels and OAuthConsumerAppDTO.setCibaNotificationChannels when making the changes.components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java (2)
325-334:⚠️ Potential issue | 🔴 CriticalNull-check
cibaAuthService(and channel list) to avoid NPEs when CIBA is absent.
cibaAuthServicecan be unavailable in OSGi, andgetSupportedNotificationChannels()may return null; both paths currently NPE.🐛 Suggested fix
- List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels(); - CIBAMetadata cibaMetadata = new CIBAMetadata(); - for (String notificationChannel : supportedNotificationChannels) { - CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel(); - ciBANotificationChannel.setName(notificationChannel); - ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames().getOrDefault(notificationChannel, - notificationChannel)); - cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel); - } - oidcMetaData.cibaMetadata(cibaMetadata); + if (cibaAuthService != null) { + List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels(); + if (supportedNotificationChannels != null) { + CIBAMetadata cibaMetadata = new CIBAMetadata(); + for (String notificationChannel : supportedNotificationChannels) { + CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel(); + ciBANotificationChannel.setName(notificationChannel); + ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames() + .getOrDefault(notificationChannel, notificationChannel)); + cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel); + } + oidcMetaData.cibaMetadata(cibaMetadata); + } + }#!/bin/bash # Inspect holder/service nullability and supported channel API. rg -n "getCibaAuthService" --type java -C2 rg -n "class ApplicationManagementServiceHolder" --type java -C2 rg -n "getSupportedNotificationChannels" --type java -C2🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java` around lines 325 - 334, Check for null before using the CIBA service and its channel list: ensure cibaAuthService is not null before calling cibaAuthService.getSupportedNotificationChannels(), and null-check the returned supportedNotificationChannels (or treat null as empty list) before iterating; if null/absent, skip building CIBAMetadata or leave oidcMetaData.cibaMetadata(null) as appropriate. Update the block that references cibaAuthService, getSupportedNotificationChannels, CIBAMetadata, and oidcMetaData to guard against NPEs by adding the checks and only populating cibaMetadata when channels are present.
26-31:⚠️ Potential issue | 🔴 CriticalAdd null checks before using cibaAuthService and its return value to prevent NPE.
Line 325-327 call
cibaAuthService.getSupportedNotificationChannels()and iterate the result without null guards. If the CIBA service is unavailable or the method returns null, this will throw NullPointerException. Add defensive checks:List<String> supportedNotificationChannels = cibaAuthService != null ? cibaAuthService.getSupportedNotificationChannels() : null; CIBAMetadata cibaMetadata = new CIBAMetadata(); if (supportedNotificationChannels != null) { for (String notificationChannel : supportedNotificationChannels) { // existing loop body } }The wiring pattern itself is correct and consistent with existing dependency injection practices.
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java` around lines 26 - 31, The code calls cibaAuthService.getSupportedNotificationChannels() and iterates its result without null checks; update ServerApplicationMetadataService to first guard that cibaAuthService is not null and that getSupportedNotificationChannels() returns a non-null, non-empty List before iterating (e.g., assign supportedNotificationChannels from cibaAuthService != null ? cibaAuthService.getSupportedNotificationChannels() : null and only loop when supportedNotificationChannels != null), ensuring CIBAMetadata population logic remains unchanged and avoiding NPEs when cibaAuthService or its return value is null.components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java (1)
237-251:⚠️ Potential issue | 🟡 MinorTrim and drop blank notification channels after splitting.
Comma-splitting can retain whitespace or empty tokens (e.g.,
"email, sms"or"email,,sms"), which leaks invalid channel names.🛠️ Suggested fix
import java.util.Collections; import java.util.List; +import java.util.stream.Collectors;- } else { - return Arrays.asList(oAuthConsumerAppDTO.getCibaNotificationChannels().split(",")); - } + } else { + return Arrays.stream(oAuthConsumerAppDTO.getCibaNotificationChannels().split(",")) + .map(String::trim) + .filter(StringUtils::isNotBlank) + .collect(Collectors.toList()); + }🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java` around lines 237 - 251, The getCibaNotificationChannels method can return values with surrounding whitespace or empty tokens after splitting; update getCibaNotificationChannels(OAuthConsumerAppDTO) to split the CIBA notification string, trim each token, filter out empty/blank strings, and collect to a List<String> (returning Collections.emptyList() when result is empty); this ensures CIBAAuthenticationRequestConfiguration built by buildCIBAAuthenticationRequestConfiguration(...) receives only valid, trimmed channel names.components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml (1)
5019-5034:⚠️ Potential issue | 🟡 MinorAdd a minimum to
authReqExpiryTimeto reject non‑positive values.Zero or negative expiry times aren’t meaningful for auth requests, so the schema should enforce a lower bound.
🛡️ Suggested fix
authReqExpiryTime: type: integer format: int64 + minimum: 1 description: "CIBA authentication request expiry time in seconds." example: 600🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml` around lines 5019 - 5034, Add a lower bound to the CIBAAuthenticationRequestConfiguration schema by updating the authReqExpiryTime property to reject non‑positive values: add a "minimum" constraint (e.g., minimum: 1) under the authReqExpiryTime definition so zero and negative integers are invalid during validation.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/CIBAAuthenticationRequestConfiguration.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/CIBAMetadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/CIBANotificationChannel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/OIDCMetaData.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/OpenIDConnectConfiguration.javais excluded by!**/gen/**
📒 Files selected for processing (10)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/pom.xmlcomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/pom.xmlcomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yamlpom.xml
🚧 Files skipped from review as they are similar to previous changes (4)
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/pom.xml
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.java
- pom.xml
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java`:
- Around line 324-339: The updateCIBAAuthenticationRequestConfigurations method
can NPE when consumerAppDTO.getGrantTypes() is null or when the passed
cibaAuthReq is null or its getNotificationChannels() is null; add early-return
guards at the start of updateCIBAAuthenticationRequestConfigurations to return
if cibaAuthReq == null or consumerAppDTO.getGrantTypes() is null/empty, and when
building the comma-separated string check that
cibaAuthReq.getNotificationChannels() is non-null and non-empty before using
StringBuilder (or avoid deleteCharAt by joining with a delimiter); reference
method updateCIBAAuthenticationRequestConfigurations,
OAuthConsumerAppDTO.getGrantTypes,
CIBAAuthenticationRequestConfiguration.getNotificationChannels and
OAuthConsumerAppDTO.setCibaNotificationChannels when making the changes.
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java`:
- Around line 237-251: The getCibaNotificationChannels method can return values
with surrounding whitespace or empty tokens after splitting; update
getCibaNotificationChannels(OAuthConsumerAppDTO) to split the CIBA notification
string, trim each token, filter out empty/blank strings, and collect to a
List<String> (returning Collections.emptyList() when result is empty); this
ensures CIBAAuthenticationRequestConfiguration built by
buildCIBAAuthenticationRequestConfiguration(...) receives only valid, trimmed
channel names.
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java`:
- Around line 325-334: Check for null before using the CIBA service and its
channel list: ensure cibaAuthService is not null before calling
cibaAuthService.getSupportedNotificationChannels(), and null-check the returned
supportedNotificationChannels (or treat null as empty list) before iterating; if
null/absent, skip building CIBAMetadata or leave oidcMetaData.cibaMetadata(null)
as appropriate. Update the block that references cibaAuthService,
getSupportedNotificationChannels, CIBAMetadata, and oidcMetaData to guard
against NPEs by adding the checks and only populating cibaMetadata when channels
are present.
- Around line 26-31: The code calls
cibaAuthService.getSupportedNotificationChannels() and iterates its result
without null checks; update ServerApplicationMetadataService to first guard that
cibaAuthService is not null and that getSupportedNotificationChannels() returns
a non-null, non-empty List before iterating (e.g., assign
supportedNotificationChannels from cibaAuthService != null ?
cibaAuthService.getSupportedNotificationChannels() : null and only loop when
supportedNotificationChannels != null), ensuring CIBAMetadata population logic
remains unchanged and avoiding NPEs when cibaAuthService or its return value is
null.
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml`:
- Around line 5019-5034: Add a lower bound to the
CIBAAuthenticationRequestConfiguration schema by updating the authReqExpiryTime
property to reject non‑positive values: add a "minimum" constraint (e.g.,
minimum: 1) under the authReqExpiryTime definition so zero and negative integers
are invalid during validation.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java (1)
325-334:⚠️ Potential issue | 🔴 CriticalNPE:
cibaAuthServiceis not null-guarded before use.
ApplicationManagementServiceHolder.getCibaAuthService()resolves viaPrivilegedCarbonContext.getOSGiService(...), which returnsnullwhen the CIBA bundle is not deployed. Line 325 dereferences it unconditionally, crashinggetOIDCMetadata()for every caller when CIBA is unavailable.supportedNotificationChannelsis also not null-checked before iteration at line 327.The established codebase pattern for optional OSGi services (see
getWSTrustMetadata()at line 348) is to null-check and either gracefully skip or throw an explicit error.🐛 Proposed fix — guard both dereferences
- List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels(); - CIBAMetadata cibaMetadata = new CIBAMetadata(); - for (String notificationChannel : supportedNotificationChannels) { - CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel(); - ciBANotificationChannel.setName(notificationChannel); - ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames().getOrDefault(notificationChannel, - notificationChannel)); - cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel); - } - oidcMetaData.cibaMetadata(cibaMetadata); + if (cibaAuthService != null) { + List<String> supportedNotificationChannels = cibaAuthService.getSupportedNotificationChannels(); + if (supportedNotificationChannels != null) { + CIBAMetadata cibaMetadata = new CIBAMetadata(); + for (String notificationChannel : supportedNotificationChannels) { + CIBANotificationChannel ciBANotificationChannel = new CIBANotificationChannel(); + ciBANotificationChannel.setName(notificationChannel); + ciBANotificationChannel.setDisplayName(getCibaNotificationChannelNames() + .getOrDefault(notificationChannel, notificationChannel)); + cibaMetadata.addSupportedNotificationChannelsItem(ciBANotificationChannel); + } + oidcMetaData.cibaMetadata(cibaMetadata); + } + }🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java` around lines 325 - 334, getOIDCMetadata() dereferences ApplicationManagementServiceHolder.getCibaAuthService() and iterates supportedNotificationChannels without null checks, causing an NPE when the CIBA OSGi service is absent; update the block that builds CIBAMetadata to first call ApplicationManagementServiceHolder.getCibaAuthService() into a local cibaAuthService variable, null-check it (similar to getWSTrustMetadata()), and only when non-null call cibaAuthService.getSupportedNotificationChannels(), null-check supportedNotificationChannels before looping, and skip setting oidcMetaData.cibaMetadata(cibaMetadata) if service or channels are missing so callers won’t crash.
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java (1)
306-309: Consider returning an unmodifiable map fromgetCibaNotificationChannelNames().The getter exposes the raw mutable static
HashMapdirectly. Any caller invokinggetCibaNotificationChannelNames().put(...)or.remove(...)permanently corrupts the shared state. Wrapping the return value withCollections.unmodifiableMap()is a low-effort defensive guard.♻️ Proposed refactor
public static Map<String, String> getCibaNotificationChannelNames() { - return CIBA_NOTIFICATION_CHANNELS_NAMES; + return Collections.unmodifiableMap(CIBA_NOTIFICATION_CHANNELS_NAMES); }(The same applies to the pre-existing
getOAuthGrantTypeNames(), but that is outside the scope of this PR.)🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java` around lines 306 - 309, The getter getCibaNotificationChannelNames currently returns the mutable static map CIBA_NOTIFICATION_CHANNELS_NAMES directly; change it to return an unmodifiable view by wrapping the map with Collections.unmodifiableMap(CIBA_NOTIFICATION_CHANNELS_NAMES) so callers cannot mutate shared state, and ensure java.util.Collections is imported if missing.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/CIBAAuthenticationRequestConfiguration.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/CIBAMetadata.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/CIBANotificationChannel.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/OIDCMetaData.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/gen/java/org/wso2/carbon/identity/api/server/application/management/v1/OpenIDConnectConfiguration.javais excluded by!**/gen/**
📒 Files selected for processing (10)
components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/pom.xmlcomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/pom.xmlcomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.javacomponents/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yamlpom.xml
🚧 Files skipped from review as they are similar to previous changes (7)
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/pom.xml
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.java
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/resources/applications.yaml
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java
- pom.xml
- components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.java
🤖 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.application.management/org.wso2.carbon.identity.api.server.application.management.v1/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationMetadataService.java`:
- Around line 325-334: getOIDCMetadata() dereferences
ApplicationManagementServiceHolder.getCibaAuthService() and iterates
supportedNotificationChannels without null checks, causing an NPE when the CIBA
OSGi service is absent; update the block that builds CIBAMetadata to first call
ApplicationManagementServiceHolder.getCibaAuthService() into a local
cibaAuthService variable, null-check it (similar to getWSTrustMetadata()), and
only when non-null call cibaAuthService.getSupportedNotificationChannels(),
null-check supportedNotificationChannels before looping, and skip setting
oidcMetaData.cibaMetadata(cibaMetadata) if service or channels are missing so
callers won’t crash.
---
Nitpick comments:
In
`@components/org.wso2.carbon.identity.api.server.application.management/org.wso2.carbon.identity.api.server.application.management.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementConstants.java`:
- Around line 306-309: The getter getCibaNotificationChannelNames currently
returns the mutable static map CIBA_NOTIFICATION_CHANNELS_NAMES directly; change
it to return an unmodifiable view by wrapping the map with
Collections.unmodifiableMap(CIBA_NOTIFICATION_CHANNELS_NAMES) so callers cannot
mutate shared state, and ensure java.util.Collections is imported if missing.
|
PR builder started |
shashimalcse
previously approved these changes
Feb 23, 2026
|
PR builder completed |
Contributor
Author
|
Integration tests are failed because newly added ciba properties are not available in the |
SujanSanjula96
approved these changes
Feb 23, 2026
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
$subject
Related Issue(s)
**Note: Merge after: wso2-extensions/identity-inbound-auth-oauth#3030
Summary by CodeRabbit
New Features
Chores