priyam99 opened a new pull request, #170:
URL: https://github.com/apache/fineract-loan-origination/pull/170
## Description
When a customer asks for an application that belongs to someone else, the
ownership check correctly blocks it, but the API answers with a 500. Besides
looking like a server crash, that also gives something away: a reference that
doesn't exist comes back as 404, so a 500 tells you the application is real and
just isn't yours. Since references are sequential (`LOS-2026-00001`, `00002`,
…), that's enough to probe for other customers' applications.
The cause is in `CustomerLoanApplicationController.assertOwnedByCaller()`.
It throws `AccessDeniedException`, which `GlobalExceptionHandler` doesn't
handle, so it ends up in the catch-all 500.
I changed it to throw `ApplicationNotFoundException`, the same exception the
service throws when a reference doesn't exist. A non-owner now gets exactly the
same 404 and message as for a missing application, so the two can't be told
apart. No new handler was needed, since that exception already maps to 404.
This is also what `docs/api/rest-api.adoc` already promises: "Returns `404` if
not found or owned by a different customer."
The same check guards three endpoints, so all of them are covered:
application details, credit score, and submit. The issue only mentions the
first two, but submit had the same problem.
I kept this separate from #159 on purpose. Once ownership failures stop
using `AccessDeniedException`, mapping that exception to 403 in #159 can't
accidentally turn these into 403s and leak the same information.
Closes #160
## Verification
I ran `./mvnw spotless:apply`, `./mvnw apache-rat:check` and the full
`./mvnw clean verify` with Docker running. Everything passes: 196 unit tests
and 7 Cucumber scenarios.
New tests in `CustomerLoanApplicationControllerTest` (MockMvc with the real
service and a logged-in customer, only the repositories mocked):
- the owner can view their application and its credit score (200)
- another customer gets 404 with no application data for details and credit
score, and the score is never read
- another customer gets 404 trying to submit it, and it stays `DRAFT`
- the 404 another customer gets is the same as the 404 for a reference that
doesn't exist
With the fix taken out, the four "another customer" tests fail with 500.
I also ran the app locally (dev profile, mock Fineract) with two customers,
Alice and Bob. Alice created `LOS-2026-00001` and Bob tried to read it.
Before:
```
$ curl -H 'Authorization: Bearer <bob>'
.../api/v1/customer/loan-applications/LOS-2026-00001
{"error":"Internal Server Error","message":"An unexpected error
occurred.","status":500,...}
HTTP 500
$ curl -H 'Authorization: Bearer <bob>'
.../api/v1/customer/loan-applications/LOS-2026-99999
{"error":"Not Found","message":"No loan application found with reference
[LOS-2026-99999] for tenant [default].","status":404,...}
HTTP 404
```
After:
```
$ curl -H 'Authorization: Bearer <bob>'
.../api/v1/customer/loan-applications/LOS-2026-00001
{"error":"Not Found","message":"No loan application found with reference
[LOS-2026-00001] for tenant [default].","status":404,...}
HTTP 404
$ curl -H 'Authorization: Bearer <bob>'
.../api/v1/customer/loan-applications/LOS-2026-00001/credit-score
HTTP 404 (same body)
$ curl -X POST -H 'Authorization: Bearer <bob>'
.../api/v1/customer/loan-applications/LOS-2026-00001/submit
HTTP 404 (same body)
$ curl -H 'Authorization: Bearer <alice>'
.../api/v1/customer/loan-applications/LOS-2026-00001
HTTP 200 (owner still works, status still DRAFT after Bob's submit attempt)
```
No frontend changes.
One thing I noticed but left out of scope: if an application somehow has no
applicant profile, the ownership check fails with a different "profile not
found" 404 before the comparison runs. That shouldn't happen with normal data,
so I didn't change it here.
## AI assistance (optional)
- Tool / model: Claude Code (Claude Opus 5.5)
- How it was used: helped me trace the ownership check, draft the change and
tests, and run the build and the local before/after check. I reviewed every
line before pushing.
## Checklist
- [x] This PR is one logical change, not a code dump.
- [x] I added or updated tests, or explained why tests are not needed.
- [x] New files carry the ASF license header.
- [x] Docs are updated. (Nothing to change; the API docs already say 404 for
this case.)
- [x] All commits are signed.
- [x] I followed the [AI
Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy)
and the [ASF Generative Tooling
Guidance](https://www.apache.org/legal/generative-tooling.html).
--
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]
