Fix issue in consent mapping persistence step

[malshaniS]

Fix issue in consent mapping persistence step

There is an issue in the consent mapping persistence step where an additional record is stored with the account ID set to 'n/a' for each consent mapping.

Issue link: https://github.com//issues/885

Doc Issue: Optional, link issue from documentation repository

Applicable Labels: Spec, product, version, type (specify requested labels)

---

Development Checklist

1. Build complete solution with pull request in place.
2. Ran checkstyle plugin with pull request in place.
3. Ran Findbugs plugin with pull request in place.
4. Ran FindSecurityBugs plugin and verified report.
5. Formatted code according to WSO2 code style.
6. Have you verified the PR doesn't commit any keys, passwords, tokens, usernames, or other secrets?
7. Migration scripts written (if applicable).
8. Have you followed secure coding standards in WSO2 Secure Engineering Guidelines?

Testing Checklist

1. Written unit tests.
2. Verified tests in multiple database environments (if applicable).
3. Tested with BI enabled (if applicable).

Summary by CodeRabbit

• Bug Fixes
  • Enhanced consent processing to handle edge cases where accounts lack authorization or consent is denied. The system now applies default permissions instead of returning an error, ensuring smoother operations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[wso2-engineering]

Log Improvement Suggestion No: 1

Suggested change

	} else if (!hasAuthorizedAccounts || !isApproved) {
	// in case a consent is denied or has no account mappings
	} else if (!hasAuthorizedAccounts || !isApproved) {
	log.debug("Consent denied or has no account mappings. hasAuthorizedAccounts: {}, isApproved: {}", hasAuthorizedAccounts, isApproved);
	// in case a consent is denied or has no account mappings

[wso2-engineering]

AI Agent Log Improvement Checklist

⚠️ Warning: AI-Generated Review Comments

• 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

[coderabbitai]

Walkthrough

Modified consent persistence logic in DefaultConsentPersistStep to unify handling of denied requests and cases with no authorized accounts, mapping both scenarios to "n/a" with default permissions rather than throwing an exception.

Changes

Cohort / File(s)	Change Summary
Consent Management 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/DefaultConsentPersistStep.java	Modified getConsentedAccounts method conditional logic: combined handling of denied requests and no-authorized-accounts scenarios into unified logic that assigns default permissions with "n/a" key instead of throwing ConsentException. Updated accompanying comment to reflect new behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A hop through consent flows, we've tightened the way,
No more exceptions when accounts stray,
With "n/a" defaults in harmony now,
Denied and empty treated—a logical bow! 🎀

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix issue in consent mapping persistence step' clearly and specifically describes the main change in the changeset, matching the documented fix in the code.
Description check	✅ Passed	The PR description includes the required issue link and explains the problem, but the checklists are incomplete with all items unchecked and several optional sections missing (automation test details, conformance test details).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfba919 and 468447a.

📒 Files selected for processing (1)

• 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/DefaultConsentPersistStep.java

🔇 Additional comments (1)

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/DefaultConsentPersistStep.java (1)

265-272: Verify: denied consent with accounts still stores both accounts and "n/a".

The fix correctly addresses the case where approved consents with accounts were incorrectly getting an "n/a" entry. However, when hasAuthorizedAccounts=true && isApproved=false (consent denied but accounts were selected), the condition !hasAuthorizedAccounts || !isApproved evaluates to true, resulting in both the accounts (added at lines 260-263) AND the "n/a" entry being stored.

Is this the intended behavior? If denied consents should only store "n/a" (not the selected accounts), consider clearing the map before adding "n/a":

Suggested fix if only "n/a" should be stored for denied consents

         if (!hasAuthorizedAccounts && isApproved) {
             log.error(ConsentAuthorizeConstants.ACCOUNT_ID_NOT_FOUND_ERROR);
             throw new ConsentException(ResponseStatus.BAD_REQUEST,
                     ConsentAuthorizeConstants.ACCOUNT_ID_NOT_FOUND_ERROR);
         } else if (!hasAuthorizedAccounts || !isApproved) {
             // in case a consent is denied or has no account mappings
+            accountIDsMapWithPermissions.clear();
             accountIDsMapWithPermissions.put("n/a", permissionsDefault);
         }

Additionally, as noted in prior review feedback, adding debug logging here would help troubleshoot consent flow issues.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}