From a3bbc3077689203918b810678715aee83085875a Mon Sep 17 00:00:00 2001 From: ds-ext-sceronik Date: Wed, 22 Nov 2023 13:54:30 +0100 Subject: [PATCH 1/3] chore: TRACEFOSS-XXXX fix security sql injection issue by adding validation for owner paramether --- DEPENDENCIES_FRONTEND | 4 ++-- .../application/asbuilt/rest/AssetAsBuiltController.java | 3 ++- .../application/asplanned/rest/AssetAsPlannedController.java | 3 ++- .../assets/application/base/service/AssetBaseService.java | 3 ++- .../traceability/assets/domain/base/AssetRepository.java | 2 +- .../assets/domain/base/service/AbstractAssetBaseService.java | 2 +- .../asbuilt/repository/AssetAsBuiltRepositoryImpl.java | 2 +- .../asplanned/repository/AssetAsPlannedRepositoryImpl.java | 2 +- .../tractusx/traceability/common/repository/SqlUtil.java | 5 +++-- 9 files changed, 15 insertions(+), 11 deletions(-) diff --git a/DEPENDENCIES_FRONTEND b/DEPENDENCIES_FRONTEND index de42d65ebc..4024fe1071 100644 --- a/DEPENDENCIES_FRONTEND +++ b/DEPENDENCIES_FRONTEND @@ -709,7 +709,7 @@ npm/npmjs/-/node-hook/1.0.0, MIT, approved, clearlydefined npm/npmjs/-/node-releases/2.0.10, MIT, approved, #1954 npm/npmjs/-/nopt/6.0.0, ISC, approved, clearlydefined npm/npmjs/-/normalize-package-data/2.5.0, BSD-2-Clause AND Apache-2.0 AND MIT, approved, #2499 -npm/npmjs/-/normalize-package-data/3.0.3, BSD-2-Clause, approved, clearlydefined +npm/npmjs/-/normalize-package-data/3.0.3, BSD-2-Clause AND BSD-3-Clause AND BSD-2-Clause AND (BSD-2-Clause AND JSON) AND (Apache-2.0 AND MIT) AND MIT, restricted, #11614 npm/npmjs/-/normalize-package-data/5.0.0, BSD-3-Clause, approved, #5757 npm/npmjs/-/normalize-path/3.0.0, MIT, approved, clearlydefined npm/npmjs/-/normalize-range/0.1.2, MIT, approved, clearlydefined @@ -742,7 +742,7 @@ npm/npmjs/-/onetime/5.1.2, MIT, approved, clearlydefined npm/npmjs/-/onetime/6.0.0, MIT, approved, clearlydefined npm/npmjs/-/open/8.4.1, MIT, approved, #7102 npm/npmjs/-/open/8.4.2, MIT, approved, #7102 -npm/npmjs/-/opener/1.5.2, MIT OR WTFPL OR (MIT AND WTFPL), approved, clearlydefined +npm/npmjs/-/opener/1.5.2, MIT AND WTFPL AND WTFPL, approved, #11619 npm/npmjs/-/optionator/0.9.1, MIT, approved, #9208 npm/npmjs/-/ora/5.4.1, MIT, approved, clearlydefined npm/npmjs/-/os-tmpdir/1.0.2, MIT, approved, clearlydefined diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/asbuilt/rest/AssetAsBuiltController.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/asbuilt/rest/AssetAsBuiltController.java index 41a74e6a2b..8ed056f1ef 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/asbuilt/rest/AssetAsBuiltController.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/asbuilt/rest/AssetAsBuiltController.java @@ -38,6 +38,7 @@ import org.eclipse.tractusx.traceability.assets.application.base.request.SyncAssetsRequest; import org.eclipse.tractusx.traceability.assets.application.base.request.UpdateAssetRequest; import org.eclipse.tractusx.traceability.assets.application.base.service.AssetBaseService; +import org.eclipse.tractusx.traceability.assets.domain.base.model.Owner; import org.eclipse.tractusx.traceability.common.model.BaseRequestFieldMapper; import org.eclipse.tractusx.traceability.common.model.PageResult; import org.eclipse.tractusx.traceability.common.request.OwnPageable; @@ -252,7 +253,7 @@ public PageResult assets(OwnPageable pageable, SearchCrite mediaType = "application/json", schema = @Schema(implementation = ErrorResponse.class)))}) @GetMapping("distinctFilterValues") - public List distinctFilterValues(@QueryParam("fieldName") String fieldName, @QueryParam("size") Long size, @QueryParam("startWith") String startWith, @QueryParam("owner") String owner) { + public List distinctFilterValues(@QueryParam("fieldName") String fieldName, @QueryParam("size") Long size, @QueryParam("startWith") String startWith, @QueryParam("owner") Owner owner) { return assetBaseService.getDistinctFilterValues(fieldMapper.mapRequestFieldName(fieldName), startWith, size, owner); } diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/asplanned/rest/AssetAsPlannedController.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/asplanned/rest/AssetAsPlannedController.java index c6823672e0..1ece7c5c38 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/asplanned/rest/AssetAsPlannedController.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/asplanned/rest/AssetAsPlannedController.java @@ -36,6 +36,7 @@ import org.eclipse.tractusx.traceability.assets.application.base.request.SyncAssetsRequest; import org.eclipse.tractusx.traceability.assets.application.base.request.UpdateAssetRequest; import org.eclipse.tractusx.traceability.assets.application.base.service.AssetBaseService; +import org.eclipse.tractusx.traceability.assets.domain.base.model.Owner; import org.eclipse.tractusx.traceability.common.model.BaseRequestFieldMapper; import org.eclipse.tractusx.traceability.common.model.PageResult; import org.eclipse.tractusx.traceability.common.request.OwnPageable; @@ -248,7 +249,7 @@ public PageResult assets(OwnPageable pageable, SearchCri mediaType = "application/json", schema = @Schema(implementation = ErrorResponse.class)))}) @GetMapping("distinctFilterValues") - public List distinctFilterValues(@QueryParam("fieldName") String fieldName, @QueryParam("size") Long size, @QueryParam("startWith") String startWith, @QueryParam("owner") String owner) { + public List distinctFilterValues(@QueryParam("fieldName") String fieldName, @QueryParam("size") Long size, @QueryParam("startWith") String startWith, @QueryParam("owner") Owner owner) { return assetService.getDistinctFilterValues(fieldMapper.mapRequestFieldName(fieldName), startWith, size, owner); } diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/base/service/AssetBaseService.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/base/service/AssetBaseService.java index c5af2487b8..85ccd62fd1 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/base/service/AssetBaseService.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/application/base/service/AssetBaseService.java @@ -19,6 +19,7 @@ package org.eclipse.tractusx.traceability.assets.application.base.service; import org.eclipse.tractusx.traceability.assets.domain.base.model.AssetBase; +import org.eclipse.tractusx.traceability.assets.domain.base.model.Owner; import org.eclipse.tractusx.traceability.assets.domain.base.model.QualityType; import org.eclipse.tractusx.traceability.common.model.PageResult; import org.eclipse.tractusx.traceability.common.model.SearchCriteria; @@ -49,5 +50,5 @@ public interface AssetBaseService { AssetBase updateQualityType(String assetId, QualityType qualityType); - List getDistinctFilterValues(String fieldName, String startWith, Long size, String owner); + List getDistinctFilterValues(String fieldName, String startWith, Long size, Owner owner); } diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/domain/base/AssetRepository.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/domain/base/AssetRepository.java index 6501fe9491..6dd399f679 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/domain/base/AssetRepository.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/domain/base/AssetRepository.java @@ -50,5 +50,5 @@ public interface AssetRepository { long countAssetsByOwner(Owner owner); - List getFieldValues(String fieldName, String startWith, Long resultLimit, String owner); + List getFieldValues(String fieldName, String startWith, Long resultLimit, Owner owner); } diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/domain/base/service/AbstractAssetBaseService.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/domain/base/service/AbstractAssetBaseService.java index 1d385dcba1..c3e50695af 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/domain/base/service/AbstractAssetBaseService.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/domain/base/service/AbstractAssetBaseService.java @@ -154,7 +154,7 @@ public Map getAssetsCountryMap() { } @Override - public List getDistinctFilterValues(String fieldName, String startWith, Long size, String owner) { + public List getDistinctFilterValues(String fieldName, String startWith, Long size, Owner owner) { if (isSupportedEnumType(fieldName)) { return getAssetEnumFieldValues(fieldName); } diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/infrastructure/asbuilt/repository/AssetAsBuiltRepositoryImpl.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/infrastructure/asbuilt/repository/AssetAsBuiltRepositoryImpl.java index 519cd70cfb..2c523cad5b 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/infrastructure/asbuilt/repository/AssetAsBuiltRepositoryImpl.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/infrastructure/asbuilt/repository/AssetAsBuiltRepositoryImpl.java @@ -88,7 +88,7 @@ public PageResult getAssets(Pageable pageable, SearchCriteria searchC } @Override - public List getFieldValues(String fieldName, String startWith, Long resultLimit, String owner) { + public List getFieldValues(String fieldName, String startWith, Long resultLimit, Owner owner) { String databaseFieldName = toDatabaseName(fieldName); String getFieldValuesQuery = "SELECT DISTINCT " + databaseFieldName + " FROM assets_as_built" + combineWhereClause(constructLikeWildcardQuery(databaseFieldName, startWith), constructAndOwnerWildcardQuery(owner)) + " ORDER BY " + databaseFieldName + " ASC LIMIT :resultLimit"; return entityManager.createNativeQuery(getFieldValuesQuery, String.class) diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/infrastructure/asplanned/repository/AssetAsPlannedRepositoryImpl.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/infrastructure/asplanned/repository/AssetAsPlannedRepositoryImpl.java index d5b4074ef8..925c6025aa 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/infrastructure/asplanned/repository/AssetAsPlannedRepositoryImpl.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/assets/infrastructure/asplanned/repository/AssetAsPlannedRepositoryImpl.java @@ -123,7 +123,7 @@ public long countAssetsByOwner(Owner owner) { } @Override - public List getFieldValues(String fieldName, String startWith, Long resultLimit, String owner) { + public List getFieldValues(String fieldName, String startWith, Long resultLimit, Owner owner) { String databaseFieldName = toDatabaseName(fieldName); String getFieldValuesQuery = "SELECT DISTINCT " + databaseFieldName + " FROM assets_as_planned" + combineWhereClause(constructLikeWildcardQuery(databaseFieldName, startWith), constructAndOwnerWildcardQuery(owner)) + " ORDER BY " + databaseFieldName + " ASC LIMIT :resultLimit"; diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/common/repository/SqlUtil.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/common/repository/SqlUtil.java index 79780e9ed6..4376aa05a1 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/common/repository/SqlUtil.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/common/repository/SqlUtil.java @@ -21,6 +21,7 @@ import lombok.experimental.UtilityClass; import org.apache.commons.lang3.StringUtils; +import org.eclipse.tractusx.traceability.assets.domain.base.model.Owner; import java.util.Arrays; import java.util.List; @@ -52,10 +53,10 @@ public static String constructLikeWildcardQuery(String databaseFieldName, String return " ( " + databaseFieldName + " LIKE '" + startsWith + "%')"; } - public static String constructAndOwnerWildcardQuery(String owner) { + public static String constructAndOwnerWildcardQuery(Owner owner) { if (Objects.isNull(owner)) { return ""; } - return " owner='" + owner + "' "; + return " owner='" + owner.name() + "' "; } } From 063a2b3edb599d0a67c058c0163948c98204ec17 Mon Sep 17 00:00:00 2001 From: ds-ext-sceronik Date: Wed, 22 Nov 2023 14:00:02 +0100 Subject: [PATCH 2/3] chore: TRACEFOSS-XXXX rollback DEPENDENCIES_FRONTEND --- DEPENDENCIES_FRONTEND | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DEPENDENCIES_FRONTEND b/DEPENDENCIES_FRONTEND index 4024fe1071..de42d65ebc 100644 --- a/DEPENDENCIES_FRONTEND +++ b/DEPENDENCIES_FRONTEND @@ -709,7 +709,7 @@ npm/npmjs/-/node-hook/1.0.0, MIT, approved, clearlydefined npm/npmjs/-/node-releases/2.0.10, MIT, approved, #1954 npm/npmjs/-/nopt/6.0.0, ISC, approved, clearlydefined npm/npmjs/-/normalize-package-data/2.5.0, BSD-2-Clause AND Apache-2.0 AND MIT, approved, #2499 -npm/npmjs/-/normalize-package-data/3.0.3, BSD-2-Clause AND BSD-3-Clause AND BSD-2-Clause AND (BSD-2-Clause AND JSON) AND (Apache-2.0 AND MIT) AND MIT, restricted, #11614 +npm/npmjs/-/normalize-package-data/3.0.3, BSD-2-Clause, approved, clearlydefined npm/npmjs/-/normalize-package-data/5.0.0, BSD-3-Clause, approved, #5757 npm/npmjs/-/normalize-path/3.0.0, MIT, approved, clearlydefined npm/npmjs/-/normalize-range/0.1.2, MIT, approved, clearlydefined @@ -742,7 +742,7 @@ npm/npmjs/-/onetime/5.1.2, MIT, approved, clearlydefined npm/npmjs/-/onetime/6.0.0, MIT, approved, clearlydefined npm/npmjs/-/open/8.4.1, MIT, approved, #7102 npm/npmjs/-/open/8.4.2, MIT, approved, #7102 -npm/npmjs/-/opener/1.5.2, MIT AND WTFPL AND WTFPL, approved, #11619 +npm/npmjs/-/opener/1.5.2, MIT OR WTFPL OR (MIT AND WTFPL), approved, clearlydefined npm/npmjs/-/optionator/0.9.1, MIT, approved, #9208 npm/npmjs/-/ora/5.4.1, MIT, approved, clearlydefined npm/npmjs/-/os-tmpdir/1.0.2, MIT, approved, clearlydefined From cb5afe019b66131f6a088d157e66a74ff4a894bf Mon Sep 17 00:00:00 2001 From: ds-ext-sceronik Date: Wed, 22 Nov 2023 14:39:08 +0100 Subject: [PATCH 3/3] chore: TRACEFOSS-XXXX tests added and changelog --- CHANGELOG.md | 1 + .../common/config/ErrorHandlingConfig.java | 9 +++++++ .../AssetAsBuiltControllerFilterValuesIT.java | 25 +++++++++++++++++++ ...ssetAsPlannedControllerFilterValuesIT.java | 25 +++++++++++++++++++ 4 files changed, 60 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10ee044baf..9e2a874c90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Changed - Fixed helm repository path for backend & frontend (wrong prefix) +- Autocomplete endpoints changed owner String type param to Owner for input validation and sql injection prevention ### Removed - apk upgrade in docker image built as requested by TRG 4.02 diff --git a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/common/config/ErrorHandlingConfig.java b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/common/config/ErrorHandlingConfig.java index 0d4e77fcfa..8720cd5222 100644 --- a/tx-backend/src/main/java/org/eclipse/tractusx/traceability/common/config/ErrorHandlingConfig.java +++ b/tx-backend/src/main/java/org/eclipse/tractusx/traceability/common/config/ErrorHandlingConfig.java @@ -57,6 +57,7 @@ import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; import java.io.IOException; import java.util.NoSuchElementException; @@ -260,4 +261,12 @@ ResponseEntity handleSubmodelNotFoundException(SubmodelNotFoundEx return ResponseEntity.status(HttpStatus.NOT_FOUND) .body(new ErrorResponse(errorMessage)); } + + @ExceptionHandler(MethodArgumentTypeMismatchException.class) + public ResponseEntity handleMethodArgumentTypeMismatchException(final MethodArgumentTypeMismatchException exception) { + log.error("MethodArgumentTypeMismatchException exception", exception); + + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body(new ErrorResponse(exception.getMessage())); + } } diff --git a/tx-backend/src/test/java/org/eclipse/tractusx/traceability/integration/assets/AssetAsBuiltControllerFilterValuesIT.java b/tx-backend/src/test/java/org/eclipse/tractusx/traceability/integration/assets/AssetAsBuiltControllerFilterValuesIT.java index d2076dd2bf..dad62f8dd2 100644 --- a/tx-backend/src/test/java/org/eclipse/tractusx/traceability/integration/assets/AssetAsBuiltControllerFilterValuesIT.java +++ b/tx-backend/src/test/java/org/eclipse/tractusx/traceability/integration/assets/AssetAsBuiltControllerFilterValuesIT.java @@ -119,6 +119,31 @@ void givenNotEnumTypeFieldNameAndSizeAndOwnerOwn_whenCallDistinctFilterValues_th .body("size()", is(1)); } + @Test + void givenNotExistentOwnerEnumValue_whenCallDistinctFilterValues_thenProperResponse() throws JoseException { + // given + assetsSupport.defaultAssetsStored(); + String fieldName = "id"; + String resultLimit = "100"; + String owner = "NON_EXISTENT_ENUM_VALUE"; + + // then + given() + .header(oAuth2Support.jwtAuthorization(ADMIN)) + .contentType(ContentType.JSON) + .log().all() + .when() + .param("fieldName", fieldName) + .param("size", resultLimit) + .param("owner", owner) + .get("/api/assets/as-built/distinctFilterValues") + .then() + .log().all() + .statusCode(400) + .assertThat() + .body("size()", is(1)); + } + @Test void givenNotEnumTypeFieldNameAndSizeAndOwnerSupplier_whenCallDistinctFilterValues_thenProperResponse() throws JoseException { // given diff --git a/tx-backend/src/test/java/org/eclipse/tractusx/traceability/integration/assets/AssetAsPlannedControllerFilterValuesIT.java b/tx-backend/src/test/java/org/eclipse/tractusx/traceability/integration/assets/AssetAsPlannedControllerFilterValuesIT.java index 2a0a6d41a1..3ccefaf9d8 100644 --- a/tx-backend/src/test/java/org/eclipse/tractusx/traceability/integration/assets/AssetAsPlannedControllerFilterValuesIT.java +++ b/tx-backend/src/test/java/org/eclipse/tractusx/traceability/integration/assets/AssetAsPlannedControllerFilterValuesIT.java @@ -95,6 +95,31 @@ void givenNotEnumTypeFieldNameAndSizeAndOwnerOwn_whenCallDistinctFilterValues_th .body("size()", is(1)); } + @Test + void givenWrongOwnerEnum_whenCallDistinctFilterValues_thenProperResponse() throws JoseException { + // given + assetsSupport.defaultAssetsAsPlannedStored(); + String fieldName = "id"; + String resultLimit = "100"; + String owner = "nonExistentEnumValue"; + + // then + given() + .header(oAuth2Support.jwtAuthorization(ADMIN)) + .contentType(ContentType.JSON) + .log().all() + .when() + .param("fieldName", fieldName) + .param("size", resultLimit) + .param("owner", owner) + .get("/api/assets/as-planned/distinctFilterValues") + .then() + .log().all() + .statusCode(400) + .assertThat() + .body("size()", is(1)); + } + @Test void givenNotEnumTypeFieldNameAndSizeAndOwnerSupplier_whenCallDistinctFilterValues_thenProperResponse() throws JoseException { // given