diff --git a/docs/src/main/sphinx/security/ldap.rst b/docs/src/main/sphinx/security/ldap.rst index c73f96010e50..6b0d2ea4e3fb 100644 --- a/docs/src/main/sphinx/security/ldap.rst +++ b/docs/src/main/sphinx/security/ldap.rst @@ -109,6 +109,8 @@ Property Description ``ldap.ignore-referrals`` Ignore referrals to other LDAP servers while performing search queries. Defaults to ``false``. ``ldap.cache-ttl`` LDAP cache duration. Defaults to ``1h``. +``ldap.timeout.connection`` Timeout for establishing an LDAP connection. +``ldap.timeout.read`` Timeout for reading data from an LDAP connection. ================================== ====================================================== Based on the LDAP server implementation type, the property diff --git a/plugin/trino-password-authenticators/pom.xml b/plugin/trino-password-authenticators/pom.xml index 0c442213ecc7..e1cc49474b4a 100644 --- a/plugin/trino-password-authenticators/pom.xml +++ b/plugin/trino-password-authenticators/pom.xml @@ -122,6 +122,13 @@ test + + eu.rekawek.toxiproxy + toxiproxy-java + 2.1.5 + test + + org.assertj assertj-core @@ -134,6 +141,12 @@ test + + org.testcontainers + toxiproxy + test + + org.testng testng diff --git a/plugin/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/JdkLdapAuthenticatorClient.java b/plugin/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/JdkLdapAuthenticatorClient.java index 0bdd6a0fcdc8..e73350d8ea73 100644 --- a/plugin/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/JdkLdapAuthenticatorClient.java +++ b/plugin/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/JdkLdapAuthenticatorClient.java @@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableSet; import io.airlift.log.Logger; import io.airlift.security.pem.PemReader; +import io.airlift.units.Duration; import io.trino.spi.security.AccessDeniedException; import javax.inject.Inject; @@ -65,11 +66,23 @@ public JdkLdapAuthenticatorClient(LdapConfig ldapConfig) log.warn("Passwords will be sent in the clear to the LDAP server. Please consider using SSL to connect."); } - this.basicEnvironment = ImmutableMap.builder() - .put(INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory") + ImmutableMap.Builder builder = ImmutableMap.builder(); + + builder.put(INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory") .put(PROVIDER_URL, ldapUrl) - .put(REFERRAL, ldapConfig.isIgnoreReferrals() ? "ignore" : "follow") - .buildOrThrow(); + .put(REFERRAL, ldapConfig.isIgnoreReferrals() ? "ignore" : "follow"); + + ldapConfig.getLdapConnectionTimeout() + .map(Duration::toMillis) + .map(String::valueOf) + .ifPresent(timeout -> builder.put("com.sun.jndi.ldap.connect.timeout", timeout)); + + ldapConfig.getLdapReadTimeout() + .map(Duration::toMillis) + .map(String::valueOf) + .ifPresent(timeout -> builder.put("com.sun.jndi.ldap.read.timeout", timeout)); + + this.basicEnvironment = builder.buildOrThrow(); this.sslContext = Optional.ofNullable(ldapConfig.getTrustCertificate()) .map(JdkLdapAuthenticatorClient::createSslContext); diff --git a/plugin/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapConfig.java b/plugin/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapConfig.java index 647b2920de2f..75a5f91d6502 100644 --- a/plugin/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapConfig.java +++ b/plugin/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapConfig.java @@ -27,6 +27,7 @@ import java.io.File; import java.util.List; +import java.util.Optional; import java.util.concurrent.TimeUnit; import static com.google.common.base.Strings.nullToEmpty; @@ -44,6 +45,8 @@ public class LdapConfig private String bindPassword; private boolean ignoreReferrals; private Duration ldapCacheTtl = new Duration(1, TimeUnit.HOURS); + private Optional ldapConnectionTimeout = Optional.empty(); + private Optional ldapReadTimeout = Optional.empty(); @NotNull @Pattern(regexp = "^ldaps?://.*", message = "Invalid LDAP server URL. Expected ldap:// or ldaps://") @@ -194,4 +197,30 @@ public LdapConfig setLdapCacheTtl(Duration ldapCacheTtl) this.ldapCacheTtl = ldapCacheTtl; return this; } + + public Optional getLdapConnectionTimeout() + { + return ldapConnectionTimeout; + } + + @Config("ldap.timeout.connect") + @ConfigDescription("Timeout for establishing a connection") + public LdapConfig setLdapConnectionTimeout(Duration ldapConnectionTimeout) + { + this.ldapConnectionTimeout = Optional.ofNullable(ldapConnectionTimeout); + return this; + } + + public Optional getLdapReadTimeout() + { + return ldapReadTimeout; + } + + @Config("ldap.timeout.read") + @ConfigDescription("Timeout for reading data from LDAP") + public LdapConfig setLdapReadTimeout(Duration ldapReadTimeout) + { + this.ldapReadTimeout = Optional.ofNullable(ldapReadTimeout); + return this; + } } diff --git a/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticator.java b/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticator.java index 3e2c5ac0ff03..cad01ea3dbf7 100644 --- a/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticator.java +++ b/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticator.java @@ -17,6 +17,7 @@ import io.trino.plugin.password.ldap.TestingOpenLdapServer.DisposableSubContext; import io.trino.spi.security.AccessDeniedException; import io.trino.spi.security.BasicPrincipal; +import org.testcontainers.containers.Network; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -37,7 +38,10 @@ public class TestLdapAuthenticator public void setup() throws Exception { - openLdapServer = new TestingOpenLdapServer(); + Network network = Network.newNetwork(); + closer.register(network::close); + + openLdapServer = new TestingOpenLdapServer(network); closer.register(openLdapServer); openLdapServer.start(); diff --git a/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticatorWithTimeouts.java b/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticatorWithTimeouts.java new file mode 100644 index 000000000000..abfd6b1f282f --- /dev/null +++ b/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticatorWithTimeouts.java @@ -0,0 +1,122 @@ +/* + * Licensed 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 io.trino.plugin.password.ldap; + +import com.google.common.io.Closer; +import io.airlift.units.Duration; +import io.trino.plugin.password.ldap.TestingOpenLdapServer.DisposableSubContext; +import io.trino.spi.security.BasicPrincipal; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.ToxiproxyContainer; +import org.testcontainers.containers.ToxiproxyContainer.ContainerProxy; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import static eu.rekawek.toxiproxy.model.ToxicDirection.DOWNSTREAM; +import static io.trino.plugin.password.ldap.TestingOpenLdapServer.LDAP_PORT; +import static java.lang.String.format; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.testng.Assert.assertEquals; + +public class TestLdapAuthenticatorWithTimeouts +{ + private final Closer closer = Closer.create(); + + private TestingOpenLdapServer openLdapServer; + private String proxyLdapUrl; + + @BeforeClass + public void setup() + throws Exception + { + Network network = Network.newNetwork(); + closer.register(network::close); + + ToxiproxyContainer proxyServer = new ToxiproxyContainer("shopify/toxiproxy:2.1.0") + .withNetwork(network); + closer.register(proxyServer::close); + proxyServer.start(); + + openLdapServer = new TestingOpenLdapServer(network); + closer.register(openLdapServer); + openLdapServer.start(); + + ContainerProxy proxy = proxyServer.getProxy(openLdapServer.getNetworkAlias(), LDAP_PORT); + proxy.toxics() + .latency("latency", DOWNSTREAM, 5_000); + proxyLdapUrl = format("ldap://%s:%s", proxy.getContainerIpAddress(), proxy.getProxyPort()); + } + + @AfterClass(alwaysRun = true) + public void close() + throws Exception + { + closer.close(); + } + + @Test + public void testConnectTimeout() + throws Exception + { + try (DisposableSubContext organization = openLdapServer.createOrganization(); + DisposableSubContext ignored = openLdapServer.createUser(organization, "alice", "alice-pass")) { + LdapConfig ldapConfig = new LdapConfig() + .setLdapUrl(proxyLdapUrl) + .setLdapConnectionTimeout(new Duration(1, SECONDS)) + .setUserBindSearchPatterns("uid=${USER}," + organization.getDistinguishedName()); + + LdapAuthenticator ldapAuthenticator = new LdapAuthenticator(new JdkLdapAuthenticatorClient(ldapConfig), ldapConfig); + assertThatThrownBy(() -> ldapAuthenticator.createAuthenticatedPrincipal("alice", "alice-pass")) + .isInstanceOf(RuntimeException.class) + .hasMessageMatching(".*Authentication error.*"); + + LdapConfig withIncreasedTimeout = ldapConfig.setLdapConnectionTimeout(new Duration(30, SECONDS)); + assertEquals( + new LdapAuthenticator(new JdkLdapAuthenticatorClient(withIncreasedTimeout), withIncreasedTimeout) + .createAuthenticatedPrincipal("alice", "alice-pass"), + new BasicPrincipal("alice")); + } + } + + @Test + public void testReadTimeout() + throws Exception + { + try (DisposableSubContext organization = openLdapServer.createOrganization(); + DisposableSubContext group = openLdapServer.createGroup(organization); + DisposableSubContext alice = openLdapServer.createUser(organization, "alice", "alice-pass")) { + openLdapServer.addUserToGroup(alice, group); + + LdapConfig ldapConfig = new LdapConfig() + .setLdapUrl(proxyLdapUrl) + .setLdapReadTimeout(new Duration(1, SECONDS)) + .setUserBindSearchPatterns("uid=${USER}," + organization.getDistinguishedName()) + .setUserBaseDistinguishedName(organization.getDistinguishedName()) + .setGroupAuthorizationSearchPattern(format("(&(objectClass=groupOfNames)(cn=group_*)(member=uid=${USER},%s))", organization.getDistinguishedName())); + + LdapAuthenticator ldapAuthenticator = new LdapAuthenticator(new JdkLdapAuthenticatorClient(ldapConfig), ldapConfig); + assertThatThrownBy(() -> ldapAuthenticator.createAuthenticatedPrincipal("alice", "alice-pass")) + .isInstanceOf(RuntimeException.class) + .hasMessageMatching(".*Authentication error.*"); + + LdapConfig withIncreasedTimeout = ldapConfig.setLdapReadTimeout(new Duration(30, SECONDS)); + assertEquals( + new LdapAuthenticator(new JdkLdapAuthenticatorClient(withIncreasedTimeout), withIncreasedTimeout) + .createAuthenticatedPrincipal("alice", "alice-pass"), + new BasicPrincipal("alice")); + } + } +} diff --git a/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapConfig.java b/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapConfig.java index 2271ffc01b10..4e9130f1e702 100644 --- a/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapConfig.java +++ b/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapConfig.java @@ -50,7 +50,9 @@ public void testDefault() .setBindDistingushedName(null) .setBindPassword(null) .setIgnoreReferrals(false) - .setLdapCacheTtl(new Duration(1, TimeUnit.HOURS))); + .setLdapCacheTtl(new Duration(1, TimeUnit.HOURS)) + .setLdapConnectionTimeout(null) + .setLdapReadTimeout(null)); } @Test @@ -70,6 +72,8 @@ public void testExplicitConfig() .put("ldap.bind-password", "password1234") .put("ldap.ignore-referrals", "true") .put("ldap.cache-ttl", "2m") + .put("ldap.timeout.connect", "3m") + .put("ldap.timeout.read", "4m") .buildOrThrow(); LdapConfig expected = new LdapConfig() @@ -82,7 +86,9 @@ public void testExplicitConfig() .setBindDistingushedName("CN=User Name,OU=CITY_OU,OU=STATE_OU,DC=domain,DC=domain_root") .setBindPassword("password1234") .setIgnoreReferrals(true) - .setLdapCacheTtl(new Duration(2, TimeUnit.MINUTES)); + .setLdapCacheTtl(new Duration(2, TimeUnit.MINUTES)) + .setLdapConnectionTimeout(new Duration(3, TimeUnit.MINUTES)) + .setLdapReadTimeout(new Duration(4, TimeUnit.MINUTES)); assertFullMapping(properties, expected); } diff --git a/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestingOpenLdapServer.java b/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestingOpenLdapServer.java index 708396882073..7c6fc37305a6 100644 --- a/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestingOpenLdapServer.java +++ b/plugin/trino-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestingOpenLdapServer.java @@ -18,6 +18,7 @@ import com.google.common.io.Closer; import io.trino.testing.TestingProperties; import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.Network; import org.testcontainers.containers.startupcheck.IsRunningStartupCheckStrategy; import org.testcontainers.containers.wait.strategy.HostPortWaitStrategy; @@ -46,14 +47,15 @@ public class TestingOpenLdapServer { private static final String BASE_DISTINGUISED_NAME = "dc=trino,dc=testldap,dc=com"; - private static final int LDAP_PORT = 389; + public static final int LDAP_PORT = 389; private final Closer closer = Closer.create(); private final GenericContainer openLdapServer; - public TestingOpenLdapServer() + public TestingOpenLdapServer(Network network) { openLdapServer = new GenericContainer<>("ghcr.io/trinodb/testing/centos7-oj11-openldap:" + TestingProperties.getDockerImagesVersion()) + .withNetwork(network) .withExposedPorts(LDAP_PORT) .withStartupCheckStrategy(new IsRunningStartupCheckStrategy()) .waitingFor(new HostPortWaitStrategy()) @@ -67,6 +69,11 @@ public void start() openLdapServer.start(); } + public String getNetworkAlias() + { + return openLdapServer.getNetworkAliases().get(0); + } + public String getLdapUrl() { return format("ldap://%s:%s", openLdapServer.getContainerIpAddress(), openLdapServer.getMappedPort(LDAP_PORT));