Skip to content

Commit

Permalink
Implement IdentityPlugin in Security plugin (opensearch-project#3538)
Browse files Browse the repository at this point in the history 
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
Signed-off-by: Peter Nied <petern@amazon.com>
Co-authored-by: Peter Nied <petern@amazon.com>
  • Loading branch information
@stephen-crawford @peternied
stephen-crawford and peternied authored Nov 1, 2023
1 parent d10475c commit 4676452
Show file tree
Hide file tree
Showing 13 changed files with 627 additions and 155 deletions.
89 changes: 84 additions & 5 deletions src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@

package org.opensearch.security.http;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Base64;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
Expand All @@ -29,10 +29,12 @@
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.message.BasicHeader;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil;
import org.opensearch.test.framework.OnBehalfOfConfig;
import org.opensearch.test.framework.RolesMapping;
Expand All @@ -47,6 +49,7 @@
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.contains;
import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
Expand All @@ -67,6 +70,11 @@ public class OnBehalfOfJwtAuthenticationTest {
StandardCharsets.UTF_8
)
);
private static final String alternativeSigningKey = Base64.getEncoder()
.encodeToString(
"alternativeSigningKeyalternativeSigningKeyalternativeSigningKeyalternativeSigningKey".getBytes(StandardCharsets.UTF_8)
);

private static final String encryptionKey = Base64.getEncoder().encodeToString("encryptionKey".getBytes(StandardCharsets.UTF_8));
public static final String ADMIN_USER_NAME = "admin";
public static final String OBO_USER_NAME_WITH_PERM = "obo_user";
Expand Down Expand Up @@ -110,18 +118,36 @@ public class OnBehalfOfJwtAuthenticationTest {
protected final static TestSecurityConfig.User HOST_MAPPING_OBO_USER = new TestSecurityConfig.User(OBO_USER_NAME_WITH_HOST_MAPPING)
.roles(HOST_MAPPING_ROLE, ROLE_WITH_OBO_PERM);

private static OnBehalfOfConfig defaultOnBehalfOfConfig() {
return new OnBehalfOfConfig().oboEnabled(oboEnabled).signingKey(signingKey).encryptionKey(encryptionKey);
}

@ClassRule
public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
.anonymousAuth(false)
.users(ADMIN_USER, OBO_USER, OBO_USER_NO_PERM, HOST_MAPPING_OBO_USER)
.nodeSettings(
Map.of(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, true, SECURITY_RESTAPI_ROLES_ENABLED, List.of("user_admin__all_access"))
Map.of(
SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX,
true,
SECURITY_RESTAPI_ROLES_ENABLED,
ADMIN_USER.getRoleNames(),
SECURITY_RESTAPI_ADMIN_ENABLED,
true,
"plugins.security.unsupported.restapi.allow_securityconfig_modification",
true
)
)
.authc(AUTHC_HTTPBASIC_INTERNAL)
.rolesMapping(new RolesMapping(HOST_MAPPING_ROLE).hostIPs(HOST_MAPPING_IP))
.onBehalfOf(new OnBehalfOfConfig().oboEnabled(oboEnabled).signingKey(signingKey).encryptionKey(encryptionKey))
.onBehalfOf(defaultOnBehalfOfConfig())
.build();

@Before
public void before() {
patchOnBehalfOfConfig(defaultOnBehalfOfConfig());
}

@Test
public void shouldAuthenticateWithOBOTokenEndPoint() {
String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD);
Expand Down Expand Up @@ -196,7 +222,7 @@ public void shouldNotAuthenticateWithInvalidDurationSeconds() {
client.confirmCorrectCredentials(ADMIN_USER_NAME);
TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_DESCRIPTION_WITH_INVALID_DURATIONSECONDS);
response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
assertThat(response.getTextFromJsonBody("/error"), equalTo("durationSeconds must be an integer."));
assertThat(response.getTextFromJsonBody("/error"), equalTo("durationSeconds must be a number."));
}
}

Expand All @@ -210,6 +236,44 @@ public void shouldNotAuthenticateWithInvalidAPIParameter() {
}
}

@Test
public void shouldNotAllowTokenWhenOboIsDisabled() {
final String oboToken = generateOboToken(OBO_USER_NAME_WITH_PERM, DEFAULT_PASSWORD);
final Header oboHeader = new BasicHeader("Authorization", "Bearer " + oboToken);
authenticateWithOboToken(oboHeader, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_OK);

// Disable OBO via config and see that the authenticator doesn't authorize
patchOnBehalfOfConfig(defaultOnBehalfOfConfig().oboEnabled(false));
authenticateWithOboToken(oboHeader, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_UNAUTHORIZED);

// Reenable OBO via config and see that the authenticator is working again
patchOnBehalfOfConfig(defaultOnBehalfOfConfig().oboEnabled(true));
authenticateWithOboToken(oboHeader, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_OK);
}

@Test
public void oboSigningCheckChangeIsDetected() {
final String oboTokenOrignalKey = generateOboToken(OBO_USER_NAME_WITH_PERM, DEFAULT_PASSWORD);
final Header oboHeaderOriginalKey = new BasicHeader("Authorization", "Bearer " + oboTokenOrignalKey);
authenticateWithOboToken(oboHeaderOriginalKey, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_OK);

// Change the signing key
patchOnBehalfOfConfig(defaultOnBehalfOfConfig().signingKey(alternativeSigningKey));

// Original key should no longer work
authenticateWithOboToken(oboHeaderOriginalKey, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_UNAUTHORIZED);

// Generate new key, check that it is valid
final String oboTokenOtherKey = generateOboToken(OBO_USER_NAME_WITH_PERM, DEFAULT_PASSWORD);
final Header oboHeaderOtherKey = new BasicHeader("Authorization", "Bearer " + oboTokenOtherKey);
authenticateWithOboToken(oboHeaderOtherKey, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_OK);

// Change back to the original signing key and the original key still works, and the new key doesn't
patchOnBehalfOfConfig(defaultOnBehalfOfConfig());
authenticateWithOboToken(oboHeaderOriginalKey, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_OK);
authenticateWithOboToken(oboHeaderOtherKey, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_UNAUTHORIZED);
}

