From 094b77832fbb58e146ee68f5ed4f8cfdc952a09b Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Fri, 20 Aug 2021 10:10:04 +0530 Subject: [PATCH] Accept richer inputs from welcome signup form (#6650) * Accept richer inputs from welcome signup form * Send subscribe event when user has approved it --- .../server/configurations/InstanceConfig.java | 6 +-- .../server/configurations/SecurityConfig.java | 1 + .../server/constants/AnalyticsEvents.java | 5 +- .../server/constants/ConfigNames.java | 13 +++++ .../server/controllers/ConfigController.java | 2 +- .../server/controllers/UserController.java | 6 ++- .../com/appsmith/server/domains/UserData.java | 3 ++ .../server/dtos/UserSignupRequestDTO.java | 30 +++++++++++ .../server/services/AnalyticsService.java | 10 ++-- .../server/services/ConfigService.java | 8 ++- .../server/services/ConfigServiceImpl.java | 20 ++++++- .../server/services/UserDataService.java | 2 + .../server/services/UserDataServiceImpl.java | 8 +++ .../appsmith/server/solutions/UserSignup.java | 52 +++++++++++++++---- .../server/solutions/UserSignupTest.java | 14 +++-- 15 files changed, 152 insertions(+), 28 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ConfigNames.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/UserSignupRequestDTO.java diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/InstanceConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/InstanceConfig.java index 72eaa0ff2e..7d24c1efbd 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/InstanceConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/InstanceConfig.java @@ -9,7 +9,6 @@ import com.appsmith.server.services.ConfigService; import io.sentry.Sentry; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import net.minidev.json.JSONObject; import org.springframework.boot.context.event.ApplicationReadyEvent; import org.springframework.context.ApplicationListener; import org.springframework.core.ParameterizedTypeReference; @@ -71,10 +70,7 @@ public class InstanceConfig implements ApplicationListener configService - .updateByName(Appsmith.APPSMITH_REGISTERED, new Config( - new JSONObject(Map.of("value", true)), - Appsmith.APPSMITH_REGISTERED - )) + .save(Appsmith.APPSMITH_REGISTERED, Map.of("value", true)) ); } } 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 8813919f35..c7bc2f10d0 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 @@ -135,6 +135,7 @@ public class SecurityConfig { // This is because the flow enters AclFilter as well and needs to be whitelisted there .matchers(ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, Url.LOGIN_URL), ServerWebExchangeMatchers.pathMatchers(HttpMethod.POST, USER_URL), + ServerWebExchangeMatchers.pathMatchers(HttpMethod.POST, USER_URL + "/super"), ServerWebExchangeMatchers.pathMatchers(HttpMethod.POST, USER_URL + "/forgotPassword"), ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, USER_URL + "/verifyPasswordResetToken"), ServerWebExchangeMatchers.pathMatchers(HttpMethod.PUT, USER_URL + "/resetPassword"), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/AnalyticsEvents.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/AnalyticsEvents.java index d8bffd7401..7ce497c917 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/AnalyticsEvents.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/AnalyticsEvents.java @@ -11,7 +11,10 @@ public enum AnalyticsEvents { UPDATE_LAYOUT, PUBLISH_APPLICATION("publish_APPLICATION"), FORK, - GENERATE_CRUD_PAGE("generate_CRUD_PAGE") + GENERATE_CRUD_PAGE("generate_CRUD_PAGE"), + CREATE_SUPERUSER, + SUBSCRIBE_MARKETING_EMAILS, + UNSUBSCRIBE_MARKETING_EMAILS, ; private final String eventName; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ConfigNames.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ConfigNames.java new file mode 100644 index 0000000000..64d2a1e4a5 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ConfigNames.java @@ -0,0 +1,13 @@ +package com.appsmith.server.constants; + +/** + * Names used in config collections. + */ +public class ConfigNames { + + public static final String COMPANY_NAME = "company-name"; + + // Disallow instantiation of this class. + private ConfigNames() {} + +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ConfigController.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ConfigController.java index ce043ec80e..dd397717c6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ConfigController.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ConfigController.java @@ -30,7 +30,7 @@ public class ConfigController { @PutMapping("/name/{name}") public Mono> updateByName(@PathVariable String name, @RequestBody Config config) { - return service.updateByName(name, config) + return service.updateByName(config) .map(resource -> new ResponseDTO<>(HttpStatus.OK.value(), resource, null)); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java index d5f61000a1..fb9d7367b6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java @@ -7,6 +7,7 @@ import com.appsmith.server.dtos.InviteUsersDTO; import com.appsmith.server.dtos.ResetUserPasswordDTO; import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.dtos.UserProfileDTO; +import com.appsmith.server.dtos.UserSignupRequestDTO; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserDataService; import com.appsmith.server.services.UserOrganizationService; @@ -75,7 +76,10 @@ public class UserController extends BaseController { } @PostMapping("/super") - public Mono> createSuperUser(@Valid @RequestBody User resource, ServerWebExchange exchange) { + public Mono> createSuperUser( + @Valid @RequestBody UserSignupRequestDTO resource, + ServerWebExchange exchange + ) { return userSignup.signupAndLoginSuper(resource, exchange) .map(created -> new ResponseDTO<>(HttpStatus.CREATED.value(), created, null)); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java index 55e36368d0..f776cde544 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java @@ -24,6 +24,9 @@ public class UserData extends BaseDomain { @JsonIgnore String userId; + // Role of the user in their organization, example, Designer, Developer, Product Lead etc. + private String role; + // The ID of the asset which has the profile photo of this user. private String profilePhotoAssetId; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/UserSignupRequestDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/UserSignupRequestDTO.java new file mode 100644 index 0000000000..f212c52a6c --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/UserSignupRequestDTO.java @@ -0,0 +1,30 @@ +package com.appsmith.server.dtos; + +import com.appsmith.server.domains.LoginSource; +import com.appsmith.server.domains.UserState; +import lombok.Data; + +@Data +public class UserSignupRequestDTO { + + private String email; + + private String name; + + private LoginSource source = LoginSource.FORM; + + private UserState state = UserState.ACTIVATED; + + private boolean isEnabled = true; + + private String password; + + private String role; + + private String companyName; + + private boolean allowCollectingAnonymousData; + + private boolean signupForNewsletter; + +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/AnalyticsService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/AnalyticsService.java index d3f77f98d0..def9d25099 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/AnalyticsService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/AnalyticsService.java @@ -65,10 +65,6 @@ public class AnalyticsService { }); } - public void sendEvent(String event, String userId) { - sendEvent(event, userId, null); - } - public void sendEvent(String event, String userId, Map properties) { if (!isActive()) { return; @@ -78,10 +74,12 @@ public class AnalyticsService { // java.lang.UnsupportedOperationException: null // at java.base/java.util.ImmutableCollections.uoe(ImmutableCollections.java) // at java.base/java.util.ImmutableCollections$AbstractImmutableMap.put(ImmutableCollections.java) - Map analyticsProperties = new HashMap<>(properties); + Map analyticsProperties = properties == null ? new HashMap<>() : new HashMap<>(properties); // Hash usernames at all places for self-hosted instance - if (!commonConfig.isCloudHosting()) { + if (!commonConfig.isCloudHosting() + // But send the email intact for the subscribe event, which is sent only if the user has explicitly agreed to it. + && !AnalyticsEvents.SUBSCRIBE_MARKETING_EMAILS.name().equals(event)) { final String hashedUserId = DigestUtils.sha256Hex(userId); analyticsProperties.remove("request"); if (!CollectionUtils.isEmpty(analyticsProperties)) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigService.java index 1c14df1dec..e645fcb7f7 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigService.java @@ -6,10 +6,16 @@ import com.appsmith.server.domains.Datasource; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import java.util.Map; + public interface ConfigService { Mono getByName(String name); - Mono updateByName(String name, Config config); + Mono updateByName(Config config); + + Mono save(Config config); + + Mono save(String name, Map config); Mono getInstanceId(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java index f5c1a4106b..8ce0ad2066 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ConfigServiceImpl.java @@ -10,12 +10,14 @@ import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.ConfigRepository; import com.appsmith.server.repositories.DatasourceRepository; import lombok.extern.slf4j.Slf4j; +import net.minidev.json.JSONObject; import org.springframework.stereotype.Service; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import java.util.Collections; import java.util.List; +import java.util.Map; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; @@ -47,7 +49,8 @@ public class ConfigServiceImpl implements ConfigService { } @Override - public Mono updateByName(String name, Config config) { + public Mono updateByName(Config config) { + final String name = config.getName(); return repository.findByName(name) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.CONFIG, name))) .flatMap(dbConfig -> { @@ -57,6 +60,21 @@ public class ConfigServiceImpl implements ConfigService { }); } + @Override + public Mono save(Config config) { + return repository.findByName(config.getName()) + .flatMap(dbConfig -> { + dbConfig.setConfig(config.getConfig()); + return repository.save(dbConfig); + }) + .switchIfEmpty(Mono.defer(() -> repository.save(config))); + } + + @Override + public Mono save(String name, Map config) { + return save(new Config(new JSONObject(config), name)); + } + @Override public Mono getInstanceId() { if (instanceId != null) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataService.java index 5d1052b2b2..99f0c03e24 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataService.java @@ -19,6 +19,8 @@ public interface UserDataService { Mono updateForCurrentUser(UserData updates); + Mono updateForUser(User user, UserData updates); + Mono setViewedCurrentVersionReleaseNotes(User user); Mono setViewedCurrentVersionReleaseNotes(User user, String version); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataServiceImpl.java index f2dcc895da..a8aeb1d492 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataServiceImpl.java @@ -106,6 +106,14 @@ public class UserDataServiceImpl extends BaseService updateForUser(User user, UserData updates) { + updates.setUserId(user.getId()); + final Mono updaterMono = update(user.getId(), updates); + final Mono creatorMono = Mono.just(updates).flatMap(this::create); + return updaterMono.switchIfEmpty(creatorMono); + } + @Override public Mono update(String userId, UserData resource) { if (userId == null) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java index c84fb714ea..e858bcabf4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserSignup.java @@ -3,15 +3,21 @@ package com.appsmith.server.solutions; import com.appsmith.external.models.Policy; import com.appsmith.server.acl.AclPermission; import com.appsmith.server.authentication.handlers.AuthenticationSuccessHandler; +import com.appsmith.server.constants.AnalyticsEvents; +import com.appsmith.server.constants.ConfigNames; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.LoginSource; import com.appsmith.server.domains.User; +import com.appsmith.server.domains.UserData; import com.appsmith.server.domains.UserState; +import com.appsmith.server.dtos.UserSignupRequestDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.PolicyUtils; -import com.appsmith.server.repositories.UserRepository; +import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.CaptchaService; +import com.appsmith.server.services.ConfigService; +import com.appsmith.server.services.UserDataService; import com.appsmith.server.services.UserService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -47,10 +53,12 @@ import static org.springframework.security.web.server.context.WebSessionServerSe public class UserSignup { private final UserService userService; + private final UserDataService userDataService; private final CaptchaService captchaService; private final AuthenticationSuccessHandler authenticationSuccessHandler; + private final ConfigService configService; + private final AnalyticsService analyticsService; private final PolicyUtils policyUtils; - private final UserRepository userRepository; private static final ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); @@ -109,10 +117,10 @@ public class UserSignup { String recaptchaToken = exchange.getRequest().getQueryParams().getFirst("recaptchaToken"); return captchaService.verify(recaptchaToken).flatMap(verified -> { - if (!verified) { - return Mono.error(new AppsmithException(AppsmithError.GOOGLE_RECAPTCHA_FAILED)); - } - return exchange.getFormData(); + if (!Boolean.TRUE.equals(verified)) { + return Mono.error(new AppsmithException(AppsmithError.GOOGLE_RECAPTCHA_FAILED)); + } + return exchange.getFormData(); }) .map(formData -> { final User user = new User(); @@ -151,19 +159,45 @@ public class UserSignup { }); } - public Mono signupAndLoginSuper(User user, ServerWebExchange exchange) { + public Mono signupAndLoginSuper(UserSignupRequestDTO userFromRequest, ServerWebExchange exchange) { return userService.isUsersEmpty() .flatMap(isEmpty -> { - if (!isEmpty) { + if (!Boolean.TRUE.equals(isEmpty)) { return Mono.error(new AppsmithException(AppsmithError.UNAUTHORIZED_ACCESS)); } + final User user = new User(); + user.setEmail(userFromRequest.getEmail()); + user.setName(userFromRequest.getName()); + user.setSource(userFromRequest.getSource()); + user.setState(userFromRequest.getState()); + user.setIsEnabled(userFromRequest.isEnabled()); + user.setPassword(userFromRequest.getPassword()); + policyUtils.addPoliciesToExistingObject(Map.of( AclPermission.MANAGE_INSTANCE_ENV.getValue(), - Policy.builder().permission(AclPermission.MANAGE_INSTANCE_ENV.getValue()).users(Set.of(user.getUsername())).build() + Policy.builder().permission(AclPermission.MANAGE_INSTANCE_ENV.getValue()).users(Set.of(user.getEmail())).build() ), user); return signupAndLogin(user, exchange); + }) + .flatMap(user -> { + final UserData userData = new UserData(); + userData.setRole(userFromRequest.getRole()); + + if (userFromRequest.isSignupForNewsletter()) { + analyticsService.sendEvent( + AnalyticsEvents.SUBSCRIBE_MARKETING_EMAILS.name(), + user.getEmail(), + Map.of("id", user.getEmail()) + ); + } + + return Mono.when( + userDataService.updateForUser(user, userData), + configService.save(ConfigNames.COMPANY_NAME, Map.of("value", userFromRequest.getCompanyName())), + analyticsService.sendObjectEvent(AnalyticsEvents.CREATE_SUPERUSER, user, null) + ).thenReturn(user); }); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/UserSignupTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/UserSignupTest.java index 4e6527db1a..66d1bfb4ac 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/UserSignupTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/UserSignupTest.java @@ -5,8 +5,10 @@ import com.appsmith.server.domains.User; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.PolicyUtils; import com.appsmith.server.helpers.ValidationUtils; -import com.appsmith.server.repositories.UserRepository; +import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.CaptchaService; +import com.appsmith.server.services.ConfigService; +import com.appsmith.server.services.UserDataService; import com.appsmith.server.services.UserService; import org.junit.Before; import org.junit.Test; @@ -21,23 +23,29 @@ public class UserSignupTest { @MockBean private UserService userService; + @MockBean + private UserDataService userDataService; + @MockBean private CaptchaService captchaService; @MockBean private AuthenticationSuccessHandler authenticationSuccessHandler; + @MockBean + private ConfigService configService; + @MockBean private PolicyUtils policyUtils; @MockBean - private UserRepository userRepository; + private AnalyticsService analyticsService; private UserSignup userSignup; @Before public void setUp() { - userSignup = new UserSignup(userService, captchaService, authenticationSuccessHandler, policyUtils, userRepository); + userSignup = new UserSignup(userService, userDataService, captchaService, authenticationSuccessHandler, configService, analyticsService, policyUtils); } private String createRandomString(int length) {