You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by rc...@apache.org on 2020/02/06 03:54:48 UTC

[james-project] 15/16: JAMES-2950 Add routes tests to check that we cannot create users with illegal characters in their local part

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 <rc...@linagora.com>
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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org