From 6f69f1935c7d0d4c4e8ffee4b2e20c1ffa81d77a Mon Sep 17 00:00:00 2001 From: Nilansh Bansal Date: Thu, 4 May 2023 10:49:45 +0530 Subject: [PATCH] fix: display name validation (server) (#22927) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > This PR adds a validation regex for display name, it allows for Accented characters such as (ä ö ü è ß) etc, and alphanumeric with some special characters dot (.), apostrophe ('), hyphen (-) and spaces. > It also allows for chinese characters, eg. 황현미 Fixes #22578 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - 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: Anand Srinivasan (cherry picked from commit 3915334dd9ef185f95581165415af55212aff617) --- app/client/src/ce/constants/messages.ts | 3 ++ app/client/src/ce/sagas/userSagas.tsx | 12 +++++- .../server/services/ce/UserServiceCEImpl.java | 17 +++++++- .../server/services/UserServiceTest.java | 39 ++++++++++++++++--- 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/app/client/src/ce/constants/messages.ts b/app/client/src/ce/constants/messages.ts index 4272db8b48..fdbabc79fa 100644 --- a/app/client/src/ce/constants/messages.ts +++ b/app/client/src/ce/constants/messages.ts @@ -171,6 +171,9 @@ export const USERS_HAVE_ACCESS_TO_ONLY_THIS_APP = () => "Users will only have access to this application"; export const NO_USERS_INVITED = () => "You haven't invited any users yet"; +export const UPDATE_USER_DETAILS_FAILED = () => + "Unable to update user details."; + export const CREATE_PASSWORD_RESET_SUCCESS = () => `Your password has been set`; export const CREATE_PASSWORD_RESET_SUCCESS_LOGIN_LINK = () => `Login`; diff --git a/app/client/src/ce/sagas/userSagas.tsx b/app/client/src/ce/sagas/userSagas.tsx index b0dfe22a86..70f2ce04a3 100644 --- a/app/client/src/ce/sagas/userSagas.tsx +++ b/app/client/src/ce/sagas/userSagas.tsx @@ -21,6 +21,7 @@ import UserApi from "@appsmith/api/UserApi"; import { AUTH_LOGIN_URL, SETUP } from "constants/routes"; import history from "utils/history"; import type { ApiResponse } from "api/ApiResponses"; +import type { ErrorActionPayload } from "sagas/ErrorSagas"; import { validateResponse, getResponseErrorMessage, @@ -72,6 +73,8 @@ import type { SegmentState } from "reducers/uiReducers/analyticsReducer"; import type FeatureFlags from "entities/FeatureFlags"; import UsagePulse from "usagePulse"; import { isAirgapped } from "@appsmith/utils/airgapHelpers"; +import { UPDATE_USER_DETAILS_FAILED } from "ce/constants/messages"; +import { createMessage } from "design-system-old/build/constants/messages"; export function* createUserSaga( action: ReduxActionWithPromise, @@ -382,9 +385,16 @@ export function* updateUserDetailsSaga(action: ReduxAction) { }); } } catch (error) { + const payload: ErrorActionPayload = { + show: true, + error: { + message: + (error as Error).message ?? createMessage(UPDATE_USER_DETAILS_FAILED), + }, + }; yield put({ type: ReduxActionErrorTypes.UPDATE_USER_DETAILS_ERROR, - payload: (error as Error).message, + payload, }); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java index fb2c5bf0a3..d7d5b79192 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java @@ -69,6 +69,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.regex.Pattern; import static com.appsmith.server.acl.AclPermission.MANAGE_USERS; import static com.appsmith.server.helpers.ValidationUtils.LOGIN_PASSWORD_MAX_LENGTH; @@ -100,6 +101,7 @@ public class UserServiceCEImpl extends BaseService private static final String FORGOT_PASSWORD_CLIENT_URL_FORMAT = "%s/user/resetPassword?token=%s"; private static final String INVITE_USER_CLIENT_URL_FORMAT = "%s/user/signup?email=%s"; public static final String INVITE_USER_EMAIL_TEMPLATE = "email/inviteUserTemplate.html"; + private static final Pattern ALLOWED_ACCENTED_CHARACTERS_PATTERN = Pattern.compile("^[\\p{L} 0-9 .\'\\-]+$"); @Autowired public UserServiceCEImpl(Scheduler scheduler, @@ -659,6 +661,14 @@ public class UserServiceCEImpl extends BaseService return Flux.error(new AppsmithException(AppsmithError.UNSUPPORTED_OPERATION)); } + private boolean validateName(String name){ + /* + Regex allows for Accented characters and alphanumeric with some special characters dot (.), apostrophe ('), + hyphen (-) and spaces + */ + return ALLOWED_ACCENTED_CHARACTERS_PATTERN.matcher(name).matches(); + } + @Override public Mono updateCurrentUser(final UserUpdateDTO allUpdates, ServerWebExchange exchange) { List> monos = new ArrayList<>(); @@ -668,7 +678,12 @@ public class UserServiceCEImpl extends BaseService if (allUpdates.hasUserUpdates()) { final User updates = new User(); - updates.setName(allUpdates.getName()); + String inputName = allUpdates.getName(); + boolean isValidName = validateName(inputName); + if (!isValidName){ + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.NAME)); + } + updates.setName(inputName); updatedUserMono = sessionUserService.getCurrentUser() .flatMap(user -> update(user.getEmail(), updates, fieldName(QUser.user.email)) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java index bd3bbcb86c..821a6ed979 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java @@ -415,12 +415,39 @@ public class UserServiceTest { @WithUserDetails(value = "api_user") public void updateNameOfUser() { UserUpdateDTO updateUser = new UserUpdateDTO(); - updateUser.setName("New name of api_user"); + updateUser.setName("New name of api user"); StepVerifier.create(userService.updateCurrentUser(updateUser, null)) .assertNext(user -> { assertNotNull(user); - assertThat(user.getEmail()).isEqualTo("api_user"); - assertThat(user.getName()).isEqualTo("New name of api_user"); + assertEquals("api_user", user.getEmail()); + assertEquals("New name of api user", user.getName()); + }) + .verifyComplete(); + } + + @Test + @WithUserDetails(value = "api_user") + public void updateNameOfUser_WithNotAllowedSpecialCharacter_InvalidName() { + UserUpdateDTO updateUser = new UserUpdateDTO(); + updateUser.setName("invalid name@symbol"); + StepVerifier.create(userService.updateCurrentUser(updateUser, null)) + .expectErrorMatches(throwable -> + throwable instanceof AppsmithException + && + throwable.getMessage().contains(AppsmithError.INVALID_PARAMETER.getMessage(FieldName.NAME)) + ) + .verify(); + } + + @Test + @WithUserDetails(value = "api_user") + public void updateNameOfUser_WithAccentedCharacters_IsValid() { + UserUpdateDTO updateUser = new UserUpdateDTO(); + updateUser.setName("ä ö ü è ß Test . '- ðƒ 你好 123'"); + StepVerifier.create(userService.updateCurrentUser(updateUser, null)) + .assertNext(user -> { + assertNotNull(user); + assertEquals("ä ö ü è ß Test . '- ðƒ 你好 123'", user.getName()); }) .verifyComplete(); } @@ -491,9 +518,9 @@ public class UserServiceTest { final UserData userData = tuple.getT2(); assertNotNull(user); assertNotNull(userData); - assertThat(user.getName()).isEqualTo("New name of user here"); - assertThat(userData.getRole()).isEqualTo("New role of user"); - assertThat(userData.getUseCase()).isEqualTo("New use case"); + assertEquals("New name of user here", user.getName()); + assertEquals("New role of user", userData.getRole()); + assertEquals("New use case", userData.getUseCase()); }) .verifyComplete(); }