This is an automated email from the ASF dual-hosted git repository. rcordier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit da95587404a3f658b7978b602ed8068acd049bc4 Author: Rene Cordier <[email protected]> AuthorDate: Thu Oct 31 16:33:05 2019 +0700 JAMES-2950 Add routes tests to check that we cannot create users with illegal characters in their local part --- .../james/user/lib/AbstractUsersRepository.java | 4 +- .../james/webadmin/routes/UserRoutesTest.java | 74 +++++++++++++++++++--- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java b/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java index d0dd787..b770ff1 100644 --- a/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java +++ b/server/data/data-library/src/main/java/org/apache/james/user/lib/AbstractUsersRepository.java @@ -96,7 +96,7 @@ public abstract class AbstractUsersRepository implements UsersRepository, Config } } - if (!assertLocalPartValid(username)) { + if (!isLocalPartValid(username)) { throw new InvalidUsernameException(String.format("Given Username '%s' should not contain any of those characters: %s", username.asString(), ILLEGAL_USERNAME_CHARACTERS)); } @@ -156,7 +156,7 @@ public abstract class AbstractUsersRepository implements UsersRepository, Config } } - private boolean assertLocalPartValid(Username username) { + private boolean isLocalPartValid(Username username) { return CharMatcher.anyOf(ILLEGAL_USERNAME_CHARACTERS) .matchesNoneOf(username.getLocalPart()); } diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java index fc83f11..89112c7 100644 --- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java +++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java @@ -32,11 +32,12 @@ import static org.mockito.Mockito.when; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; import org.apache.james.core.Domain; import org.apache.james.core.Username; -import org.apache.james.domainlist.api.DomainList; +import org.apache.james.domainlist.api.DomainListException; import org.apache.james.domainlist.api.mock.SimpleDomainList; import org.apache.james.user.api.UsersRepository; import org.apache.james.user.api.UsersRepositoryException; @@ -50,6 +51,7 @@ import org.apache.james.webadmin.utils.JsonTransformer; import org.eclipse.jetty.http.HttpStatus; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.ExtensionContext; @@ -57,6 +59,9 @@ import org.junit.jupiter.api.extension.ParameterContext; import org.junit.jupiter.api.extension.ParameterResolutionException; import org.junit.jupiter.api.extension.ParameterResolver; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import com.google.common.collect.ImmutableMap; @@ -67,16 +72,22 @@ class UserRoutesTest { private static class UserRoutesExtension implements BeforeEachCallback, AfterEachCallback, ParameterResolver { - static UserRoutesExtension withVirtualHosting() { - SimpleDomainList domainList = new SimpleDomainList(); + static UserRoutesExtension withVirtualHosting() throws DomainListException { + SimpleDomainList domainList = setupDomainList(); return new UserRoutesExtension(MemoryUsersRepository.withVirtualHosting(domainList), domainList); } - static UserRoutesExtension withoutVirtualHosting() { - SimpleDomainList domainList = new SimpleDomainList(); + static UserRoutesExtension withoutVirtualHosting() throws DomainListException { + SimpleDomainList domainList = setupDomainList(); return new UserRoutesExtension(MemoryUsersRepository.withoutVirtualHosting(domainList), domainList); } + private static SimpleDomainList setupDomainList() throws DomainListException { + SimpleDomainList domainList = new SimpleDomainList(); + domainList.addDomain(DOMAIN); + return domainList; + } + final MemoryUsersRepository usersRepository; final SimpleDomainList domainList; @@ -88,9 +99,7 @@ class UserRoutesTest { } @Override - public void beforeEach(ExtensionContext extensionContext) throws Exception { - domainList.addDomain(DOMAIN); - + public void beforeEach(ExtensionContext extensionContext) { webAdminServer = startServer(usersRepository); } @@ -310,6 +319,39 @@ class UserRoutesTest { } } + @TestInstance(TestInstance.Lifecycle.PER_CLASS) + interface IllegalCharactersErrorHandlingContract { + + default Stream<Arguments> illegalCharacters() { + return Stream.of( + Arguments.of("\""), + Arguments.of("("), + Arguments.of(")"), + Arguments.of(","), + Arguments.of(":"), + Arguments.of(";"), + Arguments.of("<"), + Arguments.of(">"), + Arguments.of("@"), + Arguments.of("["), + Arguments.of("\\"), + Arguments.of("]"), + Arguments.of(" ") + ); + } + + @ParameterizedTest + @MethodSource("illegalCharacters") + default void putShouldReturnBadRequestWhenUsernameContainsSpecialCharacter(String illegalCharacter) { + given() + .body("{\"password\":\"password\"}") + .when() + .put("user" + illegalCharacter + "name@" + DOMAIN.name()) + .then() + .statusCode(HttpStatus.BAD_REQUEST_400); + } + } + interface MockBehaviorErrorHandlingContract { @Test @@ -443,6 +485,9 @@ class UserRoutesTest { @RegisterExtension UserRoutesExtension extension = UserRoutesExtension.withVirtualHosting(); + WithVirtualHosting() throws DomainListException { + } + @Test void puttingWithDomainPartInUsernameTwoTimesShouldBeAllowed() { // Given @@ -527,6 +572,11 @@ class UserRoutesTest { .then() .statusCode(HttpStatus.NO_CONTENT_204); } + + @Nested + class IllegalCharacterErrorHandlingTest implements UserRoutesContract.IllegalCharactersErrorHandlingContract { + + } } @Nested @@ -535,6 +585,9 @@ class UserRoutesTest { @RegisterExtension UserRoutesExtension extension = UserRoutesExtension.withoutVirtualHosting(); + WithoutVirtualHosting() throws DomainListException { + } + @Test void puttingWithoutDomainPartInUsernameTwoTimesShouldBeAllowed() { // Given @@ -619,5 +672,10 @@ class UserRoutesTest { .then() .statusCode(HttpStatus.NO_CONTENT_204); } + + @Nested + class IllegalCharacterErrorHandlingTest implements UserRoutesContract.IllegalCharactersErrorHandlingContract { + + } } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
