FINERACT-2455: WC - Add missing permissions for transaction undo and charge creation - #6449
Conversation
5f3a8a4 to
8fbea6b
Compare
|
@oleksii-novikov-onix Please review the below finding / concern:
Lets fix this as well please
|
eeaebf4 to
a5f8258
Compare
|
|
@oleksii-novikov-onix Please review my finding: CommandWrapperConstants.java:186 — the entity rename is persisted into m_portfolio_command_source.entity_name, but no backfill ships with it, so pre-upgrade WC undo audit rows stay under ENTITY_WORKINGCAPITALLOANTRANSACTION: like 'WORKINGCAPITALLOANTRANSACTION%' audit filters miss them, and the limited-checker join on p.entity_name = aud.entity_name will never match them against the new _CHECKER permission. |
a5f8258 to
c437ec6
Compare
Fixed, added changeset wcl-0081-5 to 0081_wc_loan_missing_command_permissions.xml, backfilling m_portfolio_command_source.entity_name from ENTITY_WORKINGCAPITALLOANTRANSACTION to WORKINGCAPITALLOANTRANSACTION. |
c437ec6 to
83b1bcd
Compare
galovics
left a comment
There was a problem hiding this comment.
Checked the permission codes against what the command framework actually derives: undoWorkingCapitalLoanTransaction pairs ACTION_UNDO with ENTITY_WORKINGCAPITALLOANTRANSACTION, which after the constant fix resolves to UNDO_WORKINGCAPITALLOANTRANSACTION (before it was UNDO_ENTITY_WORKINGCAPITALLOANTRANSACTION, which matched no seeded row). createWorkingCapitalLoanCharge resolves to CREATE_WORKINGCAPITALLOANCHARGE. The handler's @CommandType uses the same constant so routing is unaffected, and the _CHECKER codes match what AppUser builds. I also compared all 36 WC @CommandType pairs against the seeded m_permission rows - after this PR every WC command has a matching permission, so nothing else is left super-user-only.
Liquibase follows the module conventions (idempotent sqlCheck/MARK_RAN, _CHECKER rows with can_maker_checker=false, no deletes, no edits to existing changesets), and renaming entity_name in m_portfolio_command_source (wcl-0081-5) keeps the audit history searchable. E2E covers allowed and 403 paths for both undo and charge creation, plus a user with only the undo permission undoing a charge adjustment.
Non-blocking:
- Five other open WC PRs (#6454, #6446, #6425, #6399, #6398) also add a
0081_*part. Not a real Liquibase conflict (identity is id + author + file), but whoever merges second needs to renumber. - The
columnExistsprecondition onm_portfolio_command_source.entity_nameinwcl-0081-5can never fail (column is in the base schema) - either drop it or replace it with asqlCheckfor rows still holding the old value. - The description says three rows but the diff also seeds
UNDO_..._CHECKERand does the command-source rewrite. - Admins now have to grant the two permissions themselves (no
m_role_permissiontouch, which is the convention) - worth a line in the release notes.
Recommendation: APPROVE
83b1bcd to
b0121b3
Compare
…issions for transaction undo and charge creation
b0121b3 to
4db1e17
Compare
Description
Undoing a Working Capital loan transaction and adding a charge to a Working Capital loan both check permission codes that have no row in m_permission, so only a super user can run them. Transaction undo also had a typo in its entity constant, which made the checked code UNDO_ENTITY_WORKINGCAPITALLOANTRANSACTION.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Your assigned reviewer(s) will follow our guidelines for code reviews.