Aman-Mittal opened a new issue, #663:
URL: https://github.com/apache/fineract-backoffice-ui/issues/663
A screen lists records from one domain and links to another, where the
destination route carries a read code the listing itself does not require. A
user who can reach the list is therefore offered a control that can only land
on `/forbidden`.
This is not a security problem — Fineract refuses the operation either way,
and the route guard does refuse the navigation. It is a false promise, and it
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, and say
nothing about whether the application offered the control in the first place.
## Confirmed against a real instance
Signed in as a seeded user holding only `READ_CLIENT`, 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}`.
## Instances found
| Screen | Listing needs | Control goes to | Which needs |
|---|---|---|---|
| Client → account tabs | `READ_CLIENT` | loan / savings / fixed / recurring
screens | each account's own read code |
| Group → members | `READ_GROUP` | `/clients/view/:id` | `READ_CLIENT` |
| Center → groups | `READ_CENTER` | `/groups/view/:id` | `READ_GROUP` |
| Asset owner | ungated | `/loans/view/:id` | `READ_LOAN` |
| Offices list | `READ_OFFICE` | `/organization/offices/edit/:id` |
`UPDATE_OFFICE` |
| Office transactions | `READ_OFFICETRANSACTION` |
`.../office-transactions/create` | `CREATE_OFFICETRANSACTION` |
## The two correct treatments
The codebase already encodes the distinction in its two directives, and it
turns on *what the control does* rather than how privileged it is:
- **Removed** (`*appHasPermission`) — anything that navigates elsewhere: a
nav entry, a Create button that opens a form. There is nothing to explain; the
destination is simply not part of this user's application.
- **Disabled with the reason** (`appRequiresPermission`) — an action on the
record already on screen. Hiding it would leave the user to conclude the
feature is missing.
A third case was not covered by either: a **cross-domain link in a list**.
Both directives replace the element and neither has an else-branch, but the
record's name should still render — its existence is not the secret, and
dropping the row would misrepresent the data. `createPermissionCheck()` was
added for that.
## How these were found, and the limits of it
A static audit of controls navigating to a permission-gated route without a
matching guard produced 15 candidates; live probing reduced that to the six
above. The rest were list-row links requiring the same read code the user
already needed to reach the list, or controls already carrying
`appRequiresPermission`, which disables rather than removes and so blocks the
click.
**Roughly half the static predictions did not survive contact with a real
backend**, so each row in the table above is backed by an observed click rather
than by the audit alone. Anyone extending this should probe rather than trust a
grep.
## Status
- Offices edit button — fixed in #657
- Client account links, Office Transactions create — fixed in #660
- Group members, Center groups, Asset owner — #662
The audit was not exhaustive: it only inspected `[link]`, `[routerLink]` and
`routerLink` targets in component templates. Controls that navigate from a
`(click)` handler calling `router.navigate(...)` were not covered and may hold
more instances.
Part of #123.
--
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]
