From 31f41558fc9a80b7d8ada7f3140598da126ce3c2 Mon Sep 17 00:00:00 2001 From: "abraham.menyhart" Date: Tue, 30 May 2023 16:43:39 +0200 Subject: [PATCH] FINERACT-1724: Datatables query API fix - invalid query filter --- .../ReadWriteNonCoreDataServiceImpl.java | 132 ++++-------------- .../ReadWriteNonCoreDataServiceImplTest.java | 65 +++++---- 2 files changed, 66 insertions(+), 131 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java index fac9c6e39..fc981e2b8 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java @@ -19,6 +19,7 @@ package org.apache.fineract.infrastructure.dataqueries.service; import static java.util.Arrays.asList; +import static java.util.Locale.ENGLISH; import static org.apache.fineract.infrastructure.core.data.ApiParameterError.parameterErrorWithValue; import com.google.common.base.Splitter; @@ -33,18 +34,14 @@ import java.sql.Date; import java.sql.SQLException; import java.sql.Statement; import java.sql.Timestamp; -import java.sql.Types; import java.time.LocalDate; import java.time.LocalDateTime; import java.time.format.DateTimeFormatter; -import java.time.format.DateTimeParseException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.stream.Stream; import javax.persistence.PersistenceException; @@ -84,7 +81,6 @@ import org.apache.fineract.infrastructure.security.service.SqlInjectionPreventer import org.apache.fineract.infrastructure.security.utils.ColumnValidator; import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator; import org.apache.fineract.useradministration.domain.AppUser; -import org.jetbrains.annotations.NotNull; import org.springframework.dao.DataAccessException; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.EmptyResultDataAccessException; @@ -113,21 +109,15 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ .put("number", "INT").put("boolean", "boolean").put("decimal", "DECIMAL").put("date", "DATE").put("datetime", "TIMESTAMP") .put("text", "TEXT").put("dropdown", "INT").build(); - private static final List stringDataTypes = Arrays.asList("char", "varchar", "blob", "text", "tinyblob", "tinytext", - "mediumblob", "mediumtext", "longblob", "longtext"); - private static final DateTimeFormatter DATA_TABLE_DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - private static final DateTimeFormatter DATA_TABLE_DATETIME_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); - public static final String PG_BOOLEAN_TYPE = "boolean"; - public static final String PG_BOOL_TYPE = "bool"; - public static final String PG_INT_TYPE = "integer"; - public static final String PG_BIGINT_TYPE = "bigint"; - public static final String PG_DATE_TYPE = "date"; - public static final String PG_NUMERIC_TYPE = "numeric"; - public static final String PG_TEXT_TYPE = "text"; - public static final String PG_VARCHAR_TYPE = "character varying"; - public static final String BIT_TYPE = "bit"; + private static final List stringDataTypes = asList("char", "varchar", "blob", "text", "tinyblob", "tinytext", "mediumblob", + "mediumtext", "longblob", "longtext"); + private static final String DATE_FORMAT = "yyyy-MM-dd"; + public static final String DATE_TIME_FORMAT = "yyyy-MM-dd HH:mm:ss"; + private static final DateTimeFormatter DATA_TABLE_DATE_FORMAT = DateTimeFormatter.ofPattern(DATE_FORMAT); + private static final DateTimeFormatter DATA_TABLE_DATETIME_FORMAT = DateTimeFormatter.ofPattern(DATE_TIME_FORMAT); + public static final String MYSQL_DATETIME_TYPE = "datetime"; - public static final String MYSQL_DATE_TYPE = "date"; + public static final String POSTGRES_TIMESTAMP_TYPE = "timestamp without time zone"; private final JdbcTemplate jdbcTemplate; private final DatabaseTypeResolver databaseTypeResolver; @@ -205,24 +195,15 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ @Override public List queryDataTable(String datatable, final String columnFilter, String valueFilter, String resultColumns) { - Arrays.asList(datatable, columnFilter, valueFilter, resultColumns).forEach(SQLInjectionValidator::validateDynamicQuery); + asList(datatable, columnFilter, valueFilter, resultColumns).forEach(SQLInjectionValidator::validateDynamicQuery); List resultsetColumnHeaderData = genericDataService.fillResultsetColumnHeaders(datatable); validateRequestParams(columnFilter, valueFilter, resultColumns, resultsetColumnHeaderData); - String filterColumnType = resultsetColumnHeaderData.stream().filter(column -> Objects.equals(columnFilter, column.getColumnName())) - .findFirst().map(ResultsetColumnHeaderData::getColumnType).orElse(columnFilter + " does not exist in datatable"); - String sql = "select " + resultColumns + " from " + datatable + " where " + columnFilter + " = ?"; - SqlRowSet rowSet = null; - if (databaseTypeResolver.isPostgreSQL()) { - sql = "select " + escapeFieldNames(resultColumns) + " from " + sqlGenerator.escape(datatable) + " where " - + escapeFieldNames(columnFilter) + " = ?"; - rowSet = callFilteredPgSql(sql, valueFilter, filterColumnType); - } else if (databaseTypeResolver.isMySQL()) { - rowSet = callFilteredMysql(sql, valueFilter, filterColumnType); - } else { - throw new IllegalStateException("Database type is not supported"); - } + Object validatedValueFilter = getValidatedValueFilter(columnFilter, valueFilter, resultsetColumnHeaderData); + String sql = "select " + escapeFieldNames(resultColumns) + " from " + sqlGenerator.escape(datatable) + " where " + + escapeFieldNames(columnFilter) + " = ?"; + SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql, validatedValueFilter); String[] resultColumnNames = resultColumns.split(","); List results = new ArrayList<>(); @@ -233,6 +214,18 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ return results; } + private Object getValidatedValueFilter(String columnFilter, String valueFilter, + List resultsetColumnHeaderData) { + ResultsetColumnHeaderData columnFilterHeader = resultsetColumnHeaderData.stream() + .filter(headerData -> headerData.getColumnName().equals(columnFilter)).findAny().orElseThrow(); + if (MYSQL_DATETIME_TYPE.equals(columnFilterHeader.getColumnType()) + || POSTGRES_TIMESTAMP_TYPE.equals(columnFilterHeader.getColumnType())) { + return validateColumn(columnFilterHeader, valueFilter, DATE_TIME_FORMAT, ENGLISH); + } else { + return validateColumn(columnFilterHeader, valueFilter, DATE_FORMAT, ENGLISH); + } + } + private void extractResults(SqlRowSet rowSet, String[] resultColumnNames, List results) { JsonObject json = new JsonObject(); for (String rcn : resultColumnNames) { @@ -264,77 +257,6 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ } } - @NotNull - private SqlRowSet callFilteredMysql(String sql, String valueFilter, String filterColumnType) { - Object finalValueFilter = valueFilter; - SqlRowSet rowSet; - if (BIT_TYPE.equalsIgnoreCase(filterColumnType)) { - int[] argType = new int[1]; - argType[0] = Types.BIT; - finalValueFilter = BooleanUtils.toString(BooleanUtils.toBooleanObject(valueFilter), "1", "0", "null"); - rowSet = jdbcTemplate.queryForRowSet(sql, new Object[] { finalValueFilter }, argType); - } else if (MYSQL_DATE_TYPE.equalsIgnoreCase(filterColumnType)) { - int[] argType = new int[1]; - argType[0] = Types.DATE; - try { - rowSet = jdbcTemplate.queryForRowSet(sql, new Object[] { LocalDate.parse(valueFilter, DATA_TABLE_DATE_FORMAT) }, argType); - } catch (DateTimeParseException e) { - List paramErrors = new ArrayList<>(); - paramErrors.add(parameterErrorWithValue("400", - "Unsupported input type for datatable query! Use format: 'yyyy-MM-dd'. Column filter: " + filterColumnType, - "valueFilter", valueFilter)); - throw new PlatformApiDataValidationException(paramErrors, e); - } - } else if (MYSQL_DATETIME_TYPE.equals(filterColumnType)) { - int[] argType = new int[1]; - argType[0] = Types.TIMESTAMP; - try { - rowSet = jdbcTemplate.queryForRowSet(sql, new Object[] { LocalDateTime.parse(valueFilter, DATA_TABLE_DATETIME_FORMAT) }, - argType); - } catch (DateTimeParseException e) { - List paramErrors = new ArrayList<>(); - paramErrors.add(parameterErrorWithValue("400", - "Unsupported input type for datatable query! Use format: 'yyyy-MM-dd HH:mm:ss'. Column filter: " + filterColumnType, - "valueFilter", valueFilter)); - throw new PlatformApiDataValidationException(paramErrors, e); - } - } else { - rowSet = jdbcTemplate.queryForRowSet(sql, finalValueFilter); - } - return rowSet; - } - - @NotNull - private SqlRowSet callFilteredPgSql(String sql, String valueFilter, String filterColumnType) { - Object finalValueFilter = valueFilter; - int[] argType = new int[1]; - if (BIT_TYPE.equalsIgnoreCase(filterColumnType)) { - finalValueFilter = BooleanUtils.toString(BooleanUtils.toBooleanObject(valueFilter), "1", "0", "null"); - argType[0] = Types.BIT; - } else if (PG_BOOLEAN_TYPE.equalsIgnoreCase(filterColumnType) || PG_BOOL_TYPE.equalsIgnoreCase(filterColumnType)) { - finalValueFilter = BooleanUtils.toString(BooleanUtils.toBooleanObject(valueFilter), "true", "false", "null"); - argType[0] = Types.BOOLEAN; - } else if (PG_INT_TYPE.equalsIgnoreCase(filterColumnType)) { - argType[0] = Types.INTEGER; - } else if (PG_BIGINT_TYPE.equalsIgnoreCase(filterColumnType)) { - argType[0] = Types.BIGINT; - } else if (PG_DATE_TYPE.equalsIgnoreCase(filterColumnType)) { - argType[0] = Types.DATE; - } else if (filterColumnType.toLowerCase().contains("timestamp")) { - argType[0] = Types.TIMESTAMP; - } else if (PG_NUMERIC_TYPE.equalsIgnoreCase(filterColumnType)) { - argType[0] = Types.DECIMAL; - } else if (PG_TEXT_TYPE.equalsIgnoreCase(filterColumnType) || PG_VARCHAR_TYPE.equalsIgnoreCase(filterColumnType)) { - argType[0] = Types.VARCHAR; - } else { - List paramErrors = new ArrayList<>(); - paramErrors.add(parameterErrorWithValue("400", "Unsupported input type for datatable query! Column filter: " + filterColumnType, - "valueFilter", valueFilter)); - throw new PlatformApiDataValidationException(paramErrors); - } - return jdbcTemplate.queryForRowSet(sql, new Object[] { finalValueFilter }, argType); - } - private String escapeFieldNames(final String inputFields) { final String delimiter = ","; if (StringUtils.isBlank(inputFields)) { @@ -348,7 +270,7 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ return String.join(",", outputFields); } - private static void validateRequestParams(String columnFilter, String valueFilter, String resultColumns, + private void validateRequestParams(String columnFilter, String valueFilter, String resultColumns, List resultsetColumnHeaderData) { List paramErrors = new ArrayList<>(); List dataTableColumnNames = resultsetColumnHeaderData.stream().map(ResultsetColumnHeaderData::getColumnName).toList(); diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImplTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImplTest.java index b3d6b96a8..fb1bc4b25 100644 --- a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImplTest.java +++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImplTest.java @@ -18,23 +18,27 @@ */ package org.apache.fineract.infrastructure.dataqueries.service; +import static java.util.Collections.emptyList; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.when; import com.google.gson.JsonObject; -import java.util.Collections; import java.util.List; +import java.util.stream.Stream; import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator; import org.apache.fineract.infrastructure.core.service.database.DatabaseTypeResolver; import org.apache.fineract.infrastructure.dataqueries.data.ResultsetColumnHeaderData; import org.apache.fineract.infrastructure.security.utils.SQLInjectionException; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentMatchers; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; @@ -81,44 +85,53 @@ public class ReadWriteNonCoreDataServiceImplTest { @Test public void testQueryDataTableSuccess() { SqlRowSet sqlRS = Mockito.mock(SqlRowSet.class); - when(jdbcTemplate.queryForRowSet(eq("select rc1,rc2 from table where cf1 = ?"), any(Object[].class), any(int[].class))) - .thenReturn(sqlRS); + when(jdbcTemplate.queryForRowSet(eq("select rc1,rc2 from table where cf1 = ?"), any(Object.class))).thenReturn(sqlRS); when(sqlRS.next()).thenReturn(true).thenReturn(false); - when(sqlRS.getObject(ArgumentMatchers.anyString())).thenReturn("value1").thenReturn("value2"); - when(sqlGenerator.escape(ArgumentMatchers.anyString())).thenReturn("rc1").thenReturn("rc2").thenReturn("table").thenReturn("cf1"); + when(sqlRS.getObject(anyString())).thenReturn("value1").thenReturn("value2"); + when(sqlGenerator.escape(anyString())).thenReturn("rc1").thenReturn("rc2").thenReturn("table").thenReturn("cf1"); when(databaseTypeResolver.isPostgreSQL()).thenReturn(true); - ResultsetColumnHeaderData cf1 = ResultsetColumnHeaderData.detailed("cf1", "text", 10L, false, false, null, null, false, false); - ResultsetColumnHeaderData rc1 = ResultsetColumnHeaderData.detailed("rc1", "text", 10L, false, false, null, null, false, false); - ResultsetColumnHeaderData rc2 = ResultsetColumnHeaderData.detailed("rc2", "text", 10L, false, false, null, null, false, false); + ResultsetColumnHeaderData cf1 = ResultsetColumnHeaderData.detailed("cf1", "text", 10L, false, false, emptyList(), null, false, + false); + ResultsetColumnHeaderData rc1 = ResultsetColumnHeaderData.detailed("rc1", "text", 10L, false, false, emptyList(), null, false, + false); + ResultsetColumnHeaderData rc2 = ResultsetColumnHeaderData.detailed("rc2", "text", 10L, false, false, emptyList(), null, false, + false); when(genericDataService.fillResultsetColumnHeaders("table")).thenReturn(List.of(cf1, rc1, rc2)); List results = underTest.queryDataTable("table", "cf1", "vf1", "rc1,rc2"); - Assertions.assertEquals("value1", results.get(0).get("rc1").getAsString()); - Assertions.assertEquals("value2", results.get(0).get("rc2").getAsString()); + assertEquals("value1", results.get(0).get("rc1").getAsString()); + assertEquals("value2", results.get(0).get("rc2").getAsString()); } @Test public void testQueryDataTableValidationError() { - when(genericDataService.fillResultsetColumnHeaders("table")).thenReturn(Collections.emptyList()); + when(genericDataService.fillResultsetColumnHeaders("table")).thenReturn(emptyList()); assertThrows(PlatformApiDataValidationException.class, () -> underTest.queryDataTable("table", "cf1", "vf1", "rc1,rc2")); } - @Test - public void testInvalidDatabase() { - SqlRowSet sqlRS = Mockito.mock(SqlRowSet.class); - when(jdbcTemplate.queryForRowSet(eq("select rc1,rc2 from table where cf1 = ?"), any(Object[].class), any(int[].class))) - .thenReturn(sqlRS); - when(sqlRS.next()).thenReturn(true).thenReturn(false); - when(sqlRS.getObject(ArgumentMatchers.anyString())).thenReturn("value1").thenReturn("value2"); - when(databaseTypeResolver.isPostgreSQL()).thenReturn(false); - when(databaseTypeResolver.isMySQL()).thenReturn(false); - ResultsetColumnHeaderData cf1 = ResultsetColumnHeaderData.detailed("cf1", "text", 10L, false, false, null, null, false, false); - ResultsetColumnHeaderData rc1 = ResultsetColumnHeaderData.detailed("rc1", "text", 10L, false, false, null, null, false, false); - ResultsetColumnHeaderData rc2 = ResultsetColumnHeaderData.detailed("rc2", "text", 10L, false, false, null, null, false, false); + @ParameterizedTest + @MethodSource("provideParameters") + public void testQueryDataTableInvalidParameterError(String columnType, String errorMessage) { + ResultsetColumnHeaderData cf1 = ResultsetColumnHeaderData.detailed("cf1", columnType, 10L, false, false, emptyList(), null, false, + false); + ResultsetColumnHeaderData rc1 = ResultsetColumnHeaderData.detailed("rc1", "text", 10L, false, false, emptyList(), null, false, + false); + ResultsetColumnHeaderData rc2 = ResultsetColumnHeaderData.detailed("rc2", "text", 10L, false, false, emptyList(), null, false, + false); when(genericDataService.fillResultsetColumnHeaders("table")).thenReturn(List.of(cf1, rc1, rc2)); - assertThrows(IllegalStateException.class, () -> underTest.queryDataTable("table", "cf1", "vf1", "rc1,rc2")); + PlatformApiDataValidationException thrown = assertThrows(PlatformApiDataValidationException.class, + () -> underTest.queryDataTable("table", "cf1", "vf1", "rc1,rc2")); + + assertEquals(1, thrown.getErrors().size()); + assertEquals(errorMessage, thrown.getErrors().get(0).getUserMessageGlobalisationCode()); + } + + private static Stream provideParameters() { + return Stream.of(Arguments.of("timestamp without time zone", "validation.msg.invalid.dateFormat.format"), + Arguments.of("INTEGER", "validation.msg.invalid.integer.format"), + Arguments.of("BOOLEAN", "validation.msg.invalid.boolean.format")); } }