FINERACT-1483: fix Sonar vulnerabilities with Minor severity

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;
     }
 

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);

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.apache.fineract.infrastructure.security.utils.LogParameterEscapeUtil;
-import org.slf4j.LoggerFactory;
 
+@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);
         }
     }
 }

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]", "_");
+    }
+}

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 new CachingConnectionFactory(amqConnectionFactory());
-        return connectionFactory;
     }
 
     @Bean

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())) {
+        if (!("USER".equalsIgnoreCase(function) && userId.equals(getId()))) {
-            // abstain from validation as user allowed fetch their own data no
-            // matter what permissions they have.
-        } else {
             validateHasReadPermission(function);
         }
     }

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);
+        });
+    }
+
+}

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 `_`