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 577cbe24499d77a9f6dc070347aa133195636665 Author: Benoit TELLIER <btell...@linagora.com> AuthorDate: Thu Oct 17 10:12:59 2024 +0200 JAMES-2182 Fix rights for CREATE --- .../apache/james/mailbox/MailboxManagerTest.java | 76 ++++++++++++++++++++++ .../james/mailbox/store/StoreMailboxManager.java | 31 +++++++-- .../james/mailbox/store/StoreRightManager.java | 14 ++++ .../apache/james/mpt/host/ExternalHostSystem.java | 5 ++ .../james/imap/scripts/SharingAccessLRK.test | 6 +- 5 files changed, 120 insertions(+), 12 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 528dea7b8c..8384acd1eb 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 @@ -271,6 +271,82 @@ public abstract class MailboxManagerTest<T extends MailboxManager> { assertThat(mailboxId.get()).isEqualTo(retrievedMailbox.getId()); } + @Test + void shareeShouldBeAbleToCreateMailbox() throws Exception { + // GIVEN USER1 shared his INBOX + session = mailboxManager.createSystemSession(USER_1); + MailboxPath mailboxPath = MailboxPath.inbox(session); + mailboxManager.createMailbox(mailboxPath, session); + mailboxManager.applyRightsCommand(mailboxPath, + MailboxACL.command() + .key(MailboxACL.EntryKey.createUserEntryKey(USER_2)) + .rights(MailboxACL.Rfc4314Rights.of(ImmutableList.of(MailboxACL.Right.Lookup, + MailboxACL.Right.Read, MailboxACL.Right.CreateMailbox))) + .asAddition(), session); + + // When USER_2 create a mailbox child + MailboxSession session2 = mailboxManager.createSystemSession(USER_2); + MailboxPath childPath = MailboxPath.inbox(session).child("child", session2.getPathDelimiter()); + mailboxManager.createMailbox(childPath, session2); + + // Then child path inherit rights + assertThat(mailboxManager.getMailbox(childPath, session) + .getMailboxEntity().getACL().getEntries().get(MailboxACL.EntryKey.createUserEntryKey(USER_2))) + .isEqualTo(MailboxACL.Rfc4314Rights.fromSerializedRfc4314Rights("lrk")); + } + + @Test + void shareeShouldBeAbleToCreateMailboxChildren() throws Exception { + // GIVEN USER1 shared his INBOX + session = mailboxManager.createSystemSession(USER_1); + MailboxPath mailboxPath = MailboxPath.inbox(session); + mailboxManager.createMailbox(mailboxPath, session); + mailboxManager.applyRightsCommand(mailboxPath, + MailboxACL.command() + .key(MailboxACL.EntryKey.createUserEntryKey(USER_2)) + .rights(MailboxACL.Rfc4314Rights.of(ImmutableList.of(MailboxACL.Right.Lookup, + MailboxACL.Right.Read, MailboxACL.Right.CreateMailbox))) + .asAddition(), session); + + // When USER_2 create a mailbox child + MailboxSession session2 = mailboxManager.createSystemSession(USER_2); + MailboxPath childPath = MailboxPath.inbox(session) + .child("child", session2.getPathDelimiter()) + .child("anotherkid", session2.getPathDelimiter()); + mailboxManager.createMailbox(childPath, session2); + + // Then child path inherit rights + assertThat(mailboxManager.getMailbox(childPath, session) + .getMailboxEntity().getACL().getEntries().get(MailboxACL.EntryKey.createUserEntryKey(USER_2))) + .isEqualTo(MailboxACL.Rfc4314Rights.fromSerializedRfc4314Rights("lrk")); + } + + @Test + void shareeShouldBeAbleToCreateMailboxChildrenIntermediatePaths() throws Exception { + // GIVEN USER1 shared his INBOX + session = mailboxManager.createSystemSession(USER_1); + MailboxPath mailboxPath = MailboxPath.inbox(session); + mailboxManager.createMailbox(mailboxPath, session); + mailboxManager.applyRightsCommand(mailboxPath, + MailboxACL.command() + .key(MailboxACL.EntryKey.createUserEntryKey(USER_2)) + .rights(MailboxACL.Rfc4314Rights.of(ImmutableList.of(MailboxACL.Right.Lookup, + MailboxACL.Right.Read, MailboxACL.Right.CreateMailbox))) + .asAddition(), session); + + // When USER_2 create a mailbox child + MailboxSession session2 = mailboxManager.createSystemSession(USER_2); + MailboxPath intermediatePath = MailboxPath.inbox(session) + .child("child", session2.getPathDelimiter()); + MailboxPath childPath = intermediatePath.child("anotherkid", session2.getPathDelimiter()); + mailboxManager.createMailbox(childPath, session2); + + // Then child path inherit rights + assertThat(mailboxManager.getMailbox(intermediatePath, session) + .getMailboxEntity().getACL().getEntries().get(MailboxACL.EntryKey.createUserEntryKey(USER_2))) + .isEqualTo(MailboxACL.Rfc4314Rights.fromSerializedRfc4314Rights("lrk")); + } + @Test void creatingMixedCaseINBOXShouldCreateItAsINBOX() throws Exception { session = mailboxManager.createSystemSession(USER_1); 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 37e68ce6bb..5e7d20f951 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 @@ -339,7 +339,7 @@ public class StoreMailboxManager implements MailboxManager { public Mono<MailboxId> createMailboxReactive(MailboxPath mailboxPath, CreateOption createOption, MailboxSession mailboxSession) { LOGGER.debug("createMailbox {}", mailboxPath); - return assertMailboxPathBelongToUserReactive(mailboxSession, mailboxPath) + return assertCanCreateReactive(mailboxSession, mailboxPath) .then(doCreateMailboxReactive(mailboxPath, createOption, mailboxSession)); } @@ -429,15 +429,32 @@ public class StoreMailboxManager implements MailboxManager { LOGGER.info("{} mailbox was created concurrently", mailboxPath.asString()); return Mono.empty(); }) - .flatMap(any -> createSubscriptionIfNeeded(mailboxPath, createOption, mailboxSession).thenReturn(any)); + .flatMap(any -> createSubscriptionIfNeeded(mailboxPath, createOption, mailboxSession).thenReturn(any)) + .flatMap(any -> inheritRightsReactive(mailboxSession, mailboxPath).thenReturn(any)); } - private Mono<Void> assertMailboxPathBelongToUserReactive(MailboxSession mailboxSession, MailboxPath mailboxPath) { - if (!mailboxPath.belongsTo(mailboxSession)) { - return Mono.error(new InsufficientRightsException("mailboxPath '" + mailboxPath.asString() + "'" - + " does not belong to user '" + mailboxSession.getUser().asString() + "'")); + + private Mono<Void> assertCanCreateReactive(MailboxSession session, MailboxPath path) { + if (path.belongsTo(session)) { + return Mono.empty(); } - return Mono.empty(); + + return nearestExistingParent(session, path) + .filterWhen(parent -> hasRightReactive(parent, Right.CreateMailbox, session)) + .switchIfEmpty(Mono.error(new InsufficientRightsException("user '" + session.getUser().asString() + "' is not allowed to create the mailbox '" + path.asString() + "'"))) + .then(); + } + + private Mono<MailboxPath> nearestExistingParent(MailboxSession session, MailboxPath path) { + return Flux.fromIterable(path.getParents(session.getPathDelimiter()).reversed()) + .filterWhen(parent -> mailboxExists(parent, session)) + .next(); + } + + private Mono<Void> inheritRightsReactive(MailboxSession mailboxSession, MailboxPath path) { + return nearestExistingParent(mailboxSession, path) + .flatMap(parent -> Mono.from(listRightsReactive(parent, mailboxSession))) + .flatMap(acl -> storeRightManager.setRightsReactiveWithoutAccessControl(path, acl, mailboxSession)); } @Override 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 47faddf9a0..5eab78dc2f 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 @@ -335,4 +335,18 @@ public class StoreRightManager implements RightManager { } return new MailboxACL(ImmutableMap.of(userAsKey, rights)); } + + /** Sets an ACL for a mailbox *WITHOUT* checking if the user of current session is allowed to do so. + * We need this when creating a mailbox, to copy the ACL of the parent mailbox for all users. + */ + public Mono<Void> setRightsReactiveWithoutAccessControl(MailboxPath mailboxPath, MailboxACL mailboxACL, MailboxSession session) { + try { + assertSharesBelongsToUserDomain(mailboxPath.getUser(), mailboxACL.getEntries()); + } catch (DifferentDomainException e) { + return Mono.error(e); + } + MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session); + return mapper.findMailboxByPath(mailboxPath) + .flatMap(Throwing.<Mailbox, Mono<Void>>function(mailbox -> setRights(mailboxACL, mapper, mailbox, session)).sneakyThrow()); + } } diff --git a/mpt/core/src/main/java/org/apache/james/mpt/host/ExternalHostSystem.java b/mpt/core/src/main/java/org/apache/james/mpt/host/ExternalHostSystem.java index dbc1b13385..ac45ae29e1 100644 --- a/mpt/core/src/main/java/org/apache/james/mpt/host/ExternalHostSystem.java +++ b/mpt/core/src/main/java/org/apache/james/mpt/host/ExternalHostSystem.java @@ -126,4 +126,9 @@ public class ExternalHostSystem extends ExternalSessionFactory implements ImapHo public void grantRights(MailboxPath mailboxPath, Username userName, MailboxACL.Rfc4314Rights rights) throws Exception { throw new NotImplementedException("Not implemented"); } + + @Override + public void fillMailbox(MailboxPath mailboxPath) throws Exception { + throw new NotImplementedException("Not implemented"); + } } diff --git a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRK.test b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRK.test index 34a583572e..57da76bf11 100644 --- a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRK.test +++ b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/SharingAccessLRK.test @@ -65,12 +65,8 @@ S: a5 NO DELETE failed. No such mailbox. C: a5 SETACL #user.boby.mailbox-lrk imapuser lra S: a5 NO SETACL You need the Administer right to perform command SETACL on mailbox #user.boby.mailbox-lrk. -# TODO -# - MUST be able to create mailboxes -# - When a new mailbox is created, it SHOULD inherit the ACL from the parent mailbox (if one exists) in the defined hierarchy. -# CF https://github.com/giz-berlin/james-project/commit/9541be2da2a2da9a5bc5343e20ef152c5b473727#diff-78bbaf3505920ad8d1c2dfb95f2cbc4de2ed01c358645ad50e9b531483e3c25aR342 C: a7 CREATE #user.boby.mailbox-lrk.evev -S: a7 NO CREATE processing failed. +S: a7 OK \[MAILBOXID \(.*\)\] CREATE completed. C: a3 SELECT #user.boby.mailbox-lrk S: \* OK \[MAILBOXID \(.*\)\] Ok --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org