fix: Don't disclose sensitive info in case of DuplicateKeyException (a…

…ppsmithorg#21568) (appsmithorg#21596)

## Description

When data integrity is violated in the case of `DuplicateKeyException`
sensitive information like Appsmith URI is exposed to error messages.
This is a big security risk that needs to be fixed and any error message
shouldn't display Appsmith server credentials.


[RCA](appsmithorg#21568 (comment))

[Slack
Thread](https://theappsmith.slack.com/archives/C0423TJFUJK/p1679082313650259)

Fixes appsmithorg#21568 


Media
> A video or a GIF is preferred. when using Loom, don’t embed because it
looks like it’s a GIF. instead, just link to the video


## Type of change

> Please delete options that are not relevant.

- Bug fix (non-breaking change which fixes an issue)
- Chore (housekeeping or task changes that don't impact user perception)


## How Has This Been Tested?
- Manual
- JUnit

### Test Plan
> Add Testsmith test cases links that relate to this PR

### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)


## Checklist:
### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test

Co-authored-by: Aishwarya UR <[email protected]>

app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java

@@ -69,7 +69,7 @@ public enum AppsmithError {
     JSON_PROCESSING_ERROR(400, AppsmithErrorCode.JSON_PROCESSING_ERROR.getCode(), "Json processing error with error {0}", AppsmithErrorAction.LOG_EXTERNALLY, "Json processing error", ErrorType.INTERNAL_ERROR, null),
     INVALID_CREDENTIALS(200, AppsmithErrorCode.INVALID_CREDENTIALS.getCode(), "Invalid credentials provided. Did you input the credentials correctly?", AppsmithErrorAction.DEFAULT, "Invalid credentials", ErrorType.AUTHENTICATION_ERROR, null),
     UNAUTHORIZED_ACCESS(403, AppsmithErrorCode.UNAUTHORIZED_ACCESS.getCode(), "Unauthorized access", AppsmithErrorAction.DEFAULT, "Unauthorized access", ErrorType.AUTHENTICATION_ERROR, null),
-    DUPLICATE_KEY(409, AppsmithErrorCode.DUPLICATE_KEY.getCode(), "Unexpected state: Duplicate key error. Message: {0}. Please reach out to Appsmith customer support to report this",
+    DUPLICATE_KEY(409, AppsmithErrorCode.DUPLICATE_KEY.getCode(), "Duplicate key error: An object with the name {0} already exists. Please use a different name or reach out to Appsmith customer support to resolve this.",
             AppsmithErrorAction.DEFAULT, "Duplicate key", ErrorType.BAD_REQUEST, null),
     USER_ALREADY_EXISTS_SIGNUP(409, AppsmithErrorCode.USER_ALREADY_EXISTS_SIGNUP.getCode(), "There is already an account registered with this email {0}. Please sign in instead.",
             AppsmithErrorAction.DEFAULT, "Account already exists with this email", ErrorType.BAD_REQUEST, null),

app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java

@@ -5,6 +5,7 @@
 import com.appsmith.external.exceptions.ErrorDTO;
 import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
 import com.appsmith.server.dtos.ResponseDTO;
+import com.appsmith.server.exceptions.util.DuplicateKeyExceptionUtils;
 import com.appsmith.server.helpers.RedisUtils;
 import io.micrometer.core.instrument.util.StringUtils;
 import com.appsmith.server.filters.MDCFilter;
@@ -116,8 +117,9 @@ public Mono<ResponseDTO<ErrorDTO>> catchDuplicateKeyException(org.springframewor
         doLog(e);
 
         String urlPath = exchange.getRequest().getPath().toString();
+        String conflictingObjectName = DuplicateKeyExceptionUtils.extractConflictingObjectName(e.getCause().getMessage());
         ResponseDTO<ErrorDTO> response =  new ResponseDTO<>(appsmithError.getHttpErrorCode(), new ErrorDTO(appsmithError.getAppErrorCode(), appsmithError.getErrorType(),
-                appsmithError.getMessage(e.getCause().getMessage()), appsmithError.getTitle()));
+                appsmithError.getMessage(conflictingObjectName), appsmithError.getTitle()));
 
         return getResponseDTOMono(urlPath, response);
     }

app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/util/DuplicateKeyExceptionUtils.java

@@ -0,0 +1,25 @@
+package com.appsmith.server.exceptions.util;
+
+import lombok.extern.slf4j.Slf4j;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+@Slf4j
+public class DuplicateKeyExceptionUtils {
+    private final static Pattern pattern = Pattern.compile("dup key: \\{ .*:(.*)}'");
+
+    public static String extractConflictingObjectName(String duplicateKeyErrorMessage) {
+        Matcher matcher = pattern.matcher(duplicateKeyErrorMessage);
+        if (matcher.find()) {
+            return matcher.group(1).trim();
+        }
+        log.warn("DuplicateKeyException regex needs attention. It's unable to extract object name from the error message. Possible reason: the underlying library may have changed the format of the error message.");
+        /*
+            [Fallback strategy]
+            AppsmithError.DUPLICATE_KEY has a placeholder where it expects the name of the object that conflicts with the existing names.
+            In case the execution reaches here we don't want to render `null` rather the string returned as below will yet make the message look good.
+         */
+        return "that you provided";
+    }
+}

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java

@@ -194,7 +194,7 @@ public Mono<UpdateResult> addPageToApplication(Application application, PageDTO
                         }
                     });
         } else {
-            return Mono.error(new AppsmithException(AppsmithError.DUPLICATE_KEY, "Page already exists with id " + page.getId()));
+            return Mono.error(new AppsmithException(AppsmithError.DUPLICATE_KEY, page.getId()));
         }
 
     }

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationServiceCEImpl.java

