diff --git a/app/server/pom.xml b/app/server/pom.xml index 11b90b2c3d..2cf6b31bc2 100644 --- a/app/server/pom.xml +++ b/app/server/pom.xml @@ -18,6 +18,25 @@ 11 + + + spring-milestones + Spring Milestones + https://repo.spring.io/milestone + + false + + + + jboss-maven2-release-repository + JBoss Spring Repository + https://repository.jboss.org/nexus/content/repositories/public/ + + false + + + + org.springframework.boot @@ -55,6 +74,16 @@ org.springframework.boot spring-boot-starter-data-mongodb-reactive + + org.hibernate.validator + hibernate-validator + 6.0.17.Final + + + org.glassfish + javax.el + 3.0.0 + org.projectlombok lombok @@ -71,12 +100,17 @@ spring-boot-starter-test test - org.springframework.security spring-security-test test + + io.projectreactor + reactor-test + 3.2.11.RELEASE + test + diff --git a/app/server/src/main/java/com/appsmith/server/configurations/ClientUserRepository.java b/app/server/src/main/java/com/appsmith/server/configurations/ClientUserRepository.java index 05e9649261..d630f3a22e 100644 --- a/app/server/src/main/java/com/appsmith/server/configurations/ClientUserRepository.java +++ b/app/server/src/main/java/com/appsmith/server/configurations/ClientUserRepository.java @@ -24,14 +24,14 @@ import java.util.Map; * This code has been copied from WebSessionServerOAuth2AuthorizedClientRepository.java * which also implements ServerOAuth2AuthorizedClientRepository. This was done to make changes * to saveAuthorizedClient to also handle adding users to UserRepository. - * + *

* This was done because on authorization, the user needs to be stored in appsmith domain. * To achieve this, saveAuthorizedClient function has been edited in the following manner. * In the reactive flow, post doOnSuccess transformation, another Mono.then has been added. In this, * Authentication object is passed to checkAndCreateUser function. This object is used to get OidcUser from which * user attributes like name, email, etc are extracted. If the user doesnt exist in User * Repository, a new user is created and saved. - * + *

