Skip to content

Commit e8e9c08

Browse files
committed
Enable security manager for active directory tests
When active directory tests switched to using test rules for manager docker fixtures, the security manager was disabled. This commit re-enables the security manager, and isolates the docker setup/start to not run under security manager.
1 parent b000271 commit e8e9c08

File tree

10 files changed

+50
-14
lines changed

10 files changed

+50
-14
lines changed

test/fixtures/testcontainer-utils/build.gradle

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ configurations.all {
55
}
66

77
dependencies {
8-
testImplementation project(':test:framework')
8+
implementation project(':libs:elasticsearch-core')
9+
implementation project(':test:framework')
910
api "junit:junit:${versions.junit}"
1011
api "org.testcontainers:testcontainers:${versions.testcontainer}"
1112
implementation "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"

test/fixtures/testcontainer-utils/src/main/java/org/elasticsearch/test/fixtures/testcontainers/DockerEnvironmentAwareTestContainer.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package org.elasticsearch.test.fixtures.testcontainers;
1010

11+
import org.elasticsearch.bootstrap.BootstrapForTesting;
1112
import org.elasticsearch.test.fixtures.CacheableTestFixture;
1213
import org.junit.Assume;
1314
import org.junit.rules.TestRule;
@@ -65,6 +66,7 @@ public void start() {
6566
Assume.assumeFalse("Docker support excluded on OS", EXCLUDED_OS);
6667
Assume.assumeTrue("Docker probing succesful", DOCKER_PROBING_SUCCESSFUL);
6768
withLogConsumer(new Slf4jLogConsumer(LOGGER));
69+
BootstrapForTesting.doWithSecurityManagerDisabled(super::start);
6870
super.start();
6971
}
7072

test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
import org.junit.Assert;
3131

3232
import java.io.Closeable;
33+
import java.io.IOException;
3334
import java.io.InputStream;
35+
import java.io.UncheckedIOException;
3436
import java.lang.invoke.MethodHandles;
3537
import java.net.SocketPermission;
3638
import java.net.URL;
@@ -52,6 +54,7 @@
5254
import java.util.Objects;
5355
import java.util.Properties;
5456
import java.util.Set;
57+
import java.util.function.Supplier;
5558
import java.util.stream.Collectors;
5659

5760
import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
@@ -349,19 +352,17 @@ static Set<URL> parseClassPathWithSymlinks() throws Exception {
349352
public static void ensureInitialized() {}
350353

351354
/**
352-
* Temporarily dsiables security manager for a test.
353-
*
354-
* <p> This method is only callable by {@link org.elasticsearch.test.ESTestCase}.
355+
* Temporarily disables security manager for a test.
355356
*
356357
* @return A closeable object which restores the test security manager
357358
*/
358359
@SuppressWarnings("removal")
359360
public static Closeable disableTestSecurityManager() {
360-
var caller = Thread.currentThread().getStackTrace()[2];
361-
if (ESTestCase.class.getName().equals(caller.getClassName()) == false) {
362-
throw new SecurityException("Cannot disable test SecurityManager directly. Use @NoSecurityManager to disable on a test suite");
363-
}
364361
final var sm = System.getSecurityManager();
362+
if (sm == null) {
363+
throw new SecurityException("SecurityManager already disabled. This is indicative of a test bug. " +
364+
"Please ensure callers to this method close the returned Closeable.");
365+
}
365366
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
366367
Security.setSecurityManager(null);
367368
return null;
@@ -371,4 +372,28 @@ public static Closeable disableTestSecurityManager() {
371372
return null;
372373
});
373374
}
375+
376+
/**
377+
* Runs the given action, returning the produced object, without the security manager.
378+
* This is a convenience method for {@link #disableTestSecurityManager()}.
379+
*/
380+
public static <T> T doWithSecurityManagerDisabled(Supplier<T> action) {
381+
try (var ignore = BootstrapForTesting.disableTestSecurityManager()) {
382+
return action.get();
383+
} catch (IOException e) {
384+
throw new UncheckedIOException(e);
385+
}
386+
}
387+
388+
/**
389+
* Runs the given action without the security manager.
390+
* This is a convenience method for {@link #disableTestSecurityManager()}.
391+
*/
392+
public static void doWithSecurityManagerDisabled(Runnable action) {
393+
try (var ignore = BootstrapForTesting.disableTestSecurityManager()) {
394+
action.run();
395+
} catch (IOException e) {
396+
throw new UncheckedIOException(e);
397+
}
398+
}
374399
}

x-pack/qa/third-party/active-directory/build.gradle

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ tasks.named("forbiddenPatterns").configure {
2626
}
2727

