Fix jjwt sub parsing (#872)

* Fix jjwt sub parsing

* Improve JwtHelper test coverage above 0.90

Add tests covering createSignedJwt, the unrecognized PEM header
branch, isSkdEnabled fall-through paths, the default constructor,
and both Deserializer overloads. JwtHelper instruction coverage
goes from 90% to 100% and branch coverage from 71% to 92%.

Author: symphony-enrico

symphony-bdk-core/src/main/java/com/symphony/bdk/core/auth/jwt/JwtHelper.java

@@ -9,6 +9,8 @@
 import io.jsonwebtoken.Claims;
 import io.jsonwebtoken.JwtException;
 import io.jsonwebtoken.Jwts;
+import io.jsonwebtoken.io.DeserializationException;
+import io.jsonwebtoken.io.Deserializer;
 import org.apiguardian.api.API;
 import org.bouncycastle.asn1.pkcs.RSAPrivateKey;
 import org.bouncycastle.crypto.params.RSAPrivateCrtKeyParameters;
@@ -19,6 +21,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.Reader;
 import java.io.StringReader;
 import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
@@ -32,6 +35,7 @@
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.util.Base64;
 import java.util.Date;
+import java.util.Map;
 
 /**
  * JWT helper class, used to :
@@ -119,6 +123,7 @@ public static UserClaim validateJwt(String jwt, String certificate) throws AuthI
     try {
       final Claims claims = Jwts.parser()
           .verifyWith(x509Certificate.getPublicKey())
+          .json(normalizeSubjectDeserializer())
           .build()
           .parseSignedClaims(jwt)
           .getPayload();
@@ -170,6 +175,39 @@ private static String dropBearer(String jwt) {
     return jwt.replace("Bearer ", "");
   }
 
+  @SuppressWarnings("unchecked")
+  private static Deserializer<Map<String, ?>> normalizeSubjectDeserializer() {
+    // jjwt 0.12+ strictly requires 'sub' to be a String (RFC 7519), but the pod may send it
+    // as a numeric user ID (Long). Coerce it to String before jjwt validates registered claims.
+    return new Deserializer<Map<String, ?>>() {
+      @Override
+      public Map<String, ?> deserialize(byte[] bytes) throws DeserializationException {
+        try {
+          return normalize(mapper.readValue(bytes, Map.class));
+        } catch (IOException e) {
+          throw new DeserializationException("Unable to deserialize JWT claims", e);
+        }
+      }
+
+      @Override
+      public Map<String, ?> deserialize(Reader reader) throws DeserializationException {
+        try {
+          return normalize(mapper.readValue(reader, Map.class));
+        } catch (IOException e) {
+          throw new DeserializationException("Unable to deserialize JWT claims", e);
+        }
+      }
+
+      private Map<String, Object> normalize(Map<String, Object> map) {
+        Object sub = map.get("sub");
+        if (sub instanceof Number) {
+          map.put("sub", sub.toString());
+        }
+        return map;
+      }
+    };
+  }
+
   private static PrivateKey parsePKCS1PrivateKey(String pemPrivateKey) throws GeneralSecurityException {
     try (final PemReader pemReader = new PemReader(new StringReader(pemPrivateKey))) {
       final PemObject privateKeyObject = pemReader.readPemObject();

symphony-bdk-core/src/test/java/com/symphony/bdk/core/auth/JwtHelperTest.java

@@ -15,6 +15,8 @@
 import com.migcomponents.migbase64.Base64;
 import io.jsonwebtoken.Jwts;
 import io.jsonwebtoken.SignatureAlgorithm;
+import io.jsonwebtoken.io.DeserializationException;
+import io.jsonwebtoken.io.Deserializer;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
 import org.bouncycastle.asn1.ASN1Encodable;
@@ -24,16 +26,24 @@
 import org.bouncycastle.util.io.pem.PemWriter;
 import org.junit.jupiter.api.Test;
 
+import java.io.ByteArrayInputStream;
 import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
 import java.io.StringWriter;
+import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
 import java.security.Key;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.KeyStore;
 import java.security.PrivateKey;
+import java.security.Signature;
 import java.security.cert.Certificate;
 import java.util.Date;
+import java.util.Map;
 
 /**
  * Test class for the {@link JwtHelper}.
@@ -141,6 +151,153 @@ public void testCheckSkdEnabled() {
     assertFalse(JwtHelper.isSkdEnabled("invalid.jwt.value"));
   }
 
+  /**
+   * The Symphony pod sets 'sub' as a numeric userId (Long) which violates RFC 7519 but is the real
+   * payload shape. jjwt 0.12+ rejects this unless we normalize it before claim validation.
+   */
+  @Test
+  @SneakyThrows
+  public void testValidateJwtWithNumericSubject() {
+    final KeyStore keyStore = getKeyStoreFromFile();
+    final Certificate certificate = keyStore.getCertificate(CERT_ALIAS);
+    final String certificatePem = java.util.Base64.getEncoder().encodeToString(certificate.getEncoded());
+    final PrivateKey privateKey = (PrivateKey) keyStore.getKey(CERT_ALIAS, CERT_PASSWORD.toCharArray());
+
+    // Build header + payload manually so 'sub' is serialised as a JSON number (Long),
+    // exactly as the Symphony pod does — jjwt builder would coerce it to String.
+    String header = java.util.Base64.getUrlEncoder().withoutPadding()
+        .encodeToString("{\"alg\":\"RS256\",\"typ\":\"JWT\"}".getBytes(StandardCharsets.UTF_8));
+
+    long expiration = System.currentTimeMillis() / 1000 + 3600;
+    String payloadJson = "{"
+        + "\"iss\":\"Symphony Communication Services LLC.\","
+        + "\"sub\":699220675788807,"
+        + "\"aud\":\"ai-agent-studio-local\","
+        + "\"user\":{"
+        +   "\"id\":699220675788807,"
+        +   "\"emailAddress\":\"test.user@symphony.com\","
+        +   "\"firstName\":\"Test\","
+        +   "\"lastName\":\"User\","
+        +   "\"displayName\":\"Test User\","
+        +   "\"company\":\"Test Company\","
+        +   "\"companyId\":\"10175\","
+        +   "\"username\":\"test.user@symphony.com\","
+        +   "\"avatarUrl\":\"../avatars/static/150/default.png\","
+        +   "\"avatarSmallUrl\":\"../avatars/static/50/default.png\""
+        + "},"
+        + "\"exp\":" + expiration
+        + "}";
+
+    String payload = java.util.Base64.getUrlEncoder().withoutPadding()
+        .encodeToString(payloadJson.getBytes(StandardCharsets.UTF_8));
+
+    String signingInput = header + "." + payload;
+    Signature signer = Signature.getInstance("SHA256withRSA");
+    signer.initSign(privateKey);
+    signer.update(signingInput.getBytes(StandardCharsets.UTF_8));
+    String signature = java.util.Base64.getUrlEncoder().withoutPadding().encodeToString(signer.sign());
+
+    String jwt = signingInput + "." + signature;
+
+    UserClaim userClaim = JwtHelper.validateJwt(jwt, certificatePem);
+
+    assertNotNull(userClaim);
+    assertEquals(699220675788807L, userClaim.getId());
+    assertEquals("test.user@symphony.com", userClaim.getEmailAddress());
+    assertEquals("Test", userClaim.getFirstName());
+    assertEquals("User", userClaim.getLastName());
+    assertEquals("Test User", userClaim.getDisplayName());
+    assertEquals("Test Company", userClaim.getCompany());
+    assertEquals("10175", userClaim.getCompanyId());
+    assertEquals("test.user@symphony.com", userClaim.getUsername());
+    assertEquals("../avatars/static/150/default.png", userClaim.getAvatarUrl());
+    assertEquals("../avatars/static/50/default.png", userClaim.getAvatarSmallUrl());
+  }
+
+  @Test
+  @SneakyThrows
+  public void testCreateSignedJwt() {
+    final KeyStore keyStore = getKeyStoreFromFile();
+    final PrivateKey privateKey = (PrivateKey) keyStore.getKey(CERT_ALIAS, CERT_PASSWORD.toCharArray());
+
+    String jwt = JwtHelper.createSignedJwt("alice", JwtHelper.JWT_EXPIRATION_MILLIS, privateKey);
+
+    assertNotNull(jwt);
+    // jjwt compact form must have exactly three dot-separated segments
+    assertEquals(3, jwt.split("\\.").length);
+  }
+
+  @Test
+  void loadUnknownFormatPrivateKey() {
+    String unknownFormat = "-----BEGIN SOMETHING ELSE-----\nabcdef\n-----END SOMETHING ELSE-----";
+    GeneralSecurityException ex = assertThrows(
+        GeneralSecurityException.class, () -> JwtHelper.parseRsaPrivateKey(unknownFormat));
+    assertTrue(ex.getMessage().contains("Header not recognized"));
+  }
+
+  @Test
+  public void testIsSkdEnabledMissingClaim() {
+    // JWT parses successfully but has no canUseSimplifiedKeyDelivery claim → final return false
+    assertFalse(JwtHelper.isSkdEnabled(JWT));
+  }
+
+  @Test
+  public void testIsSkdEnabledNonBooleanClaim() {
+    // canUseSimplifiedKeyDelivery present but as a string → falls through to final return false
+    // payload: {"sub":"123","canUseSimplifiedKeyDelivery":"yes"}
+    String jwtWithStringSkd =
+        "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9."
+            + "eyJzdWIiOiIxMjMiLCJjYW5Vc2VTaW1wbGlmaWVkS2V5RGVsaXZlcnkiOiJ5ZXMifQ."
+            + "signature";
+    assertFalse(JwtHelper.isSkdEnabled(jwtWithStringSkd));
+  }
+
+  @Test
+  public void testIsSkdEnabledTriggersCatch() {
+    // Too few segments → extractDecodedClaims throws → catch returns false
+    assertFalse(JwtHelper.isSkdEnabled("only.two"));
+  }
+
+  @Test
+  public void testJwtHelperConstructor() {
+    // Cover the implicit default constructor of the utility class
+    assertNotNull(new JwtHelper());
+  }
+
+  @Test
+  @SneakyThrows
+  public void testNormalizeSubjectDeserializerByteArray() {
+    // The byte[] overload is unreachable through jjwt's public API (Reader is used),
+    // so exercise it directly via reflection to cover both the success and the
+    // IOException catch path.
+    Method method = JwtHelper.class.getDeclaredMethod("normalizeSubjectDeserializer");
+    method.setAccessible(true);
+    @SuppressWarnings("unchecked")
+    Deserializer<Map<String, ?>> deserializer = (Deserializer<Map<String, ?>>) method.invoke(null);
+
+    Map<String, ?> result = deserializer.deserialize(
+        "{\"sub\":12345,\"user\":{\"id\":42}}".getBytes(StandardCharsets.UTF_8));
+    assertEquals("12345", result.get("sub"));
+
+    assertThrows(DeserializationException.class,
+        () -> deserializer.deserialize("not-json".getBytes(StandardCharsets.UTF_8)));
+  }
+
+  @Test
+  @SneakyThrows
+  public void testNormalizeSubjectDeserializerReaderIOException() {
+    // Exercise the IOException catch in the Reader overload by feeding malformed JSON
+    Method method = JwtHelper.class.getDeclaredMethod("normalizeSubjectDeserializer");
+    method.setAccessible(true);
+    @SuppressWarnings("unchecked")
+    Deserializer<Map<String, ?>> deserializer = (Deserializer<Map<String, ?>>) method.invoke(null);
+
+    try (Reader reader = new InputStreamReader(
+        new ByteArrayInputStream("not-json".getBytes(StandardCharsets.UTF_8)), StandardCharsets.UTF_8)) {
+      assertThrows(DeserializationException.class, () -> deserializer.deserialize(reader));
+    }
+  }
+
   @SneakyThrows
   private static String generatePkcs8RsaPrivateKey() {
     final KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA");