Fix bugs, vulnerability and code smell issues: remove useless imports…

…, constructors and comments, add logging for exceptions, refactor some methods and properties; fix tests
bulktrade · Feb 13, 2017 · ff0128d · ff0128d
1 parent ff8b88f
commit ff0128d
Show file tree

Hide file tree

Showing 37 changed files with 158 additions and 401 deletions.
diff --git a/modules/core/pom.xml b/modules/core/pom.xml
@@ -2,24 +2,24 @@
 <project xmlns="http://maven.apache.org/POM/4.0.0"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
-    <parent>
-        <artifactId>smsc</artifactId>
-        <groupId>io.smsc</groupId>
-        <version>0.0.1-SNAPSHOT</version>
-        <relativePath>../../pom.xml</relativePath>
-    </parent>
-    <modelVersion>4.0.0</modelVersion>
+        <parent>
+            <artifactId>smsc</artifactId>
+            <groupId>io.smsc</groupId>
+            <version>0.0.1-SNAPSHOT</version>
+            <relativePath>../../pom.xml</relativePath>
+        </parent>
+        <modelVersion>4.0.0</modelVersion>
 
-    <artifactId>core</artifactId>
-    <packaging>jar</packaging>
-    <version>0.0.1-SNAPSHOT</version>
+        <artifactId>core</artifactId>
+        <packaging>jar</packaging>
+        <version>0.0.1-SNAPSHOT</version>
 
-    <properties>
+        <properties>
         <timestamp>${maven.build.timestamp}</timestamp>
         <maven.build.timestamp.format>yyyy-MM-dd-HH:mm</maven.build.timestamp.format>
-    </properties>
+        </properties>
 
-    <dependencies>
+        <dependencies>
         <dependency>
             <groupId>io.smsc</groupId>
             <artifactId>admin</artifactId>
@@ -129,9 +129,9 @@
             <groupId>io.jsonwebtoken</groupId>
             <artifactId>jjwt</artifactId>
         </dependency>
-    </dependencies>
+        </dependencies>
 
-    <build>
+        <build>
         <finalName>smsc</finalName>
         <plugins>
             <plugin>

diff --git a/modules/core/src/main/java/io/smsc/Application.java b/modules/core/src/main/java/io/smsc/Application.java
@@ -11,7 +11,7 @@
 @PropertySource(value = "classpath:application.properties")
 @PropertySource(value = "classpath:${smsc.database.dialect:hsqldb}.properties")
 @ComponentScan("io.smsc")
