From 8e30fa72240c6eafa7e379ce5dfffe446017d540 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Thu, 11 Apr 2024 17:21:00 +0530 Subject: [PATCH 01/13] fix: Don't send events with `null` to Segment (#32600) Recently, we made two fixes to `SegmentConfig` to fix NPEs that were preventing some events from being sent to Segment. 1. https://github.com/appsmithorg/appsmith/pull/32498 2. https://github.com/appsmithorg/appsmith/pull/32351 But this ended up sending _too much_ to Segment now. That, and clearly we weren't missing anything by not sending those events with `null` in them. So we're bringing back that protection of NPEs. The protection of not sending value-less events to Segment. But in a more, educated way. We're adding `null` checks, and not sending anything when we see a `null`. Considering that context, a more accurate diff to review would be between the [`SegmentConfig` before those two PRs](https://github.com/appsmithorg/appsmith/blob/d6e74bf0122b0e5a91e175c3d2a70fef9fd68bb1/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java), and the `SegmentConfig` in this PR. This diff can be seen here. (cherry picked from commit c8316447643594ff20f1ab9ddb9cc340ac1200eb) --- .../server/configurations/SegmentConfig.java | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java index c2b72d52b4..54199ed44b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java @@ -6,7 +6,7 @@ import com.segment.analytics.messages.TrackMessage; import lombok.Data; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang3.ObjectUtils; +import org.apache.commons.lang.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; import org.springframework.beans.factory.annotation.Autowired; @@ -60,18 +60,29 @@ public class SegmentConfig { .build(); logProcessorWithErrorHandler.onError(logData -> { final Throwable error = logData.getError(); - final String message = ObjectUtils.defaultIfNull(error == null ? null : error.getMessage(), ""); - final Object args = ObjectUtils.defaultIfNull(logData.getArgs(), Collections.emptyList()); - final String stackTrace = - ObjectUtils.defaultIfNull(error == null ? null : ExceptionUtils.getStackTrace(error), ""); + + if (logData.getMessage() == null) { + log.error("Message is null for log data: {}", logData); + return; + } + + if (error == null) { + log.error("Error is null for log: {}", logData.getMessage()); + return; + } + + if (error.getMessage() == null) { + log.error("Error message is null for log: {}", logData.getMessage()); + return; + } analyticsOnAnalytics.enqueue(TrackMessage.builder("segment_error") .userId("segmentError") .properties(Map.of( - "message", ObjectUtils.defaultIfNull(logData.getMessage(), ""), - "error", message, - "args", args, - "stackTrace", stackTrace))); + "message", logData.getMessage(), + "error", error.getMessage(), + "args", ObjectUtils.defaultIfNull(logData.getArgs(), Collections.emptyList()), + "stackTrace", ExceptionUtils.getStackTrace(error)))); }); return analytics; From 9b2f186e141b6633dcdab6ff2de6e21dd8892f13 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Mon, 15 Apr 2024 12:10:43 +0530 Subject: [PATCH 02/13] chore: Revert all recent changes to SegmentConfig (cherry picked from commit 39fae54f67819d64011979f33b6945399e1e07ef) --- .../server/configurations/SegmentConfig.java | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java index 54199ed44b..d1663a30c0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SegmentConfig.java @@ -60,27 +60,11 @@ public class SegmentConfig { .build(); logProcessorWithErrorHandler.onError(logData -> { final Throwable error = logData.getError(); - - if (logData.getMessage() == null) { - log.error("Message is null for log data: {}", logData); - return; - } - - if (error == null) { - log.error("Error is null for log: {}", logData.getMessage()); - return; - } - - if (error.getMessage() == null) { - log.error("Error message is null for log: {}", logData.getMessage()); - return; - } - analyticsOnAnalytics.enqueue(TrackMessage.builder("segment_error") .userId("segmentError") .properties(Map.of( "message", logData.getMessage(), - "error", error.getMessage(), + "error", error == null ? "" : error.getMessage(), "args", ObjectUtils.defaultIfNull(logData.getArgs(), Collections.emptyList()), "stackTrace", ExceptionUtils.getStackTrace(error)))); }); From 239f573755d97780977244250872ad9ed5ecbf61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csneha122=E2=80=9D?= <β€œsneha@appsmith.com”> Date: Thu, 18 Apr 2024 18:27:26 +0530 Subject: [PATCH 03/13] fix: graphQL bindings issue resolved (#32760) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR resolved issue with graphQL, where if we create a graphQL with a binding and run such query, it returns with error response saying the query execution has timed out. A user had raised this issue [here](https://discord.com/channels/725602949748752515/1230134890704539709). The issue was not at all reproducible on local, but It was continuously erroring out on production, release and even DPs. From the logs, we can see that issue was with `GraphQLDataTypeUtils.java` class. This class was somehow not getting initialised. This was due to recent changes made in [PR](https://github.com/appsmithorg/appsmith/pull/32595), where the version for package was updated to v2.17.0, on local it was compiling properly but on release, production and other builds, it still seems to be referencing to the older version of it, we do not know the reason of it right now, we should further investigate this. But since this was a critical issue and we need to get user unblocked, we are going ahead with an alternate fix where we have moved the initialisation of objectMapper to serialisationUtils.java file. ![Screenshot 2024-04-18 at 11 22 55 AM](https://github.com/appsmithorg/appsmith/assets/30018882/9735aaed-07c2-402b-a798-d7a4c1b67a19) 1. Drag and drop a text widget on canvas, change text value to `US` 2. Create a new graphQL query with url as `https://countries.trevorblades.com/` 3. Add following in the query body and execute the API ``` { country(code: {{Text1.text}}) { name } } ``` Fixes #32748 _or_ Fixes [`Issue URL`](https://github.com/appsmithorg/appsmith/issues/32748) > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ /ok-to-test tags="@tag.All" > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: a03f29f8078398a4c317e852667270c0810bddea > Cypress dashboard url: Click here! - **Refactor** - Updated serialization configuration across various data types and plugins to enhance functionality and compatibility using a centralized utility method. - **New Features** - Introduced a new method to standardize object mapper configurations, improving serialization processes across the application. --------- Co-authored-by: β€œsneha122” <β€œsneha@appsmith.com”> --- .../main/java/com/appsmith/external/datatypes/ArrayType.java | 5 ++--- .../main/java/com/appsmith/external/datatypes/DateType.java | 5 ++--- .../java/com/appsmith/external/datatypes/JsonObjectType.java | 5 ++--- .../java/com/appsmith/external/datatypes/StringType.java | 5 ++--- .../main/java/com/appsmith/external/datatypes/TimeType.java | 5 ++--- .../java/com/appsmith/external/datatypes/TimestampType.java | 5 ++--- .../com/appsmith/external/helpers/DataTypeStringUtils.java | 5 ++--- .../main/java/com/appsmith/external/helpers/PluginUtils.java | 5 ++--- .../external/helpers/restApiUtils/helpers/DataUtils.java | 4 ++-- .../main/java/com/appsmith/external/plugins/BasePlugin.java | 5 ++--- .../appsmith/external/services/ce/FilterDataServiceCE.java | 4 ++-- .../src/main/java/com/appsmith/util/SerializationUtils.java | 4 ++++ .../main/java/com/external/utils/WhereConditionUtils.java | 5 ++--- .../com/external/config/GetDatasourceMetadataMethod.java | 5 ++--- .../main/java/com/external/utils/GraphQLDataTypeUtils.java | 5 ++--- .../java/com/external/plugins/commands/MongoCommand.java | 5 ++--- .../com/external/plugins/datatypes/MySQLDateTimeType.java | 5 ++--- .../src/main/java/com/external/plugins/SaasPlugin.java | 5 ++--- 18 files changed, 38 insertions(+), 49 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/ArrayType.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/ArrayType.java index f9b853a85f..b085bfcdec 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/ArrayType.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/ArrayType.java @@ -3,8 +3,8 @@ package com.appsmith.external.datatypes; import com.appsmith.external.constants.DataType; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import net.minidev.json.JSONArray; import net.minidev.json.parser.JSONParser; @@ -12,8 +12,7 @@ import reactor.core.Exceptions; public class ArrayType implements AppsmithType { - private static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); @Override public boolean test(String s) { diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/DateType.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/DateType.java index 596cfcdf68..2d1e479900 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/DateType.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/DateType.java @@ -3,8 +3,8 @@ package com.appsmith.external.datatypes; import com.appsmith.external.constants.DataType; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import reactor.core.Exceptions; @@ -16,8 +16,7 @@ import java.util.regex.Matcher; public class DateType implements AppsmithType { - private static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); @Override public boolean test(String s) { diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/JsonObjectType.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/JsonObjectType.java index 6fd9a92adf..77a64b1a04 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/JsonObjectType.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/JsonObjectType.java @@ -3,8 +3,8 @@ package com.appsmith.external.datatypes; import com.appsmith.external.constants.DataType; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.gson.Gson; import com.google.gson.JsonObject; @@ -22,8 +22,7 @@ import java.util.regex.Matcher; public class JsonObjectType implements AppsmithType { private static final TypeAdapter strictGsonObjectAdapter = new Gson().getAdapter(JsonObject.class); - private static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); @Override public boolean test(String s) { diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/StringType.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/StringType.java index 8e9e1c19ab..7b1d0fedee 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/StringType.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/StringType.java @@ -3,8 +3,8 @@ package com.appsmith.external.datatypes; import com.appsmith.external.constants.DataType; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import reactor.core.Exceptions; @@ -12,8 +12,7 @@ import java.util.regex.Matcher; public class StringType implements AppsmithType { - private static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); @Override public boolean test(String s) { diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/TimeType.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/TimeType.java index 101df6be25..d56743b2b3 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/TimeType.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/TimeType.java @@ -3,8 +3,8 @@ package com.appsmith.external.datatypes; import com.appsmith.external.constants.DataType; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import reactor.core.Exceptions; @@ -16,8 +16,7 @@ import java.util.regex.Matcher; public class TimeType implements AppsmithType { - private static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); @Override public boolean test(String s) { diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/TimestampType.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/TimestampType.java index 69b7728a56..12bcd75028 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/TimestampType.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/datatypes/TimestampType.java @@ -3,8 +3,8 @@ package com.appsmith.external.datatypes; import com.appsmith.external.constants.DataType; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import reactor.core.Exceptions; @@ -16,8 +16,7 @@ import java.util.regex.Matcher; public class TimestampType implements AppsmithType { - private static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); @Override public boolean test(String s) { diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/DataTypeStringUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/DataTypeStringUtils.java index 071b6ec2c0..950850a20c 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/DataTypeStringUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/DataTypeStringUtils.java @@ -8,8 +8,8 @@ import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException import com.appsmith.external.models.Param; import com.appsmith.external.models.ParsedDataType; import com.appsmith.external.plugins.SmartSubstitutionInterface; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -54,8 +54,7 @@ public class DataTypeStringUtils { public static Pattern placeholderPattern = Pattern.compile(APPSMITH_SUBSTITUTION_PLACEHOLDER); - private static ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); private static final TypeAdapter strictGsonObjectAdapter = new Gson().getAdapter(JsonObject.class); diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/PluginUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/PluginUtils.java index 009f04857e..43b8bd533c 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/PluginUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/PluginUtils.java @@ -10,8 +10,8 @@ import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.Endpoint; import com.appsmith.external.models.Param; import com.appsmith.external.models.Property; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; @@ -47,8 +47,7 @@ import static com.appsmith.external.constants.CommonFieldName.VALUE; @Slf4j public class PluginUtils { - private static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); public static final TypeReference STRING_TYPE = new TypeReference<>() { @Override public Type getType() { diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/DataUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/DataUtils.java index eeb948322a..4ebf294ead 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/DataUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/DataUtils.java @@ -7,8 +7,8 @@ import com.appsmith.external.helpers.PluginUtils; import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.ApiContentType; import com.appsmith.external.models.Property; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -77,7 +77,7 @@ public class DataUtils { } public DataUtils() { - this.objectMapper = new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + this.objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); this.objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/BasePlugin.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/BasePlugin.java index a9dada951f..17ca7b8cae 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/BasePlugin.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/BasePlugin.java @@ -1,14 +1,13 @@ package com.appsmith.external.plugins; -import com.fasterxml.jackson.core.StreamReadFeature; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.databind.ObjectMapper; import org.pf4j.Plugin; import org.pf4j.PluginWrapper; public abstract class BasePlugin extends Plugin { - protected static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + protected static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); public BasePlugin(PluginWrapper wrapper) { super(wrapper); diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/services/ce/FilterDataServiceCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/services/ce/FilterDataServiceCE.java index d63c32361b..b0ec7e0129 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/services/ce/FilterDataServiceCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/services/ce/FilterDataServiceCE.java @@ -8,7 +8,7 @@ import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.models.Condition; import com.appsmith.external.models.UQIDataFilterParams; -import com.fasterxml.jackson.core.StreamReadFeature; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; @@ -98,7 +98,7 @@ public class FilterDataServiceCE implements IFilterDataServiceCE { public FilterDataServiceCE() { - objectMapper = new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); try { connection = DriverManager.getConnection(URL); diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/SerializationUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/SerializationUtils.java index 7f81862616..b123faedb6 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/SerializationUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/SerializationUtils.java @@ -69,4 +69,8 @@ public class SerializationUtils { builder.registerTypeAdapter(HttpMethod.class, new HttpMethodConverter()); }; } + + public static ObjectMapper getObjectMapperWithSourceInLocationEnabled() { + return new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + } } diff --git a/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/utils/WhereConditionUtils.java b/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/utils/WhereConditionUtils.java index 0cc59580c9..44a4b30edc 100644 --- a/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/utils/WhereConditionUtils.java +++ b/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/utils/WhereConditionUtils.java @@ -5,8 +5,8 @@ import com.appsmith.external.constants.DataType; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.helpers.DataTypeStringUtils; +import com.appsmith.util.SerializationUtils; import com.external.plugins.exceptions.FirestoreErrorMessages; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.cloud.firestore.FieldPath; import com.google.cloud.firestore.Query; @@ -20,8 +20,7 @@ import java.util.List; public class WhereConditionUtils { - protected static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + protected static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); public static Query applyWhereConditional(Query query, String strPath, String operatorString, String strValue) throws AppsmithPluginException { diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/GetDatasourceMetadataMethod.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/GetDatasourceMetadataMethod.java index 97609cde72..b3d4fdca58 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/GetDatasourceMetadataMethod.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/GetDatasourceMetadataMethod.java @@ -4,9 +4,9 @@ import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.Property; +import com.appsmith.util.SerializationUtils; import com.appsmith.util.WebClientUtils; import com.external.constants.FieldName; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; @@ -29,8 +29,7 @@ import static org.springframework.util.StringUtils.hasLength; @Slf4j public class GetDatasourceMetadataMethod { - protected static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + protected static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); public static Mono getDatasourceMetadata(DatasourceConfiguration datasourceConfiguration) { diff --git a/app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/utils/GraphQLDataTypeUtils.java b/app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/utils/GraphQLDataTypeUtils.java index b8b2ea7128..1f24392ce6 100644 --- a/app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/utils/GraphQLDataTypeUtils.java +++ b/app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/utils/GraphQLDataTypeUtils.java @@ -2,8 +2,8 @@ package com.external.utils; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import graphql.parser.InvalidSyntaxException; import graphql.parser.Parser; @@ -20,8 +20,7 @@ import static com.appsmith.external.helpers.SmartSubstitutionHelper.APPSMITH_SUB public class GraphQLDataTypeUtils { public static final String GRAPHQL_BODY_ENDS_WITH_PARAM_REGEX = "[\\w\\W]+:$"; - public static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + public static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); public static String smartlyReplaceGraphQLQueryBodyPlaceholderWithValue( String queryBody, String replacement, List> insertedParams) { diff --git a/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/commands/MongoCommand.java b/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/commands/MongoCommand.java index 6e4006269a..f92f9f8f8b 100644 --- a/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/commands/MongoCommand.java +++ b/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/commands/MongoCommand.java @@ -4,9 +4,9 @@ import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException import com.appsmith.external.helpers.PluginUtils; import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.DatasourceStructure; +import com.appsmith.util.SerializationUtils; import com.external.plugins.exceptions.MongoPluginError; import com.external.plugins.exceptions.MongoPluginErrorMessages; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.Getter; import lombok.NoArgsConstructor; @@ -33,8 +33,7 @@ import static com.external.plugins.constants.FieldName.COLLECTION; public abstract class MongoCommand { String collection; List fieldNamesWithNoConfiguration; - protected static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + protected static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); public MongoCommand(ActionConfiguration actionConfiguration) { diff --git a/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/datatypes/MySQLDateTimeType.java b/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/datatypes/MySQLDateTimeType.java index b5344b7ec3..4ce8359f0d 100644 --- a/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/datatypes/MySQLDateTimeType.java +++ b/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/datatypes/MySQLDateTimeType.java @@ -4,8 +4,8 @@ import com.appsmith.external.constants.DataType; import com.appsmith.external.datatypes.AppsmithType; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.util.SerializationUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.ObjectMapper; import reactor.core.Exceptions; @@ -16,8 +16,7 @@ import java.time.format.DateTimeParseException; import java.util.regex.Matcher; public class MySQLDateTimeType implements AppsmithType { - private static final ObjectMapper objectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private static final ObjectMapper objectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); @Override public boolean test(String s) { diff --git a/app/server/appsmith-plugins/saasPlugin/src/main/java/com/external/plugins/SaasPlugin.java b/app/server/appsmith-plugins/saasPlugin/src/main/java/com/external/plugins/SaasPlugin.java index 18383c5c84..f1b1535305 100644 --- a/app/server/appsmith-plugins/saasPlugin/src/main/java/com/external/plugins/SaasPlugin.java +++ b/app/server/appsmith-plugins/saasPlugin/src/main/java/com/external/plugins/SaasPlugin.java @@ -12,13 +12,13 @@ import com.appsmith.external.plugins.BasePlugin; import com.appsmith.external.plugins.PluginExecutor; import com.appsmith.external.plugins.SmartSubstitutionInterface; import com.appsmith.external.services.SharedConfig; +import com.appsmith.util.SerializationUtils; import com.appsmith.util.WebClientUtils; import com.external.helpers.RequestCaptureFilter; import com.external.plugins.exceptions.SaaSErrorMessages; import com.external.plugins.exceptions.SaaSPluginError; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.StreamReadFeature; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; @@ -61,8 +61,7 @@ public class SaasPlugin extends BasePlugin { // Setting max content length. This would've been coming from `spring.codec.max-in-memory-size` property if the // `WebClient` instance was loaded as an auto-wired bean. private final ExchangeStrategies EXCHANGE_STRATEGIES; - private final ObjectMapper saasObjectMapper = - new ObjectMapper().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature()); + private final ObjectMapper saasObjectMapper = SerializationUtils.getObjectMapperWithSourceInLocationEnabled(); public SaasPluginExecutor(SharedConfig sharedConfig) { this.sharedConfig = sharedConfig; From c2955f8c2f6a012e23b17a6f9f16dbb3555be94e Mon Sep 17 00:00:00 2001 From: Nidhi Date: Thu, 25 Apr 2024 15:27:54 +0530 Subject: [PATCH 04/13] chore: Added indices for collection id in actions (#32953) --- ...ation051AddNewActionCollectionIdIndex.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration051AddNewActionCollectionIdIndex.java diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration051AddNewActionCollectionIdIndex.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration051AddNewActionCollectionIdIndex.java new file mode 100644 index 0000000000..a7e44a852d --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration051AddNewActionCollectionIdIndex.java @@ -0,0 +1,56 @@ +package com.appsmith.server.migrations.db.ce; + +import com.appsmith.external.models.GitSyncedDomain; +import com.appsmith.server.domains.NewAction; +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.extern.slf4j.Slf4j; +import org.bson.Document; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.index.CompoundIndexDefinition; +import org.springframework.data.mongodb.core.index.Index; + +import static com.appsmith.server.migrations.DatabaseChangelog1.dropIndexIfExists; +import static com.appsmith.server.migrations.DatabaseChangelog1.ensureIndexes; + +@Slf4j +@ChangeUnit(order = "051", id = "add-idx-new-action-coll-id", author = " ") +public class Migration051AddNewActionCollectionIdIndex { + + private final MongoTemplate mongoTemplate; + + public Migration051AddNewActionCollectionIdIndex(MongoTemplate mongoTemplate) { + this.mongoTemplate = mongoTemplate; + } + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void addMissingIndexInNewActionCollection() { + + String unpublishedCollectionIdIndexName = "unpublished_collectionId_deletedAt_index"; + String publishedCollectionIdIndexName = "published_collectionId_deletedAt_index"; + + // Prod index name + dropIndexIfExists(mongoTemplate, NewAction.class, unpublishedCollectionIdIndexName); + dropIndexIfExists(mongoTemplate, NewAction.class, "unpublishedAction.collectionId_1_deletedAt_1"); + dropIndexIfExists(mongoTemplate, NewAction.class, publishedCollectionIdIndexName); + dropIndexIfExists(mongoTemplate, NewAction.class, "publishedAction.collectionId_1_deletedAt_1"); + + Document unpublishedDoc = new Document(); + unpublishedDoc.put(NewAction.Fields.unpublishedAction_collectionId, 1); + unpublishedDoc.put(GitSyncedDomain.Fields.deletedAt, 1); + Index unpublishedCollectionIdIndex = + new CompoundIndexDefinition(unpublishedDoc).named(unpublishedCollectionIdIndexName); + ensureIndexes(mongoTemplate, NewAction.class, unpublishedCollectionIdIndex); + + Document publishedDoc = new Document(); + publishedDoc.put(NewAction.Fields.publishedAction_collectionId, 1); + publishedDoc.put(GitSyncedDomain.Fields.deletedAt, 1); + Index publishedCollectionIdIndex = + new CompoundIndexDefinition(publishedDoc).named(publishedCollectionIdIndexName); + ensureIndexes(mongoTemplate, NewAction.class, publishedCollectionIdIndex); + } +} From dc6fadaa454c9d7c93443d4d766d860025efc598 Mon Sep 17 00:00:00 2001 From: Nidhi Date: Thu, 25 Apr 2024 17:26:52 +0530 Subject: [PATCH 05/13] =?UTF-8?q?Revert=20"chore:=20GitSynced=20and=20Bran?= =?UTF-8?q?chAware=20are=20two=20different=20types=20(#32=E2=80=A6=20(#329?= =?UTF-8?q?65)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../appsmith/external/models/BaseDomain.java | 7 +++++++ .../external/models/BranchAwareDomain.java | 4 ++-- .../appsmith/external/models/Datasource.java | 2 +- .../external/models/DatasourceStorage.java | 2 +- .../external/models/GitSyncedDomain.java | 21 ------------------- .../ApplicationImportServiceCEImpl.java | 6 ++++-- .../server/domains/ce/CustomJSLibCE.java | 3 +-- .../ArtifactBasedImportableServiceCE.java | 13 ++---------- .../server/migrations/DatabaseChangelog2.java | 7 +++---- ...ation051AddNewActionCollectionIdIndex.java | 5 ++--- 10 files changed, 23 insertions(+), 47 deletions(-) delete mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/GitSyncedDomain.java diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BaseDomain.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BaseDomain.java index f1153ffa2d..9c10df1ad6 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BaseDomain.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BaseDomain.java @@ -4,6 +4,7 @@ import com.appsmith.external.helpers.Identifiable; import com.appsmith.external.views.FromRequest; import com.appsmith.external.views.Git; import com.appsmith.external.views.Views; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonView; import lombok.AccessLevel; import lombok.Getter; @@ -91,6 +92,12 @@ public abstract class BaseDomain implements Persistable, AppsmithDomain, @JsonView(Views.Public.class) public Set userPermissions = new HashSet<>(); + // This field will only be used for git related functionality to sync the action object across different instances. + // This field will be deprecated once we move to the new git sync implementation. + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @JsonView({Views.Internal.class, Git.class}) + String gitSyncId; + public void sanitiseToExportDBObject() { this.setCreatedAt(null); this.setUpdatedAt(null); diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BranchAwareDomain.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BranchAwareDomain.java index d33855af90..aae686b3c7 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BranchAwareDomain.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BranchAwareDomain.java @@ -11,7 +11,7 @@ import static com.appsmith.external.helpers.StringUtils.dotted; @Setter @Getter @FieldNameConstants -public abstract class BranchAwareDomain extends GitSyncedDomain { +public abstract class BranchAwareDomain extends BaseDomain { // This field will be used to store the default/root resource IDs for branched resources generated for git // connected applications and will be used to connect resources across the branches @JsonView(Views.Internal.class) @@ -23,7 +23,7 @@ public abstract class BranchAwareDomain extends GitSyncedDomain { super.sanitiseToExportDBObject(); } - public static class Fields extends GitSyncedDomain.Fields { + public static class Fields extends BaseDomain.Fields { public static final String defaultResources_applicationId = dotted(defaultResources, DefaultResources.Fields.applicationId); public static final String defaultResources_branchName = diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java index cdf0931e96..4a2c979007 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java @@ -25,7 +25,7 @@ import java.util.Set; @NoArgsConstructor @Document @FieldNameConstants -public class Datasource extends GitSyncedDomain { +public class Datasource extends BranchAwareDomain { @Transient public static final String DEFAULT_NAME_PREFIX = "Untitled datasource"; diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStorage.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStorage.java index 361356d23e..ccd3cb1234 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStorage.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStorage.java @@ -26,7 +26,7 @@ import static com.appsmith.external.constants.PluginConstants.DEFAULT_REST_DATAS @NoArgsConstructor @Document @FieldNameConstants -public class DatasourceStorage extends GitSyncedDomain { +public class DatasourceStorage extends BaseDomain { @JsonView(Views.Public.class) String datasourceId; diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/GitSyncedDomain.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/GitSyncedDomain.java deleted file mode 100644 index 10b9ca1b55..0000000000 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/GitSyncedDomain.java +++ /dev/null @@ -1,21 +0,0 @@ -package com.appsmith.external.models; - -import com.appsmith.external.views.Git; -import com.appsmith.external.views.Views; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.annotation.JsonView; -import lombok.Getter; -import lombok.Setter; -import lombok.experimental.FieldNameConstants; - -@Getter -@Setter -@FieldNameConstants -public abstract class GitSyncedDomain extends BaseDomain { - // This field will only be used for git related functionality to sync the action object across different instances. - @JsonProperty(access = JsonProperty.Access.READ_ONLY) - @JsonView({Views.Internal.class, Git.class}) - String gitSyncId; - - public static class Fields extends BaseDomain.Fields {} -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/applications/imports/ApplicationImportServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/applications/imports/ApplicationImportServiceCEImpl.java index 2f142d6db2..d8889bd077 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/applications/imports/ApplicationImportServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/applications/imports/ApplicationImportServiceCEImpl.java @@ -310,8 +310,10 @@ public class ApplicationImportServiceCEImpl } if (applicationJson.getCustomJSLibList() != null) { - List importedCustomJSLibList = - applicationJson.getCustomJSLibList().stream().collect(Collectors.toList()); + List importedCustomJSLibList = applicationJson.getCustomJSLibList().stream() + .peek(customJSLib -> customJSLib.setGitSyncId( + null)) // setting this null so that this custom js lib can be imported again + .collect(Collectors.toList()); applicationJson.setCustomJSLibList(importedCustomJSLibList); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/CustomJSLibCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/CustomJSLibCE.java index 843075e356..4e3c5bca54 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/CustomJSLibCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/CustomJSLibCE.java @@ -1,6 +1,5 @@ package com.appsmith.server.domains.ce; -import com.appsmith.external.models.BaseDomain; import com.appsmith.external.models.BranchAwareDomain; import com.appsmith.external.views.Git; import com.appsmith.external.views.Views; @@ -23,7 +22,7 @@ import java.util.Set; @ToString @NoArgsConstructor @FieldNameConstants -public class CustomJSLibCE extends BaseDomain { +public class CustomJSLibCE extends BranchAwareDomain { /* Library name */ @JsonView({Views.Public.class, Git.class}) String name; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/imports/importable/artifactbased/ArtifactBasedImportableServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/imports/importable/artifactbased/ArtifactBasedImportableServiceCE.java index 82a423f9ae..ff8f44ab5b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/imports/importable/artifactbased/ArtifactBasedImportableServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/imports/importable/artifactbased/ArtifactBasedImportableServiceCE.java @@ -1,7 +1,6 @@ package com.appsmith.server.imports.importable.artifactbased; import com.appsmith.external.models.BaseDomain; -import com.appsmith.external.models.GitSyncedDomain; import com.appsmith.server.domains.Artifact; import com.appsmith.server.domains.Context; import com.appsmith.server.dtos.ImportingMetaDTO; @@ -36,19 +35,11 @@ public interface ArtifactBasedImportableServiceCE entityInCurrentArtifact, T entity) { - if (entity instanceof GitSyncedDomain gitSyncedEntity) { - return entityInCurrentArtifact.get(gitSyncedEntity.getGitSyncId()); - } else { - return entity; - } + return entityInCurrentArtifact.get(entity.getGitSyncId()); } default T getExistingEntityInOtherBranchForImportedEntity( MappedImportableResourcesDTO mappedImportableResourcesDTO, Map entityInOtherArtifact, T entity) { - if (entity instanceof GitSyncedDomain gitSyncedEntity) { - return entityInOtherArtifact.get(gitSyncedEntity.getGitSyncId()); - } else { - return entity; - } + return entityInOtherArtifact.get(entity.getGitSyncId()); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java index a40385b6cc..ca7ad5f15a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java @@ -4,7 +4,6 @@ import com.appsmith.external.converters.ISOStringToInstantConverter; import com.appsmith.external.models.BaseDomain; import com.appsmith.external.models.BranchAwareDomain; import com.appsmith.external.models.Datasource; -import com.appsmith.external.models.GitSyncedDomain; import com.appsmith.external.models.PluginType; import com.appsmith.external.models.Policy; import com.appsmith.server.acl.AclPermission; @@ -116,7 +115,7 @@ public class DatabaseChangelog2 { ActionCollection.class, makeIndex( defaultResources + "." + FieldName.APPLICATION_ID, - GitSyncedDomain.Fields.gitSyncId, + BaseDomain.Fields.gitSyncId, FieldName.DELETED) .named("defaultApplicationId_gitSyncId_deleted")); @@ -125,7 +124,7 @@ public class DatabaseChangelog2 { NewAction.class, makeIndex( defaultResources + "." + FieldName.APPLICATION_ID, - GitSyncedDomain.Fields.gitSyncId, + BaseDomain.Fields.gitSyncId, FieldName.DELETED) .named("defaultApplicationId_gitSyncId_deleted")); @@ -134,7 +133,7 @@ public class DatabaseChangelog2 { NewPage.class, makeIndex( defaultResources + "." + FieldName.APPLICATION_ID, - GitSyncedDomain.Fields.gitSyncId, + BaseDomain.Fields.gitSyncId, FieldName.DELETED) .named("defaultApplicationId_gitSyncId_deleted")); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration051AddNewActionCollectionIdIndex.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration051AddNewActionCollectionIdIndex.java index a7e44a852d..72766b584c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration051AddNewActionCollectionIdIndex.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration051AddNewActionCollectionIdIndex.java @@ -1,6 +1,5 @@ package com.appsmith.server.migrations.db.ce; -import com.appsmith.external.models.GitSyncedDomain; import com.appsmith.server.domains.NewAction; import io.mongock.api.annotations.ChangeUnit; import io.mongock.api.annotations.Execution; @@ -41,14 +40,14 @@ public class Migration051AddNewActionCollectionIdIndex { Document unpublishedDoc = new Document(); unpublishedDoc.put(NewAction.Fields.unpublishedAction_collectionId, 1); - unpublishedDoc.put(GitSyncedDomain.Fields.deletedAt, 1); + unpublishedDoc.put(NewAction.Fields.deletedAt, 1); Index unpublishedCollectionIdIndex = new CompoundIndexDefinition(unpublishedDoc).named(unpublishedCollectionIdIndexName); ensureIndexes(mongoTemplate, NewAction.class, unpublishedCollectionIdIndex); Document publishedDoc = new Document(); publishedDoc.put(NewAction.Fields.publishedAction_collectionId, 1); - publishedDoc.put(GitSyncedDomain.Fields.deletedAt, 1); + publishedDoc.put(NewAction.Fields.deletedAt, 1); Index publishedCollectionIdIndex = new CompoundIndexDefinition(publishedDoc).named(publishedCollectionIdIndexName); ensureIndexes(mongoTemplate, NewAction.class, publishedCollectionIdIndex); From cd8ad11d075b9aab56bb7356617662f5350048ec Mon Sep 17 00:00:00 2001 From: Nidhi Date: Tue, 30 Apr 2024 04:44:35 +0530 Subject: [PATCH 06/13] fix: Empty plugin ids in workspaces are breaking application load (#33042) --- .../com/appsmith/server/plugins/base/PluginServiceCEImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/plugins/base/PluginServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/plugins/base/PluginServiceCEImpl.java index f14b49432d..4918834a3b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/plugins/base/PluginServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/plugins/base/PluginServiceCEImpl.java @@ -51,6 +51,7 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -118,6 +119,7 @@ public class PluginServiceCEImpl extends BaseService pluginIds = workspace.getPlugins().stream() .map(WorkspacePlugin::getPluginId) + .filter(Objects::nonNull) .collect(Collectors.toUnmodifiableSet()); return repository.findAllById(pluginIds); From 2f0431f1f5559c575fbc7fcd9880cff256ce8122 Mon Sep 17 00:00:00 2001 From: Ankita Kinger Date: Mon, 29 Apr 2024 17:17:59 +0530 Subject: [PATCH 07/13] fix: Updating the action check to initialize the super user sagas (#33037) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Updating the action calls for sagas to trigger the Admin setting APIs. This was required after the consolidated API changes came in, as the API takes long time to load and `FETCH_USER_SUCCESS` call happens before `FETCH_FEATURE_FLAGS_SUCCESS` call. We need both of the actions to be successful before adding the saga. Hence updating it to the action `END_CONSOLIDATED_PAGE_LOAD` now. Fixes [#33034](https://github.com/appsmithorg/appsmith/issues/33034) ## Automation /ok-to-test tags="@tag.Settings" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: a5f7a066af16a450d67596a732c6a00cd1c6d9ff > Cypress dashboard url: Click here! ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Enhanced the initialization process for super users to conditionally trigger actions based on specific feature availability. - **Refactor** - Updated logic in the Super User initialization to improve how feature flags and user details are checked and utilized. --- app/client/src/ee/sagas/SuperUserSagas.tsx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/app/client/src/ee/sagas/SuperUserSagas.tsx b/app/client/src/ee/sagas/SuperUserSagas.tsx index d7f1e3d21b..3f9184157c 100644 --- a/app/client/src/ee/sagas/SuperUserSagas.tsx +++ b/app/client/src/ee/sagas/SuperUserSagas.tsx @@ -7,14 +7,21 @@ import { RestryRestartServerPoll, SendTestEmail, } from "ce/sagas/SuperUserSagas"; -import type { ReduxAction } from "@appsmith/constants/ReduxActionConstants"; import { ReduxActionTypes } from "@appsmith/constants/ReduxActionConstants"; import type { User } from "constants/userConstants"; -import { takeLatest, all } from "redux-saga/effects"; +import { takeLatest, all, select } from "redux-saga/effects"; +import { getCurrentUser } from "selectors/usersSelectors"; +import type { FeatureFlags } from "@appsmith/entities/FeatureFlag"; +import { selectFeatureFlags } from "@appsmith/selectors/featureFlagsSelectors"; +import { isGACEnabled } from "@appsmith/utils/planHelpers"; +import { getShowAdminSettings } from "@appsmith/utils/BusinessFeatures/adminSettingsHelpers"; -export function* InitSuperUserSaga(action: ReduxAction) { - const user = action.payload; - if (user.isSuperUser) { +export function* InitSuperUserSaga() { + const user: User = yield select(getCurrentUser); + const featureFlags: FeatureFlags = yield select(selectFeatureFlags); + const isFeatureEnabled = isGACEnabled(featureFlags); + + if (getShowAdminSettings(isFeatureEnabled, user)) { yield all([ takeLatest(ReduxActionTypes.FETCH_ADMIN_SETTINGS, FetchAdminSettingsSaga), takeLatest( @@ -34,7 +41,7 @@ export function* InitSuperUserSaga(action: ReduxAction) { export default function* SuperUserSagas() { yield takeLatest( - ReduxActionTypes.FETCH_USER_DETAILS_SUCCESS, + ReduxActionTypes.END_CONSOLIDATED_PAGE_LOAD, InitSuperUserSaga, ); } From 6972e91ebb52a61a07f4ff135fee617f3ba77f39 Mon Sep 17 00:00:00 2001 From: Nidhi Date: Tue, 30 Apr 2024 13:05:13 +0530 Subject: [PATCH 08/13] fix: Remove atomic pushes temporarily (#33052) --- .../main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java index a7a22b4a0e..ad0373928c 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java @@ -225,7 +225,6 @@ public class GitExecutorCEImpl implements GitExecutor { StringBuilder result = new StringBuilder("Pushed successfully with status : "); git.push() - .setAtomic(true) .setTransportConfigCallback(transportConfigCallback) .setRemote(remoteUrl) .call() From a054e8cc1b07e7d1b8343ee776f9b96dfb6671e2 Mon Sep 17 00:00:00 2001 From: Nilansh Bansal Date: Mon, 6 May 2024 16:30:29 +0530 Subject: [PATCH 09/13] fix: authenticationType field saved to database (#33196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description With the introduction of FromRequest for the create datasource API in [this](https://github.com/appsmithorg/appsmith/pull/33039) PR, we have restricted the fields that will be accepted from the client in the request body. Due to this the `authenticationType` field was not getting accepted from the client in the create datasource API, which lead to the failure of airtable queries. This PR allows the acceptance of `authenticationType` field from the client. Discussions [here](https://theappsmith.slack.com/archives/CTHN8GX5Y/p1714974437746959). Fixes #33184 ## Automation /ok-to-test tags="@tag.Datasource" ### :mag: Cypress test results > [!CAUTION] > πŸ”΄ πŸ”΄ πŸ”΄ Some tests have failed. > Workflow run: > Commit: a292aa075d3598389410b5c201f58c489fd2dfc2 > Cypress dashboard: Click here! > The following are new failures, please fix them before merging the PR:
    >
  1. cypress/e2e/Regression/ServerSide/QueryPane/DSDocs_Spec.ts
> To know the list of identified flaky tests - Refer here ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No (cherry picked from commit a5edfb025a613af376e5ee10d42b6eec7951c0a4) --- .../java/com/appsmith/external/models/AuthenticationDTO.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/AuthenticationDTO.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/AuthenticationDTO.java index 37e588c3ca..e9fe57a845 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/AuthenticationDTO.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/AuthenticationDTO.java @@ -1,6 +1,7 @@ package com.appsmith.external.models; import com.appsmith.external.constants.Authentication; +import com.appsmith.external.views.FromRequest; import com.appsmith.external.views.Views; import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; @@ -40,7 +41,7 @@ public class AuthenticationDTO implements AppsmithDomain { IN_PROGRESS_PERMISSIONS_GRANTED }; - @JsonView(Views.Public.class) + @JsonView({Views.Public.class, FromRequest.class}) String authenticationType; @JsonView(Views.Public.class) From 86e1f895a7419710a3fb699e5301a8bcc0074722 Mon Sep 17 00:00:00 2001 From: Nidhi Date: Mon, 6 May 2024 18:25:32 +0530 Subject: [PATCH 10/13] fix: Start using SHA2 instead of SHA1 algorithms for signing during git operations (#33166) --- app/client/cypress/support/Pages/GitSync.ts | 1 - app/server/appsmith-git/pom.xml | 13 +- .../helpers/SshTransportConfigCallback.java | 114 +++++++++++++++--- app/server/appsmith-interfaces/pom.xml | 6 + .../external/git/constants/SSHConstants.java | 12 ++ .../external/git/utils/CryptoUtil.java | 42 +++++++ .../server/helpers/GitDeployKeyGenerator.java | 77 +++++++----- 7 files changed, 213 insertions(+), 52 deletions(-) create mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java create mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java diff --git a/app/client/cypress/support/Pages/GitSync.ts b/app/client/cypress/support/Pages/GitSync.ts index b40cb9b335..7dbbc5eed5 100644 --- a/app/client/cypress/support/Pages/GitSync.ts +++ b/app/client/cypress/support/Pages/GitSync.ts @@ -157,7 +157,6 @@ export class GitSync { cy.get("@guid").then((uid) => { cy.wait(`@generateKey-${repoName}`).then((result: any) => { let generatedKey = result.response.body.data.publicKey; - generatedKey = generatedKey.slice(0, generatedKey.length - 1); // fetch the generated key and post to the github repo cy.request({ method: "POST", diff --git a/app/server/appsmith-git/pom.xml b/app/server/appsmith-git/pom.xml index 32caa1558e..877477edc1 100644 --- a/app/server/appsmith-git/pom.xml +++ b/app/server/appsmith-git/pom.xml @@ -23,13 +23,6 @@ provided - - - org.eclipse.jgit - org.eclipse.jgit.ssh.jsch - 6.6.1.202309021850-r - - io.projectreactor reactor-test @@ -47,6 +40,12 @@ interfaces 1.0-SNAPSHOT + + + org.eclipse.jgit + org.eclipse.jgit.ssh.apache + 6.6.1.202309021850-r + org.springframework spring-test diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java index bb74d5ad6e..25ab61c26d 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java @@ -1,26 +1,51 @@ package com.appsmith.git.helpers; -import com.jcraft.jsch.JSch; -import com.jcraft.jsch.JSchException; -import com.jcraft.jsch.Session; +import com.appsmith.external.git.utils.CryptoUtil; +import lombok.extern.slf4j.Slf4j; +import org.bouncycastle.crypto.params.AsymmetricKeyParameter; +import org.bouncycastle.crypto.util.OpenSSHPublicKeyUtil; +import org.bouncycastle.jcajce.spec.OpenSSHPrivateKeySpec; +import org.bouncycastle.jcajce.spec.OpenSSHPublicKeySpec; +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.bouncycastle.util.io.pem.PemReader; import org.eclipse.jgit.api.TransportConfigCallback; +import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.SshSessionFactory; import org.eclipse.jgit.transport.SshTransport; import org.eclipse.jgit.transport.Transport; -import org.eclipse.jgit.transport.ssh.jsch.JschConfigSessionFactory; -import org.eclipse.jgit.transport.ssh.jsch.OpenSshConfig; -import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.transport.sshd.ServerKeyDatabase; +import org.eclipse.jgit.transport.sshd.SshdSessionFactory; + +import java.io.File; +import java.io.IOException; +import java.io.StringReader; +import java.net.InetSocketAddress; +import java.security.KeyFactory; +import java.security.KeyPair; +import java.security.NoSuchAlgorithmException; +import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.spec.EncodedKeySpec; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.PKCS8EncodedKeySpec; +import java.util.Base64; +import java.util.List; + +import static com.appsmith.external.git.constants.SSHConstants.ECDSA_KEY_FACTORY_IDENTIFIER_BC; +import static com.appsmith.external.git.constants.SSHConstants.RSA_KEY_FACTORY_IDENTIFIER; +import static com.appsmith.external.git.constants.SSHConstants.RSA_TYPE; /** * A custom TransportConfigCallback class that loads private key and public key from the provided strings in constructor. * An instance of this class will be used as follows: - * + *

* TransportConfigCallback transportConfigCallback = new SshTransportConfigCallback(PVT_KEY_STRING, PUB_KEY_STRING); * Git.open(gitRepoDirFile) // gitRepoDirFile is an instance of File - * .push() - * .setTransportConfigCallback(transportConfigCallback) - * .call(); + * .push() + * .setTransportConfigCallback(transportConfigCallback) + * .call(); */ +@Slf4j public class SshTransportConfigCallback implements TransportConfigCallback { private String privateKey; private String publicKey; @@ -30,17 +55,72 @@ public class SshTransportConfigCallback implements TransportConfigCallback { this.publicKey = publicKey; } - private final SshSessionFactory sshSessionFactory = new JschConfigSessionFactory() { + private final SshSessionFactory sshSessionFactory = new SshdSessionFactory() { + @Override - protected void configure(OpenSshConfig.Host hc, Session session) { - session.setConfig("StrictHostKeyChecking", "no"); + protected ServerKeyDatabase getServerKeyDatabase(File homeDir, File sshDir) { + return new ServerKeyDatabase() { + @Override + public List lookup( + String connectAddress, InetSocketAddress remoteAddress, Configuration config) { + return List.of(); + } + + @Override + public boolean accept( + String connectAddress, + InetSocketAddress remoteAddress, + PublicKey serverKey, + Configuration config, + CredentialsProvider provider) { + return true; + } + }; } @Override - protected JSch createDefaultJSch(FS fs) throws JSchException { - JSch jSch = super.createDefaultJSch(fs); - jSch.addIdentity("id_rsa", privateKey.getBytes(), publicKey.getBytes(), "".getBytes()); - return jSch; + protected Iterable getDefaultKeys(File sshDir) { + + try { + KeyPair keyPair; + KeyFactory keyFactory; + PublicKey generatedPublicKey; + + if (publicKey.startsWith(RSA_TYPE)) { + keyFactory = KeyFactory.getInstance(RSA_KEY_FACTORY_IDENTIFIER); + + generatedPublicKey = keyFactory.generatePublic(CryptoUtil.decodeOpenSSHRSA(publicKey.getBytes())); + + } else { + keyFactory = KeyFactory.getInstance(ECDSA_KEY_FACTORY_IDENTIFIER_BC, new BouncyCastleProvider()); + String[] fields = publicKey.split(" "); + AsymmetricKeyParameter keyParameter = OpenSSHPublicKeyUtil.parsePublicKey( + Base64.getDecoder().decode(fields[1].getBytes())); + OpenSSHPublicKeySpec keySpec = + new OpenSSHPublicKeySpec(OpenSSHPublicKeyUtil.encodePublicKey(keyParameter)); + + generatedPublicKey = keyFactory.generatePublic(keySpec); + } + + EncodedKeySpec privateKeySpec; + String[] splitKeys = privateKey.split("-----.*-----\n"); + if (splitKeys.length > 1) { + byte[] content = new PemReader(new StringReader(privateKey)) + .readPemObject() + .getContent(); + privateKeySpec = new OpenSSHPrivateKeySpec(content); + } else { + privateKeySpec = new PKCS8EncodedKeySpec(Base64.getDecoder().decode(privateKey)); + } + + PrivateKey generatedPrivateKey = keyFactory.generatePrivate(privateKeySpec); + + keyPair = new KeyPair(generatedPublicKey, generatedPrivateKey); + return List.of(keyPair); + } catch (NoSuchAlgorithmException | InvalidKeySpecException | IOException e) { + log.debug("Error while associating keys for signing: ", e); + throw new RuntimeException(e); + } } }; diff --git a/app/server/appsmith-interfaces/pom.xml b/app/server/appsmith-interfaces/pom.xml index bcd2728a8d..b75fe86830 100644 --- a/app/server/appsmith-interfaces/pom.xml +++ b/app/server/appsmith-interfaces/pom.xml @@ -22,6 +22,12 @@ com.hierynomus sshj 0.35.0 + + + org.bouncycastle + bcprov-jdk15on + + diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java new file mode 100644 index 0000000000..14602b251c --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java @@ -0,0 +1,12 @@ +package com.appsmith.external.git.constants; + +public class SSHConstants { + public static final String RSA_TYPE = "ssh-rsa"; + public static final String RSA_TYPE_PREFIX = RSA_TYPE + " "; + private static final String ECDSA_TYPE = "ecdsa-sha2-nistp256"; + public static final String ECDSA_TYPE_PREFIX = ECDSA_TYPE + " "; + + public static final String RSA_KEY_FACTORY_IDENTIFIER = "RSA"; + public static final String ECDSA_KEY_FACTORY_IDENTIFIER = "EC"; + public static final String ECDSA_KEY_FACTORY_IDENTIFIER_BC = "ECDSA"; +} diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java new file mode 100644 index 0000000000..02e0f78c27 --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java @@ -0,0 +1,42 @@ +package com.appsmith.external.git.utils; + +import java.math.BigInteger; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.security.spec.RSAPublicKeySpec; +import java.util.Base64; + +import static com.appsmith.external.git.constants.SSHConstants.RSA_TYPE; + +public class CryptoUtil { + public static RSAPublicKeySpec decodeOpenSSHRSA(byte[] input) { + String[] fields = new String(input, StandardCharsets.US_ASCII).split(" "); + if ((fields.length < 2) || (!fields[0].equals(RSA_TYPE))) + throw new IllegalArgumentException("Unsupported type"); + byte[] std = Base64.getDecoder().decode(fields[1]); + return decodeRSAPublicSSH(std); + } + + static RSAPublicKeySpec decodeRSAPublicSSH(byte[] encoded) { + ByteBuffer input = ByteBuffer.wrap(encoded); + String type = asString(input); + if (!RSA_TYPE.equals(type)) throw new IllegalArgumentException("Unsupported type"); + BigInteger exp = sshInt(input); + BigInteger mod = sshInt(input); + return new RSAPublicKeySpec(mod, exp); + } + + private static String asString(ByteBuffer buf) { + return new String(lenVal(buf), StandardCharsets.US_ASCII); + } + + private static BigInteger sshInt(ByteBuffer buf) { + return new BigInteger(+1, lenVal(buf)); + } + + private static byte[] lenVal(ByteBuffer buf) { + byte[] copy = new byte[buf.getInt()]; + buf.get(copy); + return copy; + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java index d75697cdad..a8f0bd844e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java @@ -1,22 +1,32 @@ package com.appsmith.server.helpers; -import com.appsmith.git.helpers.StringOutputStream; import com.appsmith.server.constants.Assets; import com.appsmith.server.domains.GitAuth; import com.appsmith.server.dtos.GitDeployKeyDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; -import com.jcraft.jsch.JSch; -import com.jcraft.jsch.JSchException; -import com.jcraft.jsch.KeyPair; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; +import org.bouncycastle.crypto.params.AsymmetricKeyParameter; +import org.bouncycastle.crypto.util.OpenSSHPublicKeyUtil; +import org.bouncycastle.crypto.util.PublicKeyFactory; +import java.io.IOException; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; +import java.security.PublicKey; import java.time.Instant; import java.util.ArrayList; +import java.util.Base64; import java.util.List; -import static org.reflections.Reflections.log; +import static com.appsmith.external.git.constants.SSHConstants.ECDSA_KEY_FACTORY_IDENTIFIER; +import static com.appsmith.external.git.constants.SSHConstants.ECDSA_TYPE_PREFIX; +import static com.appsmith.external.git.constants.SSHConstants.RSA_KEY_FACTORY_IDENTIFIER; +import static com.appsmith.external.git.constants.SSHConstants.RSA_TYPE_PREFIX; +@Slf4j public class GitDeployKeyGenerator { public enum supportedProtocols { ECDSA(256, ""), @@ -41,44 +51,57 @@ public class GitDeployKeyGenerator { } public static GitAuth generateSSHKey(String keyType) { - JSch jsch = new JSch(); - KeyPair kpair; - int key; + String alg; int keySize; - if (!StringUtils.isEmpty(keyType) && keyType.equals(supportedProtocols.RSA.name())) { - key = KeyPair.RSA; - keySize = supportedProtocols.RSA.key_size; - } else { - key = KeyPair.ECDSA; - keySize = supportedProtocols.ECDSA.key_size; - } + KeyPair keyPair; + String publicKey; try { - kpair = KeyPair.genKeyPair(jsch, key, keySize); - } catch (JSchException e) { - log.error("failed to generate ECDSA key pair", e); - throw new AppsmithException(AppsmithError.SSH_KEY_GENERATION_ERROR); + if (!StringUtils.isEmpty(keyType) && keyType.equals(supportedProtocols.RSA.name())) { + alg = RSA_KEY_FACTORY_IDENTIFIER; + keySize = supportedProtocols.RSA.key_size; + keyPair = getKeyPair(alg, keySize); + publicKey = writeJavaPublicKeyToSSH2(keyPair.getPublic(), RSA_TYPE_PREFIX); + } else { + alg = ECDSA_KEY_FACTORY_IDENTIFIER; + keySize = supportedProtocols.ECDSA.key_size; + keyPair = getKeyPair(alg, keySize); + publicKey = writeJavaPublicKeyToSSH2(keyPair.getPublic(), ECDSA_TYPE_PREFIX); + } + } catch (NoSuchAlgorithmException | IOException e) { + log.debug("Error while creating key pair", e); + throw new AppsmithException(AppsmithError.SSH_KEY_GENERATION_ERROR, e); } - StringOutputStream privateKeyOutput = new StringOutputStream(); - StringOutputStream publicKeyOutput = new StringOutputStream(); - - kpair.writePrivateKey(privateKeyOutput); - kpair.writePublicKey(publicKeyOutput, "appsmith"); - GitAuth gitAuth = new GitAuth(); - gitAuth.setPublicKey(publicKeyOutput.toString()); - gitAuth.setPrivateKey(privateKeyOutput.toString()); + gitAuth.setPublicKey(publicKey); + byte[] encodedPrivateKey = keyPair.getPrivate().getEncoded(); + gitAuth.setPrivateKey(Base64.getEncoder().encodeToString(encodedPrivateKey)); gitAuth.setGeneratedAt(Instant.now()); gitAuth.setDocUrl(Assets.GIT_DEPLOY_KEY_DOC_URL); return gitAuth; } + private static KeyPair getKeyPair(String alg, int keySize) throws NoSuchAlgorithmException { + KeyPair keyPair; + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance(alg); + keyPairGenerator.initialize(keySize); + keyPair = keyPairGenerator.generateKeyPair(); + + return keyPair; + } + public static List getSupportedProtocols() { List protocolList = new ArrayList<>(); protocolList.add(supportedProtocols.ECDSA.getProtocolDetails()); protocolList.add(supportedProtocols.RSA.getProtocolDetails()); return protocolList; } + + private static String writeJavaPublicKeyToSSH2(final PublicKey publicKey, String prefix) throws IOException { + AsymmetricKeyParameter key = PublicKeyFactory.createKey(publicKey.getEncoded()); + final byte[] sshKey = OpenSSHPublicKeyUtil.encodePublicKey(key); + return prefix + Base64.getEncoder().encodeToString(sshKey) + " appsmith"; + } } From aac3f95b10f9248745ac355a98c44d8e494a36be Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 8 May 2024 11:46:37 +0530 Subject: [PATCH 11/13] chore: Add validations for content-type in request (#33220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /ok-to-test tags="@tag.All" > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: a3d6f94446f358c3d248da08c2a9e1671e537dd3 > Cypress dashboard url: Click here! --- .../server/configurations/SecurityConfig.java | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) 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 6f645a4f3a..c8eeb6af43 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 @@ -7,6 +7,7 @@ import com.appsmith.server.authentication.oauth2clientrepositories.CustomOauth2C import com.appsmith.server.constants.FieldName; import com.appsmith.server.constants.Url; import com.appsmith.server.domains.User; +import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.filters.CSRFFilter; import com.appsmith.server.filters.ConditionalFilter; import com.appsmith.server.filters.LoginRateLimitFilter; @@ -14,6 +15,7 @@ import com.appsmith.server.helpers.RedirectHelper; import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.UserService; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -23,6 +25,10 @@ import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.core.io.ClassPathResource; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.InvalidMediaTypeException; +import org.springframework.http.MediaType; +import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.config.annotation.method.configuration.EnableReactiveMethodSecurity; import org.springframework.security.config.annotation.web.reactive.EnableWebFluxSecurity; @@ -40,6 +46,8 @@ import org.springframework.security.web.server.util.matcher.ServerWebExchangeMat import org.springframework.web.reactive.function.server.RouterFunction; import org.springframework.web.reactive.function.server.RouterFunctions; import org.springframework.web.reactive.function.server.ServerResponse; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebFilterChain; import org.springframework.web.server.adapter.ForwardedHeaderTransformer; import org.springframework.web.server.session.CookieWebSessionIdResolver; import org.springframework.web.server.session.WebSessionIdResolver; @@ -156,7 +164,7 @@ public class SecurityConfig { ServerAuthenticationEntryPointFailureHandler failureHandler = new ServerAuthenticationEntryPointFailureHandler(authenticationEntryPoint); - return http + return http.addFilterAt(this::sanityCheckFilter, SecurityWebFiltersOrder.FIRST) // The native CSRF solution doesn't work with WebFlux, yet, but only for WebMVC. So we make our own. .csrf(csrfSpec -> csrfSpec.disable()) .addFilterAt(new CSRFFilter(objectMapper), SecurityWebFiltersOrder.CSRF) @@ -263,4 +271,37 @@ public class SecurityConfig { user.setIsAnonymous(true); return user; } + + private Mono sanityCheckFilter(ServerWebExchange exchange, WebFilterChain chain) { + // 1. Check if the content-type is valid at all. Mostly just checks if it contains a `/`. + MediaType contentType; + try { + contentType = exchange.getRequest().getHeaders().getContentType(); + } catch (InvalidMediaTypeException e) { + return writeErrorResponse(exchange, chain, e.getMessage()); + } + + // 2. Check if it's a content-type our controllers actually work with. + if (contentType != null + && !MediaType.APPLICATION_JSON.equalsTypeAndSubtype(contentType) + && !MediaType.APPLICATION_FORM_URLENCODED.equalsTypeAndSubtype(contentType) + && !MediaType.MULTIPART_FORM_DATA.equalsTypeAndSubtype(contentType)) { + return writeErrorResponse(exchange, chain, "Unsupported Content-Type"); + } + + return chain.filter(exchange); + } + + private Mono writeErrorResponse(ServerWebExchange exchange, WebFilterChain chain, String message) { + final ServerHttpResponse response = exchange.getResponse(); + response.setStatusCode(HttpStatus.BAD_REQUEST); + response.getHeaders().setContentType(MediaType.APPLICATION_JSON); + try { + return response.writeWith(Mono.just(response.bufferFactory() + .wrap(objectMapper.writeValueAsBytes( + new ResponseDTO<>(response.getStatusCode().value(), null, message, false))))); + } catch (JsonProcessingException ex) { + return chain.filter(exchange); + } + } } From 5fc22113ca0fb9140a7edd5bfaa137563fd09833 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 8 May 2024 15:47:47 +0530 Subject: [PATCH 12/13] fix: Fix OAuth2 client secret showing up in UI (#33281) /ok-to-test tags="@tag.Sanity" --- .../src/main/java/com/appsmith/external/models/OAuth2.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/OAuth2.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/OAuth2.java index 54e94e2551..5386fb6b3a 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/OAuth2.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/OAuth2.java @@ -56,7 +56,7 @@ public class OAuth2 extends AuthenticationDTO { @JsonView({Views.Public.class, FromRequest.class}) String clientId; - @Encrypted @JsonView({Views.Public.class, FromRequest.class}) + @Encrypted @JsonView(FromRequest.class) String clientSecret; @JsonView({Views.Public.class, FromRequest.class}) From 9bcec069b9a42dcdefa2ad9e2e76440d30b8ba3a Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Wed, 15 May 2024 11:04:32 +0530 Subject: [PATCH 13/13] fix: added regex (#33426) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > Added regex for supporting custom usernames in ssh Fixes #19881 ## Automation /ok-to-test tags="@tag.Git" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: 05f2b6f8e6bbbbd6f05e70d9fa9194a07c89d86d > Cypress dashboard url: Click here! ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No --- .../com/appsmith/server/helpers/GitUtils.java | 7 +++++++ .../com/appsmith/server/helpers/GitUtilsTest.java | 15 +++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java index 0387e059bd..f8d6b03f84 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java @@ -26,6 +26,9 @@ public class GitUtils { public static final Pattern URL_PATTERN_WITHOUT_SCHEME = Pattern.compile("^git@(?.+?):/*(?.+?)(\\.git)?$"); + public static final Pattern URL_PATTERN_WITH_CUSTOM_USERNAME = + Pattern.compile("^(ssh://)?[a-zA-Z0-9]+@(?.+?):/*(?.+?)(\\\\.git)?$"); + /** * Sample repo urls : * git@example.com:user/repoName.git @@ -45,6 +48,10 @@ public class GitUtils { match = URL_PATTERN_WITHOUT_SCHEME.matcher(sshUrl); } + if (!match.matches()) { + match = URL_PATTERN_WITH_CUSTOM_USERNAME.matcher(sshUrl); + } + if (!match.matches()) { throw new AppsmithException( AppsmithError.INVALID_GIT_CONFIGURATION, diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitUtilsTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitUtilsTest.java index 0409ec3152..f388da74cf 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitUtilsTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitUtilsTest.java @@ -60,6 +60,14 @@ public class GitUtilsTest { assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( "ssh://git@tim.tam.example.com:9876/v3/sladeping/pyhe/SpaceJunk")) .isEqualTo("https://tim.tam.example.com/v3/sladeping/pyhe/SpaceJunk"); + + // custom ssh username: + assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl("custom@vs-ssh.visualstudio.com:v3/newJet/ai/zilla")) + .isEqualTo("https://vs-ssh.visualstudio.com/v3/newJet/ai/zilla"); + + assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( + "ssh://custom@vs-ssh.visualstudio.com:v3/newJet/ai/zilla")) + .isEqualTo("https://vs-ssh.visualstudio.com/v3/newJet/ai/zilla"); } @Test @@ -131,6 +139,13 @@ public class GitUtilsTest { assertThat(GitUtils.getRepoName("user@host.xz:path/to/repo.git")).isEqualTo("repo"); assertThat(GitUtils.getRepoName("org-987654321@github.com:org_name/repository_name.git")) .isEqualTo("repository_name"); + + // custom ssh username: + assertThat(GitUtils.getRepoName("custom@vs-ssh.visualstudio.com:v3/newJet/ai/zilla")) + .isEqualTo("zilla"); + + assertThat(GitUtils.getRepoName("ssh://custom@vs-ssh.visualstudio.com:v3/newJet/ai/zilla")) + .isEqualTo("zilla"); } @Test