rmannibucau commented on code in PR #7:
URL: https://github.com/apache/geronimo-jwt-auth/pull/7#discussion_r1557913665


##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -79,9 +101,48 @@ private void init() {
                                     .collect(Collectors.toSet()))
                                 .orElseGet(HashSet::new);
         ofNullable(config.read("issuer.default", config.read(Names.ISSUER, 
null))).ifPresent(defaultIssuers::add);
+        jwksUrl = config.read("mp.jwt.verify.publickey.location", null);
+        readerFactory = Json.createReaderFactory(emptyMap());
+        ofNullable(jwksUrl).ifPresent(url -> {
+            HttpClient.Builder builder = HttpClient.newBuilder();

Review Comment:
   guess minimum is a `protected void customize(HttpClient.Builder)` method - 
to be transparent I can't replace by custom code fillung the key map with that 
cause of missing sslContext+sslParameters setup, not asking to include it by 
default but enable it - guess the protected method is the fastest but I'm fine 
with an event too.



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -79,9 +101,48 @@ private void init() {
                                     .collect(Collectors.toSet()))
                                 .orElseGet(HashSet::new);
         ofNullable(config.read("issuer.default", config.read(Names.ISSUER, 
null))).ifPresent(defaultIssuers::add);
+        jwksUrl = config.read("mp.jwt.verify.publickey.location", null);
+        readerFactory = Json.createReaderFactory(emptyMap());
+        ofNullable(jwksUrl).ifPresent(url -> {
+            HttpClient.Builder builder = HttpClient.newBuilder();
+            if (getJwksRefreshInterval() != null) {
+                long secondsRefresh = getJwksRefreshInterval();
+                backgroundThread = 
Executors.newSingleThreadScheduledExecutor();
+                builder.executor(backgroundThread);
+                backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, 
getJwksRefreshInterval(), secondsRefresh, SECONDS);
+            }
+            httpClient = builder.build();
+            reloadJwksRequest = reloadRemoteKeys();// inital load, otherwise 
the background thread is too slow to start and serve
+        });
         defaultKey = config.read("public-key.default", 
config.read(Names.VERIFIER_PUBLIC_KEY, null));
     }
 
+    private Integer getJwksRefreshInterval() {
+        String interval = config.read("jwks.invalidation.interval",null);
+        if (interval != null) {
+            return Integer.parseInt(interval);
+        } else {
+            return null;
+        }
+    }
+
+    private CompletableFuture<Void> reloadRemoteKeys() {
+        HttpRequest request = 
HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", 
"application/json").build();

Review Comment:
   can be worth to be protected too, some servers are not that friendly (don't 
hit, not my choice ;))



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -16,43 +16,65 @@
  */
 package org.apache.geronimo.microprofile.impl.jwtauth.jwt;
 
+import static java.util.Collections.emptyMap;
 import static java.util.Optional.ofNullable;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.joining;
+import static java.util.stream.Collectors.toList;
+import 
org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig;
+import org.apache.geronimo.microprofile.impl.jwtauth.io.PropertiesLoader;
+import org.eclipse.microprofile.jwt.config.Names;
 
+import javax.annotation.PostConstruct;
+import javax.annotation.PreDestroy;
+import javax.enterprise.context.ApplicationScoped;
+import javax.inject.Inject;
+import javax.json.Json;
+import javax.json.JsonArray;
+import javax.json.JsonObject;
+import javax.json.JsonReader;
+import javax.json.JsonReaderFactory;
+import javax.json.JsonValue;
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.io.StringReader;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
 import java.nio.file.Files;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import javax.annotation.PostConstruct;
