This is an automated email from the ASF dual-hosted git repository.
adamsaghy pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push:
new f8d9958ea7 FINERACT-2181: Fix audit filtering
f8d9958ea7 is described below
commit f8d9958ea7732f03e0e7ceb1a620c8dfefad4ede
Author: Adam Saghy <[email protected]>
AuthorDate: Mon May 19 00:52:16 2025 +0200
FINERACT-2181: Fix audit filtering
---
.../infrastructure/core/service/DateUtils.java | 29 +++++++++++++
.../infrastructure/security/utils/SQLBuilder.java | 49 +++++++++++++++++++---
.../fineract/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 06a5b05857..ac6320aff3 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<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);
+ }
+ }
}
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 283e53bef8..e8c2bbe912 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<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.
*
@@ -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 2e615f0428..a2240e6669 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 cbc5994dd9..0d5475017f 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 23044a0c91..92182c47c2 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