FINERACT-854 Removed string concatenated SQL in GroupReadPlatform

fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/SearchParameters.java

@@ -77,9 +77,9 @@ public final class SearchParameters {
                 maxLimitAllowed, orderBy, sortOrder, staffId, accountNo, loanId, savingsId, orphansOnly, isSelfUser);
     }
 
-    public static SearchParameters forGroups(final String sqlSearch, final Long officeId, final Long staffId, final String externalId,
+    public static SearchParameters forGroups(final Long officeId, final Long staffId, final String externalId, final String name,
-            final String name, final String hierarchy, final Integer offset, final Integer limit, final String orderBy,
+            final String hierarchy, final Integer offset, final Integer limit, final String orderBy, final String sortOrder,
-            final String sortOrder, final Boolean orphansOnly) {
+            final Boolean orphansOnly) {
 
         final Integer maxLimitAllowed = getCheckedLimit(limit);
         final String accountNo = null;
@@ -87,8 +87,8 @@ public final class SearchParameters {
         final Long savingsId = null;
         final boolean isSelfUser = false;
 
-        return new SearchParameters(sqlSearch, officeId, externalId, name, hierarchy, null, null, offset, maxLimitAllowed, orderBy,
+        return new SearchParameters(officeId, externalId, name, hierarchy, null, null, offset, maxLimitAllowed, orderBy, sortOrder, staffId,
-                sortOrder, staffId, accountNo, loanId, savingsId, orphansOnly, isSelfUser);
+                accountNo, loanId, savingsId, orphansOnly, isSelfUser);
     }
 
     public static SearchParameters forOffices(final String orderBy, final String sortOrder) {
@@ -301,6 +301,34 @@ public final class SearchParameters {
 
     }
 
+    private SearchParameters(final Long officeId, final String externalId, final String name, final String hierarchy,
+            final String firstname, final String lastname, final Integer offset, final Integer limit, final String orderBy,
+            final String sortOrder, final Long staffId, final String accountNo, final Long loanId, final Long savingsId,
+            final Boolean orphansOnly, boolean isSelfUser) {
+        this.sqlSearch = null;
+        this.officeId = officeId;
+        this.externalId = externalId;
+        this.name = name;
+        this.hierarchy = hierarchy;
+        this.firstname = firstname;
+        this.lastname = lastname;
+        this.offset = offset;
+        this.limit = limit;
+        this.orderBy = orderBy;
+        this.sortOrder = sortOrder;
+        this.staffId = staffId;
+        this.accountNo = accountNo;
+        this.loanId = loanId;
+        this.savingsId = savingsId;
+        this.orphansOnly = orphansOnly;
+        this.currencyCode = null;
+        this.provisioningEntryId = null;
+        this.productId = null;
+        this.categoryId = null;
+        this.isSelfUser = isSelfUser;
+        this.status = null;
+    }
+
     private SearchParameters(final Long provisioningEntryId, final Long officeId, final Long productId, final Long categoryId,
             final Integer offset, final Integer limit) {
         this.sqlSearch = null;

fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLBuilder.java

@@ -85,7 +85,7 @@ public class SQLBuilder {
                     "criteria cannot contain more than 1 space (between column name and operator): " + trimmedCriteria);
         }
         if (!operator.equals("=") && !operator.equals("<") && !operator.equals(">") && !operator.equals("<=") && !operator.equals(">=")
-                && !operator.equals("<>") && !operator.equals("LIKE") && !operator.equals("like")) {
+                && !operator.equals("<>") && !operator.equals("LIKE") && !operator.equals("like") && !operator.toLowerCase().equals("is")) {
             // add support for SQL's BETWEEN and IN, if/when ever needed.. (it's
             // a little more than just adding above, as it can have multiple
             // arguments)

fineract-provider/src/main/java/org/apache/fineract/portfolio/group/api/CentersApiResource.java

@@ -209,8 +209,8 @@ public class CentersApiResource {
         }
         final PaginationParameters parameters = PaginationParameters.instance(paged, offset, limit, orderBy, sortOrder);
         final Boolean isOrphansOnly = false;
-        final SearchParameters searchParameters = SearchParameters.forGroups(sqlSearch, officeId, staffId, externalId, name, hierarchy,
+        final SearchParameters searchParameters = SearchParameters.forGroups(officeId, staffId, externalId, name, hierarchy, offset, limit,
-                offset, limit, orderBy, sortOrder, isOrphansOnly);
+                orderBy, sortOrder, isOrphansOnly);
         if (parameters.isPaged()) {
             final Page<CenterData> centers = this.centerReadPlatformService.retrievePagedAll(searchParameters, parameters);
             return this.toApiJsonSerializer.serialize(settings, centers, GroupingTypesApiConstants.CENTER_RESPONSE_DATA_PARAMETERS);

fineract-provider/src/main/java/org/apache/fineract/portfolio/group/api/GroupsApiResource.java

@@ -229,7 +229,6 @@ public class GroupsApiResource {
     @ApiResponses({
             @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = GroupsApiResourceSwagger.GetGroupsResponse.class))) })
     public String retrieveAll(@Context final UriInfo uriInfo,
-            @QueryParam("sqlSearch") @Parameter(description = "sqlSearch") final String sqlSearch,
             @QueryParam("officeId") @Parameter(description = "officeId") final Long officeId,
             @QueryParam("staffId") @Parameter(description = "staffId") final Long staffId,
             @QueryParam("externalId") @Parameter(description = "externalId") final String externalId,
@@ -246,8 +245,8 @@ public class GroupsApiResource {
         final PaginationParameters parameters = PaginationParameters.instance(paged, offset, limit, orderBy, sortOrder);
         final ApiRequestJsonSerializationSettings settings = this.apiRequestParameterHelper.process(uriInfo.getQueryParameters());
 
-        final SearchParameters searchParameters = SearchParameters.forGroups(sqlSearch, officeId, staffId, externalId, name, hierarchy,
+        final SearchParameters searchParameters = SearchParameters.forGroups(officeId, staffId, externalId, name, hierarchy, offset, limit,
-                offset, limit, orderBy, sortOrder, orphansOnly);
+                orderBy, sortOrder, orphansOnly);
         if (parameters.isPaged()) {
             final Page<GroupGeneralData> groups = this.groupReadPlatformService.retrievePagedAll(searchParameters, parameters);
             return this.toApiJsonSerializer.serialize(settings, groups, GroupingTypesApiConstants.GROUP_RESPONSE_DATA_PARAMETERS);

fineract-provider/src/main/java/org/apache/fineract/portfolio/group/service/GroupReadPlatformServiceImpl.java

@@ -26,10 +26,8 @@ import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.fineract.infrastructure.codes.data.CodeValueData;
 import org.apache.fineract.infrastructure.codes.service.CodeValueReadPlatformService;
-import org.apache.fineract.infrastructure.core.api.ApiParameterHelper;
 import org.apache.fineract.infrastructure.core.data.PaginationParameters;
 import org.apache.fineract.infrastructure.core.data.PaginationParametersDataValidator;
 import org.apache.fineract.infrastructure.core.domain.JdbcSupport;
@@ -39,7 +37,7 @@ import org.apache.fineract.infrastructure.core.service.RoutingDataSource;
 import org.apache.fineract.infrastructure.core.service.SearchParameters;
 import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
 import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
-import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator;
+import org.apache.fineract.infrastructure.security.utils.SQLBuilder;
 import org.apache.fineract.organisation.office.data.OfficeData;
 import org.apache.fineract.organisation.office.service.OfficeReadPlatformService;
 import org.apache.fineract.organisation.staff.data.StaffData;
@@ -148,14 +146,10 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService {
         final StringBuilder sqlBuilder = new StringBuilder(200);
         sqlBuilder.append("select SQL_CALC_FOUND_ROWS ");
         sqlBuilder.append(this.allGroupTypesDataMapper.schema());
-        sqlBuilder.append(" where o.hierarchy like ?");
-        List<Object> paramList = new ArrayList<>(Arrays.asList(hierarchySearchString));
-        final String extraCriteria = getGroupExtraCriteria(this.allGroupTypesDataMapper.schema(), paramList, searchParameters);
-        this.columnValidator.validateSqlInjection(sqlBuilder.toString(), extraCriteria);
-        if (StringUtils.isNotBlank(extraCriteria)) {
-            sqlBuilder.append(" and (").append(extraCriteria).append(")");
-        }
 
+        final SQLBuilder extraCriteria = getGroupExtraCriteria(this.allGroupTypesDataMapper.schema(), searchParameters);
+        extraCriteria.addCriteria(" o.hierarchy like ", hierarchySearchString);
+        sqlBuilder.append(" ").append(extraCriteria.getSQLTemplate());
         if (parameters.isOrderByRequested()) {
             sqlBuilder.append(" order by ").append(searchParameters.getOrderBy()).append(' ').append(searchParameters.getSortOrder());
             this.columnValidator.validateSqlInjection(sqlBuilder.toString(), searchParameters.getOrderBy(),
@@ -170,7 +164,7 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService {
         }
 
         final String sqlCountRows = "SELECT FOUND_ROWS()";
-        return this.paginationHelper.fetchPage(this.jdbcTemplate, sqlCountRows, sqlBuilder.toString(), paramList.toArray(),
+        return this.paginationHelper.fetchPage(this.jdbcTemplate, sqlCountRows, sqlBuilder.toString(), extraCriteria.getArguments(),
                 this.allGroupTypesDataMapper);
     }
 
@@ -183,15 +177,11 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService {
         final StringBuilder sqlBuilder = new StringBuilder(200);
         sqlBuilder.append("select ");
         sqlBuilder.append(this.allGroupTypesDataMapper.schema());
-        sqlBuilder.append(" where o.hierarchy like ?");
+        final SQLBuilder extraCriteria = getGroupExtraCriteria(this.allGroupTypesDataMapper.schema(), searchParameters);
-        List<Object> paramList = new ArrayList<>(Arrays.asList(hierarchySearchString));
+        extraCriteria.addCriteria("o.hierarchy like ", hierarchySearchString);
-        if (searchParameters != null) {
+
-            final String extraCriteria = getGroupExtraCriteria(this.allGroupTypesDataMapper.schema(), paramList, searchParameters);
+        sqlBuilder.append(" ").append(extraCriteria.getSQLTemplate());
 
-            if (StringUtils.isNotBlank(extraCriteria)) {
-                sqlBuilder.append(" and (").append(extraCriteria).append(")");
-            }
-        }
         if (parameters != null) {
             if (parameters.isOrderByRequested()) {
                 sqlBuilder.append(parameters.orderBySql());
@@ -203,68 +193,39 @@ public class GroupReadPlatformServiceImpl implements GroupReadPlatformService {
                 this.columnValidator.validateSqlInjection(sqlBuilder.toString(), parameters.limitSql());
             }
         }
-        return this.jdbcTemplate.query(sqlBuilder.toString(), this.allGroupTypesDataMapper, paramList.toArray());
+        return this.jdbcTemplate.query(sqlBuilder.toString(), this.allGroupTypesDataMapper, extraCriteria.getArguments());
     }
 
     // 'g.' preffix because of ERROR 1052 (23000): Column 'column_name' in where
     // clause is ambiguous
     // caused by the same name of columns in m_office and m_group tables
-    private String getGroupExtraCriteria(String schemaSql, List<Object> paramList, final SearchParameters searchCriteria) {
+    private SQLBuilder getGroupExtraCriteria(String schemaSql, final SearchParameters searchCriteria) {
 
-        StringBuilder extraCriteria = new StringBuilder(200);
+        SQLBuilder extraCriteria = new SQLBuilder();
-        extraCriteria.append(" and g.level_Id = ").append(GroupTypes.GROUP.getId());
+        if (searchCriteria == null) {
-        String sqlSearch = searchCriteria.getSqlSearch();
+            return extraCriteria;
-        if (sqlSearch != null) {
-            SQLInjectionValidator.validateSQLInput(sqlSearch);
-            sqlSearch = sqlSearch.replace(" display_name ", " g.display_name ");
-            sqlSearch = sqlSearch.replace("display_name ", "g.display_name ");
-            extraCriteria.append(" and ( ").append(sqlSearch).append(") ");
-            this.columnValidator.validateSqlInjection(schemaSql, sqlSearch);
         }
+        extraCriteria.addCriteria("g.level_Id = ", GroupTypes.GROUP.getId());
 
-        final Long officeId = searchCriteria.getOfficeId();
+        extraCriteria.addNonNullCriteria(" g.office_id = ", searchCriteria.getOfficeId());
-        if (officeId != null) {
-            paramList.add(officeId);
-            extraCriteria.append(" and g.office_id = ? ");
-        }
 
-        final String externalId = searchCriteria.getExternalId();
+        extraCriteria.addNonNullCriteria(" g.external_id =", searchCriteria.getExternalId());
-        if (externalId != null) {
-            paramList.add(ApiParameterHelper.sqlEncodeString(externalId));
-            extraCriteria.append(" and g.external_id = ? ");
-        }
 
         final String name = searchCriteria.getName();
         if (name != null) {
-            paramList.add("%" + name + "%");
+            extraCriteria.addNonNullCriteria("g.display_name like", "%" + name + "%");
-            extraCriteria.append(" and g.display_name like ? ");
         }
 
         final String hierarchy = searchCriteria.getHierarchy();
         if (hierarchy != null) {
-            paramList.add(ApiParameterHelper.sqlEncodeString(hierarchy + "%"));
+            extraCriteria.addNonNullCriteria("o.hierarchy like ? ", hierarchy + "%");
-            extraCriteria.append(" and o.hierarchy like ? ");
-        }
-
-        if (searchCriteria.isStaffIdPassed()) {
-            paramList.add(searchCriteria.getStaffId());
-            extraCriteria.append(" and g.staff_id = ? ");
-        }
-
-        if (StringUtils.isNotBlank(extraCriteria.toString())) {
-            extraCriteria.delete(0, 4);
-        }
-
-        final Long staffId = searchCriteria.getStaffId();
-        if (staffId != null) {
-            paramList.add(staffId);
-            extraCriteria.append(" and g.staff_id = ? ");
         }
+        extraCriteria.addNonNullCriteria("g.staff_id =", searchCriteria.getStaffId());
 
         if (searchCriteria.isOrphansOnly()) {
-            extraCriteria.append(" and g.parent_id IS NULL");
+            extraCriteria.addNonNullCriteria("g.parent_id IS", "NULL");
         }
-        return extraCriteria.toString();
+        return extraCriteria;
     }
 
     @Override