Aman-Mittal opened a new pull request, #371:
URL: https://github.com/apache/fineract-backoffice-ui/pull/371
Refs #370
## Summary
The tenant OIDC configuration screen sent field names Fineract has never
recognised. A configured
tenant opened it to a **blank form**, and saving replaced the stored
configuration with keys the
platform discards.
It went unnoticed because nothing could catch it: `POST`/`PUT
/v1/tenants/{tenantId}/oidc-config`
declare `requestBody: {"type": "string"}` in the OpenAPI document — no
properties, no types — so
the generated client types the body as a bare `string` and no field name is
checked at compile
time. The screen's own spec asserted the same wrong names, so component and
test agreed with each
other and not with Fineract.
| Screen sent | Platform actually uses |
|---|---|
| `issuer` | `issuerUri` |
| `jwksUrl` | `jwksUri` |
| `authorizationEndpoint` | *(does not exist)* |
| `tokenEndpoint` | *(does not exist)* |
| — | `providerType` **(NOT NULL)**, `usernameClaim`, `scopes`,
`postLogoutRedirectUri` |
Verified against a running instance:
```json
{ "tenantId": "default", "providerType": "KEYCLOAK",
"issuerUri": "https://idp.example.invalid/realms/fineract";,
"clientId": "fineract-backoffice", "jwksUri": "…",
"usernameClaim": "preferred_username", "scopes": "openid,profile,email",
"enabled": true }
```
There is no authorization or token endpoint to configure: a provider
publishes both in its
discovery document at `{issuerUri}/.well-known/openid-configuration`, which
is why the platform
stores neither and why those two fields were being silently discarded.
## Changes
- **Field names corrected** to the platform's, with `providerType` as the
documented six-value list
(`KEYCLOAK`, `GOOGLE`, `AZURE_AD`, `OKTA`, `AUTH0`, `GENERIC`) rather than
free text — a wrong
value there fails only at sign-out, which is a poor place to discover a
typo.
- **A refused save is now visible.** It previously wrote to `console.error`
and nothing else, so a
failure was indistinguishable from success. That matters more than usual
here: the platform
currently refuses *every* write (see below).
- **404 means "not configured yet"**, not a load failure. The form opens on
the platform's own
defaults instead of reporting an error.
- **An untouched client secret is omitted rather than sent blank.** `GET`
never returns it, so the
previous behaviour erased the stored secret on any unrelated edit.
- **`window.confirm` → the shared confirm dialog**, marked destructive, like
the rest of the app.
- `DOCS/OIDC.md`, linked from `security.md` and `DOCS/RBAC.md`.
## Platform defects found while establishing this
Reproduced against `apache/fineract:latest`. **None is reported upstream**;
each deserves a
FINERACT ticket, and I have not filed them.
**1 — Every write fails on PostgreSQL.** `POST` and `PUT` answer 500 for any
body, including one
omitting `enabled` entirely:
```
ERROR: column "enabled" is of type smallint but expression is of type boolean
at TenantOidcConfigRepositoryJdbc.insert
```
The column is `smallint`; the repository binds `ps.setBoolean(10,
config.isEnabled())`
(`fineract-security/src/main/java/.../TenantOidcConfigRepositoryJdbc.java`).
The PostgreSQL JDBC
driver does not coerce. Reads and deletes work.
**So this PR does not make the screen able to save.** It makes it read
correctly, stop destroying
the stored secret, and tell the user when the platform says no. The
remaining failure is upstream.
**2 — Enabling OIDC federation stops Fineract from starting.** With
`FINERACT_SECURITY_OIDC_FEDERATION_ENABLED=true`:
```
The dependencies of some of the beans in the application context form a
cycle:
oidcFederationSecurityConfig ↔
dynamicJwtIssuerAuthenticationManagerResolver
```
The feature cannot be switched on at all on this build, which blocks #370
entirely.
**3 — The documented permission is not seeded.** Upstream names
`MANAGE_TENANT_OIDC_CONFIG` as
required for this endpoint; it is not among the 698 codes `GET
/v1/permissions` returns. That is
why `/system/oidc-config` stays in the drift check's `UNRESTRICTED` list — a
gate no role can
satisfy is worse than no gate.
## #370's open question, answered
**Enabling OIDC does not disable password authentication.** With a
configuration present and
`enabled: true`, `GET /v1/offices` still answers 200 to Basic auth. Upstream
confirms the design:
Bearer routes to OIDC, Basic falls through.
So #370 is **inert rather than dangerous** — the configuration does nothing
— which is a materially
lower severity than #369, where enabling two-factor authentication takes the
deployment offline.
#369 should go first.
## Context
OIDC federation is
[FINERACT-2616](https://issues.apache.org/jira/browse/FINERACT-2616)
([apache/fineract#5883](https://github.com/apache/fineract/pull/5883)),
**resolved, fix version
1.15.0**, under the in-progress
[FINERACT-1908](https://issues.apache.org/jira/browse/FINERACT-1908)
modular-security work. It is a
resource-server design: Fineract validates a JWT someone else issued.
Obtaining that token is the
client's job, and that is exactly what this application does not do — hence
#370.
## Testing
| Command | Result |
|---|---|
| `npm run test` | **1034 SUCCESS**, 0 failed (grepped for `✘`/`FAILED`) |
| `npm run lint` / `lint:prune` | pass — no new suppressions |
| `npm run format:check` | pass |
| `npm run i18n:check` | pass — 1527 keys, en/hi/ko |
| `npm run check:route-permissions` | pass — 298 screens, 115 nav entries
agree |
| `npm run build` | pass |
| `npm run ga:check` | 8/9, **0 blocking** |
| `./scripts/check-license.sh` | pass |
The component spec goes from 3 tests to 12 and now pins a **verbatim
transcript** of a real `GET`
response. That is deliberate: it is the only thing standing between the
component and inventing
field names again, since the schemaless body means the compiler cannot help.
It must stay a
transcript rather than become whatever the component reads.
Covered: the real field names on load; 404 as "not configured"; a genuine
load failure reported; a
pre-parsed body; create versus update; the untouched secret omitted; an
entered secret sent; the
success confirmation; the spinner released when the platform refuses; and
delete asking through the
shared dialog and honouring a decline.
No e2e: the screen cannot be driven end to end while defect 1 stands, and a
test that asserts a
500 would pin the bug rather than the behaviour.
## Known limitations
- **Saving still fails** on PostgreSQL, for the upstream reason above. This
PR makes that visible
instead of silent; it cannot make it work.
- Still no OIDC **login** — that is #370, and it is blocked on defect 2.
- The three platform defects are unreported upstream.
## Breaking changes
None for users. `OIDC_CONFIG.AUTH_ENDPOINT`, `OIDC_CONFIG.TOKEN_ENDPOINT` and
`OIDC_CONFIG.JWKS_URL` are removed from all three catalogues, since the
fields they labelled do not
exist. Any deployment that had typed values into those two fields was having
them discarded
already.
--
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]
