Skip to content

Commit

Permalink
Merge branch 'main' into optional-data-exception-fix
Browse files Browse the repository at this point in the history
  • Loading branch information
cwperks committed Nov 2, 2023
2 parents 6a785c7 + 5de9686 commit caa026c
Show file tree
Hide file tree
Showing 25 changed files with 853 additions and 176 deletions.
4 changes: 2 additions & 2 deletions bwc-test/gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-8.3-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionSha256Sum=591855b517fc635b9e04de1d05d5e76ada3f89f5fc76f87978d1b245b4f69225
distributionSha256Sum=3e1af3ae886920c3ac87f7a91f816c0c7c436f276a6eefdb3da152100fef72ae
14 changes: 7 additions & 7 deletions bwc-test/gradlew
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,15 @@ if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then
case $MAX_FD in #(
max*)
# In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked.
# shellcheck disable=SC3045
# shellcheck disable=SC2039,SC3045
MAX_FD=$( ulimit -H -n ) ||
warn "Could not query maximum file descriptor limit"
esac
case $MAX_FD in #(
'' | soft) :;; #(
*)
# In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked.
# shellcheck disable=SC3045
# shellcheck disable=SC2039,SC3045
ulimit -n "$MAX_FD" ||
warn "Could not set maximum file descriptor limit to $MAX_FD"
esac
Expand Down Expand Up @@ -202,11 +202,11 @@ fi
# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'

# Collect all arguments for the java command;
# * $DEFAULT_JVM_OPTS, $JAVA_OPTS, and $GRADLE_OPTS can contain fragments of
# shell script including quotes and variable substitutions, so put them in
# double quotes to make sure that they get re-expanded; and
# * put everything else in single quotes, so that it's not re-expanded.
# Collect all arguments for the java command:
# * DEFAULT_JVM_OPTS, JAVA_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
# and any embedded shellness will be escaped.
# * For example: A user cannot expect ${Hostname} to be expanded, as it is an environment variable and will be
# treated as '${Hostname}' itself on the command line.

set -- \
"-Dorg.gradle.appname=$APP_BASE_NAME" \
Expand Down
4 changes: 2 additions & 2 deletions gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-8.3-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionSha256Sum=591855b517fc635b9e04de1d05d5e76ada3f89f5fc76f87978d1b245b4f69225
distributionSha256Sum=3e1af3ae886920c3ac87f7a91f816c0c7c436f276a6eefdb3da152100fef72ae
14 changes: 7 additions & 7 deletions gradlew
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,15 @@ if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then
case $MAX_FD in #(
max*)
# In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked.
# shellcheck disable=SC3045
# shellcheck disable=SC2039,SC3045
MAX_FD=$( ulimit -H -n ) ||
warn "Could not query maximum file descriptor limit"
esac
case $MAX_FD in #(
'' | soft) :;; #(
*)
# In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked.
# shellcheck disable=SC3045
# shellcheck disable=SC2039,SC3045
ulimit -n "$MAX_FD" ||
warn "Could not set maximum file descriptor limit to $MAX_FD"
esac
Expand Down Expand Up @@ -202,11 +202,11 @@ fi
# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'

# Collect all arguments for the java command;
# * $DEFAULT_JVM_OPTS, $JAVA_OPTS, and $GRADLE_OPTS can contain fragments of
# shell script including quotes and variable substitutions, so put them in
# double quotes to make sure that they get re-expanded; and
# * put everything else in single quotes, so that it's not re-expanded.
# Collect all arguments for the java command:
# * DEFAULT_JVM_OPTS, JAVA_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
# and any embedded shellness will be escaped.
# * For example: A user cannot expect ${Hostname} to be expanded, as it is an environment variable and will be
# treated as '${Hostname}' itself on the command line.

set -- \
"-Dorg.gradle.appname=$APP_BASE_NAME" \
Expand Down
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
@@ -0,0 +1,142 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.http;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.hc.core5.http.HttpStatus;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.opensearch.test.framework.TestIndex;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;

import java.util.List;
import java.util.Map;

