This is an automated email from the ASF dual-hosted git repository. quantranhong1999 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit d08c49f1fc2507a50c717a450c275640eea673ef Author: Quan Tran <[email protected]> AuthorDate: Tue Jul 7 09:22:12 2026 +0700 JAMES-4210 Wire per-server IMAP SASL configuration in Guice Resolve SASL mechanisms per IMAP server configuration in Guice, keep default factory override support for custom IMAP packages, and ensure capability and enable processors belong to the same IMAP processor suite. --- .../james/imap/processor/EnableProcessor.java | 12 ++- server/container/guice/protocols/imap/pom.xml | 4 + .../james/modules/protocols/IMAPServerModule.java | 84 ++++++++++++++-- .../ImapDefaultSaslMechanismFactories.java | 31 ++++++ .../modules/protocols/IMAPServerModuleTest.java | 106 +++++++++++++++++++++ 5 files changed, 226 insertions(+), 11 deletions(-) diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/EnableProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/EnableProcessor.java index 551bac70cf..b3318e9b89 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/EnableProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/EnableProcessor.java @@ -52,15 +52,15 @@ import reactor.core.publisher.Mono; public class EnableProcessor extends AbstractMailboxProcessor<EnableRequest> implements CapabilityImplementingProcessor { private static final Logger LOGGER = LoggerFactory.getLogger(EnableProcessor.class); - private static final List<PermitEnableCapabilityProcessor> capabilities = new ArrayList<>(); public static final String ENABLED_CAPABILITIES = "ENABLED_CAPABILITIES"; private static final List<Capability> CAPS = ImmutableList.of(SUPPORTS_ENABLE); - private final CapabilityProcessor capabilityProcessor; + private final List<PermitEnableCapabilityProcessor> capabilities = new ArrayList<>(); + private CapabilityProcessor capabilityProcessor; public EnableProcessor(MailboxManager mailboxManager, StatusResponseFactory factory, List<PermitEnableCapabilityProcessor> capabilities, MetricFactory metricFactory, CapabilityProcessor capabilityProcessor) { this(mailboxManager, factory, metricFactory, capabilityProcessor); - EnableProcessor.capabilities.addAll(capabilities); + this.capabilities.addAll(capabilities); } @Inject @@ -70,6 +70,12 @@ public class EnableProcessor extends AbstractMailboxProcessor<EnableRequest> imp this.capabilityProcessor = capabilityProcessor; } + /** + * Use the capability processor from the same IMAP suite. + */ + public void configureCapabilityProcessor(CapabilityProcessor capabilityProcessor) { + this.capabilityProcessor = capabilityProcessor; + } @Override protected Mono<Void> processRequestReactive(EnableRequest request, ImapSession session, Responder responder) { diff --git a/server/container/guice/protocols/imap/pom.xml b/server/container/guice/protocols/imap/pom.xml index 11a22740b5..dc99029ba7 100644 --- a/server/container/guice/protocols/imap/pom.xml +++ b/server/container/guice/protocols/imap/pom.xml @@ -53,6 +53,10 @@ <artifactId>testing-base</artifactId> <scope>test</scope> </dependency> + <dependency> + <groupId>${james.protocols.groupId}</groupId> + <artifactId>protocols-sasl</artifactId> + </dependency> <dependency> <groupId>com.google.inject</groupId> <artifactId>guice</artifactId> diff --git a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/IMAPServerModule.java b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/IMAPServerModule.java index 3c3fc4ee3d..5733cea9cd 100644 --- a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/IMAPServerModule.java +++ b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/IMAPServerModule.java @@ -26,6 +26,7 @@ import java.util.stream.Stream; import org.apache.commons.configuration2.HierarchicalConfiguration; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.commons.configuration2.tree.ImmutableNode; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.apache.james.ProtocolConfigurationSanitizer; import org.apache.james.RunArguments; @@ -60,6 +61,7 @@ import org.apache.james.imap.processor.CapabilityImplementingProcessor; import org.apache.james.imap.processor.CapabilityProcessor; import org.apache.james.imap.processor.DefaultProcessor; import org.apache.james.imap.processor.EnableProcessor; +import org.apache.james.imap.processor.LoginProcessor; import org.apache.james.imap.processor.NamespaceSupplier; import org.apache.james.imap.processor.PermitEnableCapabilityProcessor; import org.apache.james.imap.processor.SelectProcessor; @@ -72,12 +74,19 @@ import org.apache.james.imapserver.netty.IMAPServerFactory; import org.apache.james.lifecycle.api.ConfigurationSanitizer; import org.apache.james.metrics.api.GaugeRegistry; import org.apache.james.metrics.api.MetricFactory; +import org.apache.james.protocols.api.sasl.SaslMechanism; +import org.apache.james.protocols.api.sasl.SaslMechanismFactory; import org.apache.james.protocols.lib.netty.CertificateReloadable; import org.apache.james.protocols.netty.Encryption; +import org.apache.james.protocols.sasl.BuiltInSaslMechanismFactories; +import org.apache.james.protocols.sasl.OauthBearerSaslMechanismFactory; +import org.apache.james.protocols.sasl.PlainSaslMechanismFactory; +import org.apache.james.protocols.sasl.XOauth2SaslMechanismFactory; import org.apache.james.server.core.configuration.ConfigurationProvider; import org.apache.james.utils.ClassName; import org.apache.james.utils.GuiceLoader; import org.apache.james.utils.GuiceProbe; +import org.apache.james.utils.GuiceSaslMechanismResolver; import org.apache.james.utils.InitializationOperation; import org.apache.james.utils.InitilizationOperationBuilder; import org.apache.james.utils.KeystoreCreator; @@ -94,7 +103,6 @@ import com.google.inject.multibindings.Multibinder; import com.google.inject.multibindings.ProvidesIntoSet; public class IMAPServerModule extends AbstractModule { - private static Stream<Pair<Class, AbstractProcessor>> asPairStream(AbstractProcessor p) { return p.acceptableClasses() .stream().map(clazz -> Pair.of(clazz, p)); @@ -106,11 +114,13 @@ public class IMAPServerModule extends AbstractModule { bind(UnpooledStatusResponseFactory.class).in(Scopes.SINGLETON); bind(StatusResponseFactory.class).to(UnpooledStatusResponseFactory.class); - bind(CapabilityProcessor.class).in(Scopes.SINGLETON); - bind(AuthenticateProcessor.class).in(Scopes.SINGLETON); + // Keep CapabilityProcessor, AuthenticateProcessor and EnableProcessor unscoped: IMAP suite loading configures + // their SASL mechanisms and capability links from each server configuration. + bind(CapabilityProcessor.class); + bind(AuthenticateProcessor.class); + bind(EnableProcessor.class); bind(SelectProcessor.class).in(Scopes.SINGLETON); bind(StatusProcessor.class).in(Scopes.SINGLETON); - bind(EnableProcessor.class).in(Scopes.SINGLETON); bind(NamespaceSupplier.class).to(NamespaceSupplier.Default.class).in(Scopes.SINGLETON); bind(PathConverter.Factory.class).to(PathConverter.Factory.Default.class).in(Scopes.SINGLETON); bind(MailboxTyper.class).to(DefaultMailboxTyper.class).in(Scopes.SINGLETON); @@ -130,21 +140,35 @@ public class IMAPServerModule extends AbstractModule { @Singleton IMAPServerFactory provideServerFactory(FileSystem fileSystem, GuiceLoader guiceLoader, + GuiceSaslMechanismResolver saslMechanismResolver, + @ImapDefaultSaslMechanismFactories ImmutableList<SaslMechanismFactory> defaultSaslMechanismFactories, StatusResponseFactory statusResponseFactory, MetricFactory metricFactory, GaugeRegistry gaugeRegistry, ConnectionCheckFactory connectionCheckFactory, Encryption.Factory encryptionFactory) { - IMAPServerFactory factory = new IMAPServerFactory(fileSystem, imapSuiteLoader(guiceLoader, statusResponseFactory), metricFactory, gaugeRegistry, connectionCheckFactory); + IMAPServerFactory factory = new IMAPServerFactory(fileSystem, imapSuiteLoader(guiceLoader, saslMechanismResolver, + defaultSaslMechanismFactories, statusResponseFactory), metricFactory, gaugeRegistry, connectionCheckFactory); factory.setEncryptionFactory(encryptionFactory); return factory; } - DefaultProcessor provideClassImapProcessors(ImapPackage imapPackage, GuiceLoader guiceLoader, StatusResponseFactory statusResponseFactory) { + @Provides + @Singleton + @ImapDefaultSaslMechanismFactories + ImmutableList<SaslMechanismFactory> provideDefaultImapSaslMechanismFactories(PlainSaslMechanismFactory plain, + OauthBearerSaslMechanismFactory oauthBearer, + XOauth2SaslMechanismFactory xoauth2) { + return ImmutableList.of(plain, oauthBearer, xoauth2); + } + + DefaultProcessor provideClassImapProcessors(ImapPackage imapPackage, GuiceLoader guiceLoader, + ImmutableList<SaslMechanism> saslMechanisms, StatusResponseFactory statusResponseFactory) { ImmutableMap<Class, ImapProcessor> processors = imapPackage.processors() .stream() .map(Throwing.function(guiceLoader::instantiate)) .map(AbstractProcessor.class::cast) + .map(processor -> configureSaslMechanisms(processor, saslMechanisms)) .flatMap(IMAPServerModule::asPairStream) .collect(ImmutableMap.toImmutableMap( Pair::getLeft, @@ -168,6 +192,16 @@ public class IMAPServerModule extends AbstractModule { return new DefaultProcessor(processors, new UnknownRequestProcessor(statusResponseFactory)); } + private AbstractProcessor configureSaslMechanisms(AbstractProcessor processor, ImmutableList<SaslMechanism> saslMechanisms) { + if (processor instanceof AuthenticateProcessor authenticateProcessor) { + authenticateProcessor.configureSaslMechanisms(saslMechanisms); + } + if (processor instanceof LoginProcessor loginProcessor) { + loginProcessor.configureSaslMechanisms(saslMechanisms); + } + return processor; + } + private ImapPackage retrievePackages(GuiceLoader guiceLoader, HierarchicalConfiguration<ImmutableNode> configuration) { String[] imapPackages = configuration.getStringArray("imapPackages"); @@ -185,11 +219,39 @@ public class IMAPServerModule extends AbstractModule { return ImapPackage.and(packages); } + private ImmutableList<SaslMechanism> retrieveSaslMechanisms(GuiceSaslMechanismResolver saslMechanismResolver, + ImmutableList<SaslMechanismFactory> defaultSaslMechanismFactories, + HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException { + ImmutableList<String> mechanismFactoryClassNames = retrieveSaslMechanismFactoryClassNames(configuration); + ImmutableList<SaslMechanismFactory> enabledDefaultFactories = + BuiltInSaslMechanismFactories.enabledForServer(defaultSaslMechanismFactories, configuration); + return saslMechanismResolver.resolve(mechanismFactoryClassNames, enabledDefaultFactories, configuration); + } + + ImmutableList<String> retrieveSaslMechanismFactoryClassNames(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException { + if (!configuration.containsKey("auth.saslMechanisms")) { + return ImmutableList.of(); + } + + ImmutableList<String> mechanismFactoryClassNames = Arrays.stream(configuration.getStringArray("auth.saslMechanisms")) + .flatMap(value -> Arrays.stream(value.split(","))) + .map(String::trim) + .collect(ImmutableList.toImmutableList()); + + if (mechanismFactoryClassNames.isEmpty() || mechanismFactoryClassNames.stream().anyMatch(StringUtils::isBlank)) { + throw new ConfigurationException("auth.saslMechanisms must not be blank when configured"); + } + return mechanismFactoryClassNames; + } + private ThrowingFunction<HierarchicalConfiguration<ImmutableNode>, ImapSuite> imapSuiteLoader(GuiceLoader guiceLoader, + GuiceSaslMechanismResolver saslMechanismResolver, + ImmutableList<SaslMechanismFactory> defaultSaslMechanismFactories, StatusResponseFactory statusResponseFactory) { return configuration -> { ImapPackage imapPackage = retrievePackages(guiceLoader, configuration); - DefaultProcessor processor = provideClassImapProcessors(imapPackage, guiceLoader, statusResponseFactory); + ImmutableList<SaslMechanism> saslMechanisms = retrieveSaslMechanisms(saslMechanismResolver, defaultSaslMechanismFactories, configuration); + DefaultProcessor processor = provideClassImapProcessors(imapPackage, guiceLoader, saslMechanisms, statusResponseFactory); ImapEncoder encoder = provideImapEncoder(imapPackage, guiceLoader); ImapParserFactory imapParserFactory = provideImapCommandParserFactory(imapPackage, guiceLoader); @@ -231,6 +293,12 @@ public class IMAPServerModule extends AbstractModule { } private void configureEnable(EnableProcessor enableProcessor, ImmutableMap<Class, ImapProcessor> processorMap) { + processorMap.values().stream() + .filter(CapabilityProcessor.class::isInstance) + .map(CapabilityProcessor.class::cast) + .findFirst() + .ifPresent(enableProcessor::configureCapabilityProcessor); + processorMap.values().stream() .filter(PermitEnableCapabilityProcessor.class::isInstance) .map(PermitEnableCapabilityProcessor.class::cast) @@ -262,4 +330,4 @@ public class IMAPServerModule extends AbstractModule { FileSystem fileSystem, RunArguments runArguments) { return new ProtocolConfigurationSanitizer(configurationProvider, keystoreCreator, fileSystem, runArguments, "imapserver"); } -} \ No newline at end of file +} diff --git a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapDefaultSaslMechanismFactories.java b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapDefaultSaslMechanismFactories.java new file mode 100644 index 0000000000..6cac7200fd --- /dev/null +++ b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapDefaultSaslMechanismFactories.java @@ -0,0 +1,31 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.modules.protocols; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; + +import com.google.inject.BindingAnnotation; + +@BindingAnnotation +@Retention(RUNTIME) +public @interface ImapDefaultSaslMechanismFactories { +} diff --git a/server/container/guice/protocols/imap/src/test/java/org/apache/james/modules/protocols/IMAPServerModuleTest.java b/server/container/guice/protocols/imap/src/test/java/org/apache/james/modules/protocols/IMAPServerModuleTest.java new file mode 100644 index 0000000000..611ec5ceea --- /dev/null +++ b/server/container/guice/protocols/imap/src/test/java/org/apache/james/modules/protocols/IMAPServerModuleTest.java @@ -0,0 +1,106 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.modules.protocols; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.apache.commons.configuration2.BaseHierarchicalConfiguration; +import org.apache.commons.configuration2.ex.ConfigurationException; +import org.apache.james.protocols.sasl.OauthBearerSaslMechanismFactory; +import org.apache.james.protocols.sasl.PlainSaslMechanismFactory; +import org.apache.james.protocols.sasl.XOauth2SaslMechanismFactory; +import org.junit.jupiter.api.Test; + +import com.google.common.collect.ImmutableList; + +class IMAPServerModuleTest { + private final IMAPServerModule testee = new IMAPServerModule(); + + @Test + void provideDefaultImapSaslMechanismFactoriesShouldReturnJamesDefaults() { + // GIVEN no auth.saslMechanisms configuration + // WHEN IMAP provides its default SASL factories + + // THEN existing James IMAP defaults are preserved in order + assertThat(testee.provideDefaultImapSaslMechanismFactories( + new PlainSaslMechanismFactory(), + new OauthBearerSaslMechanismFactory(), + new XOauth2SaslMechanismFactory())) + .map(factory -> factory.getClass().getSimpleName()) + .containsExactly( + PlainSaslMechanismFactory.class.getSimpleName(), + OauthBearerSaslMechanismFactory.class.getSimpleName(), + XOauth2SaslMechanismFactory.class.getSimpleName()); + } + + @Test + void retrieveSaslMechanismFactoryClassNamesShouldReturnEmptyWhenAbsent() throws Exception { + // GIVEN no auth.saslMechanisms configuration. + // The empty configured list lets the resolver use the Guice-provided default factory list. + // Community custom IMAP packages can override that default factory list to avoid breaking changes. + BaseHierarchicalConfiguration configuration = new BaseHierarchicalConfiguration(); + + // WHEN auth.saslMechanisms is absent + ImmutableList<String> mechanismFactoryClassNames = testee.retrieveSaslMechanismFactoryClassNames(configuration); + + // THEN there is no configured override + assertThat(mechanismFactoryClassNames).isEmpty(); + } + + @Test + void retrieveSaslMechanismFactoryClassNamesShouldReturnConfiguredSaslFactoryList() throws Exception { + // GIVEN an explicit server-specific SASL factory list + BaseHierarchicalConfiguration configuration = new BaseHierarchicalConfiguration(); + configuration.addProperty("auth.saslMechanisms", + "PlainSaslMechanismFactory,com.example.CustomSaslMechanismFactory,PlainSaslMechanismFactory"); + + // WHEN IMAP resolves configured factory class names + ImmutableList<String> mechanismFactoryClassNames = testee.retrieveSaslMechanismFactoryClassNames(configuration); + + // THEN the exact configured order is passed to the resolver + assertThat(mechanismFactoryClassNames) + .containsExactly("PlainSaslMechanismFactory", "com.example.CustomSaslMechanismFactory", "PlainSaslMechanismFactory"); + } + + @Test + void retrieveSaslMechanismFactoryClassNamesShouldRejectBlankConfiguredList() { + // GIVEN auth.saslMechanisms is present but blank + BaseHierarchicalConfiguration configuration = new BaseHierarchicalConfiguration(); + configuration.addProperty("auth.saslMechanisms", " "); + + // WHEN resolving factory class names + // THEN startup fails instead of silently disabling all mechanisms + assertThatThrownBy(() -> testee.retrieveSaslMechanismFactoryClassNames(configuration)) + .isInstanceOf(ConfigurationException.class); + } + + @Test + void retrieveSaslMechanismFactoryClassNamesShouldRejectBlankEntry() { + // GIVEN auth.saslMechanisms contains a blank entry + BaseHierarchicalConfiguration configuration = new BaseHierarchicalConfiguration(); + configuration.addProperty("auth.saslMechanisms", "PlainSaslMechanismFactory,,XOauth2SaslMechanismFactory"); + + // WHEN resolving factory class names + // THEN startup fails with an invalid configured list + assertThatThrownBy(() -> testee.retrieveSaslMechanismFactoryClassNames(configuration)) + .isInstanceOf(ConfigurationException.class); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
