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

Implement on behalf of token passing for extensions #8679

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
2837200
Outline changes
stephen-crawford Jul 12, 2023
570c04b
Basic outline
stephen-crawford Jul 12, 2023
abd83c4
Basic token passing
stephen-crawford Jul 13, 2023
0b4d85d
Merge branch 'main' into tokenPassingForExtensions
stephen-crawford Jul 13, 2023
afc6b34
Minor changes
stephen-crawford Jul 13, 2023
ee602cf
Swap issue token type
stephen-crawford Jul 13, 2023
095932d
Update changelog
stephen-crawford Jul 13, 2023
676d14f
SPotless
stephen-crawford Jul 13, 2023
b224770
Merge branch 'main' into tokenPassingForExtensions
stephen-crawford Jul 13, 2023
3db805c
Add test
stephen-crawford Jul 13, 2023
4d6e2fd
Swap out authentication
stephen-crawford Jul 13, 2023
7b1757e
Merge branch 'main' into tokenPassingForExtensions
stephen-crawford Jul 13, 2023
b15df51
Swap to string, obj map
stephen-crawford Jul 13, 2023
d937ca9
Update with Craig's feedback
stephen-crawford Jul 14, 2023
af9d2d7
Add javadoc
stephen-crawford Jul 14, 2023
f6209d4
add annotation
stephen-crawford Jul 14, 2023
acd3330
Fix missing path import
stephen-crawford Jul 14, 2023
ed2d09c
Fix test
stephen-crawford Jul 14, 2023
4fdfaf7
boost coverage
stephen-crawford Jul 14, 2023
a746202
Add interface for AuthToken
RyanL1997 Jul 18, 2023
52fa085
Merge pull request #6 from RyanL1997/interfaceForTokenPassing
stephen-crawford Jul 19, 2023
acf6c76
Update server/src/main/java/org/opensearch/identity/noop/NoopTokenMan…
stephen-crawford Jul 20, 2023
a703786
Merge branch 'main' into tokenPassingForExtensions
stephen-crawford Jul 24, 2023
417f419
Merge branch 'opensearch-project:main' into tokenPassingForExtensions
stephen-crawford Jul 24, 2023
1070564
Resolve comments
stephen-crawford Jul 24, 2023
0f6b361
Merge branch 'main' into tokenPassingForExtensions
stephen-crawford Jul 24, 2023
3ae548c
Add javadoc
stephen-crawford Jul 24, 2023
a7b9107
Make token check happen before write
stephen-crawford Jul 27, 2023
d7cf2ea
Fix test
stephen-crawford Jul 27, 2023
5db9178
Spotless
stephen-crawford Jul 27, 2023
2aa6d4f
trigger retry
stephen-crawford Jul 27, 2023
f073f5c
changes
stephen-crawford Jul 28, 2023
6d21516
Merge branch 'opensearch-project:main' into tokenPassingForExtensions
stephen-crawford Jul 28, 2023
46a6812
Swap issuer with subject
stephen-crawford Jul 28, 2023
bc6a18d
Merge branch 'opensearch-project:main' into tokenPassingForExtensions
stephen-crawford Jul 28, 2023
977d145
Merge branch 'opensearch-project:main' into tokenPassingForExtensions
stephen-crawford Jul 28, 2023
3440e16
Merge branch 'opensearch-project:main' into tokenPassingForExtensions
stephen-crawford Jul 31, 2023
45c3dcd
Boost coverage
stephen-crawford Jul 31, 2023
67806ed
Boost coverage
stephen-crawford Jul 31, 2023
5f09660
Merge branch 'main' into tokenPassingForExtensions
stephen-crawford Aug 3, 2023
3845842
Merge branch 'main' into tokenPassingForExtensions
stephen-crawford Aug 8, 2023
d990651
Fix
stephen-crawford Aug 11, 2023
5479019
spotless
stephen-crawford Aug 11, 2023
d2b7519
remove bad decleration
stephen-crawford Aug 11, 2023
40afb9e
Fix changelog
stephen-crawford Aug 11, 2023
5d17b26
Changelog
stephen-crawford Aug 11, 2023
2a8b4ae
Merge branch 'opensearch-project:main' into tokenPassingForExtensions
stephen-crawford Aug 11, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -51,15 +54,16 @@
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) {
reta marked this conversation as resolved.
Show resolved Hide resolved

String password = generatePassword();
stephen-crawford marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand All @@ -68,6 +72,11 @@
return token;
}

