Aman-Mittal opened a new pull request, #217:
URL: https://github.com/apache/fineract-backoffice-ui/pull/217
Closes #212, #213, #214, #215, #216.
`npm run ga:check` reported **3/8 gates passing, 4 blocking failures**. This
takes it to **7/8,
0 blocking** — the remaining one is the advisory adapter-boundary backlog.
I confirmed each failure in the code before changing anything, rather than
taking the report's
word for it.
### Why these were worse together than apart
The login form offered third-party hosts as one-click options. The endpoint
override accepted any
string and persisted it. The auth interceptor attached `Authorization:
Basic` to **every** outgoing
request regardless of destination — no URL comparison appeared anywhere in
the file.
Chain them and you have a supported path to pointing the application at an
attacker's host and
posting real credentials to it on the next sign-in. Missing security headers
removed the
browser-side backstop that would otherwise limit where an injected script
could send anything.
Nothing here calls a third party today, which is exactly why this was cheap
now: the first
analytics pixel, error reporter or vendor SDK sharing this `HttpClient`
would have inherited the
problem silently.
### What changed
- **Interceptor** sends credentials — and the tenant header — only to the
configured API. Relative
URLs are same-origin by construction; absolute ones are compared by
origin, so a cross-origin
API the deployment configured still authenticates and nothing else does.
The tenant header is
withheld too, since it names the institution.
- **`setApiUrl`** validates against an allow-list and *returns* whether it
accepted, so the login
form stops **before** authenticating rather than after. A stored override
clears the same bar:
local storage is writable by anything running as the page, so it is not
more trusted than a
fresh value.
- **Server picker** is built from deployment configuration instead of
hard-coded hosts.
- **`deploy/nginx.conf`** sets CSP, HSTS, `X-Frame-Options`,
`X-Content-Type-Options` and
`Referrer-Policy`, all with `always` so they survive error responses —
which is where a
reflected payload would land. The CSP admits no `unsafe-eval`;
`unsafe-inline` is styles-only.
### A bug I caught while writing it
The allow-list check initially read the config signal. But
`getStoredOverride` runs inside a
**field initialiser**, before that signal exists — it would have thrown on
startup, in the one code
path every user hits. The predicate now takes the allowed origins as an
argument, and the loader
passes the freshly-loaded list rather than reading back a config that has
not been merged yet.
### Verification
| Check | Result |
|---|---|
| `npm run ga:check` | **3/8 → 7/8**, 0 blocking |
| Unit tests | **766 passing** (744 → 766) |
| Mocked Playwright | **210/210 passing** |
| `nginx -t` on the real image | passes |
| Headers observed on a live response | all five present |
| `tsc` (app + spec), build, lint, format, i18n, icons, licence | clean |
Headers were checked by serving the config from the actual `nginx:alpine`
image and reading the
response, not by grepping the file.
### Two notes for review
The existing `ConfigService` specs asserted the **old permissive behaviour**
— they set
cross-origin URLs and expected them to stick. Those now assert the new rule,
with added cases for
refusal and for a non-allow-listed stored value being ignored.
Storage access in that spec is funnelled through three helpers, which takes
the adapter-boundary
suppression baseline **down rather than up: 435 → 431**. New violations
would have failed lint;
adding suppressions would have been the wrong direction.
--
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]
