Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,33 @@
import static org.apache.james.mailbox.fixture.MailboxFixture.CEDRIC;
import static org.apache.james.mailbox.fixture.MailboxFixture.INBOX_ALICE;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
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;
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 All @@ -65,14 +72,15 @@ class StoreRightManagerTest {
MailboxSession aliceSession;
MailboxACLResolver mailboxAclResolver;
MailboxMapper mockedMailboxMapper;
EventBus eventBus;

@BeforeEach
void setup() {
aliceSession = MailboxSessionUtil.create(MailboxFixture.ALICE);
MailboxSessionMapperFactory mockedMapperFactory = mock(MailboxSessionMapperFactory.class);
mockedMailboxMapper = mock(MailboxMapper.class);
mailboxAclResolver = new UnionMailboxACLResolver();
EventBus eventBus = mock(EventBus.class);
eventBus = mock(EventBus.class);
when(mockedMapperFactory.getMailboxMapper(aliceSession))
.thenReturn(mockedMailboxMapper);

Expand Down Expand Up @@ -208,7 +216,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 +226,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 +236,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 +296,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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.nio.charset.StandardCharsets;
import java.util.stream.Stream;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand Down Expand Up @@ -273,6 +274,7 @@ default void readShouldReturnLongSavedData(BlobStore.StoragePolicy storagePolicy
assertThat(read).hasSameContentAs(new ByteArrayInputStream(ELEVEN_KILOBYTES));
}

@Disabled("Unstable test")
@ParameterizedTest
@MethodSource("storagePolicies")
default void readShouldReturnBigSavedData(BlobStore.StoragePolicy storagePolicy) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -95,6 +96,7 @@ void setUp() throws Exception {
ldapRepository = startUsersRepository(config);
}

@Disabled("Unstable test")
@Test
void listShouldReturnEmptyWhenNoEntity() throws Exception {
assertThat(ImmutableList.copyOf(ldapRepository.list()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ Feature: Mailbox creation and sharing
And "alice@domain.tld" has a mailbox "shared"
And "alice@domain.tld" shares her mailbox "shared" with "bob@domain.tld" with "aeilrwt" rights

Scenario: A sharee should not be able to update shared mailbox rights
Scenario: A sharee without administer right should not be able to update shared mailbox rights
Given "alice@domain.tld" shares her mailbox "shared" with "bob@domain.tld" with "eilrwt" rights
When "bob@domain.tld" shares "alice@domain.tld" delegated mailbox "shared" with rights "aeilrwt" with "bob@domain.tld"
Then mailbox "shared" owned by "alice@domain.tld" is not updated

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@

import org.apache.james.backends.opensearch.DockerOpenSearch;
import org.apache.james.util.docker.Images;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.extension.RegisterExtension;

@Disabled
class ES6ReporterTest extends ESReporterContract {

@RegisterExtension
Expand Down