Skip to content

[ENHANCEMENT] Implement negative ACL for JMAP #3058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chibenwa wants to merge 5 commits into
base: 3.8.x
Choose a base branch
from
Open

[ENHANCEMENT] Implement negative ACL for JMAP #3058

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
29e297c
[ENHANCEMENT] Implement negative ACL for JMAP
chibenwa Jun 1, 2026
4830196
fixup! [ENHANCEMENT] Implement negative ACL for JMAP
chibenwa Jun 3, 2026
9a9aeb5
fixup! fixup! [ENHANCEMENT] Implement negative ACL for JMAP
chibenwa Jun 4, 2026
8554c1e
Restrict rights in MailboxCreationAndSharing
chibenwa Jun 10, 2026
a622695
Disable unstable test
chibenwa Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,13 @@ public void setRights(MailboxPath mailboxPath, MailboxACL mailboxACL, MailboxSes

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");
}
}
}
Expand Down Expand Up @@ -313,16 +316,19 @@ public MailboxACL getResolvedMailboxACL(Mailbox mailbox, MailboxSession mailboxS
* 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().asString());
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));
}
}
74 changes: 71 additions & 3 deletions mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,24 @@
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;
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;
Expand Down Expand Up @@ -208,7 +213,7 @@ void filteredForSessionShouldBeIdentityWhenOwner() throws UnsupportedRightExcept
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);
}
Expand All @@ -218,7 +223,7 @@ void filteredForSessionShouldBeIdentityWhenAdmin() throws UnsupportedRightExcept
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);
}
Expand All @@ -228,7 +233,7 @@ void filteredForSessionShouldContainOnlyLoggedUserWhenReadWriteAccess() throws U
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));
}
Expand Down Expand Up @@ -288,4 +293,67 @@ void applyRightsCommandShouldThrowWhenDomainsAreDifferent() {
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));
}
}