-import javax.enterprise.context.ApplicationScoped;
-import javax.inject.Inject;
-
-import 
org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig;
-import org.apache.geronimo.microprofile.impl.jwtauth.io.PropertiesLoader;
-import org.eclipse.microprofile.jwt.config.Names;
-
 @ApplicationScoped
 public class KidMapper {
     @Inject
     private GeronimoJwtAuthConfig config;
 
-    private final ConcurrentMap<String, String> keyMapping = new 
ConcurrentHashMap<>();
+    private ConcurrentMap<String, String> keyMapping = new 
ConcurrentHashMap<>();
     private final Map<String, Collection<String>> issuerMapping = new 
HashMap<>();
     private String defaultKey;
+    private String jwksUrl;
     private Set<String> defaultIssuers;
-
+    private JsonReaderFactory readerFactory;
+    private CompletableFuture<Void> reloadJwksRequest;
+    HttpClient httpClient;

Review Comment:
   private?



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -79,9 +101,48 @@ private void init() {
                                     .collect(Collectors.toSet()))
                                 .orElseGet(HashSet::new);
         ofNullable(config.read("issuer.default", config.read(Names.ISSUER, 
null))).ifPresent(defaultIssuers::add);
+        jwksUrl = config.read("mp.jwt.verify.publickey.location", null);
+        readerFactory = Json.createReaderFactory(emptyMap());
+        ofNullable(jwksUrl).ifPresent(url -> {
+            HttpClient.Builder builder = HttpClient.newBuilder();
+            if (getJwksRefreshInterval() != null) {
+                long secondsRefresh = getJwksRefreshInterval();
+                backgroundThread = 
Executors.newSingleThreadScheduledExecutor();
+                builder.executor(backgroundThread);
+                backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, 
getJwksRefreshInterval(), secondsRefresh, SECONDS);
+            }
+            httpClient = builder.build();
+            reloadJwksRequest = reloadRemoteKeys();// inital load, otherwise 
the background thread is too slow to start and serve
+        });
         defaultKey = config.read("public-key.default", 
config.read(Names.VERIFIER_PUBLIC_KEY, null));
     }
 
