diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java index 5d3262799f..e68a5ef2d7 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -16,7 +16,6 @@ import java.util.Optional; import java.util.function.LongSupplier; -import com.google.common.base.Strings; import org.apache.cxf.jaxrs.json.basic.JsonMapObjectReaderWriter; import org.apache.cxf.rs.security.jose.jwk.JsonWebKey; import org.apache.cxf.rs.security.jose.jwk.KeyType; @@ -32,6 +31,8 @@ import org.opensearch.common.settings.Settings; import org.opensearch.security.ssl.util.ExceptionUtils; +import static org.opensearch.security.util.AuthTokenUtils.isKeyNull; + public class JwtVendor { private static final Logger logger = LogManager.getLogger(JwtVendor.class); @@ -53,7 +54,7 @@ public JwtVendor(final Settings settings, final Optional timeProvi throw ExceptionUtils.createJwkCreationException(e); } this.jwtProducer = jwtProducer; - if (settings.get("encryption_key") == null) { + if (isKeyNull(settings, "encryption_key")) { throw new IllegalArgumentException("encryption_key cannot be null"); } else { this.claimsEncryptionKey = settings.get("encryption_key"); @@ -73,9 +74,8 @@ public JwtVendor(final Settings settings, final Optional timeProvi * Encryption Algorithm: HS512 * */ static JsonWebKey createJwkFromSettings(Settings settings) throws Exception { - String signingKey = settings.get("signing_key"); - - if (!Strings.isNullOrEmpty(signingKey)) { + if (!isKeyNull(settings, "signing_key")) { + String signingKey = settings.get("signing_key"); JsonWebKey jwk = new JsonWebKey(); diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index 467edd8ac4..c47e850b75 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -43,13 +43,12 @@ import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; +import static org.opensearch.security.util.AuthTokenUtils.isAccessToRestrictedEndpoints; public class OnBehalfOfAuthenticator implements HTTPAuthenticator { private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)"; private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); - private static final String ON_BEHALF_OF_SUFFIX = "api/generateonbehalfoftoken"; - private static final String ACCOUNT_SUFFIX = "api/account"; protected final Logger log = LogManager.getLogger(this.getClass()); @@ -233,8 +232,7 @@ private void logDebug(String message, Object... args) { public Boolean isRequestAllowed(final RestRequest request) { Matcher matcher = PATTERN_PATH_PREFIX.matcher(request.path()); final String suffix = matcher.matches() ? matcher.group(2) : null; - if (request.method() == RestRequest.Method.POST && ON_BEHALF_OF_SUFFIX.equals(suffix) - || request.method() == RestRequest.Method.PUT && ACCOUNT_SUFFIX.equals(suffix)) { + if (isAccessToRestrictedEndpoints(request, suffix)) { final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); log.error(exception.toString()); return false; diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java index fcbf985f60..0de83f2e2e 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java @@ -67,6 +67,8 @@ import org.opensearch.security.securityconf.impl.v7.ConfigV7.AuthzDomain; import org.opensearch.security.support.ReflectionHelper; +import static org.opensearch.security.util.AuthTokenUtils.isKeyNull; + public class DynamicConfigModelV7 extends DynamicConfigModel { private final ConfigV7 config; @@ -383,7 +385,7 @@ private void buildAAA() { * order: -1 - prioritize the OBO authentication when it gets enabled */ Settings oboSettings = getDynamicOnBehalfOfSettings(); - if (oboSettings.get("signing_key") != null && oboSettings.get("encryption_key") != null) { + if (!isKeyNull(oboSettings, "signing_key") && !isKeyNull(oboSettings, "encryption_key")) { final AuthDomain _ad = new AuthDomain( new NoOpAuthenticationBackend(Settings.EMPTY, null), new OnBehalfOfAuthenticator(getDynamicOnBehalfOfSettings(), this.cih.getClusterName()), diff --git a/src/main/java/org/opensearch/security/util/AuthTokenUtils.java b/src/main/java/org/opensearch/security/util/AuthTokenUtils.java new file mode 100644 index 0000000000..30f331d3a7 --- /dev/null +++ b/src/main/java/org/opensearch/security/util/AuthTokenUtils.java @@ -0,0 +1,41 @@ +/* + * 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.util; + +import org.opensearch.common.settings.Settings; +import org.opensearch.rest.RestRequest; + +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.rest.RestRequest.Method.PUT; + +public class AuthTokenUtils { + private static final String ON_BEHALF_OF_SUFFIX = "api/generateonbehalfoftoken"; + private static final String ACCOUNT_SUFFIX = "api/account"; + + public static Boolean isAccessToRestrictedEndpoints(final RestRequest request, final String suffix) { + if (suffix == null) { + return false; + } + switch (suffix) { + case ON_BEHALF_OF_SUFFIX: + return request.method() == POST; + case ACCOUNT_SUFFIX: + return request.method() == PUT; + default: + return false; + } + } + + public static Boolean isKeyNull(Settings settings, String key) { + return settings.get(key) == null; + } +} diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/AuthTokenUtilsTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/AuthTokenUtilsTest.java new file mode 100644 index 0000000000..d563308e31 --- /dev/null +++ b/src/test/java/org/opensearch/security/authtoken/jwt/AuthTokenUtilsTest.java @@ -0,0 +1,78 @@ +/* + * 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.authtoken.jwt; + +import org.opensearch.common.settings.Settings; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.rest.RestRequest; +import org.opensearch.security.util.AuthTokenUtils; +import org.opensearch.test.rest.FakeRestRequest; +import org.junit.Test; + +import java.util.Collections; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class AuthTokenUtilsTest { + + @Test + public void testIsAccessToRestrictedEndpointsForOnBehalfOfToken() { + NamedXContentRegistry namedXContentRegistry = new NamedXContentRegistry(Collections.emptyList()); + + FakeRestRequest request = new FakeRestRequest.Builder(namedXContentRegistry).withPath("/api/generateonbehalfoftoken") + .withMethod(RestRequest.Method.POST) + .build(); + + assertTrue(AuthTokenUtils.isAccessToRestrictedEndpoints(request, "api/generateonbehalfoftoken")); + } + + @Test + public void testIsAccessToRestrictedEndpointsForAccount() { + NamedXContentRegistry namedXContentRegistry = new NamedXContentRegistry(Collections.emptyList()); + + FakeRestRequest request = new FakeRestRequest.Builder(namedXContentRegistry).withPath("/api/account") + .withMethod(RestRequest.Method.PUT) + .build(); + + assertTrue(AuthTokenUtils.isAccessToRestrictedEndpoints(request, "api/account")); + } + + @Test + public void testIsAccessToRestrictedEndpointsFalseCase() { + NamedXContentRegistry namedXContentRegistry = new NamedXContentRegistry(Collections.emptyList()); + + FakeRestRequest request = new FakeRestRequest.Builder(namedXContentRegistry).withPath("/api/someotherendpoint") + .withMethod(RestRequest.Method.GET) + .build(); + + assertFalse(AuthTokenUtils.isAccessToRestrictedEndpoints(request, "api/someotherendpoint")); + } + + @Test + public void testIsKeyNullWithNullValue() { + Settings settings = Settings.builder().put("someKey", (String) null).build(); + assertTrue(AuthTokenUtils.isKeyNull(settings, "someKey")); + } + + @Test + public void testIsKeyNullWithNonNullValue() { + Settings settings = Settings.builder().put("someKey", "value").build(); + assertFalse(AuthTokenUtils.isKeyNull(settings, "someKey")); + } + + @Test + public void testIsKeyNullWithAbsentKey() { + Settings settings = Settings.builder().build(); + assertTrue(AuthTokenUtils.isKeyNull(settings, "absentKey")); + } +}