[james-project] 03/05: JAMES-2993 getMailbox by path should assert that user has the rights to access the mailbox before returning it

[rcordier]

This is an automated email from the ASF dual-hosted git repository.

rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit c49320836b0da126a3f629f5fcc5f5e9bc2e0ddc
Author: Rene Cordier <rcord...@linagora.com>
AuthorDate: Mon Jan 13 14:31:26 2020 +0700

    JAMES-2993 getMailbox by path should assert that user has the rights to 
access the mailbox before returning it
---
 .../java/org/apache/james/mailbox/MailboxManagerTest.java   |  5 -----
 .../org/apache/james/mailbox/store/StoreMailboxManager.java | 13 +++++++++----
 2 files changed, 9 insertions(+), 9 deletions(-)

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 55d00f4..8ef3425 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
@@ -83,7 +83,6 @@ import org.apache.james.util.concurrency.ConcurrentTestRunner;
 import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
@@ -2027,8 +2026,6 @@ public abstract class MailboxManagerTest<T extends 
MailboxManager> {
         }
 
         @Test
-        @Disabled("JAMES-2993 It seems that getMailbox by path in 
MailboxManager does not check that the mailbox belongs to user, "
-                + "compared to getMailbox by id that actually does assert it")
         void 
copyMessagesShouldThrowWhenCopyingMessageFromMailboxNotBelongingToSameUser() 
throws Exception {
             MailboxSession sessionUser1 = 
mailboxManager.createSystemSession(USER_1);
             MailboxSession sessionUser2 = 
mailboxManager.createSystemSession(USER_2);
@@ -2117,8 +2114,6 @@ public abstract class MailboxManagerTest<T extends 
MailboxManager> {
         }
 
         @Test
-        @Disabled("JAMES-2993 It seems that getMailbox by path in 
MailboxManager does not check that the mailbox belongs to user, "
-            + "compared to getMailbox by id that actually does assert it")
         void getMailboxByPathShouldThrowWhenMailboxNotBelongingToUser() throws 
Exception {
             MailboxSession sessionUser1 = 
mailboxManager.createSystemSession(USER_1);
             MailboxSession sessionUser2 = 
mailboxManager.createSystemSession(USER_2);
diff --git 
a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
 
b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
index e9668c9..6d05e4e 100644
--- 
a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
+++ 
b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
@@ -273,11 +273,16 @@ public class StoreMailboxManager implements 
MailboxManager {
             LOGGER.info("Mailbox '{}' not found.", mailboxPath);
             throw new MailboxNotFoundException(mailboxPath);
 
-        } else {
-            LOGGER.debug("Loaded mailbox {}", mailboxPath);
+        }
 
-            return createMessageManager(mailboxRow, session);
+        if (!assertUserHasAccessTo(mailboxRow, session)) {
+            LOGGER.info("Mailbox '{}' does not belong to user '{}' but to 
'{}'", mailboxPath, session.getUser(), mailboxRow.getUser());
+            throw new MailboxNotFoundException(mailboxPath);
         }
+
+        LOGGER.debug("Loaded mailbox {}", mailboxPath);
+
+        return createMessageManager(mailboxRow, session);
     }
 
     @Override
@@ -291,7 +296,7 @@ public class StoreMailboxManager implements MailboxManager {
             throw new MailboxNotFoundException(mailboxId);
         }
 
-        if (! assertUserHasAccessTo(mailboxRow, session)) {
+        if (!assertUserHasAccessTo(mailboxRow, session)) {
             LOGGER.info("Mailbox '{}' does not belong to user '{}' but to 
'{}'", mailboxId.serialize(), session.getUser(), mailboxRow.getUser());
             throw new MailboxNotFoundException(mailboxId);
         }


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