feat(sdk): replace ayza libraries with TrustProvider on JCA#366
feat(sdk): replace ayza libraries with TrustProvider on JCA#366mkleene wants to merge 3 commits into
Conversation
Remove the three io.github.hakky54:ayza* dependencies and replace their TLS trust-material role with an SDK-owned TrustProvider built on provider-agnostic JCA APIs (CertificateFactory, KeyStore, TrustManagerFactory, SSLContext). This works under any registered crypto provider, including BC-FIPS, and avoids hardcoded provider names. - Add TrustProvider and package-private CompositeX509ExtendedTrustManager for combining JVM default + custom trust material. - SDKBuilder: replace SSLFactory field with SSLSocketFactory + X509TrustManager. sslFactory(SSLFactory) becomes sslFactory(SSLSocketFactory); add sslFactory(SSLSocketFactory, X509TrustManager) for callers that have a matching trust manager. sslFactoryFromDirectory / sslFactoryFromKeyStore signatures and semantics are preserved, now backed by TrustProvider internally. - TokenSource takes SSLSocketFactory directly. - Command.java --insecure path uses TrustProvider.insecure(). - SDKBuilderTest reworked to drop nl.altindag imports and use TrustProvider + standard JCA. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the external SSLFactory dependency with a native JCA-based implementation using the new TrustProvider and CompositeX509ExtendedTrustManager classes. The SDKBuilder, TokenSource, and associated tests have been updated to work with standard SSLSocketFactory and X509TrustManager objects. Reviewers suggested using InputStream.readAllBytes() for better performance, deduplicating keystore types during loading, and ensuring that X509TrustManager instances are provided alongside socket factories to avoid deprecated OkHttp methods and potential validation issues.
| private static byte[] readAll(InputStream in) throws IOException { | ||
| java.io.ByteArrayOutputStream out = new java.io.ByteArrayOutputStream(); | ||
| byte[] buf = new byte[8192]; | ||
| int n; | ||
| while ((n = in.read(buf)) >= 0) { | ||
| out.write(buf, 0, n); | ||
| } | ||
| return out.toByteArray(); | ||
| } |
There was a problem hiding this comment.
The readAll method is redundant as InputStream.readAllBytes() is available since Java 9. Since the project targets Java 11, it's better to use the built-in method for better performance and readability.
private static byte[] readAll(InputStream in) throws IOException {
return in.readAllBytes();
}| // registered fulfills the request. | ||
| byte[] bytes = readAll(in); | ||
| KeyStoreException last = null; | ||
| for (String type : new String[]{KeyStore.getDefaultType(), "JKS", "PKCS12"}) { |
There was a problem hiding this comment.
The loop iterates over a fixed array that likely contains duplicate values (e.g., KeyStore.getDefaultType() is often "PKCS12" on modern JVMs). This leads to redundant attempts to load the keystore if the first attempt fails.
| for (String type : new String[]{KeyStore.getDefaultType(), "JKS", "PKCS12"}) { | |
| for (String type : new java.util.LinkedHashSet<>(java.util.Arrays.asList(KeyStore.getDefaultType(), "JKS", "PKCS12"))) { |
| // Caller supplied an SSLSocketFactory without a matching trust manager (e.g. via | ||
| // sslFactory(SSLSocketFactory)). Falls back to OkHttp's reflection-based platform | ||
| // default trust manager — only the SSLSocketFactory governs the actual handshake. | ||
| httpClient.sslSocketFactory(sslSocketFactory); | ||
| } |
There was a problem hiding this comment.
The httpClient.sslSocketFactory(SSLSocketFactory) method is deprecated in OkHttp 4.x. It relies on reflection to find a matching X509TrustManager, which can fail on modern JVMs with strict encapsulation or on Android. Since the SDK now provides a TrustProvider that returns both the socket factory and the trust manager, it's highly recommended to always provide both to OkHttp. For the sslFactory(SSLSocketFactory) overload, consider documenting that it may fail on some platforms or require a matching trust manager.
| * @param sslSocketFactory Optional SSLSocketFactory for token endpoint requests | ||
| */ | ||
| public TokenSource(ClientAuthentication clientAuth, RSAKey rsaKey, URI tokenEndpointURI, AuthorizationGrant authzGrant, SSLFactory sslFactory) { | ||
| public TokenSource(ClientAuthentication clientAuth, RSAKey rsaKey, URI tokenEndpointURI, AuthorizationGrant authzGrant, SSLSocketFactory sslSocketFactory) { |
There was a problem hiding this comment.
The TokenSource constructor now takes an SSLSocketFactory but not the matching X509TrustManager. Similar to the OkHttp client in SDKBuilder, the underlying HTTP client used by the Nimbus OAuth2 SDK may require the trust manager for proper certificate validation and platform compatibility. Consider passing the X509TrustManager to TokenSource as well.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| try { | ||
| SSLContext ctx = SSLContext.getInstance("TLS"); | ||
| X509ExtendedTrustManager trustAll = new InsecureTrustManager(); | ||
| ctx.init(new KeyManager[0], new TrustManager[]{trustAll}, new SecureRandom()); |
2 participants
Remove the three io.github.hakky54:ayza* dependencies and replace their TLS trust-material role with an SDK-owned TrustProvider built on provider-agnostic JCA APIs (CertificateFactory, KeyStore, TrustManagerFactory, SSLContext). This works under any registered crypto provider, including BC-FIPS, and avoids hardcoded provider names.