From f8d9958ea7732f03e0e7ceb1a620c8dfefad4ede Mon Sep 17 00:00:00 2001 From: Adam Saghy Date: Mon, 19 May 2025 00:52:16 +0200 Subject: [PATCH] FINERACT-2181: Fix audit filtering --- .../core/service/DateUtils.java | 29 +++++++++++ .../security/utils/SQLBuilder.java | 49 ++++++++++++++++--- .../commands/api/AuditsApiResource.java | 36 ++++++++++++-- .../commands/data/request/AuditRequest.java | 31 ++++++++++-- .../infrastructure.sqlbuilder.feature | 8 +-- 5 files changed, 134 insertions(+), 19 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java index 06a5b0585..ac6320aff 100644 --- a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java +++ b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/DateUtils.java @@ -23,18 +23,22 @@ import static java.time.temporal.ChronoUnit.DAYS; import jakarta.validation.constraints.NotNull; import java.time.LocalDate; import java.time.LocalDateTime; +import java.time.LocalTime; import java.time.OffsetDateTime; import java.time.ZoneId; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; import java.time.temporal.ChronoUnit; +import java.time.temporal.TemporalAccessor; import java.util.List; import java.util.Locale; import java.util.Optional; import org.apache.fineract.infrastructure.core.data.ApiParameterError; import org.apache.fineract.infrastructure.core.domain.FineractPlatformTenant; import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; +import org.apache.fineract.infrastructure.core.serialization.JsonParserHelper; public final class DateUtils { @@ -466,4 +470,29 @@ public final class DateUtils { } 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 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); + } + } } diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLBuilder.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLBuilder.java index 283e53bef..e8c2bbe91 100644 --- a/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLBuilder.java +++ b/fineract-core/src/main/java/org/apache/fineract/infrastructure/security/utils/SQLBuilder.java @@ -22,7 +22,9 @@ import java.sql.PreparedStatement; import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.function.Consumer; import java.util.regex.Pattern; +import lombok.Getter; /** * Utility to assemble the WHERE clause of an SQL query without the risk of SQL injection. @@ -57,8 +59,10 @@ public class SQLBuilder { * placeholder) * @param argument * 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()) { 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); } + // TODO: Would be better to use SqlOperator functionality to handle if (sb.length() > 0) { - sb.append(" AND "); + sb.append(whereLogicalOperator.getSqlStr()); } sb.append(trimmedCriteria); sb.append(" ?"); @@ -101,15 +106,32 @@ public class SQLBuilder { 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. */ - public void addNonNullCriteria(String criteria, Object argument) { + public void addNonNullCriteria(String criteria, Object argument, WhereLogicalOperator whereLogicalOperator) { 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 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. * @@ -141,9 +163,9 @@ public class SQLBuilder { StringBuilder whereClause = new StringBuilder("SQLBuilder{"); for (int i = 0; i < args.size(); i++) { if (i != 0) { - whereClause.append(" AND "); + whereClause.append(WhereLogicalOperator.AND.getSqlStr()); } else { - whereClause.append("WHERE "); + whereClause.append("WHERE "); } Object currentArg = args.get(i); whereClause.append(crts.get(i)); @@ -163,4 +185,19 @@ public class SQLBuilder { whereClause.append("}"); return whereClause.toString(); } + + @Getter + public enum WhereLogicalOperator { + + NONE(""), // + AND(" AND "), // + OR(" OR "); // + + private final String sqlStr; + + WhereLogicalOperator(String sqlStr) { + this.sqlStr = sqlStr; + } + + } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/commands/api/AuditsApiResource.java b/fineract-provider/src/main/java/org/apache/fineract/commands/api/AuditsApiResource.java index 2e615f042..a2240e666 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/commands/api/AuditsApiResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/commands/api/AuditsApiResource.java @@ -119,10 +119,38 @@ public class AuditsApiResource { extraCriteria.addNonNullCriteria("aud.resource_id = ", auditRequest.getResourceId()); extraCriteria.addNonNullCriteria("aud.maker_id = ", auditRequest.getMakerId()); extraCriteria.addNonNullCriteria("aud.checker_id = ", auditRequest.getCheckerId()); - extraCriteria.addNonNullCriteria("aud.made_on_date >= ", auditRequest.getMakerDateTimeFrom()); - extraCriteria.addNonNullCriteria("aud.made_on_date <= ", auditRequest.getMakerDateTimeTo()); - extraCriteria.addNonNullCriteria("aud.checked_on_date >= ", auditRequest.getCheckerDateTimeFrom()); - extraCriteria.addNonNullCriteria("aud.checked_on_date <= ", auditRequest.getCheckerDateTimeTo()); + if (auditRequest.getMakerDateTimeFrom() != null) { + extraCriteria.addSubOperation((SQLBuilder criteria) -> { + criteria.addNonNullCriteria("aud.made_on_date >= ", auditRequest.getMakerDateTimeFrom(), + 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.office_id = ", auditRequest.getOfficeId()); extraCriteria.addNonNullCriteria("aud.group_id = ", auditRequest.getGroupId()); diff --git a/fineract-provider/src/main/java/org/apache/fineract/commands/data/request/AuditRequest.java b/fineract-provider/src/main/java/org/apache/fineract/commands/data/request/AuditRequest.java index cbc5994dd..0d5475017 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/commands/data/request/AuditRequest.java +++ b/fineract-provider/src/main/java/org/apache/fineract/commands/data/request/AuditRequest.java @@ -21,10 +21,12 @@ package org.apache.fineract.commands.data.request; import jakarta.ws.rs.QueryParam; import java.io.Serial; import java.io.Serializable; -import java.time.ZonedDateTime; +import java.time.LocalDateTime; +import java.time.LocalTime; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; +import org.apache.fineract.infrastructure.core.service.DateUtils; @Setter @Getter @@ -43,15 +45,15 @@ public class AuditRequest implements Serializable { @QueryParam("makerId") private Long makerId; @QueryParam("makerDateTimeFrom") - private ZonedDateTime makerDateTimeFrom; + private String makerDateTimeFrom; @QueryParam("makerDateTimeTo") - private ZonedDateTime makerDateTimeTo; + private String makerDateTimeTo; @QueryParam("checkerId") private Long checkerId; @QueryParam("checkerDateTimeFrom") - private ZonedDateTime checkerDateTimeFrom; + private String checkerDateTimeFrom; @QueryParam("checkerDateTimeTo") - private ZonedDateTime checkerDateTimeTo; + private String checkerDateTimeTo; @QueryParam("status") private String status; @QueryParam("clientId") @@ -66,5 +68,24 @@ public class AuditRequest implements Serializable { private Long savingsAccountId; @QueryParam("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); + } } diff --git a/fineract-provider/src/test/resources/features/infrastructure/infrastructure.sqlbuilder.feature b/fineract-provider/src/test/resources/features/infrastructure/infrastructure.sqlbuilder.feature index 23044a0c9..92182c47c 100644 --- a/fineract-provider/src/test/resources/features/infrastructure/infrastructure.sqlbuilder.feature +++ b/fineract-provider/src/test/resources/features/infrastructure/infrastructure.sqlbuilder.feature @@ -28,10 +28,10 @@ Feature: SQL Builder Examples: | criteria1 | argument1 | criteria2 | argument2 | criteria3 | argument3 | criteria4 | argument4 | template | expected | - | | | | | | | | | | 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]} | - | ref = | NULL | | | | | | | WHERE ref = ? | SQLBuilder{WHERE ref = [null]} | - | hobby LIKE | Mifos/Apache Fineract | hobby like | Mifos/Apache Fineract | | | | | WHERE hobby LIKE ? AND hobby like ? | | + | | | | | | | | | | 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]} | + | ref = | NULL | | | | | | | WHERE ref = ? | SQLBuilder{WHERE ref = [null]} | + | hobby LIKE | Mifos/Apache Fineract | hobby like | Mifos/Apache Fineract | | | | | WHERE hobby LIKE ? AND hobby like ? | | @sqlbuilder Scenario Outline: Verify that SQL builder detects illegal criteria