Skip to content

Commit

Permalink
fix(#2651): incorrect redirect to dashboard instead of app module (#2689
Browse files Browse the repository at this point in the history 
)

* fix(#2651): Fix app module and deactivate it for default installation

* fix(#2651): Refactor and deprecate AssetDashboardResource

* fix(#2651): Add sanitize file name to FileManger

* fix(#2651): Check for file type in file manager

* fix(#2651): Check for file type in AssetDashboardResource

* fix(#2651): Fix e2e test for user roles
  • Loading branch information
@tenthe
tenthe authored Apr 5, 2024
1 parent e4a004e commit 223aa0a
Show file tree
Hide file tree
Showing 10 changed files with 233 additions and 50 deletions.
55 changes: 46 additions & 9 deletions ...es-pipeline-management/src/main/java/org/apache/streampipes/manager/file/FileManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,22 +60,33 @@ public static FileMetadata storeFile(String user,
String filename,
InputStream fileInputStream) throws IOException {

String filetype = filename.substring(filename.lastIndexOf(".") + 1);
var filetype = filename.substring(filename.lastIndexOf(".") + 1);

fileInputStream = cleanFile(fileInputStream, filetype);
fileInputStream = validateFileNameAndCleanFile(filename, filetype, fileInputStream);

FileMetadata fileMetadata = makeFileMetadata(user, filename, filetype);
new FileHandler().storeFile(filename, fileInputStream);
storeFileMetadata(fileMetadata);
return fileMetadata;
var sanitizedFilename = sanitizeFilename(filename);

writeToFile(sanitizedFilename, fileInputStream);

return makeAndStoreFileMetadata(user, sanitizedFilename, filetype);
}


public static void deleteFile(String id) {
FileMetadata fileMetadata = getFileMetadataStorage().getMetadataById(id);
new FileHandler().deleteFile(fileMetadata.getFilename());
getFileMetadataStorage().deleteFileMetadata(id);
}

private static InputStream validateFileNameAndCleanFile(String filename,
String filetype,
InputStream fileInputStream) {
if (!validateFileType(filename)) {
throw new IllegalArgumentException("Filetype for file %s not allowed".formatted(filename));
}

return cleanFile(fileInputStream, filetype);
}

/**
* Remove Byte Order Mark (BOM) from csv files
Expand All @@ -84,7 +95,7 @@ public static void deleteFile(String id) {
* @param filetype file of type
* @return input stream without BOM
*/
public static InputStream cleanFile(InputStream fileInputStream, String filetype) {
protected static InputStream cleanFile(InputStream fileInputStream, String filetype) {
if (Filetypes.CSV.getFileExtensions().contains(filetype.toLowerCase())) {
fileInputStream = new BOMInputStream(fileInputStream);
}
Expand All @@ -97,17 +108,37 @@ public static boolean checkFileContentChanged(String filename, String hash) thro
return !fileHash.equals(hash);
}

private static void storeFileMetadata(FileMetadata fileMetadata) {
getFileMetadataStorage().addFileMetadata(fileMetadata);
public static String sanitizeFilename(String filename) {
return filename.replaceAll("[^a-zA-Z0-9.\\-]", "_");
}

public static boolean validateFileType(String filename) {
return Filetypes.getAllFileExtensions()
.stream()
.anyMatch(filename::endsWith);
}

protected static void writeToFile(String sanitizedFilename, InputStream fileInputStream) throws IOException {
new FileHandler().storeFile(sanitizedFilename, fileInputStream);
}


private static IFileMetadataStorage getFileMetadataStorage() {
return StorageDispatcher
.INSTANCE
.getNoSqlStore()
.getFileMetadataStorage();
}

protected static FileMetadata makeAndStoreFileMetadata(String user,
String sanitizedFilename,
String filetype) {
var fileMetadata = makeFileMetadata(user, sanitizedFilename, filetype);
storeFileMetadata(fileMetadata);

return fileMetadata;
}

private static FileMetadata makeFileMetadata(String user,
String filename,
String filetype) {
Expand All @@ -121,6 +152,11 @@ private static FileMetadata makeFileMetadata(String user,
return fileMetadata;
}

private static void storeFileMetadata(FileMetadata fileMetadata) {
getFileMetadataStorage().addFileMetadata(fileMetadata);
}


private static List<FileMetadata> filterFiletypes(List<FileMetadata> allFiles, String filetypes) {
return allFiles
.stream()
Expand All @@ -129,4 +165,5 @@ private static List<FileMetadata> filterFiletypes(List<FileMetadata> allFiles, S
.anyMatch(ft -> ft.equals(fileMetadata.getFiletype())))
.collect(Collectors.toList());
}

}
112 changes: 91 additions & 21 deletions ...ipeline-management/src/test/java/org/apache/streampipes/manager/file/TestFileManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,46 +18,116 @@
package org.apache.streampipes.manager.file;

import org.apache.commons.io.IOUtils;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;

public class TestFileManager {

@Test
public void storeFile_throwsExceptionForInvalidFileType() {
var filename = "testFile.invalid";

assertThrows(IllegalArgumentException.class, () ->
FileManager.storeFile("", filename, mock(InputStream.class)));
}

@Test
public void testCleanFileWithoutBom() throws IOException {
String expected = "test";
InputStream inputStream = IOUtils.toInputStream(expected, StandardCharsets.UTF_8);
InputStream resultStream = FileManager.cleanFile(inputStream, "CSV");
String resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8);
var expected = "test";
var inputStream = IOUtils.toInputStream(expected, StandardCharsets.UTF_8);
var resultStream = FileManager.cleanFile(inputStream, "CSV");
var resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8);

Assertions.assertEquals(expected, resultString);
assertEquals(expected, resultString);
}

@Test
public void testCleanFileWithBom() throws IOException {
String expected = "test";
String utf8Bom = "\uFEFF";
String inputString = utf8Bom + expected;
InputStream inputStream = IOUtils.toInputStream(inputString, StandardCharsets.UTF_8);
InputStream resultStream = FileManager.cleanFile(inputStream, "CSV");
String resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8);
var expected = "test";
var utf8Bom = "\uFEFF";
var inputString = utf8Bom + expected;
var inputStream = IOUtils.toInputStream(inputString, StandardCharsets.UTF_8);
var resultStream = FileManager.cleanFile(inputStream, "CSV");
var resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8);

Assertions.assertEquals(expected, resultString);
assertEquals(expected, resultString);
}

@Test
public void testCleanFileWithBomAndUmlauts() throws IOException {
String expected = "testäüö";
String utf8Bom = "\uFEFF";
String inputString = utf8Bom + expected;
InputStream inputStream = IOUtils.toInputStream(inputString, StandardCharsets.UTF_8);
InputStream resultStream = FileManager.cleanFile(inputStream, "CSV");
String resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8);

Assertions.assertEquals(expected, resultString);
var expected = "testäüö";
var utf8Bom = "\uFEFF";
var inputString = utf8Bom + expected;
var inputStream = IOUtils.toInputStream(inputString, StandardCharsets.UTF_8);
var resultStream = FileManager.cleanFile(inputStream, "CSV");
var resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8);

assertEquals(expected, resultString);
}

@Test
public void sanitizeFilename_replacesNonAlphanumericCharactersWithUnderscore() {
var filename = "file@name#with$special%characters";
var sanitizedFilename = FileManager.sanitizeFilename(filename);
assertEquals("file_name_with_special_characters", sanitizedFilename);
}

@Test
public void sanitizeFilename_keepsAlphanumericAndDotAndHyphenCharacters() {
var filename = "file.name-with_alphanumeric123";
var sanitizedFilename = FileManager.sanitizeFilename(filename);
assertEquals(filename, sanitizedFilename);
}

@Test
public void sanitizeFilename_returnsUnderscoreForFilenameWithAllSpecialCharacters() {
var filename = "@#$%^&*()";
var sanitizedFilename = FileManager.sanitizeFilename(filename);
assertEquals("_________", sanitizedFilename);
}

@Test
public void sanitizeFilename_returnsEmptyStringForEmptyFilename() {
var filename = "";
var sanitizedFilename = FileManager.sanitizeFilename(filename);
assertEquals("", sanitizedFilename);
}

@Test
public void sanitizeFilename_removesSingleParentDirectory() {
var filename = "../file.csv";
var sanitizedFilename = FileManager.sanitizeFilename(filename);
assertEquals(".._file.csv", sanitizedFilename);
}

@Test
public void sanitizeFilename_removesDoubleParentDirectoryy() {
var filename = "../../file";
var sanitizedFilename = FileManager.sanitizeFilename(filename);
assertEquals(".._.._file", sanitizedFilename);
}

@Test
public void validateFileName_returnsTrueForCsv() {
assertTrue(FileManager.validateFileType("file.csv"));
}

@Test
public void validateFileName_returnsTrueForJson() {
assertTrue(FileManager.validateFileType("file.json"));
}

@Test
public void validateFileName_returnsFalseForSh() {
assertFalse(FileManager.validateFileType("file.sh"));
}
}
57 changes: 44 additions & 13 deletions streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/AssetDashboardResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package org.apache.streampipes.rest.impl;

import org.apache.streampipes.manager.file.FileManager;
import org.apache.streampipes.model.client.assetdashboard.AssetDashboardConfig;
import org.apache.streampipes.rest.core.base.impl.AbstractRestResource;
import org.apache.streampipes.rest.shared.exception.SpMessageException;
Expand All @@ -43,6 +44,7 @@

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.FileNameMap;
import java.net.URLConnection;
import java.nio.file.Files;
Expand All @@ -51,6 +53,7 @@

@RestController
@RequestMapping("/api/v2/asset-dashboards")
@Deprecated(forRemoval = true, since = "0.95.0")
public class AssetDashboardResource extends AbstractRestResource {

private static final Logger LOG = LoggerFactory.getLogger(AssetDashboardResource.class);
Expand All @@ -59,44 +62,52 @@ public class AssetDashboardResource extends AbstractRestResource {

@GetMapping(path = "/{dashboardId}", produces = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<AssetDashboardConfig> getAssetDashboard(@PathVariable("dashboardId") String dashboardId) {
return ok(getNoSqlStorage().getAssetDashboardStorage().getAssetDashboard(dashboardId));
return ok(getNoSqlStorage().getAssetDashboardStorage()
.getAssetDashboard(dashboardId));
}

@PutMapping(
path = "/{dashboardId}",
produces = MediaType.APPLICATION_JSON_VALUE,
consumes = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<Void> updateAssetDashboard(@PathVariable("dashboardId") String dashboardId,
@RequestBody AssetDashboardConfig dashboardConfig) {
public ResponseEntity<Void> updateAssetDashboard(
@PathVariable("dashboardId") String dashboardId,
@RequestBody AssetDashboardConfig dashboardConfig
) {
AssetDashboardConfig dashboard = getAssetDashboardStorage().getAssetDashboard(dashboardId);
dashboardConfig.setRev(dashboard.getRev());
getNoSqlStorage().getAssetDashboardStorage().updateAssetDashboard(dashboardConfig);
getNoSqlStorage().getAssetDashboardStorage()
.updateAssetDashboard(dashboardConfig);
return ok();
}

@GetMapping(produces = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<List<AssetDashboardConfig>> getAllDashboards() {
return ok(getNoSqlStorage().getAssetDashboardStorage().getAllAssetDashboards());
return ok(getNoSqlStorage().getAssetDashboardStorage()
.getAllAssetDashboards());
}

@PostMapping(
produces = MediaType.APPLICATION_JSON_VALUE,
consumes = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<Void> storeAssetDashboard(@RequestBody AssetDashboardConfig dashboardConfig) {
getNoSqlStorage().getAssetDashboardStorage().storeAssetDashboard(dashboardConfig);
getNoSqlStorage().getAssetDashboardStorage()
.storeAssetDashboard(dashboardConfig);
return ok();
}

@DeleteMapping(path = "/{dashboardId}")
public ResponseEntity<Void> deleteAssetDashboard(@PathVariable("dashboardId") String dashboardId) {
getNoSqlStorage().getAssetDashboardStorage().deleteAssetDashboard(dashboardId);
getNoSqlStorage().getAssetDashboardStorage()
.deleteAssetDashboard(dashboardId);
return ok();
}

@GetMapping(path = "/images/{imageName}")
public ResponseEntity<byte[]> getDashboardImage(@PathVariable("imageName") String imageName) {
try {
java.nio.file.Path path = Paths.get(getTargetFile(imageName));
var sanitizedFileName = FileManager.sanitizeFilename(imageName);
java.nio.file.Path path = Paths.get(getTargetFile(sanitizedFileName));
File file = new File(path.toString());
FileNameMap fileNameMap = URLConnection.getFileNameMap();
String mimeType = fileNameMap.getContentTypeFor(file.getName());
Expand All @@ -114,16 +125,35 @@ public ResponseEntity<byte[]> getDashboardImage(@PathVariable("imageName") Strin

@PostMapping(path = "/images", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
public ResponseEntity<Void> storeDashboardImage(@RequestPart("file_upload") MultipartFile fileDetail) {
File targetDirectory = new File(getTargetDirectory());
try {
var fileName = fileDetail.getName();

if (!FileManager.validateFileType(fileName)) {
LOG.error("File type of file " + fileName + " is not supported");
fail();
}

storeFile(fileDetail.getName(), fileDetail.getInputStream());
return ok();
} catch (IOException e) {
LOG.error("Could not extract image input stream from request", e);
return fail();
}
}

private void storeFile(String fileName, InputStream fileInputStream) {

var targetDirectory = new File(getTargetDirectory());
if (!targetDirectory.exists()) {
targetDirectory.mkdirs();
}

File targetFile = new File(getTargetFile(fileDetail.getName()));
var sanitizedFileName = FileManager.sanitizeFilename(fileName);

var targetFile = new File(getTargetFile(sanitizedFileName));

try {
FileUtils.copyInputStreamToFile(fileDetail.getInputStream(), targetFile);
return ok();
FileUtils.copyInputStreamToFile(fileInputStream, targetFile);
} catch (IOException e) {
LOG.error(e.getMessage(), e);
throw new SpMessageException(HttpStatus.INTERNAL_SERVER_ERROR, e);
Expand All @@ -140,6 +170,7 @@ private String getTargetFile(String filename) {
}

private IAssetDashboardStorage getAssetDashboardStorage() {
return StorageDispatcher.INSTANCE.getNoSqlStore().getAssetDashboardStorage();
return StorageDispatcher.INSTANCE.getNoSqlStore()
.getAssetDashboardStorage();
}
}
Loading

0 comments on commit 223aa0a

Please sign in to comment.