Enable Unavailable Accounts Display in Authorization Screen to Support CDS Toolkit#900
Open
Enable Unavailable Accounts Display in Authorization Screen to Support CDS Toolkit#900
Conversation
|
|
|
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:
WalkthroughThis PR enhances the consent authorization UI with additional customizable display sections, introduces response payload signing configuration, fixes a JavaScript variable reference bug, and adds numerous Javadoc punctuation improvements across multiple files. Changes
Sequence Diagram(s)sequenceDiagram
participant AuthStep as ExternalAPIConsentRetrievalStep
participant DTO as PopulateConsentAuthorizeScreenDTO
participant Util as Utils
participant JSP as fs_default_account_selection.jsp
participant UI as Browser/UI
AuthStep->>AuthStep: Retrieve response with additionalDisplayData
AuthStep->>DTO: Set additionalDisplayData field
Util->>Util: Extract ADDITIONAL_DISPLAY_DATA from dataSetMap
Util->>Util: Store under ADDITIONAL_DISPLAY_SECTION_TAG
DTO->>JSP: Pass additionalSections in request
JSP->>JSP: Iterate sections: render heading, subHeading
JSP->>JSP: Iterate displayList items per section
JSP->>UI: Render dynamic UI sections with separators
UI->>UI: Attach tooltip popovers via tooltip-functions.js
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple cohorts with varying complexity: extensive Javadoc fixes are repetitive and low-effort, but the new feature (additional display data) introduces a new DTO, backend logic, configuration, and UI rendering requiring separate reasoning across multiple layers. The response payload signing feature adds straightforward configuration and conditional logic. Mixed heterogeneity demands attention across frontend and backend components, but individual changes within each layer are self-contained. Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
...vices/accelerator/consent/mgt/extensions/authorize/impl/ExternalAPIConsentRetrievalStep.java
Show resolved
Hide resolved
...vices/accelerator/consent/mgt/extensions/authorize/impl/ExternalAPIConsentRetrievalStep.java
Show resolved
Hide resolved
Comment on lines
+208
to
+209
| List<Map<String, Object>> displayData = (List<Map<String, Object>>) | ||
| dataSetMap.get(ConsentAuthorizeConstants.DISPLAY_DATA); |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 3
Suggested change
| List<Map<String, Object>> displayData = (List<Map<String, Object>>) | |
| dataSetMap.get(ConsentAuthorizeConstants.DISPLAY_DATA); | |
| List<Map<String, Object>> displayData = (List<Map<String, Object>>) | |
| dataSetMap.get(ConsentAuthorizeConstants.DISPLAY_DATA); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Retrieved display data for consent customization with size: " + | |
| (displayData != null ? displayData.size() : 0)); | |
| } |
Contributor
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 | ||
| #### Log Improvement Suggestion No: 2 | ||
| #### Log Improvement Suggestion No: 3 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
financial-services-accelerator/react-apps/self-care-portal/src/main/java/org/wso2/financial/services/accelerator/scp/webapp/service/ResourceInterceptorService.java (1)
159-160:⚠️ Potential issue | 🟡 MinorIncorrect null check for
optString()return value.
JSONObject.optString()returns an empty string""when the key is missing, notnull. The null check on line 160 will always pass. While this likely works by accident (sincecontainsKey("")returns false), the logic should explicitly check for empty strings.🔧 Suggested fix
String clientId = dataElementJson.optString(Constants.CLIENT_ID_CC); - if (clientId != null && appClientNameMap.containsKey(clientId)) { + if (StringUtils.isNotEmpty(clientId) && appClientNameMap.containsKey(clientId)) { dataElementJson.put(Constants.SOFTWARE_CLIENT_NAME, appClientNameMap.get(clientId)); dataElementJson.put(Constants.LOGO_URL, appLogoUrlMap.get(clientId));
🤖 Fix all issues with AI agents
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/model/DisplayDataDTO.java`:
- Around line 1-9: This file is missing the Apache 2.0 license header; add the
same standard Apache-2.0 header used elsewhere in the repo (e.g., the header
from ExternalAPIConsentRetrievalStep.java) immediately above the package
declaration for the DisplayDataDTO class so the top of the file contains the
full copyright/license block consistent with other files.
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp`:
- Around line 127-132: The JSP prints unescaped user input via
record.displayName inside the c:forEach, causing XSS; replace the raw EL output
with the JSTL escape mechanism (use the c:out tag with the record.displayName
value) so the value is HTML-escaped before rendering, and ensure the JSTL core
taglib (prefix c) is declared in the JSP if not already present.
🧹 Nitpick comments (4)
financial-services-accelerator/react-apps/self-care-portal/src/main/java/org/wso2/financial/services/accelerator/scp/webapp/exception/TokenGenerationException.java (1)
24-24: Improve Javadoc sentence structure.The Javadoc description uses an incomplete sentence. "Throws if errors occurred..." should be revised to follow standard exception class documentation conventions using passive voice and consistent tense.
📝 Suggested Javadoc improvement
-* Throws if errors occurred during the request forwarding process. +* Thrown when errors occur during the request forwarding process.financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.service/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/service/ConsentCoreService.java (1)
766-774: Consider completing the Javadoc documentation.The Javadoc has incomplete parameter, return, and exception descriptions. Additionally, there are minor grammar issues: "consents which has" should be "consents which have" and "a expiring" should be "an expiring".
📝 Proposed Javadoc improvement
/** - * This method is used to fetch consents which has a expiring time as a consent attribute. - * (eligible for expiration) - * `@param` statusesEligibleForExpiration - * `@return` - * `@throws` ConsentManagementException + * This method is used to fetch consents which have an expiring time as a consent attribute + * and are eligible for expiration. + * + * `@param` statusesEligibleForExpiration the consent statuses that are eligible for expiration + * `@return` a list of detailed consent resources eligible for expiration + * `@throws` ConsentManagementException if an error occurs while retrieving the consents */financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/impl/ExternalAPIConsentRetrievalStep.java (1)
161-169: Consider checking for empty display data before adding to JSON.The current implementation adds an empty
JSONArrayto the response whendisplayDatais non-null but contains no items. Consider adding an emptiness check for consistency with how the UI handles this data (the JSP checksnot empty displayData).♻️ Suggested refinement
// Append display data, if exists, to json object - if (responseDTO.getDisplayData() != null) { + if (responseDTO.getDisplayData() != null && + responseDTO.getDisplayData().getDisplayData() != null && + !responseDTO.getDisplayData().getDisplayData().isEmpty()) { jsonObject.put( ConsentAuthorizeConstants.DISPLAY_DATA, new JSONArray(responseDTO.getDisplayData().getDisplayData()) ); } -financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp (1)
99-120: Hardcoded UI strings limit internationalization.The text content ("Accounts Unavailable To Share", explanation text, "Why can't I share these?") is hardcoded in English. Other strings in this file are passed via session attributes (e.g.,
${buttonNext},${appRequestsDetails}). Consider externalizing these strings for consistency and i18n support.
...o2/financial/services/accelerator/consent/mgt/extensions/authorize/model/DisplayDataDTO.java
Outdated
Show resolved
Hide resolved
Comment on lines
+127
to
+132
| <c:forEach var="record" items="${displayData}"> | ||
| <label> | ||
| ${record.displayName} | ||
| </label> | ||
| <br/> | ||
| </c:forEach> |
There was a problem hiding this comment.
XSS vulnerability: Escape user-facing output.
${record.displayName} is rendered without escaping. If displayName contains HTML or JavaScript, it will be executed in the user's browser. Use <c:out> to safely escape the value.
🔒 Proposed fix
<c:forEach var="record" items="${displayData}">
<label>
- ${record.displayName}
+ <c:out value="${record.displayName}"/>
</label>
<br/>
</c:forEach>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <c:forEach var="record" items="${displayData}"> | |
| <label> | |
| ${record.displayName} | |
| </label> | |
| <br/> | |
| </c:forEach> | |
| <c:forEach var="record" items="${displayData}"> | |
| <label> | |
| <c:out value="${record.displayName}"/> | |
| </label> | |
| <br/> | |
| </c:forEach> |
🤖 Prompt for AI Agents
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp`
around lines 127 - 132, The JSP prints unescaped user input via
record.displayName inside the c:forEach, causing XSS; replace the raw EL output
with the JSTL escape mechanism (use the c:out tag with the record.displayName
value) so the value is HTML-escaped before rendering, and ensure the JSTL core
taglib (prefix c) is declared in the JSP if not already present.
…ist of sections that can be customized
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In
`@financial-services-accelerator/common-mediation-policies/consent-enforcement/consent-enforcement-payload-mediator/pom.xml`:
- Line 111: The coverage minimum in the consent-enforcement-payload-mediator
pom.xml was lowered to 0.7 and should either be restored to 0.8 and supplemented
with tests or the rationale recorded; restore the <minimum> value back to 0.80
in the pom.xml for the consent-enforcement-payload-mediator module and add unit
tests exercising the new code paths to meet the original gate, or if you intend
to keep 0.7 permanently, add a clear justification in the PR description or
commit message referencing the consent-enforcement-payload-mediator module and
the coverage change so future maintainers understand why the threshold was
lowered.
In
`@financial-services-accelerator/common-mediation-policies/consent-enforcement/consent-enforcement-payload-mediator/src/main/java/org/wso2/financial/services/apim/mediation/policies/consent/enforcement/constants/ConsentEnforcementConstants.java`:
- Line 45: The DOMS_ENABLED constant in ConsentEnforcementConstants
(ConsentEnforcementConstants.DOMS_ENABLED) is defined as an empty string and
unused; either remove this unused constant or set it to the proper configuration
key string consistent with other "Configs" entries (e.g., the actual DOMS config
path in your environment) and update any code that should reference it to use
ConsentEnforcementConstants.DOMS_ENABLED; locate the constant in the
ConsentEnforcementConstants class and delete it if not needed, or replace the
empty value with the correct config key and run a search to replace any
hardcoded strings to reference this constant.
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/model/DisplayDataInnerItemDTO.java`:
- Around line 79-82: The addItem method in DisplayDataInnerItemDTO currently
adds the provided Map to displayList without validating it, which allows a null
entry to be stored and later cause NPEs; update the addItem(Map<String, Object>
item) method to first validate that item is not null (e.g., if (item == null)
throw new IllegalArgumentException("item cannot be null")) and also ensure the
displayList field is initialized before adding (initialize displayList if null),
so callers either get a clear exception for invalid input or the item is safely
added.
- Around line 1-14: This file is missing the standard Apache 2.0 license header;
add the project's canonical license comment block (including copyright owner and
Apache 2.0 terms) directly above the package declaration in the
DisplayDataInnerItemDTO class file so it matches other files in the repo (ensure
the header appears before "package
org.wso2.financial.services.accelerator.consent.mgt.extensions.authorize.model;"
and preserve the existing imports and class declaration).
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp`:
- Line 99: The HTML uses a static id "UnavailableAccountPopover" inside the
<c:forEach> loop which creates duplicate ids; update the id generation inside
the loop (the element with id "UnavailableAccountPopover") to include a unique
suffix (for example using the loop index or a unique account identifier like
${status.index} or ${account.id}) and update any JS that queries it
(document.getElementById or selectors) to use the same constructed id so each
iteration yields a unique id and lookups remain correct.
- Around line 105-107: The element id "unavailablePopoverContentElement" is
declared inside the loop and will be duplicated; change it to produce unique
identifiers per iteration or use a class plus a data attribute instead so
tooltip JS can target each instance. Specifically, update the JSP output for the
anchor with id "unavailablePopoverContentElement" to either append the loop
index (from the loop variable or c:forEach varStatus) to the id (e.g.,
unavailablePopoverContentElement_${status.index}) or replace the id with a class
(e.g., unavailablePopoverContentElement) and a unique data attribute (e.g.,
data-section="${section.id}") and then update the tooltip initialization code to
select by that class/data attribute.
- Around line 88-119: The EL outputs in the JSP (section.heading,
section.subHeading, section.description, record.displayName) are rendered
unescaped and pose an XSS risk; update the fs_default_account_selection.jsp to
render these user-facing values using JSTL escaping (replace raw EL like
${section.heading} etc. with <c:out value="${section.heading}" /> and similarly
for ${section.subHeading}, ${section.description}, and ${record.displayName}) so
external API content is HTML-escaped before insertion into the DOM.
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.consent.mgt.endpoint/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/endpoint/api/ConsentValidationEndpoint.java`:
- Around line 202-208: ConsentValidationEndpoint currently puts raw JSON into
responsePayload under ConsentExtensionConstants.CONSENT_INFO when
ConsentUtils.isCdsDeploymentEnabled() is true, but downstream
PaymentService.getRequest() and VrpService.getRequest() expect a JWT (three
dot-separated segments); change the endpoint to always send a signed JWT: call
ConsentUtils.signJWTWithDefaultKey(consentInfoPayload) for both branches (i.e.,
remove or bypass the raw JSON branch) so CONSENT_INFO is always a JWT, ensuring
compatibility with the existing getRequest() parsing logic in PaymentService and
VrpService.
🧹 Nitpick comments (1)
financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.consent.mgt.endpoint/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/endpoint/api/ConsentValidationEndpoint.java (1)
209-212: Misleading error message when CDS deployment is enabled.The catch block logs "Error occurred while getting private key", but when CDS deployment is enabled, no private key retrieval occurs. If
generatePayload()throws, this message would be confusing in logs. Consider a more generic error message.Suggested improvement
} catch (Exception e) { - log.error("Error occurred while getting private key", e); - throw new ConsentException(ResponseStatus.INTERNAL_SERVER_ERROR, "Error while getting private key"); + log.error("Error occurred while building consent validation response payload", e); + throw new ConsentException(ResponseStatus.INTERNAL_SERVER_ERROR, + "Error while building consent validation response"); }
...r/common-mediation-policies/consent-enforcement/consent-enforcement-payload-mediator/pom.xml
Outdated
Show resolved
Hide resolved
...vices/apim/mediation/policies/consent/enforcement/constants/ConsentEnforcementConstants.java
Outdated
Show resolved
Hide resolved
...ial/services/accelerator/consent/mgt/extensions/authorize/model/DisplayDataInnerItemDTO.java
Outdated
Show resolved
Hide resolved
...ial/services/accelerator/consent/mgt/extensions/authorize/model/DisplayDataInnerItemDTO.java
Outdated
Show resolved
Hide resolved
...ervices.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp
Outdated
Show resolved
Hide resolved
...ervices.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp
Outdated
Show resolved
Hide resolved
...ervices.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp
Outdated
Show resolved
Hide resolved
.../wso2/financial/services/accelerator/consent/mgt/endpoint/api/ConsentValidationEndpoint.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/js/tooltip-functions.js`:
- Around line 1-6: The current code calls $(document).ready) and does element
lookups into $infoIcon and $hiddenContent before checking that $/jQuery exists,
which will throw if jQuery is missing; to fix, first verify window.jQuery (or $)
is defined and that $.fn.popover is a function, then inside that guard call
$(document).ready and perform the element lookups for
'#unavailablePopoverContentElement' and '#UnavailableAccountPopover' and proceed
only if those jQuery selections have length; adjust the logic around the
existing symbols $(document).ready, $.fn.popover, $infoIcon and $hiddenContent
so no $() is invoked unless $ is present.
- Around line 8-16: The popover currently uses unescaped HTML from
`#UnavailableAccountPopover` (populated from section.description) and is
initialized with $infoIcon.popover({ html: true, ... }), which allows XSS; fix
by escaping the JSP EL where these values are rendered (wrap
section.description, section.heading, section.subHeading with <c:out
value="${...}" escapeXml="true"/> in fs_default_account_selection.jsp) and/or
stop treating the popover content as raw HTML (remove or set html:false in the
$infoIcon.popover call or ensure the JS uses the escaped text from
`#UnavailableAccountPopover`) so the content is rendered as text not executable
HTML.
...nancial.services.accelerator.authentication.endpoint/src/main/webapp/js/tooltip-functions.js
Outdated
Show resolved
Hide resolved
...nancial.services.accelerator.authentication.endpoint/src/main/webapp/js/tooltip-functions.js
Outdated
Show resolved
Hide resolved
duleepa-a
changed the title
duleepa-a
changed the title
Ashi1993
reviewed
Feb 19, 2026
...es/accelerator/consent/mgt/extensions/authorize/model/PopulateConsentAuthorizeScreenDTO.java
Show resolved
Hide resolved
Ashi1993
reviewed
Feb 19, 2026
...financial.services.accelerator.authentication.endpoint/src/main/webapp/css/custom-common.css
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/js/tooltip-functions.js (1)
20-22: Nit: inconsistent indentation on line 21 (return;uses 3 spaces, rest of file uses 4).♻️ Proposed fix
if (typeof $ !== 'function') { - return; + return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/js/tooltip-functions.js` around lines 20 - 22, The indentation for the early return inside the conditional that checks for jQuery ("if (typeof $ !== 'function') { return; }") is inconsistent; update the whitespace so the "return;" uses the same 4-space indentation as the rest of the file (align the "return;" to match existing indentation style within tooltip-functions.js) to keep formatting consistent.financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/model/AdditionalDisplayDataDTO.java (2)
52-53:@ValidonList<Map<String, Object>>is a no-op.Bean validation cascades
@Validto eachMap<String, Object>element, butMapitself carries no constraint annotations, so nothing is actually validated. This annotation is misleading and can safely be removed.♻️ Proposed fix
- `@Valid` private List<Map<String, Object>> displayList = new ArrayList<>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/model/AdditionalDisplayDataDTO.java` around lines 52 - 53, The `@Valid` on the displayList field in AdditionalDisplayDataDTO is a no-op because it would cascade validation to each Map<String,Object> but Map elements have no constraint annotations; remove the `@Valid` annotation from the displayList declaration (the List<Map<String, Object>> displayList field in class AdditionalDisplayDataDTO) to avoid misleading code and keep the field as a plain List initialized to new ArrayList<>(); if validation of map contents is required later, replace the Map with a dedicated DTO with constraint annotations and then reintroduce `@Valid` on the List.
97-100:addItemsilently acceptsnulland the expected map key ("displayText") is an undocumented implicit contract.The JSP renders
${record.displayText}, meaning callers must populate each item map with key"displayText". This is nowhere documented in the API. Consider either a null-guard or a Javadoc note documenting expected keys.♻️ Proposed improvement
/** * Convenience method to add a display item. + * The item map should contain at minimum a {`@code` "displayText"} key + * for rendering in the consent authorization UI. + * + * `@param` item non-null map of display key-value pairs + * `@throws` IllegalArgumentException if item is null */ public AdditionalDisplayDataDTO addItem(Map<String, Object> item) { + if (item == null) { + throw new IllegalArgumentException("Display item must not be null"); + } this.displayList.add(item); return this; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/model/AdditionalDisplayDataDTO.java` around lines 97 - 100, The addItem method in AdditionalDisplayDataDTO currently accepts null and relies on an undocumented implicit key "displayText"; update the addItem(Map<String,Object> item) implementation to guard against null (reject null with IllegalArgumentException or ignore) and validate that item.containsKey("displayText") (throw IllegalArgumentException if missing), or alternatively add a clear Javadoc on AdditionalDisplayDataDTO.addItem and the displayList field specifying that each map must contain the "displayText" key (and any other required keys) and their expected value types so callers and JSP rendering (${record.displayText}) are explicit about the contract.financial-services-accelerator/components/org.wso2.financial.services.accelerator.common/src/main/java/org/wso2/financial/services/accelerator/common/constant/FinancialServicesConstants.java (1)
62-62:CONSENT_RESPONSE_PAYLOAD_SIGNINGnaming is inconsistent with other boolean config constants.Boolean-flag constants in this class use the
IS_prefix or_ENABLEDsuffix (e.g.,IS_PSU_FEDERATED,IS_CONSENT_PERIODICAL_EXPIRATION_ENABLED,IS_CONSENT_AMENDMENT_HISTORY_ENABLED). Consider renaming toIS_CONSENT_RESPONSE_PAYLOAD_SIGNING_ENABLEDfor consistency.♻️ Proposed rename
- public static final String CONSENT_RESPONSE_PAYLOAD_SIGNING = "Consent.Validation.ResponsePayloadSigning.Enabled"; + public static final String IS_CONSENT_RESPONSE_PAYLOAD_SIGNING_ENABLED = "Consent.Validation.ResponsePayloadSigning.Enabled";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@financial-services-accelerator/components/org.wso2.financial.services.accelerator.common/src/main/java/org/wso2/financial/services/accelerator/common/constant/FinancialServicesConstants.java` at line 62, Rename the boolean config constant CONSENT_RESPONSE_PAYLOAD_SIGNING to IS_CONSENT_RESPONSE_PAYLOAD_SIGNING_ENABLED to match the class convention (other constants like IS_PSU_FEDERATED, IS_CONSENT_PERIODICAL_EXPIRATION_ENABLED); update the constant name in FinancialServicesConstants (replace CONSENT_RESPONSE_PAYLOAD_SIGNING) and refactor all usages across the codebase to reference IS_CONSENT_RESPONSE_PAYLOAD_SIGNING_ENABLED to avoid compilation errors and keep naming consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp`:
- Line 113: The JSP is referencing the wrong map key: replace uses of
${record.displayText} in fs_default_account_selection.jsp with
${record.displayName} so it matches the backend map keys populated in Utils (and
TestConstants), or alternatively change the backend to populate the
"displayText" key in Utils.java/TestConstants.java; ensure the JSP, Utils (map
population), and TestConstants all use the same key ("displayName" preferred) to
avoid silent rendering failures.
---
Duplicate comments:
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp`:
- Around line 93-116: The JSP outputs from the additionalSections block render
unescaped EL values and can lead to XSS; replace each raw EL usage (e.g.,
${section.heading}, ${section.subHeading}, ${section.description}, the
title="${section.subHeading}", and ${record.displayText}) with JSP escaping
using <c:out value="..."/> (use <c:out value='${section.subHeading}'/> for the
title attribute) so that all user-provided fields are HTML-escaped before
rendering.
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/js/tooltip-functions.js`:
- Around line 35-41: The popover is created with html: true and content pulled
from $hiddenContent.html(), which allows unescaped section.description to
execute as HTML; change the popover to use safe text content by setting html:
false and passing $hiddenContent.text() to $infoIcon.popover (or alternatively
ensure the JSP fully escapes section.description before inserting it into
.unavailable-account-popover) so that the popover displays literal text rather
than executing injected markup; locate the $infoIcon.popover call in
tooltip-functions.js and update the html and content usage accordingly.
---
Nitpick comments:
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.common/src/main/java/org/wso2/financial/services/accelerator/common/constant/FinancialServicesConstants.java`:
- Line 62: Rename the boolean config constant CONSENT_RESPONSE_PAYLOAD_SIGNING
to IS_CONSENT_RESPONSE_PAYLOAD_SIGNING_ENABLED to match the class convention
(other constants like IS_PSU_FEDERATED,
IS_CONSENT_PERIODICAL_EXPIRATION_ENABLED); update the constant name in
FinancialServicesConstants (replace CONSENT_RESPONSE_PAYLOAD_SIGNING) and
refactor all usages across the codebase to reference
IS_CONSENT_RESPONSE_PAYLOAD_SIGNING_ENABLED to avoid compilation errors and keep
naming consistent.
In
`@financial-services-accelerator/components/org.wso2.financial.services.accelerator.consent.mgt.extensions/src/main/java/org/wso2/financial/services/accelerator/consent/mgt/extensions/authorize/model/AdditionalDisplayDataDTO.java`:
- Around line 52-53: The `@Valid` on the displayList field in
AdditionalDisplayDataDTO is a no-op because it would cascade validation to each
Map<String,Object> but Map elements have no constraint annotations; remove the
`@Valid` annotation from the displayList declaration (the List<Map<String,
Object>> displayList field in class AdditionalDisplayDataDTO) to avoid
misleading code and keep the field as a plain List initialized to new
ArrayList<>(); if validation of map contents is required later, replace the Map
with a dedicated DTO with constraint annotations and then reintroduce `@Valid` on
the List.
- Around line 97-100: The addItem method in AdditionalDisplayDataDTO currently
accepts null and relies on an undocumented implicit key "displayText"; update
the addItem(Map<String,Object> item) implementation to guard against null
(reject null with IllegalArgumentException or ignore) and validate that
item.containsKey("displayText") (throw IllegalArgumentException if missing), or
alternatively add a clear Javadoc on AdditionalDisplayDataDTO.addItem and the
displayList field specifying that each map must contain the "displayText" key
(and any other required keys) and their expected value types so callers and JSP
rendering (${record.displayText}) are explicit about the contract.
In
`@financial-services-accelerator/internal-webapps/org.wso2.financial.services.accelerator.authentication.endpoint/src/main/webapp/js/tooltip-functions.js`:
- Around line 20-22: The indentation for the early return inside the conditional
that checks for jQuery ("if (typeof $ !== 'function') { return; }") is
inconsistent; update the whitespace so the "return;" uses the same 4-space
indentation as the rest of the file (align the "return;" to match existing
indentation style within tooltip-functions.js) to keep formatting consistent.
...ervices.accelerator.authentication.endpoint/src/main/webapp/fs_default_account_selection.jsp
Show resolved
Hide resolved
malshaniS
reviewed
Feb 23, 2026
...ava/org/wso2/financial/services/accelerator/common/config/FinancialServicesConfigParser.java
Show resolved
Hide resolved
Ashi1993
reviewed
Feb 23, 2026
...is/carbon-home/repository/resources/conf/templates/repository/conf/financial-services.xml.j2
Outdated
Show resolved
Hide resolved
imesh94
reviewed
Feb 27, 2026
...vices/accelerator/consent/mgt/extensions/authorize/impl/ExternalAPIConsentRetrievalStep.java
Show resolved
Hide resolved
imesh94
reviewed
Feb 27, 2026
...vices/accelerator/consent/mgt/extensions/authorize/impl/ExternalAPIConsentRetrievalStep.java
Outdated
Show resolved
Hide resolved
- Extend account model to carry tooltip title and description - Render tooltip icon and hidden tooltip content for account entries - Support tooltip content updates for select-based account picker - Rename generic tooltip content class for reuse across tooltip scenarios
malshaniS
approved these changes
Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
Enable Unavailable Accounts Display in Authorization Screen to Support CDS Toolkit
This change enhances the Accelerator to support CDS Toolkit authorization screen requirements while keeping the implementation generic and reusable.
Added a reusable displayData structure to the consent authorization response to enable displaying unavailable accounts on the authorization screen, allowing the CDS Toolkit to meet its UI requirements.
Introduced the is_cds_deployment configuration property to control JWT signing behavior in the consent enforcement policy. Since CDS deployments include a downstream policy that performs JWT signing after consent enforcement, this configuration prevents double-signing by skipping signing at the consent enforcement stage.
Development Checklist
Testing Checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style