diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java index d66d97bc08..5707940356 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java @@ -37,6 +37,7 @@ import java.util.HashSet; import static com.appsmith.server.constants.Url.ACTION_COLLECTION_URL; import static com.appsmith.server.constants.Url.ACTION_URL; import static com.appsmith.server.constants.Url.APPLICATION_URL; +import static com.appsmith.server.constants.Url.ASSET_URL; import static com.appsmith.server.constants.Url.PAGE_URL; import static com.appsmith.server.constants.Url.TENANT_URL; import static com.appsmith.server.constants.Url.THEME_URL; @@ -125,6 +126,7 @@ public class SecurityConfig { ServerWebExchangeMatchers.pathMatchers(HttpMethod.PUT, USER_URL + "/invite/confirm"), ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, USER_URL + "/me"), ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, USER_URL + "/features"), + ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, ASSET_URL + "/*"), ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, ACTION_URL + "/**"), ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, ACTION_COLLECTION_URL + "/view"), ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, PAGE_URL + "/**"), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/InstanceAdminControllerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/InstanceAdminControllerCE.java index 56282a0e43..22497e6da5 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/InstanceAdminControllerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/InstanceAdminControllerCE.java @@ -8,6 +8,7 @@ import com.appsmith.server.solutions.EnvManager; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.PutMapping; @@ -39,15 +40,26 @@ public class InstanceAdminControllerCE { return envManager.download(exchange); } - @PutMapping("/env") - public Mono> saveEnvChanges( + @Deprecated + @PutMapping(value = "/env", consumes = {MediaType.APPLICATION_JSON_VALUE}) + public Mono> saveEnvChangesJSON( @Valid @RequestBody Map changes ) { - log.debug("Applying env updates {}", changes); + log.debug("Applying env updates {}", changes.keySet()); return envManager.applyChanges(changes) .map(res -> new ResponseDTO<>(HttpStatus.OK.value(), res, null)); } + @PutMapping(value = "/env", consumes = {MediaType.MULTIPART_FORM_DATA_VALUE}) + public Mono> saveEnvChangesMultipartFormData( + ServerWebExchange exchange + ) { + log.debug("Applying env updates from form data"); + return exchange.getMultipartData() + .flatMap(envManager::applyChangesFromMultipartFormData) + .map(res -> new ResponseDTO<>(HttpStatus.OK.value(), res, null)); + } + @PostMapping("/restart") public Mono> restart() { log.debug("Received restart request"); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCE.java index 579d659de5..986aab1dcf 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCE.java @@ -5,11 +5,13 @@ import org.springframework.http.codec.multipart.Part; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Mono; +import java.util.List; + public interface AssetServiceCE { Mono getById(String id); - Mono upload(Part filePart, int i, boolean isThumbnail); + Mono upload(List fileParts, int i, boolean isThumbnail); Mono remove(String assetId); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCEImpl.java index 3e44625100..bf97540bf2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AssetServiceCEImpl.java @@ -24,7 +24,10 @@ import java.awt.*; import java.awt.image.BufferedImage; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.util.List; +import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import static com.appsmith.server.constants.Constraint.THUMBNAIL_PHOTO_DIMENSION; @@ -36,7 +39,13 @@ public class AssetServiceCEImpl implements AssetServiceCE { private final AnalyticsService analyticsService; - private static final Set ALLOWED_CONTENT_TYPES = Set.of(MediaType.IMAGE_JPEG, MediaType.IMAGE_PNG); + private static final Set ALLOWED_CONTENT_TYPES = Set.of( + MediaType.IMAGE_JPEG, + MediaType.IMAGE_PNG, + MediaType.valueOf("image/svg+xml"), + MediaType.valueOf("image/x-icon"), + MediaType.valueOf("image/vnd.microsoft.icon") + ); @Override public Mono getById(String id) { @@ -44,29 +53,32 @@ public class AssetServiceCEImpl implements AssetServiceCE { } @Override - public Mono upload(Part filePart, int maxFileSizeKB, boolean isThumbnail) { - if (filePart == null) { + public Mono upload(List fileParts, int maxFileSizeKB, boolean isThumbnail) { + fileParts = fileParts.stream().filter(Objects::nonNull).collect(Collectors.toList()); + + if (fileParts.isEmpty()) { return Mono.error(new AppsmithException(AppsmithError.VALIDATION_FAILURE, "Please upload a valid image.")); } + final Part firstPart = fileParts.get(0); + // The reason we restrict file types here is to avoid having to deal with dangerous image types such as SVG, - // which can have arbitrary HTML/JS inside of them. - final MediaType contentType = filePart.headers().getContentType(); + // which can have arbitrary HTML/JS inside them. + final MediaType contentType = firstPart.headers().getContentType(); if (contentType == null || !ALLOWED_CONTENT_TYPES.contains(contentType)) { return Mono.error(new AppsmithException( AppsmithError.VALIDATION_FAILURE, - "Please upload a valid image. Only JPEG and PNG are allowed." + "Please upload a valid image. Only JPEG, PNG, SVG and ICO are allowed." )); } - final Flux contentCache = filePart.content().cache(); + final Flux contentCache = Flux.fromIterable(fileParts).flatMap(Part::content).cache(); - return contentCache.count() - .defaultIfEmpty(0L) - .flatMap(count -> { - // Default implementation for the BufferFactory used breaks down the FilePart into chunks of 4KB. - // So we multiply the count of chunks with 4 to get an estimate on the file size in KB. - if (4 * count > maxFileSizeKB) { + return contentCache + .map(DataBuffer::readableByteCount) + .reduce(Integer::sum) + .flatMap(size -> { + if (size > maxFileSizeKB * 1024) { return Mono.error(new AppsmithException(AppsmithError.PAYLOAD_TOO_LARGE, maxFileSizeKB)); } return DataBufferUtils.join(contentCache); @@ -99,17 +111,27 @@ public class AssetServiceCEImpl implements AssetServiceCE { } private Asset createAsset(DataBuffer dataBuffer, MediaType srcContentType, boolean createThumbnail) throws IOException { - byte[] imageData; - MediaType contentType; + byte[] imageData = null; + MediaType contentType = srcContentType; - if(createThumbnail) { - imageData = resizeImage(dataBuffer); + if (createThumbnail) { + try { + imageData = resizeImage(dataBuffer); + } finally { + // The `resizeImage` function, calls `ImageIO.read` which changes the read position of this `dataBuffer`. + // This becomes a problem for us, since we attempt to read it ourselves, if the image is not resized. + dataBuffer.readPosition(0); + } + } + + if (imageData != null) { + // A JPEG thumbnail creation was successful. contentType = MediaType.IMAGE_JPEG; } else { imageData = new byte[dataBuffer.readableByteCount()]; dataBuffer.read(imageData); - contentType = srcContentType; } + DataBufferUtils.release(dataBuffer); return new Asset(contentType, imageData); } @@ -117,6 +139,10 @@ public class AssetServiceCEImpl implements AssetServiceCE { private byte[] resizeImage(DataBuffer dataBuffer) throws IOException { int dimension = THUMBNAIL_PHOTO_DIMENSION; BufferedImage bufferedImage = ImageIO.read(dataBuffer.asInputStream()); + if (bufferedImage == null) { + // This is true for SVG and ICO images. + return null; + } Image scaledImage = bufferedImage.getScaledInstance(dimension, dimension, Image.SCALE_SMOOTH); BufferedImage imageBuff = new BufferedImage(dimension, dimension, BufferedImage.TYPE_INT_RGB); imageBuff.getGraphics().drawImage(scaledImage, 0, 0, new Color(0,0,0), null); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java index f84fab80d9..e2fb69cc30 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java @@ -204,7 +204,7 @@ public class UserDataServiceCEImpl extends BaseService prevAssetIdMono = getForCurrentUser() .map(userData -> ObjectUtils.defaultIfNull(userData.getProfilePhotoAssetId(), "")); - final Mono uploaderMono = assetService.upload(filePart, MAX_PROFILE_PHOTO_SIZE_KB, true); + final Mono uploaderMono = assetService.upload(List.of(filePart), MAX_PROFILE_PHOTO_SIZE_KB, true); return Mono.zip(prevAssetIdMono, uploaderMono) .flatMap(tuple -> { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java index a0337bf5cf..348abeab0b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/WorkspaceServiceCEImpl.java @@ -529,11 +529,15 @@ public class WorkspaceServiceCEImpl extends BaseService uploadLogo(String workspaceId, Part filePart) { + if (filePart == null) { + return Mono.error(new AppsmithException(AppsmithError.VALIDATION_FAILURE, "Please upload a valid image.")); + } + final Mono findWorkspaceMono = repository.findById(workspaceId, workspacePermission.getEditPermission()) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.WORKSPACE, workspaceId))); // We don't execute the upload Mono if we don't find the workspace. - final Mono uploadAssetMono = assetService.upload(filePart, Constraint.WORKSPACE_LOGO_SIZE_KB, false); + final Mono uploadAssetMono = assetService.upload(List.of(filePart), Constraint.WORKSPACE_LOGO_SIZE_KB, false); return findWorkspaceMono .flatMap(workspace -> Mono.zip(Mono.just(workspace), uploadAssetMono)) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCE.java index 1d5554739a..2e337fe7aa 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCE.java @@ -3,6 +3,8 @@ package com.appsmith.server.solutions.ce; import com.appsmith.server.domains.User; import com.appsmith.server.dtos.EnvChangesResponseDTO; import com.appsmith.server.dtos.TestEmailConfigRequestDTO; +import org.springframework.http.codec.multipart.Part; +import org.springframework.util.MultiValueMap; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Mono; @@ -16,6 +18,12 @@ public interface EnvManagerCE { Mono applyChanges(Map changes); + Mono applyChangesFromMultipartFormData(MultiValueMap formData); + + void setAnalyticsEventAction(Map properties, String newVariable, String originalVariable, String authEnv); + + Mono> handleFileUpload(String key, List parts); + Map parseToMap(String content); Mono> getAll(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java index d5c8be34ae..73142e9264 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java @@ -30,18 +30,22 @@ import com.appsmith.server.services.UserService; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.SerializationFeature; import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; +import org.springframework.http.codec.multipart.FilePart; +import org.springframework.http.codec.multipart.Part; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.mail.MailException; import org.springframework.mail.SimpleMailMessage; import org.springframework.mail.javamail.JavaMailSender; import org.springframework.mail.javamail.JavaMailSenderImpl; +import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Flux; @@ -53,6 +57,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; @@ -184,7 +189,7 @@ public class EnvManagerCEImpl implements EnvManagerCE { variablesNotInWhitelist.removeAll(tenantConfigWhitelist); if (!variablesNotInWhitelist.isEmpty()) { - throw new AppsmithException(AppsmithError.UNAUTHORIZED_ACCESS); + throw new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST); } if (changes.containsKey(APPSMITH_MAIL_HOST.name())) { @@ -275,7 +280,7 @@ public class EnvManagerCEImpl implements EnvManagerCE { // validate input is in the format email,email,email and is not empty if (!ValidationUtils.validateEmailCsv(emailCsv)) { - return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "Admin Email")); + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "Admin Emails")); } else { // make sure user is not removing own email Set adminEmails = TextUtils.csvToSet(emailCsv); if (!adminEmails.contains(user.getEmail())) { // user can not remove own email address @@ -298,7 +303,7 @@ public class EnvManagerCEImpl implements EnvManagerCE { return Arrays.stream(fields) .map(field -> { JsonProperty jsonProperty = field.getDeclaredAnnotation(JsonProperty.class); - return jsonProperty.value(); + return jsonProperty == null ? field.getName() : jsonProperty.value(); }).collect(Collectors.toSet()); } @@ -330,7 +335,7 @@ public class EnvManagerCEImpl implements EnvManagerCE { private Mono updateTenantConfiguration(String tenantId, Map changes) { TenantConfiguration tenantConfiguration = new TenantConfiguration(); // Write the changes to the tenant collection in configuration field - return Flux.fromStream(changes.entrySet().stream()) + return Flux.fromIterable(changes.entrySet()) .map(map -> { String key = map.getKey(); String value = map.getValue(); @@ -358,7 +363,15 @@ public class EnvManagerCEImpl implements EnvManagerCE { return Mono.error(e); } Map originalVariables = parseToMap(originalContent); - final List changedContent = transformEnvContent(originalContent, changes); + + final Map envFileChanges = new HashMap<>(changes); + final Set tenantConfigurationKeys = allowedTenantConfiguration(); + for (final String key : changes.keySet()) { + if (tenantConfigurationKeys.contains(key)) { + envFileChanges.remove(key); + } + } + final List changedContent = transformEnvContent(originalContent, envFileChanges); try { Files.write(envFilePath, changedContent); @@ -446,6 +459,41 @@ public class EnvManagerCEImpl implements EnvManagerCE { }); } + @Override + public Mono applyChangesFromMultipartFormData(MultiValueMap formData) { + return Flux.fromIterable(formData.entrySet()) + .flatMap(entry -> { + final String key = entry.getKey(); + final List parts = entry.getValue(); + final boolean isFile = parts.size() > 0 && parts.get(0) instanceof FilePart; + + if (isFile) { + return handleFileUpload(key, parts); + } + + return DataBufferUtils + .join(Flux.fromIterable(parts).flatMapSequential(Part::content)) + .flatMap(dataBuffer -> { + final byte[] content; + try (InputStream inputStream = dataBuffer.asInputStream(true)) { + content = inputStream.readAllBytes(); + } catch (IOException e) { + log.error("Unable to read multipart form data, in env change API", e); + return Mono.error(new AppsmithException(AppsmithError.IO_ERROR, "unable to read data")); + } + return Mono.just(Map.entry(key, new String(content, StandardCharsets.UTF_8))); + }); + }) + .collectMap(Map.Entry::getKey, Map.Entry::getValue) + .flatMap(this::applyChanges); + } + + @Override + @NotNull + public Mono> handleFileUpload(String key, List parts) { + return Mono.error(new AppsmithException(AppsmithError.UNSUPPORTED_OPERATION, "File upload is not supported")); + } + /** * Sends analytics events after an admin setting update. * @@ -507,6 +555,7 @@ public class EnvManagerCEImpl implements EnvManagerCE { * @param originalVariable Already existing env variable value * @param authEnv Env variable name */ + @Override public void setAnalyticsEventAction(Map properties, String newVariable, String originalVariable, String authEnv) { // Authentication configuration added if (!newVariable.isEmpty() && (originalVariable == null || originalVariable.isEmpty())) { @@ -587,22 +636,7 @@ public class EnvManagerCEImpl implements EnvManagerCE { envKeyValueMap.put(APPSMITH_INSTANCE_NAME.name(), commonConfig.getInstanceName()); } - // Add the variables from the tenant configuration to be returned to the client - Mono tenantMono = tenantService.findById(user.getTenantId(), MANAGE_TENANT); - - return Mono.zip(Mono.justOrEmpty(envKeyValueMap), tenantMono) - .map(tuple -> { - Map envFileMap = tuple.getT1(); - Tenant tenant = tuple.getT2(); - Map configMap = objectMapper.convertValue(tenant.getTenantConfiguration(), new TypeReference<>() { - }); - Map envMap = new HashMap<>(); - envMap.putAll(envFileMap); - if (!CollectionUtils.isNullOrEmpty(configMap)) { - envMap.putAll(configMap); - } - return envMap; - }); + return Mono.justOrEmpty(envKeyValueMap); }); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java index 0c990f8911..2c2fa05ab9 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java @@ -272,7 +272,7 @@ public class EnvManagerTest { ) )) .matches(value -> value instanceof AppsmithException - && AppsmithError.UNAUTHORIZED_ACCESS.equals(((AppsmithException) value).getError())); + && AppsmithError.GENERIC_BAD_REQUEST.equals(((AppsmithException) value).getError())); } @Test