Fix issue with jwt attribute parsing of lists (#4884)

Signed-off-by: Craig Perkins <[email protected]>
opensearch-project · Nov 8, 2024 · ddcecb5 · ddcecb5
1 parent 1c898dc
commit ddcecb5
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 3 deletions.
diff --git a/src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java
@@ -33,6 +33,7 @@
 import org.opensearch.common.logging.DeprecationLogger;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.concurrent.ThreadContext;
+import org.opensearch.security.DefaultObjectMapper;
 import org.opensearch.security.auth.HTTPAuthenticator;
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityResponse;
@@ -185,7 +186,22 @@ private AuthCredentials extractCredentials0(final SecurityRequest request) {
                 final AuthCredentials ac = new AuthCredentials(subject, roles).markComplete();
 
                 for (Entry<String, Object> claim : claims.entrySet()) {
-                    ac.addAttribute("attr.jwt." + claim.getKey(), String.valueOf(claim.getValue()));
+                    String key = "attr.jwt." + claim.getKey();
+                    Object value = claim.getValue();
+
+                    if (value instanceof Collection<?>) {
+                        try {
+                            // Convert the list to a JSON array string
+                            String jsonValue = DefaultObjectMapper.writeValueAsString(value, false);
+                            ac.addAttribute(key, jsonValue);
+                        } catch (Exception e) {
+                            log.warn("Failed to convert list claim to JSON for key: " + key, e);
+                            // Fallback to string representation
+                            ac.addAttribute(key, String.valueOf(value));
+                        }
+                    } else {
+                        ac.addAttribute(key, String.valueOf(value));
+                    }
                 }
 
                 return ac;

diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java
@@ -14,6 +14,7 @@
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Optional;
@@ -31,6 +32,7 @@
 import org.opensearch.SpecialPermission;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.concurrent.ThreadContext;
+import org.opensearch.security.DefaultObjectMapper;
 import org.opensearch.security.auth.HTTPAuthenticator;
 import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil;
 import org.opensearch.security.filter.SecurityRequest;
@@ -204,7 +206,22 @@ private AuthCredentials extractCredentials0(final SecurityRequest request) {
             final AuthCredentials ac = new AuthCredentials(subject, roles, backendRoles).markComplete();
 
             for (Entry<String, Object> claim : claims.entrySet()) {
-                ac.addAttribute("attr.jwt." + claim.getKey(), String.valueOf(claim.getValue()));
+                String key = "attr.jwt." + claim.getKey();
+                Object value = claim.getValue();
+
+                if (value instanceof Collection<?>) {
+                    try {
+                        // Convert the list to a JSON array string
+                        String jsonValue = DefaultObjectMapper.writeValueAsString(value, false);
+                        ac.addAttribute(key, jsonValue);
+                    } catch (Exception e) {
+                        log.warn("Failed to convert list claim to JSON for key: " + key, e);
+                        // Fallback to string representation
+                        ac.addAttribute(key, String.valueOf(value));
+                    }
+                } else {
+                    ac.addAttribute(key, String.valueOf(value));
+                }
             }
 
             return ac;

diff --git a/src/test/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java b/src/test/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java
@@ -18,9 +18,11 @@
 import java.security.PrivateKey;
 import java.security.PublicKey;
 import java.security.SecureRandom;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import javax.crypto.SecretKey;
 
@@ -40,7 +42,9 @@
 import io.jsonwebtoken.security.Keys;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -120,6 +124,67 @@ public void testInvalid() throws Exception {
         Assert.assertNull(credentials);
     }
 
+    @Test
+    public void testJwtAttributeParsing() throws Exception {
+        Map<String, String> expectedAttributes = new HashMap<>();
+        expectedAttributes.put("attr.jwt.sub", "Leonard McCoy");
+        expectedAttributes.put("attr.jwt.list", "[\"a\",\"b\",\"c\"]");
+
+        String jwsToken = Jwts.builder()
+            .setSubject("Leonard McCoy")
+            .claim("list", List.of("a", "b", "c"))
+            .signWith(Keys.hmacShaKeyFor(secretKeyBytes), SignatureAlgorithm.HS512)
+            .compact();
+
+        Settings settings = Settings.builder().put("signing_key", BaseEncoding.base64().encode(secretKeyBytes)).build();
+
+        HTTPJwtAuthenticator jwtAuth = new HTTPJwtAuthenticator(settings, null);
+        Map<String, String> headers = new HashMap<String, String>();
+        headers.put("Authorization", "Bearer " + jwsToken);
+
+        AuthCredentials credentials = jwtAuth.extractCredentials(
+            new FakeRestRequest(headers, new HashMap<String, String>()).asSecurityRequest(),
+            null
+        );
+
+        assertNotNull(credentials);
+        assertThat(credentials.getUsername(), is("Leonard McCoy"));
+        assertThat(credentials.getAttributes(), equalTo(expectedAttributes));
+    }
+
+    @Test
+    public void testJwtAttributeParsingMixedDataType() throws Exception {
+        Map<String, String> expectedAttributes = new HashMap<>();
+        expectedAttributes.put("attr.jwt.sub", "Leonard McCoy");
+        expectedAttributes.put("attr.jwt.list", "[\"a\",1,null,2.0]");
+
+        List<Object> elements = new ArrayList<>();
+        elements.add("a");
+        elements.add(1);
+        elements.add(null);
+        elements.add(2.0);
+        String jwsToken = Jwts.builder()
+            .setSubject("Leonard McCoy")
+            .claim("list", elements)
+            .signWith(Keys.hmacShaKeyFor(secretKeyBytes), SignatureAlgorithm.HS512)
+            .compact();
+
+        Settings settings = Settings.builder().put("signing_key", BaseEncoding.base64().encode(secretKeyBytes)).build();
+
+        HTTPJwtAuthenticator jwtAuth = new HTTPJwtAuthenticator(settings, null);
+        Map<String, String> headers = new HashMap<String, String>();
+        headers.put("Authorization", "Bearer " + jwsToken);
+
+        AuthCredentials credentials = jwtAuth.extractCredentials(
+            new FakeRestRequest(headers, new HashMap<String, String>()).asSecurityRequest(),
+            null
+        );
+
+        assertNotNull(credentials);
+        assertThat(credentials.getUsername(), is("Leonard McCoy"));
+        assertThat(credentials.getAttributes(), equalTo(expectedAttributes));
+    }
+
     /** Here is the original encoded jwt token generation with cxf library:
      *
      * String base64EncodedSecret = Base64.getEncoder().encodeToString(someSecret.getBytes(StandardCharsets.UTF_8));

diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java
@@ -267,7 +267,7 @@ public void testBearer() throws Exception {
         Map<String, String> expectedAttributes = new HashMap<>();
         expectedAttributes.put("attr.jwt.iss", "cluster_0");
         expectedAttributes.put("attr.jwt.sub", "Leonard McCoy");
-        expectedAttributes.put("attr.jwt.aud", "[ext_0]");
+        expectedAttributes.put("attr.jwt.aud", "[\"ext_0\"]");
 
         String jwsToken = Jwts.builder()
             .setIssuer(clusterName)