This is an automated email from the ASF dual-hosted git repository. rouazana pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 7082fb304dd399dc6d7794a2fc93feec8bc59313 Author: Tran Tien Duc <[email protected]> AuthorDate: Tue Apr 23 11:29:26 2019 +0700 JAMES-2729 GroupRoutes validating source domain existence --- .../apache/james/webadmin/routes/GroupsRoutes.java | 16 ++---- .../james/webadmin/routes/GroupsRoutesTest.java | 67 +++++++++++----------- src/site/markdown/server/manage-webadmin.md | 2 +- 3 files changed, 39 insertions(+), 46 deletions(-) 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 fdb471b..0247dd3 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 @@ -39,6 +39,7 @@ import org.apache.james.domainlist.api.DomainListException; import org.apache.james.rrt.api.MappingAlreadyExistsException; import org.apache.james.rrt.api.RecipientRewriteTable; import org.apache.james.rrt.api.RecipientRewriteTableException; +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.rrt.lib.Mappings; @@ -139,7 +140,7 @@ public class GroupsRoutes implements Routes { @ApiResponses(value = { @ApiResponse(code = HttpStatus.NO_CONTENT_204, message = "OK", response = List.class), @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = GROUP_ADDRESS + " or group structure format is not valid"), - @ApiResponse(code = HttpStatus.FORBIDDEN_403, message = "server doesn't own the domain"), + @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "Domain in the source is not managed by the DomainList"), @ApiResponse(code = HttpStatus.CONFLICT_409, message = "requested group address is already used for another purpose"), @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500, message = "Internal server error - Something went bad on the server side.") @@ -147,7 +148,6 @@ public class GroupsRoutes implements Routes { public HaltException addToGroup(Request request, Response response) throws RecipientRewriteTableException, UsersRepositoryException, DomainListException { MailAddress groupAddress = MailAddressParser.parseMailAddress(request.params(GROUP_ADDRESS), ADDRESS_TYPE); Domain domain = groupAddress.getDomain(); - ensureRegisteredDomain(domain); ensureNotShadowingAnotherAddress(groupAddress); MailAddress userAddress = MailAddressParser.parseMailAddress(request.params(USER_ADDRESS), ADDRESS_TYPE); MappingSource source = MappingSource.fromUser(User.fromLocalPartWithDomain(groupAddress.getLocalPart(), domain)); @@ -160,15 +160,11 @@ public class GroupsRoutes implements Routes { recipientRewriteTable.addGroupMapping(source, userAddress.asString()); } catch (MappingAlreadyExistsException e) { // do nothing - } - } - - private void ensureRegisteredDomain(Domain domain) throws DomainListException { - if (!domainList.containsDomain(domain)) { + } catch (SourceDomainIsNotInDomainListException e) { throw ErrorResponder.builder() - .statusCode(HttpStatus.FORBIDDEN_403) - .type(ErrorType.INVALID_ARGUMENT) - .message("Server doesn't own the domain: " + domain.name()) + .statusCode(HttpStatus.BAD_REQUEST_400) + .type(ErrorResponder.ErrorType.INVALID_ARGUMENT) + .message(e.getMessage()) .haltError(); } } 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 713a8ff..ffc42f6 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 @@ -28,8 +28,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.CoreMatchers.is; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import java.util.List; import java.util.Map; @@ -40,7 +42,6 @@ import org.apache.james.dnsservice.api.DNSService; import org.apache.james.domainlist.api.DomainList; import org.apache.james.domainlist.memory.MemoryDomainList; import org.apache.james.metrics.logger.DefaultMetricFactory; -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; @@ -66,6 +67,7 @@ import io.restassured.http.ContentType; class GroupsRoutesTest { private static final Domain DOMAIN = Domain.of("b.com"); + private static final Domain ALIAS_DOMAIN = Domain.of("alias"); private static final String GROUP1 = "group1" + "@" + DOMAIN.name(); private static final String GROUP2 = "group2" + "@" + DOMAIN.name(); private static final String GROUP_WITH_SLASH = "group10/10" + "@" + DOMAIN.name(); @@ -108,6 +110,8 @@ class GroupsRoutesTest { DNSService dnsService = mock(DNSService.class); domainList = new MemoryDomainList(dnsService); domainList.addDomain(DOMAIN); + domainList.addDomain(ALIAS_DOMAIN); + memoryRecipientRewriteTable.setDomainList(domainList); usersRepository = MemoryUsersRepository.withVirtualHosting(); usersRepository.setDomainList(domainList); MappingSourceModule mappingSourceModule = new MappingSourceModule(); @@ -355,24 +359,6 @@ class GroupsRoutesTest { } @Test - void putUserInGroupShouldNotAllowGroupOnUnregisteredDomain() { - Map<String, Object> errors = when() - .put("group@unregisteredDomain" + SEPARATOR + USER_A) - .then() - .statusCode(HttpStatus.FORBIDDEN_403) - .contentType(ContentType.JSON) - .extract() - .body() - .jsonPath() - .getMap("."); - - assertThat(errors) - .containsEntry("statusCode", HttpStatus.FORBIDDEN_403) - .containsEntry("type", "InvalidArgument") - .containsEntry("message", "Server doesn't own the domain: unregisteredDomain"); - } - - @Test void putUserInGroupShouldNotAllowUserShadowing() throws UsersRepositoryException { usersRepository.addUser(USER_A, "whatever"); @@ -447,7 +433,7 @@ class GroupsRoutesTest { super.setUp(); memoryRecipientRewriteTable.addErrorMapping(MappingSource.fromUser("error", DOMAIN), "disabled"); memoryRecipientRewriteTable.addRegexMapping(MappingSource.fromUser("regex", DOMAIN), ".*@b\\.com"); - memoryRecipientRewriteTable.addAliasDomainMapping(MappingSource.fromDomain(Domain.of("alias")), DOMAIN); + memoryRecipientRewriteTable.addAliasDomainMapping(MappingSource.fromDomain(ALIAS_DOMAIN), DOMAIN); } @@ -456,13 +442,15 @@ class GroupsRoutesTest { @Nested class ExceptionHandling { - private RecipientRewriteTable memoryRecipientRewriteTable; + private MemoryRecipientRewriteTable memoryRecipientRewriteTable; + private DomainList domainList; @BeforeEach void setUp() throws Exception { - memoryRecipientRewriteTable = mock(RecipientRewriteTable.class); + memoryRecipientRewriteTable = spy(new MemoryRecipientRewriteTable()); UsersRepository userRepository = mock(UsersRepository.class); - DomainList domainList = mock(DomainList.class); + domainList = mock(DomainList.class); + memoryRecipientRewriteTable.setDomainList(domainList); Mockito.when(domainList.containsDomain(any())).thenReturn(true); createServer(new GroupsRoutes(memoryRecipientRewriteTable, userRepository, domainList, new JsonTransformer())); } @@ -506,6 +494,27 @@ class GroupsRoutesTest { } @Test + void putASourceContainingANotManagedDomainShouldReturnBadRequest() throws Exception { + doReturn(false) + .when(domainList).containsDomain(any()); + + Map<String, Object> errors = when() + .put("[email protected]" + SEPARATOR + USER_A) + .then() + .statusCode(HttpStatus.BAD_REQUEST_400) + .contentType(ContentType.JSON) + .extract() + .body() + .jsonPath() + .getMap("."); + + assertThat(errors) + .containsEntry("statusCode", HttpStatus.BAD_REQUEST_400) + .containsEntry("type", "InvalidArgument") + .containsEntry("message", "Source domain 'not-managed-domain.tld' is not managed by the domainList"); + } + + @Test void putUserInGroupWithSlashShouldReturnNotFound() { when() .put(GROUP_WITH_SLASH + SEPARATOR + USER_A) @@ -669,18 +678,6 @@ class GroupsRoutesTest { } @Test - void getShouldReturnErrorWhenRecipientRewriteTableExceptionIsThrown() throws Exception { - doThrow(RecipientRewriteTableException.class) - .when(memoryRecipientRewriteTable) - .getStoredMappings(any()); - - when() - .get(GROUP1) - .then() - .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500); - } - - @Test void getShouldReturnErrorWhenRuntimeExceptionIsThrown() throws Exception { doThrow(RuntimeException.class) .when(memoryRecipientRewriteTable) diff --git a/src/site/markdown/server/manage-webadmin.md b/src/site/markdown/server/manage-webadmin.md index bebce84..f6ce1cc 100644 --- a/src/site/markdown/server/manage-webadmin.md +++ b/src/site/markdown/server/manage-webadmin.md @@ -1201,7 +1201,7 @@ Response codes: - 204: Success - 400: Group structure or member is not valid - - 403: Server does not own the requested domain + - 400: Domain in the source is not managed by the DomainList - 409: Requested group address is already used for another purpose ### Removing a group member --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
