Aman-Mittal opened a new issue, #691:
URL: https://github.com/apache/fineract-backoffice-ui/issues/691
`loans.routes.ts` gates every loan transaction form on `UPDATE_LOAN`:
```ts
{
path: ':loanId/transactions/:type',
canActivate: [authGuard, permissionGuard],
data: { permissions: 'UPDATE_LOAN' },
loadComponent: () => import('./loan-transaction-form.component')…
}
```
That one declaration serves disburse, repayment and the rest. It disagrees
with the platform in **both** directions, and it disagrees with the
application's own action gates.
## The platform's answer
Asked with an empty body, so nothing is written — Fineract authorises before
it validates, so a code-holder gets 400 and a non-holder 403:
| Account holds | `POST /loans/{id}?command=disburse` | `POST
/loans/{id}/transactions?command=repayment` |
| --- | --- | --- |
| `DISBURSE_LOAN` | **400** — authorised | — |
| `REPAYMENT_LOAN` | — | **400** — authorised |
| `UPDATE_LOAN` | **403** — refused | **403** — refused |
Verified against the local e2e stack. `UPDATE_LOAN` is
`portfolio`/`LOAN`/`UPDATE`; the commands live in the `transaction_loan`
grouping as `DISBURSE_LOAN` and `REPAYMENT_LOAN`.
## What the application does
The action layer is correct — `loan-view.component.ts` gates the buttons on
the codes the platform actually wants:
```html
<ion-button data-testid="loan-repayment-action"
appRequiresPermission="REPAYMENT_LOAN" …>
<ion-button data-testid="loan-disburse-action"
appRequiresPermission="DISBURSE_LOAN" …>
```
So the two layers contradict each other:
1. **A `DISBURSE_LOAN` holder is offered an enabled Disburse button and is
then refused the form.** The route wants `UPDATE_LOAN`, which they do not hold
and — per the table — do not need. The only control the screen offers them
leads to Access Denied. Same for `REPAYMENT_LOAN` and Repayment.
2. **An `UPDATE_LOAN` holder is admitted to a form whose submit can only
403.** `DOCS/RBAC.md` states the client-side gate exists so that "a user is not
led into a screen whose every request will 403". For this route that does not
hold.
Not a security issue — Fineract refuses the operation either way. It is a
false promise, the class `rbac-dead-end-controls.spec.ts` exists to catch.
## Already covered
`e2e/loan-flows-non-admin-roles.spec.ts` asserts both halves against a real
platform, with the accounts built through the UI rather than seeded:
- "the disburser is offered the control, and the route refuses it" — expects
`/forbidden`
- "and the same route admits an `UPDATE_LOAN` holder the platform will
refuse" — expects the form
Each carries a message saying that a pass after the fix means the assertion
needs inverting, so the spec will fail loudly rather than silently assert the
old behaviour.
## Suggested fix
The required code depends on `:type`, so no single declaration can be right
— the same argument `products/:accountType/:accountId/action/:command` is
already recorded under in `scripts/check-route-permissions.mjs`:
> Dispatch route: the permission depends on `:accountType` and `:command` at
runtime, so any single declaration here would be wrong for most of the commands
it serves
Two options:
1. **Map `:type` to its code**, in `permissionGuard` or a small
`loanTransactionPermission(type)` helper, so each command is gated on the code
the platform enforces. Keeps the guard's promise.
2. **Record the route as a dispatch route** in `UNRESTRICTED` with the same
reasoning as the products one, and rely on the action gates plus the platform.
Honest, and less work, but it gives up the "no screen that will only 403"
guarantee for this URL.
(1) looks preferable: the type is in the path, so the code is knowable
before the component loads, which is not true of the products dispatch route.
## Also in that branch
`loan-view.component.ts` had no `data-testid` on the Disburse button, unlike
Approve and Repayment. That matters beyond tests: when `appRequiresPermission`
refuses a control it replaces the accessible name with the reason, so a
name-based locator stops finding the button in exactly the state a permission
check needs to assert on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
