diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceDomain.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceDomain.java index 41c068b8d70c..add622a0fe51 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceDomain.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceDomain.java @@ -38,7 +38,9 @@ public enum FileResourceDomain { DOCUMENT("document"), MESSAGE_ATTACHMENT("messageAttachment"), USER_AVATAR("userAvatar"), - ORG_UNIT("organisationUnit"); + ORG_UNIT("organisationUnit"), + ICON("icon"), + JOB_DATA("jobData"); /** Container name to use when storing blobs of this FileResourceDomain */ private String containerName; diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceService.java index 0628d7183b5a..12e29cc41159 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceService.java @@ -129,6 +129,7 @@ public void saveFileResource(FileResource fileResource, File file) { @Override @Transactional public String saveFileResource(FileResource fileResource, byte[] bytes) { + validateFileResource(fileResource); fileResource.setStorageStatus(FileResourceStorageStatus.PENDING); fileResourceStore.save(fileResource); sessionFactory.getCurrentSession().flush(); diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/FileResourceControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/FileResourceControllerTest.java index db17a8d84ac1..5f449ffb7865 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/FileResourceControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/FileResourceControllerTest.java @@ -29,16 +29,77 @@ import static org.junit.jupiter.api.Assertions.assertEquals; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.jsontree.JsonObject; +import org.hisp.dhis.jsontree.JsonString; import org.hisp.dhis.web.HttpStatus; import org.hisp.dhis.webapi.DhisControllerConvenienceTest; import org.hisp.dhis.webapi.json.domain.JsonWebMessage; import org.junit.jupiter.api.Test; +import org.springframework.core.io.ClassPathResource; import org.springframework.mock.web.MockMultipartFile; class FileResourceControllerTest extends DhisControllerConvenienceTest { + @Test + void testSaveBadAvatarImageData() { + MockMultipartFile image = + new MockMultipartFile( + "file", "OU_profile_image.png", "image/png", "<>".getBytes()); + HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image); + JsonString errorMessage = + response.content(HttpStatus.INTERNAL_SERVER_ERROR).getString("message"); + assertEquals("Failed to resize image: src cannot be null", errorMessage.string()); + } + + @Test + void testSaveBadAvatarContentType() { + MockMultipartFile image = + new MockMultipartFile( + "file", "OU_profile_image.png", "image/tiff", "<>".getBytes()); + HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image); + JsonString errorMessage = response.content(HttpStatus.CONFLICT).getString("message"); + assertEquals( + "Invalid content type, valid content types are: image/jpeg,image/png,image/gif", + errorMessage.string()); + } + + @Test + void testSaveBadAvatarFileExtension() { + MockMultipartFile image = + new MockMultipartFile( + "file", "OU_profile_image.tiff", "image/png", "<>".getBytes()); + HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image); + JsonString errorMessage = response.content(HttpStatus.CONFLICT).getString("message"); + assertEquals( + "Wrong file extension, valid extensions are: jpg,jpeg,png,gif", errorMessage.string()); + } + + @Test + void testSaveBadAvatarFileSize() { + byte[] bytes = new byte[2_000_001]; + MockMultipartFile image = + new MockMultipartFile("file", "OU_profile_image.png", "image/png", bytes); + HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image); + JsonString errorMessage = response.content(HttpStatus.CONFLICT).getString("message"); + assertEquals( + "File size can't be bigger than 2000000, current file size 2000001", errorMessage.string()); + } + + @Test + void testSaveGoodAvatar() throws IOException { + File file = new ClassPathResource("file/dhis2.png").getFile(); + MockMultipartFile image = + new MockMultipartFile("file", "dhis2.png", "image/png", Files.readAllBytes(file.toPath())); + HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image); + JsonObject savedObject = + response.content(HttpStatus.ACCEPTED).getObject("response").getObject("fileResource"); + assertEquals("dhis2.png", savedObject.getString("name").string()); + } + @Test void testSaveOrgUnitImage() { MockMultipartFile image = diff --git a/dhis-2/dhis-test-web-api/src/test/resources/file/dhis2.png b/dhis-2/dhis-test-web-api/src/test/resources/file/dhis2.png new file mode 100644 index 000000000000..7b82d1a8f2db Binary files /dev/null and b/dhis-2/dhis-test-web-api/src/test/resources/file/dhis2.png differ diff --git a/dhis-2/dhis-web-api/pom.xml b/dhis-2/dhis-web-api/pom.xml index ecbf29550fe5..4f9cb00008af 100644 --- a/dhis-2/dhis-web-api/pom.xml +++ b/dhis-2/dhis-web-api/pom.xml @@ -339,6 +339,12 @@ classgraph + + + org.imgscalr + imgscalr-lib + + org.hamcrest diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/FileResourceController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/FileResourceController.java index c50e5af93f38..21110aeb0edc 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/FileResourceController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/FileResourceController.java @@ -30,6 +30,9 @@ import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.error; import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.notFound; import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.unauthorized; +import static org.hisp.dhis.webapi.utils.FileResourceUtils.resizeAvatarToDefaultSize; +import static org.hisp.dhis.webapi.utils.FileResourceUtils.resizeIconToDefaultSize; +import static org.hisp.dhis.webapi.utils.FileResourceUtils.validateCustomIconFile; import com.google.common.base.MoreObjects; import java.io.IOException; @@ -145,8 +148,19 @@ public WebMessage saveFileResource( @RequestParam MultipartFile file, @RequestParam(defaultValue = "DATA_VALUE") FileResourceDomain domain, @RequestParam(required = false) String uid) - throws WebMessageException, IOException { - FileResource fileResource = fileResourceUtils.saveFileResource(uid, file, domain); + throws IOException, WebMessageException { + FileResource fileResource; + + if (domain.equals(FileResourceDomain.ICON)) { + validateCustomIconFile(file); + fileResource = fileResourceUtils.saveFileResource(uid, resizeIconToDefaultSize(file), domain); + } else if (domain.equals(FileResourceDomain.USER_AVATAR)) { + fileResourceUtils.validateUserAvatar(file); + fileResource = + fileResourceUtils.saveFileResource(uid, resizeAvatarToDefaultSize(file), domain); + } else { + fileResource = fileResourceUtils.saveFileResource(uid, file, domain); + } WebMessage webMessage = new WebMessage(Status.OK, HttpStatus.ACCEPTED); webMessage.setResponse(new FileResourceWebMessageResponse(fileResource)); diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/utils/FileResourceUtils.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/utils/FileResourceUtils.java index 174ab1b1705a..a62f894ac872 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/utils/FileResourceUtils.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/utils/FileResourceUtils.java @@ -27,21 +27,33 @@ */ package org.hisp.dhis.webapi.utils; +import static org.apache.commons.io.FilenameUtils.getExtension; import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.conflict; import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.error; import static org.hisp.dhis.external.conf.ConfigurationKey.CSP_HEADER_VALUE; +import static org.imgscalr.Scalr.resize; import com.google.common.hash.Hashing; import com.google.common.io.ByteSource; +import java.awt.image.BufferedImage; import java.io.File; +import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.nio.file.Files; +import java.util.List; +import java.util.Objects; +import javax.annotation.Nonnull; +import javax.imageio.ImageIO; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.fileupload.FileItem; +import org.apache.commons.fileupload.disk.DiskFileItemFactory; import org.apache.commons.io.FilenameUtils; import org.apache.commons.io.input.NullInputStream; import org.apache.commons.lang3.StringUtils; +import org.hisp.dhis.common.IllegalQueryException; import org.hisp.dhis.dxf2.webmessage.WebMessageException; import org.hisp.dhis.external.conf.DhisConfigurationProvider; import org.hisp.dhis.feedback.ErrorCode; @@ -49,12 +61,14 @@ import org.hisp.dhis.fileresource.FileResourceDomain; import org.hisp.dhis.fileresource.FileResourceService; import org.hisp.dhis.fileresource.ImageFileDimension; +import org.imgscalr.Scalr.Mode; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; import org.springframework.stereotype.Component; import org.springframework.util.InvalidMimeTypeException; import org.springframework.util.MimeTypeUtils; import org.springframework.web.multipart.MultipartFile; +import org.springframework.web.multipart.commons.CommonsMultipartFile; /** * @author Lars Helge Overland @@ -64,6 +78,23 @@ public class FileResourceUtils { @Autowired private FileResourceService fileResourceService; + private static final List CUSTOM_ICON_VALID_ICON_EXTENSIONS = List.of("png"); + + private static final long CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES = 25_000_000; + + private static final int CUSTOM_ICON_TARGET_HEIGHT = 48; + private static final int CUSTOM_ICON_TARGET_WIDTH = 48; + + private static final int AVATAR_TARGET_HEIGHT = 200; + private static final int AVATAR_TARGET_WIDTH = 200; + + private static final long MAX_AVATAR_FILE_SIZE_IN_BYTES = 2_000_000; + + private static final List ALLOWED_AVATAR_FILE_EXTENSIONS = + List.of("jpg", "jpeg", "png", "gif"); + private static final List ALLOWED_AVATAR_MIME_TYPES = + List.of("image/jpeg", "image/png", "image/gif"); + /** * Transfers the given multipart file content to a local temporary file. * @@ -213,4 +244,94 @@ public InputStream openStream() throws IOException { } } } + + public void validateUserAvatar(@Nonnull MultipartFile file) { + validateContentType(file.getContentType(), ALLOWED_AVATAR_MIME_TYPES); + validateFileExtension(file.getOriginalFilename(), ALLOWED_AVATAR_FILE_EXTENSIONS); + validateFileSize(file, MAX_AVATAR_FILE_SIZE_IN_BYTES); + } + + private void validateContentType(String contentType, @Nonnull List validExtensions) { + if (contentType == null) { + throw new IllegalQueryException("Invalid content type, content type is NULL"); + } + contentType = contentType.split(";")[0].trim(); + if (!validExtensions.contains(contentType)) { + throw new IllegalQueryException( + "Invalid content type, valid content types are: " + String.join(",", validExtensions)); + } + } + + public static void validateCustomIconFile(MultipartFile file) { + validateFileExtension(file.getOriginalFilename(), CUSTOM_ICON_VALID_ICON_EXTENSIONS); + validateFileSize(file, CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES); + } + + private static void validateFileExtension(String fileName, List validExtension) { + if (getExtension(fileName) == null || !validExtension.contains(getExtension(fileName))) { + throw new IllegalQueryException( + "Wrong file extension, valid extensions are: " + String.join(",", validExtension)); + } + } + + private static void validateFileSize(@Nonnull MultipartFile file, long maxFileSizeInBytes) { + if (file.getSize() > maxFileSizeInBytes) { + throw new IllegalQueryException( + String.format( + "File size can't be bigger than %d, current file size %d", + maxFileSizeInBytes, file.getSize())); + } + } + + public static MultipartFile resizeImageToCustomSize( + MultipartFile multipartFile, int targetWidth, int targetHeight, Mode resizeMode) + throws IOException { + File tmpFile = null; + + try { + BufferedImage resizedImage = + resize( + ImageIO.read(multipartFile.getInputStream()), resizeMode, targetWidth, targetHeight); + + tmpFile = Files.createTempFile("org.hisp.dhis", ".tmp").toFile(); + + ImageIO.write( + resizedImage, + Objects.requireNonNull(getExtension(multipartFile.getOriginalFilename())), + tmpFile); + + FileItem fileItem = + new DiskFileItemFactory() + .createItem( + "file", + Files.probeContentType(tmpFile.toPath()), + false, + multipartFile.getOriginalFilename()); + + try (InputStream in = new FileInputStream(tmpFile); + OutputStream out = fileItem.getOutputStream()) { + in.transferTo(out); + } + + return new CommonsMultipartFile(fileItem); + } catch (Exception e) { + throw new IOException("Failed to resize image: " + e.getMessage()); + } finally { + if (tmpFile != null && tmpFile.exists()) { + Files.delete(tmpFile.toPath()); + } + } + } + + public static MultipartFile resizeIconToDefaultSize(MultipartFile multipartFile) + throws IOException { + return resizeImageToCustomSize( + multipartFile, CUSTOM_ICON_TARGET_WIDTH, CUSTOM_ICON_TARGET_HEIGHT, Mode.FIT_EXACT); + } + + public static MultipartFile resizeAvatarToDefaultSize(MultipartFile multipartFile) + throws IOException { + return resizeImageToCustomSize( + multipartFile, AVATAR_TARGET_WIDTH, AVATAR_TARGET_HEIGHT, Mode.AUTOMATIC); + } }