JAMES-2219 Moving mailboxes and delegation is not compatible
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/51cc52ae Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/51cc52ae Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/51cc52ae Branch: refs/heads/master Commit: 51cc52ae3f91160a333ecac9afba1e8ff4e16884 Parents: 35663c2 Author: benwa <[email protected]> Authored: Thu Nov 16 11:40:23 2017 +0700 Committer: Antoine Duprat <[email protected]> Committed: Thu Nov 16 14:19:17 2017 +0100 ---------------------------------------------------------------------- .../cucumber/GetMailboxesMethodStepdefs.java | 34 +++++++++- .../cucumber/SetMailboxesMethodStepdefs.java | 39 +++++++++++ .../resources/cucumber/GetMailboxes.feature | 69 ++++++++++++++++++++ .../methods/SetMailboxesUpdateProcessor.java | 39 ++++++++--- 4 files changed, 172 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/51cc52ae/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/GetMailboxesMethodStepdefs.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/GetMailboxesMethodStepdefs.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/GetMailboxesMethodStepdefs.java index 73272cf..677ebfa 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/GetMailboxesMethodStepdefs.java +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/GetMailboxesMethodStepdefs.java @@ -23,10 +23,14 @@ import static com.jayway.jsonpath.Criteria.where; import static com.jayway.jsonpath.Filter.filter; import static org.assertj.core.api.Assertions.assertThat; +import java.util.ArrayList; import java.util.List; import javax.inject.Inject; +import org.apache.james.mailbox.model.MailboxConstants; +import org.apache.james.mailbox.store.mail.model.Mailbox; + import cucumber.api.java.en.Then; import cucumber.api.java.en.When; import cucumber.runtime.java.guice.ScenarioScoped; @@ -39,11 +43,13 @@ public class GetMailboxesMethodStepdefs { private final UserStepdefs userStepdefs; private final HttpClient httpClient; + private final MainStepdefs mainStepdefs; @Inject - private GetMailboxesMethodStepdefs(UserStepdefs userStepdefs, HttpClient httpClient) { + private GetMailboxesMethodStepdefs(UserStepdefs userStepdefs, HttpClient httpClient, MainStepdefs mainStepdefs) { this.userStepdefs = userStepdefs; this.httpClient = httpClient; + this.mainStepdefs = mainStepdefs; } @When("^\"([^\"]*)\" lists mailboxes$") @@ -71,6 +77,32 @@ public class GetMailboxesMethodStepdefs { .containsOnly(namespace); } + @Then("^the mailboxes should contain \"([^\"]*)\" in \"([^\"]*)\" namespace, with parent mailbox \"([^\"]*)\" of user \"([^\"]*)\"$") + public void assertMailboxesNames(String mailboxName, String namespace, String parentName, String parentOwner) throws Exception { + Mailbox parentMailbox = mainStepdefs.mailboxProbe.getMailbox(MailboxConstants.USER_NAMESPACE, parentOwner, parentName); + assertThat(httpClient.jsonPath.<List<String>>read(ARGUMENTS + ".list[*].name")).contains(mailboxName); + assertThat(httpClient.jsonPath.<List<String>>read(ARGUMENTS + ".list[?].namespace.type", + filter(where("name").is(mailboxName)))) + .containsOnly(namespace); + assertThat(httpClient.jsonPath.<List<String>>read(ARGUMENTS + ".list[?].parentId", + filter(where("name").is(mailboxName)))) + .containsOnly(parentMailbox.getMailboxId().serialize()); + } + + @Then("^the mailboxes should contain \"([^\"]*)\" in \"([^\"]*)\" namespace, with no parent mailbox$") + public void assertMailboxesNamesNoParent(String mailboxName, String namespace) throws Exception { + ArrayList<Object> noParent = new ArrayList<>(); + noParent.add(null); // Trick to allow collection with null element matching + + assertThat(httpClient.jsonPath.<List<String>>read(ARGUMENTS + ".list[*].name")).contains(mailboxName); + assertThat(httpClient.jsonPath.<List<String>>read(ARGUMENTS + ".list[?].namespace.type", + filter(where("name").is(mailboxName)))) + .containsOnly(namespace); + assertThat(httpClient.jsonPath.<List<String>>read(ARGUMENTS + ".list[?].parentId", + filter(where("name").is(mailboxName)))) + .isEqualTo(noParent); + } + @Then("^the mailboxes should not contain \"([^\"]*)\"$") public void assertNoMailboxesNames(String mailboxName) throws Exception { assertThat(httpClient.jsonPath.<List<String>>read(ARGUMENTS + ".list[*].name")).doesNotContain(mailboxName); http://git-wip-us.apache.org/repos/asf/james-project/blob/51cc52ae/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/SetMailboxesMethodStepdefs.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/SetMailboxesMethodStepdefs.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/SetMailboxesMethodStepdefs.java index ef22490..96c1ad1 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/SetMailboxesMethodStepdefs.java +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/SetMailboxesMethodStepdefs.java @@ -137,6 +137,45 @@ public class SetMailboxesMethodStepdefs { renamingMailbox(mailbox, newMailboxName); } + @When("^\"([^\"]*)\" moves the mailbox \"([^\"]*)\" owned by \"([^\"]*)\", into mailbox \"([^\"]*)\" owned by \"([^\"]*)\"$") + public void movingMailbox(String user, String mailboxName, String mailboxOwner, String newParentName, String newParentOwner) throws Throwable { + Mailbox mailbox = mainStepdefs.mailboxProbe.getMailbox("#private", mailboxOwner, mailboxName); + Mailbox newParent = mainStepdefs.mailboxProbe.getMailbox("#private", newParentOwner, newParentName); + String requestBody = + "[" + + " [ \"setMailboxes\"," + + " {" + + " \"update\": {" + + " \"" + mailbox.getMailboxId().serialize() + "\" : {" + + " \"parentId\" : \"" + newParent.getMailboxId().serialize() + "\"" + + " }" + + " }" + + " }," + + " \"#0\"" + + " ]" + + "]"; + userStepdefs.execWithUser(user, () -> httpClient.post(requestBody)); + } + + @When("^\"([^\"]*)\" moves the mailbox \"([^\"]*)\" owned by \"([^\"]*)\" as top level mailbox$") + public void movingMailboxAsTopLevel(String user, String mailboxName, String mailboxOwner) throws Throwable { + Mailbox mailbox = mainStepdefs.mailboxProbe.getMailbox("#private", mailboxOwner, mailboxName); + String requestBody = + "[" + + " [ \"setMailboxes\"," + + " {" + + " \"update\": {" + + " \"" + mailbox.getMailboxId().serialize() + "\" : {" + + " \"parentId\" : null" + + " }" + + " }" + + " }," + + " \"#0\"" + + " ]" + + "]"; + userStepdefs.execWithUser(user, () -> httpClient.post(requestBody)); + } + @When("^\"([^\"]*)\" creates mailbox \"([^\"]*)\" with creationId \"([^\"]*)\" in mailbox \"([^\"]*)\" owned by \"([^\"]*)\"$") public void createChildMailbox(String user, String mailboxName, String creationId, String parentMailboxName, String parentOwner) throws Throwable { Mailbox parentMailbox = mainStepdefs.mailboxProbe.getMailbox(MailboxConstants.USER_NAMESPACE, parentOwner, parentMailboxName); http://git-wip-us.apache.org/repos/asf/james-project/blob/51cc52ae/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMailboxes.feature ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMailboxes.feature b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMailboxes.feature index 9d5dbb0..36ff9b4 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMailboxes.feature +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/GetMailboxes.feature @@ -295,3 +295,72 @@ Feature: GetMailboxes method Scenario: A sharee should receive a not created response when trying to create a shared mailbox child When "[email protected]" creates mailbox "sharedChild" with creationId "c-01" in mailbox "shared" owned by "[email protected]" Then mailbox with creationId "c-01" is not created + + Scenario: A sharee moving a delegated mailbox as top level should be rejected + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]" as top level mailbox + Then mailbox "shared.sharedChild" owned by "[email protected]" is not updated + + Scenario: A sharee moving a delegated mailbox as top level should not move mailbox + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]" as top level mailbox + Then "[email protected]" lists mailboxes + And the mailboxes should contain "sharedChild" in "Personal" namespace, with parent mailbox "shared" of user "[email protected]" + + Scenario: A sharee moving a delegated mailbox into sharer mailboxes should be rejected + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" has a mailbox "otherShared" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + And "[email protected]" shares her mailbox "otherShared" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]", into mailbox "otherShared" owned by "[email protected]" + Then mailbox "shared.sharedChild" owned by "[email protected]" is not updated + + Scenario: A sharee moving a delegated mailbox into another delegated mailbox should not move mailbox + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" has a mailbox "otherShared" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + And "[email protected]" shares her mailbox "otherShared" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]", into mailbox "otherShared" owned by "[email protected]" + Then "[email protected]" lists mailboxes + And the mailboxes should contain "sharedChild" in "Personal" namespace, with parent mailbox "shared" of user "[email protected]" + + Scenario: A sharee moving a delegated mailbox to his mailboxes should be rejected + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" has a mailbox "other" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]", into mailbox "other" owned by "[email protected]" + Then mailbox "shared.sharedChild" owned by "[email protected]" is not updated + + Scenario: A sharee moving a delegated mailbox into his mailbox should not move mailbox + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" has a mailbox "other" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]", into mailbox "other" owned by "[email protected]" + Then "[email protected]" lists mailboxes + And the mailboxes should contain "sharedChild" in "Personal" namespace, with parent mailbox "shared" of user "[email protected]" + + Scenario: A sharee should be able to retrieve a mailbox after sharer moved it as a top level mailbox + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]" as top level mailbox + Then "[email protected]" lists mailboxes + And the mailboxes should contain "sharedChild" in "Delegated" namespace, with no parent mailbox + + Scenario: A sharee should be able to retrieve a mailbox after sharer moved it into another mailbox + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" has a mailbox "other" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]", into mailbox "other" owned by "[email protected]" + Then "[email protected]" lists mailboxes + And the mailboxes should contain "sharedChild" in "Delegated" namespace, with parent mailbox "other" of user "[email protected]" + + Scenario: A sharer can not move its mailboxes to someone else delegated mailboxes + Given "[email protected]" has a mailbox "shared.sharedChild" + And "[email protected]" has a mailbox "other" + And "[email protected]" shares her mailbox "shared.sharedChild" with "[email protected]" with "aeilrwt" rights + And "[email protected]" shares her mailbox "other" with "[email protected]" with "aeilrwt" rights + When "[email protected]" moves the mailbox "shared.sharedChild" owned by "[email protected]", into mailbox "other" owned by "[email protected]" + Then mailbox "shared.sharedChild" owned by "[email protected]" is not updated + http://git-wip-us.apache.org/repos/asf/james-project/blob/51cc52ae/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java index 645182e..b9b554d 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesUpdateProcessor.java @@ -26,6 +26,7 @@ import java.util.Optional; import javax.inject.Inject; import org.apache.james.jmap.exceptions.MailboxHasChildException; +import org.apache.james.jmap.exceptions.MailboxNotOwnedException; import org.apache.james.jmap.exceptions.MailboxParentNotFoundException; import org.apache.james.jmap.exceptions.SystemMailboxNotUpdatableException; import org.apache.james.jmap.model.MailboxFactory; @@ -40,6 +41,7 @@ import org.apache.james.jmap.model.mailbox.Role; import org.apache.james.jmap.utils.MailboxUtils; import org.apache.james.mailbox.MailboxManager; import org.apache.james.mailbox.MailboxSession; +import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.SubscriptionManager; import org.apache.james.mailbox.exception.DifferentDomainException; import org.apache.james.mailbox.exception.MailboxException; @@ -130,6 +132,11 @@ public class SetMailboxesUpdateProcessor implements SetMailboxesProcessor { .type("invalidArguments") .description("Cannot update a parent mailbox.") .build()); + } catch (MailboxNotOwnedException e) { + responseBuilder.notUpdated(mailboxId, SetError.builder() + .type("invalidArguments") + .description("Parent mailbox is not owned.") + .build()); } catch (MailboxExistsException e) { responseBuilder.notUpdated(mailboxId, SetError.builder() .type("invalidArguments") @@ -188,20 +195,36 @@ public class SetMailboxesUpdateProcessor implements SetMailboxesProcessor { } private void validateParent(Mailbox mailbox, MailboxUpdateRequest updateRequest, MailboxSession mailboxSession) throws MailboxException, MailboxHasChildException { - if (isParentIdInRequest(updateRequest)) { MailboxId newParentId = updateRequest.getParentId().get(); - try { - mailboxManager.getMailbox(newParentId, mailboxSession); - } catch (MailboxNotFoundException e) { - throw new MailboxParentNotFoundException(newParentId); - } - if (mustChangeParent(mailbox.getParentId(), newParentId) && mailboxUtils.hasChildren(mailbox.getId(), mailboxSession)) { - throw new MailboxHasChildException(); + MessageManager parent = retrieveParent(mailboxSession, newParentId); + if (mustChangeParent(mailbox.getParentId(), newParentId)) { + assertNoChildren(mailbox, mailboxSession); + assertOwned(mailboxSession, parent); } } } + private void assertNoChildren(Mailbox mailbox, MailboxSession mailboxSession) throws MailboxException, MailboxHasChildException { + if (mailboxUtils.hasChildren(mailbox.getId(), mailboxSession)) { + throw new MailboxHasChildException(); + } + } + + private void assertOwned(MailboxSession mailboxSession, MessageManager parent) throws MailboxException { + if (!parent.getMailboxPath().belongsTo(mailboxSession)) { + throw new MailboxNotOwnedException(); + } + } + + private MessageManager retrieveParent(MailboxSession mailboxSession, MailboxId newParentId) throws MailboxException { + try { + return mailboxManager.getMailbox(newParentId, mailboxSession); + } catch (MailboxNotFoundException e) { + throw new MailboxParentNotFoundException(newParentId); + } + } + private boolean isParentIdInRequest(MailboxUpdateRequest updateRequest) { return updateRequest.getParentId() != null && updateRequest.getParentId().isPresent(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
