This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch 3.8.x in repository https://gitbox.apache.org/repos/asf/james-project.git
commit c0ffa4e6ab58ebdd67b5a2a4eccfc21bd1aeb99b Author: Benoit TELLIER <[email protected]> AuthorDate: Mon Jun 1 21:35:07 2026 +0200 [ENHANCEMENT] Implement negative ACL for JMAP Backport to 3.8.x: production fix (StoreRightManager) and unit tests only; the JMAP integration tests are omitted as the rfc-8621 contract files have diverged on this branch. (cherry picked from commit f800b484b51008e5de98258dae22a978a9e1ccf1) --- .../james/mailbox/store/StoreRightManager.java | 20 ++++-- .../james/mailbox/store/StoreRightManagerTest.java | 74 +++++++++++++++++++++- 2 files changed, 84 insertions(+), 10 deletions(-) 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 5c4b20b07f..1607ef7fed 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 @@ -265,10 +265,13 @@ public class StoreRightManager implements RightManager { 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 { + // Use the effective rights (positive entries minus applicable negative entries) rather + // than the raw positive ACL entry, so that a negative ACL removing Administer is honoured. + Rfc4314Rights rights = myRights(mailbox, session); + if (rights.isEmpty()) { throw new MailboxNotFoundException(mailbox.getMailboxId()); + } else if (!rights.contains(Right.Administer)) { + throw new InsufficientRightsException("Setting ACL is only permitted to the owner and admins of the mailbox"); } } } @@ -313,16 +316,19 @@ public class StoreRightManager implements RightManager { * the connected user. */ @VisibleForTesting - static MailboxACL filteredForSession(Mailbox mailbox, MailboxACL acl, MailboxSession mailboxSession) { + MailboxACL filteredForSession(Mailbox mailbox, MailboxACL acl, MailboxSession mailboxSession) throws UnsupportedRightException { if (mailbox.generateAssociatedPath().belongsTo(mailboxSession)) { return acl; } MailboxACL.EntryKey userAsKey = MailboxACL.EntryKey.createUserEntryKey(mailboxSession.getUser()); - Rfc4314Rights rights = acl.getEntries().getOrDefault(userAsKey, new Rfc4314Rights()); - if (rights.contains(MailboxACL.Right.Administer)) { + // Expose the full ACL only when the user effectively holds Administer (i.e. after applicable + // negative ACL entries have been subtracted), not merely from their raw positive entry. + Rfc4314Rights effectiveRights = aclResolver.resolveRights(mailboxSession.getUser(), acl, mailbox.getUser()); + if (effectiveRights.contains(MailboxACL.Right.Administer)) { return acl; } - return new MailboxACL(ImmutableMap.of(userAsKey, rights)); + Rfc4314Rights ownRights = acl.getEntries().getOrDefault(userAsKey, new Rfc4314Rights()); + return new MailboxACL(ImmutableMap.of(userAsKey, ownRights)); } } diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java index f9360efe97..42270860c1 100644 --- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java +++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java @@ -31,12 +31,16 @@ import static org.mockito.Mockito.when; import javax.mail.Flags; import org.apache.james.core.Username; +import org.apache.james.events.Event; import org.apache.james.events.EventBus; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MailboxSessionUtil; +import org.apache.james.mailbox.acl.ACLDiff; import org.apache.james.mailbox.acl.MailboxACLResolver; import org.apache.james.mailbox.acl.UnionMailboxACLResolver; +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; @@ -44,6 +48,7 @@ import org.apache.james.mailbox.fixture.MailboxFixture; import org.apache.james.mailbox.model.Mailbox; import org.apache.james.mailbox.model.MailboxACL; import org.apache.james.mailbox.model.MailboxACL.ACLCommand; +import org.apache.james.mailbox.model.MailboxACL.EntryKey; import org.apache.james.mailbox.model.MailboxACL.Right; import org.apache.james.mailbox.model.MailboxConstants; import org.apache.james.mailbox.model.MailboxId; @@ -208,7 +213,7 @@ class StoreRightManagerTest { MailboxACL acl = new MailboxACL() .apply(MailboxACL.command().rights(Right.Read, Right.Write).forUser(BOB).asAddition()) .apply(MailboxACL.command().rights(Right.Read, Right.Write, Right.Administer).forUser(CEDRIC).asAddition()); - MailboxACL actual = StoreRightManager.filteredForSession( + MailboxACL actual = storeRightManager.filteredForSession( new Mailbox(INBOX_ALICE, UID_VALIDITY, MAILBOX_ID), acl, aliceSession); assertThat(actual).isEqualTo(acl); } @@ -218,7 +223,7 @@ class StoreRightManagerTest { MailboxACL acl = new MailboxACL() .apply(MailboxACL.command().rights(Right.Read, Right.Write).forUser(BOB).asAddition()) .apply(MailboxACL.command().rights(Right.Read, Right.Write, Right.Administer).forUser(CEDRIC).asAddition()); - MailboxACL actual = StoreRightManager.filteredForSession( + MailboxACL actual = storeRightManager.filteredForSession( new Mailbox(INBOX_ALICE, UID_VALIDITY, MAILBOX_ID), acl, MailboxSessionUtil.create(CEDRIC)); assertThat(actual).isEqualTo(acl); } @@ -228,7 +233,7 @@ class StoreRightManagerTest { MailboxACL acl = new MailboxACL() .apply(MailboxACL.command().rights(Right.Read, Right.Write).forUser(BOB).asAddition()) .apply(MailboxACL.command().rights(Right.Read, Right.Write, Right.Administer).forUser(CEDRIC).asAddition()); - MailboxACL actual = StoreRightManager.filteredForSession( + MailboxACL actual = storeRightManager.filteredForSession( new Mailbox(INBOX_ALICE, UID_VALIDITY, MAILBOX_ID), acl, MailboxSessionUtil.create(BOB)); assertThat(actual.getEntries()).containsKey(MailboxACL.EntryKey.createUserEntryKey(BOB)); } @@ -288,4 +293,67 @@ class StoreRightManagerTest { assertThatThrownBy(() -> storeRightManager.applyRightsCommand(mailboxPath, aclCommand, aliceSession)) .isInstanceOf(DifferentDomainException.class); } + + @Test + void applyRightsCommandShouldThrowWhenAdministerIsRemovedByANegativeAclEntry() throws Exception { + // Alice holds a raw positive Administer grant, but an applicable negative ACL entry + // (as can be set through IMAP SETACL "-alice a") removes Administer from her effective rights. + MailboxPath mailboxPath = MailboxPath.forUser(BOB, "mailbox"); + Mailbox mailbox = new Mailbox(mailboxPath, UID_VALIDITY, MAILBOX_ID); + mailbox.setACL(new MailboxACL() + .apply(MailboxACL.command().forUser(ALICE).rights(Right.Lookup, Right.Administer).asAddition()) + .apply(MailboxACL.command().key(EntryKey.createUserEntryKey(ALICE, true)).rights(Right.Administer).asAddition())); + + // Sanity check: Alice's effective rights no longer contain Administer + assertThat(storeRightManager.hasRight(mailbox, Right.Administer, aliceSession)).isFalse(); + + ACLCommand aclCommand = MailboxACL.command() + .forUser(ALICE) + .rights(Right.Read) + .asAddition(); + + when(mockedMailboxMapper.findMailboxByPath(mailboxPath)).thenReturn(Mono.just(mailbox)); + + assertThatThrownBy(() -> storeRightManager.applyRightsCommand(mailboxPath, aclCommand, aliceSession)) + .isInstanceOf(InsufficientRightsException.class); + } + + @Test + void applyRightsCommandShouldNotThrowWhenEffectiveAdministerIsHeld() throws Exception { + // Baseline counterpart: with a positive Administer grant and no negating entry, + // the mutation is legitimately authorized. + MailboxPath mailboxPath = MailboxPath.forUser(BOB, "mailbox"); + Mailbox mailbox = new Mailbox(mailboxPath, UID_VALIDITY, MAILBOX_ID); + mailbox.setACL(new MailboxACL(new MailboxACL.Entry(ALICE.asString(), Right.Lookup, Right.Administer))); + + assertThat(storeRightManager.hasRight(mailbox, Right.Administer, aliceSession)).isTrue(); + + ACLCommand aclCommand = MailboxACL.command() + .forUser(ALICE) + .rights(Right.Read) + .asAddition(); + + when(mockedMailboxMapper.findMailboxByPath(mailboxPath)).thenReturn(Mono.just(mailbox)); + when(mockedMailboxMapper.updateACL(mailbox, aclCommand)).thenReturn(Mono.just(ACLDiff.computeDiff(MailboxACL.EMPTY, MailboxACL.EMPTY))); + when(eventBus.dispatch(any(Event.class), any(MailboxIdRegistrationKey.class))).thenReturn(Mono.empty()); + + assertThatCode(() -> storeRightManager.applyRightsCommand(mailboxPath, aclCommand, aliceSession)) + .doesNotThrowAnyException(); + } + + @Test + void filteredForSessionShouldHideOtherEntriesWhenAdministerIsRemovedByANegativeAclEntry() throws UnsupportedRightException { + // BOB holds a raw positive Administer grant negated by an applicable negative ACL entry: + // his effective Administer right is false, so he must not be granted full-ACL visibility. + MailboxACL acl = new MailboxACL() + .apply(MailboxACL.command().forUser(BOB).rights(Right.Lookup, Right.Administer).asAddition()) + .apply(MailboxACL.command().key(EntryKey.createUserEntryKey(BOB, true)).rights(Right.Administer).asAddition()) + .apply(MailboxACL.command().rights(Right.Read, Right.Write, Right.Administer).forUser(CEDRIC).asAddition()); + + MailboxACL actual = storeRightManager.filteredForSession( + new Mailbox(INBOX_ALICE, UID_VALIDITY, MAILBOX_ID), acl, MailboxSessionUtil.create(BOB)); + + assertThat(actual.getEntries()) + .doesNotContainKey(EntryKey.createUserEntryKey(CEDRIC)); + } } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
