Skip to content
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

Allow Plugins to request to perform cluster actions and index actions with their assigned PluginSubject and prompt on install #15778

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
43e8974
WIP on requesting perms and displaying on install
cwperks Sep 5, 2024
a841f06
Actually compile a fake plugin in InstallPluginCommandTests
cwperks Sep 5, 2024
edfbe36
Merge branch 'main' into identity-aware-plugin-request-perms
cwperks Sep 16, 2024
5821632
Add test for prompt when installing IdentityAwarePlugin
cwperks Sep 16, 2024
1106978
Update warning message
cwperks Sep 16, 2024
4a29367
Get requested actions from plugin-permissions.yml
cwperks Sep 17, 2024
f4bd921
Allow plugin dev to configure a description to describe why plugin is…
cwperks Sep 17, 2024
454a5dd
Add requested actions to PluginInfo and pass PluginInfo IdentityPlugi…
cwperks Sep 19, 2024
c031a1b
Merge branch 'main' into identity-aware-plugin-request-perms
cwperks Sep 19, 2024
a2fb0fd
Add null check
cwperks Sep 19, 2024
758b671
Check stream version
cwperks Sep 19, 2024
4cdd593
Check version when serializing
cwperks Sep 19, 2024
394c47a
Handle case where requestedActions is null
cwperks Sep 19, 2024
fde386d
Merge branch 'main' into identity-aware-plugin-request-perms
cwperks Sep 20, 2024
157740b
Ensure requestedActions is non-null
cwperks Sep 20, 2024
bf44a29
Simplify tests
cwperks Sep 20, 2024
8b16051
modify correct build.gradle
cwperks Sep 20, 2024
c12df2d
Remove unused code
cwperks Sep 20, 2024
abbb6f0
Remove unused import
cwperks Sep 20, 2024
df6853b
Merge branch 'main' into identity-aware-plugin-request-perms
cwperks Oct 2, 2024
6b64364
Merge branch 'main' into identity-aware-plugin-request-perms
cwperks Oct 25, 2024
51a6cde
Merge branch 'main' into identity-aware-plugin-request-perms
cwperks Oct 29, 2024
5b8c079
Update ToXContent
cwperks Oct 29, 2024
ddeb16b
Set to CURRENT until backport
cwperks Oct 29, 2024
ea1af1f
Merge branch 'main' into identity-aware-plugin-request-perms
cwperks Nov 5, 2024
765570b
Merge branch 'main' into identity-aware-plugin-request-perms
cwperks Feb 27, 2025
84c202f
Fix precommit
cwperks Feb 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.opensearch.common.cli.EnvironmentAwareCommand;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.hash.MessageDigests;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.io.IOUtils;
import org.opensearch.env.Environment;
import org.opensearch.plugins.Platforms;
Expand Down Expand Up @@ -95,6 +96,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -106,6 +108,10 @@
import java.util.stream.Collectors;

import static org.opensearch.cli.Terminal.Verbosity.VERBOSE;
import static org.opensearch.plugins.PluginInfo.CLUSTER_ACTIONS_SETTING;
import static org.opensearch.plugins.PluginInfo.DESCRIPTION_SETTING;
import static org.opensearch.plugins.PluginInfo.INDEX_ACTIONS_SETTING;
import static org.opensearch.plugins.PluginInfo.parseRequestedActions;

/**
* A command for the plugin cli to install a plugin into opensearch.
Expand Down Expand Up @@ -850,7 +856,34 @@ private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoo
} else {
permissions = Collections.emptySet();
}
PluginSecurity.confirmPolicyExceptions(terminal, permissions, isBatch);

Path actions = tmpRoot.resolve(PluginInfo.OPENSEARCH_PLUGIN_ACTIONS);
Settings requestedActions = Settings.EMPTY;

if (Files.exists(actions)) {
requestedActions = parseRequestedActions(actions);
}

final Map<String, List<String>> requestedIndexActions = new HashMap<>();

final List<String> requestedClusterActions = CLUSTER_ACTIONS_SETTING.get(requestedActions);
final Settings requestedIndexActionsGroup = INDEX_ACTIONS_SETTING.get(requestedActions);
final String pluginActionDescription = DESCRIPTION_SETTING.get(requestedActions);
if (!requestedIndexActionsGroup.keySet().isEmpty()) {
for (String indexPattern : requestedIndexActionsGroup.keySet()) {
List<String> indexActionsForPattern = requestedIndexActionsGroup.getAsList(indexPattern);
requestedIndexActions.put(indexPattern, indexActionsForPattern);
}
}

PluginSecurity.confirmPolicyExceptions(
terminal,
permissions,
pluginActionDescription,
requestedClusterActions,
requestedIndexActions,
isBatch
);

String targetFolderName = info.getTargetFolderName();
final Path destination = env.pluginsDir().resolve(targetFolderName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

Expand All @@ -64,10 +65,17 @@ class PluginSecurity {
/**
* prints/confirms policy exceptions with the user
*/
static void confirmPolicyExceptions(Terminal terminal, Set<String> permissions, boolean batch) throws UserException {
static void confirmPolicyExceptions(
Terminal terminal,
Set<String> permissions,
String description,
List<String> requestedClusterActions,
Map<String, List<String>> requestedIndexActions,
boolean batch
) throws UserException {
List<String> requested = new ArrayList<>(permissions);
if (requested.isEmpty()) {
terminal.println(Verbosity.VERBOSE, "plugin has a policy file with no additional permissions");
if (requested.isEmpty() && requestedClusterActions.isEmpty() && requestedIndexActions.isEmpty()) {
terminal.println(Verbosity.VERBOSE, "plugin has not requested any additional permissions");
} else {

// sort permissions in a reasonable order
Expand All @@ -76,12 +84,56 @@ static void confirmPolicyExceptions(Terminal terminal, Set<String> permissions,
terminal.errorPrintln(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
terminal.errorPrintln(Verbosity.NORMAL, "@ WARNING: plugin requires additional permissions @");
terminal.errorPrintln(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
// print all permissions:
for (String permission : requested) {
terminal.errorPrintln(Verbosity.NORMAL, "* " + permission);
if (!requested.isEmpty()) {
// print all permissions:
for (String permission : requested) {
terminal.errorPrintln(Verbosity.NORMAL, "* " + permission);
}
terminal.errorPrintln(
Verbosity.NORMAL,
"See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html"
);
terminal.errorPrintln(Verbosity.NORMAL, "for descriptions of what these permissions allow and the associated risks.");
terminal.errorPrintln(Verbosity.NORMAL, "");
terminal.errorPrintln(Verbosity.NORMAL, "Plugin requests permission to perform the following transport actions. Any index");
terminal.errorPrintln(
Verbosity.NORMAL,
"pattern that appears below is a default value and may change depending on plugin settings."
);
}
terminal.errorPrintln(Verbosity.NORMAL, "See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html");
terminal.errorPrintln(Verbosity.NORMAL, "for descriptions of what these permissions allow and the associated risks.");
if (!requestedClusterActions.isEmpty() || !requestedIndexActions.isEmpty()) {
terminal.errorPrintln(Verbosity.NORMAL, "");
if (description != null) {
terminal.errorPrintln(Verbosity.NORMAL, description);
terminal.errorPrintln(Verbosity.NORMAL, "");
}
terminal.errorPrintln(Verbosity.NORMAL, "Cluster Actions");
terminal.errorPrintln(Verbosity.NORMAL, "---------------");
terminal.errorPrintln(Verbosity.NORMAL, "");
if (requestedClusterActions.isEmpty()) {
terminal.errorPrintln(Verbosity.NORMAL, "None");
} else {
for (String clusterAction : requestedClusterActions) {
terminal.errorPrintln(Verbosity.NORMAL, "* " + clusterAction);
}
}
terminal.errorPrintln(Verbosity.NORMAL, "");
terminal.errorPrintln(Verbosity.NORMAL, "Index Actions");
terminal.errorPrintln(Verbosity.NORMAL, "-------------");
terminal.errorPrintln(Verbosity.NORMAL, "");
if (requestedIndexActions.isEmpty()) {
terminal.errorPrintln(Verbosity.NORMAL, "None");
} else {
for (Map.Entry<String, List<String>> entry : requestedIndexActions.entrySet()) {
terminal.errorPrintln(Verbosity.NORMAL, "Index Pattern: " + entry.getKey());
terminal.errorPrintln(Verbosity.NORMAL, "");
for (String indexAction : entry.getValue()) {
terminal.errorPrintln(Verbosity.NORMAL, "* " + indexAction);
}
}
}
}

prompt(terminal, batch);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,11 @@ static String createPluginUrl(String name, Path structure, String... additionalP
return createPlugin(name, structure, additionalProps).toUri().toURL().toString();
}

/** creates a plugin .zip and returns the url for testing */
static String createPluginWithRequestedActionsUrl(String name, Path structure, String... additionalProps) throws IOException {
return createPluginWithRequestedActions(name, structure, additionalProps).toUri().toURL().toString();
}

static void writePlugin(String name, Path structure, String... additionalProps) throws IOException {
String[] properties = Stream.concat(
Stream.of(
Expand All @@ -289,6 +294,31 @@ static void writePlugin(String name, Path structure, String... additionalProps)
writeJar(structure.resolve("plugin.jar"), className);
}

static void writePluginWithRequestedActions(String name, Path structure, String... additionalProps) throws IOException {
String[] properties = Stream.concat(
Stream.of(
"description",
"fake desc",
"name",
name,
"version",
"1.0",
"opensearch.version",
Version.CURRENT.toString(),
"java.version",
System.getProperty("java.specification.version"),
"classname",
"FakePlugin"
),
Arrays.stream(additionalProps)
).toArray(String[]::new);

PluginTestUtil.writePluginProperties(structure, properties);
writePluginPermissionsYaml(structure);
String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin";
writeJar(structure.resolve("plugin.jar"), className);
}

static void writePlugin(String name, Path structure, SemverRange opensearchVersionRange, String... additionalProps) throws IOException {
String[] properties = Stream.concat(
Stream.of(
Expand Down Expand Up @@ -329,11 +359,25 @@ static void writePluginSecurityPolicy(Path pluginDir, String... permissions) thr
Files.write(pluginDir.resolve("plugin-security.policy"), securityPolicyContent.toString().getBytes(StandardCharsets.UTF_8));
}

static void writePluginPermissionsYaml(Path pluginDir, String... permissions) throws IOException {
String permissionsYamlContent = "cluster.actions:\n"
+ " - cluster:monitor/health\n"
+ "indices.actions:\n"
+ " example-index*:\n"
+ " - indices:data/write/index*";
Files.write(pluginDir.resolve("plugin-permissions.yml"), permissionsYamlContent.getBytes(StandardCharsets.UTF_8));
}

static Path createPlugin(String name, Path structure, String... additionalProps) throws IOException {
writePlugin(name, structure, additionalProps);
return writeZip(structure, null);
}

static Path createPluginWithRequestedActions(String name, Path structure, String... additionalProps) throws IOException {
writePluginWithRequestedActions(name, structure, additionalProps);
return writeZip(structure, null);
}

void installPlugin(String pluginUrl, Path home) throws Exception {
installPlugin(pluginUrl, home, skipJarHellCommand);
}
Expand Down Expand Up @@ -1410,43 +1454,37 @@ private String signature(final byte[] bytes, final PGPSecretKey secretKey) {
// checks the plugin requires a policy confirmation, and does not install when that is rejected by the user
// the plugin is installed after this method completes
private void assertPolicyConfirmation(Tuple<Path, Environment> env, String pluginZip, String... warnings) throws Exception {
for (int i = 0; i < warnings.length; ++i) {
String warning = warnings[i];
for (int j = 0; j < i; ++j) {
terminal.addTextInput("y"); // accept warnings we have already tested
}
// default answer, does not install
terminal.addTextInput("");
UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1()));
assertEquals("installation aborted by user", e.getMessage());

assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning));
try (Stream<Path> fileStream = Files.list(env.v2().pluginsDir())) {
assertThat(fileStream.collect(Collectors.toList()), empty());
}
// default answer, does not install
terminal.addTextInput("");
UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1()));
assertEquals("installation aborted by user", e.getMessage());

// explicitly do not install
terminal.reset();
for (int j = 0; j < i; ++j) {
terminal.addTextInput("y"); // accept warnings we have already tested
}
terminal.addTextInput("n");
e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1()));
assertEquals("installation aborted by user", e.getMessage());
assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning));
try (Stream<Path> fileStream = Files.list(env.v2().pluginsDir())) {
assertThat(fileStream.collect(Collectors.toList()), empty());
}
try (Stream<Path> fileStream = Files.list(env.v2().pluginsDir())) {
assertThat(fileStream.collect(Collectors.toList()), empty());
}

// allow installation
for (String warning : warnings) {
assertThat(terminal.getErrorOutput(), containsString(warning));
}

// explicitly do not install
terminal.reset();
for (int j = 0; j < warnings.length; ++j) {
terminal.addTextInput("y");
terminal.addTextInput("n");
e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1()));
assertEquals("installation aborted by user", e.getMessage());
try (Stream<Path> fileStream = Files.list(env.v2().pluginsDir())) {
assertThat(fileStream.collect(Collectors.toList()), empty());
}
for (String warning : warnings) {
assertThat(terminal.getErrorOutput(), containsString(warning));
}

// allow installation
terminal.reset();
terminal.addTextInput("y");
installPlugin(pluginZip, env.v1());
for (String warning : warnings) {
assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning));
assertThat(terminal.getErrorOutput(), containsString(warning));
}
}

Expand All @@ -1456,7 +1494,16 @@ public void testPolicyConfirmation() throws Exception {
writePluginSecurityPolicy(pluginDir, "setAccessible", "setFactory");
String pluginZip = createPluginUrl("fake", pluginDir);

assertPolicyConfirmation(env, pluginZip, "plugin requires additional permissions");
assertPolicyConfirmation(env, pluginZip, "WARNING: plugin requires additional permissions");
assertPlugin("fake", pluginDir, env.v2());
}

public void testRequestedActionsConfirmation() throws Exception {
Tuple<Path, Environment> env = createEnv(fs, temp);
Path pluginDir = createPluginDir(temp);
String pluginZip = createPluginWithRequestedActionsUrl("fake", pluginDir);

assertPolicyConfirmation(env, pluginZip, "WARNING: plugin requires additional permissions", "Cluster Actions", "Index Actions");
assertPlugin("fake", pluginDir, env.v2());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.opensearch.plugins.ActionPlugin;
import org.opensearch.plugins.IdentityPlugin;
import org.opensearch.plugins.Plugin;
import org.opensearch.plugins.PluginInfo;
import org.opensearch.repositories.RepositoriesService;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestChannel;
Expand Down Expand Up @@ -138,7 +139,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
}
}

