From fbc688b3db6beafb2717314f7e513724e4122b9f Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 29 Sep 2023 16:15:40 -0700 Subject: [PATCH 01/25] Add another test case for jwtvendor Signed-off-by: Ryan Liang --- .../security/authtoken/jwt/JwtVendorTest.java | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) 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 aa8faa284d..d38f517457 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -124,7 +124,7 @@ public void testCreateJwtWithRoleSecurityMode() throws Exception { } @Test - public void testCreateJwtWithBadExpiry() { + public void testCreateJwtWithNegativeExpiry() { String issuer = "cluster_0"; String subject = "admin"; String audience = "audience_0"; @@ -144,6 +144,32 @@ public void testCreateJwtWithBadExpiry() { Assert.assertEquals("java.lang.Exception: The expiration time should be a positive integer", exception.getMessage()); } + @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; + LongSupplier currentTime = () -> (long) 100; + String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); + Settings settings = Settings.builder().put("signing_key", "abc123").put("encryption_key", claimsEncryptionKey).build(); + JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); + + Throwable exception = Assert.assertThrows(RuntimeException.class, () -> { + try { + jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, true); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + Assert.assertEquals( + "java.lang.Exception: The provided expiration time exceeds the maximum allowed duration of 600 seconds", + exception.getMessage() + ); + } + @Test public void testCreateJwtWithBadEncryptionKey() { String issuer = "cluster_0"; From 43fdac37dcd8a995535b7790d7f0bf9e892cefe2 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Mon, 2 Oct 2023 11:12:41 -0700 Subject: [PATCH 02/25] Fix lint Signed-off-by: Ryan Liang --- .../org/opensearch/security/authtoken/jwt/JwtVendorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d38f517457..663c4fd1e2 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -165,8 +165,8 @@ public void testCreateJwtWithExceededExpiry() throws Exception { } }); Assert.assertEquals( - "java.lang.Exception: The provided expiration time exceeds the maximum allowed duration of 600 seconds", - exception.getMessage() + "java.lang.Exception: The provided expiration time exceeds the maximum allowed duration of 600 seconds", + exception.getMessage() ); } From edfa29254cbc34d09b616c50a55b74e21025ef3b Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 4 Oct 2023 23:12:51 -0700 Subject: [PATCH 03/25] Add the test to the jwtVendor logger Signed-off-by: Ryan Liang --- .../security/authtoken/jwt/JwtVendorTest.java | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) 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 663c4fd1e2..da5e41c656 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -15,8 +15,15 @@ import org.apache.cxf.rs.security.jose.jwk.JsonWebKey; import org.apache.cxf.rs.security.jose.jws.JwsJwtCompactConsumer; import org.apache.cxf.rs.security.jose.jwt.JwtToken; +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.mockito.ArgumentCaptor; +import org.mockito.Mockito; import org.opensearch.common.settings.Settings; import org.opensearch.security.support.ConfigConstants; @@ -24,7 +31,12 @@ import java.util.Optional; import java.util.function.LongSupplier; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + public class JwtVendorTest { + private Appender mockAppender; + private ArgumentCaptor logEventCaptor; @Test public void testCreateJwkFromSettings() throws Exception { @@ -210,4 +222,44 @@ public void testCreateJwtWithBadRoles() { }); Assert.assertEquals("java.lang.Exception: Roles cannot be null", exception.getMessage()); } + + @Test + public void testCreateJwtLogsCorrectly() throws Exception { + mockAppender = Mockito.mock(Appender.class); + logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); + Mockito.when(mockAppender.getName()).thenReturn("MockAppender"); + Mockito.when(mockAppender.isStarted()).thenReturn(true); + 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", "abc123").put("encryption_key", claimsEncryptionKey).build(); + + 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 = 300; + + // Create instance of JwtVendor + JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); + + // Call the method under test + jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, false); + + // Verify the log was captured + verify(mockAppender, times(1)).append(logEventCaptor.capture()); + + // Check the log message + LogEvent logEvent = logEventCaptor.getValue(); + String logMessage = logEvent.getMessage().getFormattedMessage(); + Assert.assertTrue(logMessage.startsWith("Created JWT:")); + + String[] parts = logMessage.split("\\."); + Assert.assertTrue(parts.length >= 3); // JWTs typically have 3 parts: Header.Payload.Signature + } } From d4040147f067d7a377cc3950dbb03b3401d4c80a Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Mon, 9 Oct 2023 11:44:18 -0400 Subject: [PATCH 04/25] Add test for jwk Signed-off-by: Ryan Liang --- .../security/authtoken/jwt/JwtVendorTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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 da5e41c656..7bb79c3556 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -38,6 +38,19 @@ public class JwtVendorTest { private Appender mockAppender; private ArgumentCaptor logEventCaptor; + @Test + public void testJsonWebKeyPropertiesSetFromSettings() { + Settings jwkSettings = Settings.builder().put("key1", "value1").put("key2", "value2").build(); + + JsonWebKey jwk = new JsonWebKey(); + for (String key : jwkSettings.keySet()) { + jwk.setProperty(key, jwkSettings.get(key)); + } + + Assert.assertEquals("value1", jwk.getProperty("key1")); + Assert.assertEquals("value2", jwk.getProperty("key2")); + } + @Test public void testCreateJwkFromSettings() throws Exception { Settings settings = Settings.builder().put("signing_key", "abc123").build(); From 7bf71ba8657ebdd2e2dead101d21ef3252464581 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Mon, 9 Oct 2023 14:27:51 -0400 Subject: [PATCH 05/25] Modify the test of jwk and remove useless comments Signed-off-by: Ryan Liang --- .../security/authtoken/jwt/JwtVendorTest.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) 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 7bb79c3556..a0da50c361 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -38,6 +38,16 @@ public class JwtVendorTest { private Appender mockAppender; private ArgumentCaptor logEventCaptor; + @Test + public void testJsonWebKeyPropertiesSetFromJwkSettings() throws Exception { + Settings settings = Settings.builder().put("jwt.key.key1", "value1").put("jwt.key.key2", "value2").build(); + + JsonWebKey jwk = JwtVendor.createJwkFromSettings(settings); + + Assert.assertEquals("value1", jwk.getProperty("key1")); + Assert.assertEquals("value2", jwk.getProperty("key2")); + } + @Test public void testJsonWebKeyPropertiesSetFromSettings() { Settings jwkSettings = Settings.builder().put("key1", "value1").put("key2", "value2").build(); @@ -258,21 +268,17 @@ public void testCreateJwtLogsCorrectly() throws Exception { List backendRoles = List.of("Sales", "Support"); int expirySeconds = 300; - // Create instance of JwtVendor JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); - // Call the method under test jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, false); - // Verify the log was captured verify(mockAppender, times(1)).append(logEventCaptor.capture()); - // Check the log message LogEvent logEvent = logEventCaptor.getValue(); String logMessage = logEvent.getMessage().getFormattedMessage(); Assert.assertTrue(logMessage.startsWith("Created JWT:")); String[] parts = logMessage.split("\\."); - Assert.assertTrue(parts.length >= 3); // JWTs typically have 3 parts: Header.Payload.Signature + Assert.assertTrue(parts.length >= 3); } } From 027cd78899279ef1bf1745389667cbc530bb5f90 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Mon, 9 Oct 2023 15:31:13 -0400 Subject: [PATCH 06/25] Add tests for obo authenticator for rerequest and type Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 1ff6adee3a..03e02f4de2 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -12,11 +12,7 @@ package org.opensearch.security.http; import java.nio.charset.StandardCharsets; -import java.util.Base64; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.Map; +import java.util.*; import javax.crypto.SecretKey; @@ -31,6 +27,7 @@ import org.junit.Test; import org.opensearch.common.settings.Settings; +import org.opensearch.security.filter.SecurityResponse; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.util.FakeRestRequest; @@ -47,6 +44,20 @@ public class OnBehalfOfAuthenticatorTest { final static String signingKeyB64Encoded = BaseEncoding.base64().encode(signingKey.getBytes(StandardCharsets.UTF_8)); final static SecretKey secretKey = Keys.hmacShaKeyFor(signingKeyB64Encoded.getBytes(StandardCharsets.UTF_8)); + @Test + public void testReRequestAuthenticationReturnsEmptyOptional() { + OnBehalfOfAuthenticator authenticator = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); + Optional result = authenticator.reRequestAuthentication(null, null); + Assert.assertFalse(result.isPresent()); + } + + @Test + public void testGetTypeReturnsExpectedType() { + OnBehalfOfAuthenticator authenticator = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); + String type = authenticator.getType(); + Assert.assertEquals("onbehalfof_jwt", type); + } + @Test public void testNoKey() { Exception exception = Assert.assertThrows( From 56899cb94ea87f8dd8da966f2ca9b2a483c9448e Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Mon, 9 Oct 2023 16:57:05 -0400 Subject: [PATCH 07/25] Add tests for obo authenticator about er claim Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 03e02f4de2..bb03703ea8 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -27,6 +27,7 @@ import org.junit.Test; import org.opensearch.common.settings.Settings; +import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; import org.opensearch.security.filter.SecurityResponse; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.util.FakeRestRequest; @@ -242,7 +243,7 @@ public void testBasicAuthHeader() throws Exception { } @Test - public void testRoles() throws Exception { + public void testPlainTextedRolesFromDrClaim() { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, @@ -257,6 +258,23 @@ public void testRoles() throws Exception { Assert.assertEquals(0, credentials.getBackendRoles().size()); } + @Test + public void testRolesDecryptionFromErClaim() { + EncryptionDecryptionUtil util = new EncryptionDecryptionUtil(claimsEncryptionKey); + String encryptedRole = util.encrypt("admin,developer"); + + final AuthCredentials credentials = extractCredentialsFromJwtHeader( + signingKeyB64Encoded, + claimsEncryptionKey, + Jwts.builder().setIssuer(clusterName).setSubject("Test User").setAudience("audience_0").claim("er", encryptedRole), + true + ); + + Assert.assertNotNull(credentials); + List expectedRoles = Arrays.asList("admin", "developer"); + Assert.assertTrue(credentials.getSecurityRoles().containsAll(expectedRoles)); + } + @Test public void testNullClaim() throws Exception { From c49fa23eea185c2c43bb004c11decbe6751d8595 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Mon, 9 Oct 2023 23:08:01 -0400 Subject: [PATCH 08/25] Add the test for backend roles extraction Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index bb03703ea8..7464cdbd06 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -258,6 +258,29 @@ public void testPlainTextedRolesFromDrClaim() { Assert.assertEquals(0, credentials.getBackendRoles().size()); } + @Test + public void testBackendRolesExtraction() { + String rolesString = "role1, role2 ,role3,role4 , role5"; + + final AuthCredentials credentials = extractCredentialsFromJwtHeader( + signingKeyB64Encoded, + claimsEncryptionKey, + Jwts.builder() + .setIssuer(clusterName) + .setSubject("Test User") + .setAudience("audience_0") + .claim("br", rolesString), + true + ); + + Assert.assertNotNull(credentials); + + Set expectedBackendRoles = new HashSet<>(Arrays.asList("role1", "role2", "role3", "role4", "role5")); + Set actualBackendRoles = credentials.getBackendRoles(); + + Assert.assertTrue(actualBackendRoles.containsAll(expectedBackendRoles)); + } + @Test public void testRolesDecryptionFromErClaim() { EncryptionDecryptionUtil util = new EncryptionDecryptionUtil(claimsEncryptionKey); From 189efcdf9c885ae1963d9c3339c1584367a59bb3 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Mon, 9 Oct 2023 23:08:48 -0400 Subject: [PATCH 09/25] Fix the lint Signed-off-by: Ryan Liang --- .../security/http/OnBehalfOfAuthenticatorTest.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 7464cdbd06..6ee2bbb35f 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -263,14 +263,10 @@ public void testBackendRolesExtraction() { String rolesString = "role1, role2 ,role3,role4 , role5"; final AuthCredentials credentials = extractCredentialsFromJwtHeader( - signingKeyB64Encoded, - claimsEncryptionKey, - Jwts.builder() - .setIssuer(clusterName) - .setSubject("Test User") - .setAudience("audience_0") - .claim("br", rolesString), - true + signingKeyB64Encoded, + claimsEncryptionKey, + Jwts.builder().setIssuer(clusterName).setSubject("Test User").setAudience("audience_0").claim("br", rolesString), + true ); Assert.assertNotNull(credentials); From ea035614285add57e66fa451d6f1140604a48bf0 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 10 Oct 2023 11:22:42 -0400 Subject: [PATCH 10/25] Add SM test Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 6ee2bbb35f..59eda83003 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -26,6 +26,7 @@ import org.junit.Assert; import org.junit.Test; +import org.opensearch.SpecialPermission; import org.opensearch.common.settings.Settings; import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; import org.opensearch.security.filter.SecurityResponse; @@ -33,6 +34,8 @@ import org.opensearch.security.util.FakeRestRequest; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; public class OnBehalfOfAuthenticatorTest { final static String clusterName = "cluster_0"; @@ -223,6 +226,24 @@ public void testBearerWrongPosition() throws Exception { Assert.assertNull(credentials); } + @Test + public void testSecurityManagerCheck() { + SecurityManager mockSecurityManager = mock(SecurityManager.class); + System.setSecurityManager(mockSecurityManager); + + OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); + Map headers = new HashMap<>(); + headers.put("Authorization", "Bearer someToken"); + + try { + jwtAuth.extractCredentials(new FakeRestRequest(headers, new HashMap<>()).asSecurityRequest(), null); + } finally { + System.setSecurityManager(null); // Reset to the default security manager + } + + verify(mockSecurityManager, times(2)).checkPermission(any(SpecialPermission.class)); + } + @Test public void testBasicAuthHeader() throws Exception { String jwsToken = Jwts.builder() From 4e240455754cd4571dda6e51b58ab43f487f55cb Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 10 Oct 2023 11:24:08 -0400 Subject: [PATCH 11/25] Remove the comment Signed-off-by: Ryan Liang --- .../opensearch/security/http/OnBehalfOfAuthenticatorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 59eda83003..5ede957d3d 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -238,7 +238,7 @@ public void testSecurityManagerCheck() { try { jwtAuth.extractCredentials(new FakeRestRequest(headers, new HashMap<>()).asSecurityRequest(), null); } finally { - System.setSecurityManager(null); // Reset to the default security manager + System.setSecurityManager(null); } verify(mockSecurityManager, times(2)).checkPermission(any(SpecialPermission.class)); From eef1957519c92a592c0338a29708e12ad8a965ab Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 10 Oct 2023 11:40:24 -0400 Subject: [PATCH 12/25] Add audience test Signed-off-by: Ryan Liang --- .../security/http/OnBehalfOfAuthenticatorTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 5ede957d3d..7560869a05 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -375,6 +375,19 @@ public void testWrongSubjectKey() throws Exception { Assert.assertNull(credentials); } + @Test + public void testMissingAudienceClaim() throws Exception { + + final AuthCredentials credentials = extractCredentialsFromJwtHeader( + signingKeyB64Encoded, + claimsEncryptionKey, + Jwts.builder().setIssuer(clusterName).setSubject("Test User").claim("roles", "role1,role2"), + false + ); + + Assert.assertNull(credentials); + } + @Test public void testExp() throws Exception { From e368dabe8c484952cadb956b7c6ebbc6d4750819 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 10 Oct 2023 12:31:03 -0400 Subject: [PATCH 13/25] Test Restricted endpoints Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 7560869a05..258f262d5b 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -26,9 +26,12 @@ import org.junit.Assert; import org.junit.Test; +import org.mockito.Mockito; import org.opensearch.SpecialPermission; import org.opensearch.common.settings.Settings; +import org.opensearch.rest.RestRequest; import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; +import org.opensearch.security.filter.SecurityRequest; import org.opensearch.security.filter.SecurityResponse; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.util.FakeRestRequest; @@ -36,6 +39,8 @@ import static org.hamcrest.Matchers.equalTo; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.rest.RestRequest.Method.PUT; public class OnBehalfOfAuthenticatorTest { final static String clusterName = "cluster_0"; @@ -48,6 +53,9 @@ public class OnBehalfOfAuthenticatorTest { final static String signingKeyB64Encoded = BaseEncoding.base64().encode(signingKey.getBytes(StandardCharsets.UTF_8)); final static SecretKey secretKey = Keys.hmacShaKeyFor(signingKeyB64Encoded.getBytes(StandardCharsets.UTF_8)); + private static final String ON_BEHALF_OF_SUFFIX = "api/generateonbehalfoftoken"; + private static final String ACCOUNT_SUFFIX = "api/account"; + @Test public void testReRequestAuthenticationReturnsEmptyOptional() { OnBehalfOfAuthenticator authenticator = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); @@ -460,6 +468,26 @@ public void testDifferentIssuer() throws Exception { Assert.assertNull(credentials); } + @Test + public void testExtractCredentialsForDisallowedRequest() { + OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); + + AuthCredentials credentials = testEndpoint(jwtAuth, ON_BEHALF_OF_SUFFIX, String.valueOf(POST)); + Assert.assertNull(credentials); + + credentials = testEndpoint(jwtAuth, ACCOUNT_SUFFIX, String.valueOf(PUT)); + Assert.assertNull(credentials); + } + + private AuthCredentials testEndpoint(OnBehalfOfAuthenticator jwtAuth, String endpoint, String httpMethod) { + SecurityRequest mockedRequest = Mockito.mock(SecurityRequest.class); + Mockito.when(mockedRequest.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); + Mockito.when(mockedRequest.method()).thenReturn(RestRequest.Method.valueOf(httpMethod)); + Mockito.when(mockedRequest.path()).thenReturn("/some_prefix/" + endpoint); + + return jwtAuth.extractCredentials(mockedRequest, null); + } + /** extracts a default user credential from a request header */ private AuthCredentials extractCredentialsFromJwtHeader( final String signingKeyB64Encoded, From 168f9252f72f3c2006247305eb20e27633440558 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 10 Oct 2023 12:55:41 -0400 Subject: [PATCH 14/25] Fix the checkstyle Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 258f262d5b..80a7d1278b 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -12,7 +12,16 @@ package org.opensearch.security.http; import java.nio.charset.StandardCharsets; -import java.util.*; +import java.util.Base64; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.List; +import java.util.HashSet; +import java.util.Arrays; +import java.util.Optional; import javax.crypto.SecretKey; @@ -38,7 +47,9 @@ import static org.hamcrest.Matchers.equalTo; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.opensearch.rest.RestRequest.Method.POST; import static org.opensearch.rest.RestRequest.Method.PUT; @@ -480,7 +491,7 @@ public void testExtractCredentialsForDisallowedRequest() { } private AuthCredentials testEndpoint(OnBehalfOfAuthenticator jwtAuth, String endpoint, String httpMethod) { - SecurityRequest mockedRequest = Mockito.mock(SecurityRequest.class); + SecurityRequest mockedRequest = mock(SecurityRequest.class); Mockito.when(mockedRequest.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); Mockito.when(mockedRequest.method()).thenReturn(RestRequest.Method.valueOf(httpMethod)); Mockito.when(mockedRequest.path()).thenReturn("/some_prefix/" + endpoint); From f8e6c137f3bce110712a5d0ba089f117c948a825 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 10 Oct 2023 23:32:12 -0400 Subject: [PATCH 15/25] Add endpoints testing cases Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 80a7d1278b..8f66d56754 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -64,6 +64,7 @@ public class OnBehalfOfAuthenticatorTest { final static String signingKeyB64Encoded = BaseEncoding.base64().encode(signingKey.getBytes(StandardCharsets.UTF_8)); final static SecretKey secretKey = Keys.hmacShaKeyFor(signingKeyB64Encoded.getBytes(StandardCharsets.UTF_8)); + private static final String SECURITY_PREFIX = "/_plugins/_security/"; private static final String ON_BEHALF_OF_SUFFIX = "api/generateonbehalfoftoken"; private static final String ACCOUNT_SUFFIX = "api/account"; @@ -480,23 +481,22 @@ public void testDifferentIssuer() throws Exception { } @Test - public void testExtractCredentialsForDisallowedRequest() { - OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); - - AuthCredentials credentials = testEndpoint(jwtAuth, ON_BEHALF_OF_SUFFIX, String.valueOf(POST)); - Assert.assertNull(credentials); - - credentials = testEndpoint(jwtAuth, ACCOUNT_SUFFIX, String.valueOf(PUT)); - Assert.assertNull(credentials); - } - - private AuthCredentials testEndpoint(OnBehalfOfAuthenticator jwtAuth, String endpoint, String httpMethod) { - SecurityRequest mockedRequest = mock(SecurityRequest.class); - Mockito.when(mockedRequest.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); - Mockito.when(mockedRequest.method()).thenReturn(RestRequest.Method.valueOf(httpMethod)); - Mockito.when(mockedRequest.path()).thenReturn("/some_prefix/" + endpoint); - - return jwtAuth.extractCredentials(mockedRequest, null); + public void testRequestNotAllowed() { + OnBehalfOfAuthenticator oboAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); + + //Test POST on generate on-behalf-of token endpoint + SecurityRequest mockedRequest1 = mock(SecurityRequest.class); + Mockito.when(mockedRequest1.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); + Mockito.when(mockedRequest1.path()).thenReturn(SECURITY_PREFIX + ON_BEHALF_OF_SUFFIX); + Mockito.when(mockedRequest1.method()).thenReturn(POST); + Assert.assertFalse(oboAuth.isRequestAllowed(mockedRequest1)); + + //Test PUT on password changing endpoint + SecurityRequest mockedRequest2 = mock(SecurityRequest.class); + Mockito.when(mockedRequest2.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); + Mockito.when(mockedRequest2.path()).thenReturn(SECURITY_PREFIX + ACCOUNT_SUFFIX); + Mockito.when(mockedRequest2.method()).thenReturn(PUT); + Assert.assertFalse(oboAuth.isRequestAllowed(mockedRequest2)); } /** extracts a default user credential from a request header */ From 2a91082206e4ceaac76fb6a427c7e1ec77512019 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 10 Oct 2023 23:41:39 -0400 Subject: [PATCH 16/25] Fix lint Signed-off-by: Ryan Liang --- .../security/http/OnBehalfOfAuthenticatorTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 8f66d56754..b538c26321 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -38,7 +38,6 @@ import org.mockito.Mockito; import org.opensearch.SpecialPermission; import org.opensearch.common.settings.Settings; -import org.opensearch.rest.RestRequest; import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; import org.opensearch.security.filter.SecurityRequest; import org.opensearch.security.filter.SecurityResponse; @@ -484,14 +483,14 @@ public void testDifferentIssuer() throws Exception { public void testRequestNotAllowed() { OnBehalfOfAuthenticator oboAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); - //Test POST on generate on-behalf-of token endpoint + // Test POST on generate on-behalf-of token endpoint SecurityRequest mockedRequest1 = mock(SecurityRequest.class); Mockito.when(mockedRequest1.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); Mockito.when(mockedRequest1.path()).thenReturn(SECURITY_PREFIX + ON_BEHALF_OF_SUFFIX); Mockito.when(mockedRequest1.method()).thenReturn(POST); Assert.assertFalse(oboAuth.isRequestAllowed(mockedRequest1)); - //Test PUT on password changing endpoint + // Test PUT on password changing endpoint SecurityRequest mockedRequest2 = mock(SecurityRequest.class); Mockito.when(mockedRequest2.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); Mockito.when(mockedRequest2.path()).thenReturn(SECURITY_PREFIX + ACCOUNT_SUFFIX); From 3f542db0113424be4cea246c06dbfb010fb9257c Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 11 Oct 2023 00:41:42 -0400 Subject: [PATCH 17/25] Add null check for endpoint testing Signed-off-by: Ryan Liang --- .../opensearch/security/http/OnBehalfOfAuthenticatorTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index b538c26321..0b81c23554 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -489,6 +489,7 @@ public void testRequestNotAllowed() { Mockito.when(mockedRequest1.path()).thenReturn(SECURITY_PREFIX + ON_BEHALF_OF_SUFFIX); Mockito.when(mockedRequest1.method()).thenReturn(POST); Assert.assertFalse(oboAuth.isRequestAllowed(mockedRequest1)); + Assert.assertNull(oboAuth.extractCredentials(mockedRequest1, null)); // Test PUT on password changing endpoint SecurityRequest mockedRequest2 = mock(SecurityRequest.class); @@ -496,6 +497,7 @@ public void testRequestNotAllowed() { Mockito.when(mockedRequest2.path()).thenReturn(SECURITY_PREFIX + ACCOUNT_SUFFIX); Mockito.when(mockedRequest2.method()).thenReturn(PUT); Assert.assertFalse(oboAuth.isRequestAllowed(mockedRequest2)); + Assert.assertNull(oboAuth.extractCredentials(mockedRequest2, null)); } /** extracts a default user credential from a request header */ From ffda6b00c2974cd41c29eb00ee1e230d5aecc6b0 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 11 Oct 2023 10:19:36 -0400 Subject: [PATCH 18/25] Add missing bearer test Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 0b81c23554..6df60bfdab 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -282,6 +282,27 @@ public void testBasicAuthHeader() throws Exception { Assert.assertNull(credentials); } + @Test + public void testMissingBearerPrefixInAuthHeader() { + String jwsToken = Jwts.builder() + .setIssuer(clusterName) + .setSubject("Leonard McCoy") + .setAudience("ext_0") + .signWith(secretKey, SignatureAlgorithm.HS512) + .compact(); + + OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); + + Map headers = Collections.singletonMap(HttpHeaders.AUTHORIZATION, jwsToken); + + AuthCredentials credentials = jwtAuth.extractCredentials( + new FakeRestRequest(headers, Collections.emptyMap()).asSecurityRequest(), + null + ); + + Assert.assertNull(credentials); + } + @Test public void testPlainTextedRolesFromDrClaim() { From 2594b31629751934584eb58297967e4e1ecab132 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 11 Oct 2023 11:26:58 -0400 Subject: [PATCH 19/25] Remove useless check for jwtparser Signed-off-by: Ryan Liang --- .../opensearch/security/http/OnBehalfOfAuthenticator.java | 5 ----- .../security/http/OnBehalfOfAuthenticatorTest.java | 4 ++++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index 8499b88f62..9ff082ba9b 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -145,11 +145,6 @@ private AuthCredentials extractCredentials0(final SecurityRequest request) { return null; } - if (jwtParser == null) { - log.error("Missing Signing Key. JWT authentication will not work"); - return null; - } - String jwtToken = extractJwtFromHeader(request); if (jwtToken == null) { return null; diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 6df60bfdab..d955325083 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -561,6 +561,10 @@ private Settings disableOBOSettings() { .build(); } + private Settings noSigningKeyOBOSettings() { + return Settings.builder().put("enabled", disableOBO).put("encryption_key", claimsEncryptionKey).build(); + } + private Settings nonSpecifyOBOSetting() { return Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); } From 986ebb324671ba0e58fa2dc1b97e9665c1636ba3 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 11 Oct 2023 14:21:51 -0400 Subject: [PATCH 20/25] Add test for invalid jwt exception Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index d955325083..df1f38efad 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -32,9 +32,15 @@ import io.jsonwebtoken.security.Keys; import org.apache.commons.lang3.RandomStringUtils; import org.apache.hc.core5.http.HttpHeaders; +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.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.opensearch.SpecialPermission; import org.opensearch.common.settings.Settings; @@ -173,6 +179,39 @@ public void testDisabled() throws Exception { Assert.assertNull(credentials); } + @Test + public void testInvalidTokenException() { + Appender mockAppender = Mockito.mock(Appender.class); + ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); + Mockito.when(mockAppender.getName()).thenReturn("MockAppender"); + Mockito.when(mockAppender.isStarted()).thenReturn(true); + Logger logger = (Logger) LogManager.getLogger(OnBehalfOfAuthenticator.class); + logger.addAppender(mockAppender); + logger.setLevel(Level.DEBUG); + Mockito.doNothing().when(mockAppender).append(logEventCaptor.capture()); + + String invalidToken = "invalidToken"; + Settings settings = defaultSettings(); + + OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(settings, clusterName); + + Map headers = Collections.singletonMap(HttpHeaders.AUTHORIZATION, "Bearer " + invalidToken); + + AuthCredentials credentials = jwtAuth.extractCredentials( + new FakeRestRequest(headers, Collections.emptyMap()).asSecurityRequest(), + null + ); + + Assert.assertNull(credentials); + + boolean foundLog = logEventCaptor.getAllValues() + .stream() + .anyMatch(event -> event.getMessage().getFormattedMessage().contains("Invalid or expired JWT token.")); + Assert.assertTrue(foundLog); + + logger.removeAppender(mockAppender); + } + @Test public void testNonSpecifyOBOSetting() throws Exception { String jwsToken = Jwts.builder() From c7082416fa0ecf946beb3c59253879d463f3e1fa Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 11 Oct 2023 15:26:11 -0400 Subject: [PATCH 21/25] Add test for MissingBearerScheme Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticator.java | 10 ++---- .../http/OnBehalfOfAuthenticatorTest.java | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index 9ff082ba9b..56da7ade68 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -206,17 +206,13 @@ private String extractJwtFromHeader(SecurityRequest request) { return null; } - if (!BEARER.matcher(jwtToken).matches()) { - return null; - } - - if (jwtToken.toLowerCase().contains(BEARER_PREFIX)) { - jwtToken = jwtToken.substring(jwtToken.toLowerCase().indexOf(BEARER_PREFIX) + BEARER_PREFIX.length()); - } else { + if (!BEARER.matcher(jwtToken).matches() || !jwtToken.toLowerCase().contains(BEARER_PREFIX)) { logDebug("No Bearer scheme found in header"); return null; } + jwtToken = jwtToken.substring(jwtToken.toLowerCase().indexOf(BEARER_PREFIX) + BEARER_PREFIX.length()); + return jwtToken; } diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index df1f38efad..57e8343882 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -321,6 +321,37 @@ public void testBasicAuthHeader() throws Exception { Assert.assertNull(credentials); } + @Test + public void testMissingBearerScheme() throws Exception { + Appender mockAppender = Mockito.mock(Appender.class); + ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); + Mockito.when(mockAppender.getName()).thenReturn("MockAppender"); + Mockito.when(mockAppender.isStarted()).thenReturn(true); + Logger logger = (Logger) LogManager.getLogger(OnBehalfOfAuthenticator.class); + logger.addAppender(mockAppender); + logger.setLevel(Level.DEBUG); + Mockito.doNothing().when(mockAppender).append(logEventCaptor.capture()); + + String craftedToken = "beaRerSomeActualToken"; // This token matches the BEARER pattern but doesn't contain the BEARER_PREFIX + + OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); + Map headers = Collections.singletonMap(HttpHeaders.AUTHORIZATION, craftedToken); + + AuthCredentials credentials = jwtAuth.extractCredentials( + new FakeRestRequest(headers, Collections.emptyMap()).asSecurityRequest(), + null + ); + + Assert.assertNull(credentials); + + boolean foundLog = logEventCaptor.getAllValues() + .stream() + .anyMatch(event -> event.getMessage().getFormattedMessage().contains("No Bearer scheme found in header")); + Assert.assertTrue(foundLog); + + logger.removeAppender(mockAppender); + } + @Test public void testMissingBearerPrefixInAuthHeader() { String jwsToken = Jwts.builder() From 0cc1896b15abf68f464ad777d56ae3a4056727e0 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 11 Oct 2023 16:30:22 -0400 Subject: [PATCH 22/25] Add tests for jwk exception Signed-off-by: Ryan Liang --- .../security/authtoken/jwt/JwtVendorTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 a0da50c361..45ead74784 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -38,6 +38,16 @@ public class JwtVendorTest { private Appender mockAppender; private ArgumentCaptor logEventCaptor; + @Test + public void testCreateJwkFromSettingsThrowsException() { + Settings faultySettings = Settings.builder().put("key.someProperty", "badValue").build(); + + Exception thrownException = Assert.assertThrows(Exception.class, () -> new JwtVendor(faultySettings, null)); + + String expectedMessagePart = "An error occurred during the creation of Jwk: "; + Assert.assertTrue(thrownException.getMessage().contains(expectedMessagePart)); + } + @Test public void testJsonWebKeyPropertiesSetFromJwkSettings() throws Exception { Settings settings = Settings.builder().put("jwt.key.key1", "value1").put("jwt.key.key2", "value2").build(); From 07be8e13991102ed5aca2782ba14c0166dbdeb50 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 11 Oct 2023 17:16:27 -0400 Subject: [PATCH 23/25] Return null for obo authenticator Signed-off-by: Ryan Liang --- .../org/opensearch/security/http/OnBehalfOfAuthenticator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index 56da7ade68..4ac3be335f 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -188,6 +188,7 @@ private AuthCredentials extractCredentials0(final SecurityRequest request) { } catch (WeakKeyException e) { log.error("Cannot authenticate user with JWT because of ", e); + return null; } catch (Exception e) { if (log.isDebugEnabled()) { log.debug("Invalid or expired JWT token.", e); From 8a851ecca13102e712df844a99df8ebed8d26682 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Wed, 11 Oct 2023 18:07:10 -0400 Subject: [PATCH 24/25] Add weak key exception test Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticatorTest.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 57e8343882..94acf4c052 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -11,6 +11,7 @@ package org.opensearch.security.http; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.Collections; @@ -27,14 +28,17 @@ import com.google.common.io.BaseEncoding; import io.jsonwebtoken.JwtBuilder; +import io.jsonwebtoken.JwtParser; import io.jsonwebtoken.Jwts; import io.jsonwebtoken.SignatureAlgorithm; import io.jsonwebtoken.security.Keys; +import io.jsonwebtoken.security.WeakKeyException; import org.apache.commons.lang3.RandomStringUtils; import org.apache.hc.core5.http.HttpHeaders; 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.ErrorHandler; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.Logger; import org.junit.Assert; @@ -129,6 +133,44 @@ public void testBadKey() { Assert.assertTrue(exception.getMessage().contains("The specified key byte array is 80 bits")); } + @Test + public void testWeakKeyExceptionHandling() throws Exception { + Appender mockAppender = Mockito.mock(Appender.class); + ErrorHandler mockErrorHandler = Mockito.mock(ErrorHandler.class); + Mockito.when(mockAppender.getHandler()).thenReturn(mockErrorHandler); + Mockito.when(mockAppender.isStarted()).thenReturn(true); + + ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); + Mockito.when(mockAppender.getName()).thenReturn("MockAppender"); + Mockito.doNothing().when(mockAppender).append(logEventCaptor.capture()); + + Logger logger = (Logger) LogManager.getLogger(OnBehalfOfAuthenticator.class); + logger.addAppender(mockAppender); + + JwtParser mockJwtParser = Mockito.mock(JwtParser.class); + Mockito.when(mockJwtParser.parseClaimsJws(Mockito.anyString())).thenThrow(new WeakKeyException("Test Exception")); + + Settings settings = Settings.builder().put("signing_key", "testKey").put("encryption_key", claimsEncryptionKey).build(); + OnBehalfOfAuthenticator auth = new OnBehalfOfAuthenticator(settings, "testCluster"); + + Field jwtParserField = OnBehalfOfAuthenticator.class.getDeclaredField("jwtParser"); + jwtParserField.setAccessible(true); + jwtParserField.set(auth, mockJwtParser); + + SecurityRequest mockedRequest = Mockito.mock(SecurityRequest.class); + Mockito.when(mockedRequest.header(Mockito.anyString())).thenReturn("Bearer testToken"); + Mockito.when(mockedRequest.path()).thenReturn("/some/sample/path"); + + auth.extractCredentials(mockedRequest, null); + + boolean foundLog = logEventCaptor.getAllValues() + .stream() + .anyMatch(event -> event.getMessage().getFormattedMessage().contains("Cannot authenticate user with JWT because of ")); + Assert.assertTrue(foundLog); + + logger.removeAppender(mockAppender); + } + @Test public void testTokenMissing() throws Exception { From ac5fa2db8fe60a06745f7e7b5b6af42f9ab578c9 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Wed, 11 Oct 2023 20:23:13 -0400 Subject: [PATCH 25/25] Cleans up and add static imports Signed-off-by: Darshit Chanpura --- .../security/authtoken/jwt/JwtVendorTest.java | 91 ++++----- .../http/OnBehalfOfAuthenticatorTest.java | 180 +++++++++--------- 2 files changed, 142 insertions(+), 129 deletions(-) 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 45ead74784..03cbd20b42 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -20,10 +20,8 @@ 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.mockito.ArgumentCaptor; -import org.mockito.Mockito; import org.opensearch.common.settings.Settings; import org.opensearch.security.support.ConfigConstants; @@ -31,8 +29,15 @@ import java.util.Optional; import java.util.function.LongSupplier; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +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; @@ -42,10 +47,10 @@ public class JwtVendorTest { public void testCreateJwkFromSettingsThrowsException() { Settings faultySettings = Settings.builder().put("key.someProperty", "badValue").build(); - Exception thrownException = Assert.assertThrows(Exception.class, () -> new JwtVendor(faultySettings, null)); + Exception thrownException = assertThrows(Exception.class, () -> new JwtVendor(faultySettings, null)); String expectedMessagePart = "An error occurred during the creation of Jwk: "; - Assert.assertTrue(thrownException.getMessage().contains(expectedMessagePart)); + assertTrue(thrownException.getMessage().contains(expectedMessagePart)); } @Test @@ -54,8 +59,8 @@ public void testJsonWebKeyPropertiesSetFromJwkSettings() throws Exception { JsonWebKey jwk = JwtVendor.createJwkFromSettings(settings); - Assert.assertEquals("value1", jwk.getProperty("key1")); - Assert.assertEquals("value2", jwk.getProperty("key2")); + assertEquals("value1", jwk.getProperty("key1")); + assertEquals("value2", jwk.getProperty("key2")); } @Test @@ -67,8 +72,8 @@ public void testJsonWebKeyPropertiesSetFromSettings() { jwk.setProperty(key, jwkSettings.get(key)); } - Assert.assertEquals("value1", jwk.getProperty("key1")); - Assert.assertEquals("value2", jwk.getProperty("key2")); + assertEquals("value1", jwk.getProperty("key1")); + assertEquals("value2", jwk.getProperty("key2")); } @Test @@ -76,22 +81,22 @@ public void testCreateJwkFromSettings() throws Exception { Settings settings = Settings.builder().put("signing_key", "abc123").build(); JsonWebKey jwk = JwtVendor.createJwkFromSettings(settings); - Assert.assertEquals("HS512", jwk.getAlgorithm()); - Assert.assertEquals("sig", jwk.getPublicKeyUse().toString()); - Assert.assertEquals("abc123", jwk.getProperty("k")); + assertEquals("HS512", jwk.getAlgorithm()); + assertEquals("sig", jwk.getPublicKeyUse().toString()); + assertEquals("abc123", jwk.getProperty("k")); } @Test public void testCreateJwkFromSettingsWithoutSigningKey() { Settings settings = Settings.builder().put("jwt", "").build(); - Throwable exception = Assert.assertThrows(RuntimeException.class, () -> { + Throwable exception = assertThrows(RuntimeException.class, () -> { try { JwtVendor.createJwkFromSettings(settings); } catch (Exception e) { throw new RuntimeException(e); } }); - Assert.assertEquals( + assertEquals( "java.lang.Exception: Settings for signing key is missing. Please specify at least the option signing_key with a shared secret.", exception.getMessage() ); @@ -117,15 +122,15 @@ public void testCreateJwtWithRoles() throws Exception { JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(encodedJwt); JwtToken jwt = jwtConsumer.getJwtToken(); - Assert.assertEquals("cluster_0", jwt.getClaim("iss")); - Assert.assertEquals("admin", jwt.getClaim("sub")); - Assert.assertEquals("audience_0", jwt.getClaim("aud")); - Assert.assertNotNull(jwt.getClaim("iat")); - Assert.assertNotNull(jwt.getClaim("exp")); - Assert.assertEquals(expectedExp, jwt.getClaim("exp")); + assertEquals("cluster_0", jwt.getClaim("iss")); + assertEquals("admin", jwt.getClaim("sub")); + assertEquals("audience_0", jwt.getClaim("aud")); + assertNotNull(jwt.getClaim("iat")); + assertNotNull(jwt.getClaim("exp")); + assertEquals(expectedExp, jwt.getClaim("exp")); EncryptionDecryptionUtil encryptionUtil = new EncryptionDecryptionUtil(claimsEncryptionKey); - Assert.assertEquals(expectedRoles, encryptionUtil.decrypt(jwt.getClaim("er").toString())); - Assert.assertNull(jwt.getClaim("br")); + assertEquals(expectedRoles, encryptionUtil.decrypt(jwt.getClaim("er").toString())); + assertNull(jwt.getClaim("br")); } @Test @@ -156,16 +161,16 @@ public void testCreateJwtWithRoleSecurityMode() throws Exception { JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(encodedJwt); JwtToken jwt = jwtConsumer.getJwtToken(); - Assert.assertEquals("cluster_0", jwt.getClaim("iss")); - Assert.assertEquals("admin", jwt.getClaim("sub")); - Assert.assertEquals("audience_0", jwt.getClaim("aud")); - Assert.assertNotNull(jwt.getClaim("iat")); - Assert.assertNotNull(jwt.getClaim("exp")); - Assert.assertEquals(expectedExp, jwt.getClaim("exp")); + assertEquals("cluster_0", jwt.getClaim("iss")); + assertEquals("admin", jwt.getClaim("sub")); + assertEquals("audience_0", jwt.getClaim("aud")); + assertNotNull(jwt.getClaim("iat")); + assertNotNull(jwt.getClaim("exp")); + assertEquals(expectedExp, jwt.getClaim("exp")); EncryptionDecryptionUtil encryptionUtil = new EncryptionDecryptionUtil(claimsEncryptionKey); - Assert.assertEquals(expectedRoles, encryptionUtil.decrypt(jwt.getClaim("er").toString())); - Assert.assertNotNull(jwt.getClaim("br")); - Assert.assertEquals(expectedBackendRoles, jwt.getClaim("br")); + assertEquals(expectedRoles, encryptionUtil.decrypt(jwt.getClaim("er").toString())); + assertNotNull(jwt.getClaim("br")); + assertEquals(expectedBackendRoles, jwt.getClaim("br")); } @Test @@ -179,14 +184,14 @@ public void testCreateJwtWithNegativeExpiry() { Settings settings = Settings.builder().put("signing_key", "abc123").put("encryption_key", claimsEncryptionKey).build(); JwtVendor jwtVendor = new JwtVendor(settings, Optional.empty()); - Throwable exception = Assert.assertThrows(RuntimeException.class, () -> { + Throwable exception = assertThrows(RuntimeException.class, () -> { try { jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, List.of(), true); } catch (Exception e) { throw new RuntimeException(e); } }); - Assert.assertEquals("java.lang.Exception: The expiration time should be a positive integer", exception.getMessage()); + assertEquals("java.lang.Exception: The expiration time should be a positive integer", exception.getMessage()); } @Test @@ -202,14 +207,14 @@ public void testCreateJwtWithExceededExpiry() throws Exception { Settings settings = Settings.builder().put("signing_key", "abc123").put("encryption_key", claimsEncryptionKey).build(); JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); - Throwable exception = Assert.assertThrows(RuntimeException.class, () -> { + Throwable exception = assertThrows(RuntimeException.class, () -> { try { jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, backendRoles, true); } catch (Exception e) { throw new RuntimeException(e); } }); - Assert.assertEquals( + assertEquals( "java.lang.Exception: The provided expiration time exceeds the maximum allowed duration of 600 seconds", exception.getMessage() ); @@ -225,14 +230,14 @@ public void testCreateJwtWithBadEncryptionKey() { Settings settings = Settings.builder().put("signing_key", "abc123").build(); - Throwable exception = Assert.assertThrows(RuntimeException.class, () -> { + Throwable exception = assertThrows(RuntimeException.class, () -> { try { new JwtVendor(settings, Optional.empty()).createJwt(issuer, subject, audience, expirySeconds, roles, List.of(), true); } catch (Exception e) { throw new RuntimeException(e); } }); - Assert.assertEquals("java.lang.IllegalArgumentException: encryption_key cannot be null", exception.getMessage()); + assertEquals("java.lang.IllegalArgumentException: encryption_key cannot be null", exception.getMessage()); } @Test @@ -246,22 +251,22 @@ public void testCreateJwtWithBadRoles() { Settings settings = Settings.builder().put("signing_key", "abc123").put("encryption_key", claimsEncryptionKey).build(); JwtVendor jwtVendor = new JwtVendor(settings, Optional.empty()); - Throwable exception = Assert.assertThrows(RuntimeException.class, () -> { + Throwable exception = assertThrows(RuntimeException.class, () -> { try { jwtVendor.createJwt(issuer, subject, audience, expirySeconds, roles, List.of(), true); } catch (Exception e) { throw new RuntimeException(e); } }); - Assert.assertEquals("java.lang.Exception: Roles cannot be null", exception.getMessage()); + assertEquals("java.lang.Exception: Roles cannot be null", exception.getMessage()); } @Test public void testCreateJwtLogsCorrectly() throws Exception { - mockAppender = Mockito.mock(Appender.class); + mockAppender = mock(Appender.class); logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); - Mockito.when(mockAppender.getName()).thenReturn("MockAppender"); - Mockito.when(mockAppender.isStarted()).thenReturn(true); + when(mockAppender.getName()).thenReturn("MockAppender"); + when(mockAppender.isStarted()).thenReturn(true); Logger logger = (Logger) LogManager.getLogger(JwtVendor.class); logger.addAppender(mockAppender); logger.setLevel(Level.DEBUG); @@ -286,9 +291,9 @@ public void testCreateJwtLogsCorrectly() throws Exception { LogEvent logEvent = logEventCaptor.getValue(); String logMessage = logEvent.getMessage().getFormattedMessage(); - Assert.assertTrue(logMessage.startsWith("Created JWT:")); + assertTrue(logMessage.startsWith("Created JWT:")); String[] parts = logMessage.split("\\."); - Assert.assertTrue(parts.length >= 3); + assertTrue(parts.length >= 3); } } diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 94acf4c052..b32792190f 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -41,11 +41,9 @@ import org.apache.logging.log4j.core.ErrorHandler; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.Logger; -import org.junit.Assert; import org.junit.Test; import org.mockito.ArgumentCaptor; -import org.mockito.Mockito; import org.opensearch.SpecialPermission; import org.opensearch.common.settings.Settings; import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; @@ -55,10 +53,20 @@ import org.opensearch.security.util.FakeRestRequest; import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.opensearch.rest.RestRequest.Method.POST; import static org.opensearch.rest.RestRequest.Method.PUT; @@ -81,19 +89,19 @@ public class OnBehalfOfAuthenticatorTest { public void testReRequestAuthenticationReturnsEmptyOptional() { OnBehalfOfAuthenticator authenticator = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); Optional result = authenticator.reRequestAuthentication(null, null); - Assert.assertFalse(result.isPresent()); + assertFalse(result.isPresent()); } @Test public void testGetTypeReturnsExpectedType() { OnBehalfOfAuthenticator authenticator = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); String type = authenticator.getType(); - Assert.assertEquals("onbehalfof_jwt", type); + assertEquals("onbehalfof_jwt", type); } @Test public void testNoKey() { - Exception exception = Assert.assertThrows( + Exception exception = assertThrows( RuntimeException.class, () -> extractCredentialsFromJwtHeader( null, @@ -102,12 +110,12 @@ public void testNoKey() { false ) ); - Assert.assertTrue(exception.getMessage().contains("Unable to find on behalf of authenticator signing key")); + assertTrue(exception.getMessage().contains("Unable to find on behalf of authenticator signing key")); } @Test public void testEmptyKey() { - Exception exception = Assert.assertThrows( + Exception exception = assertThrows( RuntimeException.class, () -> extractCredentialsFromJwtHeader( null, @@ -116,12 +124,12 @@ public void testEmptyKey() { false ) ); - Assert.assertTrue(exception.getMessage().contains("Unable to find on behalf of authenticator signing key")); + assertTrue(exception.getMessage().contains("Unable to find on behalf of authenticator signing key")); } @Test public void testBadKey() { - Exception exception = Assert.assertThrows( + Exception exception = assertThrows( RuntimeException.class, () -> extractCredentialsFromJwtHeader( BaseEncoding.base64().encode(new byte[] { 1, 3, 3, 4, 3, 6, 7, 8, 3, 10 }), @@ -130,25 +138,25 @@ public void testBadKey() { false ) ); - Assert.assertTrue(exception.getMessage().contains("The specified key byte array is 80 bits")); + assertTrue(exception.getMessage().contains("The specified key byte array is 80 bits")); } @Test public void testWeakKeyExceptionHandling() throws Exception { - Appender mockAppender = Mockito.mock(Appender.class); - ErrorHandler mockErrorHandler = Mockito.mock(ErrorHandler.class); - Mockito.when(mockAppender.getHandler()).thenReturn(mockErrorHandler); - Mockito.when(mockAppender.isStarted()).thenReturn(true); + Appender mockAppender = mock(Appender.class); + ErrorHandler mockErrorHandler = mock(ErrorHandler.class); + when(mockAppender.getHandler()).thenReturn(mockErrorHandler); + when(mockAppender.isStarted()).thenReturn(true); ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); - Mockito.when(mockAppender.getName()).thenReturn("MockAppender"); - Mockito.doNothing().when(mockAppender).append(logEventCaptor.capture()); + when(mockAppender.getName()).thenReturn("MockAppender"); + doNothing().when(mockAppender).append(logEventCaptor.capture()); Logger logger = (Logger) LogManager.getLogger(OnBehalfOfAuthenticator.class); logger.addAppender(mockAppender); - JwtParser mockJwtParser = Mockito.mock(JwtParser.class); - Mockito.when(mockJwtParser.parseClaimsJws(Mockito.anyString())).thenThrow(new WeakKeyException("Test Exception")); + JwtParser mockJwtParser = mock(JwtParser.class); + when(mockJwtParser.parseClaimsJws(anyString())).thenThrow(new WeakKeyException("Test Exception")); Settings settings = Settings.builder().put("signing_key", "testKey").put("encryption_key", claimsEncryptionKey).build(); OnBehalfOfAuthenticator auth = new OnBehalfOfAuthenticator(settings, "testCluster"); @@ -157,16 +165,16 @@ public void testWeakKeyExceptionHandling() throws Exception { jwtParserField.setAccessible(true); jwtParserField.set(auth, mockJwtParser); - SecurityRequest mockedRequest = Mockito.mock(SecurityRequest.class); - Mockito.when(mockedRequest.header(Mockito.anyString())).thenReturn("Bearer testToken"); - Mockito.when(mockedRequest.path()).thenReturn("/some/sample/path"); + SecurityRequest mockedRequest = mock(SecurityRequest.class); + when(mockedRequest.header(anyString())).thenReturn("Bearer testToken"); + when(mockedRequest.path()).thenReturn("/some/sample/path"); auth.extractCredentials(mockedRequest, null); boolean foundLog = logEventCaptor.getAllValues() .stream() .anyMatch(event -> event.getMessage().getFormattedMessage().contains("Cannot authenticate user with JWT because of ")); - Assert.assertTrue(foundLog); + assertTrue(foundLog); logger.removeAppender(mockAppender); } @@ -182,7 +190,7 @@ public void testTokenMissing() throws Exception { null ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -198,7 +206,7 @@ public void testInvalid() throws Exception { new FakeRestRequest(headers, new HashMap()).asSecurityRequest(), null ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -218,19 +226,19 @@ public void testDisabled() throws Exception { new FakeRestRequest(headers, new HashMap()).asSecurityRequest(), null ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test public void testInvalidTokenException() { - Appender mockAppender = Mockito.mock(Appender.class); + Appender mockAppender = mock(Appender.class); ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); - Mockito.when(mockAppender.getName()).thenReturn("MockAppender"); - Mockito.when(mockAppender.isStarted()).thenReturn(true); + when(mockAppender.getName()).thenReturn("MockAppender"); + when(mockAppender.isStarted()).thenReturn(true); Logger logger = (Logger) LogManager.getLogger(OnBehalfOfAuthenticator.class); logger.addAppender(mockAppender); logger.setLevel(Level.DEBUG); - Mockito.doNothing().when(mockAppender).append(logEventCaptor.capture()); + doNothing().when(mockAppender).append(logEventCaptor.capture()); String invalidToken = "invalidToken"; Settings settings = defaultSettings(); @@ -244,12 +252,12 @@ public void testInvalidTokenException() { null ); - Assert.assertNull(credentials); + assertNull(credentials); boolean foundLog = logEventCaptor.getAllValues() .stream() .anyMatch(event -> event.getMessage().getFormattedMessage().contains("Invalid or expired JWT token.")); - Assert.assertTrue(foundLog); + assertTrue(foundLog); logger.removeAppender(mockAppender); } @@ -271,7 +279,7 @@ public void testNonSpecifyOBOSetting() throws Exception { new FakeRestRequest(headers, new HashMap()).asSecurityRequest(), null ); - Assert.assertNotNull(credentials); + assertNotNull(credentials); } @Test @@ -297,11 +305,11 @@ public void testBearer() throws Exception { null ); - Assert.assertNotNull(credentials); - Assert.assertEquals("Leonard McCoy", credentials.getUsername()); - Assert.assertEquals(0, credentials.getSecurityRoles().size()); - Assert.assertEquals(0, credentials.getBackendRoles().size()); - Assert.assertThat(credentials.getAttributes(), equalTo(expectedAttributes)); + assertNotNull(credentials); + assertEquals("Leonard McCoy", credentials.getUsername()); + assertEquals(0, credentials.getSecurityRoles().size()); + assertEquals(0, credentials.getBackendRoles().size()); + assertThat(credentials.getAttributes(), equalTo(expectedAttributes)); } @Test @@ -323,7 +331,7 @@ public void testBearerWrongPosition() throws Exception { null ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -360,19 +368,19 @@ public void testBasicAuthHeader() throws Exception { new FakeRestRequest(headers, Collections.emptyMap()).asSecurityRequest(), null ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test public void testMissingBearerScheme() throws Exception { - Appender mockAppender = Mockito.mock(Appender.class); + Appender mockAppender = mock(Appender.class); ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(LogEvent.class); - Mockito.when(mockAppender.getName()).thenReturn("MockAppender"); - Mockito.when(mockAppender.isStarted()).thenReturn(true); + when(mockAppender.getName()).thenReturn("MockAppender"); + when(mockAppender.isStarted()).thenReturn(true); Logger logger = (Logger) LogManager.getLogger(OnBehalfOfAuthenticator.class); logger.addAppender(mockAppender); logger.setLevel(Level.DEBUG); - Mockito.doNothing().when(mockAppender).append(logEventCaptor.capture()); + doNothing().when(mockAppender).append(logEventCaptor.capture()); String craftedToken = "beaRerSomeActualToken"; // This token matches the BEARER pattern but doesn't contain the BEARER_PREFIX @@ -384,12 +392,12 @@ public void testMissingBearerScheme() throws Exception { null ); - Assert.assertNull(credentials); + assertNull(credentials); boolean foundLog = logEventCaptor.getAllValues() .stream() .anyMatch(event -> event.getMessage().getFormattedMessage().contains("No Bearer scheme found in header")); - Assert.assertTrue(foundLog); + assertTrue(foundLog); logger.removeAppender(mockAppender); } @@ -412,7 +420,7 @@ public void testMissingBearerPrefixInAuthHeader() { null ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -425,10 +433,10 @@ public void testPlainTextedRolesFromDrClaim() { true ); - Assert.assertNotNull(credentials); - Assert.assertEquals("Leonard McCoy", credentials.getUsername()); - Assert.assertEquals(2, credentials.getSecurityRoles().size()); - Assert.assertEquals(0, credentials.getBackendRoles().size()); + assertNotNull(credentials); + assertEquals("Leonard McCoy", credentials.getUsername()); + assertEquals(2, credentials.getSecurityRoles().size()); + assertEquals(0, credentials.getBackendRoles().size()); } @Test @@ -442,12 +450,12 @@ public void testBackendRolesExtraction() { true ); - Assert.assertNotNull(credentials); + assertNotNull(credentials); Set expectedBackendRoles = new HashSet<>(Arrays.asList("role1", "role2", "role3", "role4", "role5")); Set actualBackendRoles = credentials.getBackendRoles(); - Assert.assertTrue(actualBackendRoles.containsAll(expectedBackendRoles)); + assertTrue(actualBackendRoles.containsAll(expectedBackendRoles)); } @Test @@ -462,9 +470,9 @@ public void testRolesDecryptionFromErClaim() { true ); - Assert.assertNotNull(credentials); + assertNotNull(credentials); List expectedRoles = Arrays.asList("admin", "developer"); - Assert.assertTrue(credentials.getSecurityRoles().containsAll(expectedRoles)); + assertTrue(credentials.getSecurityRoles().containsAll(expectedRoles)); } @Test @@ -477,9 +485,9 @@ public void testNullClaim() throws Exception { false ); - Assert.assertNotNull(credentials); - Assert.assertEquals("Leonard McCoy", credentials.getUsername()); - Assert.assertEquals(0, credentials.getBackendRoles().size()); + assertNotNull(credentials); + assertEquals("Leonard McCoy", credentials.getUsername()); + assertEquals(0, credentials.getBackendRoles().size()); } @Test @@ -492,10 +500,10 @@ public void testNonStringClaim() throws Exception { true ); - Assert.assertNotNull(credentials); - Assert.assertEquals("Leonard McCoy", credentials.getUsername()); - Assert.assertEquals(1, credentials.getSecurityRoles().size()); - Assert.assertTrue(credentials.getSecurityRoles().contains("123")); + assertNotNull(credentials); + assertEquals("Leonard McCoy", credentials.getUsername()); + assertEquals(1, credentials.getSecurityRoles().size()); + assertTrue(credentials.getSecurityRoles().contains("123")); } @Test @@ -508,10 +516,10 @@ public void testRolesMissing() throws Exception { false ); - Assert.assertNotNull(credentials); - Assert.assertEquals("Leonard McCoy", credentials.getUsername()); - Assert.assertEquals(0, credentials.getSecurityRoles().size()); - Assert.assertEquals(0, credentials.getBackendRoles().size()); + assertNotNull(credentials); + assertEquals("Leonard McCoy", credentials.getUsername()); + assertEquals(0, credentials.getSecurityRoles().size()); + assertEquals(0, credentials.getBackendRoles().size()); } @Test @@ -524,7 +532,7 @@ public void testWrongSubjectKey() throws Exception { false ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -537,7 +545,7 @@ public void testMissingAudienceClaim() throws Exception { false ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -550,7 +558,7 @@ public void testExp() throws Exception { false ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -563,7 +571,7 @@ public void testNbf() throws Exception { false ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -582,12 +590,12 @@ public void testRolesArray() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader(signingKeyB64Encoded, claimsEncryptionKey, builder, true); - Assert.assertNotNull(credentials); - Assert.assertEquals("Cluster_0", credentials.getUsername()); - Assert.assertEquals(3, credentials.getSecurityRoles().size()); - Assert.assertTrue(credentials.getSecurityRoles().contains("a")); - Assert.assertTrue(credentials.getSecurityRoles().contains("b")); - Assert.assertTrue(credentials.getSecurityRoles().contains("3rd")); + assertNotNull(credentials); + assertEquals("Cluster_0", credentials.getUsername()); + assertEquals(3, credentials.getSecurityRoles().size()); + assertTrue(credentials.getSecurityRoles().contains("a")); + assertTrue(credentials.getSecurityRoles().contains("b")); + assertTrue(credentials.getSecurityRoles().contains("3rd")); } @Test @@ -609,7 +617,7 @@ public void testDifferentIssuer() throws Exception { null ); - Assert.assertNull(credentials); + assertNull(credentials); } @Test @@ -618,19 +626,19 @@ public void testRequestNotAllowed() { // Test POST on generate on-behalf-of token endpoint SecurityRequest mockedRequest1 = mock(SecurityRequest.class); - Mockito.when(mockedRequest1.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); - Mockito.when(mockedRequest1.path()).thenReturn(SECURITY_PREFIX + ON_BEHALF_OF_SUFFIX); - Mockito.when(mockedRequest1.method()).thenReturn(POST); - Assert.assertFalse(oboAuth.isRequestAllowed(mockedRequest1)); - Assert.assertNull(oboAuth.extractCredentials(mockedRequest1, null)); + when(mockedRequest1.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); + when(mockedRequest1.path()).thenReturn(SECURITY_PREFIX + ON_BEHALF_OF_SUFFIX); + when(mockedRequest1.method()).thenReturn(POST); + assertFalse(oboAuth.isRequestAllowed(mockedRequest1)); + assertNull(oboAuth.extractCredentials(mockedRequest1, null)); // Test PUT on password changing endpoint SecurityRequest mockedRequest2 = mock(SecurityRequest.class); - Mockito.when(mockedRequest2.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); - Mockito.when(mockedRequest2.path()).thenReturn(SECURITY_PREFIX + ACCOUNT_SUFFIX); - Mockito.when(mockedRequest2.method()).thenReturn(PUT); - Assert.assertFalse(oboAuth.isRequestAllowed(mockedRequest2)); - Assert.assertNull(oboAuth.extractCredentials(mockedRequest2, null)); + when(mockedRequest2.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); + when(mockedRequest2.path()).thenReturn(SECURITY_PREFIX + ACCOUNT_SUFFIX); + when(mockedRequest2.method()).thenReturn(PUT); + assertFalse(oboAuth.isRequestAllowed(mockedRequest2)); + assertNull(oboAuth.extractCredentials(mockedRequest2, null)); } /** extracts a default user credential from a request header */