Skip to content

Commit

Permalink
Merge pull request #3294 from atlanhq/beta-master-helper
Browse files Browse the repository at this point in the history
Syncing Beta with Master
  • Loading branch information
hr2904 authored Jul 2, 2024
2 parents f0f0152 + f8a4c29 commit bd0d002
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -55,15 +56,15 @@ 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;

this.zipFileBasedMigrationImport = isZipFileBasedMigrationEnabled();
initConditionallyZipFileBasedMigrator();
}

private void initConditionallyZipFileBasedMigrator() {
private void initConditionallyZipFileBasedMigrator() throws AtlasBaseException {
if (!zipFileBasedMigrationImport) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())) {
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -178,24 +180,22 @@ public static void addParamsToHideInternalType(SearchFilter searchFilter) {
}

public static boolean validateFilePath(String fileToImport) {
String allowedDirectory = "/var/app/allowed/";

try {
Path normalizedPath = Paths.get(fileToImport).normalize();
String decodedPath = URLDecoder.decode(fileToImport, "UTF-8");

if (fileToImport.contains("..") || fileToImport.contains("./") || fileToImport.contains(".\\")) {
Path normalizedPath = Paths.get(decodedPath).normalize();
if (decodedPath.contains("..") || decodedPath.contains("./") || decodedPath.contains(".\\")) {
return false;
}

if (!normalizedPath.isAbsolute()) {
return false;
}

if (!normalizedPath.startsWith(Paths.get(allowedDirectory))) {
return false;
}

return true;
} catch (UnsupportedEncodingException e) {
return false;
} catch (Exception e) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -233,22 +233,29 @@ 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);
} else {
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", "");
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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<script>alert('xss')</script>", "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));
}
}
}

}

0 comments on commit bd0d002

Please sign in to comment.