diff --git a/application/src/main/java/org/thingsboard/server/controller/UserController.java b/application/src/main/java/org/thingsboard/server/controller/UserController.java index ed55bcf324..3e202de938 100644 --- a/application/src/main/java/org/thingsboard/server/controller/UserController.java +++ b/application/src/main/java/org/thingsboard/server/controller/UserController.java @@ -92,7 +92,7 @@ import static org.thingsboard.server.controller.ControllerConstants.UUID_WIKI_LI public class UserController extends BaseController { public static final String USER_ID = "userId"; - public static final String JSON_PATHS = "jsonPaths"; + public static final String PATHS = "paths"; public static final String YOU_DON_T_HAVE_PERMISSION_TO_PERFORM_THIS_OPERATION = "You don't have permission to perform this operation!"; public static final String ACTIVATE_URL_PATTERN = "%s/api/noauth/activate?activateToken=%s"; @@ -402,21 +402,17 @@ public class UserController extends BaseController { @ApiOperation(value = "Update user settings (saveUserSettings)", notes = "Update user settings for authorized user. Only specified json elements will be updated." + - "Example: you have such settings: {A:5, B:{C:10, D:5}}. Updating it with {A:10, E:6} will result in" + - "{A:10, B:{C:10, D:5}}, E:6") + "Example: you have such settings: {A:5, B:{C:10, D:20}}. Updating it with {B:{C:10, D:30}} will result in" + + "{A:5, B:{C:10, D:30}}. The same could be achieved by putting {B.D:30}") @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") @PutMapping(value = "/user/settings") - public JsonNode putUserSettings(@RequestBody JsonNode settings) throws ThingsboardException { + public void putUserSettings(@RequestBody JsonNode settings) throws ThingsboardException { SecurityUser currentUser = getCurrentUser(); - - UserSettings userSettings = new UserSettings(); - userSettings.setSettings(settings); - userSettings.setUserId(currentUser.getId()); - return userSettingsService.updateUserSettings(currentUser.getTenantId(), userSettings).getSettings(); + userSettingsService.updateUserSettings(currentUser.getTenantId(), currentUser.getId(), settings); } @ApiOperation(value = "Get user settings (getUserSettings)", - notes = "Fetch the User settings based on the provided User Id. " ) + notes = "Fetch the User settings based on authorized user. " ) @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") @GetMapping(value = "/user/settings") public JsonNode getUserSettings() throws ThingsboardException { @@ -430,12 +426,12 @@ public class UserController extends BaseController { notes = "Delete user settings by specifying list of json element xpaths. \n " + "Example: to delete B and C element in { \"A\": {\"B\": 5}, \"C\": 15} send A.B,C in jsonPaths request parameter" ) @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") - @RequestMapping(value = "/user/settings/{jsonPaths}", method = RequestMethod.DELETE) - public void deleteUserSettings( @ApiParam(value = JSON_PATHS) - @PathVariable(JSON_PATHS) String jsonPaths) throws ThingsboardException { - checkParameter(USER_ID, jsonPaths); + @RequestMapping(value = "/user/settings/{paths}", method = RequestMethod.DELETE) + public void deleteUserSettings(@ApiParam(value = PATHS) + @PathVariable(PATHS) String paths) throws ThingsboardException { + checkParameter(USER_ID, paths); SecurityUser currentUser = getCurrentUser(); - userSettingsService.deleteUserSettings(currentUser.getTenantId(), currentUser.getId(), Arrays.asList(jsonPaths.split(","))); + userSettingsService.deleteUserSettings(currentUser.getTenantId(), currentUser.getId(), Arrays.asList(paths.split(","))); } } diff --git a/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java index 81111d2de4..1e49974b28 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseUserControllerTest.java @@ -29,7 +29,6 @@ import org.springframework.context.annotation.Primary; import org.springframework.http.HttpHeaders; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.ResultActions; -import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.server.common.data.Customer; import org.thingsboard.server.common.data.StringUtils; import org.thingsboard.server.common.data.Tenant; @@ -41,7 +40,6 @@ import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.security.Authority; -import org.thingsboard.server.common.data.security.UserSettings; import org.thingsboard.server.dao.exception.DataValidationException; import org.thingsboard.server.dao.user.UserDao; import org.thingsboard.server.service.mail.TestMailService; @@ -755,34 +753,93 @@ public abstract class BaseUserControllerTest extends AbstractControllerTest { Assert.assertEquals(retrievedSettings, userSettings); } + @Test + public void testShouldNotSaveJsonWithRestrictedSymbols() throws Exception { + loginCustomerUser(); + + JsonNode userSettings = mapper.readTree("{\"A.B\":5, \"E\":18}"); + doPost("/api/user/settings", userSettings).andExpect(status().isBadRequest()); + + userSettings = mapper.readTree("{\"A,B\":5, \"E\":18}"); + doPost("/api/user/settings", userSettings).andExpect(status().isBadRequest()); + } + @Test public void testUpdateUserSettings() throws Exception { loginCustomerUser(); - JsonNode userSettings = mapper.readTree("{\"A\":5, \"B\":10, \"E\":18}"); + JsonNode userSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\":true, \"D\":\"stringValue\"}}"); JsonNode savedSettings = doPost("/api/user/settings", userSettings, JsonNode.class); Assert.assertEquals(userSettings, savedSettings); - JsonNode newSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"D\": 16}}"); - JsonNode updatedSettings = doPut("/api/user/settings", newSettings, JsonNode.class); - JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"D\": 16}, \"E\":18}"); + JsonNode newSettings = mapper.readTree("{\"A\":10}"); + doPut("/api/user/settings", newSettings); + JsonNode updatedSettings = doGet("/api/user/settings", JsonNode.class); + JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":true, \"D\":\"stringValue\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - JsonNode patchedSettings = mapper.readTree("\"C\":{\"E\": 22}}"); - updatedSettings = doPut("/api/user/settings", patchedSettings, JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"E\": 22}, \"E\":18}"); + JsonNode patchedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); + doPut("/api/user/settings", patchedSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("\"C.D\": 16}}"); - updatedSettings = doPut("/api/user/settings", patchedSettings, JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"D\": 16, \"E\": 22}, \"E\":18}"); + patchedSettings = mapper.readTree("{\"B.D\": \"stringValue3\"}"); + doPut("/api/user/settings", patchedSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\": \"stringValue3\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("\"C.D\": {\"A\":5}}}"); - updatedSettings = doPut("/api/user/settings", patchedSettings, JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":10, \"C\":{\"D\": {\"A\":5}, \"E\": 22}, \"E\":18}"); + patchedSettings = mapper.readTree("{\"B.D\": {\"E\": 76, \"F\": 92}}"); + doPut("/api/user/settings", patchedSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\": {\"E\":76, \"F\": 92}}}"); Assert.assertEquals(expectedSettings, updatedSettings); + patchedSettings = mapper.readTree("{\"B.D.E\": 100}"); + doPut("/api/user/settings", patchedSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":11, \"B\":{\"C\":false, \"D\": {\"E\":100, \"F\": 92}}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + } + + @Test + public void testShouldCreatePathIfNotExists() throws Exception { + loginCustomerUser(); + + JsonNode userSettings = mapper.readTree("{\"A\":5}"); + JsonNode savedSettings = doPost("/api/user/settings", userSettings, JsonNode.class); + Assert.assertEquals(userSettings, savedSettings); + + JsonNode newSettings = mapper.readTree("{\"B\":{\"C\": 10}}"); + doPut("/api/user/settings", newSettings); + JsonNode updatedSettings = doGet("/api/user/settings", JsonNode.class); + JsonNode expectedSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\": 10}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + + newSettings = mapper.readTree("{\"B.K\":true}"); + doPut("/api/user/settings", newSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":5, \"B\":{\"C\": 10, \"K\": true}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + + newSettings = mapper.readTree("{\"B\":{}}"); + doPut("/api/user/settings", newSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":5, \"B\":{}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + + newSettings = mapper.readTree("{\"F.G\":\"string\"}"); + doPut("/api/user/settings", newSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":5, \"B\":{}, \"F\":{\"G\": \"string\"}}"); + Assert.assertEquals(expectedSettings, updatedSettings); + + newSettings = mapper.readTree("{\"F\":{\"G\":\"string2\"}}"); + doPut("/api/user/settings", newSettings); + updatedSettings = doGet("/api/user/settings", JsonNode.class); + expectedSettings = mapper.readTree("{\"A\":5, \"B\":{}, \"F\":{\"G\": \"string2\"}}"); + Assert.assertEquals(expectedSettings, updatedSettings); } @Test diff --git a/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java b/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java index 30f9b9334a..2c6b149607 100644 --- a/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java +++ b/common/dao-api/src/main/java/org/thingsboard/server/dao/user/UserSettingsService.java @@ -15,6 +15,7 @@ */ package org.thingsboard.server.dao.user; +import com.fasterxml.jackson.databind.JsonNode; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.security.UserSettings; @@ -23,7 +24,7 @@ import java.util.List; public interface UserSettingsService { - UserSettings updateUserSettings(TenantId tenantId, UserSettings userSettings); + void updateUserSettings(TenantId tenantId, UserId userId, JsonNode settings); UserSettings saveUserSettings(TenantId tenantId, UserSettings userSettings); diff --git a/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java index dc1e18352c..593a8846cc 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/user/UserSettingsServiceImpl.java @@ -15,11 +15,15 @@ */ package org.thingsboard.server.dao.user; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.github.fge.jackson.NodeType; import com.jayway.jsonpath.DocumentContext; import com.jayway.jsonpath.JsonPath; +import com.jayway.jsonpath.PathNotFoundException; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -29,10 +33,13 @@ import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.security.UserSettings; import org.thingsboard.server.dao.entity.AbstractCachedService; +import org.thingsboard.server.dao.exception.DataValidationException; import java.util.ArrayList; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import static org.thingsboard.server.dao.service.Validator.validateId; @@ -51,13 +58,17 @@ public class UserSettingsServiceImpl extends AbstractCachedService fieldNames = updateNode.fieldNames(); + private void validateJsonKeys(JsonNode userSettings) { + Iterator fieldNames = userSettings.fieldNames(); while (fieldNames.hasNext()) { String fieldName = fieldNames.next(); - JsonNode jsonNode = mainNode.get(fieldName); - if (jsonNode != null && jsonNode.isObject()) { - merge(jsonNode, updateNode.get(fieldName)); - } else { - if (mainNode instanceof ObjectNode) { - JsonNode value = updateNode.get(fieldName); - ((ObjectNode) mainNode).set(fieldName, value); - } + if (fieldName.contains(".") || fieldName.contains(",")) { + throw new DataValidationException("Json field name should not contain \".\" or \",\" symbols"); } } - return mainNode; + } + + public JsonNode update(JsonNode mainNode, JsonNode updateNode) { + DocumentContext dcOldSettings = JsonPath.parse(mainNode.toString()); + Iterator fieldNames = updateNode.fieldNames(); + while (fieldNames.hasNext()) { + String fieldName = fieldNames.next(); + createPathIfNotExists(dcOldSettings, "$."+ fieldName); + dcOldSettings = dcOldSettings.set("$." + fieldName, getValueByNodeType(updateNode.get(fieldName))); + } + try { + return new ObjectMapper().readValue(dcOldSettings.jsonString(), ObjectNode.class); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + private static String createPathIfNotExists(DocumentContext dcOldSettings, String path) { + try { + dcOldSettings.read(path); + return path; + } catch (PathNotFoundException e) { + String lastElement = path.substring(path.lastIndexOf(".") + 1); + String pathToLastElement = path.substring(0, path.lastIndexOf(".")); + dcOldSettings.put(createPathIfNotExists(dcOldSettings, pathToLastElement), lastElement, new LinkedHashMap()); + return path; + } + } + + private static Object getValueByNodeType(final JsonNode value) + { + final NodeType type = NodeType.getNodeType(value); + switch (type) { + case STRING: + return value.textValue(); + case NUMBER: + case INTEGER: + return value.bigIntegerValue(); + case NULL: + case ARRAY: + return value; + case OBJECT: + return new ObjectMapper().convertValue(value, new TypeReference>() {}); + case BOOLEAN: + return value.booleanValue(); + default: + throw new UnsupportedOperationException(); + } } }