private String generateOboToken(String username, String password) {
try (TestRestClient client = cluster.getRestClient(username, password)) {
client.confirmCorrectCredentials(username);
Expand All @@ -233,4 +297,19 @@ private void authenticateWithOboToken(Header authHeader, String expectedUsername
}
}
}

private void patchOnBehalfOfConfig(final OnBehalfOfConfig oboConfig) {
try (final TestRestClient adminClient = cluster.getRestClient(cluster.getAdminCertificate())) {
final XContentBuilder configBuilder = XContentFactory.jsonBuilder();
configBuilder.value(oboConfig);

final String patchBody = "[{ \"op\": \"replace\", \"path\": \"/config/dynamic/on_behalf_of\", \"value\":"
+ configBuilder.toString()
+ "}]";
final var response = adminClient.patch("_plugins/_security/api/securityconfig", patchBody);
response.assertStatusCode(HttpStatus.SC_OK);
} catch (final IOException ex) {
throw new RuntimeException(ex);
}
}
}
26 changes: 22 additions & 4 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
// CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions

import com.google.common.collect.Lists;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.search.QueryCachingPolicy;
Expand Down Expand Up @@ -75,12 +76,15 @@
import org.opensearch.extensions.ExtensionsManager;
import org.opensearch.http.HttpServerTransport;
import org.opensearch.http.HttpServerTransport.Dispatcher;
import org.opensearch.identity.Subject;
import org.opensearch.identity.noop.NoopSubject;
import org.opensearch.index.IndexModule;
import org.opensearch.index.cache.query.QueryCache;
import org.opensearch.indices.IndicesService;
import org.opensearch.indices.SystemIndexDescriptor;
import org.opensearch.plugins.ClusterPlugin;
import org.opensearch.plugins.ExtensionAwarePlugin;
import org.opensearch.plugins.IdentityPlugin;
import org.opensearch.plugins.MapperPlugin;
import org.opensearch.repositories.RepositoriesService;
import org.opensearch.rest.RestController;
Expand Down Expand Up @@ -121,6 +125,7 @@
import org.opensearch.security.http.SecurityHttpServerTransport;
import org.opensearch.security.http.SecurityNonSslHttpServerTransport;
import org.opensearch.security.http.XFFResolver;
import org.opensearch.security.identity.SecurityTokenManager;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.privileges.PrivilegesInterceptor;
import org.opensearch.security.privileges.RestLayerPrivilegesEvaluator;
Expand Down Expand Up @@ -207,7 +212,8 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
ClusterPlugin,
MapperPlugin,
// CS-SUPPRESS-SINGLE: RegexpSingleline get Extensions Settings
ExtensionAwarePlugin
ExtensionAwarePlugin,
IdentityPlugin
// CS-ENFORCE-SINGLE

{
Expand All @@ -234,6 +240,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
private volatile SslExceptionHandler sslExceptionHandler;
private volatile Client localClient;
private final boolean disabled;
private volatile SecurityTokenManager tokenManager;
private volatile DynamicConfigFactory dcf;
private final List<String> demoCertHashes = new ArrayList<String>(3);
private volatile SecurityFilter sf;
Expand Down Expand Up @@ -561,9 +568,7 @@ public List<RestHandler> getRestHandlers(
principalExtractor
)
);
CreateOnBehalfOfTokenAction cobot = new CreateOnBehalfOfTokenAction(settings, threadPool, Objects.requireNonNull(cs));
dcf.registerDCFListener(cobot);
handlers.add(cobot);
handlers.add(new CreateOnBehalfOfTokenAction(tokenManager));
handlers.addAll(
SecurityRestApiActions.getHandler(
settings,
Expand Down Expand Up @@ -1035,6 +1040,7 @@ public Collection<Object> createComponents(

final XFFResolver xffResolver = new XFFResolver(threadPool);
backendRegistry = new BackendRegistry(settings, adminDns, xffResolver, auditLog, threadPool);
tokenManager = new SecurityTokenManager(cs, threadPool, userService);

final CompatConfig compatConfig = new CompatConfig(environment, transportPassiveAuthSetting);

Expand Down Expand Up @@ -1084,6 +1090,7 @@ public Collection<Object> createComponents(
dcf.registerDCFListener(evaluator);
dcf.registerDCFListener(restLayerEvaluator);
dcf.registerDCFListener(securityRestHandler);
dcf.registerDCFListener(tokenManager);
if (!(auditLog instanceof NullAuditLog)) {
// Don't register if advanced modules is disabled in which case auditlog is instance of NullAuditLog
dcf.registerDCFListener(auditLog);
Expand Down Expand Up @@ -1935,6 +1942,17 @@ private static String handleKeyword(final String field) {
return field;
}

@Override
public Subject getSubject() {
// Not supported
return new NoopSubject();
}

@Override
public SecurityTokenManager getTokenManager() {
return tokenManager;
}

public static class GuiceHolder implements LifecycleComponent {

private static RepositoriesService repositoriesService;
Expand Down
Loading

0 comments on commit 4676452

Please sign in to comment.