Skip to content

Commit

Permalink
Remove how extensions identiity is related to scope access
Browse files Browse the repository at this point in the history
Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
peternied committed May 15, 2023
1 parent 1baee4f commit 2f24ada
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 132 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,4 @@ public void authenticate(AuthToken authenticationToken) {
.orElseThrow(() -> new UnsupportedAuthenticationToken());
shiroSubject.login(authToken);
}

@Override
public Principal getAssociatedApplication() {
return null;
}

@Override
public boolean isAllowed(final List<Scope> scope) {
// Noop subject is always allowed for any scope checks
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,6 @@ public void onIndexModule(IndexModule indexModule) throws UnknownHostException {

private void onIndexModule(IndexModule indexModule, DiscoveryNode extensionNode) throws UnknownHostException {

// Authenticate subject as extension
// check if allowed

logger.info("onIndexModule index:" + indexModule.getIndex());
final CompletableFuture<IndicesModuleResponse> inProgressFuture = new CompletableFuture<>();
final CompletableFuture<AcknowledgedResponse> inProgressIndexNameFuture = new CompletableFuture<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ void registerAction(String action, String uniqueId) throws IllegalArgumentExcept
if (actionToIdMap.putIfAbsent(action, uniqueId) != null) {
throw new IllegalArgumentException("The action [" + action + "] you are trying to register is already registered");
}

// Register the action in the action module's dynamic actions map
dynamicActionRegistry.registerDynamicAction(
new ExtensionAction(uniqueId, action),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
import org.apache.logging.log4j.Logger;
import org.opensearch.OpenSearchException;
import org.opensearch.identity.noop.NoopIdentityPlugin;
import org.opensearch.extensions.ExtensionsManager;
import java.util.List;

import org.opensearch.common.settings.Settings;
import org.opensearch.plugins.IdentityPlugin;
import java.util.stream.Collectors;
Expand All @@ -26,16 +24,12 @@ public class IdentityService {

private final Settings settings;
private final IdentityPlugin identityPlugin;
private final ExtensionsManager extensionManager;

private static IdentityService instance;
public static IdentityService getInstance() { return instance; }

public IdentityService(final Settings settings,
final List<IdentityPlugin> identityPlugins,
final ExtensionsManager extensionManager) {
public IdentityService(final Settings settings, final List<IdentityPlugin> identityPlugins) {
this.settings = settings;
this.extensionManager = extensionManager;

if (identityPlugins.size() == 0) {
log.debug("Identity plugins size is 0");
Expand Down
14 changes: 6 additions & 8 deletions server/src/main/java/org/opensearch/identity/Subject.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,16 @@ public interface Subject {
*/
void authenticate(final AuthToken token);


/**
* Get the application-wide uniquely identifying associated application's principal
* @return Null, if there is not associated application
*/
@Nullable
Principal getAssociatedApplication();

/**
* Checks scopes of the current subject if they are allowed for any of the listed scopes
* @param scope The scopes to check against the subject
* @return true if allowed, false if none of the scopes are allowed.
*/
/// Draft Pull Request Remarks
// Permissions haven't been implemented yet, and there are good reasons to have permissions and scopes overlap,
// as well as have disconnected. For the moment lets look past that debate and get feedback around how
// scope might be added inside of OpenSearch and connected into various systems create security barriers between
// systems.
// This will need to be addressed before this change can come out of draft
boolean isAllowed(final List<Scope> scope);
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@ public void authenticate(AuthToken AuthToken) {
// Do nothing as noop subject is always logged in
}

@Override
public Principal getAssociatedApplication() {
// Noop subject is not aware of an application
return null;
}

@Override
public boolean isAllowed(final List<Scope> scope) {
// Noop subject is always allowed for any scope checks
Expand Down
15 changes: 8 additions & 7 deletions server/src/main/java/org/opensearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -457,19 +457,20 @@ protected Node(
// Ensure to initialize Feature Flags via the settings from opensearch.yml
FeatureFlags.initializeFeatureFlags(settings);

if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
this.extensionsManager = new ExtensionsManager(initialEnvironment.extensionDir());
} else {
this.extensionsManager = new NoopExtensionsManager();
}

final List<IdentityPlugin> identityPlugins = new ArrayList<>();
if (FeatureFlags.isEnabled(FeatureFlags.IDENTITY)) {
// If identity is enabled load plugins implementing the extension point
logger.info("Identity on so found plugins implementing: " + pluginsService.filterPlugins(IdentityPlugin.class).toString());
identityPlugins.addAll(pluginsService.filterPlugins(IdentityPlugin.class));
}
final IdentityService identityService = new IdentityService(settings, identityPlugins, extensionsManager);

final IdentityService identityService = new IdentityService(settings, identityPlugins);

if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
this.extensionsManager = new ExtensionsManager(initialEnvironment.extensionDir());
} else {
this.extensionsManager = new NoopExtensionsManager();
}

final Set<DiscoveryNodeRole> additionalRoles = pluginsService.filterPlugins(Plugin.class)
.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,5 +153,4 @@ grant codeBase "file:${gradle.worker.jar}" {
grant {
// since the gradle test worker jar is on the test classpath, our tests should be able to read it
permission java.io.FilePermission "${gradle.worker.jar}", "read";
permission java.security.AllPermission;
};

This file was deleted.

0 comments on commit 2f24ada

Please sign in to comment.