This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit c664b17b77bd0e8b3d258826cb4ed392dcdf6b0c
Author: Gautier DI FOLCO <[email protected]>
AuthorDate: Wed Oct 30 14:32:33 2019 +0100

    JAMES-2948 Improve error message for WebAdmin's user creation username 
errors
---
 .../org/apache/james/webadmin/routes/UserRoutes.java     | 16 ++++++++++++++++
 .../org/apache/james/webadmin/service/UserService.java   | 14 ++++++++++++--
 .../apache/james/webadmin/routes/UsersRoutesTest.java    | 12 ++++++++++--
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git 
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
 
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
index 75f4a92..9006e96 100644
--- 
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
+++ 
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
@@ -150,6 +150,14 @@ public class UserRoutes implements Routes {
                 .message("The user " + username + " does not exists")
                 .cause(e)
                 .haltError();
+        } catch (UserService.InvalidUsername e) {
+            LOGGER.info("Invalid username", e);
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorType.INVALID_ARGUMENT)
+                .message("Invalid username: it should be between 1 and 255 
long without '/'")
+                .cause(e)
+                .haltError();
         } catch (IllegalArgumentException e) {
             LOGGER.info("Invalid user path", e);
             throw ErrorResponder.builder()
@@ -174,6 +182,14 @@ public class UserRoutes implements Routes {
                 .message("Error while deserializing addUser request")
                 .cause(e)
                 .haltError();
+        } catch (UserService.InvalidUsername e) {
+            LOGGER.info("Invalid username", e);
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorType.INVALID_ARGUMENT)
+                .message("Invalid username: it should be between 1 and 255 
long without '/'")
+                .cause(e)
+                .haltError();
         } catch (IllegalArgumentException e) {
             LOGGER.info("Invalid user path", e);
             throw ErrorResponder.builder()
diff --git 
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
 
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
index 6f6fe41..c31357b 100644
--- 
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
+++ 
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
@@ -43,6 +43,12 @@ import spark.Response;
 
 public class UserService {
 
+    public static class InvalidUsername extends RuntimeException {
+        InvalidUsername(Exception e) {
+            super("Username invariants violation", e);
+        }
+    }
+
     private static final Logger LOGGER = 
LoggerFactory.getLogger(UserService.class);
     private static final String EMPTY_BODY = "";
     public static final int MAXIMUM_MAIL_ADDRESS_LENGTH = 255;
@@ -81,8 +87,12 @@ public class UserService {
     }
 
     private void usernamePreconditions(String username) {
-        Preconditions.checkArgument(!Strings.isNullOrEmpty(username));
-        Preconditions.checkArgument(username.length() < 
MAXIMUM_MAIL_ADDRESS_LENGTH);
+        try {
+            Preconditions.checkArgument(!Strings.isNullOrEmpty(username));
+            Preconditions.checkArgument(username.length() < 
MAXIMUM_MAIL_ADDRESS_LENGTH);
+        } catch (IllegalArgumentException e) {
+            throw new InvalidUsername(e);
+        }
     }
 
     private void upsert(User user, String username, char[] password) throws 
UsersRepositoryException {
diff --git 
a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UsersRoutesTest.java
 
b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UsersRoutesTest.java
index 6476048..dbb4fc4 100644
--- 
a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UsersRoutesTest.java
+++ 
b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UsersRoutesTest.java
@@ -23,6 +23,7 @@ import static io.restassured.RestAssured.given;
 import static io.restassured.RestAssured.when;
 import static io.restassured.RestAssured.with;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.is;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
@@ -40,6 +41,7 @@ import org.apache.james.user.memory.MemoryUsersRepository;
 import org.apache.james.webadmin.WebAdminServer;
 import org.apache.james.webadmin.WebAdminUtils;
 import org.apache.james.webadmin.service.UserService;
+import org.apache.james.webadmin.utils.ErrorResponder;
 import org.apache.james.webadmin.utils.JsonTransformer;
 import org.eclipse.jetty.http.HttpStatus;
 import org.junit.jupiter.api.AfterEach;
@@ -223,7 +225,10 @@ class UsersRoutesTest {
                     
"0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789."
 +
                     "0123456789.0123456789.0123456789.")
             .then()
-                .statusCode(HttpStatus.BAD_REQUEST_400);
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .body("statusCode", is(400))
+                .body("type", 
is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                .body("message", is("Invalid username: it should be between 1 
and 255 long without '/'"));
         }
 
         @Test
@@ -255,7 +260,10 @@ class UsersRoutesTest {
                     
"0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789."
 +
                     "0123456789.0123456789.0123456789.")
             .then()
-                .statusCode(HttpStatus.BAD_REQUEST_400);
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .body("statusCode", is(400))
+                .body("type", 
is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                .body("message", is("Invalid username: it should be between 1 
and 255 long without '/'"));
         }
 
         @Test


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to