Disable uninvited signups via environment variable (#2512)
* Disable signup API via environment variable * Allow signup for invited users, even if disabled publicly * Add test for signup when signup is disabled * Run invite flow tests when signup is disabled * Revert status annotation in signup API endpoint * Remove unused tests on invite flow * Change signup disabled error message
This commit is contained in:
parent
6704e05ad0
commit
9cd9edd3fa
|
|
@ -25,6 +25,9 @@ public class CommonConfig {
|
|||
|
||||
private static final String ELASTIC_THREAD_POOL_NAME = "appsmith-elastic-pool";
|
||||
|
||||
@Value("${signup.disabled}")
|
||||
private boolean isSignupDisabled;
|
||||
|
||||
@Value("${oauth2.allowed-domains}")
|
||||
private String allowedDomainList;
|
||||
|
||||
|
|
|
|||
|
|
@ -69,7 +69,8 @@ public enum AppsmithError {
|
|||
INVALID_CURL_METHOD(400, 4032, "Invalid method in cURL command: {0}.", AppsmithErrorAction.DEFAULT),
|
||||
OAUTH_NOT_AVAILABLE(500, 5006, "Login with {0} is not supported.", AppsmithErrorAction.LOG_EXTERNALLY),
|
||||
MARKETPLACE_NOT_CONFIGURED(500, 5007, "Marketplace is not configured.", AppsmithErrorAction.DEFAULT),
|
||||
PAYLOAD_TOO_LARGE(413, 4028, "The request payload is too large. Max allowed size for request payload is {0} KB", AppsmithErrorAction.DEFAULT)
|
||||
PAYLOAD_TOO_LARGE(413, 4028, "The request payload is too large. Max allowed size for request payload is {0} KB", AppsmithErrorAction.DEFAULT),
|
||||
SIGNUP_DISABLED(403, 4033, "Signup is restricted on this instance of Appsmith. Please contact the administrator to get an invite.", AppsmithErrorAction.DEFAULT),
|
||||
;
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import com.appsmith.external.models.Policy;
|
|||
import com.appsmith.server.acl.AclPermission;
|
||||
import com.appsmith.server.acl.AppsmithRole;
|
||||
import com.appsmith.server.acl.RoleGraph;
|
||||
import com.appsmith.server.configurations.CommonConfig;
|
||||
import com.appsmith.server.constants.FieldName;
|
||||
import com.appsmith.server.domains.Application;
|
||||
import com.appsmith.server.domains.InviteUser;
|
||||
|
|
@ -69,6 +70,7 @@ public class UserServiceImpl extends BaseService<UserRepository, User, String> i
|
|||
private final UserOrganizationService userOrganizationService;
|
||||
private final RoleGraph roleGraph;
|
||||
private final ConfigService configService;
|
||||
private final CommonConfig commonConfig;
|
||||
|
||||
private static final String WELCOME_USER_EMAIL_TEMPLATE = "email/welcomeUserTemplate.html";
|
||||
private static final String FORGOT_PASSWORD_EMAIL_TEMPLATE = "email/forgotPasswordTemplate.html";
|
||||
|
|
@ -96,7 +98,8 @@ public class UserServiceImpl extends BaseService<UserRepository, User, String> i
|
|||
OrganizationRepository organizationRepository,
|
||||
UserOrganizationService userOrganizationService,
|
||||
RoleGraph roleGraph,
|
||||
ConfigService configService) {
|
||||
ConfigService configService,
|
||||
CommonConfig commonConfig) {
|
||||
super(scheduler, validator, mongoConverter, reactiveMongoTemplate, repository, analyticsService);
|
||||
this.organizationService = organizationService;
|
||||
this.analyticsService = analyticsService;
|
||||
|
|
@ -110,6 +113,7 @@ public class UserServiceImpl extends BaseService<UserRepository, User, String> i
|
|||
this.userOrganizationService = userOrganizationService;
|
||||
this.roleGraph = roleGraph;
|
||||
this.configService = configService;
|
||||
this.commonConfig = commonConfig;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -515,7 +519,11 @@ public class UserServiceImpl extends BaseService<UserRepository, User, String> i
|
|||
}
|
||||
return Mono.error(new AppsmithException(AppsmithError.USER_ALREADY_EXISTS_SIGNUP, user.getUsername()));
|
||||
})
|
||||
.switchIfEmpty(userCreate(user))
|
||||
.switchIfEmpty(
|
||||
commonConfig.isSignupDisabled()
|
||||
? Mono.error(new AppsmithException(AppsmithError.SIGNUP_DISABLED))
|
||||
: userCreate(user)
|
||||
)
|
||||
.flatMap(savedUser -> sendWelcomeEmail(savedUser, finalOriginHeader));
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -77,3 +77,6 @@ management.metrics.web.server.request.ignore-trailing-slash=true
|
|||
management.metrics.web.server.request.autotime.percentiles=0.5, 0.9, 0.95, 0.99
|
||||
management.metrics.web.server.request.autotime.percentiles-histogram=true
|
||||
management.metrics.distribution.sla.[http.server.requests]=1s
|
||||
|
||||
# Support disabling signup with an environment variable
|
||||
signup.disabled = ${APPSMITH_SIGNUP_DISABLED:false}
|
||||
|
|
|
|||
|
|
@ -263,30 +263,6 @@ public class UserServiceTest {
|
|||
.verifyComplete();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockAppsmithUser
|
||||
public void confirmInviteTokenFlow() {
|
||||
User newUser = new User();
|
||||
newUser.setEmail("newEmail@newEmail.com");
|
||||
newUser.setIsEnabled(false);
|
||||
newUser.setInviteToken("inviteToken");
|
||||
|
||||
userRepository.save(newUser).block();
|
||||
|
||||
newUser.setPassword("newPassword");
|
||||
|
||||
Mono<User> afterConfirmationUserMono = userService.confirmInviteUser(newUser, "http://localhost:8080")
|
||||
.then(userRepository.findByEmail("newEmail@newEmail.com"));
|
||||
|
||||
StepVerifier.create(afterConfirmationUserMono)
|
||||
.assertNext(user -> {
|
||||
assertThat(user).isNotNull();
|
||||
assertThat(user.getIsEnabled()).isTrue();
|
||||
})
|
||||
.verifyComplete();
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockAppsmithUser
|
||||
public void signUpViaFormLoginIfAlreadyInvited() {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,199 @@
|
|||
package com.appsmith.server.services;
|
||||
|
||||
import com.appsmith.external.models.Policy;
|
||||
import com.appsmith.server.acl.AppsmithRole;
|
||||
import com.appsmith.server.configurations.CommonConfig;
|
||||
import com.appsmith.server.configurations.WithMockAppsmithUser;
|
||||
import com.appsmith.server.domains.Application;
|
||||
import com.appsmith.server.domains.InviteUser;
|
||||
import com.appsmith.server.domains.LoginSource;
|
||||
import com.appsmith.server.domains.Organization;
|
||||
import com.appsmith.server.domains.User;
|
||||
import com.appsmith.server.exceptions.AppsmithError;
|
||||
import com.appsmith.server.exceptions.AppsmithException;
|
||||
import com.appsmith.server.repositories.UserRepository;
|
||||
import com.appsmith.server.solutions.UserSignup;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.security.crypto.password.PasswordEncoder;
|
||||
import org.springframework.security.test.context.support.WithUserDetails;
|
||||
import org.springframework.test.annotation.DirtiesContext;
|
||||
import org.springframework.test.context.junit4.SpringRunner;
|
||||
import reactor.core.publisher.Mono;
|
||||
import reactor.test.StepVerifier;
|
||||
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
|
||||
import static com.appsmith.server.acl.AclPermission.MANAGE_APPLICATIONS;
|
||||
import static com.appsmith.server.acl.AclPermission.READ_APPLICATIONS;
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
@Slf4j
|
||||
@RunWith(SpringRunner.class)
|
||||
@SpringBootTest(properties = { "signup.disabled = true" })
|
||||
@DirtiesContext
|
||||
public class UserServiceWithDisabledSignupTest {
|
||||
|
||||
@Autowired
|
||||
UserService userService;
|
||||
|
||||
@Autowired
|
||||
OrganizationService organizationService;
|
||||
|
||||
@Autowired
|
||||
ApplicationService applicationService;
|
||||
|
||||
@Autowired
|
||||
UserRepository userRepository;
|
||||
|
||||
@Autowired
|
||||
PasswordEncoder passwordEncoder;
|
||||
|
||||
@Autowired
|
||||
CommonConfig commonConfig;
|
||||
|
||||
Mono<User> userMono;
|
||||
|
||||
Mono<Organization> organizationMono;
|
||||
|
||||
@Autowired
|
||||
UserSignup userSignup;
|
||||
|
||||
@Before
|
||||
public void setup() {
|
||||
userMono = userService.findByEmail("usertest@usertest.com");
|
||||
organizationMono = organizationService.getBySlug("spring-test-organization");
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockAppsmithUser
|
||||
public void createNewUserValidWhenDisabled() {
|
||||
User newUser = new User();
|
||||
newUser.setEmail("new-user-email@email.com");
|
||||
newUser.setPassword("new-user-test-password");
|
||||
|
||||
Mono<User> userMono = userService.create(newUser);
|
||||
|
||||
StepVerifier.create(userMono)
|
||||
.expectErrorMatches(error -> {
|
||||
assertThat(error).isInstanceOf(AppsmithException.class);
|
||||
assertThat(((AppsmithException) error).getError()).isEqualTo(AppsmithError.SIGNUP_DISABLED);
|
||||
return true;
|
||||
})
|
||||
.verify();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DirtiesContext
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void inviteUserToApplicationValidAsAdmin() {
|
||||
InviteUser inviteUser = new InviteUser();
|
||||
inviteUser.setEmail("inviteUserToApplication@test.com");
|
||||
inviteUser.setRole(AppsmithRole.APPLICATION_ADMIN);
|
||||
|
||||
Mono<Application> applicationMono = applicationService.findByName("LayoutServiceTest TestApplications", MANAGE_APPLICATIONS)
|
||||
.switchIfEmpty(Mono.error(new Exception("No such app")));
|
||||
|
||||
Mono<User> userMono = applicationMono.flatMap(application -> userService
|
||||
.inviteUserToApplication(inviteUser, "http://localhost:8080", application.getId())).cache();
|
||||
|
||||
Mono<Application> updatedApplication = userMono.then(applicationService.findByName("LayoutServiceTest TestApplications", MANAGE_APPLICATIONS));
|
||||
|
||||
StepVerifier.create(Mono.zip(updatedApplication, userMono))
|
||||
.assertNext(tuple -> {
|
||||
Application application = tuple.getT1();
|
||||
User user = tuple.getT2();
|
||||
|
||||
Policy manageAppPolicy = Policy.builder()
|
||||
.permission(MANAGE_APPLICATIONS.getValue())
|
||||
.users(Set.of("api_user", user.getUsername()))
|
||||
.groups(new HashSet<>())
|
||||
.build();
|
||||
|
||||
Policy readAppPolicy = Policy.builder()
|
||||
.permission(READ_APPLICATIONS.getValue())
|
||||
.users(Set.of("api_user", user.getUsername()))
|
||||
.groups(new HashSet<>())
|
||||
.build();
|
||||
|
||||
|
||||
assertThat(application).isNotNull();
|
||||
assertThat(application.getPolicies()).isNotEmpty();
|
||||
assertThat(application.getPolicies()).containsAll(Set.of(manageAppPolicy, readAppPolicy));
|
||||
|
||||
assertThat(user).isNotNull();
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void inviteUserToApplicationValidAsViewer() {
|
||||
InviteUser inviteUser = new InviteUser();
|
||||
inviteUser.setEmail("inviteUserToApplication@test.com");
|
||||
inviteUser.setRole(AppsmithRole.APPLICATION_VIEWER);
|
||||
|
||||
Mono<Application> applicationMono = applicationService.findByName("LayoutServiceTest TestApplications", READ_APPLICATIONS)
|
||||
.switchIfEmpty(Mono.error(new Exception("No such app")));
|
||||
|
||||
Mono<User> userMono = applicationMono.flatMap(application -> userService
|
||||
.inviteUserToApplication(inviteUser, "http://localhost:8080", application.getId())).cache();
|
||||
|
||||
Mono<Application> updatedApplication = userMono.then(applicationService.findByName("LayoutServiceTest TestApplications", READ_APPLICATIONS));
|
||||
|
||||
|
||||
StepVerifier.create(Mono.zip(updatedApplication, userMono))
|
||||
.assertNext(tuple -> {
|
||||
Application application = tuple.getT1();
|
||||
User user = tuple.getT2();
|
||||
|
||||
Policy readAppPolicy = Policy.builder()
|
||||
.permission(READ_APPLICATIONS.getValue())
|
||||
.users(Set.of("api_user", user.getUsername()))
|
||||
.groups(new HashSet<>())
|
||||
.build();
|
||||
|
||||
Policy manageAppPolicy = Policy.builder()
|
||||
.permission(MANAGE_APPLICATIONS.getValue())
|
||||
.users(Set.of("api_user"))
|
||||
.build();
|
||||
|
||||
assertThat(application).isNotNull();
|
||||
assertThat(application.getPolicies()).isNotEmpty();
|
||||
assertThat(application.getPolicies()).containsAll(Set.of(manageAppPolicy, readAppPolicy));
|
||||
|
||||
assertThat(user).isNotNull();
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockAppsmithUser
|
||||
public void signUpViaFormLoginIfAlreadyInvited() {
|
||||
User newUser = new User();
|
||||
newUser.setEmail("alreadyInvited@alreadyInvited.com");
|
||||
newUser.setIsEnabled(false);
|
||||
|
||||
userRepository.save(newUser).block();
|
||||
|
||||
User signupUser = new User();
|
||||
signupUser.setEmail(newUser.getEmail());
|
||||
signupUser.setPassword("password");
|
||||
signupUser.setSource(LoginSource.FORM);
|
||||
|
||||
Mono<User> userMono = userService.create(signupUser);
|
||||
|
||||
StepVerifier.create(userMono)
|
||||
.assertNext(user -> {
|
||||
assertThat(user.getEmail().equals(newUser.getEmail()));
|
||||
assertThat(user.getSource().equals(LoginSource.FORM));
|
||||
assertThat(user.getIsEnabled()).isTrue();
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user