+    private Integer getJwksRefreshInterval() {
+        String interval = config.read("jwks.invalidation.interval",null);
+        if (interval != null) {
+            return Integer.parseInt(interval);
+        } else {
+            return null;
+        }
+    }
+
+    private CompletableFuture<Void> reloadRemoteKeys() {
+        HttpRequest request = 
HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", 
"application/json").build();
+        CompletableFuture<HttpResponse<String>> httpResponseCompletableFuture 
= httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString());
+        CompletableFuture<Void> ongoingRequest = 
httpResponseCompletableFuture.thenApply(result -> {

Review Comment:
   `.thenApplyAsync(...., httpClient.executor().orElseThrow())` otherwise it is 
ran in commonPool which is something we don't want nor we control there.



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -79,9 +101,48 @@ private void init() {
                                     .collect(Collectors.toSet()))
                                 .orElseGet(HashSet::new);
         ofNullable(config.read("issuer.default", config.read(Names.ISSUER, 
null))).ifPresent(defaultIssuers::add);
+        jwksUrl = config.read("mp.jwt.verify.publickey.location", null);
+        readerFactory = Json.createReaderFactory(emptyMap());
+        ofNullable(jwksUrl).ifPresent(url -> {
+            HttpClient.Builder builder = HttpClient.newBuilder();
+            if (getJwksRefreshInterval() != null) {

Review Comment:
   store the interval in a local var to avoid to call it N times



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -79,9 +101,48 @@ private void init() {
                                     .collect(Collectors.toSet()))
                                 .orElseGet(HashSet::new);
         ofNullable(config.read("issuer.default", config.read(Names.ISSUER, 
null))).ifPresent(defaultIssuers::add);
+        jwksUrl = config.read("mp.jwt.verify.publickey.location", null);
+        readerFactory = Json.createReaderFactory(emptyMap());
+        ofNullable(jwksUrl).ifPresent(url -> {
+            HttpClient.Builder builder = HttpClient.newBuilder();
+            if (getJwksRefreshInterval() != null) {
+                long secondsRefresh = getJwksRefreshInterval();
+                backgroundThread = 
Executors.newSingleThreadScheduledExecutor();
+                builder.executor(backgroundThread);
+                backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, 
getJwksRefreshInterval(), secondsRefresh, SECONDS);
+            }
+            httpClient = builder.build();
+            reloadJwksRequest = reloadRemoteKeys();// inital load, otherwise 
the background thread is too slow to start and serve

Review Comment:
   if interval is null or < 0 just drop the client (`httpClient=null`)



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -79,9 +101,48 @@ private void init() {
                                     .collect(Collectors.toSet()))
                                 .orElseGet(HashSet::new);
         ofNullable(config.read("issuer.default", config.read(Names.ISSUER, 
null))).ifPresent(defaultIssuers::add);
+        jwksUrl = config.read("mp.jwt.verify.publickey.location", null);
+        readerFactory = Json.createReaderFactory(emptyMap());
+        ofNullable(jwksUrl).ifPresent(url -> {
+            HttpClient.Builder builder = HttpClient.newBuilder();
+            if (getJwksRefreshInterval() != null) {
+                long secondsRefresh = getJwksRefreshInterval();
+                backgroundThread = 
Executors.newSingleThreadScheduledExecutor();
+                builder.executor(backgroundThread);
+                backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, 
getJwksRefreshInterval(), secondsRefresh, SECONDS);
+            }
+            httpClient = builder.build();
+            reloadJwksRequest = reloadRemoteKeys();// inital load, otherwise 
the background thread is too slow to start and serve
+        });
         defaultKey = config.read("public-key.default", 
config.read(Names.VERIFIER_PUBLIC_KEY, null));
     }
 
+    private Integer getJwksRefreshInterval() {
+        String interval = config.read("jwks.invalidation.interval",null);
+        if (interval != null) {
+            return Integer.parseInt(interval);
+        } else {
+            return null;
+        }
+    }
+
+    private CompletableFuture<Void> reloadRemoteKeys() {
+        HttpRequest request = 
HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", 
"application/json").build();
+        CompletableFuture<HttpResponse<String>> httpResponseCompletableFuture 
= httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString());
+        CompletableFuture<Void> ongoingRequest = 
httpResponseCompletableFuture.thenApply(result -> {
+            List<JWK> jwks = parseKeys(result);
+            ConcurrentHashMap<String, String> newKeys = new 
ConcurrentHashMap<>();
+            jwks.forEach(key -> newKeys.put(key.getKid(), key.toPemKey()));
+            keyMapping = newKeys;
+            return null;
+        });
+
+        ongoingRequest.thenRun(() -> {

Review Comment:
   not needed, just set `reloadJwksRequest` in previous line



##########
src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapperTest.java:
##########
@@ -0,0 +1,52 @@
+package org.apache.geronimo.microprofile.impl.jwtauth.jwt;
+
+import static javax.ws.rs.client.ClientBuilder.newClient;
+import static org.testng.Assert.assertEquals;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.net.URL;
+
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.container.test.api.RunAsClient;
+import org.jboss.arquillian.test.api.ArquillianResource;
+import org.jboss.arquillian.testng.Arquillian;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.Test;
+
+public class KidMapperTest extends Arquillian {
+
+    private static JwksServer jwksServer;
+
+    @Deployment()
+    public static WebArchive createDeployment() throws Exception {
+        jwksServer = new JwksServer();
+        jwksServer.start();
+        System.setProperty("mp.jwt.verify.publickey.location", 
"http://localhost:"; + jwksServer.getPort() + "/jwks.json");

Review Comment:
   where is it cleaned?
   
   tip: inject it in the war as a mp properties file
   
   side note: true for all tests



##########
src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/JwksServer.java:
##########
@@ -0,0 +1,56 @@
+package org.apache.geronimo.microprofile.impl.jwtauth.jwt;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.PrintWriter;
+import java.net.ServerSocket;
+import java.net.Socket;
+
+class JwksServer {

Review Comment:
   why not `HttpServer`? - not a blocker more curious



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -120,7 +181,44 @@ private String tryLoad(final String value) {
             throw new IllegalArgumentException(e);
         }
 
-        // else direct value
+        // load jwks via url
+        if (jwksUrl != null) {
+            if(reloadJwksRequest != null) {
+                try {
+                    reloadJwksRequest.get();
+                } catch (InterruptedException | ExecutionException e) {
+                    throw new IllegalStateException(e);
+                }
+            }
+                String key = keyMapping.get(value);
+                if (key != null) {
+                    return key;
+                }
+
+        }
         return value;
     }
+
+    private List<JWK> parseKeys(HttpResponse<String> keyResponse) {
+        StringReader stringReader = new StringReader(keyResponse.body());

Review Comment:
   > try (final JsonReader reader = readerfactory.createReader(new 
StringReader(keyResponse.body()))) {...}
   
   otherwise you leak memory (jsonp)



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -79,9 +101,48 @@ private void init() {
                                     .collect(Collectors.toSet()))
                                 .orElseGet(HashSet::new);
         ofNullable(config.read("issuer.default", config.read(Names.ISSUER, 
null))).ifPresent(defaultIssuers::add);
+        jwksUrl = config.read("mp.jwt.verify.publickey.location", null);
+        readerFactory = Json.createReaderFactory(emptyMap());
+        ofNullable(jwksUrl).ifPresent(url -> {
+            HttpClient.Builder builder = HttpClient.newBuilder();
+            if (getJwksRefreshInterval() != null) {
+                long secondsRefresh = getJwksRefreshInterval();
+                backgroundThread = 
Executors.newSingleThreadScheduledExecutor();

Review Comment:
   `ThreadFactory` to name the thread (kill -3 friendly ;))



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -120,7 +181,44 @@ private String tryLoad(final String value) {
             throw new IllegalArgumentException(e);
         }
 
-        // else direct value
+        // load jwks via url
+        if (jwksUrl != null) {
+            if(reloadJwksRequest != null) {
+                try {
+                    reloadJwksRequest.get();
+                } catch (InterruptedException | ExecutionException e) {
+                    throw new IllegalStateException(e);
+                }
+            }
+                String key = keyMapping.get(value);
+                if (key != null) {
+                    return key;
+                }
+
+        }
         return value;
     }
+
+    private List<JWK> parseKeys(HttpResponse<String> keyResponse) {
+        StringReader stringReader = new StringReader(keyResponse.body());
+        JsonReader jwksReader = readerFactory.createReader(stringReader);
+        JsonObject keySet = jwksReader.readObject();
+        JsonArray keys = keySet.getJsonArray("keys");
+        return keys.stream()
+                .map(JsonValue::asJsonObject)
+                .map(JWK::new)
+                .filter(it -> it.getUse() == null || "sig".equals(it.getUse()))
+                .collect(toList());
+    }
+
+    @PreDestroy
+    private void destroy() {
+        if (backgroundThread != null) {
+            backgroundThread.shutdown();

Review Comment:
     + await (1mn can be hardcoded since it should be almost immediate - one 
call duration) to try our best to not leak a thread before quitting this method 
(the class being the thread owner)
   
   side note: the cancel should be done before the await



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -16,43 +16,65 @@
  */
 package org.apache.geronimo.microprofile.impl.jwtauth.jwt;
 
+import static java.util.Collections.emptyMap;
 import static java.util.Optional.ofNullable;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.joining;
+import static java.util.stream.Collectors.toList;
+import 
org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig;
+import org.apache.geronimo.microprofile.impl.jwtauth.io.PropertiesLoader;
+import org.eclipse.microprofile.jwt.config.Names;
 
+import javax.annotation.PostConstruct;
+import javax.annotation.PreDestroy;
+import javax.enterprise.context.ApplicationScoped;
+import javax.inject.Inject;
+import javax.json.Json;
+import javax.json.JsonArray;
+import javax.json.JsonObject;
+import javax.json.JsonReader;
+import javax.json.JsonReaderFactory;
+import javax.json.JsonValue;
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.io.StringReader;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
 import java.nio.file.Files;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import javax.annotation.PostConstruct;
-import javax.enterprise.context.ApplicationScoped;
-import javax.inject.Inject;
-
-import 
org.apache.geronimo.microprofile.impl.jwtauth.config.GeronimoJwtAuthConfig;
-import org.apache.geronimo.microprofile.impl.jwtauth.io.PropertiesLoader;
-import org.eclipse.microprofile.jwt.config.Names;
-
 @ApplicationScoped
 public class KidMapper {
     @Inject
     private GeronimoJwtAuthConfig config;
 
-    private final ConcurrentMap<String, String> keyMapping = new 
ConcurrentHashMap<>();
+    private ConcurrentMap<String, String> keyMapping = new 
ConcurrentHashMap<>();

Review Comment:
   volatile (not critical at all, just want to ensure we thought about it and 
use it or not deliberatly)



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -120,7 +181,44 @@ private String tryLoad(final String value) {
             throw new IllegalArgumentException(e);
         }
 
-        // else direct value
+        // load jwks via url
+        if (jwksUrl != null) {
+            if(reloadJwksRequest != null) {
+                try {
+                    reloadJwksRequest.get();
+                } catch (InterruptedException | ExecutionException e) {

Review Comment:
   split `InterruptedException` and `Thread.currentThread().interrupt()` there ?
   
   catch of `ExecutionException` should `throw e.getCause() instanceof 
RuntimeException ? (RuntimeException) e.getCause() : new 
IllegalStateException(e.getCause());` probably



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -120,7 +181,44 @@ private String tryLoad(final String value) {
             throw new IllegalArgumentException(e);
         }
 
-        // else direct value
+        // load jwks via url
+        if (jwksUrl != null) {
+            if(reloadJwksRequest != null) {
+                try {
+                    reloadJwksRequest.get();
+                } catch (InterruptedException | ExecutionException e) {
+                    throw new IllegalStateException(e);
+                }
+            }
+                String key = keyMapping.get(value);
+                if (key != null) {
+                    return key;
+                }
+
+        }
         return value;
     }
+
+    private List<JWK> parseKeys(HttpResponse<String> keyResponse) {
+        StringReader stringReader = new StringReader(keyResponse.body());
+        JsonReader jwksReader = readerFactory.createReader(stringReader);
+        JsonObject keySet = jwksReader.readObject();
+        JsonArray keys = keySet.getJsonArray("keys");
+        return keys.stream()
+                .map(JsonValue::asJsonObject)
+                .map(JWK::new)
+                .filter(it -> it.getUse() == null || "sig".equals(it.getUse()))

Review Comment:
   tip: filter should be before map generally speaking even if not critical



##########
src/main/java/org/apache/geronimo/microprofile/impl/jwtauth/jwt/KidMapper.java:
##########
@@ -79,9 +101,48 @@ private void init() {
                                     .collect(Collectors.toSet()))
                                 .orElseGet(HashSet::new);
         ofNullable(config.read("issuer.default", config.read(Names.ISSUER, 
null))).ifPresent(defaultIssuers::add);
+        jwksUrl = config.read("mp.jwt.verify.publickey.location", null);
+        readerFactory = Json.createReaderFactory(emptyMap());
+        ofNullable(jwksUrl).ifPresent(url -> {
+            HttpClient.Builder builder = HttpClient.newBuilder();
+            if (getJwksRefreshInterval() != null) {
+                long secondsRefresh = getJwksRefreshInterval();
+                backgroundThread = 
Executors.newSingleThreadScheduledExecutor();
+                builder.executor(backgroundThread);
+                backgroundThread.scheduleAtFixedRate(this::reloadRemoteKeys, 
getJwksRefreshInterval(), secondsRefresh, SECONDS);
+            }
+            httpClient = builder.build();
+            reloadJwksRequest = reloadRemoteKeys();// inital load, otherwise 
the background thread is too slow to start and serve
+        });
         defaultKey = config.read("public-key.default", 
config.read(Names.VERIFIER_PUBLIC_KEY, null));
     }
 
+    private Integer getJwksRefreshInterval() {
+        String interval = config.read("jwks.invalidation.interval",null);
+        if (interval != null) {
+            return Integer.parseInt(interval);
+        } else {
+            return null;
+        }
+    }
+
+    private CompletableFuture<Void> reloadRemoteKeys() {
+        HttpRequest request = 
HttpRequest.newBuilder().GET().uri(URI.create(jwksUrl)).header("Accept", 
"application/json").build();
+        CompletableFuture<HttpResponse<String>> httpResponseCompletableFuture 
= httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString());
+        CompletableFuture<Void> ongoingRequest = 
httpResponseCompletableFuture.thenApply(result -> {
+            List<JWK> jwks = parseKeys(result);
+            ConcurrentHashMap<String, String> newKeys = new 
ConcurrentHashMap<>();

Review Comment:
   > keyMapping = parseKeys(result).stream().collect(toMap(JWK::kid, 
JWK::toPemKey));
   
   side note: should we tolerate kid-less keys if single and use the default id?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@geronimo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to