@Override
public Subject authenticateToken(AuthToken authToken) {
return new NoopSubject();

Check warning on line 77 in plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroTokenManager.java

View check run for this annotation

Codecov / codecov/patch

plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroTokenManager.java#L77

Added line #L77 was not covered by tests
}

public boolean validateToken(AuthToken token) {
if (token instanceof BasicAuthToken) {
final BasicAuthToken basicAuthToken = (BasicAuthToken) token;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)));
Expand Down Expand Up @@ -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);
}
Expand All @@ -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() {
Expand All @@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -41,7 +42,8 @@ public void initializeServicesAndRestHandler(
TransportService transportService,
ClusterService clusterService,
Settings initialEnvironmentSettings,
NodeClient client
NodeClient client,
IdentityService identityService
) {
// no-op
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -86,7 +87,7 @@
this.headers = headers;
this.mediaType = mediaType;
this.content = content;
this.principalIdentifierToken = principalIdentifier;
this.principalIdentifierToken = requireNonNull(principalIdentifier);
this.httpVersion = httpVersion;
}

Expand Down Expand Up @@ -280,7 +281,7 @@
}

/**
* 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.
Expand All @@ -291,6 +292,9 @@
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.");

Check warning on line 296 in server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java#L296

Added line #L296 was not covered by tests
}
return getXContentType().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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.
Expand All @@ -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;
}

/**
Expand All @@ -62,7 +65,8 @@ public TransportResponse handleRegisterRestActionsRequest(
restActionsRequest,
discoveryExtensionNode,
transportService,
dynamicActionRegistry
dynamicActionRegistry,
identityService
);
restController.registerHandler(handler);
return new AcknowledgedResponse(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -54,19 +57,13 @@

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");
Expand All @@ -82,7 +79,8 @@
RegisterRestActionsRequest restActionsRequest,
DiscoveryExtensionNode discoveryExtensionNode,
TransportService transportService,
DynamicActionRegistry dynamicActionRegistry
DynamicActionRegistry dynamicActionRegistry,
IdentityService identityService
) {
this.pathPrefix = "/_extensions/_" + restActionsRequest.getUniqueId();
RestRequest.Method method;
Expand Down Expand Up @@ -147,6 +145,7 @@

this.discoveryExtensionNode = discoveryExtensionNode;
this.transportService = transportService;
this.identityService = identityService;
}

@Override
Expand Down Expand Up @@ -240,12 +239,15 @@
};

try {

// Will be replaced with ExtensionTokenProcessor and PrincipalIdentifierToken classes from feature/identity
stephen-crawford marked this conversation as resolved.
Show resolved Hide resolved
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());

Check warning on line 249 in server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java#L247-L249

Added lines #L247 - L249 were not covered by tests
stephen-crawford marked this conversation as resolved.
Show resolved Hide resolved

transportService.sendRequest(
discoveryExtensionNode,
ExtensionsManager.REQUEST_REST_EXECUTE_ON_EXTENSION_ACTION,
Expand All @@ -259,7 +261,7 @@
filteredHeaders,
contentType,
content,
requestIssuerIdentity,
tokenManager.issueOnBehalfOfToken(subject, claims).asAuthHeaderValue(),

Check warning on line 264 in server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java#L264

Added line #L264 was not covered by tests
httpVersion
),
restExecuteOnExtensionResponseHandler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading