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 9408328b9f..accea8ec88 100644 --- a/common/src/main/java/org/apache/atlas/repository/Constants.java +++ b/common/src/main/java/org/apache/atlas/repository/Constants.java @@ -431,7 +431,8 @@ 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 ATTR_CONTRACT_JSON = "dataContractJson"; public static final String STRUCT_STARRED_DETAILS = "StarredDetails"; public static final String KEYCLOAK_ROLE_ADMIN = "$admin"; 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 + 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..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; @@ -47,6 +48,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 +194,9 @@ public AtlasImportResult run(AtlasImportRequest request, String userName, String if (StringUtils.isBlank(fileName)) { throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, "FILENAME parameter not found"); } - + if(!FilterUtil.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 +301,5 @@ boolean checkHiveTableIncrementalSkipLineage(AtlasImportRequest importRequest, A private boolean isMigrationMode(AtlasImportRequest request) { return request.getOptions().containsKey(AtlasImportRequest.OPTION_KEY_MIGRATION); } + } 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 3d357ddcfa..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,22 +18,28 @@ 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; 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; @@ -52,8 +58,11 @@ public DataMigrationStatusService(AtlasGraph graph) { } - public void init(String fileToImport) { + public void init(String fileToImport) throws AtlasBaseException { try { + if(!validateFilePath(fileToImport)){ + 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); @@ -66,9 +75,13 @@ public void init(String fileToImport) { getCreate(fileToImport); } - public MigrationImportStatus getCreate(String fileName) { + + public MigrationImportStatus getCreate(String fileName) throws AtlasBaseException { MigrationImportStatus create = null; try { + if(!validateFilePath(fileName)){ + 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); 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/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 98add96a39..1407c3c2ef 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; @@ -19,11 +15,10 @@ 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; -import com.fasterxml.jackson.databind.ObjectMapper; - import java.util.*; import static org.apache.atlas.AtlasErrorCode.*; @@ -43,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); } @@ -78,12 +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 (!isEqualContract(contractString, (String) existingContractEntity.getAttribute(ATTR_CONTRACT))) { + DataContract contract = DataContract.deserialize(contractString); + String existingContractString = getContractString(existingContractEntity); + 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"); } @@ -105,7 +106,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); @@ -116,7 +117,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; @@ -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(getContractString(currentVersionEntity)))) { resetAllRelationshipAttributes(entity); // No change in contract, metadata changed updateExistingVersion(context, entity, currentVersionEntity); @@ -168,22 +170,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)); @@ -304,4 +290,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 dc3cdb466b..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 @@ -9,6 +9,7 @@ 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.commons.lang.StringUtils; @@ -27,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(); + 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 @@ -184,6 +187,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 +223,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 { @@ -216,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; @@ -242,11 +279,45 @@ 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()); } } + @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); + } + + public static String serializeJSON(DataContract contract) throws AtlasBaseException { + + try { + return objectMapperJSON.writeValueAsString(contract); + } catch (JsonProcessingException ex) { + throw new AtlasBaseException(JSON_ERROR, ex.getMessage()); + } + } } 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..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 @@ -29,6 +29,10 @@ 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; import java.util.List; import java.util.Objects; @@ -174,4 +178,26 @@ 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) { + + try { + String decodedPath = URLDecoder.decode(fileToImport, "UTF-8"); + + Path normalizedPath = Paths.get(decodedPath).normalize(); + if (decodedPath.contains("..") || decodedPath.contains("./") || decodedPath.contains(".\\")) { + return false; + } + + if (!normalizedPath.isAbsolute()) { + return false; + } + + return true; + } catch (UnsupportedEncodingException e) { + return false; + } catch (Exception e) { + return false; + } + } } 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 new file mode 100644 index 0000000000..8676093a79 --- /dev/null +++ b/repository/src/test/java/org/apache/atlas/repository/util/FilterUtilTest.java @@ -0,0 +1,36 @@ +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() { + // 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."}, + {"/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."}, + {"/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."} + }; + + 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/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java b/webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java index 2721efa6f2..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 @@ -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; /** @@ -231,14 +233,31 @@ 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 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); + } + } + public static String sanitizeRedirectLocation(String redirectLocation) { + if (redirectLocation == null) return null; + try { + String preProcessedUrl = redirectLocation.replace("\r", "").replace("\n", ""); + + preProcessedUrl = preProcessedUrl.replaceAll("%(?![0-9a-fA-F]{2})", "%25"); + + String encodedUrl = URLEncoder.encode(preProcessedUrl, "UTF-8"); + + 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..5119ba7cc6 --- /dev/null +++ b/webapp/src/test/java/org/apache/atlas/web/filters/MetaStoreActiveServerFilterTest.java @@ -0,0 +1,39 @@ +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() { + 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)); + } + } + } + +}