Skip to content

Commit ca7060f

Browse files
committed
WEB-112: Sanitize GSIM fields before calculateLoanSchedule to avoid NPE
- Add sanitizeLoanPayload method to LoansService to remove unsafe GSIM linkage fields (linkAccountId, linkAccountOwnerId, linkAccountOwnerName, linkedSavingsAccount) before posting to backend - Apply sanitization in calculateLoanSchedule endpoint to prevent payload from triggering NullPointerException in Fineract LoanScheduleCalculationPlatformServiceImpl - Add unit tests for payload sanitization to verify forbidden fields are removed and allowed fields are preserved - Fixes 500 Internal Server Error when generating repayment schedule for GLIM loans linked to GSIM accounts
1 parent d89dfdb commit ca7060f

2 files changed

Lines changed: 123 additions & 1 deletion

File tree

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { TestBed } from '@angular/core/testing';
2+
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
3+
import { provideHttpClient } from '@angular/common/http';
4+
import { LoansService } from './loans.service';
5+
import { SettingsService } from 'app/settings/settings.service';
6+
import { Dates } from 'app/core/utils/dates';
7+
8+
describe('LoansService', () => {
9+
let service: LoansService;
10+
let httpMock: HttpTestingController;
11+
12+
beforeEach(() => {
13+
const mockSettings: Partial<SettingsService> = {
14+
language: { name: 'English', code: 'en' },
15+
dateFormat: 'yyyy-MM-dd',
16+
businessDate: new Date(),
17+
maxFutureDate: new Date()
18+
};
19+
const mockDates: Partial<Dates> = {
20+
formatDate: (d: any) => d,
21+
formatDateAsString: (d: any) => String(d)
22+
} as any;
23+
24+
TestBed.configureTestingModule({
25+
providers: [
26+
LoansService,
27+
provideHttpClient(),
28+
provideHttpClientTesting(),
29+
{ provide: SettingsService, useValue: mockSettings },
30+
{ provide: Dates, useValue: mockDates }]
31+
});
32+
service = TestBed.inject(LoansService);
33+
httpMock = TestBed.inject(HttpTestingController);
34+
});
35+
36+
afterEach(() => {
37+
httpMock.verify();
38+
});
39+
40+
it('should sanitize all GSIM fields before posting calculateLoanSchedule', () => {
41+
const payload = {
42+
principal: 1000,
43+
linkAccountId: 42,
44+
linkAccountOwnerName: 'Alice',
45+
linkedSavingsAccount: { id: 5 },
46+
nested: {
47+
linkAccountOwnerId: 7,
48+
linkAccountOwnerName: 'Bob',
49+
ok: 'keep'
50+
},
51+
members: [
52+
{ id: 1, linkAccountId: 99 },
53+
{ id: 2, name: 'foo', linkedSavingsAccount: { id: 9 } }
54+
]
55+
} as any;
56+
57+
const original = JSON.parse(JSON.stringify(payload));
58+
59+
service.calculateLoanSchedule(payload).subscribe(() => {});
60+
61+
const req = httpMock.expectOne('/loans?command=calculateLoanSchedule');
62+
expect(req.request.method).toBe('POST');
63+
64+
const body = req.request.body;
65+
66+
// All forbidden keys should be removed at every depth
67+
expect(body.linkAccountId).toBeUndefined();
68+
expect(body.linkAccountOwnerName).toBeUndefined();
69+
expect(body.linkedSavingsAccount).toBeUndefined();
70+
expect(body.nested.linkAccountOwnerId).toBeUndefined();
71+
expect(body.nested.linkAccountOwnerName).toBeUndefined();
72+
expect(body.members[0].linkAccountId).toBeUndefined();
73+
expect(body.members[1].linkedSavingsAccount).toBeUndefined();
74+
75+
// Allowed keys should be preserved
76+
expect(body.principal).toBe(1000);
77+
expect(body.nested.ok).toBe('keep');
78+
expect(body.members[1].name).toBe('foo');
79+
80+
// Original caller payload must remain unmutated
81+
expect(payload).toEqual(original);
82+
83+
req.flush({});
84+
});
85+
86+
it('should keep non-object primitives intact', () => {
87+
const payload = { value: 1, text: 'abc' };
88+
service.calculateLoanSchedule(payload).subscribe(() => {});
89+
const req = httpMock.expectOne('/loans?command=calculateLoanSchedule');
90+
expect(req.request.body.value).toBe(1);
91+
expect(req.request.body.text).toBe('abc');
92+
req.flush({});
93+
});
94+
});

src/app/loans/loans.service.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ export interface WorkingCapitalLoanDiscountUpdateRequest {
3030
providedIn: 'root'
3131
})
3232
export class LoansService {
33+
private static readonly FORBIDDEN_LOAN_PAYLOAD_KEYS: ReadonlySet<string> = new Set([
34+
'linkAccountId',
35+
'linkAccountOwnerId',
36+
'linkAccountOwnerName',
37+
'linkedSavingsAccount'
38+
]);
39+
3340
private http = inject(HttpClient);
3441
private settingsService = inject(SettingsService);
3542
private dateUtils = inject(Dates);
@@ -701,8 +708,29 @@ export class LoansService {
701708
return this.http.post('/batches?enclosingTransaction=true', payload);
702709
}
703710

711+
/**
712+
* Calculate repayment schedule preview.
713+
* Sanitize payload to avoid sending GSIM-linked fields that may crash backend.
714+
*/
704715
calculateLoanSchedule(payload: any): Observable<any> {
705-
return this.http.post('/loans?command=calculateLoanSchedule', payload);
716+
const sanitized = this.sanitizeLoanPayload(payload);
717+
return this.http.post('/loans?command=calculateLoanSchedule', sanitized);
718+
}
719+
720+
/**
721+
* Remove potentially unsafe GSIM linkage fields from payload recursively.
722+
* This is a defensive workaround until backend validation is fixed in Fineract.
723+
* Does not mutate the caller's payload; returns a new sanitized copy.
724+
*/
725+
private sanitizeLoanPayload(obj: any): any {
726+
if (obj === null || typeof obj !== 'object') return obj;
727+
if (Array.isArray(obj)) return obj.map((item) => this.sanitizeLoanPayload(item));
728+
const out: Record<string, any> = {};
729+
for (const key of Object.keys(obj)) {
730+
if (LoansService.FORBIDDEN_LOAN_PAYLOAD_KEYS.has(key)) continue;
731+
out[key] = this.sanitizeLoanPayload(obj[key]);
732+
}
733+
return out;
706734
}
707735

708736
attachLoanOriginator(loanId: string, originatorId: string): Observable<any> {

0 commit comments

Comments
 (0)