import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertNotNull;
import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY;
import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_KEY;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;

@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class ServiceAccountAuthenticationTest {

public static final String SERVICE_ATTRIBUTE = "service";

static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS);

public static final String SERVICE_ACCOUNT_USER_NAME = "test-service-account";

static final TestSecurityConfig.User SERVICE_ACCOUNT_ADMIN_USER = new TestSecurityConfig.User(SERVICE_ACCOUNT_USER_NAME).attr(
SERVICE_ATTRIBUTE,
"true"
)
.roles(
new TestSecurityConfig.Role("test-service-account-role").clusterPermissions("*")
.indexPermissions("*", "system:admin/system_index")
.on("*")
);

private static final TestIndex TEST_NON_SYS_INDEX = TestIndex.name("test-non-sys-index")
.setting("index.number_of_shards", 1)
.setting("index.number_of_replicas", 0)
.build();

private static final TestIndex TEST_SYS_INDEX = TestIndex.name("test-sys-index")
.setting("index.number_of_shards", 1)
.setting("index.number_of_replicas", 0)
.build();

@ClassRule
public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
.anonymousAuth(false)
.users(ADMIN_USER, SERVICE_ACCOUNT_ADMIN_USER)
.nodeSettings(
Map.of(
SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY,
true,
SECURITY_SYSTEM_INDICES_ENABLED_KEY,
true,
SECURITY_RESTAPI_ROLES_ENABLED,
List.of("user_admin__all_access"),
SECURITY_SYSTEM_INDICES_KEY,
List.of(TEST_SYS_INDEX.getName())
)
)
.authc(AUTHC_HTTPBASIC_INTERNAL)
.indices(TEST_NON_SYS_INDEX, TEST_SYS_INDEX)
.build();

@Test
public void testClusterHealthWithServiceAccountCred() {
try (TestRestClient client = cluster.getRestClient(SERVICE_ACCOUNT_ADMIN_USER)) {
client.confirmCorrectCredentials(SERVICE_ACCOUNT_USER_NAME);
TestRestClient.HttpResponse response = client.get("_cluster/health");
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);

String responseBody = response.getBody();

assertNotNull("Response body should not be null", responseBody);
assertTrue(responseBody.contains("\"type\":\"security_exception\""));
}
}

@Test
public void testReadSysIndexWithServiceAccountCred() {
try (TestRestClient client = cluster.getRestClient(SERVICE_ACCOUNT_ADMIN_USER)) {
client.confirmCorrectCredentials(SERVICE_ACCOUNT_USER_NAME);
TestRestClient.HttpResponse response = client.get(TEST_SYS_INDEX.getName());
response.assertStatusCode(HttpStatus.SC_OK);

String responseBody = response.getBody();

assertNotNull("Response body should not be null", responseBody);
assertTrue(responseBody.contains(TEST_SYS_INDEX.getName()));
}
}

@Test
public void testReadNonSysIndexWithServiceAccountCred() {
try (TestRestClient client = cluster.getRestClient(SERVICE_ACCOUNT_ADMIN_USER)) {
client.confirmCorrectCredentials(SERVICE_ACCOUNT_USER_NAME);
TestRestClient.HttpResponse response = client.get(TEST_NON_SYS_INDEX.getName());
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);

String responseBody = response.getBody();

assertNotNull("Response body should not be null", responseBody);
assertTrue(responseBody.contains("\"type\":\"security_exception\""));
}
}

@Test
public void testReadBothWithServiceAccountCred() {
TestRestClient client = cluster.getRestClient(SERVICE_ACCOUNT_ADMIN_USER);
client.confirmCorrectCredentials(SERVICE_ACCOUNT_USER_NAME);
TestRestClient.HttpResponse response = client.get((TEST_SYS_INDEX.getName() + "," + TEST_NON_SYS_INDEX.getName()));
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);

String responseBody = response.getBody();

assertNotNull("Response body should not be null", responseBody);
assertTrue(responseBody.contains("\"type\":\"security_exception\""));

}
}
Loading

0 comments on commit caa026c

Please sign in to comment.