JAMES-2188 A user can't set a message in a mailbox from someone else.
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/bb1eeec8 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/bb1eeec8 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/bb1eeec8 Branch: refs/heads/master Commit: bb1eeec860dfadc0663939836c662f2f2865e598 Parents: 6ff67ca Author: Antoine Duprat <adup...@linagora.com> Authored: Mon Oct 16 14:46:40 2017 +0200 Committer: benwa <btell...@linagora.com> Committed: Wed Oct 18 09:01:56 2017 +0700 ---------------------------------------------------------------------- .../jmap/methods/MailboxRightsException.java | 29 +++++++ .../methods/SetMessagesCreationProcessor.java | 41 ++++++++- .../SetMessagesCreationProcessorTest.java | 87 +++++++++++++++++++- 3 files changed, 151 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/bb1eeec8/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MailboxRightsException.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MailboxRightsException.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MailboxRightsException.java new file mode 100644 index 0000000..f47e7d0 --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MailboxRightsException.java @@ -0,0 +1,29 @@ +/**************************************************************** + * 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.jmap.methods; + +import org.apache.james.mailbox.exception.MailboxException; + +public class MailboxRightsException extends MailboxException { + + public MailboxRightsException() { + super(); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/bb1eeec8/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 3087e71..8c482e3 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 @@ -57,6 +57,7 @@ import org.apache.james.jmap.send.MailSpool; import org.apache.james.jmap.utils.SystemMailboxesProvider; import org.apache.james.lifecycle.api.LifecycleUtil; import org.apache.james.mailbox.AttachmentManager; +import org.apache.james.mailbox.MailboxManager; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.exception.AttachmentNotFoundException; @@ -65,6 +66,7 @@ import org.apache.james.mailbox.exception.MailboxNotFoundException; import org.apache.james.mailbox.model.AttachmentId; import org.apache.james.mailbox.model.Cid; import org.apache.james.mailbox.model.ComposedMessageId; +import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.model.MessageAttachment; import org.apache.james.metrics.api.MetricFactory; import org.apache.james.metrics.api.TimeMetric; @@ -93,6 +95,8 @@ public class SetMessagesCreationProcessor implements SetMessagesProcessor { private final SystemMailboxesProvider systemMailboxesProvider; private final AttachmentManager attachmentManager; private final MetricFactory metricFactory; + private final MailboxManager mailboxManager; + private final MailboxId.Factory mailboxIdFactory; @VisibleForTesting @Inject SetMessagesCreationProcessor(MIMEMessageConverter mimeMessageConverter, @@ -100,7 +104,10 @@ public class SetMessagesCreationProcessor implements SetMessagesProcessor { MailFactory mailFactory, MessageFactory messageFactory, SystemMailboxesProvider systemMailboxesProvider, - AttachmentManager attachmentManager, MetricFactory metricFactory) { + AttachmentManager attachmentManager, + MetricFactory metricFactory, + MailboxManager mailboxManager, + MailboxId.Factory mailboxIdFactory) { this.mimeMessageConverter = mimeMessageConverter; this.mailSpool = mailSpool; this.mailFactory = mailFactory; @@ -108,6 +115,8 @@ public class SetMessagesCreationProcessor implements SetMessagesProcessor { this.systemMailboxesProvider = systemMailboxesProvider; this.attachmentManager = attachmentManager; this.metricFactory = metricFactory; + this.mailboxManager = mailboxManager; + this.mailboxIdFactory = mailboxIdFactory; } @Override @@ -167,6 +176,14 @@ public class SetMessagesCreationProcessor implements SetMessagesProcessor { .description(e.getMessage()) .build()); + } catch (MailboxRightsException e) { + LOG.error("Appending message in an unknown mailbox", e); + responseBuilder.notCreated(create.getCreationId(), + SetError.builder() + .type("error") + .description("MailboxId invalid") + .build()); + } catch (MailboxException | MessagingException e) { LOG.error("Unexpected error while creating message", e); responseBuilder.notCreated(create.getCreationId(), @@ -223,7 +240,12 @@ public class SetMessagesCreationProcessor implements SetMessagesProcessor { return AttachmentId.from(attachment.getBlobId().getRawValue()); } - private void validateRights(CreationMessageEntry entry, MailboxSession session) throws MailboxSendingNotAllowedException { + private void validateRights(CreationMessageEntry entry, MailboxSession session) throws MailboxSendingNotAllowedException, MailboxRightsException { + validateUserIsInSenders(entry, session); + validateIsUserOwnerOfMailboxes(entry, session); + } + + private void validateUserIsInSenders(CreationMessageEntry entry, MailboxSession session) throws MailboxSendingNotAllowedException { List<String> allowedSenders = ImmutableList.of(session.getUser().getUserName()); if (!isAllowedFromAddress(entry.getValue(), allowedSenders)) { throw new MailboxSendingNotAllowedException(allowedSenders); @@ -238,7 +260,20 @@ public class SetMessagesCreationProcessor implements SetMessagesProcessor { .orElse(false); } - + @VisibleForTesting void validateIsUserOwnerOfMailboxes(CreationMessageEntry entry, MailboxSession session) throws MailboxRightsException { + if (containsMailboxNotOwn(entry.getValue().getMailboxIds(), session)) { + throw new MailboxRightsException(); + } + } + + private boolean containsMailboxNotOwn(List<String> mailboxIds, MailboxSession session) { + return mailboxIds.stream() + .map(mailboxIdFactory::fromString) + .map(Throwing.function(mailboxId -> mailboxManager.getMailbox(mailboxId, session))) + .map(Throwing.function(MessageManager::getMailboxPath)) + .anyMatch(path -> !session.getUser().isSameUser(path.getUser())); + } + private MessageWithId handleOutboxMessages(CreationMessageEntry entry, MailboxSession session) throws MailboxException, MessagingException { MessageManager outbox = getMailboxWithRole(session, Role.OUTBOX).orElseThrow(() -> new MailboxNotFoundException(Role.OUTBOX.serialize())); if (!isRequestForSending(entry.getValue(), session)) { http://git-wip-us.apache.org/repos/asf/james-project/blob/bb1eeec8/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java index c1b9f00..8618b11 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java @@ -56,15 +56,19 @@ import org.apache.james.jmap.utils.HtmlTextExtractor; import org.apache.james.jmap.utils.SystemMailboxesProvider; import org.apache.james.mailbox.AttachmentManager; import org.apache.james.mailbox.BlobManager; +import org.apache.james.mailbox.MailboxManager; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.MessageUid; import org.apache.james.mailbox.exception.AttachmentNotFoundException; import org.apache.james.mailbox.exception.MailboxException; +import org.apache.james.mailbox.exception.MailboxNotFoundException; import org.apache.james.mailbox.inmemory.InMemoryId; import org.apache.james.mailbox.mock.MockMailboxSession; import org.apache.james.mailbox.model.AttachmentId; import org.apache.james.mailbox.model.ComposedMessageId; +import org.apache.james.mailbox.model.MailboxId; +import org.apache.james.mailbox.model.MailboxId.Factory; import org.apache.james.mailbox.model.MailboxPath; import org.apache.james.mailbox.model.MessageId; import org.apache.james.mailbox.model.TestMessageId; @@ -113,6 +117,8 @@ public class SetMessagesCreationProcessorTest { private MockMailboxSession session; private MIMEMessageConverter mimeMessageConverter; private AttachmentManager mockedAttachmentManager; + private MailboxManager mockedMailboxManager; + private Factory mockedMailboxIdFactory; private SetMessagesCreationProcessor sut; private MessageManager outbox; private MessageManager drafts; @@ -121,6 +127,7 @@ public class SetMessagesCreationProcessorTest { @Rule public ExpectedException expectedException = ExpectedException.none(); + @Before public void setUp() throws MailboxException { HtmlTextExtractor htmlTextExtractor = mock(HtmlTextExtractor.class); @@ -133,13 +140,20 @@ public class SetMessagesCreationProcessorTest { mockedMailSpool = mock(MailSpool.class); mockedMailFactory = mock(MailFactory.class); mockedAttachmentManager = mock(AttachmentManager.class); + mockedMailboxManager = mock(MailboxManager.class); + mockedMailboxIdFactory = mock(MailboxId.Factory.class); fakeSystemMailboxesProvider = new TestSystemMailboxesProvider(() -> optionalOutbox, () -> optionalDrafts); session = new MockMailboxSession(USER); mimeMessageConverter = new MIMEMessageConverter(); - sut = new SetMessagesCreationProcessor(mimeMessageConverter, mockedMailSpool, mockedMailFactory, messageFactory, fakeSystemMailboxesProvider, mockedAttachmentManager, new NoopMetricFactory()); + sut = new SetMessagesCreationProcessor(mimeMessageConverter, mockedMailSpool, mockedMailFactory, messageFactory, fakeSystemMailboxesProvider, mockedAttachmentManager, new NoopMetricFactory(), mockedMailboxManager, mockedMailboxIdFactory); outbox = mock(MessageManager.class); + when(mockedMailboxIdFactory.fromString(OUTBOX_ID.serialize())) + .thenReturn(OUTBOX_ID); + when(mockedMailboxManager.getMailbox(OUTBOX_ID, session)) + .thenReturn(outbox); + when(outbox.getId()).thenReturn(OUTBOX_ID); when(outbox.getMailboxPath()).thenReturn(MailboxPath.forUser(USER, OUTBOX)); @@ -216,7 +230,7 @@ public class SetMessagesCreationProcessorTest { @Test public void processShouldReturnNonEmptyCreatedWhenRequestHasNonEmptyCreate() throws MailboxException { // Given - sut = new SetMessagesCreationProcessor(mimeMessageConverter, mockedMailSpool, mockedMailFactory, messageFactory, fakeSystemMailboxesProvider, mockedAttachmentManager, new NoopMetricFactory()); + sut = new SetMessagesCreationProcessor(mimeMessageConverter, mockedMailSpool, mockedMailFactory, messageFactory, fakeSystemMailboxesProvider, mockedAttachmentManager, new NoopMetricFactory(), mockedMailboxManager, mockedMailboxIdFactory); // When SetMessagesResponse result = sut.process(createMessageInOutbox, session); @@ -231,7 +245,7 @@ public class SetMessagesCreationProcessorTest { public void processShouldReturnErrorWhenOutboxNotFound() { // Given TestSystemMailboxesProvider doNotProvideOutbox = new TestSystemMailboxesProvider(Optional::empty, () -> optionalDrafts); - SetMessagesCreationProcessor sut = new SetMessagesCreationProcessor(mimeMessageConverter, mockedMailSpool, mockedMailFactory, messageFactory, doNotProvideOutbox, mockedAttachmentManager, new NoopMetricFactory()); + SetMessagesCreationProcessor sut = new SetMessagesCreationProcessor(mimeMessageConverter, mockedMailSpool, mockedMailFactory, messageFactory, doNotProvideOutbox, mockedAttachmentManager, new NoopMetricFactory(), mockedMailboxManager, mockedMailboxIdFactory); // When SetMessagesResponse actual = sut.process(createMessageInOutbox, session); @@ -347,6 +361,73 @@ public class SetMessagesCreationProcessorTest { .isInstanceOf(AttachmentsNotFoundException.class) .matches(e -> ((AttachmentsNotFoundException)e).getAttachmentIds().containsAll(ImmutableSet.of(unknownBlobId1, unknownBlobId2))); } + + @Test + public void validateIsUserOwnerOfMailboxesShouldThrowWhenMailboxIdDoesntExist() throws Exception { + InMemoryId mailboxId = InMemoryId.of(6789); + when(mockedMailboxManager.getMailbox(mailboxId, session)) + .thenThrow(new MailboxNotFoundException(mailboxId)); + when(mockedMailboxIdFactory.fromString(mailboxId.serialize())) + .thenReturn(mailboxId); + CreationMessageId creationMessageId = CreationMessageId.of("anything-really"); + CreationMessageEntry entry = new CreationMessageEntry(creationMessageId, creationMessageBuilder.mailboxId(mailboxId.serialize()).build()); + + assertThatThrownBy(() -> sut.validateIsUserOwnerOfMailboxes(entry, session)); + } + + @Test + public void validateIsUserOwnerOfMailboxesShouldThrowWhenRetrievingMailboxPathFails() throws Exception { + CreationMessageId creationMessageId = CreationMessageId.of("anything-really"); + InMemoryId mailboxId = InMemoryId.of(6789); + MessageManager mailbox = mock(MessageManager.class); + when(mockedMailboxManager.getMailbox(mailboxId, session)) + .thenReturn(mailbox); + when(mockedMailboxIdFactory.fromString(mailboxId.serialize())) + .thenReturn(mailboxId); + when(mailbox.getMailboxPath()) + .thenThrow(new MailboxException()); + + CreationMessageEntry entry = new CreationMessageEntry(creationMessageId, creationMessageBuilder.mailboxId(mailboxId.serialize()).build()); + + assertThatThrownBy(() -> sut.validateIsUserOwnerOfMailboxes(entry, session)); + } + + @Test + public void validateIsUserOwnerOfMailboxesShouldThrowWhenUserIsNotTheOwnerOfTheMailbox() throws Exception { + CreationMessageId creationMessageId = CreationMessageId.of("anything-really"); + InMemoryId mailboxId = InMemoryId.of(6789); + MessageManager mailbox = mock(MessageManager.class); + when(mockedMailboxManager.getMailbox(mailboxId, session)) + .thenReturn(mailbox); + when(mockedMailboxIdFactory.fromString(mailboxId.serialize())) + .thenReturn(mailboxId); + when(mailbox.getMailboxPath()) + .thenReturn(MailboxPath.forUser("otheru...@example.com", mailboxId.serialize())); + + + CreationMessageEntry entry = new CreationMessageEntry(creationMessageId, creationMessageBuilder.mailboxId(mailboxId.serialize()).build()); + + assertThatThrownBy(() -> sut.validateIsUserOwnerOfMailboxes(entry, session)) + .isInstanceOf(MailboxRightsException.class); + } + + @Test + public void validateIsUserOwnerOfMailboxesShouldNotThrowWhenUserIsTheOwnerOfTheMailbox() throws Exception { + CreationMessageId creationMessageId = CreationMessageId.of("anything-really"); + InMemoryId mailboxId = InMemoryId.of(6789); + MessageManager mailbox = mock(MessageManager.class); + when(mockedMailboxManager.getMailbox(mailboxId, session)) + .thenReturn(mailbox); + when(mockedMailboxIdFactory.fromString(mailboxId.serialize())) + .thenReturn(mailboxId); + when(mailbox.getMailboxPath()) + .thenReturn(MailboxPath.forUser(USER, mailboxId.serialize())); + + + CreationMessageEntry entry = new CreationMessageEntry(creationMessageId, creationMessageBuilder.mailboxId(mailboxId.serialize()).build()); + + sut.validateIsUserOwnerOfMailboxes(entry, session); + } public static class TestSystemMailboxesProvider implements SystemMailboxesProvider { --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
