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
The following commit(s) were added to refs/heads/master by this push:
new 27b703b JAMES-3657 Modular entity validation for webadmin-data (#678)
27b703b is described below
commit 27b703b8f14dab03eefdc6264d96d844da3e1358
Author: Benoit TELLIER <[email protected]>
AuthorDate: Tue Oct 5 09:00:27 2021 +0700
JAMES-3657 Modular entity validation for webadmin-data (#678)
**Why?**: Adding a new entity requires to modify all the other
entity, which is error prone, and also fastidious...
These entity includes: groups, aliases, users.
**How?**: Write a modular validation system that all types would use.
Adding a new type would just require to add its validator.
**Side fixes**
* JAMES-3657 PUT aliases 400 -> 409 if the user is referenced by another
entity
---
.../pages/distributed/operate/webadmin.adoc | 2 +-
.../james/modules/server/DataRoutesModules.java | 17 +++++
.../apache/james/webadmin/routes/AliasRoutes.java | 25 ++++---
.../apache/james/webadmin/routes/GroupsRoutes.java | 21 ++++--
.../apache/james/webadmin/routes/UserRoutes.java | 7 +-
.../service/AggregateUserEntityValidator.java | 43 +++++++++++
.../service/DefaultUserEntityValidator.java | 49 +++++++++++++
.../RecipientRewriteTableUserEntityValidator.java | 68 ++++++++++++++++++
.../webadmin/service/UserEntityValidator.java | 84 ++++++++++++++++++++++
.../apache/james/webadmin/service/UserService.java | 27 +++----
.../james/webadmin/routes/AliasRoutesTest.java | 41 +++++++++--
.../james/webadmin/routes/GroupsRoutesTest.java | 35 +++++++--
.../james/webadmin/routes/UserRoutesTest.java | 58 ++++++++++++---
src/site/markdown/server/manage-webadmin.md | 2 +-
14 files changed, 422 insertions(+), 57 deletions(-)
diff --git a/docs/modules/servers/pages/distributed/operate/webadmin.adoc
b/docs/modules/servers/pages/distributed/operate/webadmin.adoc
index 7eb8320..4e4c439 100644
--- a/docs/modules/servers/pages/distributed/operate/webadmin.adoc
+++ b/docs/modules/servers/pages/distributed/operate/webadmin.adoc
@@ -2593,10 +2593,10 @@ Response codes:
* 204: OK
* 400: Alias structure or member is not valid
-* 400: The alias source exists as an user already
* 400: Source and destination can’t be the same!
* 400: Domain in the destination or source is not managed by the
DomainList
+* 409: The alias source exists as an user already
* 409: The addition of the alias would lead to a loop and thus cannot be
performed
==== Removing an alias of an user
diff --git
a/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java
b/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java
index 1014264..ef49eac 100644
---
a/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java
+++
b/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java
@@ -19,6 +19,8 @@
package org.apache.james.modules.server;
+import java.util.Set;
+
import org.apache.james.webadmin.Routes;
import org.apache.james.webadmin.dto.MappingSourceModule;
import org.apache.james.webadmin.mdc.RequestLogger;
@@ -32,9 +34,15 @@ import org.apache.james.webadmin.routes.MappingRoutes;
import org.apache.james.webadmin.routes.RegexMappingRoutes;
import org.apache.james.webadmin.routes.UserCreationRequestLogger;
import org.apache.james.webadmin.routes.UserRoutes;
+import org.apache.james.webadmin.service.AggregateUserEntityValidator;
+import org.apache.james.webadmin.service.DefaultUserEntityValidator;
+import
org.apache.james.webadmin.service.RecipientRewriteTableUserEntityValidator;
+import org.apache.james.webadmin.service.UserEntityValidator;
import org.apache.james.webadmin.utils.JsonTransformerModule;
import com.google.inject.AbstractModule;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
import com.google.inject.multibindings.Multibinder;
public class DataRoutesModules extends AbstractModule {
@@ -56,5 +64,14 @@ public class DataRoutesModules extends AbstractModule {
jsonTransformerModuleMultibinder.addBinding().to(MappingSourceModule.class);
Multibinder.newSetBinder(binder(),
RequestLogger.class).addBinding().to(UserCreationRequestLogger.class);
+
+ Multibinder.newSetBinder(binder(),
UserEntityValidator.class).addBinding().to(DefaultUserEntityValidator.class);
+ Multibinder.newSetBinder(binder(),
UserEntityValidator.class).addBinding().to(RecipientRewriteTableUserEntityValidator.class);
+ }
+
+ @Provides
+ @Singleton
+ UserEntityValidator userEntityValidator(Set<UserEntityValidator>
validatorSet) {
+ return new AggregateUserEntityValidator(validatorSet);
}
}
diff --git
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/AliasRoutes.java
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/AliasRoutes.java
index 82c96a6..9109916 100644
---
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/AliasRoutes.java
+++
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/AliasRoutes.java
@@ -20,10 +20,12 @@
package org.apache.james.webadmin.routes;
import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static
org.apache.james.webadmin.service.UserEntityValidator.EntityType.ALIAS;
import static spark.Spark.halt;
import java.util.Comparator;
import java.util.List;
+import java.util.Optional;
import javax.inject.Inject;
import javax.ws.rs.DELETE;
@@ -46,10 +48,11 @@ import
org.apache.james.rrt.api.SourceDomainIsNotInDomainListException;
import org.apache.james.rrt.lib.Mapping;
import org.apache.james.rrt.lib.MappingSource;
import org.apache.james.user.api.UsersRepository;
-import org.apache.james.user.api.UsersRepositoryException;
import org.apache.james.webadmin.Constants;
import org.apache.james.webadmin.Routes;
import org.apache.james.webadmin.dto.AliasSourcesResponse;
+import org.apache.james.webadmin.service.UserEntityValidator;
+import org.apache.james.webadmin.service.UserEntityValidator.ValidationFailure;
import org.apache.james.webadmin.utils.ErrorResponder;
import org.apache.james.webadmin.utils.JsonTransformer;
import org.eclipse.jetty.http.HttpStatus;
@@ -86,14 +89,16 @@ public class AliasRoutes implements Routes {
private static final String ADDRESS_TYPE = "alias";
private final UsersRepository usersRepository;
+ private final UserEntityValidator userEntityValidator;
private final DomainList domainList;
private final JsonTransformer jsonTransformer;
private final RecipientRewriteTable recipientRewriteTable;
@Inject
@VisibleForTesting
- AliasRoutes(RecipientRewriteTable recipientRewriteTable, UsersRepository
usersRepository, DomainList domainList, JsonTransformer jsonTransformer) {
+ AliasRoutes(RecipientRewriteTable recipientRewriteTable, UsersRepository
usersRepository, UserEntityValidator userEntityValidator, DomainList
domainList, JsonTransformer jsonTransformer) {
this.usersRepository = usersRepository;
+ this.userEntityValidator = userEntityValidator;
this.domainList = domainList;
this.jsonTransformer = jsonTransformer;
this.recipientRewriteTable = recipientRewriteTable;
@@ -143,13 +148,13 @@ public class AliasRoutes implements Routes {
@ApiResponses(value = {
@ApiResponse(code = HttpStatus.NO_CONTENT_204, message = "OK"),
@ApiResponse(code = HttpStatus.BAD_REQUEST_400, message =
ALIAS_DESTINATION_ADDRESS + " or alias structure format is not valid"),
- @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "The alias
source exists as an user already"),
+ @ApiResponse(code = HttpStatus.CONFLICT_409, message = "The alias
source exists as an user already"),
@ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Source and
destination can't be the same!"),
@ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Domain in
the destination or source is not managed by the DomainList"),
@ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500,
message = "Internal server error - Something went bad on the
server side.")
})
- public HaltException addAlias(Request request, Response response) throws
UsersRepositoryException, RecipientRewriteTableException, DomainListException {
+ public HaltException addAlias(Request request, Response response) throws
Exception {
MailAddress aliasSourceAddress =
MailAddressParser.parseMailAddress(request.params(ALIAS_SOURCE_ADDRESS),
ADDRESS_TYPE);
ensureUserDoesNotExist(aliasSourceAddress);
MailAddress destinationAddress =
MailAddressParser.parseMailAddress(request.params(ALIAS_DESTINATION_ADDRESS),
ADDRESS_TYPE);
@@ -189,14 +194,14 @@ public class AliasRoutes implements Routes {
}
}
- private void ensureUserDoesNotExist(MailAddress mailAddress) throws
UsersRepositoryException {
+ private void ensureUserDoesNotExist(MailAddress mailAddress) throws
Exception {
Username username = usersRepository.getUsername(mailAddress);
-
- if (usersRepository.contains(username)) {
+ Optional<ValidationFailure> validationFailure =
userEntityValidator.canCreate(username, ImmutableSet.of(ALIAS));
+ if (validationFailure.isPresent()) {
throw ErrorResponder.builder()
- .statusCode(HttpStatus.BAD_REQUEST_400)
- .type(ErrorResponder.ErrorType.INVALID_ARGUMENT)
- .message("The alias source exists as an user already")
+ .statusCode(HttpStatus.CONFLICT_409)
+ .type(ErrorResponder.ErrorType.WRONG_STATE)
+ .message(validationFailure.get().errorMessage())
.haltError();
}
}
diff --git
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java
index 79135e4..ec1eca6 100644
---
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java
+++
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/GroupsRoutes.java
@@ -20,6 +20,7 @@
package org.apache.james.webadmin.routes;
import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static
org.apache.james.webadmin.service.UserEntityValidator.EntityType.GROUP;
import static spark.Spark.halt;
import java.util.List;
@@ -44,9 +45,10 @@ import org.apache.james.rrt.lib.Mapping;
import org.apache.james.rrt.lib.MappingSource;
import org.apache.james.rrt.lib.Mappings;
import org.apache.james.user.api.UsersRepository;
-import org.apache.james.user.api.UsersRepositoryException;
import org.apache.james.webadmin.Constants;
import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.service.UserEntityValidator;
+import org.apache.james.webadmin.service.UserEntityValidator.ValidationFailure;
import org.apache.james.webadmin.utils.ErrorResponder;
import org.apache.james.webadmin.utils.ErrorResponder.ErrorType;
import org.apache.james.webadmin.utils.JsonTransformer;
@@ -54,6 +56,7 @@ import org.eclipse.jetty.http.HttpStatus;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import io.swagger.annotations.Api;
@@ -84,14 +87,16 @@ public class GroupsRoutes implements Routes {
private static final String USER_ADDRESS_TYPE = "group member";
private final UsersRepository usersRepository;
+ private final UserEntityValidator userEntityValidator;
private final JsonTransformer jsonTransformer;
private final RecipientRewriteTable recipientRewriteTable;
@Inject
@VisibleForTesting
GroupsRoutes(RecipientRewriteTable recipientRewriteTable, UsersRepository
usersRepository,
- JsonTransformer jsonTransformer) {
+ UserEntityValidator userEntityValidator, JsonTransformer
jsonTransformer) {
this.usersRepository = usersRepository;
+ this.userEntityValidator = userEntityValidator;
this.jsonTransformer = jsonTransformer;
this.recipientRewriteTable = recipientRewriteTable;
}
@@ -143,7 +148,7 @@ public class GroupsRoutes implements Routes {
@ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500,
message = "Internal server error - Something went bad on the
server side.")
})
- public HaltException addToGroup(Request request, Response response) throws
UsersRepositoryException {
+ public HaltException addToGroup(Request request, Response response) throws
Exception {
MailAddress groupAddress =
MailAddressParser.parseMailAddress(request.params(GROUP_ADDRESS),
GROUP_ADDRESS_TYPE);
Domain domain = groupAddress.getDomain();
ensureNotShadowingAnotherAddress(groupAddress);
@@ -179,12 +184,14 @@ public class GroupsRoutes implements Routes {
}
}
- private void ensureNotShadowingAnotherAddress(MailAddress groupAddress)
throws UsersRepositoryException {
- if
(usersRepository.contains(usersRepository.getUsername(groupAddress))) {
+ private void ensureNotShadowingAnotherAddress(MailAddress groupAddress)
throws Exception {
+ Username username = usersRepository.getUsername(groupAddress);
+ Optional<ValidationFailure> validationFailure =
userEntityValidator.canCreate(username, ImmutableSet.of(GROUP));
+ if (validationFailure.isPresent()) {
throw ErrorResponder.builder()
.statusCode(HttpStatus.CONFLICT_409)
- .type(ErrorType.INVALID_ARGUMENT)
- .message("Requested group address is already used for another
purpose")
+ .type(ErrorType.WRONG_STATE)
+ .message(validationFailure.get().errorMessage())
.haltError();
}
}
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 7b0b120..db02680 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
@@ -47,7 +47,6 @@ import org.apache.james.webadmin.dto.UserResponse;
import org.apache.james.webadmin.service.UserService;
import org.apache.james.webadmin.utils.ErrorResponder;
import org.apache.james.webadmin.utils.ErrorResponder.ErrorType;
-import org.apache.james.webadmin.utils.JsonExtractException;
import org.apache.james.webadmin.utils.JsonExtractor;
import org.apache.james.webadmin.utils.JsonTransformer;
import org.apache.james.webadmin.utils.Responses;
@@ -228,7 +227,7 @@ public class UserRoutes implements Routes {
return Constants.EMPTY_BODY;
}
- private HaltException upsertUser(Request request, Response response)
throws JsonExtractException {
+ private HaltException upsertUser(Request request, Response response)
throws Exception {
Username username = extractUsername(request);
try {
boolean isForced = request.queryParams().contains(FORCE_PARAM);
@@ -250,8 +249,8 @@ public class UserRoutes implements Routes {
LOGGER.info(e.getMessage());
throw ErrorResponder.builder()
.statusCode(HttpStatus.CONFLICT_409)
- .type(ErrorType.INVALID_ARGUMENT)
- .message("User already exists")
+ .type(ErrorType.WRONG_STATE)
+ .message(e.getMessage())
.cause(e)
.haltError();
} catch (UsersRepositoryException e) {
diff --git
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/AggregateUserEntityValidator.java
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/AggregateUserEntityValidator.java
new file mode 100644
index 0000000..8bc54d2
--- /dev/null
+++
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/AggregateUserEntityValidator.java
@@ -0,0 +1,43 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.webadmin.service;
+
+import java.util.Optional;
+import java.util.Set;
+
+import org.apache.james.core.Username;
+
+import com.github.fge.lambdas.Throwing;
+
+public class AggregateUserEntityValidator implements UserEntityValidator {
+ private final Set<UserEntityValidator> validatorSet;
+
+ public AggregateUserEntityValidator(Set<UserEntityValidator> validatorSet)
{
+ this.validatorSet = validatorSet;
+ }
+
+ @Override
+ public Optional<ValidationFailure> canCreate(Username username,
Set<EntityType> ignoredTypes) throws Exception {
+ return validatorSet.stream()
+ .map(Throwing.<UserEntityValidator,
Optional<ValidationFailure>>function(validator -> validator.canCreate(username,
ignoredTypes)).sneakyThrow())
+ .flatMap(Optional::stream)
+ .findFirst();
+ }
+}
diff --git
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/DefaultUserEntityValidator.java
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/DefaultUserEntityValidator.java
new file mode 100644
index 0000000..b08a0e6
--- /dev/null
+++
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/DefaultUserEntityValidator.java
@@ -0,0 +1,49 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.webadmin.service;
+
+import java.util.Optional;
+import java.util.Set;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+
+public class DefaultUserEntityValidator implements UserEntityValidator {
+ private final UsersRepository usersRepository;
+
+ @Inject
+ public DefaultUserEntityValidator(UsersRepository usersRepository) {
+ this.usersRepository = usersRepository;
+ }
+
+ @Override
+ public Optional<ValidationFailure> canCreate(Username username,
Set<EntityType> ignoredTypes) throws UsersRepositoryException {
+ if (ignoredTypes.contains(EntityType.USER)) {
+ return Optional.empty();
+ }
+ if (usersRepository.contains(username)) {
+ return Optional.of(new ValidationFailure("'" + username.asString()
+ "' user already exist"));
+ }
+ return Optional.empty();
+ }
+}
diff --git
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/RecipientRewriteTableUserEntityValidator.java
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/RecipientRewriteTableUserEntityValidator.java
new file mode 100644
index 0000000..2274784
--- /dev/null
+++
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/RecipientRewriteTableUserEntityValidator.java
@@ -0,0 +1,68 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.webadmin.service;
+
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Stream;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.rrt.api.RecipientRewriteTable;
+import org.apache.james.rrt.api.RecipientRewriteTableException;
+import org.apache.james.rrt.lib.Mapping;
+import org.apache.james.rrt.lib.MappingSource;
+import org.apache.james.rrt.lib.Mappings;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+
+public class RecipientRewriteTableUserEntityValidator implements
UserEntityValidator {
+ private static final Map<EntityType, Mapping.Type> TYPE_CORRESPONDENCE =
ImmutableMap.of(
+ EntityType.ALIAS, Mapping.Type.Alias,
+ EntityType.GROUP, Mapping.Type.Group);
+
+ private final RecipientRewriteTable rrt;
+
+ @Inject
+ public RecipientRewriteTableUserEntityValidator(RecipientRewriteTable rrt)
{
+ this.rrt = rrt;
+ }
+
+ @Override
+ public Optional<ValidationFailure> canCreate(Username username,
Set<EntityType> ignoredTypes) throws RecipientRewriteTableException {
+ Mappings mappings =
rrt.getStoredMappings(MappingSource.fromUser(username));
+
+ return filterIgnored(mappings, ignoredTypes)
+ .findFirst()
+ .map(mapping -> new ValidationFailure("'" + username.asString() +
"' already have associated mappings: " + mapping.asString()));
+ }
+
+ private Stream<Mapping> filterIgnored(Mappings mappings, Set<EntityType>
ignoredTypes) {
+ final ImmutableSet<Mapping.Type> types = ignoredTypes.stream()
+ .flatMap(type ->
Optional.ofNullable(TYPE_CORRESPONDENCE.get(type)).stream())
+ .collect(ImmutableSet.toImmutableSet());
+
+ return mappings.asStream()
+ .filter(mapping -> !types.contains(mapping.getType()));
+ }
+}
diff --git
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserEntityValidator.java
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserEntityValidator.java
new file mode 100644
index 0000000..5ebadf8
--- /dev/null
+++
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserEntityValidator.java
@@ -0,0 +1,84 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.webadmin.service;
+
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+
+import org.apache.james.core.Username;
+
+import com.google.common.collect.ImmutableSet;
+
+public interface UserEntityValidator {
+ class ValidationFailure {
+ private final String errorMessage;
+
+ public ValidationFailure(String errorMessage) {
+ this.errorMessage = errorMessage;
+ }
+
+ public String errorMessage() {
+ return errorMessage;
+ }
+ }
+
+ class EntityType {
+ public static final EntityType GROUP = new EntityType("group");
+ public static final EntityType ALIAS = new EntityType("alias");
+ public static final EntityType USER = new EntityType("user");
+
+ private final String type;
+
+ public EntityType(String type) {
+ this.type = type;
+ }
+
+ public String getType() {
+ return type;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(type);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof EntityType) {
+ EntityType other = (EntityType) obj;
+ return Objects.equals(this.type, other.type);
+ }
+ return false;
+ }
+ }
+
+ UserEntityValidator NOOP = (username, ignoredTypes) -> Optional.empty();
+
+ static UserEntityValidator aggregate(UserEntityValidator... validators) {
+ return new
AggregateUserEntityValidator(ImmutableSet.copyOf(validators));
+ }
+
+ Optional<ValidationFailure> canCreate(Username username, Set<EntityType>
ignoredTypes) throws Exception;
+
+ default Optional<ValidationFailure> canCreate(Username username) throws
Exception {
+ return canCreate(username, ImmutableSet.of());
+ }
+}
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 2ea68b2..38cbabb 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
@@ -32,20 +32,18 @@ import org.apache.james.user.api.UsersRepositoryException;
import org.apache.james.user.api.model.User;
import org.apache.james.util.streams.Iterators;
import org.apache.james.webadmin.dto.UserResponse;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.apache.james.webadmin.service.UserEntityValidator.ValidationFailure;
import com.google.common.collect.ImmutableList;
public class UserService {
-
- private static final Logger LOGGER =
LoggerFactory.getLogger(UserService.class);
-
private final UsersRepository usersRepository;
+ private final UserEntityValidator userEntityValidator;
@Inject
- public UserService(UsersRepository usersRepository) {
+ public UserService(UsersRepository usersRepository, UserEntityValidator
userEntityValidator) {
this.usersRepository = usersRepository;
+ this.userEntityValidator = userEntityValidator;
}
public List<UserResponse> getUsers() throws UsersRepositoryException {
@@ -61,9 +59,13 @@ public class UserService {
usersRepository.removeUser(username);
}
- public void upsertUser(Username username, char[] password) throws
UsersRepositoryException {
+ public void upsertUser(Username username, char[] password) throws
Exception {
User user = usersRepository.getUserByName(username);
if (user == null) {
+ Optional<ValidationFailure> validationFailure =
userEntityValidator.canCreate(username);
+ if (validationFailure.isPresent()) {
+ throw new
AlreadyExistInUsersRepositoryException(validationFailure.get().errorMessage());
+ }
usersRepository.addUser(username, new String(password));
} else {
user.setPassword(new String(password));
@@ -75,13 +77,12 @@ public class UserService {
return usersRepository.contains(username);
}
- public void insertUser(Username username, char[] password) throws
UsersRepositoryException, AlreadyExistInUsersRepositoryException {
- User user = usersRepository.getUserByName(username);
- if (user == null) {
- usersRepository.addUser(username, new String(password));
- } else {
- throw new AlreadyExistInUsersRepositoryException("User " +
username + " already exists.");
+ public void insertUser(Username username, char[] password) throws
Exception {
+ Optional<ValidationFailure> validationFailure =
userEntityValidator.canCreate(username);
+ if (validationFailure.isPresent()) {
+ throw new
AlreadyExistInUsersRepositoryException(validationFailure.get().errorMessage());
}
+ usersRepository.addUser(username, new String(password));
}
}
diff --git
a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/AliasRoutesTest.java
b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/AliasRoutesTest.java
index 198d51c..7d8d09b 100644
---
a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/AliasRoutesTest.java
+++
b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/AliasRoutesTest.java
@@ -50,6 +50,9 @@ import org.apache.james.user.memory.MemoryUsersRepository;
import org.apache.james.webadmin.WebAdminServer;
import org.apache.james.webadmin.WebAdminUtils;
import org.apache.james.webadmin.dto.MappingSourceModule;
+import org.apache.james.webadmin.service.DefaultUserEntityValidator;
+import
org.apache.james.webadmin.service.RecipientRewriteTableUserEntityValidator;
+import org.apache.james.webadmin.service.UserEntityValidator;
import org.apache.james.webadmin.utils.JsonTransformer;
import org.eclipse.jetty.http.HttpStatus;
import org.junit.jupiter.api.AfterEach;
@@ -59,7 +62,6 @@ import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import io.restassured.RestAssured;
-import io.restassured.filter.log.LogDetail;
import io.restassured.http.ContentType;
class AliasRoutesTest {
@@ -122,7 +124,12 @@ class AliasRoutesTest {
usersRepository.addUser(Username.of(BOB_WITH_SLASH),
BOB_WITH_SLASH_PASSWORD);
usersRepository.addUser(Username.of(ALICE), ALICE_PASSWORD);
- createServer(new AliasRoutes(memoryRecipientRewriteTable,
usersRepository, domainList, new JsonTransformer(module)),
+
+ UserEntityValidator validator = UserEntityValidator.aggregate(
+ new DefaultUserEntityValidator(usersRepository),
+ new
RecipientRewriteTableUserEntityValidator(memoryRecipientRewriteTable));
+
+ createServer(new AliasRoutes(memoryRecipientRewriteTable,
usersRepository, validator, domainList, new JsonTransformer(module)),
new AddressMappingRoutes(memoryRecipientRewriteTable));
}
@@ -179,6 +186,26 @@ class AliasRoutesTest {
}
@Test
+ void putShouldDetectConflictsWithGroups() throws Exception {
+
memoryRecipientRewriteTable.addGroupMapping(MappingSource.fromUser(Username.of(ALICE_ALIAS)),
BOB);
+
+ Map<String, Object> errors = when()
+ .put(ALICE + SEPARATOR + "sources" + SEPARATOR +
ALICE_ALIAS)
+ .then()
+ .contentType(ContentType.JSON)
+ .statusCode(HttpStatus.CONFLICT_409)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.CONFLICT_409)
+ .containsEntry("type", "WrongState")
+ .containsEntry("message", "'[email protected]' already have
associated mappings: group:[email protected]");
+ }
+
+ @Test
void getNotRegisteredAliasesShouldReturnEmptyList() {
when()
.get("[email protected]")
@@ -238,7 +265,7 @@ class AliasRoutesTest {
Map<String, Object> errors = when()
.put(BOB + SEPARATOR + "sources" + SEPARATOR + ALICE)
.then()
- .statusCode(HttpStatus.BAD_REQUEST_400)
+ .statusCode(HttpStatus.CONFLICT_409)
.contentType(ContentType.JSON)
.extract()
.body()
@@ -246,9 +273,9 @@ class AliasRoutesTest {
.getMap(".");
assertThat(errors)
- .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400)
- .containsEntry("type", "InvalidArgument")
- .containsEntry("message", "The alias source exists as an user
already");
+ .containsEntry("statusCode", HttpStatus.CONFLICT_409)
+ .containsEntry("type", "WrongState")
+ .containsEntry("message", "'[email protected]' user already exist");
}
@Test
@@ -446,7 +473,7 @@ class AliasRoutesTest {
domainList = mock(DomainList.class);
memoryRecipientRewriteTable.setDomainList(domainList);
Mockito.when(domainList.containsDomain(any())).thenReturn(true);
- createServer(new AliasRoutes(memoryRecipientRewriteTable,
userRepository, domainList, new JsonTransformer()),
+ createServer(new AliasRoutes(memoryRecipientRewriteTable,
userRepository, UserEntityValidator.NOOP, domainList, new JsonTransformer()),
new AddressMappingRoutes(memoryRecipientRewriteTable));
}
diff --git
a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java
b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java
index 5726d24..d461c93 100644
---
a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java
+++
b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/GroupsRoutesTest.java
@@ -52,6 +52,9 @@ import org.apache.james.user.memory.MemoryUsersRepository;
import org.apache.james.webadmin.WebAdminServer;
import org.apache.james.webadmin.WebAdminUtils;
import org.apache.james.webadmin.dto.MappingSourceModule;
+import org.apache.james.webadmin.service.DefaultUserEntityValidator;
+import
org.apache.james.webadmin.service.RecipientRewriteTableUserEntityValidator;
+import org.apache.james.webadmin.service.UserEntityValidator;
import org.apache.james.webadmin.utils.JsonTransformer;
import org.eclipse.jetty.http.HttpStatus;
import org.junit.jupiter.api.AfterEach;
@@ -61,7 +64,6 @@ import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import io.restassured.RestAssured;
-import io.restassured.filter.log.LogDetail;
import io.restassured.http.ContentType;
class GroupsRoutesTest {
@@ -114,7 +116,10 @@ class GroupsRoutesTest {
memoryRecipientRewriteTable.setConfiguration(RecipientRewriteTableConfiguration.DEFAULT_ENABLED);
usersRepository =
MemoryUsersRepository.withVirtualHosting(domainList);
MappingSourceModule mappingSourceModule = new
MappingSourceModule();
- createServer(new GroupsRoutes(memoryRecipientRewriteTable,
usersRepository, new JsonTransformer(mappingSourceModule)),
+ UserEntityValidator validator = UserEntityValidator.aggregate(
+ new DefaultUserEntityValidator(usersRepository),
+ new
RecipientRewriteTableUserEntityValidator(memoryRecipientRewriteTable));
+ createServer(new GroupsRoutes(memoryRecipientRewriteTable,
usersRepository, validator, new JsonTransformer(mappingSourceModule)),
new AddressMappingRoutes(memoryRecipientRewriteTable));
}
@@ -320,6 +325,26 @@ class GroupsRoutesTest {
}
@Test
+ void putShouldNotConflictWithAlias() throws Exception {
+
memoryRecipientRewriteTable.addAliasMapping(MappingSource.fromUser(Username.of(GROUP1)),
USER_A);
+
+ Map<String, Object> errors = when()
+ .put(GROUP1 + SEPARATOR + USER_A)
+ .then()
+ .contentType(ContentType.JSON)
+ .statusCode(HttpStatus.CONFLICT_409)
+ .extract()
+ .body()
+ .jsonPath()
+ .getMap(".");
+
+ assertThat(errors)
+ .containsEntry("statusCode", HttpStatus.CONFLICT_409)
+ .containsEntry("type", "WrongState")
+ .containsEntry("message", "'[email protected]' already have
associated mappings: alias:[email protected]");
+ }
+
+ @Test
void putUserInGroupWithEncodedSlashShouldCreateGroup() {
when()
.put(GROUP_WITH_ENCODED_SLASH + SEPARATOR + USER_A);
@@ -395,8 +420,8 @@ class GroupsRoutesTest {
assertThat(errors)
.containsEntry("statusCode", HttpStatus.CONFLICT_409)
- .containsEntry("type", "InvalidArgument")
- .containsEntry("message", "Requested group address is already
used for another purpose");
+ .containsEntry("type", "WrongState")
+ .containsEntry("message", "'[email protected]' user already exist");
}
@Test
@@ -473,7 +498,7 @@ class GroupsRoutesTest {
domainList = mock(DomainList.class);
memoryRecipientRewriteTable.setDomainList(domainList);
Mockito.when(domainList.containsDomain(any())).thenReturn(true);
- createServer(new GroupsRoutes(memoryRecipientRewriteTable,
userRepository, new JsonTransformer()),
+ createServer(new GroupsRoutes(memoryRecipientRewriteTable,
userRepository, UserEntityValidator.NOOP, new JsonTransformer()),
new AddressMappingRoutes(memoryRecipientRewriteTable));
}
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 a5fe202..065d93b 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
@@ -55,6 +55,9 @@ import org.apache.james.user.api.model.User;
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.DefaultUserEntityValidator;
+import
org.apache.james.webadmin.service.RecipientRewriteTableUserEntityValidator;
+import org.apache.james.webadmin.service.UserEntityValidator;
import org.apache.james.webadmin.service.UserService;
import org.apache.james.webadmin.utils.ErrorResponder;
import org.apache.james.webadmin.utils.JsonTransformer;
@@ -118,7 +121,7 @@ class UserRoutesTest {
@Override
public void beforeEach(ExtensionContext extensionContext) {
- webAdminServer = startServer(usersRepository);
+ webAdminServer = startServer(usersRepository,
recipientRewriteTable);
}
@Override
@@ -145,8 +148,11 @@ class UserRoutesTest {
throw new RuntimeException("Unknown parameter type: " +
parameterType);
}
- private WebAdminServer startServer(UsersRepository usersRepository) {
- WebAdminServer server = WebAdminUtils.createWebAdminServer(new
UserRoutes(new UserService(usersRepository), canSendFrom, new
JsonTransformer()))
+ private WebAdminServer startServer(UsersRepository usersRepository,
RecipientRewriteTable recipientRewriteTable) {
+ UserEntityValidator validator = UserEntityValidator.aggregate(
+ new DefaultUserEntityValidator(usersRepository),
+ new
RecipientRewriteTableUserEntityValidator(recipientRewriteTable));
+ WebAdminServer server = WebAdminUtils.createWebAdminServer(new
UserRoutes(new UserService(usersRepository, validator), canSendFrom, new
JsonTransformer()))
.start();
RestAssured.requestSpecification =
WebAdminUtils.buildRequestSpecification(server)
@@ -409,7 +415,7 @@ class UserRoutesTest {
@Test
default void
putShouldFailOnRepositoryExceptionOnGetUserByName(UsersRepository
usersRepository) throws Exception {
-
when(usersRepository.getUserByName(USERNAME_WITH_DOMAIN)).thenThrow(new
UsersRepositoryException("message"));
+
when(usersRepository.contains(USERNAME_WITH_DOMAIN)).thenThrow(new
UsersRepositoryException("message"));
given()
.body("{\"password\":\"password\"}")
@@ -469,7 +475,7 @@ class UserRoutesTest {
@Test
default void
putShouldFailOnUnknownExceptionOnGetUserByName(UsersRepository usersRepository)
throws Exception {
-
when(usersRepository.getUserByName(USERNAME_WITH_DOMAIN)).thenThrow(new
RuntimeException());
+
when(usersRepository.contains(USERNAME_WITH_DOMAIN)).thenThrow(new
RuntimeException());
given()
.body("{\"password\":\"password\"}")
@@ -512,6 +518,8 @@ class UserRoutesTest {
private static final Username USERNAME_WITHOUT_DOMAIN =
Username.of("username");
private static final Username USERNAME_WITH_DOMAIN =
Username.fromLocalPartWithDomain(USERNAME_WITHOUT_DOMAIN.asString(),
DOMAIN);
+ private static final Username OTHER_USERNAME_WITH_DOMAIN =
+ Username.fromLocalPartWithDomain("other", DOMAIN);
private static final String PASSWORD = "password";
@Nested
@@ -574,6 +582,38 @@ class UserRoutesTest {
}
@Test
+ void putShouldFailWhenConflictWithAlias(RecipientRewriteTable
recipientRewriteTable) throws Exception {
+
recipientRewriteTable.addAliasMapping(MappingSource.fromUser(USERNAME_WITH_DOMAIN),
+ OTHER_USERNAME_WITH_DOMAIN.asString());
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.CONFLICT_409)
+ .body("statusCode", is(HttpStatus.CONFLICT_409))
+ .body("type",
is(ErrorResponder.ErrorType.WRONG_STATE.getType()))
+ .body("message", is("'username@domain' already have associated
mappings: alias:other@domain"));
+ }
+
+ @Test
+ void putShouldFailWhenConflictWithGroup(RecipientRewriteTable
recipientRewriteTable) throws Exception {
+
recipientRewriteTable.addGroupMapping(MappingSource.fromUser(USERNAME_WITH_DOMAIN),
+ OTHER_USERNAME_WITH_DOMAIN.asString());
+
+ given()
+ .body("{\"password\":\"password\"}")
+ .when()
+ .put(USERNAME_WITH_DOMAIN.asString())
+ .then()
+ .statusCode(HttpStatus.CONFLICT_409)
+ .body("statusCode", is(HttpStatus.CONFLICT_409))
+ .body("type",
is(ErrorResponder.ErrorType.WRONG_STATE.getType()))
+ .body("message", is("'username@domain' already have associated
mappings: group:other@domain"));
+ }
+
+ @Test
void
putWithDomainPartInUsernameShouldReturnOkWhenWithA255LongUsername() {
String usernameTail = "@" + DOMAIN.name();
given()
@@ -631,8 +671,8 @@ class UserRoutesTest {
.then()
.statusCode(HttpStatus.CONFLICT_409)
.body("statusCode", is(HttpStatus.CONFLICT_409))
- .body("type",
is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
- .body("message", is("User already exists"));
+ .body("type",
is(ErrorResponder.ErrorType.WRONG_STATE.getType()))
+ .body("message", is("'username@domain' user already exist"));
}
@Test
@@ -838,8 +878,8 @@ class UserRoutesTest {
.then()
.statusCode(HttpStatus.CONFLICT_409)
.body("statusCode", is(HttpStatus.CONFLICT_409))
- .body("type",
is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
- .body("message", is("User already exists"));
+ .body("type",
is(ErrorResponder.ErrorType.WRONG_STATE.getType()))
+ .body("message", is("'username' user already exist"));
}
@Test
diff --git a/src/site/markdown/server/manage-webadmin.md
b/src/site/markdown/server/manage-webadmin.md
index b190270..633f4ef 100644
--- a/src/site/markdown/server/manage-webadmin.md
+++ b/src/site/markdown/server/manage-webadmin.md
@@ -2365,9 +2365,9 @@ Response codes:
- 204: OK
- 400: Alias structure or member is not valid
- - 400: The alias source exists as an user already
- 400: Source and destination can't be the same!
- 400: Domain in the destination or source is not managed by the DomainList
+ - 409: The alias source exists as an user already
- 409: The creation of the alias would lead to a loop and thus cannot be
performed
### Removing an alias of an user
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
