From f1216c97204967ffc9f297623c4678b64108b8c9 Mon Sep 17 00:00:00 2001 From: AndrewVolosytnykhThingsboard Date: Wed, 16 Jun 2021 18:58:55 +0300 Subject: [PATCH] Url validation for Ota Package and tests --- .../controller/OtaPackageController.java | 8 +-- .../BaseOtaPackageControllerTest.java | 27 +++++----- .../server/dao/ota/OtaPackageService.java | 6 +-- .../server/dao/ota/BaseOtaPackageService.java | 12 ++++- .../service/BaseOtaPackageServiceTest.java | 52 +++++++++++++++---- .../thingsboard/rest/client/RestClient.java | 6 ++- 6 files changed, 77 insertions(+), 34 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/OtaPackageController.java b/application/src/main/java/org/thingsboard/server/controller/OtaPackageController.java index e28c5e37e1..ba68b79211 100644 --- a/application/src/main/java/org/thingsboard/server/controller/OtaPackageController.java +++ b/application/src/main/java/org/thingsboard/server/controller/OtaPackageController.java @@ -34,10 +34,10 @@ import org.thingsboard.server.common.data.OtaPackage; import org.thingsboard.server.common.data.OtaPackageInfo; import org.thingsboard.server.common.data.audit.ActionType; import org.thingsboard.server.common.data.exception.ThingsboardException; -import org.thingsboard.server.common.data.ota.ChecksumAlgorithm; -import org.thingsboard.server.common.data.ota.OtaPackageType; import org.thingsboard.server.common.data.id.DeviceProfileId; import org.thingsboard.server.common.data.id.OtaPackageId; +import org.thingsboard.server.common.data.ota.ChecksumAlgorithm; +import org.thingsboard.server.common.data.ota.OtaPackageType; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.queue.util.TbCoreComponent; @@ -109,12 +109,12 @@ public class OtaPackageController extends BaseController { @PreAuthorize("hasAnyAuthority('TENANT_ADMIN')") @RequestMapping(value = "/otaPackage", method = RequestMethod.POST) @ResponseBody - public OtaPackageInfo saveOtaPackageInfo(@RequestBody OtaPackageInfo otaPackageInfo) throws ThingsboardException { + public OtaPackageInfo saveOtaPackageInfo(@RequestBody OtaPackageInfo otaPackageInfo, @RequestParam boolean isUrl) throws ThingsboardException { boolean created = otaPackageInfo.getId() == null; try { otaPackageInfo.setTenantId(getTenantId()); checkEntity(otaPackageInfo.getId(), otaPackageInfo, Resource.OTA_PACKAGE); - OtaPackageInfo savedOtaPackageInfo = otaPackageService.saveOtaPackageInfo(otaPackageInfo); + OtaPackageInfo savedOtaPackageInfo = otaPackageService.saveOtaPackageInfo(otaPackageInfo, isUrl); logEntityAction(savedOtaPackageInfo.getId(), savedOtaPackageInfo, null, created ? ActionType.ADDED : ActionType.UPDATED, null); return savedOtaPackageInfo; diff --git a/application/src/test/java/org/thingsboard/server/controller/BaseOtaPackageControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/BaseOtaPackageControllerTest.java index 6aa341cef6..373f026196 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseOtaPackageControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/BaseOtaPackageControllerTest.java @@ -98,7 +98,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION); - OtaPackageInfo savedFirmwareInfo = save(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false); Assert.assertNotNull(savedFirmwareInfo); Assert.assertNotNull(savedFirmwareInfo.getId()); @@ -109,7 +109,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes savedFirmwareInfo.setAdditionalInfo(JacksonUtil.newObjectNode()); - save(savedFirmwareInfo); + save(savedFirmwareInfo, false); OtaPackageInfo foundFirmwareInfo = doGet("/api/otaPackage/info/" + savedFirmwareInfo.getId().getId().toString(), OtaPackageInfo.class); Assert.assertEquals(foundFirmwareInfo.getTitle(), savedFirmwareInfo.getTitle()); @@ -123,7 +123,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION); - OtaPackageInfo savedFirmwareInfo = save(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false); Assert.assertNotNull(savedFirmwareInfo); Assert.assertNotNull(savedFirmwareInfo.getId()); @@ -134,7 +134,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes savedFirmwareInfo.setAdditionalInfo(JacksonUtil.newObjectNode()); - save(savedFirmwareInfo); + save(savedFirmwareInfo, false); OtaPackageInfo foundFirmwareInfo = doGet("/api/otaPackage/info/" + savedFirmwareInfo.getId().getId().toString(), OtaPackageInfo.class); Assert.assertEquals(foundFirmwareInfo.getTitle(), savedFirmwareInfo.getTitle()); @@ -157,10 +157,10 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION); - OtaPackageInfo savedFirmwareInfo = save(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false); loginDifferentTenant(); - doPost("/api/otaPackage", savedFirmwareInfo, OtaPackageInfo.class, status().isForbidden()); + doPost("/api/otaPackage?isUrl=false", savedFirmwareInfo, OtaPackageInfo.class, status().isForbidden()); deleteDifferentTenant(); } @@ -172,7 +172,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION); - OtaPackageInfo savedFirmwareInfo = save(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false); OtaPackageInfo foundFirmware = doGet("/api/otaPackage/info/" + savedFirmwareInfo.getId().getId().toString(), OtaPackageInfo.class); Assert.assertNotNull(foundFirmware); @@ -187,7 +187,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION); - OtaPackageInfo savedFirmwareInfo = save(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false); MockMultipartFile testData = new MockMultipartFile("file", FILE_NAME, CONTENT_TYPE, DATA.array()); @@ -207,7 +207,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION); - OtaPackageInfo savedFirmwareInfo = save(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false); doDelete("/api/otaPackage/" + savedFirmwareInfo.getId().getId().toString()) .andExpect(status().isOk()); @@ -226,7 +226,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION + i); - OtaPackageInfo savedFirmwareInfo = save(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false); if (i > 100) { MockMultipartFile testData = new MockMultipartFile("file", FILE_NAME, CONTENT_TYPE, DATA.array()); @@ -269,7 +269,7 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION + i); - OtaPackageInfo savedFirmwareInfo = save(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = save(firmwareInfo, false); if (i > 100) { MockMultipartFile testData = new MockMultipartFile("file", FILE_NAME, CONTENT_TYPE, DATA.array()); @@ -316,9 +316,8 @@ public abstract class BaseOtaPackageControllerTest extends AbstractControllerTes Assert.assertEquals(allOtaPackages, allLoadedOtaPackages); } - - private OtaPackageInfo save(OtaPackageInfo firmwareInfo) throws Exception { - return doPost("/api/otaPackage", firmwareInfo, OtaPackageInfo.class); + private OtaPackageInfo save(OtaPackageInfo firmwareInfo, boolean isUrl) throws Exception { + return doPost("/api/otaPackage?isUrl=" + isUrl, firmwareInfo, OtaPackageInfo.class); } protected OtaPackageInfo savaData(String urlTemplate, MockMultipartFile content, String... params) throws Exception { diff --git a/common/dao-api/src/main/java/org/thingsboard/server/dao/ota/OtaPackageService.java b/common/dao-api/src/main/java/org/thingsboard/server/dao/ota/OtaPackageService.java index fea29681c1..b14653b1d0 100644 --- a/common/dao-api/src/main/java/org/thingsboard/server/dao/ota/OtaPackageService.java +++ b/common/dao-api/src/main/java/org/thingsboard/server/dao/ota/OtaPackageService.java @@ -18,11 +18,11 @@ package org.thingsboard.server.dao.ota; import com.google.common.util.concurrent.ListenableFuture; import org.thingsboard.server.common.data.OtaPackage; import org.thingsboard.server.common.data.OtaPackageInfo; -import org.thingsboard.server.common.data.ota.ChecksumAlgorithm; -import org.thingsboard.server.common.data.ota.OtaPackageType; import org.thingsboard.server.common.data.id.DeviceProfileId; import org.thingsboard.server.common.data.id.OtaPackageId; import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.common.data.ota.ChecksumAlgorithm; +import org.thingsboard.server.common.data.ota.OtaPackageType; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; @@ -30,7 +30,7 @@ import java.nio.ByteBuffer; public interface OtaPackageService { - OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo); + OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo, boolean isUrl); OtaPackage saveOtaPackage(OtaPackage otaPackage); diff --git a/dao/src/main/java/org/thingsboard/server/dao/ota/BaseOtaPackageService.java b/dao/src/main/java/org/thingsboard/server/dao/ota/BaseOtaPackageService.java index 881baea4f1..f30bc5d91f 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/ota/BaseOtaPackageService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/ota/BaseOtaPackageService.java @@ -77,8 +77,11 @@ public class BaseOtaPackageService implements OtaPackageService { private TbTenantProfileCache tenantProfileCache; @Override - public OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo) { + public OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo, boolean isUrl) { log.trace("Executing saveOtaPackageInfo [{}]", otaPackageInfo); + if(isUrl && (StringUtils.isEmpty(otaPackageInfo.getUrl()) || otaPackageInfo.getUrl().trim().length() == 0)) { + throw new DataValidationException("Ota package URL should be specified!"); + } otaPackageInfoValidator.validate(otaPackageInfo, OtaPackageInfo::getTenantId); try { OtaPackageId otaPackageId = otaPackageInfo.getId(); @@ -277,7 +280,9 @@ public class BaseOtaPackageService implements OtaPackageService { throw new DataValidationException("Wrong otaPackage file!"); } } else { - //TODO: validate url + if(otaPackage.getData() != null) { + throw new DataValidationException("File can't be saved if URL present!"); + } } } @@ -336,6 +341,9 @@ public class BaseOtaPackageService implements OtaPackageService { if (otaPackageOld.getDataSize() != null && !otaPackageOld.getDataSize().equals(otaPackage.getDataSize())) { throw new DataValidationException("Updating otaPackage data size is prohibited!"); } + if(otaPackageOld.getUrl() != null && !otaPackageOld.getUrl().equals(otaPackage.getUrl())) { + throw new DataValidationException("Updating otaPackage URL is prohibited!"); + } } private void validateImpl(OtaPackageInfo otaPackageInfo) { diff --git a/dao/src/test/java/org/thingsboard/server/dao/service/BaseOtaPackageServiceTest.java b/dao/src/test/java/org/thingsboard/server/dao/service/BaseOtaPackageServiceTest.java index 35063eef75..d045a84a57 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/service/BaseOtaPackageServiceTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/service/BaseOtaPackageServiceTest.java @@ -163,7 +163,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { firmware.setVersion(VERSION); firmware.setUrl(URL); firmware.setDataSize(0L); - OtaPackageInfo savedFirmware = otaPackageService.saveOtaPackageInfo(firmware); + OtaPackageInfo savedFirmware = otaPackageService.saveOtaPackageInfo(firmware, true); Assert.assertNotNull(savedFirmware); Assert.assertNotNull(savedFirmware.getId()); @@ -174,7 +174,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { Assert.assertEquals(firmware.getContentType(), savedFirmware.getContentType()); savedFirmware.setAdditionalInfo(JacksonUtil.newObjectNode()); - otaPackageService.saveOtaPackageInfo(savedFirmware); + otaPackageService.saveOtaPackageInfo(savedFirmware, true); OtaPackage foundFirmware = otaPackageService.findOtaPackageById(tenantId, savedFirmware.getId()); Assert.assertEquals(foundFirmware.getTitle(), savedFirmware.getTitle()); @@ -190,7 +190,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { firmwareInfo.setType(FIRMWARE); firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION); - OtaPackageInfo savedFirmwareInfo = otaPackageService.saveOtaPackageInfo(firmwareInfo); + OtaPackageInfo savedFirmwareInfo = otaPackageService.saveOtaPackageInfo(firmwareInfo, false); Assert.assertNotNull(savedFirmwareInfo); Assert.assertNotNull(savedFirmwareInfo.getId()); @@ -216,7 +216,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { savedFirmwareInfo = otaPackageService.findOtaPackageInfoById(tenantId, savedFirmwareInfo.getId()); savedFirmwareInfo.setAdditionalInfo(JacksonUtil.newObjectNode()); - otaPackageService.saveOtaPackageInfo(savedFirmwareInfo); + otaPackageService.saveOtaPackageInfo(savedFirmwareInfo, false); OtaPackage foundFirmware = otaPackageService.findOtaPackageById(tenantId, firmware.getId()); firmware.setAdditionalInfo(JacksonUtil.newObjectNode()); @@ -399,7 +399,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { firmwareInfo.setType(FIRMWARE); firmwareInfo.setTitle(TITLE); firmwareInfo.setVersion(VERSION); - otaPackageService.saveOtaPackageInfo(firmwareInfo); + otaPackageService.saveOtaPackageInfo(firmwareInfo, false); OtaPackageInfo newFirmwareInfo = new OtaPackageInfo(); newFirmwareInfo.setTenantId(tenantId); @@ -410,7 +410,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { thrown.expect(DataValidationException.class); thrown.expectMessage("OtaPackage with such title and version already exists!"); - otaPackageService.saveOtaPackageInfo(newFirmwareInfo); + otaPackageService.saveOtaPackageInfo(newFirmwareInfo, false); } @Test @@ -506,7 +506,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { firmware.setType(FIRMWARE); firmware.setTitle(TITLE); firmware.setVersion(VERSION); - OtaPackageInfo savedFirmware = otaPackageService.saveOtaPackageInfo(firmware); + OtaPackageInfo savedFirmware = otaPackageService.saveOtaPackageInfo(firmware, false); OtaPackageInfo foundFirmware = otaPackageService.findOtaPackageInfoById(tenantId, savedFirmware.getId()); Assert.assertNotNull(foundFirmware); @@ -543,7 +543,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { firmwareWithUrl.setUrl(URL); firmwareWithUrl.setDataSize(0L); - OtaPackageInfo savedFwWithUrl = otaPackageService.saveOtaPackageInfo(firmwareWithUrl); + OtaPackageInfo savedFwWithUrl = otaPackageService.saveOtaPackageInfo(firmwareWithUrl, true); savedFwWithUrl.setHasData(true); firmwares.add(savedFwWithUrl); @@ -588,7 +588,7 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { firmwareWithUrl.setUrl(URL); firmwareWithUrl.setDataSize(0L); - OtaPackageInfo savedFwWithUrl = otaPackageService.saveOtaPackageInfo(firmwareWithUrl); + OtaPackageInfo savedFwWithUrl = otaPackageService.saveOtaPackageInfo(firmwareWithUrl, true); savedFwWithUrl.setHasData(true); firmwares.add(savedFwWithUrl); @@ -627,6 +627,40 @@ public abstract class BaseOtaPackageServiceTest extends AbstractServiceTest { Assert.assertTrue(pageData.getData().isEmpty()); } + @Test + public void testSaveOtaPackageInfoWithBlankAndEmptyUrl() { + OtaPackageInfo firmwareInfo = new OtaPackageInfo(); + firmwareInfo.setDeviceProfileId(deviceProfileId); + firmwareInfo.setType(FIRMWARE); + firmwareInfo.setTitle(TITLE); + firmwareInfo.setVersion(VERSION); + firmwareInfo.setUrl(" "); + thrown.expect(DataValidationException.class); + thrown.expectMessage("Ota package URL should be specified!"); + otaPackageService.saveOtaPackageInfo(firmwareInfo, true); + firmwareInfo.setUrl(""); + otaPackageService.saveOtaPackageInfo(firmwareInfo, true); + } + + @Test + public void testSaveOtaPackageUrlCantBeUpdated() { + OtaPackageInfo firmwareInfo = new OtaPackageInfo(); + firmwareInfo.setDeviceProfileId(deviceProfileId); + firmwareInfo.setType(FIRMWARE); + firmwareInfo.setTitle(TITLE); + firmwareInfo.setVersion(VERSION); + firmwareInfo.setUrl(URL); + firmwareInfo.setTenantId(tenantId); + + OtaPackageInfo savedFirmwareInfo = otaPackageService.saveOtaPackageInfo(firmwareInfo, true); + + thrown.expect(DataValidationException.class); + thrown.expectMessage("Updating otaPackage URL is prohibited!"); + + savedFirmwareInfo.setUrl("https://newurl.com"); + otaPackageService.saveOtaPackageInfo(savedFirmwareInfo, true); + } + private OtaPackage createFirmware(TenantId tenantId, String version) { OtaPackage firmware = new OtaPackage(); firmware.setTenantId(tenantId); diff --git a/rest-client/src/main/java/org/thingsboard/rest/client/RestClient.java b/rest-client/src/main/java/org/thingsboard/rest/client/RestClient.java index b716935b2a..41d5d6fe76 100644 --- a/rest-client/src/main/java/org/thingsboard/rest/client/RestClient.java +++ b/rest-client/src/main/java/org/thingsboard/rest/client/RestClient.java @@ -2967,8 +2967,10 @@ public class RestClient implements ClientHttpRequestInterceptor, Closeable { ).getBody(); } - public OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo) { - return restTemplate.postForEntity(baseURL + "/api/otaPackage", otaPackageInfo, OtaPackageInfo.class).getBody(); + public OtaPackageInfo saveOtaPackageInfo(OtaPackageInfo otaPackageInfo, boolean isUrl) { + Map params = new HashMap<>(); + params.put("isUrl", Boolean.toString(isUrl)); + return restTemplate.postForEntity(baseURL + "/api/otaPackage?isUrl={isUrl}", otaPackageInfo, OtaPackageInfo.class, params).getBody(); } public OtaPackageInfo saveOtaPackageData(OtaPackageId otaPackageId, String checkSum, ChecksumAlgorithm checksumAlgorithm, MultipartFile file) throws Exception {