Skip to content

Commit

Permalink
Clean up subject creep
Browse files Browse the repository at this point in the history
Signed-off-by: Peter Nied <petern@amazon.com>
  • Loading branch information
peternied committed Jul 17, 2023
1 parent d8578f7 commit 49d5290
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 133 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@
package org.opensearch.extensions;

import java.io.IOException;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import org.opensearch.Application;
import org.opensearch.OpenSearchException;
Expand All @@ -28,21 +26,19 @@
import org.opensearch.core.common.io.stream.Writeable;
import org.opensearch.core.xcontent.ToXContentFragment;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.identity.Subject;
import org.opensearch.identity.scopes.Scope;
import org.opensearch.identity.tokens.AuthToken;

/**
* Discover extensions running independently or in a separate process
*
* @opensearch.internal
*/
public class DiscoveryExtensionNode extends DiscoveryNode implements Writeable, ToXContentFragment, Subject, Application {
public class DiscoveryExtensionNode extends DiscoveryNode implements Writeable, ToXContentFragment, Application {

private Version minimumCompatibleVersion;
private List<ExtensionDependency> dependencies = Collections.emptyList();
private List<String> implementedInterfaces = Collections.emptyList();
private List<Scope> scopes = List.of();
private final List<Scope> scopes;

public DiscoveryExtensionNode(
String name,
Expand Down Expand Up @@ -145,14 +141,4 @@ private void validate() {
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return null;
}

@Override
public void authenticate(AuthToken token) {

}

@Override
public Optional<Principal> getApplication() {
return Optional.of(this.getPrincipal());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import java.io.IOException;
import java.net.InetAddress;
import java.security.Principal;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -50,7 +49,6 @@
import org.opensearch.extensions.rest.RestActionsRequestHandler;
import org.opensearch.extensions.settings.CustomSettingsRequestHandler;
import org.opensearch.extensions.settings.RegisterCustomSettingsRequest;
import org.opensearch.identity.scopes.Scope;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.ConnectTransportException;
import org.opensearch.transport.TransportException;
Expand Down Expand Up @@ -511,19 +509,4 @@ Settings getEnvironmentSettings() {
public Set<Setting<?>> getAdditionalSettings() {
return this.additionalSettings;
}

public Set<Scope> getScopes(Principal principal) {
if (this.getExtensionIdMap().containsKey(principal.getName())) {
return this.getExtensionIdMap().get(principal.getName()).getScopes();
}
return Set.of();
}

/** Checks whether there is an application associated with the given principal or not
* @param principal The principal for the application you are trying to find
* @return Whether the application exists (TRUE) or not (FALSE)
* */
public boolean applicationExists(Principal principal) {
return (this.getExtensionIdMap().containsKey(principal.getName()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import org.opensearch.extensions.ExtensionsManager;

import org.opensearch.identity.scopes.ApplicationScope;
import org.opensearch.identity.scopes.Scope;
import org.opensearch.identity.tokens.AuthToken;
Expand All @@ -33,24 +33,15 @@
public class ApplicationAwareSubject implements Subject {

private final Subject wrapped;
private final ExtensionsManager extensionsManager;
private final ApplicationManager applicationManager;

/**
* We wrap a basic Subject object to create an ApplicationAwareSubject -- this should come from the IdentityService
* @param wrapped The Subject to be wrapped
*/
public ApplicationAwareSubject(final Subject wrapped, ExtensionsManager extensionsManager) {
public ApplicationAwareSubject(final Subject wrapped, final ApplicationManager applicationManager) {
this.wrapped = wrapped;
this.extensionsManager = extensionsManager;
}

/**
* Use the ApplicationManager to get the scopes associated with the principal of the wrapped Subject.
* Because the wrapped subject is just a basic Subject, it may not know its own scopes. This circumvents this issue.
* @return A set of Strings representing the scopes associated with the wrapped subject's principal
*/
public Set<Scope> getScopes() {
return extensionsManager.getScopes(wrapped.getPrincipal());
this.applicationManager = applicationManager;
}

/**
Expand All @@ -60,36 +51,29 @@ public Set<Scope> getScopes() {
*/
public boolean isAllowed(final List<Scope> scopes) {

final Optional<Principal> optionalPrincipal = this.getApplication();

if (optionalPrincipal.isEmpty()) {
final Optional<Principal> application = this.getApplication();
if (application.isEmpty()) {
// If there is no application, actions are allowed by default

return true;
}

if (!extensionsManager.applicationExists(this.getPrincipal())) {

final Optional<Set<Scope>> scopesOfApplication = applicationManager.getScopes(application.get());
if (scopesOfApplication.isEmpty()) {
// If no matching application was found, actions are denied by default
return false;
}

final Set<Scope> scopesOfApplication = this.getScopes();

boolean isApplicationSuperUser = scopesOfApplication.contains(ApplicationScope.SUPER_USER_ACCESS);

final boolean isApplicationSuperUser = scopesOfApplication.get().contains(ApplicationScope.SUPER_USER_ACCESS);
if (isApplicationSuperUser) {

return true;
}

Set<Scope> intersection = new HashSet<>(scopesOfApplication);

// Retain only the elements present in the list
intersection.retainAll(scopes);

boolean isMatchingScopePresent = !intersection.isEmpty();
final Set<Scope> scopesCopy = new HashSet<>(scopesOfApplication.get());
scopesCopy.retainAll(scopes);

return isMatchingScopePresent;
final boolean hasMatchingScopes = !scopesCopy.isEmpty();
return hasMatchingScopes;
}

// Passthroughs for wrapped subject
Expand All @@ -114,7 +98,7 @@ public boolean equals(Object obj) {
if (!(obj instanceof ApplicationAwareSubject)) {
return false;
}
ApplicationAwareSubject other = (ApplicationAwareSubject) obj;
return Objects.equals(this.getScopes(), other.getScopes());
final ApplicationAwareSubject other = (ApplicationAwareSubject) obj;
return Objects.equals(this.wrapped, other.wrapped);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.identity;

import java.security.Principal;
import java.util.concurrent.atomic.AtomicReference;
import java.util.Optional;
import java.util.Set;
import org.opensearch.extensions.ExtensionsManager;
import org.opensearch.identity.scopes.Scope;

/**
* The ApplicationManager class handles the processing and resolution of multiple types of applications. Using the class, OpenSearch can
* continue to resolve requests even when specific application types are disabled. For example, the ExtensionManager can be Noop in which case
* the ApplicationManager is able to resolve requests for other application types still
*
* @opensearch.experimental
*/
public class ApplicationManager {

private final AtomicReference<ExtensionsManager> extensionManager = new AtomicReference<ExtensionsManager>();

public void register(final ExtensionsManager manager) {
// Only allow the extension manager to be registered the first time
extensionManager.compareAndSet(null, manager);
}

public Optional<Set<Scope>> getScopes(final Principal applicationPrincipal) {
return findExtension(applicationPrincipal);
}

private Optional<Set<Scope>> findExtension(final Principal applicationPrincipal) {
if (extensionManager.get().getExtensionIdMap().containsKey(applicationPrincipal.getName())) {
return Optional.of(extensionManager.get().getExtensionIdMap().get(applicationPrincipal.getName()).getScopes());
}
return Optional.empty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.apache.logging.log4j.Logger;
import org.opensearch.OpenSearchException;
import org.opensearch.common.settings.Settings;
import org.opensearch.extensions.ExtensionsManager;
import org.opensearch.identity.noop.NoopIdentityPlugin;
import org.opensearch.identity.tokens.TokenManager;
import org.opensearch.plugins.IdentityPlugin;
Expand All @@ -26,11 +25,15 @@ public class IdentityService {

private final Settings settings;
private final IdentityPlugin identityPlugin;
private final ExtensionsManager extensionsManager;
private final ApplicationManager applicationManager;

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

if (identityPlugins.size() == 0) {
log.debug("Identity plugins size is 0");
Expand All @@ -50,7 +53,7 @@ public IdentityService(ExtensionsManager extensionsManager, final Settings setti
* Gets the current Subject
*/
public ApplicationAwareSubject getSubject() {
return new ApplicationAwareSubject(identityPlugin.getSubject(), extensionsManager);
return new ApplicationAwareSubject(identityPlugin.getSubject(), applicationManager);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions server/src/main/java/org/opensearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
import org.opensearch.bootstrap.BootstrapContext;
import org.opensearch.client.Client;
import org.opensearch.client.node.NodeClient;
import org.opensearch.identity.ApplicationManager;
import org.opensearch.cluster.ClusterInfoService;
import org.opensearch.cluster.ClusterModule;
import org.opensearch.cluster.ClusterName;
Expand Down Expand Up @@ -463,6 +464,7 @@ protected Node(
);

final Settings settings = pluginsService.updatedSettings();
final ApplicationManager applicationManager = new ApplicationManager();

// Ensure to initialize Feature Flags via the settings from opensearch.yml
FeatureFlags.initializeFeatureFlags(settings);
Expand All @@ -476,6 +478,7 @@ protected Node(
);
identityPlugins.addAll(pluginsService.filterPlugins(IdentityPlugin.class));
}
this.identityService = new IdentityService(settings, identityPlugins, applicationManager);

if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
final List<ExtensionAwarePlugin> extensionAwarePlugins = pluginsService.filterPlugins(ExtensionAwarePlugin.class);
Expand All @@ -487,8 +490,7 @@ protected Node(
} else {
this.extensionsManager = new NoopExtensionsManager();
}

this.identityService = new IdentityService(extensionsManager, settings, identityPlugins);
applicationManager.register(extensionsManager);

final Set<DiscoveryNodeRole> additionalRoles = pluginsService.filterPlugins(Plugin.class)
.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@

package org.opensearch.plugins.wrappers;

import java.security.Principal;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;
import org.opensearch.OpenSearchException;
Expand All @@ -54,9 +52,7 @@
import org.opensearch.common.settings.SettingsFilter;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.identity.IdentityService;
import org.opensearch.identity.Subject;
import org.opensearch.identity.scopes.ExtensionPointScope;
import org.opensearch.identity.tokens.AuthToken;
import org.opensearch.plugins.ActionPlugin;
import org.opensearch.rest.RestController;
import org.opensearch.rest.RestHandler;
Expand All @@ -67,7 +63,7 @@
*
* @opensearch.experimental
*/
public class ScopeProtectedActionPlugin implements ActionPlugin, Subject {
public class ScopeProtectedActionPlugin implements ActionPlugin {
private final ActionPlugin plugin;
private final IdentityService identity;

Expand Down Expand Up @@ -151,18 +147,4 @@ public Collection<RequestValidators.RequestValidator<IndicesAliasesRequest>> ind
checkIfAllowed();
return plugin.indicesAliasesRequestValidators();
}

// Implement to have access to identity methods
@Override
public Principal getPrincipal() {
return identity.getSubject().getPrincipal();
}

@Override
public void authenticate(AuthToken token) {}

@Override
public Optional<Principal> getApplication() {
return Optional.of(identity.getSubject().getPrincipal());
}
}
Loading

0 comments on commit 49d5290

Please sign in to comment.