* The ClientUserRepository is created during SecurityWebFilterChain Bean creation. By * configuring to use Oauth2Login, this ServerOAuth2AuthorizedClientRepository implementation * is injected. This hack is used to ensure that on successful authentication, we are able @@ -42,18 +42,16 @@ import java.util.Map; @Configuration public class ClientUserRepository implements ServerOAuth2AuthorizedClientRepository { + private static final String DEFAULT_AUTHORIZED_CLIENTS_ATTR_NAME = + WebSessionServerOAuth2AuthorizedClientRepository.class.getName() + ".AUTHORIZED_CLIENTS"; + private final String sessionAttributeName = DEFAULT_AUTHORIZED_CLIENTS_ATTR_NAME; UserService userService; TenantService tenantService; - public ClientUserRepository(UserService userService, TenantService tenantService) { this.userService = userService; this.tenantService = tenantService; } - private static final String DEFAULT_AUTHORIZED_CLIENTS_ATTR_NAME = - WebSessionServerOAuth2AuthorizedClientRepository.class.getName() + ".AUTHORIZED_CLIENTS"; - private final String sessionAttributeName = DEFAULT_AUTHORIZED_CLIENTS_ATTR_NAME; - @Override @SuppressWarnings("unchecked") public Mono loadAuthorizedClient(String clientRegistrationId, Authentication principal, diff --git a/app/server/src/main/java/com/appsmith/server/configurations/CommonConfig.java b/app/server/src/main/java/com/appsmith/server/configurations/CommonConfig.java index 7668a381bf..4a76935c88 100644 --- a/app/server/src/main/java/com/appsmith/server/configurations/CommonConfig.java +++ b/app/server/src/main/java/com/appsmith/server/configurations/CommonConfig.java @@ -5,6 +5,9 @@ import org.springframework.context.annotation.Configuration; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; +import javax.validation.Validation; +import javax.validation.Validator; + @Configuration public class CommonConfig { @@ -15,4 +18,8 @@ public class CommonConfig { return Schedulers.newElastic(ELASTIC_THREAD_POOL_NAME); } + @Bean + public Validator validator() { + return Validation.buildDefaultValidatorFactory().getValidator(); + } } diff --git a/app/server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java b/app/server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java index ab7c20d460..30585b1c82 100644 --- a/app/server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java +++ b/app/server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java @@ -72,7 +72,7 @@ public class SecurityConfig { .authenticated() .and().httpBasic() .and().oauth2Login() - .authorizedClientRepository(new ClientUserRepository(userService, tenantService)) + .authorizedClientRepository(new ClientUserRepository(userService, tenantService)) .and().formLogin() .and().build(); } diff --git a/app/server/src/main/java/com/appsmith/server/constants/FieldName.java b/app/server/src/main/java/com/appsmith/server/constants/FieldName.java new file mode 100644 index 0000000000..ba3ce6fc57 --- /dev/null +++ b/app/server/src/main/java/com/appsmith/server/constants/FieldName.java @@ -0,0 +1,7 @@ +package com.appsmith.server.constants; + +public class FieldName { + public static String TENANT = "tenant"; + public static String ID = "id"; + public static String NAME = "name"; +} diff --git a/app/server/src/main/java/com/appsmith/server/controllers/BaseController.java b/app/server/src/main/java/com/appsmith/server/controllers/BaseController.java index deccd3d0a5..f61763efed 100644 --- a/app/server/src/main/java/com/appsmith/server/controllers/BaseController.java +++ b/app/server/src/main/java/com/appsmith/server/controllers/BaseController.java @@ -47,7 +47,7 @@ public abstract class BaseController> update(@PathVariable ID id, @RequestBody T resource) throws Exception { + public Mono> update(@PathVariable ID id, @RequestBody T resource) { log.debug("Going to update resource with id: {}", id); return service.update(id, resource) .map(updatedResource -> new ResponseDto<>(HttpStatus.OK.value(), updatedResource, null)); diff --git a/app/server/src/main/java/com/appsmith/server/domains/Plugin.java b/app/server/src/main/java/com/appsmith/server/domains/Plugin.java index 8f71b6f9d5..7efcdfa4c9 100644 --- a/app/server/src/main/java/com/appsmith/server/domains/Plugin.java +++ b/app/server/src/main/java/com/appsmith/server/domains/Plugin.java @@ -28,9 +28,9 @@ public class Plugin extends BaseDomain { List resourceParams; List actionParams; - + String minAppsmithVersionSupported; - + String maxAppsmithVersionSupported; String version; diff --git a/app/server/src/main/java/com/appsmith/server/domains/Tenant.java b/app/server/src/main/java/com/appsmith/server/domains/Tenant.java index 26d8a3b49d..e7f80c01bc 100644 --- a/app/server/src/main/java/com/appsmith/server/domains/Tenant.java +++ b/app/server/src/main/java/com/appsmith/server/domains/Tenant.java @@ -6,6 +6,7 @@ import lombok.Setter; import lombok.ToString; import org.springframework.data.mongodb.core.mapping.Document; +import javax.validation.constraints.NotNull; import java.util.List; @@ -18,6 +19,7 @@ public class Tenant extends BaseDomain { private String domain; + @NotNull private String name; private String website; diff --git a/app/server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java b/app/server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java index d2f29b184b..b154a39c5b 100644 --- a/app/server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java +++ b/app/server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java @@ -24,7 +24,7 @@ public class GlobalExceptionHandler { * * @param e AppsmithException that will be caught by the function * @param exchange ServerWebExchange contract in order to extract the response and set the http status code - * @return Mono> + * @return Mono> */ @ExceptionHandler @ResponseBody @@ -40,7 +40,7 @@ public class GlobalExceptionHandler { * * @param e Exception that will be caught by the function * @param exchange ServerWebExchange contract in order to extract the response and set the http status code - * @return Mono> + * @return Mono> */ @ExceptionHandler @ResponseBody diff --git a/app/server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java index bca83c9989..8353b9c16f 100644 --- a/app/server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java @@ -14,6 +14,7 @@ import org.springframework.stereotype.Service; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; +import javax.validation.Validator; import javax.validation.constraints.NotNull; @Slf4j @@ -25,25 +26,30 @@ public class ActionServiceImpl extends BaseService create(@NotNull Action action) throws AppsmithException { + public Mono create(@NotNull Action action) { if (action.getId() != null) { - throw new AppsmithException("During create action, Id is not null. Can't create new action."); + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "id")); } else if (action.getResourceId() == null) { - throw new AppsmithException(AppsmithError.RESOURCE_ID_NOT_GIVEN); + return Mono.error(new AppsmithException(AppsmithError.RESOURCE_ID_NOT_GIVEN)); } Mono resourceMono = resourceService.findById(action.getResourceId()); Mono pluginMono = resourceMono.flatMap(resource -> pluginService.findById(resource.getPluginId())); - return pluginMono //Set plugin in the action before saving. .map(plugin -> { diff --git a/app/server/src/main/java/com/appsmith/server/services/BaseService.java b/app/server/src/main/java/com/appsmith/server/services/BaseService.java index 412f2072f8..261ebf2d92 100644 --- a/app/server/src/main/java/com/appsmith/server/services/BaseService.java +++ b/app/server/src/main/java/com/appsmith/server/services/BaseService.java @@ -1,5 +1,6 @@ package com.appsmith.server.services; +import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.BaseDomain; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; @@ -15,7 +16,10 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; +import javax.validation.ConstraintViolation; +import javax.validation.Validator; import java.util.Map; +import java.util.Set; public abstract class BaseService implements CrudService { @@ -27,20 +31,24 @@ public abstract class BaseService update(ID id, T resource) throws Exception { + public Mono update(ID id, T resource) { if (id == null) { - throw new Exception("Invalid id provided"); + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ID)); } Query query = new Query(Criteria.where("id").is(id)); @@ -52,8 +60,7 @@ public abstract class BaseService updateObj.set(entry.getKey(), entry.getValue())); return mongoTemplate.updateFirst(query, updateObj, resource.getClass()) - .flatMap(obj -> repository.findById(id)) - .subscribeOn(scheduler); + .flatMap(obj -> repository.findById(id)); } @Override @@ -63,13 +70,19 @@ public abstract class BaseService getById(ID id) { + if(id == null) { + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ID)); + } + return repository.findById(id) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, "resource", id))); } - + @Override - public Mono create(T object) throws AppsmithException { - return repository.save(object); + public Mono create(T object) { + return Mono.just(object) + .flatMap(this::validateObject) + .flatMap(repository::save); } private DBObject getDbObject(Object o) { @@ -77,4 +90,22 @@ public abstract class BaseService + */ + protected Mono validateObject(T obj) { + return Mono.just(obj) + .map(o -> validator.validate(o)) + .flatMap(constraint -> { + if(constraint.isEmpty()) { + return Mono.just(obj); + } + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, constraint.stream().findFirst().get().getPropertyPath())); + }); + } } \ No newline at end of file diff --git a/app/server/src/main/java/com/appsmith/server/services/CrudService.java b/app/server/src/main/java/com/appsmith/server/services/CrudService.java index 6ffb3dcaf7..f558c74735 100644 --- a/app/server/src/main/java/com/appsmith/server/services/CrudService.java +++ b/app/server/src/main/java/com/appsmith/server/services/CrudService.java @@ -1,7 +1,6 @@ package com.appsmith.server.services; import com.appsmith.server.domains.BaseDomain; -import com.appsmith.server.exceptions.AppsmithException; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -9,9 +8,9 @@ public interface CrudService { Flux get(); - Mono create(T resource) throws AppsmithException; + Mono create(T resource); - Mono update(ID id, T resource) throws Exception; + Mono update(ID id, T resource); Mono getById(ID id); } diff --git a/app/server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java index d9e5501f25..ed974d954f 100644 --- a/app/server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/LayoutServiceImpl.java @@ -9,15 +9,18 @@ import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.stereotype.Service; import reactor.core.scheduler.Scheduler; +import javax.validation.Validator; + @Slf4j @Service public class LayoutServiceImpl extends BaseService implements LayoutService { @Autowired public LayoutServiceImpl(Scheduler scheduler, + Validator validator, MongoConverter mongoConverter, ReactiveMongoTemplate reactiveMongoTemplate, LayoutRepository repository) { - super(scheduler, mongoConverter, reactiveMongoTemplate, repository); + super(scheduler, validator, mongoConverter, reactiveMongoTemplate, repository); } } diff --git a/app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java index 44073e9625..bcf5476058 100644 --- a/app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java @@ -21,6 +21,7 @@ import org.springframework.stereotype.Service; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; +import javax.validation.Validator; import java.util.ArrayList; import java.util.List; @@ -39,6 +40,7 @@ public class PluginServiceImpl extends BaseService create(Plugin plugin) throws AppsmithException { + public Mono create(Plugin plugin) { if (plugin.getId() != null) { - throw new AppsmithException(AppsmithError.INVALID_PARAMETER, "id"); + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "id")); } + plugin.setDeleted(false); return pluginRepository.save(plugin); } diff --git a/app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java index c7241db460..152f123df3 100644 --- a/app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java @@ -14,6 +14,8 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; +import javax.validation.Validator; + @Slf4j @Service public class QueryServiceImpl extends BaseService implements QueryService { @@ -22,11 +24,12 @@ public class QueryServiceImpl extends BaseService create(@NotNull Resource resource) throws AppsmithException { + public Mono create(@NotNull Resource resource) { if (resource.getId() != null) { - throw new AppsmithException(AppsmithError.INVALID_PARAMETER, "id"); + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "id")); } else if (resource.getPluginId() == null) { - throw new AppsmithException(AppsmithError.PLUGIN_ID_NOT_GIVEN); + return Mono.error(new AppsmithException(AppsmithError.PLUGIN_ID_NOT_GIVEN)); } Mono tenantMono = tenantService.findByIdAndPluginsPluginId(tenantId, resource.getPluginId()); diff --git a/app/server/src/main/java/com/appsmith/server/services/SettingServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/SettingServiceImpl.java index 3017ff5281..08dd05c8b7 100644 --- a/app/server/src/main/java/com/appsmith/server/services/SettingServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/SettingServiceImpl.java @@ -9,6 +9,8 @@ import org.springframework.stereotype.Service; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; +import javax.validation.Validator; + @Service public class SettingServiceImpl extends BaseService implements SettingService { @@ -16,10 +18,11 @@ public class SettingServiceImpl extends BaseService create(Tenant tenant) { + if (tenant == null) { + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.TENANT)); + } - log.debug("Going to create the tenant"); + log.debug("Going to create the tenant {}", tenant.getName()); return Mono.just(tenant) + .flatMap(this::validateObject) //transform the tenant data to embed setting object in each object in tenantSetting list. .flatMap(this::enhanceTenantSettingList) //Call the library function to save the updated tenant - .flatMap(tenantUpdated -> repository.save(tenantUpdated)) - .subscribeOn(scheduler); + .flatMap(repository::save); } private Mono enhanceTenantSettingList(Tenant tenant) { - if (tenant.getTenantSettings() == null) tenant.setTenantSettings(new ArrayList<>()); + if (tenant.getTenantSettings() == null) { + tenant.setTenantSettings(new ArrayList<>()); + } Flux tenantSettingFlux = Flux.fromIterable(tenant.getTenantSettings()); // For each tenant setting, fetch and embed the setting, and once all the tenant setting are done, collect it diff --git a/app/server/src/main/java/com/appsmith/server/services/UserServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/UserServiceImpl.java index e16eef5a3f..62494ed4ff 100644 --- a/app/server/src/main/java/com/appsmith/server/services/UserServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/UserServiceImpl.java @@ -11,16 +11,19 @@ import org.springframework.stereotype.Service; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; +import javax.validation.Validator; + @Service public class UserServiceImpl extends BaseService implements UserService, UserDetailsService { private UserRepository repository; public UserServiceImpl(Scheduler scheduler, + Validator validator, MongoConverter mongoConverter, ReactiveMongoTemplate reactiveMongoTemplate, UserRepository repository) { - super(scheduler, mongoConverter, reactiveMongoTemplate, repository); + super(scheduler, validator, mongoConverter, reactiveMongoTemplate, repository); this.repository = repository; } diff --git a/app/server/src/main/java/com/appsmith/server/services/WidgetServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/WidgetServiceImpl.java index 9c5586f3a3..a7886d99f5 100644 --- a/app/server/src/main/java/com/appsmith/server/services/WidgetServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/WidgetServiceImpl.java @@ -10,6 +10,8 @@ import org.springframework.stereotype.Service; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; +import javax.validation.Validator; + @Service @Slf4j public class WidgetServiceImpl extends BaseService implements WidgetService { @@ -18,10 +20,11 @@ public class WidgetServiceImpl extends BaseService tenantResponse = tenantService.create(null); + StepVerifier.create(tenantResponse) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException && + throwable.getMessage().equals(AppsmithError.INVALID_PARAMETER.getMessage(FieldName.TENANT))) + .verify(); + } + + @Test + public void nullName() { + tenant.setName(null); + Mono tenantResponse = tenantService.create(tenant); + StepVerifier.create(tenantResponse) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException && + throwable.getMessage().equals(AppsmithError.INVALID_PARAMETER.getMessage(FieldName.NAME))) + .verify(); + } + + @Test + public void validCreateTenantTest() { + Mono tenantResponse = tenantService.create(tenant); + StepVerifier.create(tenantResponse) + .assertNext(tenant1 -> { + assertThat(tenant1.getName()).isEqualTo("Test Name"); + }) + .verifyComplete(); + } + + /* Tests for Get Tenant Flow */ + + @Test + public void getTenantInvalidId() { + Mono tenantMono = tenantService.getById("random-id"); + StepVerifier.create(tenantMono) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException && + throwable.getMessage().equals(AppsmithError.NO_RESOURCE_FOUND.getMessage("resource", "random-id"))) + .verify(); + } + + @Test + public void getTenantNullId() { + Mono tenantMono = tenantService.getById(null); + StepVerifier.create(tenantMono) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException && + throwable.getMessage().equals(AppsmithError.INVALID_PARAMETER.getMessage(FieldName.ID))) + .verify(); + } + + @Test + public void validGetTenantByName() { + Mono createTenant = tenantService.create(tenant); + Mono getTenant = createTenant.flatMap(t -> tenantService.getById(t.getId())); + StepVerifier.create(getTenant) + .assertNext(t -> { + assertThat(t).isNotNull(); + assertThat(t.getName()).isEqualTo(tenant.getName()); + assertThat(t.getId()).isEqualTo(tenant.getId()); + }) + .verifyComplete(); + } + + /* Tests for Update Tenant Flow */ + @Test + public void validUpdateTenant() { + Tenant tenant = new Tenant(); + tenant.setName("Test Name"); + tenant.setDomain("example.com"); + tenant.setWebsite("https://example.com"); + + Mono createTenant = tenantService.create(tenant); + Mono updateTenant = createTenant + .map(t -> { + t.setDomain("abc.com"); + return t; + }) + .flatMap(t -> tenantService.update(t.getId(), t)) + .flatMap(t -> tenantService.getById(t.getId())); + + StepVerifier.create(updateTenant) + .assertNext(t -> { + assertThat(t).isNotNull(); + assertThat(t.getName()).isEqualTo(tenant.getName()); + assertThat(t.getId()).isEqualTo(tenant.getId()); + assertThat(t.getDomain()).isEqualTo("abc.com"); + }) + .verifyComplete(); + } +}