FINERACT-2684: Enhance Search API

fineract-provider/src/main/java/org/apache/fineract/portfolio/search/service/SearchReadServiceImpl.java

@@ -27,8 +27,8 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.fineract.infrastructure.core.data.EnumOptionData;
 import org.apache.fineract.infrastructure.core.domain.JdbcSupport;
+import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
 import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
-import org.apache.fineract.infrastructure.security.service.SqlValidator;
 import org.apache.fineract.organisation.monetary.domain.MoneyHelper;
 import org.apache.fineract.organisation.office.data.OfficeData;
 import org.apache.fineract.organisation.office.service.OfficeReadPlatformService;
@@ -55,11 +55,16 @@
 @RequiredArgsConstructor
 public class SearchReadServiceImpl implements SearchReadService {
 
+    private static final String CONDITION_BETWEEN = "between";
+    private static final String PARAM_LOAN_DATE_OPTION = "loanDateOption";
+    private static final String PARAM_OUTSTANDING_AMOUNT_PERCENTAGE_CONDITION = "outStandingAmountPercentageCondition";
+    private static final String PARAM_OUTSTANDING_AMOUNT_CONDITION = "outstandingAmountCondition";
+    private static final String VALID_SEARCH_CONDITIONS = "=, >, >=, <, <=, between";
+
     private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
     private final LoanProductReadPlatformService loanProductReadPlatformService;
     private final OfficeReadPlatformService officeReadPlatformService;
     private final DatabaseSpecificSQLGenerator sqlGenerator;
-    private final SqlValidator sqlValidator;
 
     @Override
     public List<SearchData> retriveMatchingData(final SearchConditions searchConditions) {
@@ -250,24 +255,52 @@ public List<AdHocSearchQueryData> retrieveAdHocQueryMatchingData(final AdHocQuer
     }
 
     private AdHocQuerySearchConditions convertToSearchConditions(final AdHocQuerySearchRequest request) {
-        if (request.getLoanDateOption() != null) {
-            sqlValidator.validate(request.getLoanDateOption());
+        validateLoanDateOption(request.getLoanDateOption());
+        final boolean includeOutStandingAmountPercentage = Boolean.TRUE.equals(request.getIncludeOutStandingAmountPercentage());
+        final boolean includeOutstandingAmount = Boolean.TRUE.equals(request.getIncludeOutstandingAmount());
+        final String outStandingAmountPercentageCondition = validateSearchCondition(request.getOutStandingAmountPercentageCondition(),
+                PARAM_OUTSTANDING_AMOUNT_PERCENTAGE_CONDITION, includeOutStandingAmountPercentage);
+        final String outstandingAmountCondition = validateSearchCondition(request.getOutstandingAmountCondition(),
+                PARAM_OUTSTANDING_AMOUNT_CONDITION, includeOutstandingAmount);
+
+        return AdHocQuerySearchConditions.instance(request.getLoanStatus(), request.getLoanProducts(), request.getOffices(),
+                request.getLoanDateOption(), request.getLoanFromDate(), request.getLoanToDate(), includeOutStandingAmountPercentage,
+                outStandingAmountPercentageCondition, request.getMinOutStandingAmountPercentage(),
+                request.getMaxOutStandingAmountPercentage(), request.getOutStandingAmountPercentage(), includeOutstandingAmount,
+                outstandingAmountCondition, request.getMinOutstandingAmount(), request.getMaxOutstandingAmount(),
+                request.getOutstandingAmount());
+    }
+
+    private static void validateLoanDateOption(final String loanDateOption) {
+        if (StringUtils.isBlank(loanDateOption) || SearchConstants.SearchLoanDate.getAllValues().contains(loanDateOption)) {
+            return;
         }
-        if (request.getOutStandingAmountPercentageCondition() != null) {
-            sqlValidator.validate(request.getOutStandingAmountPercentageCondition());
+        throw invalidSearchParameter(PARAM_LOAN_DATE_OPTION, loanDateOption,
+                String.join(", ", SearchConstants.SearchLoanDate.getAllValues()));
+    }
+
+    private static String validateSearchCondition(final String condition, final String parameterName, final boolean required) {
+        if (StringUtils.isBlank(condition)) {
+            if (required) {
+                throw invalidSearchParameter(parameterName, condition, VALID_SEARCH_CONDITIONS);
+            }
+            return condition;
         }
-        if (request.getOutstandingAmountCondition() != null) {
-            sqlValidator.validate(request.getOutstandingAmountCondition());
+
+        final String trimmedCondition = condition.trim();
+        if (CONDITION_BETWEEN.equalsIgnoreCase(trimmedCondition)) {
+            return CONDITION_BETWEEN;
         }
+        return switch (trimmedCondition) {
+            case "=", ">", ">=", "<", "<=" -> trimmedCondition;
+            default -> throw invalidSearchParameter(parameterName, condition, VALID_SEARCH_CONDITIONS);
+        };
+    }
 
-        return AdHocQuerySearchConditions.instance(request.getLoanStatus(), request.getLoanProducts(), request.getOffices(),
-                request.getLoanDateOption(), request.getLoanFromDate(), request.getLoanToDate(),
-                request.getIncludeOutStandingAmountPercentage() != null ? request.getIncludeOutStandingAmountPercentage() : false,
-                request.getOutStandingAmountPercentageCondition(), request.getMinOutStandingAmountPercentage(),
-                request.getMaxOutStandingAmountPercentage(), request.getOutStandingAmountPercentage(),
-                request.getIncludeOutstandingAmount() != null ? request.getIncludeOutstandingAmount() : false,
-                request.getOutstandingAmountCondition(), request.getMinOutstandingAmount(), request.getMaxOutstandingAmount(),
-                request.getOutstandingAmount());
+    private static PlatformApiDataValidationException invalidSearchParameter(final String parameterName, final String value,
+            final String expectedValues) {
+        return new PlatformApiDataValidationException("validation.msg.search.parameter.is.not.one.of.expected.enumerations",
+                "The parameter `" + parameterName + "` must be one of [ " + expectedValues + " ].", parameterName, value);
     }
 
     private static final class AdHocQuerySearchMapper implements RowMapper<AdHocSearchQueryData> {

fineract-provider/src/main/java/org/apache/fineract/portfolio/search/starter/SearchConfiguration.java

@@ -19,7 +19,6 @@
 package org.apache.fineract.portfolio.search.starter;
 
 import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
-import org.apache.fineract.infrastructure.security.service.SqlValidator;
 import org.apache.fineract.organisation.office.service.OfficeReadPlatformService;
 import org.apache.fineract.portfolio.loanproduct.service.LoanProductReadPlatformService;
 import org.apache.fineract.portfolio.search.service.SearchReadService;
@@ -36,8 +35,8 @@ public class SearchConfiguration {
     @ConditionalOnMissingBean(SearchReadService.class)
     public SearchReadService searchReadService(NamedParameterJdbcTemplate namedParameterJdbcTemplate,
             LoanProductReadPlatformService loanProductReadPlatformService, OfficeReadPlatformService officeReadPlatformService,
-            DatabaseSpecificSQLGenerator sqlGenerator, SqlValidator sqlValidator) {
+            DatabaseSpecificSQLGenerator sqlGenerator) {
         return new SearchReadServiceImpl(namedParameterJdbcTemplate, loanProductReadPlatformService, officeReadPlatformService,
-                sqlGenerator, sqlValidator);
+                sqlGenerator);
     }
 }

fineract-provider/src/test/java/org/apache/fineract/portfolio/search/service/SearchReadServiceImplTest.java

@@ -0,0 +1,98 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.search.service;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import java.math.BigDecimal;
+import java.util.List;
+import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
+import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
+import org.apache.fineract.organisation.office.service.OfficeReadPlatformService;
+import org.apache.fineract.portfolio.loanproduct.service.LoanProductReadPlatformService;
+import org.apache.fineract.portfolio.search.data.AdHocQuerySearchRequest;
+import org.apache.fineract.portfolio.search.data.AdHocSearchQueryData;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.jdbc.core.RowMapper;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
+
+@ExtendWith(MockitoExtension.class)
+class SearchReadServiceImplTest {
+
+    @Mock
+    private NamedParameterJdbcTemplate namedParameterJdbcTemplate;
+    @Mock
+    private LoanProductReadPlatformService loanProductReadPlatformService;
+    @Mock
+    private OfficeReadPlatformService officeReadPlatformService;
+    @Mock
+    private DatabaseSpecificSQLGenerator sqlGenerator;
+
+    private SearchReadServiceImpl service;
+
+    @BeforeEach
+    void setUp() {
+        service = new SearchReadServiceImpl(namedParameterJdbcTemplate, loanProductReadPlatformService, officeReadPlatformService,
+                sqlGenerator);
+    }
+
+    @Test
+    void retrieveAdHocQueryMatchingDataUsesWhitelistedOutStandingAmountPercentageCondition() {
+        final AdHocQuerySearchRequest request = AdHocQuerySearchRequest.builder().includeOutStandingAmountPercentage(true)
+                .outStandingAmountPercentageCondition("<=").outStandingAmountPercentage(BigDecimal.valueOf(80)).build();
+        doReturn(List.<AdHocSearchQueryData>of()).when(namedParameterJdbcTemplate).query(anyString(), any(MapSqlParameterSource.class),
+                any(RowMapper.class));
+
+        service.retrieveAdHocQueryMatchingData(request);
+
+        final ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
+        verify(namedParameterJdbcTemplate).query(sqlCaptor.capture(), any(MapSqlParameterSource.class), any(RowMapper.class));
+        assertTrue(sqlCaptor.getValue().contains(" a.percentOut <= :outStandingAmountPercentage "));
+    }
+
+    @Test
+    void retrieveAdHocQueryMatchingDataRejectsInjectedOutStandingAmountPercentageCondition() {
+        final AdHocQuerySearchRequest request = AdHocQuerySearchRequest.builder().includeOutStandingAmountPercentage(true)
+                .outStandingAmountPercentageCondition("<= 0 union select").outStandingAmountPercentage(BigDecimal.valueOf(80)).build();
+
+        assertThrows(PlatformApiDataValidationException.class, () -> service.retrieveAdHocQueryMatchingData(request));
+        verify(namedParameterJdbcTemplate, never()).query(anyString(), any(MapSqlParameterSource.class), any(RowMapper.class));
+    }
+
+    @Test
+    void retrieveAdHocQueryMatchingDataRejectsInjectedOutstandingAmountCondition() {
+        final AdHocQuerySearchRequest request = AdHocQuerySearchRequest.builder().includeOutstandingAmount(true)
+                .outstandingAmountCondition(">= 0 or 1 = 1").outstandingAmount(BigDecimal.valueOf(100)).build();
+
+        assertThrows(PlatformApiDataValidationException.class, () -> service.retrieveAdHocQueryMatchingData(request));
+        verify(namedParameterJdbcTemplate, never()).query(anyString(), any(MapSqlParameterSource.class), any(RowMapper.class));
+    }
+}