You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by ar...@apache.org on 2022/05/09 12:42:28 UTC
[fineract] branch develop updated: FINERACT-1483: fix Sonar vulnerabilities with `Minor` severity
This is an automated email from the ASF dual-hosted git repository.
arnold 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 665bd0f6f FINERACT-1483: fix Sonar vulnerabilities with `Minor` severity
665bd0f6f is described below
commit 665bd0f6fbdbbb772087f11c5b599e51df5c2b64
Author: taskain7 <ta...@gmail.com>
AuthorDate: Fri Apr 29 14:34:59 2022 +0200
FINERACT-1483: fix Sonar vulnerabilities with `Minor` severity
---
.../service/ReadReportingServiceImpl.java | 43 +++++++++---------
.../ContentRepositoryFactory.java | 10 +----
.../contentrepository/S3ContentRepository.java | 21 ++++-----
.../security/utils/LogParameterEscapeUtil.java | 28 ++++++++++++
.../config/MessagingConfiguration.java | 5 +--
.../useradministration/domain/AppUser.java | 15 +++----
.../security/utils/LogParameterEscapeUtilTest.java | 51 ++++++++++++++++++++++
.../infrastructure.security.utils.feature | 13 ++++++
8 files changed, 131 insertions(+), 55 deletions(-)
diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java
index bb6e8c398..4b518014e 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadReportingServiceImpl.java
@@ -37,6 +37,8 @@ import java.util.Locale;
import java.util.Map;
import java.util.Set;
import javax.ws.rs.core.StreamingOutput;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.infrastructure.core.domain.JdbcSupport;
import org.apache.fineract.infrastructure.core.exception.PlatformDataIntegrityException;
import org.apache.fineract.infrastructure.dataqueries.data.GenericResultsetData;
@@ -49,36 +51,25 @@ import org.apache.fineract.infrastructure.dataqueries.exception.ReportNotFoundEx
import org.apache.fineract.infrastructure.documentmanagement.contentrepository.FileSystemContentRepository;
import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
import org.apache.fineract.infrastructure.security.service.SqlInjectionPreventerService;
+import org.apache.fineract.infrastructure.security.utils.LogParameterEscapeUtil;
import org.apache.fineract.useradministration.domain.AppUser;
import org.owasp.esapi.ESAPI;
import org.owasp.esapi.codecs.UnixCodec;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.support.rowset.SqlRowSet;
import org.springframework.stereotype.Service;
@Service
+@Slf4j
+@RequiredArgsConstructor
public class ReadReportingServiceImpl implements ReadReportingService {
- private static final Logger LOG = LoggerFactory.getLogger(ReadReportingServiceImpl.class);
-
private final JdbcTemplate jdbcTemplate;
private final PlatformSecurityContext context;
private final GenericDataService genericDataService;
private final SqlInjectionPreventerService sqlInjectionPreventerService;
- @Autowired
- public ReadReportingServiceImpl(final PlatformSecurityContext context, final JdbcTemplate jdbcTemplate,
- final GenericDataService genericDataService, SqlInjectionPreventerService sqlInjectionPreventerService) {
- this.context = context;
- this.jdbcTemplate = jdbcTemplate;
- this.genericDataService = genericDataService;
- this.sqlInjectionPreventerService = sqlInjectionPreventerService;
- }
-
@Override
public StreamingOutput retrieveReportCSV(final String name, final String type, final Map<String, String> queryParams,
final boolean isSelfServiceUserReport) {
@@ -110,7 +101,7 @@ public class ReadReportingServiceImpl implements ReadReportingService {
final StringBuilder writer = new StringBuilder();
final List<ResultsetColumnHeaderData> columnHeaders = result.getColumnHeaders();
- LOG.info("NO. of Columns: {}", columnHeaders.size());
+ log.info("NO. of Columns: {}", columnHeaders.size());
final Integer chSize = columnHeaders.size();
for (int i = 0; i < chSize; i++) {
writer.append('"' + columnHeaders.get(i).getColumnName() + '"');
@@ -128,7 +119,7 @@ public class ReadReportingServiceImpl implements ReadReportingService {
String currVal;
final String doubleQuote = "\"";
final String twoDoubleQuotes = doubleQuote + doubleQuote;
- LOG.info("NO. of Rows: {}", data.size());
+ log.info("NO. of Rows: {}", data.size());
for (ResultsetRowData element : data) {
row = element.getRow();
rSize = row.size();
@@ -160,14 +151,20 @@ public class ReadReportingServiceImpl implements ReadReportingService {
final boolean isSelfServiceUserReport) {
final long startTime = System.currentTimeMillis();
- LOG.info("STARTING REPORT: {} Type: {}", name, type);
+ if (log.isDebugEnabled()) {
+ log.debug("STARTING REPORT: {} Type: {}", LogParameterEscapeUtil.escapeLogParameter(name),
+ LogParameterEscapeUtil.escapeLogParameter(type));
+ }
final String sql = getSQLtoRun(name, type, queryParams, isSelfServiceUserReport);
final GenericResultsetData result = this.genericDataService.fillGenericResultSet(sql);
final long elapsed = System.currentTimeMillis() - startTime;
- LOG.info("FINISHING Report/Request Name: {} - {} Elapsed Time: {}", name, type, elapsed);
+ if (log.isDebugEnabled()) {
+ log.debug("FINISHING Report/Request Name: {} - {} Elapsed Time: {}", LogParameterEscapeUtil.escapeLogParameter(name),
+ type.replaceAll("[\n\r\t]", "_"), elapsed);
+ }
return result;
}
@@ -252,7 +249,7 @@ public class ReadReportingServiceImpl implements ReadReportingService {
final List<ResultsetRowData> data = result.getData();
List<String> row;
- LOG.info("NO. of Columns: {}", columnHeaders.size());
+ log.info("NO. of Columns: {}", columnHeaders.size());
final Integer chSize = columnHeaders.size();
final Document document = new Document(PageSize.B0.rotate());
@@ -274,7 +271,7 @@ public class ReadReportingServiceImpl implements ReadReportingService {
Integer rSize;
String currColType;
String currVal;
- LOG.info("NO. of Rows: {}", data.size());
+ log.info("NO. of Rows: {}", data.size());
for (ResultsetRowData element : data) {
row = element.getRow();
rSize = row.size();
@@ -297,7 +294,7 @@ public class ReadReportingServiceImpl implements ReadReportingService {
document.close();
return genaratePdf;
} catch (final Exception e) {
- LOG.error("error.msg.reporting.error:", e);
+ log.error("error.msg.reporting.error:", e);
throw new PlatformDataIntegrityException("error.msg.exception.error", e.getMessage(), e);
}
}
@@ -482,14 +479,14 @@ public class ReadReportingServiceImpl implements ReadReportingService {
@Override
public GenericResultsetData retrieveGenericResultSetForSmsEmailCampaign(String name, String type, Map<String, String> queryParams) {
final long startTime = System.currentTimeMillis();
- LOG.info("STARTING REPORT: {} Type: {}", name, type);
+ log.info("STARTING REPORT: {} Type: {}", name, type);
final String sql = sqlToRunForSmsEmailCampaign(name, type, queryParams);
final GenericResultsetData result = this.genericDataService.fillGenericResultSet(sql);
final long elapsed = System.currentTimeMillis() - startTime;
- LOG.info("FINISHING Report/Request Name: {} - {} Elapsed Time: {}", name, type, elapsed);
+ log.info("FINISHING Report/Request Name: {} - {} Elapsed Time: {}", name, type, elapsed);
return result;
}
diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/contentrepository/ContentRepositoryFactory.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/contentrepository/ContentRepositoryFactory.java
index 4158fe015..bdfcb5c69 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/contentrepository/ContentRepositoryFactory.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/contentrepository/ContentRepositoryFactory.java
@@ -18,27 +18,21 @@
*/
package org.apache.fineract.infrastructure.documentmanagement.contentrepository;
+import lombok.RequiredArgsConstructor;
import org.apache.fineract.infrastructure.configuration.data.S3CredentialsData;
import org.apache.fineract.infrastructure.configuration.domain.ConfigurationDomainService;
import org.apache.fineract.infrastructure.configuration.service.ExternalServicesPropertiesReadPlatformService;
import org.apache.fineract.infrastructure.documentmanagement.domain.StorageType;
-import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
import org.springframework.stereotype.Component;
@Component
+@RequiredArgsConstructor
public class ContentRepositoryFactory {
private final ApplicationContext applicationContext;
private final ExternalServicesPropertiesReadPlatformService externalServicesReadPlatformService;
- @Autowired
- public ContentRepositoryFactory(final ApplicationContext applicationContext,
- final ExternalServicesPropertiesReadPlatformService externalServicesReadPlatformService) {
- this.applicationContext = applicationContext;
- this.externalServicesReadPlatformService = externalServicesReadPlatformService;
- }
-
public ContentRepository getRepository() {
final ConfigurationDomainService configurationDomainServiceJpa = this.applicationContext.getBean("configurationDomainServiceJpa",
ConfigurationDomainService.class);
diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/contentrepository/S3ContentRepository.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/contentrepository/S3ContentRepository.java
index da999a7ba..37fbf4ff4 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/contentrepository/S3ContentRepository.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/documentmanagement/contentrepository/S3ContentRepository.java
@@ -35,6 +35,7 @@ import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.util.Base64;
+import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.infrastructure.core.domain.Base64EncodedImage;
import org.apache.fineract.infrastructure.documentmanagement.command.DocumentCommand;
import org.apache.fineract.infrastructure.documentmanagement.data.DocumentData;
@@ -43,13 +44,11 @@ import org.apache.fineract.infrastructure.documentmanagement.data.ImageData;
import org.apache.fineract.infrastructure.documentmanagement.domain.StorageType;
import org.apache.fineract.infrastructure.documentmanagement.exception.ContentManagementException;
import org.apache.fineract.infrastructure.documentmanagement.exception.DocumentNotFoundException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.apache.fineract.infrastructure.security.utils.LogParameterEscapeUtil;
+@Slf4j
public class S3ContentRepository implements ContentRepository {
- private static final Logger LOG = LoggerFactory.getLogger(S3ContentRepository.class);
-
private final String s3BucketName;
private final AmazonS3 s3Client;
@@ -154,23 +153,21 @@ public class S3ContentRepository implements ContentRepository {
private void putObject(final String filename, final InputStream inputStream, final String s3UploadLocation)
throws ContentManagementException {
try {
- LOG.info("Uploading a new object to S3 {}", s3UploadLocation);
+ if (log.isDebugEnabled()) {
+ log.debug("Uploading a new object to S3 {}", LogParameterEscapeUtil.escapeLogParameter(s3UploadLocation));
+ }
this.s3Client.putObject(new PutObjectRequest(this.s3BucketName, s3UploadLocation, inputStream, new ObjectMetadata()));
- } catch (AmazonServiceException ase) {
+ } catch (AmazonClientException ase) {
throw new ContentManagementException(filename, ase.getMessage(), ase);
- } catch (final AmazonClientException ace) {
- throw new ContentManagementException(filename, ace.getMessage(), ace);
}
}
private S3Object getObject(String key) {
try {
- LOG.info("Downloading an object from Amazon S3 Bucket: {}, location: {}", this.s3BucketName, key);
+ log.info("Downloading an object from Amazon S3 Bucket: {}, location: {}", this.s3BucketName, key);
return this.s3Client.getObject(new GetObjectRequest(this.s3BucketName, key));
- } catch (AmazonServiceException ase) {
+ } catch (AmazonClientException ase) {
throw new ContentManagementException(key, ase.getMessage(), ase);
- } catch (final AmazonClientException ace) {
- throw new ContentManagementException(key, ace.getMessage(), ace);
}
}
}
diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/LogParameterEscapeUtil.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/LogParameterEscapeUtil.java
new file mode 100644
index 000000000..1e249f4f8
--- /dev/null
+++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/utils/LogParameterEscapeUtil.java
@@ -0,0 +1,28 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.utils;
+
+public final class LogParameterEscapeUtil {
+
+ private LogParameterEscapeUtil() {}
+
+ public static String escapeLogParameter(String logParameter) {
+ return logParameter.replaceAll("[\n\r\t]", "_");
+ }
+}
diff --git a/fineract-provider/src/main/java/org/apache/fineract/notification/config/MessagingConfiguration.java b/fineract-provider/src/main/java/org/apache/fineract/notification/config/MessagingConfiguration.java
index a8d2b4633..bf26aea5b 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/notification/config/MessagingConfiguration.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/notification/config/MessagingConfiguration.java
@@ -52,7 +52,7 @@ public class MessagingConfiguration {
@Bean
public ActiveMQConnectionFactory amqConnectionFactory() {
- ActiveMQConnectionFactory amqConnectionFactory = new ActiveMQConnectionFactory();
+ ActiveMQConnectionFactory amqConnectionFactory = new ActiveMQConnectionFactory(); // NOSONAR
try {
amqConnectionFactory.setBrokerURL(DEFAULT_BROKER_URL);
} catch (Exception e) {
@@ -63,8 +63,7 @@ public class MessagingConfiguration {
@Bean
public CachingConnectionFactory connectionFactory() {
- CachingConnectionFactory connectionFactory = new CachingConnectionFactory(amqConnectionFactory());
- return connectionFactory;
+ return new CachingConnectionFactory(amqConnectionFactory());
}
@Bean
diff --git a/fineract-provider/src/main/java/org/apache/fineract/useradministration/domain/AppUser.java b/fineract-provider/src/main/java/org/apache/fineract/useradministration/domain/AppUser.java
index 623c11c22..9f076e2bd 100644
--- a/fineract-provider/src/main/java/org/apache/fineract/useradministration/domain/AppUser.java
+++ b/fineract-provider/src/main/java/org/apache/fineract/useradministration/domain/AppUser.java
@@ -39,6 +39,7 @@ import javax.persistence.Table;
import javax.persistence.Temporal;
import javax.persistence.TemporalType;
import javax.persistence.UniqueConstraint;
+import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.fineract.infrastructure.core.api.JsonCommand;
import org.apache.fineract.infrastructure.core.data.EnumOptionData;
@@ -48,22 +49,20 @@ import org.apache.fineract.infrastructure.security.domain.PlatformUser;
import org.apache.fineract.infrastructure.security.exception.NoAuthorizationException;
import org.apache.fineract.infrastructure.security.service.PlatformPasswordEncoder;
import org.apache.fineract.infrastructure.security.service.RandomPasswordGenerator;
+import org.apache.fineract.infrastructure.security.utils.LogParameterEscapeUtil;
import org.apache.fineract.organisation.office.domain.Office;
import org.apache.fineract.organisation.staff.domain.Staff;
import org.apache.fineract.portfolio.client.domain.Client;
import org.apache.fineract.useradministration.service.AppUserConstants;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.userdetails.User;
+@Slf4j
@Entity
@Table(name = "m_appuser", uniqueConstraints = @UniqueConstraint(columnNames = { "username" }, name = "username_org"))
public class AppUser extends AbstractPersistableCustom implements PlatformUser {
- private static final Logger LOG = LoggerFactory.getLogger(AppUser.class);
-
@Column(name = "email", nullable = false, length = 100)
private String email;
@@ -622,16 +621,14 @@ public class AppUser extends AbstractPersistableCustom implements PlatformUser {
public void validateHasPermissionTo(final String function) {
if (hasNotPermissionTo(function)) {
final String authorizationMessage = "User has no authority to: " + function;
- LOG.info("Unauthorized access: userId: {} action: {} allowed: {}", getId(), function, getAuthorities());
+ log.info("Unauthorized access: userId: {} action: {} allowed: {}", getId(), LogParameterEscapeUtil.escapeLogParameter(function),
+ getAuthorities());
throw new NoAuthorizationException(authorizationMessage);
}
}
public void validateHasReadPermission(final String function, final Long userId) {
- if ("USER".equalsIgnoreCase(function) && userId.equals(getId())) {
- // abstain from validation as user allowed fetch their own data no
- // matter what permissions they have.
- } else {
+ if (!("USER".equalsIgnoreCase(function) && userId.equals(getId()))) {
validateHasReadPermission(function);
}
}
diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/security/utils/LogParameterEscapeUtilTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/security/utils/LogParameterEscapeUtilTest.java
new file mode 100644
index 000000000..00860a4ba
--- /dev/null
+++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/security/utils/LogParameterEscapeUtilTest.java
@@ -0,0 +1,51 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.utils;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import io.cucumber.java8.En;
+
+public class LogParameterEscapeUtilTest implements En {
+
+ private String logParameter;
+
+ private String escapedLogParameter;
+
+ public LogParameterEscapeUtilTest() {
+ Given("A simple log message without any special character", () -> {
+ logParameter = "This is a very simple String without any special character.";
+ });
+ Given("A log message with new line, carriage return and tab characters", () -> {
+ logParameter = "This String contains new line\n, carriage return\r and tab\t characters.";
+ });
+
+ When("Log parameter escape util escaping the special characters", () -> {
+ escapedLogParameter = LogParameterEscapeUtil.escapeLogParameter(logParameter);
+ });
+
+ Then("The log message stays as it is", () -> {
+ assertEquals(logParameter, escapedLogParameter);
+ });
+ Then("The escape util changes the special characters to `_`", () -> {
+ assertEquals("This String contains new line_, carriage return_ and tab_ characters.", escapedLogParameter);
+ });
+ }
+
+}
diff --git a/fineract-provider/src/test/resources/features/infrastructure/infrastructure.security.utils.feature b/fineract-provider/src/test/resources/features/infrastructure/infrastructure.security.utils.feature
new file mode 100644
index 000000000..c144e4ae9
--- /dev/null
+++ b/fineract-provider/src/test/resources/features/infrastructure/infrastructure.security.utils.feature
@@ -0,0 +1,13 @@
+Feature: Security Utils Infrastructure
+
+ @security
+ Scenario: Verify that log parameter escape util does not change strings that does not contain any special character
+ Given A simple log message without any special character
+ When Log parameter escape util escaping the special characters
+ Then The log message stays as it is
+
+ @security
+ Scenario: Verify that log parameter escape util changes special characters in string
+ Given A log message with new line, carriage return and tab characters
+ When Log parameter escape util escaping the special characters
+ Then The escape util changes the special characters to `_`