-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Enable security manager for active directory tests #112411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,13 +25,14 @@ | |
import org.elasticsearch.jdk.JarHell; | ||
import org.elasticsearch.plugins.PluginDescriptor; | ||
import org.elasticsearch.secure_sm.SecureSM; | ||
import org.elasticsearch.test.ESTestCase; | ||
import org.elasticsearch.test.PrivilegedOperations; | ||
import org.elasticsearch.test.mockito.SecureMockMaker; | ||
import org.junit.Assert; | ||
|
||
import java.io.Closeable; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.io.UncheckedIOException; | ||
import java.lang.invoke.MethodHandles; | ||
import java.net.SocketPermission; | ||
import java.net.URL; | ||
|
@@ -53,6 +54,7 @@ | |
import java.util.Objects; | ||
import java.util.Properties; | ||
import java.util.Set; | ||
import java.util.function.Supplier; | ||
import java.util.stream.Collectors; | ||
|
||
import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean; | ||
|
@@ -350,19 +352,19 @@ static Set<URL> parseClassPathWithSymlinks() throws Exception { | |
public static void ensureInitialized() {} | ||
|
||
/** | ||
* Temporarily dsiables security manager for a test. | ||
* | ||
* <p> This method is only callable by {@link org.elasticsearch.test.ESTestCase}. | ||
* Temporarily disables security manager for a test. | ||
* | ||
* @return A closeable object which restores the test security manager | ||
*/ | ||
@SuppressWarnings("removal") | ||
public static Closeable disableTestSecurityManager() { | ||
var caller = Thread.currentThread().getStackTrace()[2]; | ||
if (ESTestCase.class.getName().equals(caller.getClassName()) == false) { | ||
throw new SecurityException("Cannot disable test SecurityManager directly. Use @NoSecurityManager to disable on a test suite"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note for core/infra: I've relaxed this requirement to enable broader use of disabling security manager for parts of testing. The most important thing is to ensure security manager is not disabled and forgotten. We already return a releasable object, and additional validation ensures we haven't forgotten to re-enable security manager between tests. |
||
} | ||
final var sm = System.getSecurityManager(); | ||
if (sm == null) { | ||
throw new SecurityException( | ||
"SecurityManager already disabled. This is indicative of a test bug. " | ||
+ "Please ensure callers to this method close the returned Closeable." | ||
); | ||
} | ||
AccessController.doPrivileged((PrivilegedAction<Void>) () -> { | ||
Security.setSecurityManager(null); | ||
return null; | ||
|
@@ -372,4 +374,28 @@ public static Closeable disableTestSecurityManager() { | |
return null; | ||
}); | ||
} | ||
|
||
/** | ||
* Runs the given action, returning the produced object, without the security manager. | ||
* This is a convenience method for {@link #disableTestSecurityManager()}. | ||
*/ | ||
public static <T> T doWithSecurityManagerDisabled(Supplier<T> action) { | ||
try (var ignore = BootstrapForTesting.disableTestSecurityManager()) { | ||
return action.get(); | ||
} catch (IOException e) { | ||
throw new UncheckedIOException(e); | ||
} | ||
} | ||
|
||
/** | ||
* Runs the given action without the security manager. | ||
* This is a convenience method for {@link #disableTestSecurityManager()}. | ||
*/ | ||
public static void doWithSecurityManagerDisabled(Runnable action) { | ||
try (var ignore = BootstrapForTesting.disableTestSecurityManager()) { | ||
action.run(); | ||
} catch (IOException e) { | ||
throw new UncheckedIOException(e); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could change this to return
Releaseable
so we don't need to catchIOException
in the methods below?