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 bb4029d48f7888a90e02b9ea30c7cd0c7554e4d9
Author: LanKhuat <dlkh...@linagora.com>
AuthorDate: Tue Aug 18 11:54:58 2020 +0700
JAMES-3361 JMAP Draft: sharee should not be able to modify mailbox rights
---
.../apache/james/mailbox/MailboxManagerTest.java | 95 ++++++++++++++++++++++
.../james/mailbox/store/StoreRightManager.java | 38 ++++++---
.../cucumber/SetMailboxesMethodStepdefs.java | 46 ++++++++---
.../sharing/MailboxCreationAndSharing.feature | 4 +
4 files changed, 161 insertions(+), 22 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 78d528e..04a4a68 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
@@ -2722,9 +2722,15 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
@Nested
class RightTests {
+
+ private MailboxSession session2;
+
@BeforeEach
void setUp() {
+ assumeTrue(mailboxManager.hasCapability(MailboxCapabilities.ACL));
+
session = mailboxManager.createSystemSession(USER_1);
+ session2 = mailboxManager.createSystemSession(USER_2);
}
@Test
@@ -2796,5 +2802,94 @@ public abstract class MailboxManagerTest<T extends
MailboxManager> {
assertThatThrownBy(() -> mailboxManager.setRights(id,
MailboxACL.EMPTY, session))
.isInstanceOf(MailboxNotFoundException.class);
}
+
+ @Test
+ void setRightsByIdShouldThrowWhenNotOwner() throws Exception {
+ MailboxId id =
mailboxManager.createMailbox(MailboxPath.forUser(USER_2, "mailbox"),
session2).get();
+ mailboxManager.setRights(id,
MailboxACL.EMPTY.apply(MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(new MailboxACL.Rfc4314Rights(MailboxACL.Right.Lookup))
+ .asAddition()), session2);
+
+ assertThatThrownBy(() -> mailboxManager.setRights(id,
MailboxACL.EMPTY.apply(
+ MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(MailboxACL.FULL_RIGHTS)
+ .asAddition()), session))
+ .isInstanceOf(InsufficientRightsException.class);
+ }
+
+ @Test
+ void setRightsByPathShouldThrowWhenNotOwner() throws Exception {
+ MailboxPath mailboxPath = MailboxPath.forUser(USER_2, "mailbox");
+ mailboxManager.createMailbox(mailboxPath, session2).get();
+ mailboxManager.setRights(mailboxPath,
MailboxACL.EMPTY.apply(MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(new MailboxACL.Rfc4314Rights(MailboxACL.Right.Lookup))
+ .asAddition()), session2);
+
+ assertThatThrownBy(() -> mailboxManager.setRights(mailboxPath,
MailboxACL.EMPTY.apply(
+ MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(MailboxACL.FULL_RIGHTS)
+ .asAddition()), session))
+ .isInstanceOf(InsufficientRightsException.class);
+ }
+
+ @Test
+ void applyRightsCommandShouldThrowWhenNotOwner() throws Exception {
+ MailboxPath mailboxPath = MailboxPath.forUser(USER_2, "mailbox");
+ mailboxManager.createMailbox(mailboxPath, session2).get();
+ mailboxManager.setRights(mailboxPath,
MailboxACL.EMPTY.apply(MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(new MailboxACL.Rfc4314Rights(MailboxACL.Right.Lookup))
+ .asAddition()), session2);
+
+ assertThatThrownBy(() ->
mailboxManager.applyRightsCommand(mailboxPath,
+ MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(MailboxACL.FULL_RIGHTS)
+ .asAddition(), session))
+ .isInstanceOf(InsufficientRightsException.class);
+ }
+
+ @Test
+ void setRightsByIdShouldThrowWhenNoRights() throws Exception {
+ MailboxPath mailboxPath = MailboxPath.forUser(USER_2, "mailbox");
+ MailboxId mailboxId = mailboxManager.createMailbox(mailboxPath,
session2).get();
+
+ assertThatThrownBy(() -> mailboxManager.setRights(mailboxId,
MailboxACL.EMPTY.apply(
+ MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(MailboxACL.FULL_RIGHTS)
+ .asAddition()), session))
+ .isInstanceOf(MailboxNotFoundException.class);
+ }
+
+ @Test
+ void setRightsByPathShouldThrowWhenNoRights() throws Exception {
+ MailboxPath mailboxPath = MailboxPath.forUser(USER_2, "mailbox");
+ mailboxManager.createMailbox(mailboxPath, session2).get();
+
+ assertThatThrownBy(() -> mailboxManager.setRights(mailboxPath,
MailboxACL.EMPTY.apply(
+ MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(MailboxACL.FULL_RIGHTS)
+ .asAddition()), session))
+ .isInstanceOf(MailboxNotFoundException.class);
+ }
+
+ @Test
+ void applyRightsCommandShouldThrowWhenNoRights() throws Exception {
+ MailboxPath mailboxPath = MailboxPath.forUser(USER_2, "mailbox");
+ mailboxManager.createMailbox(mailboxPath, session2).get();
+
+ assertThatThrownBy(() ->
mailboxManager.applyRightsCommand(mailboxPath,
+ MailboxACL.command()
+ .key(MailboxACL.EntryKey.createUserEntryKey(USER_1))
+ .rights(MailboxACL.FULL_RIGHTS)
+ .asAddition(), session))
+ .isInstanceOf(MailboxNotFoundException.class);
+ }
}
}
diff --git
a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java
b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java
index ddb5e6d..afd7b90 100644
---
a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java
+++
b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java
@@ -38,6 +38,7 @@ import org.apache.james.mailbox.acl.MailboxACLResolver;
import org.apache.james.mailbox.events.EventBus;
import org.apache.james.mailbox.events.MailboxIdRegistrationKey;
import org.apache.james.mailbox.exception.DifferentDomainException;
+import org.apache.james.mailbox.exception.InsufficientRightsException;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.exception.MailboxNotFoundException;
import org.apache.james.mailbox.exception.UnsupportedRightException;
@@ -148,14 +149,18 @@ public class StoreRightManager implements RightManager {
assertSharesBelongsToUserDomain(mailboxPath.getUser(),
mailboxACLCommand);
MailboxMapper mapper =
mailboxSessionMapperFactory.getMailboxMapper(session);
block(mapper.findMailboxByPath(mailboxPath)
- .flatMap(mailbox -> mapper.updateACL(mailbox, mailboxACLCommand)
- .flatMap(aclDiff -> eventBus.dispatch(EventFactory.aclUpdated()
- .randomEventId()
- .mailboxSession(session)
- .mailbox(mailbox)
- .aclDiff(aclDiff)
- .build(),
- new MailboxIdRegistrationKey(mailbox.getMailboxId())))));
+ .flatMap(Throwing.<Mailbox, Mono<Void>>function(mailbox -> {
+ assertHaveAccessTo(mailbox, session);
+
+ return mapper.updateACL(mailbox, mailboxACLCommand)
+ .flatMap(aclDiff ->
eventBus.dispatch(EventFactory.aclUpdated()
+ .randomEventId()
+ .mailboxSession(session)
+ .mailbox(mailbox)
+ .aclDiff(aclDiff)
+ .build(),
+ new MailboxIdRegistrationKey(mailbox.getMailboxId())));
+ }).sneakyThrow()));
}
private void assertSharesBelongsToUserDomain(Username user, ACLCommand
mailboxACLCommand) throws DifferentDomainException {
@@ -208,10 +213,23 @@ public class StoreRightManager implements RightManager {
@Override
public void setRights(MailboxPath mailboxPath, MailboxACL mailboxACL,
MailboxSession session) throws MailboxException {
assertSharesBelongsToUserDomain(mailboxPath.getUser(),
mailboxACL.getEntries());
- MailboxMapper mapper =
mailboxSessionMapperFactory.getMailboxMapper(session);
+ MailboxMapper mapper =
mailboxSessionMapperFactory.getMailboxMapper(session);
block(mapper.findMailboxByPath(mailboxPath)
- .flatMap(mailbox -> setRights(mailboxACL, mapper, mailbox,
session)));
+ .flatMap(Throwing.<Mailbox, Mono<Void>>function(mailbox -> {
+ assertHaveAccessTo(mailbox, session);
+ return setRights(mailboxACL, mapper, mailbox, session);
+ }).sneakyThrow()));
+ }
+
+ private void assertHaveAccessTo(Mailbox mailbox, MailboxSession session)
throws InsufficientRightsException, MailboxNotFoundException {
+ if (!mailbox.generateAssociatedPath().belongsTo(session)) {
+ if
(mailbox.getACL().getEntries().containsKey(EntryKey.createUserEntryKey(session.getUser())))
{
+ throw new InsufficientRightsException("Setting ACL is only
permitted to the owner of the mailbox");
+ } else {
+ throw new MailboxNotFoundException(mailbox.getMailboxId());
+ }
+ }
}
@VisibleForTesting
diff --git
a/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/cucumber/SetMailboxesMethodStepdefs.java
b/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/cucumber/SetMailboxesMethodStepdefs.java
index 13547c0..b572d83 100644
---
a/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/cucumber/SetMailboxesMethodStepdefs.java
+++
b/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/cucumber/SetMailboxesMethodStepdefs.java
@@ -95,18 +95,40 @@ public class SetMailboxesMethodStepdefs {
String mailboxId = mainStepdefs.getMailboxId(owner,
mailboxName).serialize();
String requestBody =
- "[" +
- " [ \"setMailboxes\"," +
- " {" +
- " \"update\": {" +
- " \"" + mailboxId + "\" : {" +
- " \"sharedWith\" : { \"" + shareTo + "\" : " +
rightsAsString(rights) + " }" +
- " }" +
- " }" +
- " }," +
- " \"#0\"" +
- " ]" +
- "]";
+ "[" +
+ " [ \"setMailboxes\"," +
+ " {" +
+ " \"update\": {" +
+ " \"" + mailboxId + "\" : {" +
+ " \"sharedWith\" : { \"" + shareTo + "\" : " +
rightsAsString(rights) + " }" +
+ " }" +
+ " }" +
+ " }," +
+ " \"#0\"" +
+ " ]" +
+ "]";
+ httpClient.post(requestBody);
+ }
+
+ @Given("^\"([^\"]*)\" shares \"([^\"]*)\" delegated mailbox \"([^\"]*)\"
with rights \"([^\"]*)\" with \"([^\"]*)\"$")
+ public void shareMailboxWithRight(String connectedUser, String owner,
String mailboxName, String rights, String shareTo) throws Throwable {
+ userStepdefs.connectUser(connectedUser);
+
+ String mailboxId = mainStepdefs.getMailboxId(owner,
mailboxName).serialize();
+
+ String requestBody =
+ "[" +
+ " [ \"setMailboxes\"," +
+ " {" +
+ " \"update\": {" +
+ " \"" + mailboxId + "\" : {" +
+ " \"sharedWith\" : { \"" + shareTo + "\" : " +
rightsAsString(rights) + " }" +
+ " }" +
+ " }" +
+ " }," +
+ " \"#0\"" +
+ " ]" +
+ "]";
httpClient.post(requestBody);
}
diff --git
a/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/resources/cucumber/sharing/MailboxCreationAndSharing.feature
b/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/resources/cucumber/sharing/MailboxCreationAndSharing.feature
index 9083333..0aa4081 100644
---
a/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/resources/cucumber/sharing/MailboxCreationAndSharing.feature
+++
b/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/resources/cucumber/sharing/MailboxCreationAndSharing.feature
@@ -26,6 +26,10 @@ Feature: Mailbox creation and sharing
And "al...@domain.tld" has a mailbox "shared"
And "al...@domain.tld" shares her mailbox "shared" with "b...@domain.tld"
with "aeilrwt" rights
+ Scenario: A sharee should not be able to update shared mailbox rights
+ When "b...@domain.tld" shares "al...@domain.tld" delegated mailbox
"shared" with rights "aeilrwt" with "b...@domain.tld"
+ Then mailbox "shared" owned by "al...@domain.tld" is not updated
+
Scenario: A sharee should not be able to create a shared mailbox child
Given "b...@domain.tld" creates mailbox "sharedChild" with creationId
"c-01" in mailbox "shared" owned by "al...@domain.tld"
When "al...@domain.tld" lists mailboxes
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