public PluginSubject getPluginSubject(Plugin plugin) {
@Override
public PluginSubject getPluginSubject(PluginInfo pluginInfo) {
return new ShiroPluginSubject(threadPool);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
import org.apache.logging.log4j.Logger;
import org.opensearch.OpenSearchException;
import org.opensearch.common.annotation.InternalApi;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.settings.Settings;
import org.opensearch.identity.noop.NoopIdentityPlugin;
import org.opensearch.identity.tokens.TokenManager;
import org.opensearch.plugins.IdentityAwarePlugin;
import org.opensearch.plugins.IdentityPlugin;
import org.opensearch.plugins.Plugin;
import org.opensearch.plugins.PluginInfo;
import org.opensearch.threadpool.ThreadPool;

import java.util.List;
Expand Down Expand Up @@ -63,11 +65,11 @@ public TokenManager getTokenManager() {
return identityPlugin.getTokenManager();
}

public void initializeIdentityAwarePlugins(final List<IdentityAwarePlugin> identityAwarePlugins) {
public void initializeIdentityAwarePlugins(final List<Tuple<PluginInfo, Plugin>> identityAwarePlugins) {
if (identityAwarePlugins != null) {
for (IdentityAwarePlugin plugin : identityAwarePlugins) {
PluginSubject pluginSubject = identityPlugin.getPluginSubject((Plugin) plugin);
plugin.assignSubject(pluginSubject);
for (Tuple<PluginInfo, Plugin> pluginTuple : identityAwarePlugins) {
PluginSubject pluginSubject = identityPlugin.getPluginSubject(pluginTuple.v1());
((IdentityAwarePlugin) pluginTuple.v2()).assignSubject(pluginSubject);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import org.opensearch.identity.Subject;
import org.opensearch.identity.tokens.TokenManager;
import org.opensearch.plugins.IdentityPlugin;
import org.opensearch.plugins.Plugin;
import org.opensearch.plugins.PluginInfo;
import org.opensearch.threadpool.ThreadPool;

/**
Expand Down Expand Up @@ -49,7 +49,7 @@ public TokenManager getTokenManager() {
}

@Override
public PluginSubject getPluginSubject(Plugin plugin) {
public PluginSubject getPluginSubject(PluginInfo pluginInfo) {
return new NoopPluginSubject(threadPool);
}
}
Loading
Loading