-public class Application {
+class Application {
 
     public static void main(String[] args) {
 //    Solution of JCE problem for JDK 9 (reflection is not more needed)

diff --git a/modules/core/src/main/java/io/smsc/config/RepositoryIdExposingConfiguration.java b/modules/core/src/main/java/io/smsc/config/RepositoryIdExposingConfiguration.java
@@ -36,19 +36,6 @@ public class RepositoryIdExposingConfiguration extends RepositoryRestConfigurerA
 
     @Override
     public void configureRepositoryRestConfiguration(RepositoryRestConfiguration config) {
-//        ClassPathScanningCandidateComponentProvider provider = new ClassPathScanningCandidateComponentProvider(true);
-//        provider.addIncludeFilter(new AnnotationTypeFilter(Entity.class));
-//        Set<BeanDefinition> components = provider.findCandidateComponents(this.getClass().getPackage().getName());
-//        List<Class<?>> classes = new ArrayList<>();
-//
-//        components.forEach(component -> {
-//            try {
-//                classes.add(Class.forName(component.getBeanClassName()));
-//            } catch (Exception e) {
-//                e.printStackTrace();
-//            }
-//        });
-//        config.exposeIdsFor(classes.toArray(new Class[classes.size()]));
 
         // for this time only one solution which works completely
         config.exposeIdsFor(User.class, CustomerUser.class, Role.class, Customer.class, CustomerContact.class, Dashboard.class,

diff --git a/modules/core/src/main/java/io/smsc/config/SecurityConfiguration.java b/modules/core/src/main/java/io/smsc/config/SecurityConfiguration.java
@@ -55,10 +55,9 @@ public void configureAuthentication(AuthenticationManagerBuilder authenticationM
      * Gets the {@link JWTAuthenticationTokenFilter} bean
      *
      * @return authenticationTokenFilter
-     * @throws Exception if an error occurs
      */
     @Bean
-    public JWTAuthenticationTokenFilter authenticationTokenFilterBean() throws Exception {
+    public JWTAuthenticationTokenFilter authenticationTokenFilterBean() {
         return new JWTAuthenticationTokenFilter(userDetailsService, tokenGenerationService);
     }
 

diff --git a/modules/core/src/main/java/io/smsc/controller/AuthController.java b/modules/core/src/main/java/io/smsc/controller/AuthController.java
@@ -1,22 +1,20 @@
 package io.smsc.controller;
 
-import io.smsc.model.User;
 import io.smsc.security.model.*;
 import io.smsc.security.service.JWTTokenGenerationService;
 import io.smsc.security.service.JWTUserDetailsService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
-import org.springframework.security.access.prepost.PreAuthorize;
-import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.web.bind.annotation.*;
 
 import javax.servlet.http.HttpServletResponse;
 
 import java.io.IOException;
 
 import static org.springframework.util.MimeTypeUtils.APPLICATION_JSON_VALUE;
-import static org.springframework.web.bind.annotation.RequestMethod.GET;
 
 /**
  * The AuthController class is used for mapping HTTP requests for receiving and updating
@@ -28,6 +26,8 @@
 @RestController
 public class AuthController {
 
+    private static final Logger LOG = LoggerFactory.getLogger(AuthController.class);
+
     private final JWTTokenGenerationService jwtTokenGenerationService;
 
     private final JWTUserDetailsService jwtUserDetailsService;
@@ -58,6 +58,7 @@ public ResponseEntity<JWTAuthenticationResponse> token(@RequestBody JWTAuthentic
                 return new ResponseEntity<>(token, HttpStatus.OK);
             }
         } catch (Exception ex) {
+            LOG.info("Some exception occurred", ex);
             // going to send error
         }
         response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Credentials are invalid. Please enter valid username and password");
@@ -86,6 +87,7 @@ public ResponseEntity<JWTRefreshTokenResponse> token(@RequestBody JWTRefreshToke
                 return new ResponseEntity<>(token, HttpStatus.OK);
             }
         } catch (Exception ex) {
+            LOG.info("Some exception occurred", ex);
             // going to send error
         }
         response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Refresh or expired access token is invalid. Please enter valid tokens");

diff --git a/modules/core/src/main/java/io/smsc/controller/IndexController.java b/modules/core/src/main/java/io/smsc/controller/IndexController.java
@@ -19,10 +19,7 @@
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.util.Calendar;
 import java.util.Optional;
 
@@ -32,14 +29,15 @@
 public class IndexController {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(IndexController.class);
-    private static final Long lastModified = Calendar.getInstance().getTimeInMillis();
+    private static final Long LAST_MODIFIED = Calendar.getInstance().getTimeInMillis();
+
     @Autowired
     private StaticResourceService staticResourceService;
 
     @RequestMapping("/")
     @ResponseBody
     public String indexAction(ServletWebRequest servletWebRequest, HttpServletResponse response) {
-        if (servletWebRequest.checkNotModified(lastModified)) {
+        if (servletWebRequest.checkNotModified(LAST_MODIFIED)) {
             return null;
         }
 
@@ -72,17 +70,16 @@ public ResponseEntity<Resource> adminAction(
                 String classFilePath = "classpath:META-INF/resources/io.smsc.admin/" + realFilePath;
                 Resource resource = staticResourceService.getResource(classFilePath);
 
-                if (resource.exists()) {
-                    if (servletWebRequest.checkNotModified(DigestUtils.md5Hex(DigestUtils.md5(resource.getInputStream())), lastModified)) {
-                        return null;
-                    }
-
+                if (resource.exists() && servletWebRequest.checkNotModified(DigestUtils.md5Hex(DigestUtils.md5(resource.getInputStream())), LAST_MODIFIED)) {
+                    return null;
+                }
+                else if(resource.exists()) {
                     return new ResponseEntity<>(resource, HttpStatus.OK);
                 }
             }
 
             Resource resource = staticResourceService.getResource("classpath:META-INF/resources/io.smsc.admin/index.html");
-            if (servletWebRequest.checkNotModified(DigestUtils.md5Hex(DigestUtils.md5(resource.getInputStream())), lastModified)) {
+            if (servletWebRequest.checkNotModified(DigestUtils.md5Hex(DigestUtils.md5(resource.getInputStream())), LAST_MODIFIED)) {
                 return null;
             }
 
@@ -125,9 +122,10 @@ public Config configAction(HttpServletResponse response) throws IOException {
             }
 
             if (System.getenv("ADMIN_DEBUG") != null) {
-                config.debug = System.getenv("ADMIN_DEBUG").equals("true");
+                config.debug = "true".equals(System.getenv("ADMIN_DEBUG"));
             }
         } catch (Exception e) {
+            LOGGER.info("Some exception occurred", e);
             config = new Config();
         }
 

diff --git a/modules/core/src/main/java/io/smsc/exception/EmptySaltException.java b/modules/core/src/main/java/io/smsc/exception/EmptySaltException.java
@@ -1,7 +1,9 @@
 package io.smsc.exception;
 
 public class EmptySaltException extends RuntimeException {
+
     public EmptySaltException() {
+        super();
     }
 
     public EmptySaltException(String message) {

diff --git a/modules/core/src/main/java/io/smsc/model/BaseEntity.java b/modules/core/src/main/java/io/smsc/model/BaseEntity.java
@@ -1,9 +1,7 @@
 package io.smsc.model;
 
 import com.fasterxml.jackson.annotation.*;
-import org.hibernate.Hibernate;
 import org.springframework.data.annotation.LastModifiedDate;
-import org.springframework.data.domain.Persistable;
 
 import javax.persistence.*;
 import java.io.Serializable;
@@ -40,15 +38,12 @@ protected void onUpdate() {
         lastModifiedDate = new Date();
     }
 
-    public BaseEntity() {
-    }
-
     public Long getVersion() {
         return version;
     }
 
     public void setVersion(Long versionNumber) {
-        this.version = version;
+        this.version = versionNumber;
     }
 
     public Date getLastModifiedDate() {

diff --git a/modules/core/src/main/java/io/smsc/model/CustomerUser.java b/modules/core/src/main/java/io/smsc/model/CustomerUser.java
@@ -75,28 +75,9 @@ public class CustomerUser extends BaseEntity {
     @JsonBackReference
     private Customer customer;
 
-    public CustomerUser() {
-    }
-
-    public CustomerUser(CustomerUser user) {
-        this(user.getId(), user.getUsername(), user.getPassword(), user.getFirstname(), user.getSurname(), user.getEmail(), user.isActive(), user.isBlocked(), user.getCustomer());
-    }
-
-    public CustomerUser(Long id, String username, String password, String firstname, String surname, String email, boolean active, boolean blocked, Customer customer) {
-        this.id = id;
-        this.username = username;
-        this.password = password;
-        this.firstname = firstname;
-        this.surname = surname;
-        this.email = email;
-        this.active = active;
-        this.blocked = blocked;
-        this.customer = customer;
-    }
-
     @JsonIgnore
     public boolean isNew() {
-        return (getId() == null);
+        return getId() == null;
     }
 
     public Long getId() {
@@ -196,7 +177,6 @@ public String toString() {
         return "CustomerUser{" +
                 "id=" + id +
                 ", username='" + username + '\'' +
-                ", password='" + password + '\'' +
                 ", salt='" + salt + '\'' +
                 ", firstname='" + firstname + '\'' +
                 ", surname='" + surname + '\'' +

diff --git a/modules/core/src/main/java/io/smsc/model/Role.java b/modules/core/src/main/java/io/smsc/model/Role.java
@@ -51,21 +51,9 @@ private void removeRolesFromUsers() {
         }
     }
 
-    public Role() {
-    }
-
-    public Role(Role role) {
-        this(role.getId(), role.getName());
-    }
-
-    public Role(Long id, String name) {
-        this.id = id;
-        this.name = name;
-    }
-
     @JsonIgnore
     public boolean isNew() {
-        return (getId() == null);
+        return getId() == null;
     }
 
     public Long getId() {

diff --git a/modules/core/src/main/java/io/smsc/model/User.java b/modules/core/src/main/java/io/smsc/model/User.java
@@ -93,27 +93,9 @@ public class User extends BaseEntity {
     @OrderBy("id asc")
     private Set<Dashboard> dashboards;
 
-    public User() {
-    }
-
-    public User(User user) {
-        this(user.getId(), user.getUsername(), user.getPassword(), user.getFirstname(), user.getSurname(), user.getEmail(), user.isActive(), user.isBlocked());
-    }
-
-    public User(Long id, String username, String password, String firstname, String surname, String email, boolean active, boolean blocked) {
-        this.id = id;
-        this.username = username;
-        this.password = password;
-        this.firstname = firstname;
-        this.surname = surname;
-        this.email = email;
-        this.active = active;
-        this.blocked = blocked;
-    }
-
     @JsonIgnore
     public boolean isNew() {
-        return (getId() == null);
+        return getId() == null;
     }
 
     public Long getId() {
@@ -221,7 +203,6 @@ public String toString() {
         return "User{" +
                 "id=" + id +
                 ", username='" + username + '\'' +
-                ", password='" + password + '\'' +
                 ", salt='" + salt + '\'' +
                 ", firstname='" + firstname + '\'' +
                 ", surname='" + surname + '\'' +

diff --git a/modules/core/src/main/java/io/smsc/model/acl/AclClass.java b/modules/core/src/main/java/io/smsc/model/acl/AclClass.java
@@ -38,17 +38,9 @@ public class AclClass extends BaseEntity {
     @OrderBy
     private Set<AclObjectIdentity> aclObjectIdentities;
 
-    public AclClass() {
-    }
-
-    public AclClass(Long id, String className) {
-        this.id = id;
-        this.className = className;
-    }
-
     @JsonIgnore
     public boolean isNew() {
-        return (getId() == null);
+        return getId() == null;
     }
 
     public Long getId() {

diff --git a/modules/core/src/main/java/io/smsc/model/acl/AclEntry.java b/modules/core/src/main/java/io/smsc/model/acl/AclEntry.java
@@ -77,23 +77,9 @@ public class AclEntry extends BaseEntity {
     @NotNull(message = "{acl.entry.sid.validation}")
     private AclSid sid;
 
-    public AclEntry() {
-    }
-
-    public AclEntry(Long id, AclObjectIdentity aclObjectIdentity, Integer aceOrder, AclSid sid, Integer mask, Boolean granting, Boolean auditSuccess, Boolean auditFailure) {
-        this.id = id;
-        this.aclObjectIdentity = aclObjectIdentity;
-        this.aceOrder = aceOrder;
-        this.sid = sid;
-        this.mask = mask;
-        this.granting = granting;
-        this.auditSuccess = auditSuccess;
-        this.auditFailure = auditFailure;
-    }
-
     @JsonIgnore
     public boolean isNew() {
-        return (getId() == null);
+        return getId() == null;
     }
 
     public Long getId() {