From d16df47ac13187726e822938400adf1a543edaa0 Mon Sep 17 00:00:00 2001
From: Piotr Gawron <piotr.gawron@uni.lu>
Date: Thu, 15 Nov 2018 10:09:10 +0100
Subject: [PATCH] validator for integer configuration options added

---
 .../lcsb/mapviewer/api/QueryException.java    |  4 +++
 .../ConfigurationController.java              |  3 +-
 .../configuration/ConfigurationRestImpl.java  | 10 ++++--
 .../ConfigurationRestImplTest.java            | 32 +++++++++++++++++++
 .../services/impl/ConfigurationService.java   | 12 +++++++
 5 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/QueryException.java b/rest-api/src/main/java/lcsb/mapviewer/api/QueryException.java
index f3e367c483..11abd5a073 100644
--- a/rest-api/src/main/java/lcsb/mapviewer/api/QueryException.java
+++ b/rest-api/src/main/java/lcsb/mapviewer/api/QueryException.java
@@ -23,6 +23,10 @@ public class QueryException extends Exception {
 		super(message);
 	}
 
+  public QueryException(Exception e) {
+    super(e);
+  }
+
 	/**
 	 * Constructor with error message and parent exception.
 	 * 
diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/configuration/ConfigurationController.java b/rest-api/src/main/java/lcsb/mapviewer/api/configuration/ConfigurationController.java
index faf0b5f30b..00b9af74a6 100644
--- a/rest-api/src/main/java/lcsb/mapviewer/api/configuration/ConfigurationController.java
+++ b/rest-api/src/main/java/lcsb/mapviewer/api/configuration/ConfigurationController.java
@@ -20,6 +20,7 @@ import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.databind.JsonMappingException;
 
 import lcsb.mapviewer.api.BaseController;
+import lcsb.mapviewer.api.QueryException;
 import lcsb.mapviewer.common.Configuration;
 import lcsb.mapviewer.services.SecurityException;
 import lcsb.mapviewer.services.interfaces.IConfigurationService;
@@ -76,7 +77,7 @@ public class ConfigurationController extends BaseController {
       @RequestBody String body, //
       @CookieValue(value = Configuration.AUTH_TOKEN) String token, //
       @PathVariable(value = "option") String option //
-  ) throws SecurityException, JsonParseException, JsonMappingException, IOException {
+  ) throws SecurityException, JsonParseException, JsonMappingException, IOException, QueryException {
     Map<String, Object> node = parseBody(body);
     Map<String, Object> data = getData(node, "option");
     return configurationController.updateOption(token, option, data);
diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/configuration/ConfigurationRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/configuration/ConfigurationRestImpl.java
index f39bc6d4b6..e186c71150 100644
--- a/rest-api/src/main/java/lcsb/mapviewer/api/configuration/ConfigurationRestImpl.java
+++ b/rest-api/src/main/java/lcsb/mapviewer/api/configuration/ConfigurationRestImpl.java
@@ -18,7 +18,9 @@ import org.springframework.transaction.annotation.Transactional;
 import lcsb.mapviewer.annotation.services.ModelAnnotator;
 import lcsb.mapviewer.annotation.services.annotators.ElementAnnotator;
 import lcsb.mapviewer.api.BaseRestImpl;
+import lcsb.mapviewer.api.QueryException;
 import lcsb.mapviewer.common.Pair;
+import lcsb.mapviewer.common.exception.InvalidArgumentException;
 import lcsb.mapviewer.converter.IConverter;
 import lcsb.mapviewer.converter.graphics.AbstractImageGenerator;
 import lcsb.mapviewer.converter.graphics.ImageGenerators;
@@ -273,13 +275,17 @@ public class ConfigurationRestImpl extends BaseRestImpl {
   }
 
   public Map<String, Object> updateOption(String token, String option, Map<String, Object> data)
-      throws SecurityException {
+      throws SecurityException, QueryException {
     if (!getUserService().userHasPrivilege(token, PrivilegeType.CONFIGURATION_MANAGE)) {
       throw new SecurityException("Acces denied");
     }
     ConfigurationElementType type = ConfigurationElementType.valueOf(option);
     String value = (String) data.get("value");
-    configurationService.setConfigurationValue(type, value);
+    try {
+      configurationService.setConfigurationValue(type, value);
+    } catch (InvalidArgumentException e) {
+      throw new QueryException(e);
+    }
     return optionToMap(configurationService.getValue(type));
   }
 
diff --git a/rest-api/src/test/java/lcsb/mapviewer/api/configuration/ConfigurationRestImplTest.java b/rest-api/src/test/java/lcsb/mapviewer/api/configuration/ConfigurationRestImplTest.java
index d0387acc10..3bd8484bc5 100644
--- a/rest-api/src/test/java/lcsb/mapviewer/api/configuration/ConfigurationRestImplTest.java
+++ b/rest-api/src/test/java/lcsb/mapviewer/api/configuration/ConfigurationRestImplTest.java
@@ -2,7 +2,9 @@ package lcsb.mapviewer.api.configuration;
 
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -14,10 +16,12 @@ import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 
+import lcsb.mapviewer.api.QueryException;
 import lcsb.mapviewer.api.RestTestFunctions;
 import lcsb.mapviewer.model.map.MiriamType;
 import lcsb.mapviewer.model.map.reaction.type.StateTransitionReaction;
 import lcsb.mapviewer.model.map.species.GenericProtein;
+import lcsb.mapviewer.model.user.ConfigurationElementType;
 
 public class ConfigurationRestImplTest extends RestTestFunctions {
 
@@ -50,6 +54,34 @@ public class ConfigurationRestImplTest extends RestTestFunctions {
     }
   }
 
+  @Test
+  public void testSetSmtpPortToInvalid() throws Exception {
+    try {
+      Map<String, Object> data = new HashMap<>();
+      data.put("value", "not a number");
+      configurationRestImpl.updateOption(adminToken, ConfigurationElementType.EMAIL_SMTP_PORT.name(), data);
+      fail("Exception expected");
+    } catch (QueryException e) {
+    } catch (Exception e) {
+      e.printStackTrace();
+      throw e;
+    }
+  }
+
+  @Test
+  public void testSetSmtpPortToValid() throws Exception {
+    try {
+      Map<String, Object> data = new HashMap<>();
+      data.put("value", "255");
+      Map<String, Object> result = configurationRestImpl.updateOption(adminToken,
+          ConfigurationElementType.EMAIL_SMTP_PORT.name(), data);
+      assertNotNull(result);
+    } catch (Exception e) {
+      e.printStackTrace();
+      throw e;
+    }
+  }
+
   @Test
   public void testImageFormats() throws Exception {
     try {
diff --git a/service/src/main/java/lcsb/mapviewer/services/impl/ConfigurationService.java b/service/src/main/java/lcsb/mapviewer/services/impl/ConfigurationService.java
index 491868922a..90f4c7b16d 100644
--- a/service/src/main/java/lcsb/mapviewer/services/impl/ConfigurationService.java
+++ b/service/src/main/java/lcsb/mapviewer/services/impl/ConfigurationService.java
@@ -10,6 +10,7 @@ import org.springframework.transaction.annotation.Transactional;
 
 import lcsb.mapviewer.common.Configuration;
 import lcsb.mapviewer.common.FrameworkVersion;
+import lcsb.mapviewer.common.exception.InvalidArgumentException;
 import lcsb.mapviewer.model.user.ConfigurationElementType;
 import lcsb.mapviewer.model.user.ConfigurationOption;
 import lcsb.mapviewer.model.user.PrivilegeType;
@@ -57,6 +58,17 @@ public class ConfigurationService implements IConfigurationService {
 
   @Override
   public void setConfigurationValue(ConfigurationElementType type, String value) {
+    if (value == null) {
+      throw new InvalidArgumentException("null is not a proper value");
+    }
+    switch (type.getEditType()) {
+    case INTEGER:
+      if (!value.matches("-?\\d+")) {
+        throw new InvalidArgumentException("Expected integer value but got: " + value);
+      }
+      break;
+    default:
+    }
     ConfigurationOption configuration = configurationDao.getByType(type);
     if (configuration == null) {
       configuration = new ConfigurationOption();
-- 
GitLab