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 a38ef27d62..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,6 @@ 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 PATH = "path"; 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"; @@ -403,8 +402,8 @@ 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 void putUserSettings(@RequestBody JsonNode settings) throws ThingsboardException { @@ -412,20 +411,8 @@ public class UserController extends BaseController { userSettingsService.updateUserSettings(currentUser.getTenantId(), currentUser.getId(), settings); } - @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") - @PreAuthorize("hasAnyAuthority('SYS_ADMIN', 'TENANT_ADMIN', 'CUSTOMER_USER')") - @PutMapping(value = "/user/settings/{path}") - public void putUserSettings(@ApiParam(value = PATH) - @PathVariable(PATH) String path, @RequestBody JsonNode settings) throws ThingsboardException { - SecurityUser currentUser = getCurrentUser(); - userSettingsService.updateUserSettings(currentUser.getTenantId(), currentUser.getId(), path, 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 { 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 4679ee4411..d15841f5a2 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,39 +753,62 @@ 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\":{\"C\":5, \"D\":5}}"); + 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}"); doPut("/api/user/settings", newSettings); JsonNode updatedSettings = doGet("/api/user/settings", JsonNode.class); - JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":5, \"D\":5}}"); + JsonNode expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":true, \"D\":\"stringValue\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - JsonNode patchedSettings = mapper.readTree("{\"B\":{\"E\": 22}}"); + JsonNode patchedSettings = mapper.readTree("{\"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"E\": 22}}"); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\":\"stringValue2\"}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("{\"I\": 56}"); - doPut("/api/user/settings/B.E", patchedSettings); + patchedSettings = mapper.readTree("{\"B.D\": {\"E\": 56}}"); + doPut("/api/user/settings", patchedSettings); updatedSettings = doGet("/api/user/settings", JsonNode.class); - expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"E\": {\"I\":56}}}"); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\": 56}}}"); Assert.assertEquals(expectedSettings, updatedSettings); - patchedSettings = mapper.readTree("{\"I\": 76, \"F\": 92}"); - doPut("/api/user/settings/B.E", patchedSettings); + 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\":10, \"B\":{\"E\": {\"I\":76, \"F\": 92}}}"); + expectedSettings = mapper.readTree("{\"A\":10, \"B\":{\"C\":false, \"D\": {\"E\":76, \"F\": 92}}}"); Assert.assertEquals(expectedSettings, updatedSettings); } + @Test + public void testShouldNotUpdateUserSettingsWithNoExistingPath() throws Exception { + loginCustomerUser(); + + 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.E\":10}"); + doPut("/api/user/settings", newSettings).andExpect(status().isBadRequest()); + } + @Test public void testDeleteUserSettings() throws Exception { loginCustomerUser(); 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 0b2c6f5aef..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,7 +15,6 @@ */ package org.thingsboard.server.dao.user; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.id.UserId; @@ -26,7 +25,6 @@ import java.util.List; public interface UserSettingsService { void updateUserSettings(TenantId tenantId, UserId userId, JsonNode settings); - void updateUserSettings(TenantId tenantId, UserId userId, String path, 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 0c41589b95..a87d53e49d 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 @@ -20,8 +20,10 @@ 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; @@ -31,6 +33,7 @@ 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; @@ -67,26 +70,6 @@ public class UserSettingsServiceImpl extends AbstractCachedService>(){})); - try { - newUserSettings.setSettings(new ObjectMapper().readValue(dcSettings.jsonString(), ObjectNode.class)); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); - } - doSaveUserSettings(tenantId, newUserSettings); - } - @Override public UserSettings findUserSettings(TenantId tenantId, UserId userId) { log.trace("Executing findUserSettings for user [{}]", userId); @@ -119,7 +102,7 @@ public class UserSettingsServiceImpl extends AbstractCachedService fieldNames = userSettings.fieldNames(); + while (fieldNames.hasNext()) { + String fieldName = fieldNames.next(); + if (fieldName.contains(".") || fieldName.contains(",")) { + throw new DataValidationException("Json field name should not contain \".\" or \",\" symbols"); + } + } + } + public JsonNode update(JsonNode mainNode, JsonNode updateNode) { + DocumentContext dcOldSettings = JsonPath.parse(mainNode.toString()); Iterator fieldNames = updateNode.fieldNames(); while (fieldNames.hasNext()) { String fieldName = fieldNames.next(); - JsonNode value = updateNode.get(fieldName); - ((ObjectNode) mainNode).set(fieldName, value); + validatePathExists(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 void validatePathExists(DocumentContext dcOldSettings, String fieldName) { + try { + dcOldSettings.read("$." + fieldName); + }catch (PathNotFoundException e) { + throw new DataValidationException("Json element with path " + fieldName + "was not found"); + } + } + + 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(); } - return mainNode; } }