From ca7060fd19a38ab60d790af897f8a577f0587bd7 Mon Sep 17 00:00:00 2001 From: deepaksoni47 Date: Sun, 10 May 2026 13:48:32 +0530 Subject: [PATCH] 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 --- src/app/loans/loans.service.spec.ts | 94 +++++++++++++++++++++++++++++ src/app/loans/loans.service.ts | 30 ++++++++- 2 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 src/app/loans/loans.service.spec.ts diff --git a/src/app/loans/loans.service.spec.ts b/src/app/loans/loans.service.spec.ts new file mode 100644 index 0000000000..01f6f81af8 --- /dev/null +++ b/src/app/loans/loans.service.spec.ts @@ -0,0 +1,94 @@ +import { TestBed } from '@angular/core/testing'; +import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; +import { provideHttpClient } from '@angular/common/http'; +import { LoansService } from './loans.service'; +import { SettingsService } from 'app/settings/settings.service'; +import { Dates } from 'app/core/utils/dates'; + +describe('LoansService', () => { + let service: LoansService; + let httpMock: HttpTestingController; + + beforeEach(() => { + const mockSettings: Partial = { + language: { name: 'English', code: 'en' }, + dateFormat: 'yyyy-MM-dd', + businessDate: new Date(), + maxFutureDate: new Date() + }; + const mockDates: Partial = { + formatDate: (d: any) => d, + formatDateAsString: (d: any) => String(d) + } as any; + + TestBed.configureTestingModule({ + providers: [ + LoansService, + provideHttpClient(), + provideHttpClientTesting(), + { provide: SettingsService, useValue: mockSettings }, + { provide: Dates, useValue: mockDates }] + }); + service = TestBed.inject(LoansService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpMock.verify(); + }); + + it('should sanitize all GSIM fields before posting calculateLoanSchedule', () => { + const payload = { + principal: 1000, + linkAccountId: 42, + linkAccountOwnerName: 'Alice', + linkedSavingsAccount: { id: 5 }, + nested: { + linkAccountOwnerId: 7, + linkAccountOwnerName: 'Bob', + ok: 'keep' + }, + members: [ + { id: 1, linkAccountId: 99 }, + { id: 2, name: 'foo', linkedSavingsAccount: { id: 9 } } + ] + } as any; + + const original = JSON.parse(JSON.stringify(payload)); + + service.calculateLoanSchedule(payload).subscribe(() => {}); + + const req = httpMock.expectOne('/loans?command=calculateLoanSchedule'); + expect(req.request.method).toBe('POST'); + + const body = req.request.body; + + // All forbidden keys should be removed at every depth + expect(body.linkAccountId).toBeUndefined(); + expect(body.linkAccountOwnerName).toBeUndefined(); + expect(body.linkedSavingsAccount).toBeUndefined(); + expect(body.nested.linkAccountOwnerId).toBeUndefined(); + expect(body.nested.linkAccountOwnerName).toBeUndefined(); + expect(body.members[0].linkAccountId).toBeUndefined(); + expect(body.members[1].linkedSavingsAccount).toBeUndefined(); + + // Allowed keys should be preserved + expect(body.principal).toBe(1000); + expect(body.nested.ok).toBe('keep'); + expect(body.members[1].name).toBe('foo'); + + // Original caller payload must remain unmutated + expect(payload).toEqual(original); + + req.flush({}); + }); + + it('should keep non-object primitives intact', () => { + const payload = { value: 1, text: 'abc' }; + service.calculateLoanSchedule(payload).subscribe(() => {}); + const req = httpMock.expectOne('/loans?command=calculateLoanSchedule'); + expect(req.request.body.value).toBe(1); + expect(req.request.body.text).toBe('abc'); + req.flush({}); + }); +}); diff --git a/src/app/loans/loans.service.ts b/src/app/loans/loans.service.ts index f2ba2e35a2..f9a6bf9948 100644 --- a/src/app/loans/loans.service.ts +++ b/src/app/loans/loans.service.ts @@ -30,6 +30,13 @@ export interface WorkingCapitalLoanDiscountUpdateRequest { providedIn: 'root' }) export class LoansService { + private static readonly FORBIDDEN_LOAN_PAYLOAD_KEYS: ReadonlySet = new Set([ + 'linkAccountId', + 'linkAccountOwnerId', + 'linkAccountOwnerName', + 'linkedSavingsAccount' + ]); + private http = inject(HttpClient); private settingsService = inject(SettingsService); private dateUtils = inject(Dates); @@ -701,8 +708,29 @@ export class LoansService { return this.http.post('/batches?enclosingTransaction=true', payload); } + /** + * Calculate repayment schedule preview. + * Sanitize payload to avoid sending GSIM-linked fields that may crash backend. + */ calculateLoanSchedule(payload: any): Observable { - return this.http.post('/loans?command=calculateLoanSchedule', payload); + const sanitized = this.sanitizeLoanPayload(payload); + return this.http.post('/loans?command=calculateLoanSchedule', sanitized); + } + + /** + * Remove potentially unsafe GSIM linkage fields from payload recursively. + * This is a defensive workaround until backend validation is fixed in Fineract. + * Does not mutate the caller's payload; returns a new sanitized copy. + */ + private sanitizeLoanPayload(obj: any): any { + if (obj === null || typeof obj !== 'object') return obj; + if (Array.isArray(obj)) return obj.map((item) => this.sanitizeLoanPayload(item)); + const out: Record = {}; + for (const key of Object.keys(obj)) { + if (LoansService.FORBIDDEN_LOAN_PAYLOAD_KEYS.has(key)) continue; + out[key] = this.sanitizeLoanPayload(obj[key]); + } + return out; } attachLoanOriginator(loanId: string, originatorId: string): Observable {