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 ad...@apache.org on 2017/11/16 13:20:35 UTC
[8/8] james-project git commit: JAMES-2219 Moving mailboxes and
delegation is not compatible
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 <bt...@linagora.com>
Authored: Thu Nov 16 11:40:23 2017 +0700
Committer: Antoine Duprat <ad...@linagora.com>
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 "bob@domain.tld" creates mailbox "sharedChild" with creationId "c-01" in mailbox "shared" owned by "alice@domain.tld"
Then mailbox with creationId "c-01" is not created
+
+ Scenario: A sharee moving a delegated mailbox as top level should be rejected
+ Given "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ When "bob@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld" as top level mailbox
+ Then mailbox "shared.sharedChild" owned by "alice@domain.tld" is not updated
+
+ Scenario: A sharee moving a delegated mailbox as top level should not move mailbox
+ Given "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ When "bob@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld" as top level mailbox
+ Then "alice@domain.tld" lists mailboxes
+ And the mailboxes should contain "sharedChild" in "Personal" namespace, with parent mailbox "shared" of user "alice@domain.tld"
+
+ Scenario: A sharee moving a delegated mailbox into sharer mailboxes should be rejected
+ Given "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "alice@domain.tld" has a mailbox "otherShared"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ And "alice@domain.tld" shares her mailbox "otherShared" with "bob@domain.tld" with "aeilrwt" rights
+ When "bob@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld", into mailbox "otherShared" owned by "alice@domain.tld"
+ Then mailbox "shared.sharedChild" owned by "alice@domain.tld" is not updated
+
+ Scenario: A sharee moving a delegated mailbox into another delegated mailbox should not move mailbox
+ Given "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "alice@domain.tld" has a mailbox "otherShared"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ And "alice@domain.tld" shares her mailbox "otherShared" with "bob@domain.tld" with "aeilrwt" rights
+ When "bob@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld", into mailbox "otherShared" owned by "alice@domain.tld"
+ Then "alice@domain.tld" lists mailboxes
+ And the mailboxes should contain "sharedChild" in "Personal" namespace, with parent mailbox "shared" of user "alice@domain.tld"
+
+ Scenario: A sharee moving a delegated mailbox to his mailboxes should be rejected
+ Given "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "bob@domain.tld" has a mailbox "other"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ When "bob@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld", into mailbox "other" owned by "bob@domain.tld"
+ Then mailbox "shared.sharedChild" owned by "alice@domain.tld" is not updated
+
+ Scenario: A sharee moving a delegated mailbox into his mailbox should not move mailbox
+ Given "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "bob@domain.tld" has a mailbox "other"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ When "bob@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld", into mailbox "other" owned by "bob@domain.tld"
+ Then "alice@domain.tld" lists mailboxes
+ And the mailboxes should contain "sharedChild" in "Personal" namespace, with parent mailbox "shared" of user "alice@domain.tld"
+
+ Scenario: A sharee should be able to retrieve a mailbox after sharer moved it as a top level mailbox
+ Given "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ When "alice@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld" as top level mailbox
+ Then "bob@domain.tld" 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 "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "alice@domain.tld" has a mailbox "other"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ When "alice@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld", into mailbox "other" owned by "alice@domain.tld"
+ Then "bob@domain.tld" lists mailboxes
+ And the mailboxes should contain "sharedChild" in "Delegated" namespace, with parent mailbox "other" of user "alice@domain.tld"
+
+ Scenario: A sharer can not move its mailboxes to someone else delegated mailboxes
+ Given "alice@domain.tld" has a mailbox "shared.sharedChild"
+ And "bob@domain.tld" has a mailbox "other"
+ And "alice@domain.tld" shares her mailbox "shared.sharedChild" with "bob@domain.tld" with "aeilrwt" rights
+ And "bob@domain.tld" shares her mailbox "other" with "alice@domain.tld" with "aeilrwt" rights
+ When "alice@domain.tld" moves the mailbox "shared.sharedChild" owned by "alice@domain.tld", into mailbox "other" owned by "bob@domain.tld"
+ Then mailbox "shared.sharedChild" owned by "alice@domain.tld" 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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org