diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index 5a43365aee..f726a8134b 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -149,7 +149,7 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final final OBOJwtClaimsBuilder claimsBuilder = new OBOJwtClaimsBuilder(oboSettings.get("encryption_key")); // Add obo claims - claimsBuilder.issuer(cs.getClusterName().toString()); + claimsBuilder.issuer(cs.getClusterName().value()); claimsBuilder.issueTime(now); claimsBuilder.subject(user.getName()); claimsBuilder.audience(claims.getAudience()); @@ -178,7 +178,7 @@ public ExpiringBearerAuthToken issueApiToken(final String name, final Long expir final Date now = new Date(currentTimeMs); final ApiJwtClaimsBuilder claimsBuilder = new ApiJwtClaimsBuilder(); - claimsBuilder.issuer(cs.getClusterName().toString()); + claimsBuilder.issuer(cs.getClusterName().value()); claimsBuilder.issueTime(now); claimsBuilder.subject(name); claimsBuilder.audience(name); 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 7db9931ace..6112f2794f 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -18,14 +18,18 @@ import com.google.common.io.BaseEncoding; import org.apache.commons.lang3.RandomStringUtils; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.Appender; import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.Logger; import org.junit.Assert; import org.junit.Test; import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; +import org.opensearch.security.authtoken.jwt.claims.ApiJwtClaimsBuilder; import org.opensearch.security.authtoken.jwt.claims.OBOJwtClaimsBuilder; import org.opensearch.security.support.ConfigConstants; @@ -41,6 +45,11 @@ 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; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class JwtVendorTest { private Appender mockAppender; @@ -57,7 +66,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 @@ -172,174 +181,94 @@ public void testCreateJwtWithBackendRolesIncluded() throws Exception { } @Test - public void testCreateJwtWithNegativeExpiry() { + public void testCreateJwtLogsCorrectly() throws Exception { + mockAppender = mock(Appender.class); + logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); + when(mockAppender.getName()).thenReturn("MockAppender"); + when(mockAppender.isStarted()).thenReturn(true); + final Logger logger = (Logger) LogManager.getLogger(JwtVendor.class); + logger.addAppender(mockAppender); + logger.setLevel(Level.DEBUG); + + // Mock settings and other required dependencies + LongSupplier currentTime = () -> (long) 100; + String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); + Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); + + final String issuer = "cluster_0"; + final String subject = "admin"; + final String audience = "audience_0"; + final List roles = List.of("IT", "HR"); + final List backendRoles = List.of("Sales", "Support"); + int expirySeconds = 300; + + 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()); + + final LogEvent logEvent = logEventCaptor.getValue(); + final String logMessage = logEvent.getMessage().getFormattedMessage(); + assertTrue(logMessage.startsWith("Created JWT:")); + + final String[] parts = logMessage.split("\\."); + assertTrue(parts.length >= 3); + } + + @Test + public void testCreateApiTokenJwtSuccess() throws Exception { 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 OBOJwtVendor = new JwtVendor(settings); - LongSupplier currentTime = () -> (long) 100; + int expirySeconds = 300; + // 2023 oct 4, 10:00:00 AM GMT + LongSupplier currentTime = () -> 1696413600000L; + Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).build(); + + Date expiryTime = new Date(currentTime.getAsLong() + expirySeconds * 1000); - Date expiryTime = new Date(expirySeconds); - - final Throwable exception = assertThrows(RuntimeException.class, () -> { - try { - OBOJwtVendor.createJwt( - new OBOJwtClaimsBuilder(claimsEncryptionKey).addRoles(roles) - .addBackendRoles(true, List.of()) - .issuer(issuer) - .subject(subject) - .audience(audience) - .expirationTime(expiryTime) - .issueTime(new Date(currentTime.getAsLong())), - subject.toString(), - expiryTime, - (long) expirySeconds - ); - } catch (final Exception e) { - throw new RuntimeException(e); - } - }); - assertThat(exception.getMessage(), is("java.lang.IllegalArgumentException: The expiration time should be a positive integer")); + JwtVendor apiTokenJwtVendor = new JwtVendor(settings); + final ExpiringBearerAuthToken authToken = apiTokenJwtVendor.createJwt( + new ApiJwtClaimsBuilder().issuer(issuer) + .subject(subject) + .audience(audience) + .expirationTime(expiryTime) + .issueTime(new Date(currentTime.getAsLong())), + subject.toString(), + expiryTime, + (long) expirySeconds + ); + + SignedJWT signedJWT = SignedJWT.parse(authToken.getCompleteToken()); + + assertThat(signedJWT.getJWTClaimsSet().getClaims().get("iss"), equalTo("cluster_0")); + assertThat(signedJWT.getJWTClaimsSet().getClaims().get("sub"), equalTo("admin")); + assertThat(signedJWT.getJWTClaimsSet().getClaims().get("aud").toString(), equalTo("[audience_0]")); + // 2023 oct 4, 10:00:00 AM GMT + assertThat(((Date) signedJWT.getJWTClaimsSet().getClaims().get("iat")).getTime(), is(1696413600000L)); + // 2023 oct 4, 10:05:00 AM GMT + assertThat(((Date) signedJWT.getJWTClaimsSet().getClaims().get("exp")).getTime(), is(1696413900000L)); + } + + @Test + public void testKeyTooShortForApiTokenThrowsException() { + String tooShortKey = BaseEncoding.base64().encode("short_key".getBytes()); + Settings settings = Settings.builder().put("signing_key", tooShortKey).build(); + final Throwable exception = assertThrows(OpenSearchException.class, () -> { new JwtVendor(settings); }); + + assertThat(exception.getMessage(), containsString("The secret length must be at least 256 bits")); } - // - // @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 OBOJwtVendor = new JwtVendor(settings, Optional.of(currentTime)); - // - // final ExpiringBearerAuthToken authToken = OBOJwtVendor.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 OBOJwtVendor = new JwtVendor(settings, Optional.empty()); - // - // final Throwable exception = assertThrows(RuntimeException.class, () -> { - // try { - // OBOJwtVendor.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); - // logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); - // when(mockAppender.getName()).thenReturn("MockAppender"); - // when(mockAppender.isStarted()).thenReturn(true); - // final Logger logger = (Logger) LogManager.getLogger(JwtVendor.class); - // logger.addAppender(mockAppender); - // logger.setLevel(Level.DEBUG); - // - // // Mock settings and other required dependencies - // LongSupplier currentTime = () -> (long) 100; - // String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); - // Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); - // - // final String issuer = "cluster_0"; - // final String subject = "admin"; - // 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 OBOJwtVendor = new JwtVendor(settings, Optional.of(currentTime)); - // - // OBOJwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, false); - // - // verify(mockAppender, times(1)).append(logEventCaptor.capture()); - // - // final LogEvent logEvent = logEventCaptor.getValue(); - // final String logMessage = logEvent.getMessage().getFormattedMessage(); - // assertTrue(logMessage.startsWith("Created JWT:")); - // - // final String[] parts = logMessage.split("\\."); - // assertTrue(parts.length >= 3); - // } - // - // @Test - // public void testCreateJwtForApiTokenSuccess() throws Exception { - // final String issuer = "cluster_0"; - // final String subject = "test-token"; - // final String audience = "test-token"; - // - // LongSupplier currentTime = () -> (long) 100; - // String claimsEncryptionKey = "1234567890123456"; - // Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); - // final JwtVendor apiTokenJwtVendor = new JwtVendor(settings, Optional.of(currentTime)); - // final ExpiringBearerAuthToken authToken = apiTokenJwtVendor.createApiTokenJwt(issuer, subject, audience, Long.MAX_VALUE); - // - // SignedJWT signedJWT = SignedJWT.parse(authToken.getCompleteToken()); - // - // assertThat(signedJWT.getJWTClaimsSet().getClaims().get("iss"), equalTo(issuer)); - // assertThat(signedJWT.getJWTClaimsSet().getClaims().get("sub"), equalTo(subject)); - // assertThat(signedJWT.getJWTClaimsSet().getClaims().get("aud").toString(), equalTo("[" + audience + "]")); - // assertThat(signedJWT.getJWTClaimsSet().getClaims().get("iat"), is(notNullValue())); - // // Allow for millisecond to second conversion flexibility - // assertThat(((Date) signedJWT.getJWTClaimsSet().getClaims().get("exp")).getTime() / 1000, equalTo(Long.MAX_VALUE / 1000)); - // } - // - // @Test - // public void testKeyTooShortForApiTokenThrowsException() { - // String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); - // String tooShortKey = BaseEncoding.base64().encode("short_key".getBytes()); - // Settings settings = Settings.builder().put("signing_key", tooShortKey).put("encryption_key", claimsEncryptionKey).build(); - // final Throwable exception = assertThrows(OpenSearchException.class, () -> { new JwtVendor(settings, Optional.empty()); }); - // - // assertThat(exception.getMessage(), containsString("The secret length must be at least 256 bits")); - // } } diff --git a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java index 1bf03c8dc6..90531dfdb8 100644 --- a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java +++ b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java @@ -39,6 +39,7 @@ import org.opensearch.security.user.UserService; import org.opensearch.threadpool.ThreadPool; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -76,7 +77,6 @@ public void setup() { @After public void after() { - verifyNoMoreInteractions(cs); verifyNoMoreInteractions(userService); } @@ -97,7 +97,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); @@ -120,11 +120,11 @@ public void onDynamicConfigModelChanged_JwtVendorDisabled() { } /** Creates the jwt vendor and returns a mock for validation if needed */ - private DynamicConfigModel createMockJwtVendorInTokenManager() { + private DynamicConfigModel createMockJwtVendorInTokenManager(boolean includeEncryptionKey) { final Settings settings = Settings.builder() .put("enabled", true) - .put("encryption_key", "1234567890") .put("signing_key", signingKeyB64Encoded) + .put("encryption_key", (includeEncryptionKey ? "1234567890" : null)) .build(); final DynamicConfigModel dcm = mock(DynamicConfigModel.class); when(dcm.getDynamicOnBehalfOfSettings()).thenReturn(settings); @@ -218,7 +218,7 @@ public void issueOnBehalfOfToken_jwtGenerationFailure() throws Exception { tokenManager.onConfigModelChanged(configModel); when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); - createMockJwtVendorInTokenManager(); + createMockJwtVendorInTokenManager(true); when(jwtVendor.createJwt(any(), any(), any(), any())).thenThrow(new RuntimeException("foobar")); final OpenSearchSecurityException exception = assertThrows( @@ -242,7 +242,7 @@ 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(), any(), any(), any())).thenReturn(authToken); @@ -264,7 +264,7 @@ public void testCreateJwtWithNegativeExpiry() { tokenManager.onConfigModelChanged(configModel); when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); - createMockJwtVendorInTokenManager(); + createMockJwtVendorInTokenManager(true); final Throwable exception = assertThrows(RuntimeException.class, () -> { try { @@ -276,6 +276,71 @@ public void testCreateJwtWithNegativeExpiry() { assertThat(exception.getMessage(), is("java.lang.IllegalArgumentException: The expiration time should be a positive integer")); } + @Test + public void testCreateJwtWithExceededExpiry() throws Exception { + doAnswer(invockation -> 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", List.of(), null)); + 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", List.of(), null)); + 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", List.of(), null)); + 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")); + } + @Test public void issueApiToken_success() throws Exception { doAnswer(invockation -> new ClusterName("cluster17")).when(cs).getClusterName(); @@ -285,7 +350,7 @@ public void issueApiToken_success() throws Exception { final ConfigModel configModel = mock(ConfigModel.class); tokenManager.onConfigModelChanged(configModel); - createMockJwtVendorInTokenManager(); + createMockJwtVendorInTokenManager(false); final ExpiringBearerAuthToken authToken = mock(ExpiringBearerAuthToken.class); when(jwtVendor.createJwt(any(), any(), any(), any())).thenReturn(authToken); diff --git a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java index c713d8a286..9fdba2b407 100644 --- a/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/RestLayerPrivilegesEvaluatorTest.java @@ -32,7 +32,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.security.action.apitokens.ApiTokenRepository; -import org.opensearch.security.action.apitokens.Permissions; import org.opensearch.security.auditlog.NullAuditLog; import org.opensearch.security.securityconf.ConfigModel; import org.opensearch.security.securityconf.DynamicConfigModel; @@ -41,7 +40,6 @@ import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.user.User; -import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import org.mockito.quality.Strictness; @@ -151,8 +149,6 @@ public void testEvaluate_Unsuccessful() throws Exception { } PrivilegesEvaluator createPrivilegesEvaluator(SecurityDynamicConfiguration roles) { - ApiTokenRepository mockApiTokenRepository = mock(ApiTokenRepository.class); - when(mockApiTokenRepository.getApiTokenPermissionsForUser(ArgumentMatchers.any())).thenReturn(new Permissions()); PrivilegesEvaluator privilegesEvaluator = new PrivilegesEvaluator( clusterService, () -> clusterService.state(), @@ -166,7 +162,7 @@ PrivilegesEvaluator createPrivilegesEvaluator(SecurityDynamicConfiguration