Add FAPI 2.0 configuration models and connector utilities#1108
Open
VimukthiRajapaksha wants to merge 7 commits into
Open
Add FAPI 2.0 configuration models and connector utilities#1108VimukthiRajapaksha wants to merge 7 commits into
VimukthiRajapaksha wants to merge 7 commits into
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:
📝 WalkthroughWalkthroughThis pull request introduces tenant-aware FAPI (Financial-grade API) profile management across the identity API server. It adds FAPI profile selection during OIDC application configuration with validation against tenant-supported profiles, exposes allowed profiles in application metadata, provides REST endpoints for tenant FAPI configuration management, and ensures consistent model mapping between storage and API layers. The changes span both application management and configuration modules with centralized service access, error handling, and schema definitions. Sequence DiagramsequenceDiagram
participant Client
participant AppAPI as Application API
participant AppMgmt as ServerApplicationManagementService
participant MetadataSvc as ServerApplicationMetadataService
participant FapiSvc as FapiConfigMgtService
Client->>AppAPI: POST create app with OIDC (fapiProfile: "FAPI1_ADVANCED")
AppAPI->>AppMgmt: createApplication()
AppMgmt->>FapiSvc: getFapiConfig(tenant)
FapiSvc-->>AppMgmt: {supportedProfiles: ["FAPI1_ADVANCED", "FAPI2_SECURITY"]}
alt requested profile supported
AppMgmt-->>AppAPI: persist app with FAPI profile
AppAPI-->>Client: 201 Created
else unsupported
AppMgmt-->>AppAPI: 400 UNSUPPORTED_FAPI_PROFILE
AppAPI-->>Client: 400 error
end
Client->>AppAPI: GET /apps/{id}/oidc-metadata
AppAPI->>MetadataSvc: getOIDCMetadata()
MetadataSvc->>FapiSvc: getFapiConfig(tenant)
FapiSvc-->>MetadataSvc: {supportedProfiles: ["FAPI1_ADVANCED", "FAPI2_SECURITY"]}
MetadataSvc-->>AppAPI: {allowedFapiProfiles: ["FAPI1_ADVANCED", "FAPI2_SECURITY"]}
AppAPI-->>Client: 200 metadata
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
+299
to
+308
|
|
||
| /** | ||
| * Get FapiConfigMgtService. | ||
| * | ||
| * @return FapiConfigMgtService. | ||
| */ | ||
| public static FapiConfigMgtService getFapiConfigMgtService() { | ||
|
|
||
| return FapiConfigMgtServiceHolder.SERVICE; | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 1
Suggested change
| /** | |
| * Get FapiConfigMgtService. | |
| * | |
| * @return FapiConfigMgtService. | |
| */ | |
| public static FapiConfigMgtService getFapiConfigMgtService() { | |
| return FapiConfigMgtServiceHolder.SERVICE; | |
| } | |
| /** | |
| * Get FapiConfigMgtService. | |
| * | |
| * @return FapiConfigMgtService. | |
| */ | |
| public static FapiConfigMgtService getFapiConfigMgtService() { | |
| if (log.isDebugEnabled()) { | |
| log.debug("Retrieving FapiConfigMgtService from OSGi context."); | |
| } | |
| return FapiConfigMgtServiceHolder.SERVICE; | |
| } |
Comment on lines
818
to
+821
| if (applicationModel.getInboundProtocolConfiguration() != null && | ||
| applicationModel.getInboundProtocolConfiguration().getOidc() != null) { | ||
| validateCORSOrigins(applicationModel.getInboundProtocolConfiguration().getOidc().getAllowedOrigins()); | ||
| OpenIDConnectConfiguration oidcConfig = | ||
| applicationModel.getInboundProtocolConfiguration().getOidc(); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 2
Suggested change
| if (applicationModel.getInboundProtocolConfiguration() != null && | |
| applicationModel.getInboundProtocolConfiguration().getOidc() != null) { | |
| validateCORSOrigins(applicationModel.getInboundProtocolConfiguration().getOidc().getAllowedOrigins()); | |
| OpenIDConnectConfiguration oidcConfig = | |
| applicationModel.getInboundProtocolConfiguration().getOidc(); | |
| if (applicationModel.getInboundProtocolConfiguration() != null && | |
| applicationModel.getInboundProtocolConfiguration().getOidc() != null) { | |
| log.info("Validating OIDC configuration for new application"); | |
| OpenIDConnectConfiguration oidcConfig = |
Comment on lines
+2251
to
+2254
| .collect(Collectors.joining(", ")); | ||
| if (log.isDebugEnabled()) { | ||
| log.debug(String.format("Rejected fapiProfile '%s' for tenant '%s'. Supported: [%s]", | ||
| requestedProfileValue, tenantDomain, supportedProfilesStr)); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 3
Suggested change
| .collect(Collectors.joining(", ")); | |
| if (log.isDebugEnabled()) { | |
| log.debug(String.format("Rejected fapiProfile '%s' for tenant '%s'. Supported: [%s]", | |
| requestedProfileValue, tenantDomain, supportedProfilesStr)); | |
| if (log.isDebugEnabled()) { | |
| log.debug(String.format("Rejected fapiProfile '%s' for tenant '%s'. Supported: [%s]", | |
| requestedProfileValue, tenantDomain, supportedProfilesStr)); | |
| } | |
| log.warn(String.format("FAPI profile validation failed. Requested: '%s', Supported: [%s]", | |
| requestedProfileValue, supportedProfilesStr)); |
| return Collections.emptyList(); | ||
| } | ||
| String tenantDomain = ContextLoader.getTenantDomainFromContext(); | ||
| try { |
There was a problem hiding this comment.
Log Improvement Suggestion No: 4
Suggested change
| try { | |
| try { | |
| FapiConfig fapiConfig = fapiConfigMgtService.getFapiConfig(tenantDomain); | |
| if (LOG.isDebugEnabled()) { | |
| LOG.debug("Successfully retrieved FAPI config for tenant: " + tenantDomain); | |
| } |
Comment on lines
+580
to
+582
| .collect(Collectors.toList()); | ||
| } catch (FapiConfigMgtException e) { | ||
| throw handleServerError(ErrorMessage.ERROR_RETRIEVING_FAPI_METADATA, e); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 5
Suggested change
| .collect(Collectors.toList()); | |
| } catch (FapiConfigMgtException e) { | |
| throw handleServerError(ErrorMessage.ERROR_RETRIEVING_FAPI_METADATA, e); | |
| } catch (FapiConfigMgtException e) { | |
| LOG.error("Failed to retrieve FAPI config for tenant: " + tenantDomain + ". Error: " + e.getMessage()); | |
| throw handleServerError(ErrorMessage.ERROR_RETRIEVING_FAPI_METADATA, e); |
Comment on lines
93
to
+98
| consumerAppDTO.setFapiConformanceEnabled(oidcModel.getIsFAPIApplication()); | ||
| // Convert FAPI profile enum to String for storage. If no profile specified; default is set to FAPI1_ADVANCED. | ||
| if (oidcModel.getIsFAPIApplication()) { | ||
| consumerAppDTO.setFapiProfile(oidcModel.getFapiProfile() != null | ||
| ? oidcModel.getFapiProfile().toString() : FapiProfileEnum.FAPI1_ADVANCED.value()); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 6
Suggested change
| consumerAppDTO.setFapiConformanceEnabled(oidcModel.getIsFAPIApplication()); | |
| // Convert FAPI profile enum to String for storage. If no profile specified; default is set to FAPI1_ADVANCED. | |
| if (oidcModel.getIsFAPIApplication()) { | |
| consumerAppDTO.setFapiProfile(oidcModel.getFapiProfile() != null | |
| ? oidcModel.getFapiProfile().toString() : FapiProfileEnum.FAPI1_ADVANCED.value()); | |
| } | |
| consumerAppDTO.setFapiConformanceEnabled(oidcModel.getIsFAPIApplication()); | |
| // Convert FAPI profile enum to String for storage. If no profile specified; default is set to FAPI1_ADVANCED. | |
| if (oidcModel.getIsFAPIApplication()) { | |
| String fapiProfile = oidcModel.getFapiProfile() != null | |
| ? oidcModel.getFapiProfile().toString() : FapiProfileEnum.FAPI1_ADVANCED.value(); | |
| log.debug("Setting FAPI profile: " + fapiProfile + " for application: " + appName); | |
| consumerAppDTO.setFapiProfile(fapiProfile); | |
| } |
Comment on lines
56
to
58
| @Override | ||
| public OpenIDConnectConfiguration apply(OAuthConsumerAppDTO oauthAppDTO) { | ||
|
|
There was a problem hiding this comment.
Log Improvement Suggestion No: 7
Suggested change
| @Override | |
| public OpenIDConnectConfiguration apply(OAuthConsumerAppDTO oauthAppDTO) { | |
| @Override | |
| public OpenIDConnectConfiguration apply(OAuthConsumerAppDTO oauthAppDTO) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("Converting OAuthConsumerAppDTO to OpenIDConnectConfiguration for application: " + | |
| oauthAppDTO.getApplicationName()); | |
| } |
Comment on lines
+285
to
+286
| */ | ||
| private FapiProfile buildFapiProfileConfiguration(OAuthConsumerAppDTO oauthAppDTO) { |
There was a problem hiding this comment.
Log Improvement Suggestion No: 8
Suggested change
| */ | |
| private FapiProfile buildFapiProfileConfiguration(OAuthConsumerAppDTO oauthAppDTO) { | |
| private FapiProfile buildFapiProfileConfiguration(OAuthConsumerAppDTO oauthAppDTO) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("Building FAPI profile configuration for application: " + | |
| oauthAppDTO.getApplicationName()); | |
| } |
Comment on lines
58
to
+61
| // Null check for STSAdminServiceInterface is not mandatory as per the previous implementation. | ||
|
|
||
| SERVICE = new ServerApplicationMetadataService(applicationManagementService, samlSSOConfigService, | ||
| oAuthAdminService, sTSAdminServiceInterface, cibaAuthService); | ||
| oAuthAdminService, sTSAdminServiceInterface, cibaAuthService, fapiConfigMgtService); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 10
Suggested change
| // Null check for STSAdminServiceInterface is not mandatory as per the previous implementation. | |
| SERVICE = new ServerApplicationMetadataService(applicationManagementService, samlSSOConfigService, | |
| oAuthAdminService, sTSAdminServiceInterface, cibaAuthService); | |
| oAuthAdminService, sTSAdminServiceInterface, cibaAuthService, fapiConfigMgtService); | |
| // Null check for STSAdminServiceInterface is not mandatory as per the previous implementation. | |
| log.info("Initializing ServerApplicationMetadataService with all required services"); | |
| SERVICE = new ServerApplicationMetadataService(applicationManagementService, samlSSOConfigService, | |
| oAuthAdminService, sTSAdminServiceInterface, cibaAuthService, fapiConfigMgtService); |
Comment on lines
+253
to
+256
| */ | ||
| public static FapiConfigMgtService getFapiConfigMgtService() { | ||
|
|
||
| return FapiConfigMgtServiceHolder.SERVICE; |
There was a problem hiding this comment.
Log Improvement Suggestion No: 11
Suggested change
| */ | |
| public static FapiConfigMgtService getFapiConfigMgtService() { | |
| return FapiConfigMgtServiceHolder.SERVICE; | |
| public static FapiConfigMgtService getFapiConfigMgtService() { | |
| return FapiConfigMgtServiceHolder.SERVICE; | |
| log.debug("Retrieving FapiConfigMgtService from OSGi context"); | |
| return FapiConfigMgtServiceHolder.SERVICE; |
Comment on lines
+534
to
+537
| public FapiConfig getFAPIConfiguration() { | ||
|
|
||
| String tenantDomain = ContextLoader.getTenantDomainFromContext(); | ||
| try { |
There was a problem hiding this comment.
Log Improvement Suggestion No: 12
Suggested change
| public FapiConfig getFAPIConfiguration() { | |
| String tenantDomain = ContextLoader.getTenantDomainFromContext(); | |
| try { | |
| public FapiConfig getFAPIConfiguration() { | |
| String tenantDomain = ContextLoader.getTenantDomainFromContext(); | |
| log.info("Retrieving FAPI configuration for tenant: " + tenantDomain); |
Comment on lines
+558
to
+561
| public FapiConfig updateFAPIConfiguration(FapiConfig fapiConfig) { | ||
|
|
||
| String tenantDomain = ContextLoader.getTenantDomainFromContext(); | ||
| try { |
There was a problem hiding this comment.
Log Improvement Suggestion No: 13
Suggested change
| public FapiConfig updateFAPIConfiguration(FapiConfig fapiConfig) { | |
| String tenantDomain = ContextLoader.getTenantDomainFromContext(); | |
| try { | |
| public FapiConfig updateFAPIConfiguration(FapiConfig fapiConfig) { | |
| String tenantDomain = ContextLoader.getTenantDomainFromContext(); | |
| log.info("Updating FAPI configuration for tenant: " + tenantDomain); |
Comment on lines
+86
to
+88
| if (fapiConfigMgtService == null) { | ||
| throw new IllegalStateException("FapiConfigMgtService is not available from OSGi context."); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 14
Suggested change
| if (fapiConfigMgtService == null) { | |
| throw new IllegalStateException("FapiConfigMgtService is not available from OSGi context."); | |
| } | |
| if (fapiConfigMgtService == null) { | |
| log.error("FapiConfigMgtService is not available from OSGi context."); | |
| throw new IllegalStateException("FapiConfigMgtService is not available from OSGi context."); | |
| } |
Comment on lines
90
to
100
| @@ -89,7 +95,8 @@ public class ServerConfigManagementServiceFactory { | |||
| jwtClientAuthenticatorMgtService, | |||
| fraudDetectionConfigsService, | |||
| oAuth2OIDCConfigOrgUsageScopeMgtService, | |||
| compatibilitySettingsService | |||
| compatibilitySettingsService, | |||
| fapiConfigMgtService | |||
| ); | |||
There was a problem hiding this comment.
Log Improvement Suggestion No: 15
Suggested change
| SERVICE = new ServerConfigManagementService(applicationManagementService, identityProviderManager, | |
| corsManagementService, | |
| remoteLoggingConfigService, | |
| impersonationConfigMgtService, | |
| dcrConfigMgtService, | |
| jwtClientAuthenticatorMgtService, | |
| fraudDetectionConfigsService, | |
| oAuth2OIDCConfigOrgUsageScopeMgtService, | |
| compatibilitySettingsService, | |
| fapiConfigMgtService | |
| ); | |
| log.info("ServerConfigManagementService initialized successfully."); |
Comment on lines
+44
to
+47
| */ | ||
| public static FapiConfig toApiModel( | ||
| org.wso2.carbon.identity.oauth2.fapi.models.FapiConfig oauthModel) { | ||
|
|
There was a problem hiding this comment.
Log Improvement Suggestion No: 18
Suggested change
| */ | |
| public static FapiConfig toApiModel( | |
| org.wso2.carbon.identity.oauth2.fapi.models.FapiConfig oauthModel) { | |
| public static FapiConfig toApiModel( | |
| org.wso2.carbon.identity.oauth2.fapi.models.FapiConfig oauthModel) { | |
| log.debug("Converting OAuth FAPI config to API model"); | |
| FapiConfig apiModel = new FapiConfig(); |
Comment on lines
+75
to
+77
| */ | ||
| public static org.wso2.carbon.identity.oauth2.fapi.models.FapiConfig toOAuthModel(FapiConfig apiModel) { | ||
|
|
There was a problem hiding this comment.
Log Improvement Suggestion No: 19
Suggested change
| */ | |
| public static org.wso2.carbon.identity.oauth2.fapi.models.FapiConfig toOAuthModel(FapiConfig apiModel) { | |
| public static org.wso2.carbon.identity.oauth2.fapi.models.FapiConfig toOAuthModel(FapiConfig apiModel) { | |
| log.debug("Converting API FAPI config to OAuth model"); | |
| org.wso2.carbon.identity.oauth2.fapi.models.FapiConfig oauthModel = |
Comment on lines
+123
to
+127
| @Override | ||
| public Response getFAPIConfiguration() { | ||
|
|
||
| return Response.ok().entity(configManagementService.getFAPIConfiguration()).build(); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 20
Suggested change
| @Override | |
| public Response getFAPIConfiguration() { | |
| return Response.ok().entity(configManagementService.getFAPIConfiguration()).build(); | |
| } | |
| @Override | |
| public Response getFAPIConfiguration() { | |
| log.info("Retrieving FAPI configuration"); | |
| return Response.ok().entity(configManagementService.getFAPIConfiguration()).build(); |
Comment on lines
+171
to
+175
| @Override | ||
| public Response patchFAPIConfiguration(final FapiConfig fapiConfig) { | ||
|
|
||
| return Response.ok().entity(configManagementService.updateFAPIConfiguration(fapiConfig)).build(); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 21
Suggested change
| @Override | |
| public Response patchFAPIConfiguration(final FapiConfig fapiConfig) { | |
| return Response.ok().entity(configManagementService.updateFAPIConfiguration(fapiConfig)).build(); | |
| } | |
| @Override | |
| public Response patchFAPIConfiguration(final FapiConfig fapiConfig) { | |
| log.info("Updating FAPI configuration"); | |
| return Response.ok().entity(configManagementService.updateFAPIConfiguration(fapiConfig)).build(); |
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.
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.java (1)
1-2:⚠️ Potential issue | 🟡 MinorUpdate copyright header.
Header reads
Copyright (c) 2020, WSO2 Inc.; this file is modified in 2026. Please update the year range and, if applicable, align with the newerWSO2 LLC.wording used in sibling files.🤖 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.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.java` around lines 1 - 2, Update the file header comment to reflect the current year range and company naming consistency: replace the existing "Copyright (c) 2020, WSO2 Inc." line in the top comment of this file with an updated range (e.g., "2020-2026") and use the same company wording used in sibling files (e.g., "WSO2 LLC."). Ensure the change is applied to the header comment above the public class Constants so tooling and license checks pick up the new header.components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.java (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate the copyright year range.
This file has been modified in 2026, but the header still reads
2020-2025. Please extend to2020-2026.🤖 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.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.java` at line 2, Update the file header year range in the comment block to include 2026: change the copyright line in the top file header of the ConfigsServiceHolder class (class name: ConfigsServiceHolder) from "2020-2025" to "2020-2026".components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/pom.xml (1)
3-3:⚠️ Potential issue | 🟡 MinorUpdate copyright year range in header.
The header still reads
2020-2024, but this file is being modified. Consider updating to include 2026.🤖 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.configs/org.wso2.carbon.identity.api.server.configs.v1/pom.xml` at line 3, Update the header copyright year range string "Copyright (c) 2020-2024, WSO2 LLC. (http://www.wso2.com)." to include 2026 (e.g., "2020-2026") so the pom.xml file header reflects the current modification year; replace the existing copyright line with the updated year range.
🧹 Nitpick comments (8)
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 (2)
165-165: Whitespace-only change.The blank line added inside
updateAllowedOriginsis a no-op. Consider dropping it to keep the diff minimal.🤖 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` at line 165, Remove the unnecessary blank line inside the updateAllowedOrigins method in ApiModelToOAuthConsumerApp to keep the diff minimal; edit the body of updateAllowedOrigins to delete the added empty line so behavior and formatting remain unchanged.
96-97: Use.value()instead of.toString()for enum serialization to match the pattern already used for the default value.The enum has both methods, but since
FapiProfileEnum.FAPI1_ADVANCED.value()is already used in the fallback, applying the same pattern to the ternary condition improves consistency.Suggested change
- consumerAppDTO.setFapiProfile(oidcModel.getFapiProfile() != null - ? oidcModel.getFapiProfile().toString() : FapiProfileEnum.FAPI1_ADVANCED.value()); + consumerAppDTO.setFapiProfile(oidcModel.getFapiProfile() != null + ? oidcModel.getFapiProfile().value() : FapiProfileEnum.FAPI1_ADVANCED.value());🤖 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 96 - 97, The ternary used to set the FAPI profile on consumerAppDTO currently calls oidcModel.getFapiProfile().toString(); change it to use .value() for consistency with the fallback FapiProfileEnum.FAPI1_ADVANCED.value(). Update the expression in ApiModelToOAuthConsumerApp where consumerAppDTO.setFapiProfile(...) is assigned so it uses oidcModel.getFapiProfile() != null ? oidcModel.getFapiProfile().value() : FapiProfileEnum.FAPI1_ADVANCED.value(), preserving the null check and fallback behavior.components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.java (2)
19-19: Package/naming nit.The class lives in
...configs.v1.functionbut it is a general-purpose static conversion utility rather than ajava.util.functionimplementation (unlike siblings in that package). Consider moving it to autilpackage and/or renaming to follow existing util conventions in this module. Not blocking.Also applies to: 32-32
🤖 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.java` at line 19, The FAPIConnectorUtil class is a general-purpose static conversion utility placed under the configs.v1.function package (which otherwise contains java.util.function-like classes); move it to a more appropriate package such as org.wso2.carbon.identity.api.server.configs.v1.util and/or rename it to follow util conventions (e.g., FAPIConnectorConverter or FAPIConnectorUtils) to reflect its role; update the package declaration and any imports/usage of FAPIConnectorUtil throughout the module (references to FAPIConnectorUtil) to the new package/name to keep compilation and references consistent.
60-64: Silent drop when enum value cannot be mapped.
.filter(Objects::nonNull)afterFapiProfile.fromValue(...)/FapiProfileEnum.fromValue(...)will silently discard any profile value that the target enum does not recognize. If the two enums ever drift (e.g., OAuth layer adds a new profile before the API-layer model is regenerated), callers will see an incomplete list with no indication. Consider logging a warning for unmapped values so such drift is observable.Proposed change (toApiModel)
- apiModel.setSupportedProfiles( - oauthProfiles.stream() - .map(p -> FapiProfile.fromValue(p.value())) - .filter(Objects::nonNull) - .collect(Collectors.toList())); + apiModel.setSupportedProfiles( + oauthProfiles.stream() + .map(p -> { + FapiProfile mapped = FapiProfile.fromValue(p.value()); + if (mapped == null && LOG.isDebugEnabled()) { + LOG.debug("Skipping unmapped OAuth FAPI profile: " + p.value()); + } + return mapped; + }) + .filter(Objects::nonNull) + .collect(Collectors.toList()));Also applies to: 88-93
🤖 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.java` around lines 60 - 64, The mapping in FAPIConnectorUtil (e.g., in toApiModel where oauthProfiles are mapped with FapiProfile.fromValue and similarly for FapiProfileEnum.fromValue) currently filters out nulls which silently drops unknown enum values; modify the code to detect when fromValue returns null and log a warning including the original unmapped value (use the class logger in FAPIConnectorUtil), then continue to filter out nulls so behavior is unchanged but drift is observable; apply the same change to the second mapping block referenced (lines 88-93) so both mappings log unmapped profile names before they are discarded.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)
260-263: Error code numbering convention mismatch.
UNSUPPORTED_FAPI_PROFILE("65605") is raised as a 400 Bad Request viabuildClientError(UNSUPPORTED_FAPI_PROFILE, ...)inServerApplicationManagementService.validateFapiProfile. However, in this enum the 65xxx range is reserved for server errors (e.g.,ERROR_RETRIEVING_SAML_METADATA,SORTING_NOT_IMPLEMENTED,ERROR_RETRIEVING_FAPI_METADATA), while client errors use the 60xxx range (e.g.,BLOCK_SYSTEM_RESERVED_APP_CREATION60515,UNSUPPORTED_ENHANCED_ORGANIZATION_AUTHENTICATION_ENABLED_CONFIGURATION60521).Consider moving
UNSUPPORTED_FAPI_PROFILEto the 60xxx range to match the existing convention for client-side validation errors.🤖 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 260 - 263, UNSUPPORTED_FAPI_PROFILE currently uses error code "65605" which is reserved for server errors; update its numeric code to follow the client-error 60xxx convention (e.g., change "65605" to a free 60xxx code consistent with other client validation errors) so that ServerApplicationManagementService.validateFapiProfile calls to buildClientError(UNSUPPORTED_FAPI_PROFILE, ...) correctly classify it as a client error; locate the enum constant UNSUPPORTED_FAPI_PROFILE in ApplicationManagementConstants and replace the code string only, ensuring no other message text changes and that the new 60xxx value is unique within the same enum.components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml (2)
2342-2352: Consider addinguniqueItems: trueforsupportedProfiles.The array is effectively a set of profiles; adding
uniqueItems: truewould let the API layer reject duplicate entries without relying on backend de-duplication.♻️ Proposed change
supportedProfiles: type: array + uniqueItems: true description: | List of FAPI security profiles supported by the tenant.🤖 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml` around lines 2342 - 2352, The supportedProfiles array schema (supportedProfiles under the FAPI profiles block) should enforce uniqueness: add uniqueItems: true to the supportedProfiles schema so the API layer rejects duplicate profile entries; update the supportedProfiles definition (the items $ref '#/components/schemas/FapiProfile') to include uniqueItems: true and ensure any related validation tooling/schema consumers respect this constraint.
1441-1455: PATCH semantics differ from other patch endpoints.This
PATCH /configs/fapiaccepts a fullFapiConfigreplacement body rather than a JSON Patch document (as/configs/dcr,/configs/cors,/configs/impersonationetc. do with*PatchRequestschemas). If the intent is full replacement, consider usingPUTfor REST semantic consistency; if the intent is partial update, consider introducing aFapiConfigPatchRequestschema aligned with the other patch endpoints.🤖 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml` around lines 1441 - 1455, The PATCH operation "patchFAPIConfiguration" currently accepts a full FapiConfig replacement body which is inconsistent with other PATCH endpoints (/configs/dcr, /configs/cors, /configs/impersonation) that use patch request schemas; either change the HTTP verb to PUT for a true full-replace semantics or convert the operation to accept a partial update by introducing a new schema (e.g., FapiConfigPatchRequest) and updating the requestBody to reference it so PATCH behaves consistently with the other patch endpoints; update the OpenAPI operationId patchFAPIConfiguration and the requestBody $ref accordingly.components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java (1)
558-573: Return persisted state instead of the request object.
updateFAPIConfigurationreturns the caller-suppliedfapiConfigwithout reading back the persisted configuration. If the backend applies normalization (defaults, dedupe ofsupportedProfiles, coercion of a nullenabled), the response will not reflect what was actually stored. Consider fetching the updated value post-set and converting viaFAPIConnectorUtil.toApiModel(...), consistent with howupdateFraudDetectionConfigsround-trips through the backend.♻️ Proposed refactor
public FapiConfig updateFAPIConfiguration(FapiConfig fapiConfig) { String tenantDomain = ContextLoader.getTenantDomainFromContext(); try { fapiConfigMgtService.setFapiConfig(FAPIConnectorUtil.toOAuthModel(fapiConfig), tenantDomain); - return fapiConfig; + return FAPIConnectorUtil.toApiModel(fapiConfigMgtService.getFapiConfig(tenantDomain)); } catch (FapiConfigMgtClientException e) {🤖 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java` around lines 558 - 573, The method updateFAPIConfiguration currently returns the input fapiConfig instead of the persisted state; after calling fapiConfigMgtService.setFapiConfig(...) fetch the saved model from the backend and convert it to the API model before returning so the response reflects normalization/defaulting performed by the backend—use the same pattern as updateFraudDetectionConfigs: call the service to set, then retrieve the stored OAuth model (or have set return it), convert with FAPIConnectorUtil.toApiModel(...) and return that result instead of the original fapiConfig.
🤖 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/ServerApplicationManagementService.java`:
- Around line 2215-2263: The Javadoc for validateFapiProfile is inaccurate: it
claims the method no-ops when oidcConfig is null or FAPI is disabled, but the
code dereferences oidcConfig.getFapiProfile() and only no-ops when
getFapiProfile() is null or FapiConfigMgtService is unavailable; update the
JavaDoc to state the actual behavior (i.e., the method expects a non-null
OpenIDConnectConfiguration and will return immediately only if
oidcConfig.getFapiProfile() is null or when
ApplicationManagementServiceHolder.getFapiConfigMgtService() is unavailable) and
remove the incorrect "oidcConfig is null" / "FAPI is disabled" phrasing,
referencing validateFapiProfile, oidcConfig.getFapiProfile(), and
FapiConfigMgtService in the doc.
- Around line 2241-2247: The code calls
fapiConfigMgtService.getFapiConfig(tenantDomain).getSupportedProfiles() without
guarding against getFapiConfig() returning null; update
ServerApplicationManagementService to fetch the config into a local variable
(e.g., FapiConfig fapiConfig =
fapiConfigMgtService.getFapiConfig(tenantDomain)), check for null (or use
Optional.ofNullable) before calling getSupportedProfiles(), and handle the null
case by treating supportedProfiles as empty (or returning the appropriate error)
so the subsequent stream anyMatch on supportedProfiles and the uses of
requestedProfileValue / isSupported are null-safe.
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 563-584: The getAllowedFapiProfiles method currently calls
fapiConfig.getSupportedProfiles() without checking if the FapiConfig returned
from fapiConfigMgtService.getFapiConfig(tenantDomain) is null; add a null check
after calling getFapiConfig (in getAllowedFapiProfiles) and if fapiConfig is
null return Collections.emptyList() (keeping the existing debug
logging/exception handling behavior), then proceed to call
fapiConfig.getSupportedProfiles() only when fapiConfig is non-null.
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java`:
- Around line 534-573: The FapiConfigMgtClientException catch blocks in
getFAPIConfiguration and updateFAPIConfiguration are duplicating ERROR logs
because getFapiConfigErrorResponse (via ErrorResponse.Builder.build) already
logs the error; remove the explicit log.error(...) calls (or change them to
log.debug(...)) in the FapiConfigMgtClientException branches so only the builder
logs the event, while keeping the existing log.error(...) in the
FapiConfigMgtException branches for server errors; references:
getFAPIConfiguration, updateFAPIConfiguration, FapiConfigMgtClientException,
getFapiConfigErrorResponse, ErrorResponse.Builder.build.
---
Outside diff comments:
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.java`:
- Line 2: Update the file header year range in the comment block to include
2026: change the copyright line in the top file header of the
ConfigsServiceHolder class (class name: ConfigsServiceHolder) from "2020-2025"
to "2020-2026".
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.java`:
- Around line 1-2: Update the file header comment to reflect the current year
range and company naming consistency: replace the existing "Copyright (c) 2020,
WSO2 Inc." line in the top comment of this file with an updated range (e.g.,
"2020-2026") and use the same company wording used in sibling files (e.g., "WSO2
LLC."). Ensure the change is applied to the header comment above the public
class Constants so tooling and license checks pick up the new header.
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/pom.xml`:
- Line 3: Update the header copyright year range string "Copyright (c)
2020-2024, WSO2 LLC. (http://www.wso2.com)." to include 2026 (e.g., "2020-2026")
so the pom.xml file header reflects the current modification year; replace the
existing copyright line with the updated year range.
---
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 260-263: UNSUPPORTED_FAPI_PROFILE currently uses error code
"65605" which is reserved for server errors; update its numeric code to follow
the client-error 60xxx convention (e.g., change "65605" to a free 60xxx code
consistent with other client validation errors) so that
ServerApplicationManagementService.validateFapiProfile calls to
buildClientError(UNSUPPORTED_FAPI_PROFILE, ...) correctly classify it as a
client error; locate the enum constant UNSUPPORTED_FAPI_PROFILE in
ApplicationManagementConstants and replace the code string only, ensuring no
other message text changes and that the new 60xxx value is unique within the
same enum.
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`:
- Line 165: Remove the unnecessary blank line inside the updateAllowedOrigins
method in ApiModelToOAuthConsumerApp to keep the diff minimal; edit the body of
updateAllowedOrigins to delete the added empty line so behavior and formatting
remain unchanged.
- Around line 96-97: The ternary used to set the FAPI profile on consumerAppDTO
currently calls oidcModel.getFapiProfile().toString(); change it to use .value()
for consistency with the fallback FapiProfileEnum.FAPI1_ADVANCED.value(). Update
the expression in ApiModelToOAuthConsumerApp where
consumerAppDTO.setFapiProfile(...) is assigned so it uses
oidcModel.getFapiProfile() != null ? oidcModel.getFapiProfile().value() :
FapiProfileEnum.FAPI1_ADVANCED.value(), preserving the null check and fallback
behavior.
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java`:
- Around line 558-573: The method updateFAPIConfiguration currently returns the
input fapiConfig instead of the persisted state; after calling
fapiConfigMgtService.setFapiConfig(...) fetch the saved model from the backend
and convert it to the API model before returning so the response reflects
normalization/defaulting performed by the backend—use the same pattern as
updateFraudDetectionConfigs: call the service to set, then retrieve the stored
OAuth model (or have set return it), convert with
FAPIConnectorUtil.toApiModel(...) and return that result instead of the original
fapiConfig.
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.java`:
- Line 19: The FAPIConnectorUtil class is a general-purpose static conversion
utility placed under the configs.v1.function package (which otherwise contains
java.util.function-like classes); move it to a more appropriate package such as
org.wso2.carbon.identity.api.server.configs.v1.util and/or rename it to follow
util conventions (e.g., FAPIConnectorConverter or FAPIConnectorUtils) to reflect
its role; update the package declaration and any imports/usage of
FAPIConnectorUtil throughout the module (references to FAPIConnectorUtil) to the
new package/name to keep compilation and references consistent.
- Around line 60-64: The mapping in FAPIConnectorUtil (e.g., in toApiModel where
oauthProfiles are mapped with FapiProfile.fromValue and similarly for
FapiProfileEnum.fromValue) currently filters out nulls which silently drops
unknown enum values; modify the code to detect when fromValue returns null and
log a warning including the original unmapped value (use the class logger in
FAPIConnectorUtil), then continue to filter out nulls so behavior is unchanged
but drift is observable; apply the same change to the second mapping block
referenced (lines 88-93) so both mappings log unmapped profile names before they
are discarded.
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml`:
- Around line 2342-2352: The supportedProfiles array schema (supportedProfiles
under the FAPI profiles block) should enforce uniqueness: add uniqueItems: true
to the supportedProfiles schema so the API layer rejects duplicate profile
entries; update the supportedProfiles definition (the items $ref
'#/components/schemas/FapiProfile') to include uniqueItems: true and ensure any
related validation tooling/schema consumers respect this constraint.
- Around line 1441-1455: The PATCH operation "patchFAPIConfiguration" currently
accepts a full FapiConfig replacement body which is inconsistent with other
PATCH endpoints (/configs/dcr, /configs/cors, /configs/impersonation) that use
patch request schemas; either change the HTTP verb to PUT for a true
full-replace semantics or convert the operation to accept a partial update by
introducing a new schema (e.g., FapiConfigPatchRequest) and updating the
requestBody to reference it so PATCH behaves consistently with the other patch
endpoints; update the OpenAPI operationId patchFAPIConfiguration and the
requestBody $ref accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9d0da19-f13f-44b5-a893-38c6474518f9
⛔ Files ignored due to path filters (8)
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/FapiMetadata.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/FapiProfile.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/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/DCRConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiProfile.javais excluded by!**/gen/**
📒 Files selected for processing (17)
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.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/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.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/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.yamlcomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/pom.xmlcomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml
VimukthiRajapaksha
force-pushed
the
master_fapi2
branch
from
c4969f8 to
8c31c36
Compare
VimukthiRajapaksha
force-pushed
the
master_fapi2
branch
2 times, most recently
from
f11b430 to
774e460
Compare
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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java (1)
185-207:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd
FapiConfigMgtServiceparameter to the constructor.The assignment at line 207 references
fapiConfigMgtService, but this variable is not declared as a constructor parameter, causing a compilation error. The field is used in methods at lines 537 and 560, so it must be properly initialized.Proposed fix
public ServerConfigManagementService(ApplicationManagementService applicationManagementService, IdpManager idpManager, CORSManagementService corsManagementService, RemoteLoggingConfigService remoteLoggingConfigService, ImpersonationConfigMgtService impersonationConfigMgtService, DCRConfigurationMgtService dcrConfigurationMgtService, JWTClientAuthenticatorMgtService jwtClientAuthenticatorMgtService, FraudDetectionConfigsService fraudDetectionConfigsService, OAuth2OIDCConfigOrgUsageScopeMgtService oauth2OIDCConfigOrgUsageScopeMgtService, - CompatibilitySettingsService identityCompatibilitySettingsService) { + CompatibilitySettingsService identityCompatibilitySettingsService, + FapiConfigMgtService fapiConfigMgtService) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java` around lines 185 - 207, The constructor ServerConfigManagementService assigns this.fapiConfigMgtService but does not accept a FapiConfigMgtService parameter; add a constructor parameter of type FapiConfigMgtService named fapiConfigMgtService and assign it (this.fapiConfigMgtService = fapiConfigMgtService) so the field used by methods referencing fapiConfigMgtService is properly initialized; update any caller(s) of the constructor to pass the new FapiConfigMgtService dependency.
🧹 Nitpick 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/functions/application/inbound/oauth2/ApiModelToOAuthConsumerApp.java (1)
94-98: 💤 Low valueUse
.value()consistently for string conversion.Line 97 uses
.toString()for the provided profile and.value()for the default. Both methods return the same string value for OpenAPI-generated enums, but using.value()consistently aligns with patterns elsewhere in the codebase and improves clarity.Suggested change
- consumerAppDTO.setFapiProfile(oidcModel.getFapiProfile() != null - ? oidcModel.getFapiProfile().toString() : FapiProfileEnum.FAPI1_ADVANCED.value()); + consumerAppDTO.setFapiProfile(oidcModel.getFapiProfile() != null + ? oidcModel.getFapiProfile().value() : FapiProfileEnum.FAPI1_ADVANCED.value());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. 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 94 - 98, The code sets consumerAppDTO.fapiProfile using oidcModel.getFapiProfile().toString() for provided values but uses FapiProfileEnum.FAPI1_ADVANCED.value() for the default; change the provided branch to call .value() instead of .toString() so both branches use .value() consistently—update the conditional in ApiModelToOAuthConsumerApp where getIsFAPIApplication() is checked and the getFapiProfile() result is converted before passing to consumerAppDTO.setFapiProfile(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
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/ServerApplicationMetadataService.java`:
- Line 80: Remove the duplicate import of
org.apache.commons.collections.CollectionUtils in
ServerApplicationMetadataService.java: locate the redundant import statement
(the one at line ~80) and delete it so only the single import of CollectionUtils
(the existing one around line 21) remains; ensure the class
ServerApplicationMetadataService still compiles after removal.
---
Outside diff comments:
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java`:
- Around line 185-207: The constructor ServerConfigManagementService assigns
this.fapiConfigMgtService but does not accept a FapiConfigMgtService parameter;
add a constructor parameter of type FapiConfigMgtService named
fapiConfigMgtService and assign it (this.fapiConfigMgtService =
fapiConfigMgtService) so the field used by methods referencing
fapiConfigMgtService is properly initialized; update any caller(s) of the
constructor to pass the new FapiConfigMgtService dependency.
---
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/ApiModelToOAuthConsumerApp.java`:
- Around line 94-98: The code sets consumerAppDTO.fapiProfile using
oidcModel.getFapiProfile().toString() for provided values but uses
FapiProfileEnum.FAPI1_ADVANCED.value() for the default; change the provided
branch to call .value() instead of .toString() so both branches use .value()
consistently—update the conditional in ApiModelToOAuthConsumerApp where
getIsFAPIApplication() is checked and the getFapiProfile() result is converted
before passing to consumerAppDTO.setFapiProfile(...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0360326e-8fef-43d0-82ed-3e2a46f6f060
⛔ Files ignored due to path filters (8)
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/FapiMetadata.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/FapiProfile.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/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/DCRConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiProfile.javais excluded by!**/gen/**
📒 Files selected for processing (17)
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.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/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.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/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.yamlcomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/pom.xmlcomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml
🚧 Files skipped from review as they are similar to previous changes (12)
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.java
- 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.configs/org.wso2.carbon.identity.api.server.configs.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/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml
- 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.java
- 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
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.java
- 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
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.java
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/ServerApplicationManagementService.java (1)
2256-2262:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
getFapiConfig()andgetSupportedProfiles()before streaming.Line 2257 chains calls without null checking. The
FapiConfig.supportedProfilesfield is explicitly nullable and can return null, which will cause aNullPointerExceptionat the.stream()call on line 2260, bypassing error handling. Similar code inServerApplicationMetadataService(line 573-575) handles this with a null check. Apply null-safe extraction here.Suggested fix
- List<FapiProfileEnum> supportedProfiles = - fapiConfigMgtService.getFapiConfig(tenantDomain).getSupportedProfiles(); + List<FapiProfileEnum> supportedProfiles = Optional + .ofNullable(fapiConfigMgtService.getFapiConfig(tenantDomain)) + .map(config -> config.getSupportedProfiles()) + .orElse(Collections.emptyList()); String requestedProfileValue = oidcConfig.getFapiProfile().toString();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. 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/ServerApplicationManagementService.java` around lines 2256 - 2262, Guard against null FAPI config and its supportedProfiles before streaming: in ServerApplicationManagementService where you call fapiConfigMgtService.getFapiConfig(tenantDomain).getSupportedProfiles(), first retrieve the FapiConfig into a local (e.g., fapiConfig), check if fapiConfig is null or fapiConfig.getSupportedProfiles() is null, and only then perform the stream/anyMatch; if either is null, treat supportedProfiles as an empty list (so isSupported becomes false) or handle per existing error path—update the code around getFapiConfig/getSupportedProfiles to use these null checks instead of chaining calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
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/ServerApplicationManagementService.java`:
- Around line 2256-2262: Guard against null FAPI config and its
supportedProfiles before streaming: in ServerApplicationManagementService where
you call
fapiConfigMgtService.getFapiConfig(tenantDomain).getSupportedProfiles(), first
retrieve the FapiConfig into a local (e.g., fapiConfig), check if fapiConfig is
null or fapiConfig.getSupportedProfiles() is null, and only then perform the
stream/anyMatch; if either is null, treat supportedProfiles as an empty list (so
isSupported becomes false) or handle per existing error path—update the code
around getFapiConfig/getSupportedProfiles to use these null checks instead of
chaining calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2aabe779-f372-4dd1-b69d-94230e88bf5d
📒 Files selected for processing (4)
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/ServerApplicationManagementService.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/ServerApplicationMetadataService.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.javapom.xml
✅ Files skipped from review due to trivial changes (1)
- pom.xml
🚧 Files skipped from review as they are similar to previous changes (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
VimukthiRajapaksha
force-pushed
the
master_fapi2
branch
from
b9a37d9 to
d66d9d5
Compare
VimukthiRajapaksha
force-pushed
the
master_fapi2
branch
from
d66d9d5 to
a759776
Compare
hwupathum
reviewed
Jun 10, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ 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)
573-575:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle null
FapiConfigbefore reading supported profiles.At Line 573,
getFapiConfig(...)can return null; Line 574 dereferences it directly. This can fail metadata retrieval instead of returning the documented empty profile list.Suggested fix
try { FapiConfig fapiConfig = fapiConfigMgtService.getFapiConfig(tenantDomain); - List<FapiProfileEnum> supportedProfiles = fapiConfig.getSupportedProfiles(); - if (CollectionUtils.isEmpty(supportedProfiles)) { + if (fapiConfig == null || CollectionUtils.isEmpty(fapiConfig.getSupportedProfiles())) { return Collections.emptyList(); } + List<FapiProfileEnum> supportedProfiles = fapiConfig.getSupportedProfiles(); return supportedProfiles.stream() .map(FapiProfileEnum::value) .collect(Collectors.toList());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. 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 573 - 575, The code dereferences FapiConfig returned by fapiConfigMgtService.getFapiConfig(tenantDomain) without null-check; modify the metadata retrieval in ServerApplicationMetadataService so that after calling fapiConfigMgtService.getFapiConfig(...) you first check if fapiConfig is null and, if so, treat supportedProfiles as an empty list (e.g., assign Collections.emptyList() or new ArrayList<>()), then proceed to call fapiConfig.getSupportedProfiles() only when fapiConfig is non-null; update the logic around FapiConfig fapiConfig, fapiConfig.getSupportedProfiles(), and the subsequent CollectionUtils.isEmpty(supportedProfiles) check to ensure null-safe behavior and return the documented empty profile list when config is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
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 285-302: The Javadoc claims a warning is logged but the code uses
debug; update buildFapiProfileConfiguration to actually log a warning for
unrecognised values by replacing the debug check and call with a warn check and
call (use log.isWarnEnabled() and log.warn(String.format(...), e)) so
FapiProfile.fromValue failures for oauthAppDTO produce a warn-level log;
alternatively, if you prefer debug, update the method comment to reflect
debug—ensure the change references buildFapiProfileConfiguration, FapiProfile,
oauthAppDTO and the logging call.
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml`:
- Around line 1969-1975: The OpenAPI property fapiProfile's description states a
default of FAPI1_ADVANCED but the schema lacks a default; update the fapiProfile
schema entry to include default: FAPI1_ADVANCED so generated clients/docs match
runtime behavior—locate the fapiProfile property and add the default value (and
ensure the referenced schema '`#/components/schemas/FapiProfile`' accepts that
enum/value) so models like FapiProfile and any generated Java fields initialize
accordingly.
---
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 573-575: The code dereferences FapiConfig returned by
fapiConfigMgtService.getFapiConfig(tenantDomain) without null-check; modify the
metadata retrieval in ServerApplicationMetadataService so that after calling
fapiConfigMgtService.getFapiConfig(...) you first check if fapiConfig is null
and, if so, treat supportedProfiles as an empty list (e.g., assign
Collections.emptyList() or new ArrayList<>()), then proceed to call
fapiConfig.getSupportedProfiles() only when fapiConfig is non-null; update the
logic around FapiConfig fapiConfig, fapiConfig.getSupportedProfiles(), and the
subsequent CollectionUtils.isEmpty(supportedProfiles) check to ensure null-safe
behavior and return the documented empty profile list when config is absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: debebfb1-0bef-448e-b8b1-3799b5bbac51
⛔ Files ignored due to path filters (8)
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/FapiMetadata.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/FapiProfile.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/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/DCRConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiProfile.javais excluded by!**/gen/**
📒 Files selected for processing (17)
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.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/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.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/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.yamlcomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yamlpom.xml
🚧 Files skipped from review as they are similar to previous changes (13)
- 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.java
- 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
- 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.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.java
- 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.java
- 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/ServerApplicationManagementService.java
hwupathum
reviewed
Jun 11, 2026
hwupathum
reviewed
Jun 11, 2026
hwupathum
reviewed
Jun 11, 2026
hwupathum
reviewed
Jun 11, 2026
Add FapiConfig and FapiProfile model classes to the configs API for tenant-level FAPI compliance configuration (enabled flag + supported profiles) Add FapiProfile enum to the application management API module Introduce FAPIConnectorUtil for bi-directional conversion between API-layer FAPI models and OAuth bundle FAPI models Default FAPI profile to FAPI1_ADVANCED when an application is FAPI-enabled but no profile is explicitly specified Remove unused imports (FapiConfig, FapiProfile, FapiConfigMgtServerException) Fix import ordering in ServerApplicationMetadataService
VimukthiRajapaksha
force-pushed
the
master_fapi2
branch
from
a759776 to
f584a37
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java`:
- Around line 556-560: In updateFAPIConfiguration, reject a null input before
converting by adding a null guard for the fapiConfig parameter: if fapiConfig is
null, return or throw a 400-level error (e.g., BadRequestException) instead of
calling FAPIConnectorUtil.toOAuthModel(fapiConfig); only call
FAPIConnectorUtil.toOAuthModel(...) and fapiConfigMgtService.setFapiConfig(...)
when fapiConfig is non-null to avoid overwriting stored tenant FAPI
configuration with empty/disabled values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0aa87096-390a-4491-a286-374f63269ac9
⛔ Files ignored due to path filters (8)
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/FapiMetadata.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/FapiProfile.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/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/DCRConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiProfile.javais excluded by!**/gen/**
📒 Files selected for processing (17)
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.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/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.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/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.yamlcomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yamlpom.xml
🚧 Files skipped from review as they are similar to previous changes (16)
- 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/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/factories/ServerApplicationMetadataServiceFactory.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
- 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
- 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.java
- 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
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.java
- 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/ServerApplicationManagementService.java
- 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
VimukthiRajapaksha
force-pushed
the
master_fapi2
branch
from
daa312c to
7c66b39
Compare
|
PR builder started |
hwupathum
previously approved these changes
Jun 15, 2026
hwupathum
left a comment
hwupathum
left a comment
Contributor
There was a problem hiding this comment.
Can merge after integration tests pass
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
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/ServerApplicationManagementService.java`:
- Around line 2245-2269: The validateFapiProfile method currently returns early
when fapiProfile is null without validating the default profile that will be
applied downstream. Modify the early return condition to remove the check for
null fapiProfile. After the fapiConfigMgtService null check, determine the
effective requested profile by using the fapiProfile value if provided, or
defaulting to FAPI1_ADVANCED if omitted. Then validate this resolved profile
value against the supported profiles retrieved from fapiConfigMgtService,
ensuring both explicitly set and defaulted FAPI profiles are properly validated.
- Around line 2281-2284: The catch block for FapiConfigMgtException at the
specified location currently uses Utils.buildServerError with a generic error
message. Replace this with an error-building call that uses the
ERROR_RETRIEVING_FAPI_METADATA enum constant to ensure consistent and specific
API error responses. This will align the FAPI configuration retrieval failure
with the new error code instead of returning a generic server error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2392a89c-f9bf-48f8-91bb-25e34184a0db
⛔ Files ignored due to path filters (8)
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/FapiMetadata.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/FapiProfile.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/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApi.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/ConfigsApiService.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/DCRConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiConfig.javais excluded by!**/gen/**components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/gen/java/org/wso2/carbon/identity/api/server/configs/v1/model/FapiProfile.javais excluded by!**/gen/**
📒 Files selected for processing (17)
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.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/src/main/java/org/wso2/carbon/identity/api/server/application/management/v1/core/ServerApplicationManagementService.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/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.yamlcomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.javacomponents/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yamlpom.xml
🚧 Files skipped from review as they are similar to previous changes (15)
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/impl/ConfigsApiServiceImpl.java
- 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
- 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/core/functions/application/inbound/oauth2/OAuthConsumerAppToApiModel.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/DCRConnectorUtil.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/Constants.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.common/src/main/java/org/wso2/carbon/identity/api/server/configs/common/ConfigsServiceHolder.java
- 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.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/FAPIConnectorUtil.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.common/src/main/java/org/wso2/carbon/identity/api/server/application/management/common/ApplicationManagementServiceHolder.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/resources/configs.yaml
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/factories/ServerConfigManagementServiceFactory.java
- components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java
- 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
Comment on lines
+2245
to
+2269
| if (oidcConfig == null || !Boolean.TRUE.equals(oidcConfig.getIsFAPIApplication()) | ||
| || oidcConfig.getFapiProfile() == null) { | ||
| return; | ||
| } | ||
|
|
||
| /* | ||
| * FapiConfigMgtService is treated as optional here. Profile validation is an enhancement — | ||
| * silently skipping it avoids blocking all OIDC application updates in deployments where | ||
| * the FAPI bundle is not present. The configs API module (/configs/fapi) hard-requires | ||
| * the service at startup and fails fast if it is absent. | ||
| */ | ||
| FapiConfigMgtService fapiConfigMgtService = ApplicationManagementServiceHolder.getFapiConfigMgtService(); | ||
| if (fapiConfigMgtService == null) { | ||
| log.debug("FapiConfigMgtService is unavailable. Skipping fapiProfile validation."); | ||
| return; | ||
| } | ||
|
|
||
| String tenantDomain = ContextLoader.getTenantDomainFromContext(); | ||
| try { | ||
| List<FapiProfileEnum> supportedProfiles = | ||
| fapiConfigMgtService.getFapiConfig(tenantDomain).getSupportedProfiles(); | ||
| String requestedProfileValue = oidcConfig.getFapiProfile().toString(); | ||
|
|
||
| boolean isSupported = supportedProfiles.stream() | ||
| .anyMatch(p -> p.value().equals(requestedProfileValue)); |
Contributor
There was a problem hiding this comment.
Validate the effective default FAPI profile.
When isFAPIApplication is true and fapiProfile is omitted, downstream mapping applies FAPI1_ADVANCED; returning at Line 2246 skips tenant support validation for that effective value. Resolve the requested profile first and validate the default too. Based on PR objectives, omitted FAPI profiles default to FAPI1_ADVANCED.
Proposed fix
- // Only validate when the application is marked as FAPI and a specific profile is provided.
- // If isFAPIApplication is false the profile field is ignored in storage, so validation would
- // produce a misleading error for a field that has no effect.
- if (oidcConfig == null || !Boolean.TRUE.equals(oidcConfig.getIsFAPIApplication())
- || oidcConfig.getFapiProfile() == null) {
+ // Only validate when the application is marked as FAPI. If the profile is omitted,
+ // validate the default profile that is applied downstream.
+ if (oidcConfig == null || !Boolean.TRUE.equals(oidcConfig.getIsFAPIApplication())) {
return;
}
@@
- String requestedProfileValue = oidcConfig.getFapiProfile().toString();
+ String requestedProfileValue = oidcConfig.getFapiProfile() != null
+ ? oidcConfig.getFapiProfile().toString()
+ : FapiProfileEnum.FAPI1_ADVANCED.value();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
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/ServerApplicationManagementService.java`
around lines 2245 - 2269, The validateFapiProfile method currently returns early
when fapiProfile is null without validating the default profile that will be
applied downstream. Modify the early return condition to remove the check for
null fapiProfile. After the fapiConfigMgtService null check, determine the
effective requested profile by using the fapiProfile value if provided, or
defaulting to FAPI1_ADVANCED if omitted. Then validate this resolved profile
value against the supported profiles retrieved from fapiConfigMgtService,
ensuring both explicitly set and defaulted FAPI profiles are properly validated.
Comment on lines
+2281
to
+2284
| } catch (FapiConfigMgtException e) { | ||
| throw Utils.buildServerError( | ||
| String.format("Error while retrieving FAPI configuration for tenant '%s' during fapiProfile " + | ||
| "validation.", tenantDomain), e); |
Contributor
There was a problem hiding this comment.
Use the new FAPI metadata error code here.
Line 2281 currently builds a generic server error, so this retrieval-failure path does not return the new ERROR_RETRIEVING_FAPI_METADATA code. Use the enum to keep API error responses consistent.
Proposed fix
} catch (FapiConfigMgtException e) {
- throw Utils.buildServerError(
- String.format("Error while retrieving FAPI configuration for tenant '%s' during fapiProfile " +
- "validation.", tenantDomain), e);
+ ErrorMessage errorEnum = ErrorMessage.ERROR_RETRIEVING_FAPI_METADATA;
+ throw Utils.buildServerError(errorEnum.getCode(), errorEnum.getMessage(),
+ buildFormattedDescription(errorEnum.getDescription(), tenantDomain), e);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
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/ServerApplicationManagementService.java`
around lines 2281 - 2284, The catch block for FapiConfigMgtException at the
specified location currently uses Utils.buildServerError with a generic error
message. Replace this with an error-building call that uses the
ERROR_RETRIEVING_FAPI_METADATA enum constant to ensure consistent and specific
API error responses. This will align the FAPI configuration retrieval failure
with the new error code instead of returning a generic server error.
|
PR builder completed |
|
PR builder started |
|
PR builder completed |
|
PR builder started |
|
PR builder completed |
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
3 participants
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
Adds tenant-level Financial-grade API (FAPI) configuration support to the Identity Server REST API. The missing model layer between the configs API and the OAuth bundle for FAPI compliance settings is introduced.
Resolves wso2-enterprise/iam-product-management#601
Goals
via the configs API model layer.
FapiProfileenum to both the application management and configs API modules.FAPIConnectorUtilto cleanly convert between API-layer FAPI models andOAuth bundle FAPI models.
FAPI1_ADVANCEDwhen an application is markedas FAPI-enabled but no profile is explicitly provided.
Approach
FapiConfigandFapiProfileunderorg.wso2.carbon.identity.api.server.configs.v1.modelfor tenant-level FAPI config.FapiProfileenum underorg.wso2.carbon.identity.api.server.application.management.v1.FAPIConnectorUtilwithtoApiModel()andtoOAuthModel()methods forbi-directional conversion between the API layer and the OAuth bundle.
ApiModelToOAuthConsumerAppto apply theFAPI1_ADVANCEDdefault whenisFAPIApplicationistrueand no profile is specified, replacing verbose conditionalcomment logic.
User stories
Developer Checklist (Mandatory)
product-isissue to trackany behavioral change or migration impact.
Release note
Added tenant-level FAPI (Financial-grade API) configuration model support, including
enabled/disabled toggle and supported security profiles (
FAPI1_ADVANCED,FAPI2_SECURITY).Applications are now defaulted to
FAPI1_ADVANCEDwhen FAPI is enabled but no profileis specified.
Documentation
Security checks