Skip to content

Commit

Permalink
MID-7219: Fixed downloading / deleting reports on remote cluster node
Browse files Browse the repository at this point in the history 
  - Fixed incorrect content-type added by Spring
  - File lookup takes into account report type (import, export, trace)
    to lookup file for serving / deletion
  • Loading branch information
@tonydamage
tonydamage committed Oct 7, 2021
1 parent 17493bd commit 6334ccb
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 8 deletions.
31 changes: 28 additions & 3 deletions model/report-impl/src/main/java/com/evolveum/midpoint/report/impl/ReportManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import com.evolveum.midpoint.report.api.ReportService;
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.schema.SelectorOptions;
import com.evolveum.midpoint.schema.constants.ObjectTypes;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.result.OperationResultStatus;
import com.evolveum.midpoint.schema.util.MiscSchemaUtil;
Expand Down Expand Up @@ -399,11 +398,12 @@ public void deleteReportData(ReportDataType reportData, OperationResult parentRe
LOGGER.error("Couldn't delete report file {}", file);
}
} else {
String fileName = checkFileName(file, result);
String fileName = remoteFileName(reportData, null, file, task, result);
if (fileName == null) {
return;
}
String originalNodeId = reportData.getNodeRef() != null ? reportData.getNodeRef().getOid() : null;

clusterExecutionHelper.executeWithFallback(originalNodeId,
(client, node, result1) -> {
client.path(ModelPublicConstants.CLUSTER_REPORT_FILE_PATH);
Expand Down Expand Up @@ -451,13 +451,17 @@ public InputStream getReportDataStream(String reportDataOid, OperationResult par
// and should not depend on user privileges.

try {
// MID-7219: We need report type in case of cluster to look for file in correct remote folders
String reportType = null;

ReportDataType reportOutput = modelService.getObject(ReportDataType.class, reportDataOid, null, task,
result).asObjectable();

// Extra safety check: traces can be retrieved only when special authorization is present
if (ObjectTypeUtil.hasArchetype(reportOutput, SystemObjectsType.ARCHETYPE_TRACE.value())) {
securityEnforcer.authorize(ModelAuthorizationAction.READ_TRACE.getUrl(), null,
AuthorizationParameters.EMPTY, null, task, result);
reportType = "trace";
}

String filePath = reportOutput.getFilePath();
Expand All @@ -469,7 +473,7 @@ public InputStream getReportDataStream(String reportDataOid, OperationResult par
if (file.exists()) {
return FileUtils.openInputStream(file);
} else {
String fileName = checkFileName(file, result);
String fileName = remoteFileName(reportOutput, reportType, file, task, result);
if (fileName == null) {
return null;
}
Expand Down Expand Up @@ -530,6 +534,27 @@ public InputStream getReportDataStream(String reportDataOid, OperationResult par
}
}

private String remoteFileName(ReportDataType reportOutput, String reportType, File file, Task task, OperationResult result) throws ObjectNotFoundException, SchemaException, SecurityViolationException, CommunicationException, ConfigurationException, ExpressionEvaluationException {
String localFileName = checkFileName(file, result);
if (localFileName == null) {
return null;
}

if (reportType == null && reportOutput.getReportRef() != null && reportOutput.getReportRef().getOid() != null) {
try {
ReportType report = modelService.getObject(ReportType.class, reportOutput.getReportRef().getOid(), null, task, result).asObjectable();
// If direction is import
if (report.getBehavior() != null && DirectionTypeType.IMPORT.equals(report.getBehavior().getDirection())) {
reportType = "import";
}
} catch (ObjectNotFoundException e) {
// NOOP
}

}
return reportType != null ? (reportType + "/" + localFileName) : localFileName;
}

@Nullable
private String checkFileName(File file, OperationResult result) {
String fileName = file.getName();
Expand Down
49 changes: 44 additions & 5 deletions model/rest-impl/src/main/java/com/evolveum/midpoint/rest/impl/ClusterRestController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import java.io.File;
import java.io.FileInputStream;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.List;
Expand All @@ -17,9 +18,9 @@
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.schema.SelectorOptions;
import com.evolveum.midpoint.xml.ns._public.common.common_3.TaskType;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
Expand Down Expand Up @@ -247,7 +248,11 @@ public ResponseEntity<?> getReportFile(
if (resolution.status == null) {
// we are only interested in the content, not in its type nor length
// TODO how to test this?
response = ResponseEntity.ok(new FileInputStream(resolution.file));
response = ResponseEntity.ok()
// Explicitly needed, because Spring would change content type to "*"
// even if application/octet-stream is in produces section
.contentType(MediaType.APPLICATION_OCTET_STREAM)
.body(new FileInputStream(resolution.file));
} else {
response = ResponseEntity.status(resolution.status).build();
}
Expand Down Expand Up @@ -311,12 +316,26 @@ static class FileResolution {

private FileResolution resolveFile(String fileName) {
FileResolution rv = new FileResolution();
rv.file = Paths.get(midpointConfiguration.getMidpointHome(), EXPORT_DIR, fileName).toFile();

if (forbiddenFileName(fileName)) {
logger.warn("File name '{}' is forbidden", fileName);
rv.status = HttpStatus.FORBIDDEN;
} else if (!rv.file.exists()) {
return rv;
}

String[] parts = fileName.split("/");
Path directory;
if (parts.length == 1) {
directory = reportDirectory("export");
} else if (parts.length == 2) {
directory = reportDirectory(parts[0]);
fileName = parts[1];
} else {
logger.warn("Report output file '{}' is a nested file", fileName);
rv.status = HttpStatus.FORBIDDEN;
return rv;
}
rv.file = directory.resolve(fileName).toFile();
if (!rv.file.exists()) {
logger.warn("Report output file '{}' does not exist", rv.file);
rv.status = HttpStatus.NOT_FOUND;
} else if (rv.file.isDirectory()) {
Expand All @@ -326,6 +345,26 @@ private FileResolution resolveFile(String fileName) {
return rv;
}

/**
* @return trace directory if selector is "trace, import directory if selector is "import", export directory
* otherwise
*/
private Path reportDirectory(String selector) {

if ("trace".equals(selector)) {
return Paths.get(midpointConfiguration.getMidpointHome(), "trace");
}
String configKey = "import".equals(selector) ? "importFolder" : "exportFolder";
String directory = midpointConfiguration.getConfiguration(MidpointConfiguration.WEB_APP_CONFIGURATION).getString(configKey);
if (directory == null || directory.isEmpty()) {
directory = EXPORT_DIR;
}
if (directory.startsWith("/")) {
return Paths.get(directory);
}
return Paths.get(midpointConfiguration.getMidpointHome(), directory);
}

private boolean forbiddenFileName(String fileName) {
return fileName.contains("/../");
}
Expand Down

0 comments on commit 6334ccb

Please sign in to comment.