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 <steecraw@amazon.com> Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Signed-off-by: Ryan Liang <jiallian@amazon.com> Co-authored-by: Ryan Liang <jiallian@amazon.com> Co-authored-by: Peter Nied <peternied@hotmail.com>
DarshitChanpura · Sep 28, 2023 · f40d280 · f40d280
1 parent 7e9a876
commit f40d280
Show file tree

Hide file tree

Showing 18 changed files with 355 additions and 66 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Implement Visitor Design pattern in QueryBuilder to enable the capability to traverse through the complex QueryBuilder tree. ([#10110](https://github.com/opensearch-project/OpenSearch/pull/10110))
 - Add capability to restrict async durability mode for remote indexes ([#10189](https://github.com/opensearch-project/OpenSearch/pull/10189))
 
+- Support for HTTP/2 (server-side) ([#3847](https://github.com/opensearch-project/OpenSearch/pull/3847))
+- Add getter for path field in NestedQueryBuilder ([#4636](https://github.com/opensearch-project/OpenSearch/pull/4636))
+- 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 JNA version from 5.5 to 5.13 ([#9963](https://github.com/opensearch-project/OpenSearch/pull/9963))
 - Bump `peter-evans/create-or-update-comment` from 2 to 3 ([#9575](https://github.com/opensearch-project/OpenSearch/pull/9575))

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
@@ -15,8 +15,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 java.util.Arrays;
@@ -54,15 +57,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);
@@ -71,6 +75,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
@@ -10,10 +10,14 @@
 
 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.junit.Before;
 
@@ -34,16 +38,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)));
@@ -109,7 +112,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);
     }
@@ -124,6 +127,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() {
@@ -147,4 +151,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
@@ -40,6 +40,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;
@@ -141,9 +142,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
@@ -16,6 +16,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;
 
 import java.io.IOException;
@@ -40,7 +41,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.core.transport.TransportResponse;
 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.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
@@ -19,6 +19,10 @@
 import org.opensearch.extensions.DiscoveryExtensionNode;
 import org.opensearch.extensions.ExtensionsManager;
 import org.opensearch.http.HttpRequest;
+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/NoopTokenManager.java b/server/src/main/java/org/opensearch/identity/noop/NoopTokenManager.java
@@ -11,7 +11,9 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.identity.IdentityService;
+import org.opensearch.identity.Subject;
 import org.opensearch.identity.tokens.AuthToken;
+import org.opensearch.identity.tokens.OnBehalfOfClaims;
 import org.opensearch.identity.tokens.TokenManager;
 
 /**
@@ -26,8 +28,17 @@ public class NoopTokenManager implements TokenManager {
      * @return a new Noop Token
      */
     @Override
-    public AuthToken issueToken(String audience) {
+    public AuthToken issueOnBehalfOfToken(final Subject subject, final OnBehalfOfClaims claims) {
         return new AuthToken() {
+            @Override
+            public String asAuthHeaderValue() {
+                return "noopToken";
+            }
         };
     }
+
+    @Override
+    public Subject authenticateToken(AuthToken authToken) {
+        return null;
+    }
 }
diff --git a/server/src/main/java/org/opensearch/identity/tokens/AuthToken.java b/server/src/main/java/org/opensearch/identity/tokens/AuthToken.java
@@ -15,4 +15,5 @@
  */
 public interface AuthToken {
 
+    String asAuthHeaderValue();
 }
diff --git a/server/src/main/java/org/opensearch/identity/tokens/BasicAuthToken.java b/server/src/main/java/org/opensearch/identity/tokens/BasicAuthToken.java
@@ -51,4 +51,13 @@ public void revoke() {
         this.password = "";
         this.user = "";
     }
+
+    @Override
+    public String asAuthHeaderValue() {
+        if (user == null || password == null) {
+            return null;
+        }
+        String usernamepassword = user + ":" + password;
+        return Base64.getEncoder().encodeToString(usernamepassword.getBytes(StandardCharsets.UTF_8));
+    }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -15,4 +15,5 @@ @@
      */
     public interface AuthToken {
+        String asAuthHeaderValue();
     }