Use Keycloak's HttpClientProvider to jwks download#55
Conversation
…ault one. These are taken from Keycloak's internal connection pool and natively support a proxy.
|
There was a problem hiding this comment.
Pull request overview
This PR migrates from Java's default HttpClient to Keycloak's HttpClientProvider for downloading JWKS (JSON Web Key Set) data. This change enables the use of Keycloak's managed connection pool and native proxy support.
Key Changes:
- Replaced
java.net.http.HttpClientwith ApacheCloseableHttpClientobtained from Keycloak'sHttpClientProvider - Updated method signatures to accept
ScimContextparameter for accessing Keycloak's HTTP client pool - Migrated HTTP request/response handling from Java's HTTP client API to Apache HttpClient API
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| JwksUtils.java | Refactored to use HttpClientProvider from Keycloak's session instead of creating a new HttpClient, and updated to use Apache HttpClient API for the HTTP request |
| ExternalTokenVerifier.java | Updated verify() method to accept and pass ScimContext parameter to JwksUtils.getPublicKeysFromJwks() |
| AbstractScimServer.java | Updated call to verifier.verify() to pass the scimContext parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @return list of public keys | ||
| */ | ||
| public static List<JwkKey> getPublicKeysFromJwks(String jwksUrl) throws URISyntaxException, IOException, InterruptedException { | ||
| public static List<JwkKey> getPublicKeysFromJwks(String jwksUrl, ScimContext scimContext) throws URISyntaxException, IOException, InterruptedException { |
There was a problem hiding this comment.
The method signature declares that it throws InterruptedException, but Apache HttpClient's execute() method does not throw InterruptedException. This was likely carried over from the previous java.net.http.HttpClient implementation. The InterruptedException should be removed from the throws clause as it's no longer applicable.
| public static List<JwkKey> getPublicKeysFromJwks(String jwksUrl, ScimContext scimContext) throws URISyntaxException, IOException, InterruptedException { | |
| public static List<JwkKey> getPublicKeysFromJwks(String jwksUrl, ScimContext scimContext) throws URISyntaxException, IOException { |
| */ | ||
| public boolean verify(String tokenString) throws URISyntaxException, IOException, InterruptedException, JWSInputException { | ||
| for (JwkKey jwkKey : JwksUtils.getPublicKeysFromJwks(jwksUrl)) { | ||
| public boolean verify(String tokenString, ScimContext scimContext) throws URISyntaxException, IOException, InterruptedException, JWSInputException { |
There was a problem hiding this comment.
The method signature declares that it throws InterruptedException, but this is no longer thrown by the underlying Apache HttpClient implementation. This should be removed from the throws clause to accurately reflect the method's behavior.
| public boolean verify(String tokenString, ScimContext scimContext) throws URISyntaxException, IOException, InterruptedException, JWSInputException { | |
| public boolean verify(String tokenString, ScimContext scimContext) throws URISyntaxException, IOException, JWSInputException { |
Use HttpClientProvider to get an HttpClient instead of creating a default one. These are taken from Keycloak's internal connection pool and natively support a proxy.