Skip to content

Commit

Permalink
Merge pull request #360 from catenax-ng/main
Browse files Browse the repository at this point in the history
chore: TRACEFOSS-2882 SQL injection issue
  • Loading branch information
ds-mwesener authored Nov 22, 2023
2 parents 86c9445 + f80d41c commit e04208c
Show file tree
Hide file tree
Showing 12 changed files with 73 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
- Fixed several bugs in local filtering of the parts table

### Removed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -252,7 +253,7 @@ public PageResult<AssetAsBuiltResponse> assets(OwnPageable pageable, SearchCrite
mediaType = "application/json",
schema = @Schema(implementation = ErrorResponse.class)))})
@GetMapping("distinctFilterValues")
public List<String> distinctFilterValues(@QueryParam("fieldName") String fieldName, @QueryParam("size") Long size, @QueryParam("startWith") String startWith, @QueryParam("owner") String owner) {
public List<String> 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -248,7 +249,7 @@ public PageResult<AssetAsPlannedResponse> assets(OwnPageable pageable, SearchCri
mediaType = "application/json",
schema = @Schema(implementation = ErrorResponse.class)))})
@GetMapping("distinctFilterValues")
public List<String> distinctFilterValues(@QueryParam("fieldName") String fieldName, @QueryParam("size") Long size, @QueryParam("startWith") String startWith, @QueryParam("owner") String owner) {
public List<String> 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -49,5 +50,5 @@ public interface AssetBaseService {

AssetBase updateQualityType(String assetId, QualityType qualityType);

List<String> getDistinctFilterValues(String fieldName, String startWith, Long size, String owner);
List<String> getDistinctFilterValues(String fieldName, String startWith, Long size, Owner owner);
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,5 @@ public interface AssetRepository {

long countAssetsByOwner(Owner owner);

List<String> getFieldValues(String fieldName, String startWith, Long resultLimit, String owner);
List<String> getFieldValues(String fieldName, String startWith, Long resultLimit, Owner owner);
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public Map<String, Long> getAssetsCountryMap() {
}

@Override
public List<String> getDistinctFilterValues(String fieldName, String startWith, Long size, String owner) {
public List<String> getDistinctFilterValues(String fieldName, String startWith, Long size, Owner owner) {
if (isSupportedEnumType(fieldName)) {
return getAssetEnumFieldValues(fieldName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public PageResult<AssetBase> getAssets(Pageable pageable, SearchCriteria searchC
}

@Override
public List<String> getFieldValues(String fieldName, String startWith, Long resultLimit, String owner) {
public List<String> 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public long countAssetsByOwner(Owner owner) {
}

@Override
public List<String> getFieldValues(String fieldName, String startWith, Long resultLimit, String owner) {
public List<String> 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";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -260,4 +261,12 @@ ResponseEntity<ErrorResponse> handleSubmodelNotFoundException(SubmodelNotFoundEx
return ResponseEntity.status(HttpStatus.NOT_FOUND)
.body(new ErrorResponse(errorMessage));
}

@ExceptionHandler(MethodArgumentTypeMismatchException.class)
public ResponseEntity<ErrorResponse> handleMethodArgumentTypeMismatchException(final MethodArgumentTypeMismatchException exception) {
log.error("MethodArgumentTypeMismatchException exception", exception);

return ResponseEntity.status(HttpStatus.BAD_REQUEST)
.body(new ErrorResponse(exception.getMessage()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() + "' ";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e04208c

Please sign in to comment.