FINERACT-2181: Fix audit filtering

This commit is contained in:
Adam Saghy
2025-05-21 21:12:02 +02:00
parent 15c968bc1b
commit f8d9958ea7
5 changed files with 134 additions and 19 deletions
@@ -23,18 +23,22 @@ import static java.time.temporal.ChronoUnit.DAYS;
import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.NotNull;
import java.time.LocalDate; import java.time.LocalDate;
import java.time.LocalDateTime; import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.OffsetDateTime; import java.time.OffsetDateTime;
import java.time.ZoneId; import java.time.ZoneId;
import java.time.ZoneOffset; import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException; import java.time.format.DateTimeParseException;
import java.time.temporal.ChronoField;
import java.time.temporal.ChronoUnit; import java.time.temporal.ChronoUnit;
import java.time.temporal.TemporalAccessor;
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Optional; import java.util.Optional;
import org.apache.fineract.infrastructure.core.data.ApiParameterError; import org.apache.fineract.infrastructure.core.data.ApiParameterError;
import org.apache.fineract.infrastructure.core.domain.FineractPlatformTenant; import org.apache.fineract.infrastructure.core.domain.FineractPlatformTenant;
import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
import org.apache.fineract.infrastructure.core.serialization.JsonParserHelper;
public final class DateUtils { public final class DateUtils {
@@ -466,4 +470,29 @@ public final class DateUtils {
} }
return formatter; return formatter;
} }
public static LocalDateTime convertDateTimeStringToLocalDateTime(String dateTimeStr, String dateFormat, String localeStr,
LocalTime fallbackTime) {
if (dateTimeStr == null || dateTimeStr.isBlank()) {
return null;
}
final Locale locale = localeStr == null ? null : JsonParserHelper.localeFromString(localeStr);
DateTimeFormatter formatter = getDateFormatter(dateFormat, locale);
TemporalAccessor parsed = formatter.parse(dateTimeStr);
boolean hasTime = parsed.isSupported(ChronoField.HOUR_OF_DAY) && parsed.isSupported(ChronoField.MINUTE_OF_HOUR);
try {
if (hasTime) {
return LocalDateTime.from(parsed);
} else {
LocalDate date = LocalDate.from(parsed);
return LocalDateTime.of(date, fallbackTime);
}
} catch (final DateTimeParseException e) {
final List<ApiParameterError> errors = List.of(ApiParameterError.parameterError("validation.msg.invalid.date.pattern",
"The parameter date (" + dateTimeStr + ") format is invalid", "date", dateTimeStr));
throw new PlatformApiDataValidationException("validation.msg.validation.errors.exist", "Validation errors exist.", errors, e);
}
}
} }
@@ -22,7 +22,9 @@ import java.sql.PreparedStatement;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.function.Consumer;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import lombok.Getter;
/** /**
* Utility to assemble the WHERE clause of an SQL query without the risk of SQL injection. * Utility to assemble the WHERE clause of an SQL query without the risk of SQL injection.
@@ -57,8 +59,10 @@ public class SQLBuilder {
* placeholder) * placeholder)
* @param argument * @param argument
* The argument to be filtered on (e.g. "Michael" or 123). The null value is explicitly permitted. * The argument to be filtered on (e.g. "Michael" or 123). The null value is explicitly permitted.
* @param whereLogicalOperator
* operator between the criteria
*/ */
public void addCriteria(String criteria, Object argument) { public void addCriteria(String criteria, Object argument, WhereLogicalOperator whereLogicalOperator) {
if (criteria == null || criteria.trim().isEmpty()) { if (criteria == null || criteria.trim().isEmpty()) {
throw new IllegalArgumentException("criteria cannot be null"); throw new IllegalArgumentException("criteria cannot be null");
} }
@@ -92,8 +96,9 @@ public class SQLBuilder {
throw new IllegalArgumentException("criteria must end with valid SQL operator for WHERE: " + trimmedCriteria); throw new IllegalArgumentException("criteria must end with valid SQL operator for WHERE: " + trimmedCriteria);
} }
// TODO: Would be better to use SqlOperator functionality to handle
if (sb.length() > 0) { if (sb.length() > 0) {
sb.append(" AND "); sb.append(whereLogicalOperator.getSqlStr());
} }
sb.append(trimmedCriteria); sb.append(trimmedCriteria);
sb.append(" ?"); sb.append(" ?");
@@ -101,15 +106,32 @@ public class SQLBuilder {
args.add(argument); args.add(argument);
} }
public void addCriteria(String criteria, Object argument) {
addCriteria(criteria, argument, WhereLogicalOperator.AND);
}
/** /**
* Delegates to {@link #addCriteria(String, Object)} if argument is not null, otherwise does nothing. * Delegates to {@link #addCriteria(String, Object)} if argument is not null, otherwise does nothing.
*/ */
public void addNonNullCriteria(String criteria, Object argument) { public void addNonNullCriteria(String criteria, Object argument, WhereLogicalOperator whereLogicalOperator) {
if (argument != null) { if (argument != null) {
addCriteria(criteria, argument); addCriteria(criteria, argument, whereLogicalOperator);
} }
} }
public void addNonNullCriteria(String criteria, Object argument) {
addNonNullCriteria(criteria, argument, WhereLogicalOperator.AND);
}
public void addSubOperation(Consumer<SQLBuilder> subOperation) {
if (sb.length() > 0) {
sb.append(WhereLogicalOperator.AND.getSqlStr());
}
sb.append(" ( ");
subOperation.accept(this);
sb.append(" ) ");
}
/** /**
* Returns a SQL WHERE clause, created from the {@link #addCriteria(String, Object)}, with '?' placeholders. * Returns a SQL WHERE clause, created from the {@link #addCriteria(String, Object)}, with '?' placeholders.
* *
@@ -141,9 +163,9 @@ public class SQLBuilder {
StringBuilder whereClause = new StringBuilder("SQLBuilder{"); StringBuilder whereClause = new StringBuilder("SQLBuilder{");
for (int i = 0; i < args.size(); i++) { for (int i = 0; i < args.size(); i++) {
if (i != 0) { if (i != 0) {
whereClause.append(" AND "); whereClause.append(WhereLogicalOperator.AND.getSqlStr());
} else { } else {
whereClause.append("WHERE "); whereClause.append("WHERE ");
} }
Object currentArg = args.get(i); Object currentArg = args.get(i);
whereClause.append(crts.get(i)); whereClause.append(crts.get(i));
@@ -163,4 +185,19 @@ public class SQLBuilder {
whereClause.append("}"); whereClause.append("}");
return whereClause.toString(); return whereClause.toString();
} }
@Getter
public enum WhereLogicalOperator {
NONE(""), //
AND(" AND "), //
OR(" OR "); //
private final String sqlStr;
WhereLogicalOperator(String sqlStr) {
this.sqlStr = sqlStr;
}
}
} }
@@ -119,10 +119,38 @@ public class AuditsApiResource {
extraCriteria.addNonNullCriteria("aud.resource_id = ", auditRequest.getResourceId()); extraCriteria.addNonNullCriteria("aud.resource_id = ", auditRequest.getResourceId());
extraCriteria.addNonNullCriteria("aud.maker_id = ", auditRequest.getMakerId()); extraCriteria.addNonNullCriteria("aud.maker_id = ", auditRequest.getMakerId());
extraCriteria.addNonNullCriteria("aud.checker_id = ", auditRequest.getCheckerId()); extraCriteria.addNonNullCriteria("aud.checker_id = ", auditRequest.getCheckerId());
extraCriteria.addNonNullCriteria("aud.made_on_date >= ", auditRequest.getMakerDateTimeFrom()); if (auditRequest.getMakerDateTimeFrom() != null) {
extraCriteria.addNonNullCriteria("aud.made_on_date <= ", auditRequest.getMakerDateTimeTo()); extraCriteria.addSubOperation((SQLBuilder criteria) -> {
extraCriteria.addNonNullCriteria("aud.checked_on_date >= ", auditRequest.getCheckerDateTimeFrom()); criteria.addNonNullCriteria("aud.made_on_date >= ", auditRequest.getMakerDateTimeFrom(),
extraCriteria.addNonNullCriteria("aud.checked_on_date <= ", auditRequest.getCheckerDateTimeTo()); SQLBuilder.WhereLogicalOperator.NONE);
criteria.addNonNullCriteria("aud.made_on_date_utc >= ", auditRequest.getMakerDateTimeFrom(),
SQLBuilder.WhereLogicalOperator.OR);
});
}
if (auditRequest.getMakerDateTimeTo() != null) {
extraCriteria.addSubOperation((SQLBuilder criteria) -> {
criteria.addNonNullCriteria("aud.made_on_date <= ", auditRequest.getMakerDateTimeTo(),
SQLBuilder.WhereLogicalOperator.NONE);
criteria.addNonNullCriteria("aud.made_on_date_utc <= ", auditRequest.getMakerDateTimeTo(),
SQLBuilder.WhereLogicalOperator.OR);
});
}
if (auditRequest.getCheckerDateTimeFrom() != null) {
extraCriteria.addSubOperation((SQLBuilder criteria) -> {
criteria.addNonNullCriteria("aud.checked_on_date >= ", auditRequest.getCheckerDateTimeFrom(),
SQLBuilder.WhereLogicalOperator.NONE);
criteria.addNonNullCriteria("aud.checked_on_date_utc >= ", auditRequest.getCheckerDateTimeFrom(),
SQLBuilder.WhereLogicalOperator.OR);
});
}
if (auditRequest.getCheckerDateTimeTo() != null) {
extraCriteria.addSubOperation((SQLBuilder criteria) -> {
criteria.addNonNullCriteria("aud.checked_on_date <= ", auditRequest.getCheckerDateTimeTo(),
SQLBuilder.WhereLogicalOperator.NONE);
criteria.addNonNullCriteria("aud.checked_on_date_utc <= ", auditRequest.getCheckerDateTimeTo(),
SQLBuilder.WhereLogicalOperator.OR);
});
}
extraCriteria.addNonNullCriteria("aud.status = ", auditRequest.getStatus()); extraCriteria.addNonNullCriteria("aud.status = ", auditRequest.getStatus());
extraCriteria.addNonNullCriteria("aud.office_id = ", auditRequest.getOfficeId()); extraCriteria.addNonNullCriteria("aud.office_id = ", auditRequest.getOfficeId());
extraCriteria.addNonNullCriteria("aud.group_id = ", auditRequest.getGroupId()); extraCriteria.addNonNullCriteria("aud.group_id = ", auditRequest.getGroupId());
@@ -21,10 +21,12 @@ package org.apache.fineract.commands.data.request;
import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.QueryParam;
import java.io.Serial; import java.io.Serial;
import java.io.Serializable; import java.io.Serializable;
import java.time.ZonedDateTime; import java.time.LocalDateTime;
import java.time.LocalTime;
import lombok.Getter; import lombok.Getter;
import lombok.NoArgsConstructor; import lombok.NoArgsConstructor;
import lombok.Setter; import lombok.Setter;
import org.apache.fineract.infrastructure.core.service.DateUtils;
@Setter @Setter
@Getter @Getter
@@ -43,15 +45,15 @@ public class AuditRequest implements Serializable {
@QueryParam("makerId") @QueryParam("makerId")
private Long makerId; private Long makerId;
@QueryParam("makerDateTimeFrom") @QueryParam("makerDateTimeFrom")
private ZonedDateTime makerDateTimeFrom; private String makerDateTimeFrom;
@QueryParam("makerDateTimeTo") @QueryParam("makerDateTimeTo")
private ZonedDateTime makerDateTimeTo; private String makerDateTimeTo;
@QueryParam("checkerId") @QueryParam("checkerId")
private Long checkerId; private Long checkerId;
@QueryParam("checkerDateTimeFrom") @QueryParam("checkerDateTimeFrom")
private ZonedDateTime checkerDateTimeFrom; private String checkerDateTimeFrom;
@QueryParam("checkerDateTimeTo") @QueryParam("checkerDateTimeTo")
private ZonedDateTime checkerDateTimeTo; private String checkerDateTimeTo;
@QueryParam("status") @QueryParam("status")
private String status; private String status;
@QueryParam("clientId") @QueryParam("clientId")
@@ -66,5 +68,24 @@ public class AuditRequest implements Serializable {
private Long savingsAccountId; private Long savingsAccountId;
@QueryParam("processingResult") @QueryParam("processingResult")
private String processingResult; private String processingResult;
@QueryParam("dateFormat")
private String dateFormat;
@QueryParam("locale")
private String locale;
public LocalDateTime getMakerDateTimeFrom() {
return DateUtils.convertDateTimeStringToLocalDateTime(makerDateTimeFrom, dateFormat, locale, LocalTime.MIN);
}
public LocalDateTime getMakerDateTimeTo() {
return DateUtils.convertDateTimeStringToLocalDateTime(makerDateTimeTo, dateFormat, locale, LocalTime.MAX);
}
public LocalDateTime getCheckerDateTimeFrom() {
return DateUtils.convertDateTimeStringToLocalDateTime(checkerDateTimeFrom, dateFormat, locale, LocalTime.MIN);
}
public LocalDateTime getCheckerDateTimeTo() {
return DateUtils.convertDateTimeStringToLocalDateTime(checkerDateTimeTo, dateFormat, locale, LocalTime.MAX);
}
} }
@@ -28,10 +28,10 @@ Feature: SQL Builder
Examples: Examples:
| criteria1 | argument1 | criteria2 | argument2 | criteria3 | argument3 | criteria4 | argument4 | template | expected | | criteria1 | argument1 | criteria2 | argument2 | criteria3 | argument3 | criteria4 | argument4 | template | expected |
| | | | | | | | | | SQLBuilder{} | | | | | | | | | | | SQLBuilder{} |
| name = | Michael | hobby LIKE | Mifos/Apache Fineract | age < | 123 | | | WHERE name = ? AND hobby LIKE ? AND age < ? | SQLBuilder{WHERE name = ['Michael'] AND hobby LIKE ['Mifos/Apache Fineract'] AND age < [123]} | | name = | Michael | hobby LIKE | Mifos/Apache Fineract | age < | 123 | | | WHERE name = ? AND hobby LIKE ? AND age < ? | SQLBuilder{WHERE name = ['Michael'] AND hobby LIKE ['Mifos/Apache Fineract'] AND age < [123]} |
| ref = | NULL | | | | | | | WHERE ref = ? | SQLBuilder{WHERE ref = [null]} | | ref = | NULL | | | | | | | WHERE ref = ? | SQLBuilder{WHERE ref = [null]} |
| hobby LIKE | Mifos/Apache Fineract | hobby like | Mifos/Apache Fineract | | | | | WHERE hobby LIKE ? AND hobby like ? | | | hobby LIKE | Mifos/Apache Fineract | hobby like | Mifos/Apache Fineract | | | | | WHERE hobby LIKE ? AND hobby like ? | |
@sqlbuilder @sqlbuilder
Scenario Outline: Verify that SQL builder detects illegal criteria Scenario Outline: Verify that SQL builder detects illegal criteria