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/15 12:33:34 UTC

[3/4] james-project git commit: JAMES-2215 Don't disclose mailbox metadata details when shared with only lookup right

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 <bt...@linagora.com>
Authored: Tue Nov 14 10:24:49 2017 +0700
Committer: Antoine Duprat <ad...@linagora.com>
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 "alice@domain.tld" has a mailbox "shared2"
+    And "alice@domain.tld" shares her mailbox "shared2" with "bob@domain.tld" with "l" rights
+    And "alice@domain.tld" has a message "m1" in "shared2" mailbox
+    When "bob@domain.tld" ask for mailboxes
+    Then the mailbox "shared2" has 0 messages
+    And the mailbox "shared2" has 0 unseen messages
+


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org