diff --git a/CHANGELOG.md b/CHANGELOG.md index c9626ba7ed..69a9b4db94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,6 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to the [Semantic Versioning](https://semver.org/spec/v2.0.0.html). See the [CONTRIBUTING guide](./CONTRIBUTING.md#Changelog) for instructions on how to add changelog entries. ## [Unreleased 3.x] -### Added -- Provide SecureHttpTransportParameters to complement SecureTransportParameters counterpart ([#5432](https://github.com/opensearch-project/security/pull/5432)) ### Features @@ -23,7 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Refactoring - +* Refactor JWT Vender to take a claims builder and rename oboEnabled to enabled ([#5436](https://github.com/opensearch-project/security/pull/5436)) ### Maintenance diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index 1dbb10b1f8..4edb3a74a2 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -62,7 +62,6 @@ public class OnBehalfOfJwtAuthenticationTest { static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS); private static final String CREATE_OBO_TOKEN_PATH = "_plugins/_security/api/generateonbehalfoftoken"; - private static Boolean oboEnabled = true; private static final String signingKey = Base64.getEncoder() .encodeToString( "jwt signing key for an on behalf of token authentication backend for testing of OBO authentication".getBytes( @@ -118,7 +117,7 @@ public class OnBehalfOfJwtAuthenticationTest { .roles(HOST_MAPPING_ROLE, ROLE_WITH_OBO_PERM); private static OnBehalfOfConfig defaultOnBehalfOfConfig() { - return new OnBehalfOfConfig().oboEnabled(oboEnabled).signingKey(signingKey).encryptionKey(encryptionKey); + return new OnBehalfOfConfig().enabled(true).signingKey(signingKey).encryptionKey(encryptionKey); } @ClassRule @@ -155,7 +154,7 @@ public void shouldAuthenticateWithOBOTokenEndPoint() { } @Test - public void shouldNotAuthenticateWithATemperedOBOToken() { + public void shouldNotAuthenticateWithATamperedOBOToken() { String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD); oboToken = oboToken.substring(0, oboToken.length() - 1); // tampering the token Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); @@ -242,11 +241,11 @@ public void shouldNotAllowTokenWhenOboIsDisabled() { 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)); + patchOnBehalfOfConfig(defaultOnBehalfOfConfig().enabled(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)); + patchOnBehalfOfConfig(defaultOnBehalfOfConfig().enabled(true)); authenticateWithOboToken(oboHeader, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_OK); } diff --git a/src/integrationTest/java/org/opensearch/test/framework/OnBehalfOfConfig.java b/src/integrationTest/java/org/opensearch/test/framework/OnBehalfOfConfig.java index 63e1544f98..ce44781a53 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/OnBehalfOfConfig.java +++ b/src/integrationTest/java/org/opensearch/test/framework/OnBehalfOfConfig.java @@ -18,12 +18,12 @@ import org.opensearch.core.xcontent.XContentBuilder; public class OnBehalfOfConfig implements ToXContentObject { - private Boolean oboEnabled; + private Boolean enabled; private String signing_key; private String encryption_key; - public OnBehalfOfConfig oboEnabled(Boolean oboEnabled) { - this.oboEnabled = oboEnabled; + public OnBehalfOfConfig enabled(Boolean enabled) { + this.enabled = enabled; return this; } @@ -40,7 +40,7 @@ public OnBehalfOfConfig encryptionKey(String encryption_key) { @Override public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Params params) throws IOException { xContentBuilder.startObject(); - xContentBuilder.field("enabled", oboEnabled); + xContentBuilder.field("enabled", enabled); xContentBuilder.field("signing_key", signing_key); if (StringUtils.isNoneBlank(encryption_key)) { xContentBuilder.field("encryption_key", encryption_key); 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 e21d9257ff..8fd3589ebe 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -11,12 +11,11 @@ package org.opensearch.security.authtoken.jwt; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.text.ParseException; import java.util.Base64; import java.util.Date; -import java.util.List; -import java.util.Optional; -import java.util.function.LongSupplier; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -24,6 +23,7 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; +import org.opensearch.security.authtoken.jwt.claims.JwtClaimsBuilder; import com.nimbusds.jose.JOSEException; import com.nimbusds.jose.JWSAlgorithm; @@ -34,7 +34,6 @@ import com.nimbusds.jose.jwk.JWK; import com.nimbusds.jose.jwk.KeyUse; import com.nimbusds.jose.jwk.OctetSequenceKey; -import com.nimbusds.jwt.JWTClaimsSet; import com.nimbusds.jwt.SignedJWT; import static org.opensearch.security.util.AuthTokenUtils.isKeyNull; @@ -44,21 +43,11 @@ public class JwtVendor { private final JWK signingKey; private final JWSSigner signer; - private final LongSupplier timeProvider; - private final EncryptionDecryptionUtil encryptionDecryptionUtil; - private static final Integer MAX_EXPIRY_SECONDS = 600; - public JwtVendor(final Settings settings, final Optional timeProvider) { + public JwtVendor(Settings settings) { final Tuple tuple = createJwkFromSettings(settings); signingKey = tuple.v1(); signer = tuple.v2(); - - if (isKeyNull(settings, "encryption_key")) { - throw new IllegalArgumentException("encryption_key cannot be null"); - } else { - this.encryptionDecryptionUtil = new EncryptionDecryptionUtil(settings.get("encryption_key")); - } - this.timeProvider = timeProvider.orElse(System::currentTimeMillis); } /* @@ -96,46 +85,14 @@ static Tuple createJwkFromSettings(final Settings settings) { } } - public ExpiringBearerAuthToken createJwt( - final String issuer, - final String subject, - final String audience, - final long requestedExpirySeconds, - final List roles, - final List backendRoles, - final boolean includeBackendRoles - ) throws JOSEException, ParseException { - final long currentTimeMs = timeProvider.getAsLong(); - final Date now = new Date(currentTimeMs); - - final JWTClaimsSet.Builder claimsBuilder = new JWTClaimsSet.Builder(); - claimsBuilder.issuer(issuer); - claimsBuilder.issueTime(now); - claimsBuilder.subject(subject); - claimsBuilder.audience(audience); - claimsBuilder.notBeforeTime(now); - - final long expirySeconds = Math.min(requestedExpirySeconds, MAX_EXPIRY_SECONDS); - if (expirySeconds <= 0) { - throw new IllegalArgumentException("The expiration time should be a positive integer"); - } - final Date expiryTime = new Date(currentTimeMs + expirySeconds * 1000); - claimsBuilder.expirationTime(expiryTime); - - if (roles != null) { - final String listOfRoles = String.join(",", roles); - claimsBuilder.claim("er", encryptionDecryptionUtil.encrypt(listOfRoles)); - } else { - throw new IllegalArgumentException("Roles cannot be null"); - } - - if (includeBackendRoles && backendRoles != null) { - final String listOfBackendRoles = String.join(",", backendRoles); - claimsBuilder.claim("br", listOfBackendRoles); - } + @SuppressWarnings("removal") + public ExpiringBearerAuthToken createJwt(JwtClaimsBuilder claimsBuilder, String subject, Date expiryTime, Long expirySeconds) + throws JOSEException, ParseException { final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.parse(signingKey.getAlgorithm().getName())).build(); - final SignedJWT signedJwt = new SignedJWT(header, claimsBuilder.build()); + final SignedJWT signedJwt = AccessController.doPrivileged( + (PrivilegedAction) () -> new SignedJWT(header, claimsBuilder.build()) + ); // Sign the JWT so it can be serialized signedJwt.sign(signer); diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/claims/JwtClaimsBuilder.java b/src/main/java/org/opensearch/security/authtoken/jwt/claims/JwtClaimsBuilder.java new file mode 100644 index 0000000000..2f48efaeeb --- /dev/null +++ b/src/main/java/org/opensearch/security/authtoken/jwt/claims/JwtClaimsBuilder.java @@ -0,0 +1,75 @@ +/* + * 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.claims; + +import java.util.Date; + +import com.nimbusds.jwt.JWTClaimsSet; + +/** + * Builder for creating JWT claims. + * + * This class builds a claims set with standard claims such as issued_at, not before time, subject, issuer, audience, + * expiration time, and custom claims. + */ +public class JwtClaimsBuilder { + private final JWTClaimsSet.Builder builder; + + public JwtClaimsBuilder() { + this.builder = new JWTClaimsSet.Builder(); + } + + public JwtClaimsBuilder issueTime(Date issueTime) { + builder.issueTime(issueTime); + return this; + } + + public JwtClaimsBuilder notBeforeTime(Date notBeforeTime) { + builder.notBeforeTime(notBeforeTime); + return this; + } + + public JwtClaimsBuilder subject(String subject) { + builder.subject(subject); + return this; + } + + public JwtClaimsBuilder issuer(String issuer) { + builder.issuer(issuer); + return this; + } + + public JwtClaimsBuilder audience(String audience) { + builder.audience(audience); + return this; + } + + public JwtClaimsBuilder issuedAt(Date issuedAt) { + builder.issueTime(issuedAt); + return this; + } + + public JwtClaimsBuilder expirationTime(Date expirationTime) { + builder.expirationTime(expirationTime); + return this; + } + + public JwtClaimsBuilder addCustomClaim(String claimName, String value) { + builder.claim(claimName, value); + return this; + } + + public JWTClaimsSet build() { + return builder.build(); + } + +} diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/claims/OBOJwtClaimsBuilder.java b/src/main/java/org/opensearch/security/authtoken/jwt/claims/OBOJwtClaimsBuilder.java new file mode 100644 index 0000000000..22044a165d --- /dev/null +++ b/src/main/java/org/opensearch/security/authtoken/jwt/claims/OBOJwtClaimsBuilder.java @@ -0,0 +1,39 @@ +/* + * 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.claims; + +import java.util.List; + +import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; + +public class OBOJwtClaimsBuilder extends JwtClaimsBuilder { + private final EncryptionDecryptionUtil encryptionDecryptionUtil; + + public OBOJwtClaimsBuilder(String encryptionKey) { + super(); + this.encryptionDecryptionUtil = new EncryptionDecryptionUtil(encryptionKey); + } + + public OBOJwtClaimsBuilder addRoles(List roles) { + final String listOfRoles = String.join(",", roles); + this.addCustomClaim("er", encryptionDecryptionUtil.encrypt(listOfRoles)); + return this; + } + + public OBOJwtClaimsBuilder addBackendRoles(Boolean includeBackendRoles, List backendRoles) { + if (includeBackendRoles && backendRoles != null) { + final String listOfBackendRoles = String.join(",", backendRoles); + this.addCustomClaim("br", listOfBackendRoles); + } + return this; + } +} diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index 111eff7a33..1a7817049b 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -63,15 +63,14 @@ public class OnBehalfOfAuthenticator implements HTTPAuthenticator { private final JwtParser jwtParser; private final String encryptionKey; - private final Boolean oboEnabled; + private final Boolean enabled; private final String clusterName; private final EncryptionDecryptionUtil encryptionUtil; @SuppressWarnings("removal") public OnBehalfOfAuthenticator(Settings settings, String clusterName) { - String oboEnabledSetting = settings.get("enabled", "true"); - oboEnabled = Boolean.parseBoolean(oboEnabledSetting); + enabled = settings.getAsBoolean("enabled", Boolean.TRUE); encryptionKey = settings.get("encryption_key"); final SecurityManager sm = System.getSecurityManager(); @@ -165,7 +164,7 @@ public AuthCredentials run() { } private AuthCredentials extractCredentials0(final SecurityRequest request) { - if (!oboEnabled) { + if (!enabled) { log.debug("On-behalf-of authentication is disabled"); return null; } diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index 8a0c3e85f1..519162c980 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -11,9 +11,10 @@ package org.opensearch.security.identity; -import java.util.Optional; +import java.util.ArrayList; +import java.util.Date; import java.util.Set; -import java.util.stream.Collectors; +import java.util.function.LongSupplier; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -29,6 +30,7 @@ import org.opensearch.identity.tokens.TokenManager; import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken; import org.opensearch.security.authtoken.jwt.JwtVendor; +import org.opensearch.security.authtoken.jwt.claims.OBOJwtClaimsBuilder; import org.opensearch.security.securityconf.ConfigModel; import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.support.ConfigConstants; @@ -39,6 +41,8 @@ import joptsimple.internal.Strings; import org.greenrobot.eventbus.Subscribe; +import static org.opensearch.security.util.AuthTokenUtils.isKeyNull; + /** * This class is the Security Plugin's implementation of the TokenManager used by all Identity Plugins. * It handles the issuance of both Service Account Tokens and On Behalf Of tokens. @@ -50,8 +54,10 @@ public class SecurityTokenManager implements TokenManager { private final ThreadPool threadPool; private final UserService userService; - private JwtVendor jwtVendor = null; + private Settings oboSettings = null; private ConfigModel configModel = null; + private final LongSupplier timeProvider = System::currentTimeMillis; + private static final Integer OBO_MAX_EXPIRY_SECONDS = 600; public SecurityTokenManager(final ClusterService cs, final ThreadPool threadPool, final UserService userService) { this.cs = cs; @@ -66,19 +72,17 @@ public void onConfigModelChanged(final ConfigModel configModel) { @Subscribe public void onDynamicConfigModelChanged(final DynamicConfigModel dcm) { - final Settings oboSettings = dcm.getDynamicOnBehalfOfSettings(); - final Boolean enabled = oboSettings.getAsBoolean("enabled", false); - if (enabled) { - jwtVendor = createJwtVendor(oboSettings); - } else { - jwtVendor = null; + final Settings oboSettingsFromDcm = dcm.getDynamicOnBehalfOfSettings(); + final Boolean oboEnabled = oboSettingsFromDcm.getAsBoolean("enabled", false); + if (oboEnabled) { + oboSettings = oboSettingsFromDcm; } } /** For testing */ JwtVendor createJwtVendor(final Settings settings) { try { - return new JwtVendor(settings, Optional.empty()); + return new JwtVendor(settings); } catch (final Exception ex) { logger.error("Unable to create the JwtVendor instance", ex); return null; @@ -86,7 +90,7 @@ JwtVendor createJwtVendor(final Settings settings) { } public boolean issueOnBehalfOfTokenAllowed() { - return jwtVendor != null && configModel != null; + return oboSettings != null && configModel != null; } @Override @@ -115,16 +119,36 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final final TransportAddress callerAddress = null; /* OBO tokens must not roles based on location from network address */ final Set mappedRoles = configModel.mapSecurityRoles(user, callerAddress); + final long currentTimeMs = timeProvider.getAsLong(); + final Date now = new Date(currentTimeMs); + + final long expirySeconds = Math.min(claims.getExpiration(), OBO_MAX_EXPIRY_SECONDS); + if (expirySeconds <= 0) { + throw new IllegalArgumentException("The expiration time should be a positive integer"); + } + if (mappedRoles == null) { + throw new IllegalArgumentException("Roles cannot be null"); + } + if (isKeyNull(oboSettings, "encryption_key")) { + throw new IllegalArgumentException("encryption_key cannot be null"); + } + + final OBOJwtClaimsBuilder claimsBuilder = new OBOJwtClaimsBuilder(oboSettings.get("encryption_key")); + + // Add obo claims + claimsBuilder.issuer(cs.getClusterName().value()); + claimsBuilder.issueTime(now); + claimsBuilder.subject(user.getName()); + claimsBuilder.audience(claims.getAudience()); + claimsBuilder.notBeforeTime(now); + claimsBuilder.addBackendRoles(false, new ArrayList<>(user.getRoles())); + claimsBuilder.addRoles(new ArrayList<>(mappedRoles)); + + final Date expiryTime = new Date(currentTimeMs + expirySeconds * 1000); + claimsBuilder.expirationTime(expiryTime); + try { - return jwtVendor.createJwt( - cs.getClusterName().value(), - user.getName(), - claims.getAudience(), - claims.getExpiration(), - mappedRoles.stream().collect(Collectors.toList()), - user.getRoles().stream().collect(Collectors.toList()), - false - ); + return createJwtVendor(oboSettings).createJwt(claimsBuilder, user.getName(), expiryTime, expirySeconds); } catch (final Exception ex) { logger.error("Error creating OnBehalfOfToken for " + user.getName(), ex); throw new OpenSearchSecurityException("Unable to generate OnBehalfOfToken"); diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index 77fb973a52..def5247590 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -450,7 +450,7 @@ public void unknownPropertiesHandler(String name, Object value) throws JsonMappi public static class OnBehalfOfSettings { @JsonProperty("enabled") - private Boolean oboEnabled = Boolean.FALSE; + private Boolean enabled = Boolean.FALSE; @JsonProperty("signing_key") private String signingKey; @JsonProperty("encryption_key") @@ -465,12 +465,12 @@ public String configAsJson() { } } - public Boolean getOboEnabled() { - return oboEnabled; + public Boolean isEnabled() { + return enabled; } - public void setOboEnabled(Boolean oboEnabled) { - this.oboEnabled = oboEnabled; + public void setEnabled(Boolean enabled) { + this.enabled = enabled; } public String getSigningKey() { @@ -491,7 +491,7 @@ public void setEncryptionKey(String encryptionKey) { @Override public String toString() { - return "OnBehalfOfSettings [ enabled=" + oboEnabled + ", signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]"; + return "OnBehalfOfSettings [ enabled=" + enabled + ", signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]"; } } diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java index ca8b4ad14d..32bcdf2b60 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -14,7 +14,6 @@ import java.nio.charset.StandardCharsets; import java.util.Date; import java.util.List; -import java.util.Optional; import java.util.function.LongSupplier; import com.google.common.io.BaseEncoding; @@ -30,6 +29,7 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; +import org.opensearch.security.authtoken.jwt.claims.OBOJwtClaimsBuilder; import org.opensearch.security.support.ConfigConstants; import com.nimbusds.jose.JWSSigner; @@ -41,10 +41,8 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.IsNull.notNullValue; -import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -66,7 +64,7 @@ public void testCreateJwkFromSettings() { final Tuple jwk = JwtVendor.createJwkFromSettings(settings); assertThat(jwk.v1().getAlgorithm().getName(), is("HS512")); assertThat(jwk.v1().getKeyUse().toString(), is("sig")); - Assert.assertTrue(jwk.v1().toOctetSequenceKey().getKeyValue().decodeToString().startsWith(signingKey)); + assertTrue(jwk.v1().toOctetSequenceKey().getKeyValue().decodeToString().startsWith(signingKey)); } @Test @@ -100,8 +98,21 @@ public void testCreateJwtWithRoles() throws Exception { String claimsEncryptionKey = "1234567890123456"; Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); - JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); - final ExpiringBearerAuthToken authToken = jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, false); + Date expiryTime = new Date(currentTime.getAsLong() + expirySeconds * 1000); + + JwtVendor OBOJwtVendor = new JwtVendor(settings); + final ExpiringBearerAuthToken authToken = OBOJwtVendor.createJwt( + new OBOJwtClaimsBuilder(claimsEncryptionKey).addRoles(roles) + .addBackendRoles(false, backendRoles) + .issuer(issuer) + .subject(subject) + .audience(audience) + .expirationTime(expiryTime) + .issueTime(new Date(currentTime.getAsLong())), + subject.toString(), + expiryTime, + (long) expirySeconds + ); SignedJWT signedJWT = SignedJWT.parse(authToken.getCompleteToken()); @@ -137,8 +148,21 @@ public void testCreateJwtWithBackendRolesIncluded() throws Exception { .put(ConfigConstants.EXTENSIONS_BWC_PLUGIN_MODE, true) // CS-ENFORCE-SINGLE .build(); - final JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); - final ExpiringBearerAuthToken authToken = jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, true); + final JwtVendor OBOJwtVendor = new JwtVendor(settings); + Date expiryTime = new Date(currentTime.getAsLong() + expirySeconds * 1000); + + final ExpiringBearerAuthToken authToken = OBOJwtVendor.createJwt( + new OBOJwtClaimsBuilder(claimsEncryptionKey).addRoles(roles) + .addBackendRoles(true, backendRoles) + .issuer(issuer) + .subject(subject) + .audience(audience) + .expirationTime(expiryTime) + .issueTime(new Date(currentTime.getAsLong())), + subject.toString(), + expiryTime, + (long) expirySeconds + ); SignedJWT signedJWT = SignedJWT.parse(authToken.getCompleteToken()); @@ -154,87 +178,6 @@ public void testCreateJwtWithBackendRolesIncluded() throws Exception { assertThat(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("er").toString()), equalTo(expectedRoles)); } - @Test - public void testCreateJwtWithNegativeExpiry() { - String issuer = "cluster_0"; - String subject = "admin"; - String audience = "audience_0"; - List roles = List.of("admin"); - Integer expirySeconds = -300; - String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); - Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); - JwtVendor jwtVendor = new JwtVendor(settings, Optional.empty()); - - final Throwable exception = assertThrows(RuntimeException.class, () -> { - try { - jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, List.of(), true); - } catch (final Exception e) { - throw new RuntimeException(e); - } - }); - assertThat(exception.getMessage(), is("java.lang.IllegalArgumentException: The expiration time should be a positive integer")); - } - - @Test - public void testCreateJwtWithExceededExpiry() throws Exception { - String issuer = "cluster_0"; - String subject = "admin"; - String audience = "audience_0"; - List roles = List.of("IT", "HR"); - List backendRoles = List.of("Sales", "Support"); - int expirySeconds = 900_000; - LongSupplier currentTime = () -> (long) 100; - String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); - Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); - JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); - - final ExpiringBearerAuthToken authToken = jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, true); - // Expiry is a hint, the max value is controlled by the JwtVendor and reduced as is seen fit. - assertThat(authToken.getExpiresInSeconds(), not(equalTo(expirySeconds))); - assertThat(authToken.getExpiresInSeconds(), equalTo(600L)); - } - - @Test - public void testCreateJwtWithBadEncryptionKey() { - final String issuer = "cluster_0"; - final String subject = "admin"; - final String audience = "audience_0"; - final List roles = List.of("admin"); - final Integer expirySeconds = 300; - - Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).build(); - - final Throwable exception = assertThrows(RuntimeException.class, () -> { - try { - new JwtVendor(settings, Optional.empty()).createJwt(issuer, subject, audience, expirySeconds, roles, List.of(), true); - } catch (final Exception e) { - throw new RuntimeException(e); - } - }); - assertThat(exception.getMessage(), is("java.lang.IllegalArgumentException: encryption_key cannot be null")); - } - - @Test - public void testCreateJwtWithBadRoles() { - String issuer = "cluster_0"; - String subject = "admin"; - String audience = "audience_0"; - List roles = null; - Integer expirySeconds = 300; - String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); - Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); - JwtVendor jwtVendor = new JwtVendor(settings, Optional.empty()); - - final Throwable exception = assertThrows(RuntimeException.class, () -> { - try { - jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, List.of(), true); - } catch (final Exception e) { - throw new RuntimeException(e); - } - }); - assertThat(exception.getMessage(), is("java.lang.IllegalArgumentException: Roles cannot be null")); - } - @Test public void testCreateJwtLogsCorrectly() throws Exception { mockAppender = mock(Appender.class); @@ -255,11 +198,22 @@ public void testCreateJwtLogsCorrectly() throws Exception { final String audience = "audience_0"; final List roles = List.of("IT", "HR"); final List backendRoles = List.of("Sales", "Support"); - final int expirySeconds = 300; - - final JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); + int expirySeconds = 300; - jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, false); + final JwtVendor OBOJwtVendor = new JwtVendor(settings); + Date expiryTime = new Date(currentTime.getAsLong() + expirySeconds * 1000); + OBOJwtVendor.createJwt( + new OBOJwtClaimsBuilder(claimsEncryptionKey).addRoles(roles) + .addBackendRoles(true, backendRoles) + .issuer(issuer) + .subject(subject) + .audience(audience) + .expirationTime(expiryTime) + .issueTime(new Date(currentTime.getAsLong())), + subject.toString(), + expiryTime, + (long) expirySeconds + ); verify(mockAppender, times(1)).append(logEventCaptor.capture()); diff --git a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java index 42ff9e04e8..7558533656 100644 --- a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java +++ b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java @@ -12,8 +12,10 @@ package org.opensearch.security.identity; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Set; +import com.google.common.io.BaseEncoding; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -36,16 +38,15 @@ import org.opensearch.security.user.UserService; import org.opensearch.threadpool.ThreadPool; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -75,11 +76,14 @@ public void setup() { @After public void after() { - verifyNoMoreInteractions(cs); - verifyNoMoreInteractions(threadPool); verifyNoMoreInteractions(userService); } + final static String signingKey = + "This is my super safe signing key that no one will ever be able to guess. It's would take billions of years and the world's most powerful quantum computer to crack"; + final static String signingKeyB64Encoded = BaseEncoding.base64().encode(signingKey.getBytes(StandardCharsets.UTF_8)); + + @Test public void onConfigModelChanged_oboNotSupported() { final ConfigModel configModel = mock(ConfigModel.class); @@ -92,7 +96,7 @@ public void onConfigModelChanged_oboNotSupported() { @Test public void onDynamicConfigModelChanged_JwtVendorEnabled() { final ConfigModel configModel = mock(ConfigModel.class); - final DynamicConfigModel mockConfigModel = createMockJwtVendorInTokenManager(); + final DynamicConfigModel mockConfigModel = createMockJwtVendorInTokenManager(true); tokenManager.onConfigModelChanged(configModel); @@ -114,8 +118,12 @@ public void onDynamicConfigModelChanged_JwtVendorDisabled() { } /** Creates the jwt vendor and returns a mock for validation if needed */ - private DynamicConfigModel createMockJwtVendorInTokenManager() { - final Settings settings = Settings.builder().put("enabled", true).build(); + private DynamicConfigModel createMockJwtVendorInTokenManager(boolean includeEncryptionKey) { + final Settings settings = Settings.builder() + .put("enabled", true) + .put("signing_key", signingKeyB64Encoded) + .put("encryption_key", (includeEncryptionKey ? "1234567890" : null)) + .build(); final DynamicConfigModel dcm = mock(DynamicConfigModel.class); when(dcm.getDynamicOnBehalfOfSettings()).thenReturn(settings); doAnswer((invocation) -> jwtVendor).when(tokenManager).createJwtVendor(settings); @@ -207,11 +215,9 @@ public void issueOnBehalfOfToken_jwtGenerationFailure() throws Exception { tokenManager.onConfigModelChanged(configModel); when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); - createMockJwtVendorInTokenManager(); + createMockJwtVendorInTokenManager(true); - when(jwtVendor.createJwt(any(), anyString(), anyString(), anyLong(), any(), any(), anyBoolean())).thenThrow( - new RuntimeException("foobar") - ); + when(jwtVendor.createJwt(any(), any(), any(), any())).thenThrow(new RuntimeException("foobar")); final OpenSearchSecurityException exception = assertThrows( OpenSearchSecurityException.class, () -> tokenManager.issueOnBehalfOfToken(null, new OnBehalfOfClaims("elmo", 450L)) @@ -233,10 +239,10 @@ public void issueOnBehalfOfToken_success() throws Exception { tokenManager.onConfigModelChanged(configModel); when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); - createMockJwtVendorInTokenManager(); + createMockJwtVendorInTokenManager(true); final ExpiringBearerAuthToken authToken = mock(ExpiringBearerAuthToken.class); - when(jwtVendor.createJwt(any(), anyString(), anyString(), anyLong(), any(), any(), anyBoolean())).thenReturn(authToken); + when(jwtVendor.createJwt(any(), any(), any(), any())).thenReturn(authToken); final AuthToken returnedToken = tokenManager.issueOnBehalfOfToken(null, new OnBehalfOfClaims("elmo", 450L)); assertThat(returnedToken, equalTo(authToken)); @@ -244,4 +250,91 @@ public void issueOnBehalfOfToken_success() throws Exception { verify(cs).getClusterName(); verify(threadPool).getThreadContext(); } + + @Test + public void testCreateJwtWithNegativeExpiry() { + doAnswer(invocation -> true).when(tokenManager).issueOnBehalfOfTokenAllowed(); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User("Jon")); + when(threadPool.getThreadContext()).thenReturn(threadContext); + final ConfigModel configModel = mock(ConfigModel.class); + tokenManager.onConfigModelChanged(configModel); + when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); + + createMockJwtVendorInTokenManager(true); + + final Throwable exception = assertThrows(RuntimeException.class, () -> { + try { + final AuthToken returnedToken = tokenManager.issueOnBehalfOfToken(null, new OnBehalfOfClaims("elmo", -300L)); + } catch (final Exception e) { + throw new RuntimeException(e); + } + }); + assertThat(exception.getMessage(), is("java.lang.IllegalArgumentException: The expiration time should be a positive integer")); + } + + @Test + public void testCreateJwtWithExceededExpiry() throws Exception { + doAnswer(invocation -> new ClusterName("cluster17")).when(cs).getClusterName(); + doAnswer(invocation -> true).when(tokenManager).issueOnBehalfOfTokenAllowed(); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User("Jon")); + when(threadPool.getThreadContext()).thenReturn(threadContext); + final ConfigModel configModel = mock(ConfigModel.class); + tokenManager.onConfigModelChanged(configModel); + when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); + + createMockJwtVendorInTokenManager(true); + + tokenManager.issueOnBehalfOfToken(null, new OnBehalfOfClaims("elmo", 90000000L)); + // Expiry is a hint, the max value is controlled by the JwtVendor and reduced as is seen fit. + ArgumentCaptor longCaptor = ArgumentCaptor.forClass(Long.class); + verify(jwtVendor).createJwt(any(), any(), any(), longCaptor.capture()); + + assertThat(600L, equalTo(longCaptor.getValue())); + } + + @Test + public void testCreateJwtWithBadEncryptionKey() { + doAnswer(invocation -> true).when(tokenManager).issueOnBehalfOfTokenAllowed(); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User("Jon")); + when(threadPool.getThreadContext()).thenReturn(threadContext); + final ConfigModel configModel = mock(ConfigModel.class); + tokenManager.onConfigModelChanged(configModel); + when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); + + createMockJwtVendorInTokenManager(false); + + final Throwable exception = assertThrows(RuntimeException.class, () -> { + try { + tokenManager.issueOnBehalfOfToken(null, new OnBehalfOfClaims("elmo", 90000000L)); + } catch (final Exception e) { + throw new RuntimeException(e); + } + }); + assertThat(exception.getMessage(), is("java.lang.IllegalArgumentException: encryption_key cannot be null")); + } + + @Test + public void testCreateJwtWithBadRoles() { + doAnswer(invocation -> true).when(tokenManager).issueOnBehalfOfTokenAllowed(); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User("Jon")); + when(threadPool.getThreadContext()).thenReturn(threadContext); + final ConfigModel configModel = mock(ConfigModel.class); + tokenManager.onConfigModelChanged(configModel); + when(configModel.mapSecurityRoles(any(), any())).thenReturn(null); + + createMockJwtVendorInTokenManager(true); + + final Throwable exception = assertThrows(RuntimeException.class, () -> { + try { + tokenManager.issueOnBehalfOfToken(null, new OnBehalfOfClaims("elmo", 90000000L)); + } catch (final Exception e) { + throw new RuntimeException(e); + } + }); + assertThat(exception.getMessage(), is("java.lang.IllegalArgumentException: Roles cannot be null")); + } } diff --git a/src/test/java/org/opensearch/security/securityconf/impl/v7/ConfigV7Test.java b/src/test/java/org/opensearch/security/securityconf/impl/v7/ConfigV7Test.java index df1835adff..f05448bcd0 100644 --- a/src/test/java/org/opensearch/security/securityconf/impl/v7/ConfigV7Test.java +++ b/src/test/java/org/opensearch/security/securityconf/impl/v7/ConfigV7Test.java @@ -111,7 +111,7 @@ public void testOnBehalfOfSettings() { ConfigV7.OnBehalfOfSettings oboSettings; oboSettings = new ConfigV7.OnBehalfOfSettings(); - assertThat(Boolean.FALSE, is(oboSettings.getOboEnabled())); + assertThat(Boolean.FALSE, is(oboSettings.isEnabled())); Assert.assertNull(oboSettings.getSigningKey()); Assert.assertNull(oboSettings.getEncryptionKey()); }