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/09/20 20:31:58 UTC
[3/5] james-project git commit: JAMES-2145 Attachment security,
user cannot download attachment of another user
JAMES-2145 Attachment security, user cannot download attachment of another user
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/11842e0f
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/11842e0f
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/11842e0f
Branch: refs/heads/master
Commit: 11842e0f2f974e731edf175d7a1b5706e7b9242a
Parents: c9ac405
Author: quynhn <qn...@linagora.com>
Authored: Mon Sep 18 18:03:15 2017 +0700
Committer: Antoine Duprat <ad...@linagora.com>
Committed: Wed Sep 20 16:18:39 2017 +0200
----------------------------------------------------------------------
.../apache/james/mailbox/AttachmentManager.java | 2 -
.../apache/james/mailbox/MessageIdManager.java | 4 +
.../james/mailbox/model/AttachmentId.java | 9 ++-
.../james/mailbox/model/AttachmentIdTest.java | 39 +++++++---
.../CassandraMailboxSessionMapperFactory.java | 2 +-
.../mail/CassandraMessageIdMapper.java | 5 +-
.../InMemoryMailboxSessionMapperFactory.java | 4 +-
.../inmemory/InMemoryMessageIdManager.java | 41 ++++++----
.../mailbox/store/StoreAttachmentManager.java | 52 ++++++++-----
.../james/mailbox/store/StoreBlobManager.java | 43 +++++------
.../mailbox/store/StoreMessageIdManager.java | 12 +++
.../store/mail/AttachmentMapperFactory.java | 5 +-
.../mailbox/store/mail/MessageIdMapper.java | 3 +-
.../AbstractMessageIdManagerStorageTest.java | 28 +++++++
.../store/StoreAttachmentManagerTest.java | 80 ++++++++++++++++++++
.../store/TestMailboxSessionMapperFactory.java | 5 +-
.../integration/cucumber/DownloadStepdefs.java | 5 ++
.../test/resources/cucumber/DownloadGet.feature | 24 +++++-
18 files changed, 276 insertions(+), 87 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/api/src/main/java/org/apache/james/mailbox/AttachmentManager.java
----------------------------------------------------------------------
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/AttachmentManager.java b/mailbox/api/src/main/java/org/apache/james/mailbox/AttachmentManager.java
index fd21b54..fb2bc21 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/AttachmentManager.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/AttachmentManager.java
@@ -37,6 +37,4 @@ public interface AttachmentManager {
void storeAttachment(Attachment attachment, MailboxSession mailboxSession) throws MailboxException;
void storeAttachmentsForMessage(Collection<Attachment> attachments, MessageId ownerMessageId, MailboxSession mailboxSession) throws MailboxException;
-
- Collection<MessageId> getRelatedMessageIds(AttachmentId attachmentId, MailboxSession mailboxSession) throws MailboxException;
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/api/src/main/java/org/apache/james/mailbox/MessageIdManager.java
----------------------------------------------------------------------
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/MessageIdManager.java b/mailbox/api/src/main/java/org/apache/james/mailbox/MessageIdManager.java
index eb35378..309145e 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/MessageIdManager.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/MessageIdManager.java
@@ -19,7 +19,9 @@
package org.apache.james.mailbox;
+import java.util.Collection;
import java.util.List;
+import java.util.Set;
import javax.mail.Flags;
@@ -32,6 +34,8 @@ import org.apache.james.mailbox.model.MessageResult.FetchGroup;
public interface MessageIdManager {
+ Set<MessageId> accessibleMessages(Collection<MessageId> messageIds, final MailboxSession mailboxSession) throws MailboxException;
+
void setFlags(Flags newState, FlagsUpdateMode replace, MessageId messageId, List<MailboxId> mailboxIds, MailboxSession mailboxSession) throws MailboxException;
List<MessageResult> getMessages(List<MessageId> messageId, FetchGroup minimal, MailboxSession mailboxSession) throws MailboxException;
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java
----------------------------------------------------------------------
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java
index 05c3e62..81a5588 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/AttachmentId.java
@@ -40,7 +40,7 @@ public class AttachmentId {
private static final String DEFAULT_MIME_TYPE = "application/octet-stream";
public static AttachmentId forPayloadAndType(byte[] payload, String contentType) {
- Preconditions.checkArgument(payload != null);
+ Preconditions.checkNotNull(payload);
Preconditions.checkArgument(!Strings.isNullOrEmpty(contentType));
return new AttachmentId(computeRawId(payload, contentType));
@@ -60,8 +60,13 @@ public class AttachmentId {
.orElse(DEFAULT_MIME_TYPE);
}
+ public static AttachmentId from(BlobId blobId) {
+ return new AttachmentId(blobId.asString());
+ }
+
public static AttachmentId from(String id) {
- Preconditions.checkArgument(!Strings.isNullOrEmpty(id));
+ Preconditions.checkNotNull(id);
+ Preconditions.checkArgument(!id.isEmpty());
return new AttachmentId(id);
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java
----------------------------------------------------------------------
diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java
index 1f56066..148149e 100644
--- a/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/AttachmentIdTest.java
@@ -20,6 +20,7 @@
package org.apache.james.mailbox.model;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.UUID;
@@ -47,40 +48,54 @@ public class AttachmentIdTest {
AttachmentId attachmentId2 = AttachmentId.forPayloadAndType("payload".getBytes(), "text/html; charset=UTF-16");
assertThat(attachmentId.getId()).isEqualTo(attachmentId2.getId());
}
-
- @Test(expected = IllegalArgumentException.class)
+
+ @Test
public void forPayloadAndTypeShouldThrowWhenPayloadIsNull() {
- AttachmentId.forPayloadAndType(null, null);
+ assertThatThrownBy(() -> AttachmentId.forPayloadAndType(null, "text/plain")).isInstanceOf(NullPointerException.class);
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void forPayloadAndTypeShouldThrowWhenTypeIsNull() {
- AttachmentId.forPayloadAndType("payload".getBytes(), null);
+ assertThatThrownBy(() -> AttachmentId.forPayloadAndType("payload".getBytes(), null)).isInstanceOf(IllegalArgumentException.class);
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void forPayloadAndTypeShouldThrowWhenTypeIsEmpty() {
- AttachmentId.forPayloadAndType("payload".getBytes(), "");
+ assertThatThrownBy(() -> AttachmentId.forPayloadAndType("payload".getBytes(), "")).isInstanceOf(IllegalArgumentException.class);
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void fromShouldThrowWhenIdIsNull() {
- AttachmentId.from(null);
+ String value = null;
+ assertThatThrownBy(() -> AttachmentId.from(value)).isInstanceOf(NullPointerException.class);
+ }
+
+ @Test
+ public void fromShouldThrowWhenBlobIdIsNull() {
+ BlobId value = null;
+ assertThatThrownBy(() -> AttachmentId.from(value)).isInstanceOf(NullPointerException.class);
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void fromShouldThrowWhenIdIsEmpty() {
- AttachmentId.from("");
+ assertThatThrownBy(() -> AttachmentId.from("")).isInstanceOf(IllegalArgumentException.class);
}
@Test
- public void fromShouldWork() {
+ public void fromStringShouldWork() {
String expectedId = "f07e5a815613c5abeddc4b682247a4c42d8a95df";
AttachmentId attachmentId = AttachmentId.from(expectedId);
assertThat(attachmentId.getId()).isEqualTo(expectedId);
}
@Test
+ public void fromBlobIdShouldWork() {
+ String expectedId = "f07e5a815613c5abeddc4b682247a4c42d8a95df";
+ AttachmentId attachmentId = AttachmentId.from(BlobId.fromString(expectedId));
+ assertThat(attachmentId.getId()).isEqualTo(expectedId);
+ }
+
+ @Test
public void asUUIDShouldReturnAValidUUID() {
AttachmentId attachmentId = AttachmentId.from("magic");
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java
----------------------------------------------------------------------
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java
index 9d4e059..afffe6d 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java
@@ -183,7 +183,7 @@ public class CassandraMailboxSessionMapperFactory extends MailboxSessionMapperFa
}
@Override
- public AttachmentMapper getAttachmentMapper(MailboxSession session) throws MailboxException {
+ public AttachmentMapper getAttachmentMapper(MailboxSession session) {
AttachmentMapper mapper = (AttachmentMapper) session.getAttributes().get(ATTACHMENTMAPPER);
if (mapper == null) {
mapper = createAttachmentMapper(session);
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java
----------------------------------------------------------------------
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java
index ff9fb69..b5bd133 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdMapper.java
@@ -18,6 +18,7 @@
****************************************************************/
package org.apache.james.mailbox.cassandra.mail;
+import java.util.Collection;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
@@ -88,12 +89,12 @@ public class CassandraMessageIdMapper implements MessageIdMapper {
}
@Override
- public List<MailboxMessage> find(List<MessageId> messageIds, FetchType fetchType) {
+ public List<MailboxMessage> find(Collection<MessageId> messageIds, FetchType fetchType) {
return findAsStream(messageIds, fetchType)
.collect(Guavate.toImmutableList());
}
- private Stream<SimpleMailboxMessage> findAsStream(List<MessageId> messageIds, FetchType fetchType) {
+ private Stream<SimpleMailboxMessage> findAsStream(Collection<MessageId> messageIds, FetchType fetchType) {
return FluentFutureStream.ofNestedStreams(
messageIds.stream()
.map(messageId -> imapUidDAO.retrieve((CassandraMessageId) messageId, Optional.empty())))
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMailboxSessionMapperFactory.java
----------------------------------------------------------------------
diff --git a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMailboxSessionMapperFactory.java b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMailboxSessionMapperFactory.java
index adae134..6a4709c 100644
--- a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMailboxSessionMapperFactory.java
+++ b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMailboxSessionMapperFactory.java
@@ -80,7 +80,7 @@ public class InMemoryMailboxSessionMapperFactory extends MailboxSessionMapperFac
}
@Override
- public AttachmentMapper createAttachmentMapper(MailboxSession session) throws MailboxException {
+ public AttachmentMapper createAttachmentMapper(MailboxSession session) {
return attachmentMapper;
}
@@ -107,7 +107,7 @@ public class InMemoryMailboxSessionMapperFactory extends MailboxSessionMapperFac
}
@Override
- public AttachmentMapper getAttachmentMapper(MailboxSession session) throws MailboxException {
+ public AttachmentMapper getAttachmentMapper(MailboxSession session) {
return attachmentMapper;
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMessageIdManager.java
----------------------------------------------------------------------
diff --git a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMessageIdManager.java b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMessageIdManager.java
index 6d63a64..2b0256c 100644
--- a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMessageIdManager.java
+++ b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/InMemoryMessageIdManager.java
@@ -19,10 +19,14 @@
package org.apache.james.mailbox.inmemory;
+import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
+import java.util.Set;
import java.util.function.Predicate;
+import java.util.stream.Stream;
+
import javax.inject.Inject;
import javax.mail.Flags;
@@ -42,7 +46,9 @@ import org.apache.james.mailbox.model.MessageId;
import org.apache.james.mailbox.model.MessageRange;
import org.apache.james.mailbox.model.MessageResult;
import org.apache.james.mailbox.model.MessageResult.FetchGroup;
+import org.apache.james.util.OptionalUtils;
+import com.github.fge.lambdas.Throwing;
import com.github.steveash.guavate.Guavate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -58,7 +64,7 @@ public class InMemoryMessageIdManager implements MessageIdManager {
public InMemoryMessageIdManager(MailboxManager mailboxManager) {
this.mailboxManager = mailboxManager;
}
-
+
@Override
public void setFlags(Flags newState, FlagsUpdateMode flagsUpdateMode, MessageId messageId, List<MailboxId> mailboxIds, MailboxSession mailboxSession) throws MailboxException {
for (MailboxId mailboxId: mailboxIds) {
@@ -71,13 +77,21 @@ public class InMemoryMessageIdManager implements MessageIdManager {
}
@Override
+ public Set<MessageId> accessibleMessages(Collection<MessageId> messageIds, MailboxSession mailboxSession) throws MailboxException {
+ return getUsersMailboxIds(mailboxSession)
+ .stream()
+ .flatMap(Throwing.function(mailboxId -> retrieveMailboxMessages(mailboxId, messageIds, FetchGroupImpl.MINIMAL, mailboxSession)))
+ .map(MessageResult::getMessageId)
+ .collect(Guavate.toImmutableSet());
+ }
+
+
+ @Override
public List<MessageResult> getMessages(List<MessageId> messages, FetchGroup fetchGroup, final MailboxSession mailboxSession) throws MailboxException {
- ImmutableList.Builder<MessageResult> builder = ImmutableList.builder();
- List<MailboxId> userMailboxIds = getUsersMailboxIds(mailboxSession);
- for (MailboxId mailboxId: userMailboxIds) {
- builder.addAll(retrieveMailboxMessages(mailboxId, messages, fetchGroup, mailboxSession));
- }
- return builder.build();
+ return getUsersMailboxIds(mailboxSession)
+ .stream()
+ .flatMap(Throwing.function(mailboxId -> retrieveMailboxMessages(mailboxId, messages, FetchGroupImpl.MINIMAL, mailboxSession)))
+ .collect(Guavate.toImmutableList());
}
@Override
@@ -136,15 +150,10 @@ public class InMemoryMessageIdManager implements MessageIdManager {
.build();
}
- private List<MessageResult> retrieveMailboxMessages(MailboxId mailboxId, List<MessageId> messages, FetchGroup fetchGroup, MailboxSession mailboxSession) throws MailboxException {
- ImmutableList.Builder<MessageResult> builder = ImmutableList.builder();
- for (MessageId messageId: messages) {
- Optional<MessageResult> maybeMessage = findMessageWithId(mailboxId, messageId, fetchGroup, mailboxSession);
- if (maybeMessage.isPresent()) {
- builder.add(maybeMessage.get());
- }
- }
- return builder.build();
+ private Stream<MessageResult> retrieveMailboxMessages(MailboxId mailboxId, Collection<MessageId> messages, FetchGroup fetchGroup, MailboxSession mailboxSession) {
+ return messages.stream()
+ .map(Throwing.function(messageId -> findMessageWithId(mailboxId, messageId, fetchGroup, mailboxSession)))
+ .flatMap(OptionalUtils::toStream);
}
private void filterOnMailboxSession(List<MailboxId> mailboxIds, MailboxSession mailboxSession) throws MailboxNotFoundException {
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreAttachmentManager.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreAttachmentManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreAttachmentManager.java
index 1ee0c90..87de834 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreAttachmentManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreAttachmentManager.java
@@ -26,53 +26,71 @@ import javax.inject.Inject;
import org.apache.james.mailbox.AttachmentManager;
import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
import org.apache.james.mailbox.exception.AttachmentNotFoundException;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.model.Attachment;
import org.apache.james.mailbox.model.AttachmentId;
import org.apache.james.mailbox.model.MessageId;
-import org.apache.james.mailbox.store.mail.AttachmentMapper;
import org.apache.james.mailbox.store.mail.AttachmentMapperFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.github.steveash.guavate.Guavate;
+import com.google.common.base.Throwables;
public class StoreAttachmentManager implements AttachmentManager {
+ private static final Logger LOGGER = LoggerFactory.getLogger(StoreAttachmentManager.class);
private final AttachmentMapperFactory attachmentMapperFactory;
+ private final MessageIdManager messageIdManager;
@Inject
- public StoreAttachmentManager(AttachmentMapperFactory attachmentMapperFactory) {
+ public StoreAttachmentManager(AttachmentMapperFactory attachmentMapperFactory, MessageIdManager messageIdManager) {
this.attachmentMapperFactory = attachmentMapperFactory;
- }
-
- protected AttachmentMapperFactory getAttachmentMapperFactory() {
- return attachmentMapperFactory;
- }
-
- protected AttachmentMapper getAttachmentMapper(MailboxSession mailboxSession) throws MailboxException {
- return attachmentMapperFactory.getAttachmentMapper(mailboxSession);
+ this.messageIdManager = messageIdManager;
}
@Override
public Attachment getAttachment(AttachmentId attachmentId, MailboxSession mailboxSession) throws MailboxException, AttachmentNotFoundException {
- return getAttachmentMapper(mailboxSession).getAttachment(attachmentId);
+ if (!userHasAccessToAttachment(attachmentId, mailboxSession)) {
+ throw new AttachmentNotFoundException(attachmentId.getId());
+ }
+ return attachmentMapperFactory.getAttachmentMapper(mailboxSession).getAttachment(attachmentId);
}
@Override
public List<Attachment> getAttachments(List<AttachmentId> attachmentIds, MailboxSession mailboxSession) throws MailboxException {
- return getAttachmentMapper(mailboxSession).getAttachments(attachmentIds);
+ List<AttachmentId> accessibleAttachmentIds = attachmentIds.stream()
+ .filter(attachmentId -> userHasAccessToAttachment(attachmentId, mailboxSession))
+ .collect(Guavate.toImmutableList());
+
+ return attachmentMapperFactory.getAttachmentMapper(mailboxSession).getAttachments(accessibleAttachmentIds);
}
@Override
public void storeAttachment(Attachment attachment, MailboxSession mailboxSession) throws MailboxException {
- getAttachmentMapper(mailboxSession).storeAttachment(attachment);
+ attachmentMapperFactory.getAttachmentMapper(mailboxSession).storeAttachment(attachment);
}
@Override
public void storeAttachmentsForMessage(Collection<Attachment> attachments, MessageId ownerMessageId, MailboxSession mailboxSession) throws MailboxException {
- getAttachmentMapper(mailboxSession).storeAttachmentsForMessage(attachments, ownerMessageId);
+ attachmentMapperFactory.getAttachmentMapper(mailboxSession).storeAttachmentsForMessage(attachments, ownerMessageId);
}
- @Override
- public Collection<MessageId> getRelatedMessageIds(AttachmentId attachmentId, MailboxSession mailboxSession) throws MailboxException {
- return getAttachmentMapper(mailboxSession).getRelatedMessageIds(attachmentId);
+ private boolean userHasAccessToAttachment(AttachmentId attachmentId, MailboxSession mailboxSession) {
+ try {
+ return !messageIdManager
+ .accessibleMessages(getRelatedMessageIds(attachmentId, mailboxSession), mailboxSession)
+ .isEmpty();
+ } catch (MailboxException e) {
+ LOGGER.warn("Error while checking attachment related accessible message ids", e);
+ throw Throwables.propagate(e);
+ }
}
+
+ private Collection<MessageId> getRelatedMessageIds(AttachmentId attachmentId, MailboxSession mailboxSession) throws MailboxException {
+ return attachmentMapperFactory.getAttachmentMapper(mailboxSession).getRelatedMessageIds(attachmentId);
+ }
+
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreBlobManager.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreBlobManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreBlobManager.java
index c70d479..ccb4f98 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreBlobManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreBlobManager.java
@@ -19,7 +19,7 @@
package org.apache.james.mailbox.store;
-import java.io.IOException;
+import java.io.InputStream;
import java.util.Optional;
import javax.inject.Inject;
@@ -32,14 +32,13 @@ import org.apache.james.mailbox.MessageIdManager;
import org.apache.james.mailbox.exception.AttachmentNotFoundException;
import org.apache.james.mailbox.exception.BlobNotFoundException;
import org.apache.james.mailbox.exception.MailboxException;
-import org.apache.james.mailbox.model.Attachment;
import org.apache.james.mailbox.model.AttachmentId;
import org.apache.james.mailbox.model.Blob;
import org.apache.james.mailbox.model.BlobId;
import org.apache.james.mailbox.model.FetchGroupImpl;
import org.apache.james.mailbox.model.MessageId;
-import org.apache.james.mailbox.model.MessageResult;
+import com.github.fge.lambdas.Throwing;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
@@ -50,7 +49,8 @@ public class StoreBlobManager implements BlobManager {
private final MessageId.Factory messageIdFactory;
@Inject
- public StoreBlobManager(AttachmentManager attachmentManager, MessageIdManager messageIdManager, MessageId.Factory messageIdFactory) {
+ public StoreBlobManager(AttachmentManager attachmentManager, MessageIdManager messageIdManager,
+ MessageId.Factory messageIdFactory) {
this.attachmentManager = attachmentManager;
this.messageIdManager = messageIdManager;
this.messageIdFactory = messageIdFactory;
@@ -64,16 +64,14 @@ public class StoreBlobManager implements BlobManager {
@Override
public Blob retrieve(BlobId blobId, MailboxSession mailboxSession) throws MailboxException, BlobNotFoundException {
return getBlobFromAttachment(blobId, mailboxSession)
- .orElseGet(() -> getBlobFromMessage(blobId, mailboxSession)
+ .orElseGet(() -> getBlobFromMessage(blobId, mailboxSession)
.orElseThrow(() -> new BlobNotFoundException(blobId)));
}
private Optional<Blob> getBlobFromAttachment(BlobId blobId, MailboxSession mailboxSession) throws MailboxException {
try {
- Attachment attachment = attachmentManager.getAttachment(
- AttachmentId.from(blobId.asString()),
- mailboxSession);
- return Optional.of(attachment.toBlob());
+ AttachmentId attachmentId = AttachmentId.from(blobId);
+ return Optional.of(attachmentManager.getAttachment(attachmentId, mailboxSession).toBlob());
} catch (AttachmentNotFoundException e) {
return Optional.empty();
}
@@ -81,37 +79,32 @@ public class StoreBlobManager implements BlobManager {
private Optional<Blob> getBlobFromMessage(BlobId blobId, MailboxSession mailboxSession) {
return retrieveMessageId(blobId)
- .flatMap(messageId -> fromMessageId(blobId, mailboxSession, messageId));
+ .flatMap(messageId -> loadMessageAsBlob(messageId, mailboxSession))
+ .map(Throwing.function(
+ blob -> Blob.builder()
+ .id(blobId)
+ .contentType(MESSAGE_RFC822_CONTENT_TYPE)
+ .payload(IOUtils.toByteArray(blob))
+ .build()));
}
private Optional<MessageId> retrieveMessageId(BlobId blobId) {
try {
return Optional.of(messageIdFactory.fromString(blobId.asString()));
- } catch (Exception e) {
+ } catch (IllegalArgumentException e) {
return Optional.empty();
}
}
- private Optional<Blob> fromMessageId(BlobId blobId, MailboxSession mailboxSession, MessageId messageId) {
+ private Optional<InputStream> loadMessageAsBlob(MessageId messageId, MailboxSession mailboxSession) {
try {
return messageIdManager.getMessages(ImmutableList.of(messageId), FetchGroupImpl.FULL_CONTENT, mailboxSession)
.stream()
- .findFirst()
- .map(messageResult -> toBlob(blobId, messageResult));
+ .map(Throwing.function(message -> message.getFullContent().getInputStream()))
+ .findFirst();
} catch (MailboxException e) {
throw Throwables.propagate(e);
}
}
- private Blob toBlob(BlobId blobId, MessageResult messageResult) {
- try {
- return Blob.builder()
- .id(blobId)
- .contentType(MESSAGE_RFC822_CONTENT_TYPE)
- .payload(IOUtils.toByteArray(messageResult.getFullContent().getInputStream()))
- .build();
- } catch (IOException | MailboxException e) {
- throw Throwables.propagate(e);
- }
- }
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java
index 11759cb..f167a9e 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java
@@ -26,6 +26,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import javax.inject.Inject;
@@ -99,6 +100,17 @@ public class StoreMessageIdManager implements MessageIdManager {
}
@Override
+ public Set<MessageId> accessibleMessages(Collection<MessageId> messageIds, final MailboxSession mailboxSession) throws MailboxException {
+ MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession);
+ final MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession);
+ List<MailboxMessage> messageList = messageIdMapper.find(messageIds, MessageMapper.FetchType.Metadata);
+ return messageList.stream()
+ .filter(message -> mailboxBelongsToUser(mailboxSession, mailboxMapper).test(message.getMailboxId()))
+ .map(message -> message.getComposedMessageIdWithMetaData().getComposedMessageId().getMessageId())
+ .collect(Guavate.toImmutableSet());
+ }
+
+ @Override
public List<MessageResult> getMessages(List<MessageId> messageIds, final MessageResult.FetchGroup fetchGroup, final MailboxSession mailboxSession) throws MailboxException {
try {
MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession);
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/AttachmentMapperFactory.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/AttachmentMapperFactory.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/AttachmentMapperFactory.java
index e3d768d..7747067 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/AttachmentMapperFactory.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/AttachmentMapperFactory.java
@@ -19,11 +19,10 @@
package org.apache.james.mailbox.store.mail;
import org.apache.james.mailbox.MailboxSession;
-import org.apache.james.mailbox.exception.MailboxException;
public interface AttachmentMapperFactory {
- AttachmentMapper createAttachmentMapper(MailboxSession session) throws MailboxException;
+ AttachmentMapper createAttachmentMapper(MailboxSession session);
- AttachmentMapper getAttachmentMapper(MailboxSession session) throws MailboxException;
+ AttachmentMapper getAttachmentMapper(MailboxSession session);
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java
index e517e99..1e9a546 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MessageIdMapper.java
@@ -18,6 +18,7 @@
****************************************************************/
package org.apache.james.mailbox.store.mail;
+import java.util.Collection;
import java.util.List;
import java.util.Map;
@@ -34,7 +35,7 @@ import org.apache.james.mailbox.store.mail.model.MailboxMessage;
public interface MessageIdMapper {
- List<MailboxMessage> find(List<MessageId> messageIds, FetchType fetchType);
+ List<MailboxMessage> find(Collection<MessageId> messageIds, FetchType fetchType);
List<MailboxId> findMailboxes(MessageId messageId);
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerStorageTest.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerStorageTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerStorageTest.java
index 1d964cb..757b276 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerStorageTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerStorageTest.java
@@ -500,6 +500,34 @@ public abstract class AbstractMessageIdManagerStorageTest {
assertThat(messageResults.get(0).getMailboxId()).isEqualTo(mailbox2.getMailboxId());
}
+ @Test
+ public void accessibleMessagesShouldReturnMessageIdsThatBelongsToTheUser() throws Exception {
+ MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session);
+
+ assertThat(messageIdManager.accessibleMessages(ImmutableList.of(messageId), session))
+ .containsExactly(messageId);
+ }
+
+ @Test
+ public void accessibleMessagesShouldReturnEmptyWhenSuppliedMessageIdsAreEmpty() throws Exception {
+ assertThat(messageIdManager.accessibleMessages(ImmutableList.of(), session))
+ .isEmpty();
+ }
+
+ @Test
+ public void accessibleMessagesShouldFilterOutMessageIdsWhenNotExisting() throws Exception {
+ assertThat(messageIdManager.accessibleMessages(ImmutableList.of(testingData.createNotUsedMessageId()), session))
+ .isEmpty();
+ }
+
+ @Test
+ public void accessibleMessagesShouldFilterOutMessageIdsWhenNotBelongingToTheUser() throws Exception {
+ MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session);
+
+ assertThat(messageIdManager.accessibleMessages(ImmutableList.of(messageId), otherSession))
+ .isEmpty();
+ }
+
private Predicate<MessageResult> inMailbox(final MailboxId mailboxId) {
return messageResult -> messageResult.getMailboxId().equals(mailboxId);
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreAttachmentManagerTest.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreAttachmentManagerTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreAttachmentManagerTest.java
new file mode 100644
index 0000000..f205808
--- /dev/null
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreAttachmentManagerTest.java
@@ -0,0 +1,80 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.mailbox.store;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.nio.charset.StandardCharsets;
+
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
+import org.apache.james.mailbox.exception.AttachmentNotFoundException;
+import org.apache.james.mailbox.model.Attachment;
+import org.apache.james.mailbox.model.AttachmentId;
+import org.apache.james.mailbox.model.MessageId;
+import org.apache.james.mailbox.model.TestMessageId;
+import org.apache.james.mailbox.store.mail.AttachmentMapper;
+import org.apache.james.mailbox.store.mail.AttachmentMapperFactory;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+public class StoreAttachmentManagerTest {
+ private static final TestMessageId MESSAGE_ID = TestMessageId.of(1L);
+ private static final ImmutableList<MessageId> MESSAGE_IDS = ImmutableList.of(MESSAGE_ID);
+ private static final AttachmentId ATTACHMENT_ID = AttachmentId.from("1");
+ private static final Attachment ATTACHMENT = Attachment.builder()
+ .attachmentId(ATTACHMENT_ID)
+ .type("type")
+ .bytes("Any".getBytes(StandardCharsets.UTF_8))
+ .build();
+
+ private StoreAttachmentManager testee;
+ private AttachmentMapper attachmentMapper;
+ private MessageIdManager messageIdManager;
+
+ @Before
+ public void setup() throws Exception {
+ attachmentMapper = mock(AttachmentMapper.class);
+ AttachmentMapperFactory attachmentMapperFactory = mock(AttachmentMapperFactory.class);
+ when(attachmentMapperFactory.getAttachmentMapper(any(MailboxSession.class)))
+ .thenReturn(attachmentMapper);
+ messageIdManager = mock(MessageIdManager.class);
+
+ testee = new StoreAttachmentManager(attachmentMapperFactory, messageIdManager);
+ }
+
+ @Test
+ public void getAttachmentShouldThrowWhenAttachmentDoesNotBelongToUser() throws Exception {
+ MailboxSession mailboxSession = mock(MailboxSession.class);
+ when(attachmentMapper.getAttachment(ATTACHMENT_ID)).thenReturn(ATTACHMENT);
+ when(attachmentMapper.getRelatedMessageIds(ATTACHMENT_ID)).thenReturn(MESSAGE_IDS);
+ when(messageIdManager.accessibleMessages(MESSAGE_IDS, mailboxSession)).thenReturn(ImmutableSet.of());
+
+ assertThatThrownBy(() -> testee.getAttachment(ATTACHMENT_ID, mailboxSession))
+ .isInstanceOf(AttachmentNotFoundException.class);
+ }
+
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java
index a2d7b03..5a3937c 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/TestMailboxSessionMapperFactory.java
@@ -25,6 +25,7 @@ import static org.mockito.Mockito.when;
import java.util.AbstractMap;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
@@ -160,7 +161,7 @@ public class TestMailboxSessionMapperFactory extends MailboxSessionMapperFactory
messageIdMapper = new MessageIdMapper() {
@Override
- public List<MailboxMessage> find(final List<MessageId> messageIds, MessageMapper.FetchType fetchType) {
+ public List<MailboxMessage> find(final Collection<MessageId> messageIds, MessageMapper.FetchType fetchType) {
return messages.stream()
.filter(withMessageIdOneOf(messageIds))
.collect(Guavate.toImmutableList());
@@ -285,7 +286,7 @@ public class TestMailboxSessionMapperFactory extends MailboxSessionMapperFactory
messages.clear();
}
- private Predicate<MailboxMessage> withMessageIdOneOf(final List<MessageId> messageIds) {
+ private Predicate<MailboxMessage> withMessageIdOneOf(final Collection<MessageId> messageIds) {
return mailboxMessage -> messageIds.contains(mailboxMessage.getMessageId());
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java
index ac1be1f..c872e53 100644
--- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java
+++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java
@@ -340,6 +340,11 @@ public class DownloadStepdefs {
.returnResponse();
}
+ @When("^\"([^\"]*)\" delete mailbox \"([^\"]*)\"$")
+ public void deleteMailboxButNotAttachment(String username, String mailboxName) throws Exception {
+ mainStepdefs.jmapServer.getProbe(MailboxProbeImpl.class).deleteMailbox(MailboxConstants.USER_NAMESPACE, username, mailboxName);
+ }
+
@Then("^the user should be authorized$")
public void httpStatusDifferentFromUnauthorized() throws IOException {
assertThat(response.getStatusLine().getStatusCode()).isIn(200, 404);
http://git-wip-us.apache.org/repos/asf/james-project/blob/11842e0f/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature
----------------------------------------------------------------------
diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature
index b450533..2487487 100644
--- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature
+++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadGet.feature
@@ -1,6 +1,6 @@
Feature: Download GET
As a James user
- I want to retrieve my attachments
+ I want to retrieve my blobs (attachments and messages)
Background:
Given a domain named "domain.tld"
@@ -41,4 +41,24 @@ Feature: Download GET
And the user ask for messages "m1"
When "username@domain.tld" downloads the message by its blobId
Then the user should receive that blob
- And the blob size is 36
\ No newline at end of file
+ And the blob size is 36
+
+ Scenario: Deleted message should revoke attachment blob download rights
+ Given "username@domain.tld" mailbox "INBOX" contains a message "1" with an attachment "2"
+ And "username@domain.tld" delete mailbox "INBOX"
+ When "username@domain.tld" downloads "2"
+ Then the user should receive a not found response
+
+ Scenario: User cannot download attachment of another user
+ Given "username@domain.tld" mailbox "INBOX" contains a message "1" with an attachment "2"
+ And a connected user "username1@domain.tld"
+ And "username1@domain.tld" has a mailbox "INBOX"
+ When "username1@domain.tld" downloads "2"
+ Then the user should receive a not found response
+
+ Scenario: User cannot download message blob of another user
+ Given "username@domain.tld" mailbox "INBOX" contains a message "1" with an attachment "2"
+ And a connected user "username1@domain.tld"
+ And "username1@domain.tld" has a mailbox "INBOX"
+ When "username1@domain.tld" downloads "1"
+ Then the user should receive a not found response
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org