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 bt...@apache.org on 2019/11/04 11:23:46 UTC
[james-project] 10/30: JAMES-2948 Improve error message for
WebAdmin's user creation username errors
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 <gd...@linagora.com>
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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org