2828
tasks.named("test").configure {
29-
systemProperty 'tests.security.manager', 'false'
3029
include '**/*IT.class'
3130
include '**/*Tests.class'
3231
}

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractActiveDirectoryTestCase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
public abstract class AbstractActiveDirectoryTestCase extends ESTestCase {
4848

4949
@ClassRule
50-
public static final SmbTestContainer smbFixture = new SmbTestContainer();
50+
public static final SmbTestContainer smbFixture = SmbTestContainer.create();
5151
// follow referrals defaults to false here which differs from the default value of the setting
5252
// this is needed to prevent test logs being filled by errors as the default configuration of
5353
// the tests run against a vagrant samba4 instance configured as a domain controller with the

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractAdLdapRealmTestCase.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.action.ActionFuture;
1515
import org.elasticsearch.action.DocWriteResponse;
1616
import org.elasticsearch.action.get.GetResponse;
17+
import org.elasticsearch.bootstrap.BootstrapForTesting;
1718
import org.elasticsearch.client.internal.Client;
1819
import org.elasticsearch.common.bytes.BytesArray;
1920
import org.elasticsearch.common.settings.SecureString;
@@ -42,6 +43,7 @@
4243
import org.junit.ClassRule;
4344

4445
import java.io.IOException;
46+
import java.io.UncheckedIOException;
4547
import java.nio.file.Path;
4648
import java.util.Arrays;
4749
import java.util.Collections;
@@ -76,7 +78,7 @@ public abstract class AbstractAdLdapRealmTestCase extends SecurityIntegTestCase
7678
public static final String SECURITY_INDEX = "security";
7779

7880
@ClassRule
79-
public static final SmbTestContainer smbFixture = new SmbTestContainer();
81+
public static final SmbTestContainer smbFixture = SmbTestContainer.create();
8082

8183
private static final RoleMappingEntry[] AD_ROLE_MAPPING = new RoleMappingEntry[] {
8284
new RoleMappingEntry("SHIELD: [ \"CN=SHIELD,CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com\" ]", """

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryGroupsResolverTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public class ActiveDirectoryGroupsResolverTests extends GroupsResolverTestCase {
3636
private static final RealmConfig.RealmIdentifier REALM_ID = new RealmConfig.RealmIdentifier("active_directory", "ad");
3737

3838
@ClassRule
39-
public static final SmbTestContainer smbFixture = new SmbTestContainer();
39+
public static final SmbTestContainer smbFixture = SmbTestContainer.create();
4040

4141
@Before
4242
public void setReferralFollowing() {

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/UserAttributeGroupsResolverTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import com.unboundid.ldap.sdk.SearchRequest;
1212
import com.unboundid.ldap.sdk.SearchScope;
1313

14+
import org.elasticsearch.bootstrap.BootstrapForTesting;
1415
import org.elasticsearch.common.settings.Settings;
1516
import org.elasticsearch.core.TimeValue;
1617
import org.elasticsearch.test.fixtures.smb.SmbTestContainer;
@@ -37,7 +38,7 @@ public class UserAttributeGroupsResolverTests extends GroupsResolverTestCase {
3738
private static final RealmConfig.RealmIdentifier REALM_ID = new RealmConfig.RealmIdentifier("ldap", "realm1");
3839

3940
@ClassRule
40-
public static final SmbTestContainer smbFixture = new SmbTestContainer();
41+
public static final SmbTestContainer smbFixture = SmbTestContainer.create();
4142

4243
public void testResolve() throws Exception {
4344
// falling back on the 'memberOf' attribute

x-pack/test/smb-fixture/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ apply plugin: 'elasticsearch.java'
22
apply plugin: 'elasticsearch.cache-test-fixtures'
33

44
dependencies {
5+
implementation project(':test:framework')
56
api project(':test:fixtures:testcontainer-utils')
67
api "junit:junit:${versions.junit}"
78
api "org.testcontainers:testcontainers:${versions.testcontainer}"

x-pack/test/smb-fixture/src/main/java/org/elasticsearch/test/fixtures/smb/SmbTestContainer.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.test.fixtures.smb;
99

10+
import org.elasticsearch.bootstrap.BootstrapForTesting;
1011
import org.elasticsearch.test.fixtures.testcontainers.DockerEnvironmentAwareTestContainer;
1112
import org.testcontainers.images.builder.ImageFromDockerfile;
1213

@@ -16,7 +17,7 @@ public final class SmbTestContainer extends DockerEnvironmentAwareTestContainer
1617
public static final int AD_LDAP_PORT = 636;
1718
public static final int AD_LDAP_GC_PORT = 3269;
1819

19-
public SmbTestContainer() {
20+
private SmbTestContainer() {
2021
super(
2122
new ImageFromDockerfile("es-smb-fixture").withDockerfileFromBuilder(
2223
builder -> builder.from(DOCKER_BASE_IMAGE)
@@ -43,6 +44,10 @@ public SmbTestContainer() {
4344
addExposedPort(AD_LDAP_GC_PORT);
4445
}
4546

47+
public static SmbTestContainer create() {
48+
return BootstrapForTesting.doWithSecurityManagerDisabled(SmbTestContainer::new);
49+
}
50+
4651
public String getAdLdapUrl() {
4752
return "ldaps://localhost:" + getMappedPort(AD_LDAP_PORT);
4853
}

0 commit comments

Comments
 (0)