Merge pull request #36857 from sberyozkin/oidc_x5tS256

Fix OIDC key resolver to accept SHA256 certificate thumbprints
quarkusio · Nov 3, 2023 · e1bfbf8 · e1bfbf8
2 parents e6c6752 + 96ed648
commit e1bfbf8
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 3 deletions.
diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/JsonWebKeySet.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/JsonWebKeySet.java
@@ -28,6 +28,7 @@ public class JsonWebKeySet {
 
     private Map<String, Key> keysWithKeyId = new HashMap<>();
     private Map<String, Key> keysWithThumbprints = new HashMap<>();
+    private Map<String, Key> keysWithS256Thumbprints = new HashMap<>();
     private Map<String, List<Key>> keysWithoutKeyIdAndThumbprint = new HashMap<>();
 
     public JsonWebKeySet(String json) {
@@ -48,7 +49,12 @@ private void initKeys(String json) {
                     if (x5t != null && jwkKey.getKey() != null) {
                         keysWithThumbprints.put(x5t, jwkKey.getKey());
                     }
-                    if (jwkKey.getKeyId() == null && x5t == null && jwkKey.getKeyType() != null) {
+                    String x5tS256 = ((PublicJsonWebKey) jwkKey)
+                            .getX509CertificateSha256Thumbprint(calculateThumbprintIfMissing);
+                    if (x5tS256 != null && jwkKey.getKey() != null) {
+                        keysWithS256Thumbprints.put(x5tS256, jwkKey.getKey());
+                    }
+                    if (jwkKey.getKeyId() == null && x5t == null && x5tS256 == null && jwkKey.getKeyType() != null) {
                         List<Key> keys = keysWithoutKeyIdAndThumbprint.get(jwkKey.getKeyType());
                         if (keys == null) {
                             keys = new ArrayList<>();
@@ -76,6 +82,10 @@ public Key getKeyWithThumbprint(String x5t) {
         return keysWithThumbprints.get(x5t);
     }
 
+    public Key getKeyWithS256Thumbprint(String x5tS256) {
+        return keysWithS256Thumbprints.get(x5tS256);
+    }
+
     public Key getKeyWithoutKeyIdAndThumbprint(JsonWebSignature jws) {
         try {
             List<Key> keys = keysWithoutKeyIdAndThumbprint.get(jws.getKeyType());

diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java
@@ -395,7 +395,19 @@ public Key resolveKey(JsonWebSignature jws, List<JsonWebStructure> nestingContex
                     if (key == null) {
                         // if only `x5t` was set then the key must exist
                         throw new UnresolvableKeyException(
-                                String.format("JWK with thumbprint '%s' is not available", thumbprint));
+                                String.format("JWK with the certificate thumbprint '%s' is not available", thumbprint));
+                    }
+                }
+            }
+
+            if (key == null) {
+                thumbprint = jws.getHeader(HeaderParameterNames.X509_CERTIFICATE_SHA256_THUMBPRINT);
+                if (thumbprint != null) {
+                    key = getKeyWithS256Thumbprint(jws, thumbprint);
+                    if (key == null) {
+                        // if only `x5tS256` was set then the key must exist
+                        throw new UnresolvableKeyException(
+                                String.format("JWK with the SHA256 certificate thumbprint '%s' is not available", thumbprint));
                     }
                 }
             }
@@ -406,7 +418,8 @@ public Key resolveKey(JsonWebSignature jws, List<JsonWebStructure> nestingContex
 
             if (key == null) {
                 throw new UnresolvableKeyException(
-                        String.format("JWK is not available, neither 'kid' nor 'x5t' token headers are set", kid));
+                        String.format("JWK is not available, neither 'kid' nor 'x5t#S256' nor 'x5t' token headers are set",
+                                kid));
             } else {
                 return key;
             }
@@ -430,6 +443,15 @@ private Key getKeyWithThumbprint(JsonWebSignature jws, String thumbprint) {
             }
         }
 
+        private Key getKeyWithS256Thumbprint(JsonWebSignature jws, String thumbprint) {
+            if (thumbprint != null) {
+                return jwks.getKeyWithS256Thumbprint(thumbprint);
+            } else {
+                LOG.debug("Token 'x5tS256' header is not set");
+                return null;
+            }
+        }
+
         public Uni<Void> refresh() {
             final long now = now();
             if (now > lastForcedRefreshTime + forcedJwksRefreshIntervalMilliSecs) {

diff --git a/...ests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java b/...ests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java
@@ -14,6 +14,7 @@
 
 import org.awaitility.Awaitility;
 import org.hamcrest.Matchers;
+import org.jose4j.jwx.HeaderParameterNames;
 import org.junit.jupiter.api.Test;
 
 import com.github.tomakehurst.wiremock.WireMockServer;
@@ -140,6 +141,31 @@ public void testAccessAdminResourceWithCertThumbprint() {
                 .body(Matchers.containsString("admin"));
     }
 
+    @Test
+    public void testAccessAdminResourceWithWrongCertThumbprint() {
+        RestAssured.given().auth().oauth2(getAccessTokenWithWrongThumbprint("admin", Set.of("admin")))
+                .when().get("/api/admin/bearer-no-introspection")
+                .then()
+                .statusCode(401);
+    }
+
+    @Test
+    public void testAccessAdminResourceWithCertS256Thumbprint() {
+        RestAssured.given().auth().oauth2(getAccessTokenWithS256Thumbprint("admin", Set.of("admin")))
+                .when().get("/api/admin/bearer-no-introspection")
+                .then()
+                .statusCode(200)
+                .body(Matchers.containsString("admin"));
+    }
+
+    @Test
+    public void testAccessAdminResourceWithWrongCertS256Thumbprint() {
+        RestAssured.given().auth().oauth2(getAccessTokenWithWrongS256Thumbprint("admin", Set.of("admin")))
+                .when().get("/api/admin/bearer-no-introspection")
+                .then()
+                .statusCode(401);
+    }
+
     @Test
     public void testAccessAdminResourceWithCustomRolePathForbidden() {
         RestAssured.given().auth().oauth2(getAccessTokenWithCustomRolePath("admin", Set.of("admin")))
@@ -329,6 +355,33 @@ private String getAccessTokenWithThumbprint(String userName, Set<String> groups)
                 .sign("privateKeyWithoutKid.jwk");
     }
 
+    private String getAccessTokenWithWrongThumbprint(String userName, Set<String> groups) {
+        return Jwt.preferredUserName(userName)
+                .groups(groups)
+                .issuer("https://server.example.com")
+                .audience("https://service.example.com")
+                .jws().header(HeaderParameterNames.X509_CERTIFICATE_THUMBPRINT, "123")
+                .sign("privateKeyWithoutKid.jwk");
+    }
+
+    private String getAccessTokenWithS256Thumbprint(String userName, Set<String> groups) {
+        return Jwt.preferredUserName(userName)
+                .groups(groups)
+                .issuer("https://server.example.com")
+                .audience("https://service.example.com")
+                .jws().thumbprintS256(OidcWiremockTestResource.getCertificate())
+                .sign("privateKeyWithoutKid.jwk");
+    }
+
+    private String getAccessTokenWithWrongS256Thumbprint(String userName, Set<String> groups) {
+        return Jwt.preferredUserName(userName)
+                .groups(groups)
+                .issuer("https://server.example.com")
+                .audience("https://service.example.com")
+                .jws().header(HeaderParameterNames.X509_CERTIFICATE_SHA256_THUMBPRINT, "123")
+                .sign("privateKeyWithoutKid.jwk");
+    }
+
     private String getAccessTokenWithoutKidAndThumbprint(String userName, Set<String> groups) {
         return Jwt.preferredUserName(userName)
                 .groups(groups)