Aman-Mittal opened a new pull request, #660:
URL: https://github.com/apache/fineract-backoffice-ui/pull/660
Two screens offered controls to users whose only destination was
`/forbidden`. Found by driving a real Fineract instance as seeded users with
different permission combinations, then confirmed against the platform.
Neither is a security problem — Fineract refuses the operation either way —
but both are false promises, and this is the one class of RBAC defect that
route coverage cannot find. The existing `rbac-*` specs prove the guard refuses
a route the user cannot open; they say nothing about whether the application
offered the control in the first place.
## 1. A client's account links
A client's accounts come back from Fineract with `READ_CLIENT` alone, but
each account screen carries its own permission code. A user holding only
`READ_CLIENT` was shown every loan, savings and deposit account number as a
link that could only land on Access Denied.
Observed directly: signed in as a seeded `READ_CLIENT` user, opened the Loan
Accounts tab of a client with an active loan, clicked the account number,
landed on `/forbidden`. The same click as a user who also holds `READ_LOAN`
opens `/loans/view/{id}`.
The number is still rendered; only the link is withheld. The account's
existence is not the secret, and dropping the row would leave the reader
wondering where the client's loan went. Applied to all four account tables —
loans, savings, fixed deposits, recurring deposits each have their own read
code and the same defect.
This mirrors `HasPermissionDirective` rather than using it, `rbacEnabled`
short-circuit included, because the directive has no else-branch and the
plain-text fallback is the point.
## 2. The office transactions list
This screen builds its own card header and `cdk-table` instead of using
`app-data-table`, so it never picked up the two things that component gives
every other list:
- **Create was ungated.** A seeded user holding `READ_OFFICETRANSACTION` +
`READ_OFFICE` was offered the button and landed on Access Denied. Now gated
with `*appHasPermission`, which removes it — the convention for a control that
navigates elsewhere.
- **The load error handler logged to the console and nothing else.** The
global interceptor's toast does fire, but it fades, and what it leaves behind
is an empty table under its column headers, which reads as "there are no office
transactions." It now keeps a message and distinguishes a refusal from a
failure, reusing `COMMON.ERRORS.LOAD_FORBIDDEN`.
The refusal is the likely case rather than an edge one: **Fineract answers
403 to `GET /officetransactions` for a role without `READ_OFFICE`, which the
route does not declare.** So a user holding exactly what the route asks for
reaches a screen that cannot load. Worth a reviewer's opinion on whether the
route should declare both codes — I did not change it, because that changes who
sees the nav entry and is a product call.
The delete action takes `appRequiresPermission` instead, since it acts on a
row already on screen.
## Coverage
`e2e/rbac-dead-end-controls.spec.ts`, registered in `BACKEND_SPECS`. Each
assertion is paired with a user who **does** hold the code, because "the
control is absent" passes just as well against a screen that renders nothing at
all, and each is checked against the platform's own answer rather than against
another part of the UI.
```
npx playwright test --project=backend rbac-dead-end-controls.spec.ts
6 passed (53.3s)
npm run build bundle complete
npm run lint clean
npm run test:unit 278 files, 1816 tests passed
npm run typecheck:e2e clean
npm run check:icons all registered (117 icons)
npm run i18n:check no missing keys, catalogues consistent
npm run check:template-text no new untranslated text
npm run check:a11y-names every icon-only button has a name
npm run check:route-permissions 317 screens agree
npm run check:ui-primitives clean
npm run format:check clean
scripts/check-license.sh clean
```
## Not covered, and why
The delete gating has no e2e assertion: an office transaction cannot be
seeded on `apache/fineract:latest`, which answers 403 with `column
"currency_multiplesof" of relation "m_office_transaction" does not exist`.
**The create flow for office transactions is therefore non-functional against
that image regardless of permissions** — a platform/migration issue, not a UI
one, and worth raising separately. The directive itself has unit coverage.
## How these were found
A static audit of controls that navigate to a permission-gated route without
a matching guard produced 15 candidates. Live probing reduced that to the two
here: most of the rest were list-row links requiring the same read code the
user already needed to reach the list, or controls carrying
`appRequiresPermission`, which disables rather than removes and so blocks the
click. Worth stating plainly — roughly half the static predictions did not
survive contact with a real backend, which is why each one here is backed by an
observed click rather than by the audit.
--
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]
