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 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.
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);
}
}
}
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.