Repository: jwtk/jjwt (java)

Issue:
JwkSet unexpectedly "secret" / Jackson cannot serialize JwkSet
<!--
PLEASE DO NOT USE GITHUB ISSUES TO ASK QUESTIONS! 

The JJWT team uses GitHub Issues only to track actionable work that requires changes to the JJWT codebase.

If you have a usability or general question about JJWT, please instead post your question to StackOverflow using the following JJWT-specific link:

https://stackoverflow.com/questions/ask?tags=jjwt&guided=false

To help ensure you receive a timely response, please follow the StackOverflow guidelines on how to ask a good question: https://stackoverflow.com/help/how-to-ask

Ok, now that that's out of the way - if you're still sure you need to report a bug, please fill in the sections below...
-->

**Describe the bug**

The default (and only) JWK set builder considers all "keys" members as secret, even if the set contains only public keys.
In turn, this results in a failure to serialize the set using Jackson redacted data cannot be serialized.

**To Reproduce**

To repro
1. Add Jackson to the classpath
2. Create a JWK for a public key
3. Add that JWK to a JWKSet using the builder
4. Attempt to serialize that set using Jackson's ObjectMapper

A complete example follows.

```kotlin
dependencies {
    implementation("io.jsonwebtoken:jjwt-api:0.12.6")
    runtimeOnly("io.jsonwebtoken:jjwt-impl:0.12.6")
    runtimeOnly("io.jsonwebtoken:jjwt-jackson:0.12.6")
}
```

```java
static void jwkSetUnexpectedlyPrivate(RSAPublicKey pubKey) throws Exception {
    final ObjectMapper jacksonMapper = new ObjectMapper();

    final RsaPublicJwk pubJwk = Jwks.builder()
            .key(pubKey)
            .build();

    // PASS; unredacted human-readable form
    pubJwk.toString();

    // PASS; unredacted
    jacksonMapper.writeValueAsString(pubJwk);

    // When in set, now redacted
    final JwkSet jwkSet = Jwks.set()
            .add(pubJwk)
            .build();

    // FAIL? Redacted; Possibly a safe default, not terribly problematic for debug statements
    jwkSet.toString();

    // FAIL; Throws an exception. Prevents exposure of the set for external consumption of public keys.
    jacksonMapper.writeValueAsString(jwkSet);
    // com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No serializer found for class
    // io.jsonwebtoken.impl.lang.RedactedSupplier and no properties discovered to create BeanSerializer
    // (to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS)
    // (through reference chain: io.jsonwebtoken.impl.security.DefaultJwkSet["keys"])
}
```

**Expected behavior**

1. It is expected that sets are either conditionally redacted based on whether their content is redacted,
OR this behavior can be configured in some way to allow proper serialization.
2. It should be possible to serialize a JwkSet using Jackson

**Screenshots**

See code sample above.


Pull request:
JwkSet keys secrecy default
Ensured JwkSet `keys` parameter is not `secret` by default.  Each JWK element should determine which members are secret or not.

Resolves #976.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7a8fb13..5add327 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ This patch release:
   [Issue 947](https://github.com/jwtk/jjwt/issues/947).
 * Fixes a decompression memory leak in concurrent/multi-threaded environments introduced in 0.12.0 when decompressing JWTs with a `zip` header of `GZIP`. See [Issue 949](https://github.com/jwtk/jjwt/issues/949).
 * Upgrades BouncyCastle to 1.78 via [PR 941](https://github.com/jwtk/jjwt/pull/941).
+* Ensures that a `JwkSet`'s `keys` list member is no longer considered secret and is not redacted by default. However, each individual JWK element within the `keys` list may still have [redacted private or secret members](https://github.com/jwtk/jjwt?tab=readme-ov-file#jwk-tostring-safety) as expected. See [Issue 976](https://github.com/jwtk/jjwt/issues/976).
 
 ### 0.12.5
 
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultJwkSet.java b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultJwkSet.java
index a4ae214..7104991 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultJwkSet.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultJwkSet.java
@@ -35,7 +35,6 @@ public class DefaultJwkSet extends ParameterMap implements JwkSet {
         return Parameters.builder(JwkConverter.JWK_CLASS)
                 .setConverter(converter).set()
                 .setId("keys").setName("JSON Web Keys")
-                .setSecret(true)
                 .build();
     }
 
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/JwkSetConverter.java b/impl/src/main/java/io/jsonwebtoken/impl/security/JwkSetConverter.java
index b5a49f2..c13e993 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/security/JwkSetConverter.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/security/JwkSetConverter.java
@@ -91,9 +91,6 @@ public class JwkSetConverter implements Converter<JwkSet, Object> {
             String msg = "JWK Set " + PARAM + " value cannot be null.";
             throw new MalformedKeySetException(msg);
         }
-        if (val instanceof Supplier<?>) {
-            val = ((Supplier<?>) val).get();
-        }
         if (!(val instanceof Collection)) {
             String msg = "JWK Set " + PARAM + " value must be a Collection (JSON Array). Type found: " +
                     val.getClass().getName();


Test patch:
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetBuilderTest.groovy
index b94e4a0..eeb338b 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetBuilderTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetBuilderTest.groovy
@@ -278,7 +278,9 @@ class DefaultJwkSetBuilderTest {
                 key_ops: ['sign', 'encrypt'] // unrelated operations
         ]
 
-        String msg = "Invalid Map ${DefaultJwkSet.KEYS} value: <redacted>. Unable to create JWK: Unrelated key " +
+        String msg = "Invalid Map ${DefaultJwkSet.KEYS} value: " +
+                "[{kty=${badMap.kty}, k=${badMap.k}, key_ops=${badMap.key_ops}}]. " +
+                "Unable to create JWK: Unrelated key " +
                 "operations are not allowed. KeyOperation [${Jwks.OP.ENCRYPT}] is unrelated to " +
                 "[${Jwks.OP.SIGN}]."
 
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetTest.groovy
index 03b970f..a39fa37 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetTest.groovy
@@ -67,14 +67,17 @@ class DefaultJwkSetTest {
     }
 
     /**
-     * Asserts that the raw keys value is a RedactedSupplier and not a raw value due to potential sensitivity if
-     * the JwkSet contains secret or private JWKs.
+     * Asserts that the raw 'keys' value is not a RedactedSupplier per https://github.com/jwtk/jjwt/issues/976,
+     * but an internal secret key parameter does have a RedactedSupplier
      */
     @Test
-    void testKeysFromGetIsRedactedSupplier() {
+    void testGetKeysNotRedactedSupplier() {
         def jwk = Jwks.builder().key(TestKeys.HS256).build()
         def set = new DefaultJwkSet(DefaultJwkSet.KEYS, [keys: [jwk]])
-        def result = set.get('keys')
-        assertTrue result instanceof RedactedSupplier
+        def keys = set.get('keys')
+        assertFalse keys instanceof RedactedSupplier
+        keys = keys as List
+        def element = keys[0] as Map// result is an array/list, so get first JWK in the list
+        assertTrue element.k instanceof RedactedSupplier // 'k' is a secret property, should be redacted
     }
 }