From c4bc5fc2e020c90b47a606010be34aeca70ac95b Mon Sep 17 00:00:00 2001 From: Bichitra Kumar Sahoo <32828151+bichitra95@users.noreply.github.com> Date: Thu, 23 May 2024 16:00:48 +0530 Subject: [PATCH 01/16] Support dataContractSpec instead of dataContractJson --- .../apache/atlas/repository/Constants.java | 2 +- .../contract/ContractPreProcessor.java | 6 +- .../preprocessor/contract/DataContract.java | 59 ++++++++++++++++++- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/common/src/main/java/org/apache/atlas/repository/Constants.java b/common/src/main/java/org/apache/atlas/repository/Constants.java index 95a6e0c007..0cc184dfa4 100644 --- a/common/src/main/java/org/apache/atlas/repository/Constants.java +++ b/common/src/main/java/org/apache/atlas/repository/Constants.java @@ -426,7 +426,7 @@ public enum SupportedFileExtensions { XLSX, XLS, CSV } public static final String ATTR_ASSET_STARRED_BY = "assetStarredBy"; public static final String ATTR_ASSET_STARRED_AT = "assetStarredAt"; public static final String ATTR_CERTIFICATE_STATUS = "certificateStatus"; - public static final String ATTR_CONTRACT = "dataContractJson"; + public static final String ATTR_CONTRACT = "dataContractSpec"; public static final String STRUCT_STARRED_DETAILS = "StarredDetails"; public static final String KEYCLOAK_ROLE_ADMIN = "$admin"; diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java index bc9ff76cbd..0fa9dd4f21 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java @@ -19,6 +19,7 @@ import org.apache.atlas.type.AtlasEntityType; import org.apache.atlas.type.AtlasTypeRegistry; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -83,7 +84,8 @@ private void processUpdateContract(AtlasEntity entity, EntityMutationContext con AtlasEntity existingContractEntity = entityRetriever.toAtlasEntity(vertex); // No update to relationships allowed for the existing contract version resetAllRelationshipAttributes(entity); - if (!isEqualContract(contractString, (String) existingContractEntity.getAttribute(ATTR_CONTRACT))) { + if (!StringUtils.isEmpty(contractString) && + !isEqualContract(contractString, (String) existingContractEntity.getAttribute(ATTR_CONTRACT))) { // Update the same asset(entity) throw new AtlasBaseException(OPERATION_NOT_SUPPORTED, "Can't update a specific version of contract"); } @@ -128,7 +130,7 @@ private void processCreateContract(AtlasEntity entity, EntityMutationContext con // No changes in the contract, Not creating new version removeCreatingVertex(context, entity); return; - } else if (isEqualContract(contractString, (String) currentVersionEntity.getAttribute(ATTR_CONTRACT))) { + } else if (contract.equals(DataContract.deserialize((String) currentVersionEntity.getAttribute(ATTR_CONTRACT)))) { resetAllRelationshipAttributes(entity); // No change in contract, metadata changed updateExistingVersion(context, entity, currentVersionEntity); diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/DataContract.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/DataContract.java index dc3cdb466b..a14bc3b00f 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/DataContract.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/DataContract.java @@ -9,8 +9,10 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import org.apache.atlas.AtlasErrorCode; import org.apache.atlas.exception.AtlasBaseException; +import org.apache.atlas.model.instance.AtlasEntity; import org.apache.commons.lang.StringUtils; import javax.validation.*; @@ -27,7 +29,7 @@ public class DataContract { private static final String KIND_VALUE = "DataContract"; private static final Pattern versionPattern = Pattern.compile("^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$"); - private static final ObjectMapper objectMapper = new ObjectMapper(); + private static final ObjectMapper objectMapper = new ObjectMapper(new YAMLFactory()); static { objectMapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS); } @@ -184,6 +186,20 @@ public Map getUnknownFields() { return unknownFields; } + @Override + public boolean equals(Object o) { + if (this == o) { return true; } + if (o == null || getClass() != o.getClass()) { return false; } + BusinessTag that = (BusinessTag) o; + return Objects.equals(name, that.name) && + Objects.equals(unknownFields, that.unknownFields); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), name, unknownFields); + } + } @JsonIgnoreProperties(ignoreUnknown = true) @@ -206,6 +222,22 @@ public void setUnknownFields(String key, Object value) { public Map getUnknownFields() { return unknownFields; } + + @Override + public boolean equals(Object o) { + if (this == o) { return true; } + if (o == null || getClass() != o.getClass()) { return false; } + Column that = (Column) o; + return Objects.equals(name, that.name) && + Objects.equals(description, that.description) && + Objects.equals(data_type, that.data_type) && + Objects.equals(unknownFields, that.unknownFields); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), name, description, data_type, unknownFields); + } } public static DataContract deserialize(String contractString) throws AtlasBaseException { @@ -248,5 +280,30 @@ public static String serialize(DataContract contract) throws AtlasBaseException } } + @Override + public boolean equals(Object o) { + if (this == o) { return true; } + if (o == null || getClass() != o.getClass()) { return false; } + + DataContract that = (DataContract) o; + return Objects.equals(kind, that.kind) && + Objects.equals(status, that.status) && + Objects.equals(templateVersion, that.templateVersion) && + Objects.equals(data_source, that.data_source) && + Objects.equals(dataset, that.dataset) && + Objects.equals(type, that.type) && + Objects.equals(description, that.description) && + Objects.equals(owners, that.owners) && + Objects.equals(tags, that.tags) && + Objects.equals(certificate, that.certificate) && + Objects.equals(columns, that.columns) && + Objects.equals(unknownFields, that.unknownFields); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), kind, status, templateVersion, data_source, dataset, type, description, owners, + tags, certificate, columns, unknownFields); + } } From 3a63e529d6945dfa17c64b8ddfb4d3d54e75c995 Mon Sep 17 00:00:00 2001 From: Bichitra Kumar Sahoo <32828151+bichitra95@users.noreply.github.com> Date: Thu, 23 May 2024 17:48:18 +0530 Subject: [PATCH 02/16] Allow dataContractSpec to update --- .../v2/preprocessor/contract/ContractPreProcessor.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java index 0fa9dd4f21..1df66f40d4 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java @@ -84,11 +84,11 @@ private void processUpdateContract(AtlasEntity entity, EntityMutationContext con AtlasEntity existingContractEntity = entityRetriever.toAtlasEntity(vertex); // No update to relationships allowed for the existing contract version resetAllRelationshipAttributes(entity); - if (!StringUtils.isEmpty(contractString) && - !isEqualContract(contractString, (String) existingContractEntity.getAttribute(ATTR_CONTRACT))) { - // Update the same asset(entity) - throw new AtlasBaseException(OPERATION_NOT_SUPPORTED, "Can't update a specific version of contract"); - } +// if (!StringUtils.isEmpty(contractString) && +// !isEqualContract(contractString, (String) existingContractEntity.getAttribute(ATTR_CONTRACT))) { +// // Update the same asset(entity) +// throw new AtlasBaseException(OPERATION_NOT_SUPPORTED, "Can't update a specific version of contract"); +// } } private void processCreateContract(AtlasEntity entity, EntityMutationContext context) throws AtlasBaseException { /* From 7574970cd9314f8e1c7920bc366ca4cbd2d00c68 Mon Sep 17 00:00:00 2001 From: Bichitra Kumar Sahoo <32828151+bichitra95@users.noreply.github.com> Date: Mon, 27 May 2024 11:57:56 +0530 Subject: [PATCH 03/16] Update dataformat.YAML dependency version --- repository/pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/repository/pom.xml b/repository/pom.xml index a2a1a4198f..bbe15338f8 100755 --- a/repository/pom.xml +++ b/repository/pom.xml @@ -327,6 +327,11 @@ hibernate-validator 4.3.2.Final + + com.fasterxml.jackson.dataformat + jackson-dataformat-yaml + 2.12.7 + From 726fefe0241ee449d7b94e6e526bc839a4ab9202 Mon Sep 17 00:00:00 2001 From: hr2904 Date: Tue, 4 Jun 2024 18:08:11 +0530 Subject: [PATCH 04/16] Added a filepath sanitiser. --- .../repository/impexp/ImportService.java | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java b/repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java index 344e44ca06..0c00dddd0e 100644 --- a/repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java +++ b/repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java @@ -47,6 +47,8 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import static org.apache.atlas.model.impexp.AtlasImportRequest.TRANSFORMERS_KEY; @@ -191,7 +193,9 @@ public AtlasImportResult run(AtlasImportRequest request, String userName, String if (StringUtils.isBlank(fileName)) { throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "FILENAME parameter not found"); } - + if(!validateFilePath(fileName)){ + throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "FILENAME IS INVALID"); + } AtlasImportResult result = null; try { LOG.info("==> import(user={}, from={}, fileName={})", userName, requestingIP, fileName); @@ -296,4 +300,32 @@ boolean checkHiveTableIncrementalSkipLineage(AtlasImportRequest importRequest, A private boolean isMigrationMode(AtlasImportRequest request) { return request.getOptions().containsKey(AtlasImportRequest.OPTION_KEY_MIGRATION); } + + private boolean validateFilePath(String filePath) { + String allowedDirectory = "/var/app/allowed/"; + + try { + Path normalizedPath = Paths.get(filePath).normalize(); + + if (filePath.contains("..") || filePath.contains("./") || filePath.contains(".\\")) { + LOG.error("Invalid file path: directory traversal attempt detected."); + return false; + } + + if (!normalizedPath.isAbsolute()) { + LOG.error("Invalid file path: path must be absolute."); + return false; + } + + if (!normalizedPath.startsWith(Paths.get(allowedDirectory))) { + LOG.error("Invalid file path: access outside allowed directory."); + return false; + } + + return true; + } catch (Exception e) { + return false; + } + } + } From 1ebdae75233c0dcf7b31a8f1a9f311033c045877 Mon Sep 17 00:00:00 2001 From: hr2904 Date: Wed, 5 Jun 2024 12:44:54 +0530 Subject: [PATCH 05/16] Added validator for sanitation of filepath inputs. --- .../repository/impexp/ImportService.java | 30 ++----------------- .../migration/DataMigrationStatusService.java | 16 ++++++++++ .../atlas/repository/util/FilterUtil.java | 26 ++++++++++++++++ 3 files changed, 44 insertions(+), 28 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java b/repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java index 0c00dddd0e..ff4231d5de 100644 --- a/repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java +++ b/repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java @@ -31,6 +31,7 @@ import org.apache.atlas.model.typedef.AtlasTypesDef; import org.apache.atlas.repository.store.graph.BulkImporter; import org.apache.atlas.repository.store.graph.v2.EntityImportStream; +import org.apache.atlas.repository.util.FilterUtil; import org.apache.atlas.store.AtlasTypeDefStore; import org.apache.atlas.type.AtlasType; import org.apache.atlas.type.AtlasTypeRegistry; @@ -193,7 +194,7 @@ public AtlasImportResult run(AtlasImportRequest request, String userName, String if (StringUtils.isBlank(fileName)) { throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "FILENAME parameter not found"); } - if(!validateFilePath(fileName)){ + if(!FilterUtil.validateFilePath(fileName)){ throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "FILENAME IS INVALID"); } AtlasImportResult result = null; @@ -301,31 +302,4 @@ private boolean isMigrationMode(AtlasImportRequest request) { return request.getOptions().containsKey(AtlasImportRequest.OPTION_KEY_MIGRATION); } - private boolean validateFilePath(String filePath) { - String allowedDirectory = "/var/app/allowed/"; - - try { - Path normalizedPath = Paths.get(filePath).normalize(); - - if (filePath.contains("..") || filePath.contains("./") || filePath.contains(".\\")) { - LOG.error("Invalid file path: directory traversal attempt detected."); - return false; - } - - if (!normalizedPath.isAbsolute()) { - LOG.error("Invalid file path: path must be absolute."); - return false; - } - - if (!normalizedPath.startsWith(Paths.get(allowedDirectory))) { - LOG.error("Invalid file path: access outside allowed directory."); - return false; - } - - return true; - } catch (Exception e) { - return false; - } - } - } diff --git a/repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java b/repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java index 3d357ddcfa..5efa675f0c 100644 --- a/repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java +++ b/repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java @@ -18,22 +18,27 @@ package org.apache.atlas.repository.migration; +import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.model.migration.MigrationImportStatus; import org.apache.atlas.repository.Constants; import org.apache.atlas.repository.graph.AtlasGraphProvider; import org.apache.atlas.repository.graphdb.AtlasGraph; import org.apache.atlas.repository.graphdb.AtlasVertex; import org.apache.atlas.repository.store.graph.v2.AtlasGraphUtilsV2; +import org.apache.atlas.repository.util.FilterUtil; import org.apache.commons.codec.digest.DigestUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.FileInputStream; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Date; import static org.apache.atlas.repository.store.graph.v2.AtlasGraphUtilsV2.getEncodedProperty; import static org.apache.atlas.repository.store.graph.v2.AtlasGraphUtilsV2.setEncodedProperty; +import static org.apache.atlas.repository.util.FilterUtil.validateFilePath; import static org.apache.atlas.type.AtlasStructType.AtlasAttribute.encodePropertyKey; import static org.apache.atlas.type.Constants.INTERNAL_PROPERTY_KEY_PREFIX; @@ -54,9 +59,14 @@ public DataMigrationStatusService(AtlasGraph graph) { public void init(String fileToImport) { try { + if(!validateFilePath(fileToImport)){ + throw new AtlasBaseException("File Path is invalid"); + } this.status = new MigrationImportStatus(fileToImport, DigestUtils.md5Hex(new FileInputStream(fileToImport))); } catch (IOException e) { LOG.error("Not able to create Migration status", e); + } catch (AtlasBaseException e) { + LOG.error("File Path is invalid"); } if (!this.migrationStatusVertexManagement.exists(status.getFileHash())) { @@ -66,12 +76,18 @@ public void init(String fileToImport) { getCreate(fileToImport); } + public MigrationImportStatus getCreate(String fileName) { MigrationImportStatus create = null; try { + if(!validateFilePath(fileName)){ + throw new AtlasBaseException("File Path is invalid"); + } create = getCreate(new MigrationImportStatus(fileName, DigestUtils.md5Hex(new FileInputStream(fileName)))); } catch (IOException e) { LOG.error("Exception occurred while creating migration import", e); + } catch (AtlasBaseException e) { + LOG.error("File Path is invalid"); } return create; diff --git a/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java b/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java index 01c6ee0990..a5ec2f133d 100644 --- a/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java +++ b/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java @@ -29,6 +29,8 @@ import org.apache.commons.collections.functors.NotPredicate; import org.apache.commons.lang.StringUtils; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -174,4 +176,28 @@ public static void addParamsToHideInternalType(SearchFilter searchFilter) { searchFilter.setParam(SearchFilter.PARAM_NOT_NAME, Constants.TYPE_NAME_INTERNAL); searchFilter.setParam(SearchFilter.PARAM_NOT_SUPERTYPE, Constants.TYPE_NAME_INTERNAL); } + + public static boolean validateFilePath(String fileToImport) { + String allowedDirectory = "/var/app/allowed/"; + + try { + Path normalizedPath = Paths.get(fileToImport).normalize(); + + if (fileToImport.contains("..") || fileToImport.contains("./") || fileToImport.contains(".\\")) { + return false; + } + + if (!normalizedPath.isAbsolute()) { + return false; + } + + if (!normalizedPath.startsWith(Paths.get(allowedDirectory))) { + return false; + } + + return true; + } catch (Exception e) { + return false; + } + } } From 1f35c21644c8534271e777724d159406034a5a7b Mon Sep 17 00:00:00 2001 From: hr2904 Date: Mon, 10 Jun 2024 15:59:21 +0530 Subject: [PATCH 06/16] Added a sanitization method to prevent HTTP Response Splitting. --- .../atlas/web/filters/ActiveServerFilter.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java index 2721efa6f2..5c2f9180c2 100644 --- a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java +++ b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java @@ -42,6 +42,8 @@ import javax.ws.rs.HttpMethod; import javax.ws.rs.core.HttpHeaders; import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; import java.util.HashMap; /** @@ -234,11 +236,19 @@ private void handleRedirect(HttpServletRequest servletRequest, HttpServletRespon LOG.info("Not active. Redirecting to {}", redirectLocation); // A POST/PUT/DELETE require special handling by sending HTTP 307 instead of the regular 301/302. // Reference: http://stackoverflow.com/questions/2068418/whats-the-difference-between-a-302-and-a-307-redirect + String sanitizedLocation = sanitizeRedirectLocation(redirectLocation); if (isUnsafeHttpMethod(servletRequest)) { - httpServletResponse.setHeader(HttpHeaders.LOCATION, redirectLocation); + httpServletResponse.setHeader(HttpHeaders.LOCATION, sanitizedLocation); httpServletResponse.setStatus(HttpServletResponse.SC_TEMPORARY_REDIRECT); } else { - httpServletResponse.sendRedirect(redirectLocation); + httpServletResponse.sendRedirect(sanitizedLocation); + } + } + private String sanitizeRedirectLocation(String redirectLocation) { + try { + return URLEncoder.encode(redirectLocation, "UTF-8"); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException("Encoding not supported", e); } } From 38e49b6f3b5e5b23e689a15d4066e18962f372de Mon Sep 17 00:00:00 2001 From: hr2904 Date: Mon, 10 Jun 2024 16:24:45 +0530 Subject: [PATCH 07/16] Refined the method for sanitizing. --- .../org/apache/atlas/web/filters/ActiveServerFilter.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java index 5c2f9180c2..613392e26b 100644 --- a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java +++ b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java @@ -244,11 +244,13 @@ private void handleRedirect(HttpServletRequest servletRequest, HttpServletRespon httpServletResponse.sendRedirect(sanitizedLocation); } } - private String sanitizeRedirectLocation(String redirectLocation) { + private static String sanitizeRedirectLocation(String redirectLocation) { + if (redirectLocation == null) return null; try { - return URLEncoder.encode(redirectLocation, "UTF-8"); + String encodedUrl = URLEncoder.encode(redirectLocation, "UTF-8"); + return encodedUrl.replaceAll("\\r", "").replaceAll("\\n", ""); } catch (UnsupportedEncodingException e) { - throw new RuntimeException("Encoding not supported", e); + throw new RuntimeException("UTF-8 encoding not supported", e); } } From 221ad2670720c20c17afb75fb15debd0ca7fc443 Mon Sep 17 00:00:00 2001 From: Bichitra Kumar Sahoo <32828151+bichitra95@users.noreply.github.com> Date: Tue, 11 Jun 2024 12:01:03 +0530 Subject: [PATCH 08/16] Support for both dataContractJson and dataContractSpec attribute for DataContract --- .../apache/atlas/repository/Constants.java | 1 + .../contract/ContractPreProcessor.java | 51 ++++++++----------- .../preprocessor/contract/DataContract.java | 26 +++++++--- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/common/src/main/java/org/apache/atlas/repository/Constants.java b/common/src/main/java/org/apache/atlas/repository/Constants.java index 0cc184dfa4..61d5135e3b 100644 --- a/common/src/main/java/org/apache/atlas/repository/Constants.java +++ b/common/src/main/java/org/apache/atlas/repository/Constants.java @@ -427,6 +427,7 @@ public enum SupportedFileExtensions { XLSX, XLS, CSV } public static final String ATTR_ASSET_STARRED_AT = "assetStarredAt"; public static final String ATTR_CERTIFICATE_STATUS = "certificateStatus"; public static final String ATTR_CONTRACT = "dataContractSpec"; + public static final String ATTR_CONTRACT_JSON = "dataContractJson"; public static final String STRUCT_STARRED_DETAILS = "StarredDetails"; public static final String KEYCLOAK_ROLE_ADMIN = "$admin"; diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java index 1df66f40d4..869f2b5c6a 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java @@ -1,9 +1,5 @@ package org.apache.atlas.repository.store.graph.v2.preprocessor.contract; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.node.ObjectNode; - import org.apache.atlas.RequestContext; import org.apache.atlas.discovery.EntityDiscoveryService; import org.apache.atlas.exception.AtlasBaseException; @@ -23,8 +19,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.fasterxml.jackson.databind.ObjectMapper; - import java.util.*; import static org.apache.atlas.AtlasErrorCode.*; @@ -44,6 +38,7 @@ public class ContractPreProcessor extends AbstractContractPreProcessor { private static final Set contractAttributes = new HashSet<>(); static { contractAttributes.add(ATTR_CONTRACT); + contractAttributes.add(ATTR_CONTRACT_JSON); contractAttributes.add(ATTR_CERTIFICATE_STATUS); contractAttributes.add(ATTR_CONTRACT_VERSION); } @@ -79,16 +74,17 @@ public void processAttributes(AtlasStruct entityStruct, EntityMutationContext co } private void processUpdateContract(AtlasEntity entity, EntityMutationContext context) throws AtlasBaseException { - String contractString = (String) entity.getAttribute(ATTR_CONTRACT); + String contractString = getContractString(entity); AtlasVertex vertex = context.getVertex(entity.getGuid()); AtlasEntity existingContractEntity = entityRetriever.toAtlasEntity(vertex); // No update to relationships allowed for the existing contract version resetAllRelationshipAttributes(entity); -// if (!StringUtils.isEmpty(contractString) && -// !isEqualContract(contractString, (String) existingContractEntity.getAttribute(ATTR_CONTRACT))) { -// // Update the same asset(entity) -// throw new AtlasBaseException(OPERATION_NOT_SUPPORTED, "Can't update a specific version of contract"); -// } + DataContract contract = DataContract.deserialize(contractString); + String existingContractString = getContractString(existingContractEntity); + if (!StringUtils.isEmpty(contractString) && !contract.equals(DataContract.deserialize(existingContractString))) { + // Update the same asset(entity) + throw new AtlasBaseException(OPERATION_NOT_SUPPORTED, "Can't update a specific version of contract"); + } } private void processCreateContract(AtlasEntity entity, EntityMutationContext context) throws AtlasBaseException { /* @@ -107,7 +103,7 @@ private void processCreateContract(AtlasEntity entity, EntityMutationContext con String contractQName = (String) entity.getAttribute(QUALIFIED_NAME); validateAttribute(!contractQName.endsWith(String.format("/%s", CONTRACT_QUALIFIED_NAME_SUFFIX)), "Invalid qualifiedName for the contract."); - String contractString = (String) entity.getAttribute(ATTR_CONTRACT); + String contractString = getContractString(entity); DataContract contract = DataContract.deserialize(contractString); String datasetQName = contractQName.substring(0, contractQName.lastIndexOf('/')); contractQName = String.format("%s/%s/%s", datasetQName, contract.getType().name(), CONTRACT_QUALIFIED_NAME_SUFFIX); @@ -118,7 +114,8 @@ private void processCreateContract(AtlasEntity entity, EntityMutationContext con boolean contractSync = syncContractCertificateStatus(entity, contract); contractString = DataContract.serialize(contract); entity.setAttribute(ATTR_CONTRACT, contractString); - + String contractStringJSON = DataContract.serializeJSON(contract); + entity.setAttribute(ATTR_CONTRACT_JSON, contractStringJSON); AtlasEntity currentVersionEntity = getCurrentVersion(associatedAsset.getEntity().getGuid()); Long newVersionNumber = 1L; @@ -130,7 +127,7 @@ private void processCreateContract(AtlasEntity entity, EntityMutationContext con // No changes in the contract, Not creating new version removeCreatingVertex(context, entity); return; - } else if (contract.equals(DataContract.deserialize((String) currentVersionEntity.getAttribute(ATTR_CONTRACT)))) { + } else if (contract.equals(DataContract.deserialize(getContractString(currentVersionEntity)))) { resetAllRelationshipAttributes(entity); // No change in contract, metadata changed updateExistingVersion(context, entity, currentVersionEntity); @@ -170,22 +167,6 @@ private List getDiffAttributes(AtlasEntity entity, AtlasEntity latestExi return attributesSet; } - private boolean isEqualContract(String firstNode, String secondNode) throws AtlasBaseException { - ObjectMapper mapper = new ObjectMapper(); - try { - JsonNode actualObj1 = mapper.readTree(firstNode); - JsonNode actualObj2 = mapper.readTree(secondNode); - //Ignore status field change - ((ObjectNode) actualObj1).remove(CONTRACT_ATTR_STATUS); - ((ObjectNode) actualObj2).remove(CONTRACT_ATTR_STATUS); - - return actualObj1.equals(actualObj2); - } catch (JsonProcessingException e) { - throw new AtlasBaseException(JSON_ERROR, e.getMessage()); - } - - } - private void updateExistingVersion(EntityMutationContext context, AtlasEntity entity, AtlasEntity currentVersionEntity) throws AtlasBaseException { removeCreatingVertex(context, entity); entity.setAttribute(QUALIFIED_NAME, currentVersionEntity.getAttribute(QUALIFIED_NAME)); @@ -302,4 +283,12 @@ private static void validateAttribute(boolean isInvalid, String errorMessage) th if (isInvalid) throw new AtlasBaseException(BAD_REQUEST, errorMessage); } + + private static String getContractString(AtlasEntity entity) { + String contractString = (String) entity.getAttribute(ATTR_CONTRACT); + if (StringUtils.isEmpty(contractString)) { + contractString = (String) entity.getAttribute(ATTR_CONTRACT_JSON); + } + return contractString; + } } diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/DataContract.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/DataContract.java index a14bc3b00f..4ceea2853c 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/DataContract.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/DataContract.java @@ -12,7 +12,6 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import org.apache.atlas.AtlasErrorCode; import org.apache.atlas.exception.AtlasBaseException; -import org.apache.atlas.model.instance.AtlasEntity; import org.apache.commons.lang.StringUtils; import javax.validation.*; @@ -29,9 +28,11 @@ public class DataContract { private static final String KIND_VALUE = "DataContract"; private static final Pattern versionPattern = Pattern.compile("^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$"); - private static final ObjectMapper objectMapper = new ObjectMapper(new YAMLFactory()); + private static final ObjectMapper objectMapperYAML = new ObjectMapper(new YAMLFactory()); + private static final ObjectMapper objectMapperJSON = new ObjectMapper(); static { - objectMapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS); + objectMapperYAML.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS); + objectMapperJSON.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS); } @Valid @NotNull @@ -248,9 +249,13 @@ public static DataContract deserialize(String contractString) throws AtlasBaseEx DataContract contract; try { - contract = objectMapper.readValue(contractString, DataContract.class); + contract = objectMapperYAML.readValue(contractString, DataContract.class); } catch (JsonProcessingException ex) { - throw new AtlasBaseException(ex.getOriginalMessage()); + try { + contract = objectMapperJSON.readValue(contractString, DataContract.class); + } catch (JsonProcessingException e) { + throw new AtlasBaseException(ex.getOriginalMessage()); + } } contract.validate(); return contract; @@ -274,7 +279,7 @@ public void validate() throws AtlasBaseException { public static String serialize(DataContract contract) throws AtlasBaseException { try { - return objectMapper.writeValueAsString(contract); + return objectMapperYAML.writeValueAsString(contract); } catch (JsonProcessingException ex) { throw new AtlasBaseException(JSON_ERROR, ex.getMessage()); } @@ -305,5 +310,14 @@ public int hashCode() { return Objects.hash(super.hashCode(), kind, status, templateVersion, data_source, dataset, type, description, owners, tags, certificate, columns, unknownFields); } + + public static String serializeJSON(DataContract contract) throws AtlasBaseException { + + try { + return objectMapperJSON.writeValueAsString(contract); + } catch (JsonProcessingException ex) { + throw new AtlasBaseException(JSON_ERROR, ex.getMessage()); + } + } } From f88b042761e2b60b4606d90b09447fc5578aa6f3 Mon Sep 17 00:00:00 2001 From: hr2904 Date: Tue, 11 Jun 2024 13:56:35 +0530 Subject: [PATCH 09/16] Added Test Cases and fixed some edge logic in santisation methods. --- .../atlas/repository/util/FilterUtil.java | 13 ++- .../atlas/repository/util/FilterUtilTest.java | 62 ++++++++++++++ .../atlas/web/filters/ActiveServerFilter.java | 17 +++- .../MetaStoreActiveServerFilterTest.java | 85 +++++++++++++++++++ 4 files changed, 172 insertions(+), 5 deletions(-) create mode 100644 repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java create mode 100644 webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java diff --git a/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java b/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java index a5ec2f133d..6f244f5d8e 100644 --- a/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java +++ b/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java @@ -29,6 +29,8 @@ import org.apache.commons.collections.functors.NotPredicate; import org.apache.commons.lang.StringUtils; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -181,9 +183,13 @@ public static boolean validateFilePath(String fileToImport) { String allowedDirectory = "/var/app/allowed/"; try { - Path normalizedPath = Paths.get(fileToImport).normalize(); + // Decode URL-encoded characters first + String decodedPath = URLDecoder.decode(fileToImport, "UTF-8"); - if (fileToImport.contains("..") || fileToImport.contains("./") || fileToImport.contains(".\\")) { + Path normalizedPath = Paths.get(decodedPath).normalize(); + + // Check for directory traversal attempts after decoding + if (decodedPath.contains("..") || decodedPath.contains("./") || decodedPath.contains(".\\")) { return false; } @@ -196,7 +202,10 @@ public static boolean validateFilePath(String fileToImport) { } return true; + } catch (UnsupportedEncodingException e) { + return false; } catch (Exception e) { + // Handle other exceptions, such as those thrown by Paths.get() for invalid paths return false; } } diff --git a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java new file mode 100644 index 0000000000..19627b065a --- /dev/null +++ b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java @@ -0,0 +1,62 @@ +package org.apache.atlas.repository.util; + +import org.junit.Test; + +import static org.apache.atlas.repository.util.FilterUtil.validateFilePath; +import static org.junit.Assert.*; + +public class FilterUtilTest { + @Test + public void testValidateFilePath_ValidPath() { + assertTrue("Should return true for a valid path within the allowed directory.", + validateFilePath("/var/app/allowed/file.txt")); + } + + @Test + public void testValidateFilePath_RelativeTraversal() { + assertFalse("Should return false for a path attempting directory traversal.", + validateFilePath("/var/app/allowed/../notallowed/file.txt")); + } + + @Test + public void testValidateFilePath_DotSlash() { + assertFalse("Should return false for a path with relative current directory notation.", + validateFilePath("/var/app/allowed/./file.txt")); + } + + @Test + public void testValidateFilePath_BackSlash() { + assertFalse("Should return false for a path with mixed slash types potentially bypassing checks.", + validateFilePath("/var/app/allowed/.\\file.txt")); + } + + @Test + public void testValidateFilePath_NotAbsolute() { + assertFalse("Should return false for non-absolute paths.", + validateFilePath("var/app/allowed/file.txt")); + } + + @Test + public void testValidateFilePath_AbsoluteButOutsideAllowed() { + assertFalse("Should return false for absolute paths that do not start with the allowed directory.", + validateFilePath("/var/app/notallowed/file.txt")); + } + + @Test + public void testValidateFilePath_WithUnusualCharacters() { + assertFalse("Should return false for paths with unusual characters aiming to navigate directories.", + validateFilePath("/var/app/allowed/..\\file.txt")); + } + + @Test + public void testValidateFilePath_WithEncodedTraversal() { + assertFalse("Should return false for paths with URL-encoded traversal sequences.", + validateFilePath("/var/app/allowed/%2e%2e/notallowed/file.txt")); + } + + @Test + public void testValidateFilePath_CatchException() { + assertFalse("Should return false for paths that cause exceptions, like those containing null bytes.", + validateFilePath("/var/app/allowed/\0file.txt")); + } +} diff --git a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java index 613392e26b..68132f5b77 100644 --- a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java +++ b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java @@ -244,11 +244,22 @@ private void handleRedirect(HttpServletRequest servletRequest, HttpServletRespon httpServletResponse.sendRedirect(sanitizedLocation); } } - private static String sanitizeRedirectLocation(String redirectLocation) { + public static String sanitizeRedirectLocation(String redirectLocation) { if (redirectLocation == null) return null; try { - String encodedUrl = URLEncoder.encode(redirectLocation, "UTF-8"); - return encodedUrl.replaceAll("\\r", "").replaceAll("\\n", ""); + // Remove CR and LF characters to preemptively prevent response splitting + String preProcessedUrl = redirectLocation.replace("\r", "").replace("\n", ""); + + // Encode any percent signs not already part of a percent-encoded sequence + preProcessedUrl = preProcessedUrl.replaceAll("%(?![0-9a-fA-F]{2})", "%25"); + + // URL encode the entire string + String encodedUrl = URLEncoder.encode(preProcessedUrl, "UTF-8"); + + // Normalize encoded sequences that might be affected by double encoding + encodedUrl = encodedUrl.replaceAll("%25([0-9a-fA-F]{2})", "%$1"); + + return encodedUrl; } catch (UnsupportedEncodingException e) { throw new RuntimeException("UTF-8 encoding not supported", e); } diff --git a/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java b/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java new file mode 100644 index 0000000000..0c398ecc87 --- /dev/null +++ b/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java @@ -0,0 +1,85 @@ +package org.apache.atlas.web.filters; + +import org.junit.Test; + + +import static org.apache.atlas.web.filters.ActiveServerFilter.sanitizeRedirectLocation; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class MetaStoreActiveServerFilterTest { + + @Test + public void testSanitizeRedirectLocation_WithValidUrl() { + String testUrl = "http://example.com/page?param=value"; + String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("The URLs do not match.",expected, actual); + } + + @Test + public void testSanitizeRedirectLocation_WithNull() { + assertNull("Output should be null for null input.",sanitizeRedirectLocation(null)); + } + + + + + @Test + public void testSanitizeRedirectLocation_WithSpecialCharacters() { + String testUrl = "http://example.com/page?param=value&another=one"; + String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue%26another%3Done"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("Special characters should be URL encoded.", expected, actual); + } + + @Test + public void testSanitizeRedirectLocation_CorruptingCharactersForHttpSplitting() { + String testUrl = "http://example.com/page?param=value%Set-Cookie: test=evil"; + String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue%25Set-Cookie%3A+test%3Devil"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("HTTP response splitting characters and other specials should be properly encoded.", expected, actual); + } + + @Test + public void testSanitizeRedirectLocation_MultiLineQueryParameter() { + String testUrl = "http://example.com/search?query=value\n"; + String expected = "http%3A%2F%2Fexample.com%2Fsearch%3Fquery%3Dvalue%3Cscript%3Ealert%28%27xss%27%29%3C%2Fscript%3E"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("Multi-line and script injection attempts should be encoded.", expected, actual); + } + + + @Test + public void testSanitizeRedirectLocation_CRLFInjectionToSplitResponse() { + String testUrl = "http://example.com/update?action=edit%HTTP/1.1 200 OKContent-Type: text/html"; + String expected = "http%3A%2F%2Fexample.com%2Fupdate%3Faction%3Dedit%25HTTP%2F1.1+200+OKContent-Type%3A+text%2Fhtml"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("CRLF characters used to split HTTP responses should be properly encoded.", expected, actual); + } + + @Test + public void testSanitizeRedirectLocation_HeaderInjectionViaNewline() { + String testUrl = "http://example.com/login?redirect=success%Set-Cookie: sessionId=12345"; + String expected = "http%3A%2F%2Fexample.com%2Flogin%3Fredirect%3Dsuccess%25Set-Cookie%3A+sessionId%3D12345"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("Characters potentially harmful for HTTP response splitting should be encoded.", expected, actual); + } + + @Test + public void testSanitizeRedirectLocation_CRLFRemoved() { + String testUrl = "http://example.com/page\r"; + String expected = "http%3A%2F%2Fexample.com%2Fpage"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("Carriage return characters should be removed.", expected, actual); + } + + @Test + public void testSanitizeRedirectLocation_EncodedLineBreaks() { + String testUrl = "http://example.com/page?next=url%0D%0AContent-Length: %300"; + String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fnext%3Durl%0D%0AContent-Length%3A+%300"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("Encoded line breaks and attempts to continue headers should be removed.", expected, actual); + } + +} From 4165f918d4bedb6b46cf6ad2bf2f0dc1c1b321c3 Mon Sep 17 00:00:00 2001 From: hr2904 Date: Tue, 11 Jun 2024 14:01:10 +0530 Subject: [PATCH 10/16] Removed comments --- .../java/org/apache/atlas/repository/util/FilterUtil.java | 4 ---- .../java/org/apache/atlas/web/filters/ActiveServerFilter.java | 4 ---- 2 files changed, 8 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java b/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java index 6f244f5d8e..5e3cdd63b2 100644 --- a/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java +++ b/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java @@ -183,12 +183,9 @@ public static boolean validateFilePath(String fileToImport) { String allowedDirectory = "/var/app/allowed/"; try { - // Decode URL-encoded characters first String decodedPath = URLDecoder.decode(fileToImport, "UTF-8"); Path normalizedPath = Paths.get(decodedPath).normalize(); - - // Check for directory traversal attempts after decoding if (decodedPath.contains("..") || decodedPath.contains("./") || decodedPath.contains(".\\")) { return false; } @@ -205,7 +202,6 @@ public static boolean validateFilePath(String fileToImport) { } catch (UnsupportedEncodingException e) { return false; } catch (Exception e) { - // Handle other exceptions, such as those thrown by Paths.get() for invalid paths return false; } } diff --git a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java index 68132f5b77..169afd804a 100644 --- a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java +++ b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java @@ -247,16 +247,12 @@ private void handleRedirect(HttpServletRequest servletRequest, HttpServletRespon public static String sanitizeRedirectLocation(String redirectLocation) { if (redirectLocation == null) return null; try { - // Remove CR and LF characters to preemptively prevent response splitting String preProcessedUrl = redirectLocation.replace("\r", "").replace("\n", ""); - // Encode any percent signs not already part of a percent-encoded sequence preProcessedUrl = preProcessedUrl.replaceAll("%(?![0-9a-fA-F]{2})", "%25"); - // URL encode the entire string String encodedUrl = URLEncoder.encode(preProcessedUrl, "UTF-8"); - // Normalize encoded sequences that might be affected by double encoding encodedUrl = encodedUrl.replaceAll("%25([0-9a-fA-F]{2})", "%$1"); return encodedUrl; From 5d91514096693e21e3d1108b786af2cd6e829af9 Mon Sep 17 00:00:00 2001 From: hr2904 Date: Thu, 13 Jun 2024 17:51:42 +0530 Subject: [PATCH 11/16] Fixed PR comments --- .../atlas/repository/util/FilterUtil.java | 5 ---- .../atlas/repository/util/FilterUtilTest.java | 5 ---- .../MetaStoreActiveServerFilterTest.java | 24 +++++++++++++++---- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java b/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java index 5e3cdd63b2..4c03d2082f 100644 --- a/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java +++ b/repository/src/main/java/org/apache/atlas/repository/util/FilterUtil.java @@ -180,7 +180,6 @@ public static void addParamsToHideInternalType(SearchFilter searchFilter) { } public static boolean validateFilePath(String fileToImport) { - String allowedDirectory = "/var/app/allowed/"; try { String decodedPath = URLDecoder.decode(fileToImport, "UTF-8"); @@ -194,10 +193,6 @@ public static boolean validateFilePath(String fileToImport) { return false; } - if (!normalizedPath.startsWith(Paths.get(allowedDirectory))) { - return false; - } - return true; } catch (UnsupportedEncodingException e) { return false; diff --git a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java index 19627b065a..16623cb238 100644 --- a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java +++ b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java @@ -36,11 +36,6 @@ public void testValidateFilePath_NotAbsolute() { validateFilePath("var/app/allowed/file.txt")); } - @Test - public void testValidateFilePath_AbsoluteButOutsideAllowed() { - assertFalse("Should return false for absolute paths that do not start with the allowed directory.", - validateFilePath("/var/app/notallowed/file.txt")); - } @Test public void testValidateFilePath_WithUnusualCharacters() { diff --git a/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java b/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java index 0c398ecc87..ef1467c31e 100644 --- a/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java +++ b/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java @@ -10,11 +10,27 @@ public class MetaStoreActiveServerFilterTest { @Test - public void testSanitizeRedirectLocation_WithValidUrl() { - String testUrl = "http://example.com/page?param=value"; - String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue"; + public void testSanitizeRedirectLocation_WithValidUr1() { + String testUrl = "https://dom-sub-uat.atlan.com/api/meta/entity/guid/fd7a69c9-738b-4b35-a0db-1da00cbd86cd"; + String expected = "https%3A%2F%2Fdom-sub-uat.atlan.com%2Fapi%2Fmeta%2Fentity%2Fguid%2Ffd7a69c9-738b-4b35-a0db-1da00cbd86cd"; String actual = sanitizeRedirectLocation(testUrl); - assertEquals("The URLs do not match.",expected, actual); + assertEquals("The URLs do match.",expected, actual); + } + + @Test + public void testSanitizeRedirectLocation_WithValidUrl2() { + String testUrl = "https://datamesh.atlan.com/api/meta/entity/bulk?replaceBusinessAttributes=true&replaceClassifications=true"; + String expected = "https%3A%2F%2Fdatamesh.atlan.com%2Fapi%2Fmeta%2Fentity%2Fbulk%3FreplaceBusinessAttributes%3Dtrue%26replaceClassifications%3Dtrue"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("The URLs do match.",expected, actual); + } + + @Test + public void testSanitizeRedirectLocation_WithValidUrl3() { + String testUrl = "https://datamesh.atlan.com/api/meta/entity/guid/fd7a69c9-738b-4b35-a0db-1da00cbd86cd"; + String expected = "https%3A%2F%2Fdatamesh.atlan.com%2Fapi%2Fmeta%2Fentity%2Fguid%2Ffd7a69c9-738b-4b35-a0db-1da00cbd86cd"; + String actual = sanitizeRedirectLocation(testUrl); + assertEquals("The URLs do match.",expected, actual); } @Test From 33aa9c247d5b8c03fa275d105b3b3c1dcf12720e Mon Sep 17 00:00:00 2001 From: hr2904 Date: Fri, 14 Jun 2024 11:55:33 +0530 Subject: [PATCH 12/16] squashed tests into a single paramter type test. --- .../atlas/repository/util/FilterUtilTest.java | 70 ++++------- .../MetaStoreActiveServerFilterTest.java | 110 ++++-------------- 2 files changed, 48 insertions(+), 132 deletions(-) diff --git a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java index 16623cb238..48641a3cc4 100644 --- a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java +++ b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java @@ -7,51 +7,29 @@ public class FilterUtilTest { @Test - public void testValidateFilePath_ValidPath() { - assertTrue("Should return true for a valid path within the allowed directory.", - validateFilePath("/var/app/allowed/file.txt")); - } - - @Test - public void testValidateFilePath_RelativeTraversal() { - assertFalse("Should return false for a path attempting directory traversal.", - validateFilePath("/var/app/allowed/../notallowed/file.txt")); - } - - @Test - public void testValidateFilePath_DotSlash() { - assertFalse("Should return false for a path with relative current directory notation.", - validateFilePath("/var/app/allowed/./file.txt")); - } - - @Test - public void testValidateFilePath_BackSlash() { - assertFalse("Should return false for a path with mixed slash types potentially bypassing checks.", - validateFilePath("/var/app/allowed/.\\file.txt")); - } - - @Test - public void testValidateFilePath_NotAbsolute() { - assertFalse("Should return false for non-absolute paths.", - validateFilePath("var/app/allowed/file.txt")); - } - - - @Test - public void testValidateFilePath_WithUnusualCharacters() { - assertFalse("Should return false for paths with unusual characters aiming to navigate directories.", - validateFilePath("/var/app/allowed/..\\file.txt")); - } - - @Test - public void testValidateFilePath_WithEncodedTraversal() { - assertFalse("Should return false for paths with URL-encoded traversal sequences.", - validateFilePath("/var/app/allowed/%2e%2e/notallowed/file.txt")); - } - - @Test - public void testValidateFilePath_CatchException() { - assertFalse("Should return false for paths that cause exceptions, like those containing null bytes.", - validateFilePath("/var/app/allowed/\0file.txt")); + public void testValidateFilePath() { + // Array of test cases, each containing the file path and the expected boolean result + Object[][] testCases = { + {"/var/app/allowed/file.txt", true, "Should return true for a valid path within the allowed directory."}, + {"/var/app/allowed/../notallowed/file.txt", false, "Should return false for a path attempting directory traversal."}, + {"/var/app/allowed/./file.txt", false, "Should return false for a path with relative current directory notation."}, + {"/var/app/allowed/.\\file.txt", false, "Should return false for a path with mixed slash types potentially bypassing checks."}, + {"var/app/allowed/file.txt", false, "Should return false for non-absolute paths."}, + {"/var/app/allowed/..\\file.txt", false, "Should return false for paths with unusual characters aiming to navigate directories."}, + {"/var/app/allowed/%2e%2e/notallowed/file.txt", false, "Should return false for paths with URL-encoded traversal sequences."}, + {"/var/app/allowed/\0file.txt", false, "Should return false for paths that cause exceptions, like those containing null bytes."} + }; + + for (Object[] testCase : testCases) { + String path = (String) testCase[0]; + boolean expected = (Boolean) testCase[1]; + String message = (String) testCase[2]; + + if (expected) { + assertTrue(message, validateFilePath(path)); + } else { + assertFalse(message, validateFilePath(path)); + } + } } } diff --git a/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java b/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java index ef1467c31e..5119ba7cc6 100644 --- a/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java +++ b/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java @@ -10,92 +10,30 @@ public class MetaStoreActiveServerFilterTest { @Test - public void testSanitizeRedirectLocation_WithValidUr1() { - String testUrl = "https://dom-sub-uat.atlan.com/api/meta/entity/guid/fd7a69c9-738b-4b35-a0db-1da00cbd86cd"; - String expected = "https%3A%2F%2Fdom-sub-uat.atlan.com%2Fapi%2Fmeta%2Fentity%2Fguid%2Ffd7a69c9-738b-4b35-a0db-1da00cbd86cd"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("The URLs do match.",expected, actual); - } - - @Test - public void testSanitizeRedirectLocation_WithValidUrl2() { - String testUrl = "https://datamesh.atlan.com/api/meta/entity/bulk?replaceBusinessAttributes=true&replaceClassifications=true"; - String expected = "https%3A%2F%2Fdatamesh.atlan.com%2Fapi%2Fmeta%2Fentity%2Fbulk%3FreplaceBusinessAttributes%3Dtrue%26replaceClassifications%3Dtrue"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("The URLs do match.",expected, actual); - } - - @Test - public void testSanitizeRedirectLocation_WithValidUrl3() { - String testUrl = "https://datamesh.atlan.com/api/meta/entity/guid/fd7a69c9-738b-4b35-a0db-1da00cbd86cd"; - String expected = "https%3A%2F%2Fdatamesh.atlan.com%2Fapi%2Fmeta%2Fentity%2Fguid%2Ffd7a69c9-738b-4b35-a0db-1da00cbd86cd"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("The URLs do match.",expected, actual); - } - - @Test - public void testSanitizeRedirectLocation_WithNull() { - assertNull("Output should be null for null input.",sanitizeRedirectLocation(null)); - } - - - - - @Test - public void testSanitizeRedirectLocation_WithSpecialCharacters() { - String testUrl = "http://example.com/page?param=value&another=one"; - String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue%26another%3Done"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("Special characters should be URL encoded.", expected, actual); - } - - @Test - public void testSanitizeRedirectLocation_CorruptingCharactersForHttpSplitting() { - String testUrl = "http://example.com/page?param=value%Set-Cookie: test=evil"; - String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue%25Set-Cookie%3A+test%3Devil"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("HTTP response splitting characters and other specials should be properly encoded.", expected, actual); - } - - @Test - public void testSanitizeRedirectLocation_MultiLineQueryParameter() { - String testUrl = "http://example.com/search?query=value\n"; - String expected = "http%3A%2F%2Fexample.com%2Fsearch%3Fquery%3Dvalue%3Cscript%3Ealert%28%27xss%27%29%3C%2Fscript%3E"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("Multi-line and script injection attempts should be encoded.", expected, actual); - } - - - @Test - public void testSanitizeRedirectLocation_CRLFInjectionToSplitResponse() { - String testUrl = "http://example.com/update?action=edit%HTTP/1.1 200 OKContent-Type: text/html"; - String expected = "http%3A%2F%2Fexample.com%2Fupdate%3Faction%3Dedit%25HTTP%2F1.1+200+OKContent-Type%3A+text%2Fhtml"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("CRLF characters used to split HTTP responses should be properly encoded.", expected, actual); - } - - @Test - public void testSanitizeRedirectLocation_HeaderInjectionViaNewline() { - String testUrl = "http://example.com/login?redirect=success%Set-Cookie: sessionId=12345"; - String expected = "http%3A%2F%2Fexample.com%2Flogin%3Fredirect%3Dsuccess%25Set-Cookie%3A+sessionId%3D12345"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("Characters potentially harmful for HTTP response splitting should be encoded.", expected, actual); - } - - @Test - public void testSanitizeRedirectLocation_CRLFRemoved() { - String testUrl = "http://example.com/page\r"; - String expected = "http%3A%2F%2Fexample.com%2Fpage"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("Carriage return characters should be removed.", expected, actual); - } - - @Test - public void testSanitizeRedirectLocation_EncodedLineBreaks() { - String testUrl = "http://example.com/page?next=url%0D%0AContent-Length: %300"; - String expected = "http%3A%2F%2Fexample.com%2Fpage%3Fnext%3Durl%0D%0AContent-Length%3A+%300"; - String actual = sanitizeRedirectLocation(testUrl); - assertEquals("Encoded line breaks and attempts to continue headers should be removed.", expected, actual); + public void testSanitizeRedirectLocation() { + Object[][] testCases = { + {"https://dom-sub-uat.atlan.com/api/meta/entity/guid/fd7a69c9-738b-4b35-a0db-1da00cbd86cd", "https%3A%2F%2Fdom-sub-uat.atlan.com%2Fapi%2Fmeta%2Fentity%2Fguid%2Ffd7a69c9-738b-4b35-a0db-1da00cbd86cd"}, + {"https://datamesh.atlan.com/api/meta/entity/bulk?replaceBusinessAttributes=true&replaceClassifications=true", "https%3A%2F%2Fdatamesh.atlan.com%2Fapi%2Fmeta%2Fentity%2Fbulk%3FreplaceBusinessAttributes%3Dtrue%26replaceClassifications%3Dtrue"}, + {"http://example.com/page?param=value&another=one", "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue%26another%3Done"}, + {"http://example.com/page?param=value%Set-Cookie: test=evil", "http%3A%2F%2Fexample.com%2Fpage%3Fparam%3Dvalue%25Set-Cookie%3A+test%3Devil"}, + {"http://example.com/search?query=value\n", "http%3A%2F%2Fexample.com%2Fsearch%3Fquery%3Dvalue%3Cscript%3Ealert%28%27xss%27%29%3C%2Fscript%3E"}, + {"http://example.com/update?action=edit%HTTP/1.1 200 OKContent-Type: text/html", "http%3A%2F%2Fexample.com%2Fupdate%3Faction%3Dedit%25HTTP%2F1.1+200+OKContent-Type%3A+text%2Fhtml"}, + {"http://example.com/login?redirect=success%Set-Cookie: sessionId=12345", "http%3A%2F%2Fexample.com%2Flogin%3Fredirect%3Dsuccess%25Set-Cookie%3A+sessionId%3D12345"}, + {"http://example.com/page\r", "http%3A%2F%2Fexample.com%2Fpage"}, + {"http://example.com/page?next=url%0D%0AContent-Length: %300", "http%3A%2F%2Fexample.com%2Fpage%3Fnext%3Durl%0D%0AContent-Length%3A+%300"}, + {null, null} // Testing for null input + }; + + for (Object[] testCase : testCases) { + String input = (String) testCase[0]; + String expected = (String) testCase[1]; + + if (input == null) { + assertNull("Output should be null for null input.", sanitizeRedirectLocation(input)); + } else { + assertEquals("URLs should be correctly sanitized.", expected, sanitizeRedirectLocation(input)); + } + } } } From a365a1c9f0110b01d7c99ec3486f9c68caa94dfc Mon Sep 17 00:00:00 2001 From: hr2904 Date: Fri, 14 Jun 2024 12:12:42 +0530 Subject: [PATCH 13/16] added file paths variety --- .../org/apache/atlas/repository/util/FilterUtilTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java index 48641a3cc4..7edeb96293 100644 --- a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java +++ b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java @@ -11,12 +11,12 @@ public void testValidateFilePath() { // Array of test cases, each containing the file path and the expected boolean result Object[][] testCases = { {"/var/app/allowed/file.txt", true, "Should return true for a valid path within the allowed directory."}, - {"/var/app/allowed/../notallowed/file.txt", false, "Should return false for a path attempting directory traversal."}, + {"/tmp/../notallowed/file.txt", false, "Should return false for a path attempting directory traversal."}, {"/var/app/allowed/./file.txt", false, "Should return false for a path with relative current directory notation."}, - {"/var/app/allowed/.\\file.txt", false, "Should return false for a path with mixed slash types potentially bypassing checks."}, - {"var/app/allowed/file.txt", false, "Should return false for non-absolute paths."}, + {"/Users/username/repos/repo0/.\\file.txt", false, "Should return false for a path with mixed slash types potentially bypassing checks."}, + {"tmp/file.txt", false, "Should return false for non-absolute paths."}, {"/var/app/allowed/..\\file.txt", false, "Should return false for paths with unusual characters aiming to navigate directories."}, - {"/var/app/allowed/%2e%2e/notallowed/file.txt", false, "Should return false for paths with URL-encoded traversal sequences."}, + {"/Users/username/repos/repo0/%2e%2e/notallowed/file.txt", false, "Should return false for paths with URL-encoded traversal sequences."}, {"/var/app/allowed/\0file.txt", false, "Should return false for paths that cause exceptions, like those containing null bytes."} }; From 7b9ae98b7bcb2686f7e171c08ba8db8c0a146d18 Mon Sep 17 00:00:00 2001 From: hr2904 Date: Mon, 17 Jun 2024 13:30:23 +0530 Subject: [PATCH 14/16] Fixed PR comments --- .../repository/impexp/MigrationProgressService.java | 5 +++-- .../migration/DataMigrationStatusService.java | 13 +++++-------- .../store/graph/v2/bulkimport/MigrationImport.java | 2 +- .../migration/MigrationProgressServiceTest.java | 7 ++++++- .../atlas/repository/util/FilterUtilTest.java | 1 + 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java b/repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java index 6bb5f1e221..35c01ccac0 100644 --- a/repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java +++ b/repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java @@ -22,6 +22,7 @@ import org.apache.atlas.ApplicationProperties; import org.apache.atlas.AtlasException; import org.apache.atlas.annotation.AtlasService; +import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.model.impexp.MigrationStatus; import org.apache.atlas.repository.graph.AtlasGraphProvider; import org.apache.atlas.repository.graphdb.GraphDBMigrator; @@ -55,7 +56,7 @@ public class MigrationProgressService { private boolean zipFileBasedMigrationImport; @Inject - public MigrationProgressService(Configuration configuration, GraphDBMigrator migrator) { + public MigrationProgressService(Configuration configuration, GraphDBMigrator migrator) throws AtlasBaseException { this.migrator = migrator; this.cacheValidity = (configuration != null) ? configuration.getLong(MIGRATION_QUERY_CACHE_TTL, DEFAULT_CACHE_TTL_IN_SECS) : DEFAULT_CACHE_TTL_IN_SECS; @@ -63,7 +64,7 @@ public MigrationProgressService(Configuration configuration, GraphDBMigrator mig initConditionallyZipFileBasedMigrator(); } - private void initConditionallyZipFileBasedMigrator() { + private void initConditionallyZipFileBasedMigrator() throws AtlasBaseException { if (!zipFileBasedMigrationImport) { return; } diff --git a/repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java b/repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java index 5efa675f0c..852c645084 100644 --- a/repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java +++ b/repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationStatusService.java @@ -18,6 +18,7 @@ package org.apache.atlas.repository.migration; +import org.apache.atlas.AtlasErrorCode; import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.model.migration.MigrationImportStatus; import org.apache.atlas.repository.Constants; @@ -57,16 +58,14 @@ public DataMigrationStatusService(AtlasGraph graph) { } - public void init(String fileToImport) { + public void init(String fileToImport) throws AtlasBaseException { try { if(!validateFilePath(fileToImport)){ - throw new AtlasBaseException("File Path is invalid"); + throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "File Path is invalid"); } this.status = new MigrationImportStatus(fileToImport, DigestUtils.md5Hex(new FileInputStream(fileToImport))); } catch (IOException e) { LOG.error("Not able to create Migration status", e); - } catch (AtlasBaseException e) { - LOG.error("File Path is invalid"); } if (!this.migrationStatusVertexManagement.exists(status.getFileHash())) { @@ -77,17 +76,15 @@ public void init(String fileToImport) { } - public MigrationImportStatus getCreate(String fileName) { + public MigrationImportStatus getCreate(String fileName) throws AtlasBaseException { MigrationImportStatus create = null; try { if(!validateFilePath(fileName)){ - throw new AtlasBaseException("File Path is invalid"); + throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "File Path is invalid"); } create = getCreate(new MigrationImportStatus(fileName, DigestUtils.md5Hex(new FileInputStream(fileName)))); } catch (IOException e) { LOG.error("Exception occurred while creating migration import", e); - } catch (AtlasBaseException e) { - LOG.error("File Path is invalid"); } return create; diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/MigrationImport.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/MigrationImport.java index 0cc7c4a318..9edbfc1cc3 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/MigrationImport.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/MigrationImport.java @@ -86,7 +86,7 @@ public EntityMutationResponse run(EntityImportStream entityStream, AtlasImportRe return ret; } - private DataMigrationStatusService createMigrationStatusService(AtlasImportResult importResult) { + private DataMigrationStatusService createMigrationStatusService(AtlasImportResult importResult) throws AtlasBaseException { DataMigrationStatusService dataMigrationStatusService = new DataMigrationStatusService(); dataMigrationStatusService.init(importResult.getRequest().getOptions().get(AtlasImportRequest.OPTION_KEY_MIGRATION_FILE_NAME)); return dataMigrationStatusService; diff --git a/repository/src/test/java/org/apache/atlas/repository/migration/MigrationProgressServiceTest.java b/repository/src/test/java/org/apache/atlas/repository/migration/MigrationProgressServiceTest.java index 33125c86ca..b427988470 100644 --- a/repository/src/test/java/org/apache/atlas/repository/migration/MigrationProgressServiceTest.java +++ b/repository/src/test/java/org/apache/atlas/repository/migration/MigrationProgressServiceTest.java @@ -17,6 +17,7 @@ */ package org.apache.atlas.repository.migration; +import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.model.impexp.MigrationStatus; import org.apache.atlas.repository.graphdb.*; import org.apache.atlas.repository.graphdb.janus.migration.ReaderStatusManager; @@ -84,7 +85,11 @@ public void cachedStatusReturnedIfQueriedBeforeCacheExpiration() { } private MigrationProgressService getMigrationStatusForTest(Configuration cfg, TinkerGraph tg) { - return new MigrationProgressService(cfg, createMigrator(tg)); + try { + return new MigrationProgressService(cfg, createMigrator(tg)); + } catch (AtlasBaseException e) { + throw new RuntimeException(e); + } } @Test diff --git a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java index 7edeb96293..8676093a79 100644 --- a/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java +++ b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java @@ -15,6 +15,7 @@ public void testValidateFilePath() { {"/var/app/allowed/./file.txt", false, "Should return false for a path with relative current directory notation."}, {"/Users/username/repos/repo0/.\\file.txt", false, "Should return false for a path with mixed slash types potentially bypassing checks."}, {"tmp/file.txt", false, "Should return false for non-absolute paths."}, + {"", false, "Should return false for empty paths"}, {"/var/app/allowed/..\\file.txt", false, "Should return false for paths with unusual characters aiming to navigate directories."}, {"/Users/username/repos/repo0/%2e%2e/notallowed/file.txt", false, "Should return false for paths with URL-encoded traversal sequences."}, {"/var/app/allowed/\0file.txt", false, "Should return false for paths that cause exceptions, like those containing null bytes."} From 7bf5894b6966d437b1e2a525415f513a6e578ba2 Mon Sep 17 00:00:00 2001 From: hr2904 Date: Thu, 20 Jun 2024 13:57:33 +0530 Subject: [PATCH 15/16] Fixed PR comments. --- .../java/org/apache/atlas/web/filters/ActiveServerFilter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java index 169afd804a..7a30338425 100644 --- a/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java +++ b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java @@ -233,10 +233,10 @@ private void handleRedirect(HttpServletRequest servletRequest, HttpServletRespon requestURI = "/"; } String redirectLocation = activeServerAddress + requestURI; - LOG.info("Not active. Redirecting to {}", redirectLocation); + String sanitizedLocation = sanitizeRedirectLocation(redirectLocation); + LOG.info("Not active. Redirecting to {}", sanitizedLocation); // A POST/PUT/DELETE require special handling by sending HTTP 307 instead of the regular 301/302. // Reference: http://stackoverflow.com/questions/2068418/whats-the-difference-between-a-302-and-a-307-redirect - String sanitizedLocation = sanitizeRedirectLocation(redirectLocation); if (isUnsafeHttpMethod(servletRequest)) { httpServletResponse.setHeader(HttpHeaders.LOCATION, sanitizedLocation); httpServletResponse.setStatus(HttpServletResponse.SC_TEMPORARY_REDIRECT); From 2ccf0d6631cef5d4fabe4673a2f311570df17165 Mon Sep 17 00:00:00 2001 From: Bichitra Kumar Sahoo <32828151+bichitra95@users.noreply.github.com> Date: Fri, 21 Jun 2024 15:06:54 +0530 Subject: [PATCH 16/16] Bypass contract upadte for migration script --- .../graph/v2/preprocessor/contract/ContractPreProcessor.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java index 869f2b5c6a..8c7d39ea37 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v2/preprocessor/contract/ContractPreProcessor.java @@ -81,7 +81,10 @@ private void processUpdateContract(AtlasEntity entity, EntityMutationContext con resetAllRelationshipAttributes(entity); DataContract contract = DataContract.deserialize(contractString); String existingContractString = getContractString(existingContractEntity); - if (!StringUtils.isEmpty(contractString) && !contract.equals(DataContract.deserialize(existingContractString))) { + boolean requestFromMigration = RequestContext.get().getRequestContextHeaders().getOrDefault( + "x-atlan-request-id", "").contains("json-to-yaml-migration"); + if (!requestFromMigration && !StringUtils.isEmpty(contractString) && + !contract.equals(DataContract.deserialize(existingContractString))) { // Update the same asset(entity) throw new AtlasBaseException(OPERATION_NOT_SUPPORTED, "Can't update a specific version of contract"); }