JAMES-2215 Don't disclose mailbox metadata details when shared with only lookup right
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/ec485b53 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/ec485b53 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/ec485b53 Branch: refs/heads/master Commit: ec485b53d6f39d19883041b60ba2e8a11e4cbf23 Parents: cda4eec Author: benwa <[email protected]> Authored: Tue Nov 14 10:24:49 2017 +0700 Committer: Antoine Duprat <[email protected]> Committed: Wed Nov 15 09:57:30 2017 +0100 ---------------------------------------------------------------------- .../james/mailbox/MailboxManagerTest.java | 104 +++++++++++++++++++ .../james/mailbox/store/MailboxMetaData.java | 24 +++++ .../mailbox/store/StoreMessageManager.java | 31 ++++-- .../james/mailbox/copier/MailboxCopierTest.java | 7 +- .../resources/cucumber/GetMailboxes.feature | 8 ++ 5 files changed, 161 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/ec485b53/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java index be94d90..69298c7 100644 --- a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java +++ b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java @@ -22,6 +22,7 @@ import static org.assertj.core.api.Assertions.assertThat; import java.io.ByteArrayInputStream; import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; import java.util.Date; import java.util.List; import java.util.Optional; @@ -35,6 +36,7 @@ import org.apache.james.mailbox.mock.MockMailboxManager; import org.apache.james.mailbox.model.MailboxACL; import org.apache.james.mailbox.model.MailboxAnnotation; import org.apache.james.mailbox.model.MailboxAnnotationKey; +import org.apache.james.mailbox.model.MailboxCounters; import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.model.MailboxMetaData; import org.apache.james.mailbox.model.MailboxPath; @@ -42,6 +44,7 @@ import org.apache.james.mailbox.model.MessageId; import org.apache.james.mailbox.model.MultimailboxesSearchQuery; import org.apache.james.mailbox.model.SearchQuery; import org.apache.james.mailbox.model.search.MailboxQuery; +import org.assertj.core.api.JUnitSoftAssertions; import org.junit.Assume; import org.junit.Rule; import org.junit.Test; @@ -81,6 +84,9 @@ public abstract class MailboxManagerTest { @Rule public ExpectedException expected = ExpectedException.none(); + @Rule + public JUnitSoftAssertions softly = new JUnitSoftAssertions(); + private MailboxManager mailboxManager; private MailboxSession session; @@ -859,4 +865,102 @@ public abstract class MailboxManagerTest { assertThat(mailboxManager.search(mailboxQuery, session2)) .isEmpty(); } + + @Test + public void getMailboxCountersShouldReturnDefaultValueWhenNoReadRight() throws MailboxException { + Assume.assumeTrue(mailboxManager.hasCapability(MailboxCapabilities.ACL)); + MailboxSession session1 = mailboxManager.createSystemSession(USER_1); + MailboxSession session2 = mailboxManager.createSystemSession(USER_2); + MailboxPath inbox1 = MailboxPath.inbox(session1); + mailboxManager.createMailbox(inbox1, session1); + mailboxManager.setRights(inbox1, + MailboxACL.EMPTY.apply(MailboxACL.command() + .forUser(USER_2) + .rights(MailboxACL.Right.Lookup) + .asAddition()), + session1); + ByteArrayInputStream message = new ByteArrayInputStream("Subject: any\n\nbdy".getBytes(StandardCharsets.UTF_8)); + boolean isRecent = true; + mailboxManager.getMailbox(inbox1, session1) + .appendMessage(message, new Date(), session1, isRecent, new Flags()); + + MailboxCounters mailboxCounters = mailboxManager.getMailbox(inbox1, session2) + .getMailboxCounters(session2); + + assertThat(mailboxCounters) + .isEqualTo(MailboxCounters.builder() + .count(0) + .unseen(0) + .build()); + } + + @Test + public void getMailboxCountersShouldReturnStoredValueWhenReadRight() throws MailboxException { + Assume.assumeTrue(mailboxManager.hasCapability(MailboxCapabilities.ACL)); + MailboxSession session1 = mailboxManager.createSystemSession(USER_1); + MailboxSession session2 = mailboxManager.createSystemSession(USER_2); + MailboxPath inbox1 = MailboxPath.inbox(session1); + mailboxManager.createMailbox(inbox1, session1); + mailboxManager.setRights(inbox1, + MailboxACL.EMPTY.apply(MailboxACL.command() + .forUser(USER_2) + .rights(MailboxACL.Right.Lookup, MailboxACL.Right.Read) + .asAddition()), + session1); + ByteArrayInputStream message = new ByteArrayInputStream("Subject: any\n\nbdy".getBytes(StandardCharsets.UTF_8)); + boolean isRecent = true; + mailboxManager.getMailbox(inbox1, session1) + .appendMessage(message, new Date(), session1, isRecent, new Flags()); + + MailboxCounters mailboxCounters = mailboxManager.getMailbox(inbox1, session2) + .getMailboxCounters(session2); + + assertThat(mailboxCounters) + .isEqualTo(MailboxCounters.builder() + .count(1) + .unseen(1) + .build()); + } + + @Test + public void getMetaDataShouldReturnDefaultValueWhenNoReadRight() throws MailboxException { + Assume.assumeTrue(mailboxManager.hasCapability(MailboxCapabilities.ACL)); + MailboxSession session1 = mailboxManager.createSystemSession(USER_1); + MailboxSession session2 = mailboxManager.createSystemSession(USER_2); + MailboxPath inbox1 = MailboxPath.inbox(session1); + mailboxManager.createMailbox(inbox1, session1); + mailboxManager.setRights(inbox1, + MailboxACL.EMPTY.apply(MailboxACL.command() + .forUser(USER_2) + .rights(MailboxACL.Right.Lookup) + .asAddition()), + session1); + ByteArrayInputStream message = new ByteArrayInputStream("Subject: any\n\nbdy".getBytes(StandardCharsets.UTF_8)); + boolean isRecent = true; + mailboxManager.getMailbox(inbox1, session1) + .appendMessage(message, new Date(), session1, isRecent, new Flags()); + + boolean resetRecent = false; + MessageManager.MetaData metaData = mailboxManager.getMailbox(inbox1, session2) + .getMetaData(resetRecent, session2, MessageManager.MetaData.FetchGroup.UNSEEN_COUNT); + + softly.assertThat(metaData) + .extracting(MessageManager.MetaData::getHighestModSeq) + .contains(0L); + softly.assertThat(metaData) + .extracting(MessageManager.MetaData::getUidNext) + .contains(MessageUid.MIN_VALUE); + softly.assertThat(metaData) + .extracting(MessageManager.MetaData::getMessageCount) + .contains(0L); + softly.assertThat(metaData) + .extracting(MessageManager.MetaData::getUnseenCount) + .contains(0L); + softly.assertThat(metaData) + .extracting(MessageManager.MetaData::getRecent) + .contains(ImmutableList.of()); + softly.assertThat(metaData) + .extracting(MessageManager.MetaData::getPermanentFlags) + .contains(new Flags()); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/ec485b53/mailbox/store/src/main/java/org/apache/james/mailbox/store/MailboxMetaData.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/MailboxMetaData.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/MailboxMetaData.java index 7dbfa1c..a9a4f26 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/MailboxMetaData.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/MailboxMetaData.java @@ -27,13 +27,37 @@ import javax.mail.Flags; import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.MessageUid; +import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.model.MailboxACL; +import com.google.common.collect.ImmutableList; + /** * Describes the current state of a mailbox. */ public class MailboxMetaData implements MessageManager.MetaData { + public static MailboxMetaData sensibleInformationFree(MailboxACL resolvedAcl, long uidValidity, boolean writeable, boolean modSeqPermanent) throws MailboxException { + ImmutableList<MessageUid> recents = ImmutableList.of(); + MessageUid uidNext = MessageUid.MIN_VALUE; + long highestModSeq = 0L; + long messageCount = 0L; + long unseenCount = 0L; + MessageUid firstUnseen = null; + return new MailboxMetaData( + recents, + new Flags(), + uidValidity, + uidNext, + highestModSeq, + messageCount, + unseenCount, + firstUnseen, + writeable, + modSeqPermanent, + resolvedAcl); + } + private final long recentCount; private final List<MessageUid> recent; private final Flags permanentFlags; http://git-wip-us.apache.org/repos/asf/james-project/blob/ec485b53/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java index 173c247..03edb61 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java @@ -110,6 +110,11 @@ public class StoreMessageManager implements org.apache.james.mailbox.MessageMana .setMaxLineLen(-1) .build(); + private static final MailboxCounters ZERO_MAILBOX_COUNTERS = MailboxCounters.builder() + .count(0) + .unseen(0) + .build(); + /** * The minimal Permanent flags the {@link MessageManager} must support. <br> * @@ -232,7 +237,10 @@ public class StoreMessageManager implements org.apache.james.mailbox.MessageMana @Override public MailboxCounters getMailboxCounters(MailboxSession mailboxSession) throws MailboxException { - return mapperFactory.createMessageMapper(mailboxSession).getMailboxCounters(mailbox); + if (storeRightManager.hasRight(mailbox, MailboxACL.Right.Read, mailboxSession)) { + return mapperFactory.createMessageMapper(mailboxSession).getMailboxCounters(mailbox); + } + return ZERO_MAILBOX_COUNTERS; } /** @@ -463,17 +471,21 @@ public class StoreMessageManager implements org.apache.james.mailbox.MessageMana @Override public MetaData getMetaData(boolean resetRecent, MailboxSession mailboxSession, MetaData.FetchGroup fetchGroup) throws MailboxException { - - final List<MessageUid> recent; - final Flags permanentFlags = getPermanentFlags(mailboxSession); - final long uidValidity = getMailboxEntity().getUidValidity(); + MailboxACL resolvedAcl = storeRightManager.getResolvedMailboxACL(mailbox, mailboxSession); + boolean hasReadRight = storeRightManager.hasRight(mailbox, MailboxACL.Right.Read, mailboxSession); + if (!hasReadRight) { + return MailboxMetaData.sensibleInformationFree(resolvedAcl, getMailboxEntity().getUidValidity(), isWriteable(mailboxSession), isModSeqPermanent(mailboxSession)); + } + List<MessageUid> recent; + Flags permanentFlags = getPermanentFlags(mailboxSession); + long uidValidity = getMailboxEntity().getUidValidity(); MessageUid uidNext = mapperFactory.getMessageMapper(mailboxSession).getLastUid(mailbox) .map(MessageUid::next) .orElse(MessageUid.MIN_VALUE); - final long highestModSeq = mapperFactory.getMessageMapper(mailboxSession).getHighestModSeq(mailbox); - final long messageCount; - final long unseenCount; - final MessageUid firstUnseen; + long highestModSeq = mapperFactory.getMessageMapper(mailboxSession).getHighestModSeq(mailbox); + long messageCount; + long unseenCount; + MessageUid firstUnseen; switch (fetchGroup) { case UNSEEN_COUNT: unseenCount = countUnseenMessagesInMailbox(mailboxSession); @@ -507,7 +519,6 @@ public class StoreMessageManager implements org.apache.james.mailbox.MessageMana recent = new ArrayList<>(); break; } - MailboxACL resolvedAcl = storeRightManager.getResolvedMailboxACL(mailbox, mailboxSession); return new MailboxMetaData(recent, permanentFlags, uidValidity, uidNext, highestModSeq, messageCount, unseenCount, firstUnseen, isWriteable(mailboxSession), isModSeqPermanent(mailboxSession), resolvedAcl); } http://git-wip-us.apache.org/repos/asf/james-project/blob/ec485b53/mailbox/tool/src/test/java/org/apache/james/mailbox/copier/MailboxCopierTest.java ---------------------------------------------------------------------- diff --git a/mailbox/tool/src/test/java/org/apache/james/mailbox/copier/MailboxCopierTest.java b/mailbox/tool/src/test/java/org/apache/james/mailbox/copier/MailboxCopierTest.java index 0bf9ec4..7e702c8 100644 --- a/mailbox/tool/src/test/java/org/apache/james/mailbox/copier/MailboxCopierTest.java +++ b/mailbox/tool/src/test/java/org/apache/james/mailbox/copier/MailboxCopierTest.java @@ -119,7 +119,6 @@ public class MailboxCopierTest { * @throws BadCredentialsException */ private void assertMailboxManagerSize(MailboxManager mailboxManager, int multiplicationFactor) throws BadCredentialsException, MailboxException { - MailboxSession mailboxSession = mailboxManager.createSystemSession("manager"); mailboxManager.startProcessingRequest(mailboxSession); @@ -128,8 +127,10 @@ public class MailboxCopierTest { assertThat(mailboxPathList).hasSize(MockMailboxManager.EXPECTED_MAILBOXES_COUNT); for (MailboxPath mailboxPath: mailboxPathList) { - MessageManager messageManager = mailboxManager.getMailbox(mailboxPath, mailboxSession); - assertThat(messageManager.getMetaData(false, mailboxSession, FetchGroup.NO_UNSEEN).getMessageCount()).isEqualTo(MockMailboxManager.MESSAGE_PER_MAILBOX_COUNT * multiplicationFactor); + MailboxSession userSession = mailboxManager.createSystemSession(mailboxPath.getUser()); + mailboxManager.startProcessingRequest(mailboxSession); + MessageManager messageManager = mailboxManager.getMailbox(mailboxPath, userSession); + assertThat(messageManager.getMetaData(false, userSession, FetchGroup.NO_UNSEEN).getMessageCount()).isEqualTo(MockMailboxManager.MESSAGE_PER_MAILBOX_COUNT * multiplicationFactor); } mailboxManager.endProcessingRequest(mailboxSession); http://git-wip-us.apache.org/repos/asf/james-project/blob/ec485b53/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 45c545e..9bbe9cc 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 @@ -234,3 +234,11 @@ Feature: GetMailboxes method Then the mailbox "shared2" has 1 message And the mailbox "shared2" has 1 unseen message + Scenario: Lookup right should not be enough to read message and unseen counts + Given "[email protected]" has a mailbox "shared2" + And "[email protected]" shares her mailbox "shared2" with "[email protected]" with "l" rights + And "[email protected]" has a message "m1" in "shared2" mailbox + When "[email protected]" ask for mailboxes + Then the mailbox "shared2" has 0 messages + And the mailbox "shared2" has 0 unseen messages + --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
