Implement on behalf of token passing for extensions (opensearch-proje…

…ct#8679) Implement on behalf of token passing for extensions Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Ryan Liang <[email protected]> Co-authored-by: Ryan Liang <[email protected]> Co-authored-by: Peter Nied <[email protected]>
baba-devv · Aug 11, 2023 · 487e3e3 · 487e3e3
1 parent c73f727
commit 487e3e3
Show file tree

Hide file tree

Showing 20 changed files with 349 additions and 83 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Allow mmap to use new JDK-19 preview APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
 - Add events correlation engine plugin ([#6854](https://github.com/opensearch-project/OpenSearch/issues/6854))
 - Introduce new dynamic cluster setting to control slice computation for concurrent segment search ([#9107](https://github.com/opensearch-project/OpenSearch/pull/9107))
+- Implement on behalf of token passing for extensions ([#8679](https://github.com/opensearch-project/OpenSearch/pull/8679))
 
 ### Dependencies
 - Bump `log4j-core` from 2.18.0 to 2.19.0

diff --git a/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java b/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java
@@ -8,15 +8,15 @@
 
 package org.opensearch.identity.shiro;
 
-import org.opensearch.identity.Subject;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.shiro.SecurityUtils;
+import org.apache.shiro.mgt.SecurityManager;
+import org.opensearch.common.settings.Settings;
+import org.opensearch.identity.Subject;
 import org.opensearch.identity.tokens.TokenManager;
 import org.opensearch.plugins.IdentityPlugin;
-import org.opensearch.common.settings.Settings;
 import org.opensearch.plugins.Plugin;
-import org.apache.shiro.SecurityUtils;
-import org.apache.shiro.mgt.SecurityManager;
 
 /**
  * Identity implementation with Shiro

diff --git a/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroTokenManager.java b/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroTokenManager.java
@@ -22,8 +22,11 @@
 import org.apache.shiro.authc.UsernamePasswordToken;
 import org.opensearch.common.Randomness;
 import org.opensearch.identity.IdentityService;
+import org.opensearch.identity.Subject;
+import org.opensearch.identity.noop.NoopSubject;
 import org.opensearch.identity.tokens.AuthToken;
 import org.opensearch.identity.tokens.BasicAuthToken;
+import org.opensearch.identity.tokens.OnBehalfOfClaims;
 import org.opensearch.identity.tokens.TokenManager;
 import org.passay.CharacterRule;
 import org.passay.EnglishCharacterData;
@@ -51,15 +54,16 @@ public Optional<AuthenticationToken> translateAuthToken(org.opensearch.identity.
             final BasicAuthToken basicAuthToken = (BasicAuthToken) authenticationToken;
             return Optional.of(new UsernamePasswordToken(basicAuthToken.getUser(), basicAuthToken.getPassword()));
         }
-
         return Optional.empty();
     }
 
     @Override
-    public AuthToken issueToken(String audience) {
+    public AuthToken issueOnBehalfOfToken(Subject subject, OnBehalfOfClaims claims) {
 
         String password = generatePassword();
-        final byte[] rawEncoded = Base64.getEncoder().encode((audience + ":" + password).getBytes(UTF_8));
+        final byte[] rawEncoded = Base64.getEncoder().encode((claims.getAudience() + ":" + password).getBytes(UTF_8)); // Make a new
+                                                                                                                       // ShiroSubject w/
+                                                                                                                       // audience as name
         final String usernamePassword = new String(rawEncoded, UTF_8);
         final String header = "Basic " + usernamePassword;
         BasicAuthToken token = new BasicAuthToken(header);
@@ -68,6 +72,11 @@ public AuthToken issueToken(String audience) {
         return token;
     }
 
+    @Override
+    public Subject authenticateToken(AuthToken authToken) {
+        return new NoopSubject();
+    }
+
     public boolean validateToken(AuthToken token) {
         if (token instanceof BasicAuthToken) {
             final BasicAuthToken basicAuthToken = (BasicAuthToken) token;

diff --git a/...ins/identity-shiro/src/test/java/org/opensearch/identity/shiro/AuthTokenHandlerTests.java b/...ins/identity-shiro/src/test/java/org/opensearch/identity/shiro/AuthTokenHandlerTests.java
@@ -12,10 +12,13 @@
 import org.apache.shiro.authc.AuthenticationToken;
 import org.apache.shiro.authc.UsernamePasswordToken;
 import org.junit.Before;
+import org.opensearch.identity.Subject;
+import org.opensearch.identity.noop.NoopSubject;
 import org.opensearch.identity.noop.NoopTokenManager;
 import org.opensearch.identity.tokens.AuthToken;
 import org.opensearch.identity.tokens.BasicAuthToken;
 import org.opensearch.identity.tokens.BearerAuthToken;
+import org.opensearch.identity.tokens.OnBehalfOfClaims;
 import org.opensearch.test.OpenSearchTestCase;
 import org.passay.CharacterCharacteristicsRule;
 import org.passay.CharacterRule;
@@ -31,16 +34,15 @@
 public class AuthTokenHandlerTests extends OpenSearchTestCase {
 
     private ShiroTokenManager shiroAuthTokenHandler;
-    private NoopTokenManager noopTokenManager;
 
     @Before
     public void testSetup() {
         shiroAuthTokenHandler = new ShiroTokenManager();
-        noopTokenManager = new NoopTokenManager();
     }
 
     public void testShouldExtractBasicAuthTokenSuccessfully() {
         final BasicAuthToken authToken = new BasicAuthToken("Basic YWRtaW46YWRtaW4="); // admin:admin
+        assertEquals(authToken.asAuthHeaderValue(), "YWRtaW46YWRtaW4=");
 
         final AuthenticationToken translatedToken = shiroAuthTokenHandler.translateAuthToken(authToken).get();
         assertThat(translatedToken, is(instanceOf(UsernamePasswordToken.class)));
@@ -106,7 +108,7 @@ public void testShoudPassMapLookupWithToken() {
         assertTrue(authToken.getPassword().equals(shiroAuthTokenHandler.getShiroTokenPasswordMap().get(authToken)));
     }
 
-    public void testShouldPassThrougbResetToken(AuthToken token) {
+    public void testShouldPassThroughResetToken() {
         final BearerAuthToken bearerAuthToken = new BearerAuthToken("header.payload.signature");
         shiroAuthTokenHandler.resetToken(bearerAuthToken);
     }
@@ -121,6 +123,7 @@ public void testVerifyBearerTokenObject() {
         assertEquals(testGoodToken.getPayload(), "payload");
         assertEquals(testGoodToken.getSignature(), "signature");
         assertEquals(testGoodToken.toString(), "Bearer auth token with header=header, payload=payload, signature=signature");
+        assertEquals(testGoodToken.asAuthHeaderValue(), "header.payload.signature");
     }
 
     public void testGeneratedPasswordContents() {
@@ -144,4 +147,23 @@ public void testGeneratedPasswordContents() {
         validator.validate(data);
     }
 
+    public void testIssueOnBehalfOfTokenFromClaims() {
+        Subject subject = new NoopSubject();
+        OnBehalfOfClaims claims = new OnBehalfOfClaims("test", "test");
+        BasicAuthToken authToken = (BasicAuthToken) shiroAuthTokenHandler.issueOnBehalfOfToken(subject, claims);
+        assertTrue(authToken instanceof BasicAuthToken);
+        UsernamePasswordToken translatedToken = (UsernamePasswordToken) shiroAuthTokenHandler.translateAuthToken(authToken).get();
+        assertEquals(authToken.getPassword(), new String(translatedToken.getPassword()));
+        assertTrue(shiroAuthTokenHandler.getShiroTokenPasswordMap().containsKey(authToken));
+        assertEquals(shiroAuthTokenHandler.getShiroTokenPasswordMap().get(authToken), new String(translatedToken.getPassword()));
+    }
+
+    public void testTokenNoopIssuance() {
+        NoopTokenManager tokenManager = new NoopTokenManager();
+        OnBehalfOfClaims claims = new OnBehalfOfClaims("test", "test");
+        Subject subject = new NoopSubject();
+        AuthToken token = tokenManager.issueOnBehalfOfToken(subject, claims);
+        assertTrue(token instanceof AuthToken);
+    }
+
 }
diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java
@@ -49,6 +49,7 @@
 import org.opensearch.extensions.rest.RestActionsRequestHandler;
 import org.opensearch.extensions.settings.CustomSettingsRequestHandler;
 import org.opensearch.extensions.settings.RegisterCustomSettingsRequest;
+import org.opensearch.identity.IdentityService;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.ConnectTransportException;
 import org.opensearch.transport.TransportException;
@@ -142,9 +143,15 @@ public void initializeServicesAndRestHandler(
         TransportService transportService,
         ClusterService clusterService,
         Settings initialEnvironmentSettings,
-        NodeClient client
+        NodeClient client,
+        IdentityService identityService
     ) {
-        this.restActionsRequestHandler = new RestActionsRequestHandler(actionModule.getRestController(), extensionIdMap, transportService);
+        this.restActionsRequestHandler = new RestActionsRequestHandler(
+            actionModule.getRestController(),
+            extensionIdMap,
+            transportService,
+            identityService
+        );
         this.customSettingsRequestHandler = new CustomSettingsRequestHandler(settingsModule);
         this.transportService = transportService;
         this.clusterService = clusterService;

diff --git a/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java
@@ -21,6 +21,7 @@
 import org.opensearch.extensions.action.ExtensionActionRequest;
 import org.opensearch.extensions.action.ExtensionActionResponse;
 import org.opensearch.extensions.action.RemoteExtensionActionResponse;
+import org.opensearch.identity.IdentityService;
 import org.opensearch.transport.TransportService;
 
 /**
@@ -41,7 +42,8 @@ public void initializeServicesAndRestHandler(
         TransportService transportService,
         ClusterService clusterService,
         Settings initialEnvironmentSettings,
-        NodeClient client
+        NodeClient client,
+        IdentityService identityService
     ) {
         // no-op
     }

diff --git a/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java b/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java
@@ -30,6 +30,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import static java.util.Objects.requireNonNull;
 
 /**
  * Request to execute REST actions on extension node.
@@ -86,7 +87,7 @@ public ExtensionRestRequest(
         this.headers = headers;
         this.mediaType = mediaType;
         this.content = content;
-        this.principalIdentifierToken = principalIdentifier;
+        this.principalIdentifierToken = requireNonNull(principalIdentifier);
         this.httpVersion = httpVersion;
     }
 
@@ -280,7 +281,7 @@ public boolean isContentConsumed() {
     }
 
     /**
-     * Gets a parser for the contents of this request if there is content and an xContentType.
+     * Gets a parser for the contents of this request if there is content, an xContentType, and a principal identifier.
      *
      * @param xContentRegistry The extension's xContentRegistry
      * @return A parser for the given content and content type.
@@ -291,6 +292,9 @@ public final XContentParser contentParser(NamedXContentRegistry xContentRegistry
         if (!hasContent() || getXContentType() == null) {
             throw new OpenSearchParseException("There is no request body or the ContentType is invalid.");
         }
+        if (getRequestIssuerIdentity() == null) {
+            throw new OpenSearchParseException("There is no request body or the requester identity is invalid.");
+        }
         return getXContentType().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput());
     }
 

diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java b/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java
@@ -8,16 +8,16 @@
 
 package org.opensearch.extensions.rest;
 
+import java.util.Map;
 import org.opensearch.action.ActionModule.DynamicActionRegistry;
 import org.opensearch.extensions.AcknowledgedResponse;
 import org.opensearch.extensions.DiscoveryExtensionNode;
+import org.opensearch.identity.IdentityService;
 import org.opensearch.rest.RestController;
 import org.opensearch.rest.RestHandler;
 import org.opensearch.core.transport.TransportResponse;
 import org.opensearch.transport.TransportService;
 
-import java.util.Map;
-
 /**
  * Handles requests to register extension REST actions.
  *
@@ -28,6 +28,7 @@ public class RestActionsRequestHandler {
     private final RestController restController;
     private final Map<String, DiscoveryExtensionNode> extensionIdMap;
     private final TransportService transportService;
+    private final IdentityService identityService;
 
     /**
      * Instantiates a new REST Actions Request Handler using the Node's RestController.
@@ -39,11 +40,13 @@ public class RestActionsRequestHandler {
     public RestActionsRequestHandler(
         RestController restController,
         Map<String, DiscoveryExtensionNode> extensionIdMap,
-        TransportService transportService
+        TransportService transportService,
+        IdentityService identityService
     ) {
         this.restController = restController;
         this.extensionIdMap = extensionIdMap;
         this.transportService = transportService;
+        this.identityService = identityService;
     }
 
     /**
@@ -62,7 +65,8 @@ public TransportResponse handleRegisterRestActionsRequest(
             restActionsRequest,
             discoveryExtensionNode,
             transportService,
-            dynamicActionRegistry
+            dynamicActionRegistry,
+            identityService
         );
         restController.registerHandler(handler);
         return new AcknowledgedResponse(true);

diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java
@@ -17,6 +17,10 @@
 import org.opensearch.core.xcontent.MediaType;
 import org.opensearch.extensions.DiscoveryExtensionNode;
 import org.opensearch.extensions.ExtensionsManager;
+import org.opensearch.identity.IdentityService;
+import org.opensearch.identity.Subject;
+import org.opensearch.identity.tokens.OnBehalfOfClaims;
+import org.opensearch.identity.tokens.TokenManager;
 import org.opensearch.rest.BaseRestHandler;
 import org.opensearch.rest.BytesRestResponse;
 import org.opensearch.rest.NamedRoute;
@@ -31,7 +35,6 @@
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
-import java.security.Principal;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -54,19 +57,13 @@ public class RestSendToExtensionAction extends BaseRestHandler {
 
     private static final String SEND_TO_EXTENSION_ACTION = "send_to_extension_action";
     private static final Logger logger = LogManager.getLogger(RestSendToExtensionAction.class);
-    // To replace with user identity see https://github.com/opensearch-project/OpenSearch/pull/4247
-    private static final Principal DEFAULT_PRINCIPAL = new Principal() {
-        @Override
-        public String getName() {
-            return "OpenSearchUser";
-        }
-    };
 
     private final List<Route> routes;
     private final List<DeprecatedRoute> deprecatedRoutes;
     private final String pathPrefix;
     private final DiscoveryExtensionNode discoveryExtensionNode;
     private final TransportService transportService;
+    private final IdentityService identityService;
 
     private static final Set<String> allowList = Set.of("Content-Type");
     private static final Set<String> denyList = Set.of("Authorization", "Proxy-Authorization");
@@ -82,7 +79,8 @@ public RestSendToExtensionAction(
         RegisterRestActionsRequest restActionsRequest,
         DiscoveryExtensionNode discoveryExtensionNode,
         TransportService transportService,
-        DynamicActionRegistry dynamicActionRegistry
+        DynamicActionRegistry dynamicActionRegistry,
+        IdentityService identityService
     ) {
         this.pathPrefix = "/_extensions/_" + restActionsRequest.getUniqueId();
         RestRequest.Method method;
@@ -147,6 +145,7 @@ public RestSendToExtensionAction(
 
         this.discoveryExtensionNode = discoveryExtensionNode;
         this.transportService = transportService;
+        this.identityService = identityService;
     }
 
     @Override
@@ -240,12 +239,15 @@ public String executor() {
         };
 
         try {
+
             // Will be replaced with ExtensionTokenProcessor and PrincipalIdentifierToken classes from feature/identity
-            final String extensionTokenProcessor = "placeholder_token_processor";
-            final String requestIssuerIdentity = "placeholder_request_issuer_identity";
 
             Map<String, List<String>> filteredHeaders = filterHeaders(headers, allowList, denyList);
 
+            TokenManager tokenManager = identityService.getTokenManager();
+            Subject subject = this.identityService.getSubject();
+            OnBehalfOfClaims claims = new OnBehalfOfClaims(discoveryExtensionNode.getId(), subject.getPrincipal().getName());
+
             transportService.sendRequest(
                 discoveryExtensionNode,
                 ExtensionsManager.REQUEST_REST_EXECUTE_ON_EXTENSION_ACTION,
@@ -259,7 +261,7 @@ public String executor() {
                     filteredHeaders,
                     contentType,
                     content,
-                    requestIssuerIdentity,
+                    tokenManager.issueOnBehalfOfToken(subject, claims).asAuthHeaderValue(),
                     httpVersion
                 ),
                 restExecuteOnExtensionResponseHandler

diff --git a/server/src/main/java/org/opensearch/identity/noop/NoopIdentityPlugin.java b/server/src/main/java/org/opensearch/identity/noop/NoopIdentityPlugin.java
@@ -8,9 +8,9 @@
 
 package org.opensearch.identity.noop;
 
+import org.opensearch.identity.Subject;
 import org.opensearch.identity.tokens.TokenManager;
 import org.opensearch.plugins.IdentityPlugin;
-import org.opensearch.identity.Subject;
 
 /**
  * Implementation of identity plugin that does not enforce authentication or authorization