@@ -26,6 +26,7 @@
 import com.appsmith.server.dtos.GitDeployKeyDTO;
 import com.appsmith.server.exceptions.AppsmithError;
 import com.appsmith.server.exceptions.AppsmithException;
+import com.appsmith.server.exceptions.util.DuplicateKeyExceptionUtils;
 import com.appsmith.server.helpers.GitDeployKeyGenerator;
 import com.appsmith.server.helpers.PolicyUtils;
 import com.appsmith.server.helpers.ResponseUtils;
@@ -263,7 +264,7 @@ public Mono<Application> update(String id, Application application) {
                                             new AppsmithException(AppsmithError.DUPLICATE_KEY_USER_ERROR, FieldName.APPLICATION, FieldName.NAME)
                                     );
                                 }
-                                return Mono.error(new AppsmithException(AppsmithError.DUPLICATE_KEY, error.getCause().getMessage()));
+                                return Mono.error(new AppsmithException(AppsmithError.DUPLICATE_KEY, DuplicateKeyExceptionUtils.extractConflictingObjectName(((DuplicateKeyException) error).getCause().getMessage())));
                             }
                             return Mono.error(error);
                         })

app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/AppsmithErrorTest.java

@@ -1,14 +1,34 @@
 package com.appsmith.server.helpers;
 
 import com.appsmith.server.exceptions.AppsmithError;
+import com.appsmith.server.exceptions.util.DuplicateKeyExceptionUtils;
 import org.junit.jupiter.api.Test;
+import org.springframework.dao.DuplicateKeyException;
 
 import java.util.Arrays;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
 public class AppsmithErrorTest {
     @Test
     public void verifyUniquenessOfAppsmithErrorCode() {
         assert (Arrays.stream(AppsmithError.values()).map(AppsmithError::getAppErrorCode).distinct().count() == AppsmithError.values().length);
+    }
+
+    @Test
+    public void verifyDuplicateKeyExceptionDoesnotDiscloseSensitiveInformation() {
+        //Context: https://github.com/appsmithorg/appsmith/issues/21568
+        final DuplicateKeyException exception = assertThrows(
+                DuplicateKeyException.class,
+                () -> generateDuplicateKeyException());
+
+        AppsmithError appsmithError = AppsmithError.DUPLICATE_KEY;
+        assertEquals(appsmithError.getMessage("\\\"MyJSObject\\\""), appsmithError.getMessage(DuplicateKeyExceptionUtils.extractConflictingObjectName(exception.getMessage())));
+    }
 
+    private void generateDuplicateKeyException() {
+        String originalErrorMessage = "Write operation error on server localhost:27017. Write error: WriteError{code=11000, message='E11000 duplicate key error collection: appsmith.actionCollection index: unpublishedCollection.name_1 dup key: { unpublishedCollection.name: \\\"MyJSObject\\\" }', details={}}.";
+        throw new DuplicateKeyException(originalErrorMessage);
     }
 }