Repository: james-project Updated Branches: refs/heads/master 29510c2a3 -> 51cc52ae3
JAMES-2219 Check parent is not delegated during mailbox creation Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/6a9f1f93 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/6a9f1f93 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/6a9f1f93 Branch: refs/heads/master Commit: 6a9f1f9309ed88ff4e703c63d4d408358158fde9 Parents: f656a9e Author: benwa <[email protected]> Authored: Thu Nov 16 10:05:44 2017 +0700 Committer: Antoine Duprat <[email protected]> Committed: Thu Nov 16 14:19:16 2017 +0100 ---------------------------------------------------------------------- .../apache/james/mailbox/model/MailboxPath.java | 4 +++ .../mailbox/store/StoreMailboxManager.java | 13 ++++----- .../james/mailbox/store/StoreRightManager.java | 2 +- .../mailbox/store/StoreMailboxManagerTest.java | 9 ++++-- .../cucumber/SetMailboxesMethodStepdefs.java | 29 ++++++++++++++++++++ .../resources/cucumber/GetMailboxes.feature | 9 ++++++ .../methods/SetMailboxesCreationProcessor.java | 14 ++++++++++ .../methods/SetMessagesCreationProcessor.java | 2 +- .../apache/james/jmap/model/MailboxFactory.java | 6 +--- 9 files changed, 71 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java index 02daf0b..de5e9ef 100644 --- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxPath.java @@ -99,6 +99,10 @@ public class MailboxPath { return name; } + public boolean belongsTo(MailboxSession mailboxSession) { + return mailboxSession.getUser().isSameUser(user); + } + /** * Return a list of MailboxPath representing the hierarchy levels of this * MailboxPath. E.g. INBOX.main.sub would yield http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java index 88c1aea..c813fb8 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java @@ -37,7 +37,6 @@ import org.apache.james.mailbox.MailboxPathLocker; import org.apache.james.mailbox.MailboxPathLocker.LockAwareExecution; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MailboxSession.SessionType; -import org.apache.james.mailbox.MailboxSession.User; import org.apache.james.mailbox.MailboxSessionIdGenerator; import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.StandardMailboxMetaDataComparator; @@ -477,7 +476,7 @@ public class StoreMailboxManager implements MailboxManager { } private boolean belongsToCurrentUser(Mailbox mailbox, MailboxSession session) { - return session.getUser().isSameUser(mailbox.getUser()); + return mailbox.generateAssociatedPath().belongsTo(session); } private boolean userHasLookupRightsOn(Mailbox mailbox, MailboxSession session) throws MailboxException { @@ -523,7 +522,7 @@ public class StoreMailboxManager implements MailboxManager { @Override public void deleteMailbox(final MailboxPath mailboxPath, final MailboxSession session) throws MailboxException { LOGGER.info("deleteMailbox " + mailboxPath); - assertIsOwner(session.getUser(), mailboxPath); + assertIsOwner(session, mailboxPath); final MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session); Mailbox mailbox = mapper.execute((Mapper.Transaction<Mailbox>) () -> { @@ -550,14 +549,14 @@ public class StoreMailboxManager implements MailboxManager { throw new MailboxExistsException(to.toString()); } - assertIsOwner(session.getUser(), from); + assertIsOwner(session, from); MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session); mapper.execute(Mapper.toTransaction(() -> doRenameMailbox(from, to, session, mapper))); } - private void assertIsOwner(User user, MailboxPath mailboxPath) throws MailboxNotFoundException { - if (!user.isSameUser(mailboxPath.getUser())) { - LOGGER.info("Mailbox " + mailboxPath.asString() + " does not belong to " + user.getUserName()); + private void assertIsOwner(MailboxSession mailboxSession, MailboxPath mailboxPath) throws MailboxNotFoundException { + if (!mailboxPath.belongsTo(mailboxSession)) { + LOGGER.info("Mailbox " + mailboxPath.asString() + " does not belong to " + mailboxSession.getUser().getUserName()); throw new MailboxNotFoundException(mailboxPath.asString()); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java index f3209d0..9c789aa 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java @@ -241,7 +241,7 @@ public class StoreRightManager implements RightManager { */ @VisibleForTesting static MailboxACL filteredForSession(Mailbox mailbox, MailboxACL acl, MailboxSession mailboxSession) throws UnsupportedRightException { - if (mailboxSession.getUser().isSameUser(mailbox.getUser())) { + if (mailbox.generateAssociatedPath().belongsTo(mailboxSession)) { return acl; } http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java index b97d993..7228e55 100644 --- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java +++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java @@ -99,7 +99,8 @@ public class StoreMailboxManagerTest { @Test public void getMailboxShouldReturnMailboxManagerWhenKnownId() throws Exception { Mailbox mockedMailbox = mock(Mailbox.class); - when(mockedMailbox.getUser()).thenReturn(CURRENT_USER); + when(mockedMailbox.generateAssociatedPath()) + .thenReturn(MailboxPath.forUser(CURRENT_USER, "mailboxName")); when(mockedMailbox.getMailboxId()).thenReturn(MAILBOX_ID); when(mockedMailboxMapper.findMailboxById(MAILBOX_ID)).thenReturn(mockedMailbox); @@ -111,7 +112,8 @@ public class StoreMailboxManagerTest { @Test public void getMailboxShouldReturnMailboxManagerWhenKnownIdAndDifferentCaseUser() throws Exception { Mailbox mockedMailbox = mock(Mailbox.class); - when(mockedMailbox.getUser()).thenReturn("uSEr"); + when(mockedMailbox.generateAssociatedPath()) + .thenReturn(MailboxPath.forUser("uSEr", "mailboxName")); when(mockedMailbox.getMailboxId()).thenReturn(MAILBOX_ID); when(mockedMailboxMapper.findMailboxById(MAILBOX_ID)).thenReturn(mockedMailbox); @@ -124,7 +126,8 @@ public class StoreMailboxManagerTest { public void getMailboxShouldThrowWhenMailboxDoesNotMatchUserWithoutRight() throws Exception { Mailbox mockedMailbox = mock(Mailbox.class); when(mockedMailbox.getACL()).thenReturn(new MailboxACL()); - when(mockedMailbox.getUser()).thenReturn("other.user"); + when(mockedMailbox.generateAssociatedPath()) + .thenReturn(MailboxPath.forUser("other.user", "mailboxName")); when(mockedMailbox.getMailboxId()).thenReturn(MAILBOX_ID); when(mockedMailboxMapper.findMailboxById(MAILBOX_ID)).thenReturn(mockedMailbox); when(mockedMailboxMapper.findMailboxByPath(any())).thenReturn(mockedMailbox); http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/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 9d35294..ef22490 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,28 @@ public class SetMailboxesMethodStepdefs { renamingMailbox(mailbox, newMailboxName); } + @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); + userStepdefs.execWithUser(user, () -> { + String requestBody = + "[" + + " [ \"setMailboxes\"," + + " {" + + " \"create\": {" + + " \"" + creationId + "\" : {" + + " \"name\" : \"" + mailboxName + "\"," + + " \"parentId\" : \"" + parentMailbox.getMailboxId().serialize() + "\"" + + " }" + + " }" + + " }," + + " \"#0\"" + + " ]" + + "]"; + httpClient.post(requestBody); + }); + } + @When("^\"([^\"]*)\" renames (?:her|his) mailbox \"([^\"]*)\" to \"([^\"]*)\"$") public void renamingMailbox(String user, String actualMailboxName, String newMailboxName) throws Throwable { Mailbox mailbox = mainStepdefs.mailboxProbe.getMailbox("#private", user, actualMailboxName); @@ -253,4 +275,11 @@ public class SetMailboxesMethodStepdefs { assertThat(httpClient.jsonPath.<Map<String, String>>read("[0][1].notDestroyed")) .containsOnlyKeys(mailbox.getMailboxId().serialize()); } + + + @Then("^mailbox with creationId \"([^\"]*)\" is not created") + public void assertNotCreated(String creationId) throws Exception { + assertThat(httpClient.jsonPath.<Map<String, String>>read("[0][1].notCreated")) + .containsOnlyKeys(creationId); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/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 9ec81e9..9d5dbb0 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 @@ -286,3 +286,12 @@ Feature: GetMailboxes method Given "[email protected]" deletes the mailbox "shared" owned by "[email protected]" When "[email protected]" lists mailboxes Then the mailboxes should contain "shared" in "Personal" namespace + + Scenario: A sharee should not be able to create a shared mailbox child + Given "[email protected]" creates mailbox "sharedChild" with creationId "c-01" in mailbox "shared" owned by "[email protected]" + When "[email protected]" lists mailboxes + Then the mailboxes should contain "shared" in "Personal" namespace + + 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 http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java index 0f71d2e..39695b1 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java @@ -27,6 +27,7 @@ import java.util.Optional; import javax.inject.Inject; +import org.apache.james.jmap.exceptions.MailboxNotOwnedException; import org.apache.james.jmap.exceptions.MailboxParentNotFoundException; import org.apache.james.jmap.model.MailboxCreationId; import org.apache.james.jmap.model.MailboxFactory; @@ -134,6 +135,11 @@ public class SetMailboxesCreationProcessor implements SetMailboxesProcessor { .type("invalidArguments") .description("The mailbox name length is too long") .build()); + } catch (MailboxNotOwnedException e) { + builder.notCreated(mailboxCreationId, SetError.builder() + .type("invalidArguments") + .description("The mailbox can not be created with a parent mailbox belonging to another user") + .build()); } catch (MailboxNameException | MailboxParentNotFoundException e) { builder.notCreated(mailboxCreationId, SetError.builder() .type("invalidArguments") @@ -168,12 +174,20 @@ public class SetMailboxesCreationProcessor implements SetMailboxesProcessor { MailboxCreationId parentId = mailboxRequest.getParentId().get(); MailboxPath parentPath = getMailboxPath(creationIdsToCreatedMailboxId, mailboxSession, parentId); + assertBelongsToUser(parentPath, mailboxSession); + return MailboxPath.forUser(mailboxSession.getUser().getUserName(), parentPath.getName() + mailboxSession.getPathDelimiter() + mailboxRequest.getName()); } return MailboxPath.forUser(mailboxSession.getUser().getUserName(), mailboxRequest.getName()); } + private void assertBelongsToUser(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxNotOwnedException { + if (!mailboxPath.belongsTo(mailboxSession)) { + throw new MailboxNotOwnedException(); + } + } + private MailboxPath getMailboxPath(Map<MailboxCreationId, MailboxId> creationIdsToCreatedMailboxId, MailboxSession mailboxSession, MailboxCreationId parentId) throws MailboxException { Optional<MailboxId> mailboxId = getMailboxIdFromCreationId(parentId); Optional<MailboxId> mailboxIdFromCreationId = Optional.ofNullable(creationIdsToCreatedMailboxId.get(parentId)); http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java index 4bb43dc..3f81798 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java @@ -229,7 +229,7 @@ public class SetMessagesCreationProcessor implements SetMessagesProcessor { .map(mailboxIdFactory::fromString) .map(findMailbox.sneakyThrow()) .map(Throwing.function(MessageManager::getMailboxPath)) - .anyMatch(path -> !session.getUser().isSameUser(path.getUser())); + .anyMatch(path -> !path.belongsTo(session)); } private MessageWithId handleOutboxMessages(CreationMessageEntry entry, MailboxSession session) throws MailboxException, MessagingException { http://git-wip-us.apache.org/repos/asf/james-project/blob/6a9f1f93/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java index c20aaad..fca9da2 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java @@ -100,7 +100,7 @@ public class MailboxFactory { private Mailbox fromMessageManager(MessageManager messageManager, Optional<List<MailboxMetaData>> userMailboxesMetadata, MailboxSession mailboxSession) throws MailboxException { MailboxPath mailboxPath = messageManager.getMailboxPath(); - boolean isOwner = isSameUser(mailboxSession, mailboxPath); + boolean isOwner = mailboxPath.belongsTo(mailboxSession); Optional<Role> role = Role.from(mailboxPath.getName()); MailboxCounters mailboxCounters = messageManager.getMailboxCounters(mailboxSession); MessageManager.MetaData metaData = messageManager.getMetaData(NO_RESET_RECENT, mailboxSession, MessageManager.MetaData.FetchGroup.NO_COUNT); @@ -135,10 +135,6 @@ public class MailboxFactory { return MailboxNamespace.delegated(mailboxPath.getUser()); } - private boolean isSameUser(MailboxSession mailboxSession, MailboxPath mailboxPath) { - return mailboxSession.getUser().isSameUser(mailboxPath.getUser()); - } - @VisibleForTesting String getName(MailboxPath mailboxPath, MailboxSession mailboxSession) { String name = mailboxPath.getName(); if (name.contains(String.valueOf(mailboxSession.getPathDelimiter()))) { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
