Skip to content

Commit

Permalink
James-4097 Allow disabling same-domain requirement when assigning rig…
Browse files Browse the repository at this point in the history
…hts (#2573)
  • Loading branch information
j-hellenberg authored Dec 23, 2024
1 parent f6eb81f commit 4c4d6d3
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 20 deletions.
15 changes: 14 additions & 1 deletion docs/modules/servers/partials/configure/jvm.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,17 @@ james.deduplicating.blobstore.thread.switch.threshold=32768
# Count of octet from which streams are buffered to files and not to memory
james.deduplicating.blobstore.file.threshold=10240
----
----

== Allow users to have rights for shares of different domain

Typically, preventing users to obtain rights for shares of another domain is a useful security layer.
However, in multi-tenancy deployments, this can be useful (for example, students might be given access to a shared mailbox
residing under the @university.edu domain with their @student.university.edu address).

Optional. Boolean. Defaults to false.

Ex in `jvm.properties`
----
james.rights.crossdomain.allow=false
----
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@
import reactor.core.publisher.Mono;

public class StoreRightManager implements RightManager {
@VisibleForTesting
static Boolean IS_CROSS_DOMAIN_ACCESS_ALLOWED = Boolean.parseBoolean(System.getProperty("james.rights.crossdomain.allow", "false"));

private final EventBus eventBus;
private final MailboxSessionMapperFactory mailboxSessionMapperFactory;
private final MailboxACLResolver aclResolver;
Expand Down Expand Up @@ -185,7 +188,7 @@ public void applyRightsCommand(MailboxPath mailboxPath, ACLCommand mailboxACLCom
@Override
public Mono<Void> applyRightsCommandReactive(MailboxPath mailboxPath, ACLCommand mailboxACLCommand, MailboxSession session) {
return Mono.just(mailboxSessionMapperFactory.getMailboxMapper(session))
.doOnNext(Throwing.consumer(mapper -> assertSharesBelongsToUserDomain(mailboxPath.getUser(), mailboxACLCommand)))
.doOnNext(Throwing.consumer(mapper -> assertUserHasAccessToShareeDomains(mailboxPath.getUser(), mailboxACLCommand)))
.flatMap(mapper -> mapper.findMailboxByPath(mailboxPath)
.doOnNext(Throwing.consumer(mailbox -> assertHaveAccessTo(mailbox, session)))
.flatMap(mailbox -> mapper.updateACL(mailbox, mailboxACLCommand)
Expand All @@ -211,8 +214,8 @@ public void applyRightsCommand(MailboxId mailboxId, ACLCommand mailboxACLCommand
applyRightsCommand(mailbox.generateAssociatedPath(), mailboxACLCommand, session);
}

private void assertSharesBelongsToUserDomain(Username user, ACLCommand mailboxACLCommand) throws DifferentDomainException {
assertSharesBelongsToUserDomain(user, ImmutableMap.of(mailboxACLCommand.getEntryKey(), mailboxACLCommand.getRights()));
private void assertUserHasAccessToShareeDomains(Username user, ACLCommand mailboxACLCommand) throws DifferentDomainException {
assertUserHasAccessToShareeDomains(user, ImmutableMap.of(mailboxACLCommand.getEntryKey(), mailboxACLCommand.getRights()));
}

public boolean isReadWrite(MailboxSession session, Mailbox mailbox, Flags sharedPermanentFlags) {
Expand Down Expand Up @@ -263,7 +266,7 @@ public void setRights(MailboxId mailboxId, MailboxACL mailboxACL, MailboxSession

@Override
public void setRights(MailboxPath mailboxPath, MailboxACL mailboxACL, MailboxSession session) throws MailboxException {
assertSharesBelongsToUserDomain(mailboxPath.getUser(), mailboxACL.getEntries());
assertUserHasAccessToShareeDomains(mailboxPath.getUser(), mailboxACL.getEntries());

MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session);
block(mapper.findMailboxByPath(mailboxPath)
Expand All @@ -285,7 +288,13 @@ private void assertHaveAccessTo(Mailbox mailbox, MailboxSession session) throws
}

@VisibleForTesting
void assertSharesBelongsToUserDomain(Username user, Map<EntryKey, Rfc4314Rights> entries) throws DifferentDomainException {
void assertUserHasAccessToShareeDomains(Username user, Map<EntryKey, Rfc4314Rights> entries) throws DifferentDomainException {
if (IS_CROSS_DOMAIN_ACCESS_ALLOWED) {
// In this case, we impose no limitations.
return;
}

// If cross-domain sharing is disabled, a user may only access their own domain.
if (entries.keySet().stream()
.filter(entry -> !entry.getNameType().equals(NameType.special))
.map(EntryKey::getName)
Expand Down Expand Up @@ -342,7 +351,7 @@ static MailboxACL filteredForSession(Mailbox mailbox, MailboxACL acl, MailboxSes
*/
public Mono<Void> setRightsReactiveWithoutAccessControl(MailboxPath mailboxPath, MailboxACL mailboxACL, MailboxSession session) {
try {
assertSharesBelongsToUserDomain(mailboxPath.getUser(), mailboxACL.getEntries());
assertUserHasAccessToShareeDomains(mailboxPath.getUser(), mailboxACL.getEntries());
} catch (DifferentDomainException e) {
return Mono.error(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,23 @@
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 jakarta.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.MailboxException;
import org.apache.james.mailbox.exception.MailboxNotFoundException;
Expand Down Expand Up @@ -65,14 +70,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 @@ -259,22 +265,37 @@ void areDomainsDifferentShouldReturnFalseWhenDomainsAreIdentical() {
}

@Test
void assertSharesBelongsToUserDomainShouldThrowWhenOneDomainIsDifferent() throws Exception {
void assertUserHasAccessToShareeDomainsShouldThrowWhenOneDomainIsDifferent() throws Exception {
MailboxACL mailboxACL = new MailboxACL(new MailboxACL.Entry("[email protected]", Right.Write),
new MailboxACL.Entry("[email protected]", Right.Write),
new MailboxACL.Entry("[email protected]", Right.Write));

assertThatThrownBy(() -> storeRightManager.assertSharesBelongsToUserDomain(Username.of("[email protected]"), mailboxACL.getEntries()))
assertThatThrownBy(() -> storeRightManager.assertUserHasAccessToShareeDomains(Username.of("[email protected]"), mailboxACL.getEntries()))
.isInstanceOf(DifferentDomainException.class);
}

@Test
void assertSharesBelongsToUserDomainShouldNotThrowWhenDomainsAreIdentical() throws Exception {
void assertUserHasAccessToShareeDomainsShouldNotThrowWhenDomainsAreIdentical() throws Exception {
MailboxACL mailboxACL = new MailboxACL(new MailboxACL.Entry("[email protected]", Right.Write),
new MailboxACL.Entry("[email protected]", Right.Write),
new MailboxACL.Entry("[email protected]", Right.Write));

storeRightManager.assertSharesBelongsToUserDomain(Username.of("[email protected]"), mailboxACL.getEntries());
storeRightManager.assertUserHasAccessToShareeDomains(Username.of("[email protected]"), mailboxACL.getEntries());
}

@Test
void assertUserHasAccessToShareDomainsShouldNotThrowOnDifferentDomainsWhenCrossDomainAccessEnabled() throws Exception {
try {
StoreRightManager.IS_CROSS_DOMAIN_ACCESS_ALLOWED = true;

MailboxACL mailboxACL = new MailboxACL(new MailboxACL.Entry("[email protected]", Right.Write),
new MailboxACL.Entry("[email protected]", Right.Write),
new MailboxACL.Entry("[email protected]", Right.Write));

storeRightManager.assertUserHasAccessToShareeDomains(Username.of("[email protected]"), mailboxACL.getEntries());
} finally {
StoreRightManager.IS_CROSS_DOMAIN_ACCESS_ALLOWED = false;
}
}

@Test
Expand All @@ -288,4 +309,30 @@ void applyRightsCommandShouldThrowWhenDomainsAreDifferent() {
assertThatThrownBy(() -> storeRightManager.applyRightsCommand(mailboxPath, aclCommand, aliceSession))
.isInstanceOf(DifferentDomainException.class);
}

@Test
void applyRightsCommandShouldNotThrowOnDifferentDomainsWhenCrossDomainEnabled() throws MailboxException {
try {
StoreRightManager.IS_CROSS_DOMAIN_ACCESS_ALLOWED = true;

MailboxPath mailboxPath = MailboxPath.forUser(Username.of("[email protected]"), "mailbox");
Mailbox mailbox = new Mailbox(mailboxPath, UID_VALIDITY, MAILBOX_ID);
mailbox.setACL(new MailboxACL(new MailboxACL.Entry(MailboxFixture.ALICE.asString(), Right.Administer)));
ACLCommand aclCommand = MailboxACL.command()
.forUser(Username.of("[email protected]"))
.rights(Right.Read)
.asAddition();

when(mockedMailboxMapper.findMailboxByPath(mailboxPath)).thenReturn(Mono.just(mailbox));
when(mockedMailboxMapper.updateACL(mailbox, aclCommand)).thenReturn(Mono.just(ACLDiff.computeDiff(MailboxACL.EMPTY, new MailboxACL(
new MailboxACL.Entry("[email protected]", Right.Read)
))));
when(eventBus.dispatch(any(Event.class), any(MailboxIdRegistrationKey.class))).thenReturn(Mono.empty());

assertThatCode(() -> storeRightManager.applyRightsCommand(mailboxPath, aclCommand, aliceSession))
.doesNotThrowAnyException();
} finally {
StoreRightManager.IS_CROSS_DOMAIN_ACCESS_ALLOWED = false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,7 @@ sun.rmi.dgc.client.gcInterval=3600000000

# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
#james.relaxed.mailbox.name.validation=true
#james.relaxed.mailbox.name.validation=true

# Allow users to have rights for shares of different domain. Defaults to false.
#james.rights.crossdomain.allow=false
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,7 @@ jmx.remote.x.mlet.allow.getMBeansFromURL=false

# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
#james.relaxed.mailbox.name.validation=true
#james.relaxed.mailbox.name.validation=true

# Allow users to have rights for shares of different domain. Defaults to false.
#james.rights.crossdomain.allow=false
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,7 @@ sun.rmi.dgc.client.gcInterval=3600000000

# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
#james.relaxed.mailbox.name.validation=true
#james.relaxed.mailbox.name.validation=true

# Allow users to have rights for shares of different domain. Defaults to false.
#james.rights.crossdomain.allow=false
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,7 @@ jmx.remote.x.mlet.allow.getMBeansFromURL=false
# james.deduplicating.blobstore.file.threshold=10240

# From which point should range operation be used? Defaults to 3.
# james.jmap.email.set.range.threshold=3
# james.jmap.email.set.range.threshold=3

# Allow users to have rights for shares of different domain. Defaults to false.
#james.rights.crossdomain.allow=false
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,7 @@ sun.rmi.dgc.client.gcInterval=3600000000

# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
#james.relaxed.mailbox.name.validation=true
#james.relaxed.mailbox.name.validation=true

# Allow users to have rights for shares of different domain. Defaults to false.
#james.rights.crossdomain.allow=false
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,7 @@ jmx.remote.x.mlet.allow.getMBeansFromURL=false

# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
#james.relaxed.mailbox.name.validation=true
#james.relaxed.mailbox.name.validation=true

# Allow users to have rights for shares of different domain. Defaults to false.
#james.rights.crossdomain.allow=false
5 changes: 4 additions & 1 deletion server/apps/jpa-app/sample-configuration/jvm.properties
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,7 @@ openjpa.Multithreaded=true

# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
#james.relaxed.mailbox.name.validation=true
#james.relaxed.mailbox.name.validation=true

# Allow users to have rights for shares of different domain. Defaults to false.
#james.rights.crossdomain.allow=false
5 changes: 4 additions & 1 deletion server/apps/memory-app/sample-configuration/jvm.properties
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,7 @@ jmx.remote.x.mlet.allow.getMBeansFromURL=false

# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
#james.relaxed.mailbox.name.validation=true
#james.relaxed.mailbox.name.validation=true

# Allow users to have rights for shares of different domain. Defaults to false.
#james.rights.crossdomain.allow=false

0 comments on commit 4c4d6d3

Please sign in to comment.