New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
com.auth0.jwk.Jwk#getPublicKey recreates PublicKey object on every access #150
Comments
Hey @adrian-skybaker, thanks for raising. We'll look into this and see if there's something we can do to minimize the overhead. |
@adrian-skybaker you are correct that while the We can't move the calculation of the key to the constructor and then just return the computed value because of the potential exception handling. Perhaps we could implement a simple in-memory lazy loading by computing the key the first time and then storing the value: public class Jwk {
...
// added field
private PublicKey publicKey;
...
public synchronized PublicKey getPublicKey() throws InvalidPublicKeyException {
if (Objects.isNull(this.publicKey)) {
this.publicKey = computeKey();
}
return this.publicKey;
}
private synchronized PublicKey computeKey() throws InvalidPublicKeyException {
// calculate and return public key
}
} Need to think about it a bit more but interested in others thoughts. |
We're not going to implement any caching of the the public key at this time, as the introduction of caching could bring about complexities through synchronization/eviction/rotation, etc. Other Auth0 libraries also do not cache the public key. We will monitor this issue for additional interest, but in the meantime it's recommended that clients that demonstrate high usage load here cache the key locally. |
Perhaps the issue is misunderstood. The issue is that the |
This solution looks fine to me.
That's possible, but quite awkward if you want to still leverage the built-in caching and refresh logic etc. Here's our solution: /** Workaround https://github.com/auth0/jwks-rsa-java/issues/150, avoid recreating the PublicKey object every request */
static class FastJwk extends Jwk {
private PublicKey publicKey;
@Builder
public FastJwk(
String id,
String type,
String algorithm,
String usage,
List<String> operations,
String certificateUrl,
List<String> certificateChain,
String certificateThumbprint,
Map<String, Object> additionalAttributes) {
super(id, type, algorithm, usage, operations, certificateUrl, certificateChain, certificateThumbprint, additionalAttributes);
}
@Override
public PublicKey getPublicKey() throws InvalidPublicKeyException {
// We don't care about double checked locking, its harmless if we create twice in race conditions
if(this.publicKey == null) {
this.publicKey = super.getPublicKey();
}
return this.publicKey;
}
} @RequiredArgsConstructor
/** Provides {@link FastJwk} instances */
static class FastJwtProvider implements JwkProvider {
private final UrlJwkProvider delegate;
@Override
public Jwk get(String keyId) throws JwkException {
Jwk jwt = delegate.get(keyId);
return FastJwk.builder()
.id(jwt.getId())
.type(jwt.getType())
.algorithm(jwt.getAlgorithm())
.usage(jwt.getUsage())
.operations(jwt.getOperationsAsList())
.certificateUrl(jwt.getCertificateUrl())
.certificateChain(jwt.getCertificateChain())
.certificateThumbprint(jwt.getCertificateThumbprint())
.additionalAttributes(jwt.getAdditionalAttributes())
.build();
}
} UrlJwkProvider urlJwkProvider = new UrlJwkProvider("...domain...");
final JwkProvider jwtProvider = new FastJwtProvider(urlJwkProvider); |
Tagging @jimmyjames in case these comments aren't visible. I believe if the rationale for closing this was to avoid "synchronization/eviction/rotation", then the change is misunderstood, none of these aspects are required. |
👋 hey @adrian-skybaker, thanks for all the info and your implementation notes. We will add this feature to our backlog; that said, if you (or anyone else) would like to provide a PR before we get to it (I think the snippet I provided above will work), I'm happy to review! |
Even when using a cached JwkProvider, the
com.auth0.jwk.Jwk#getPublicKey
method will reconstruct a public key object in memory on every call.Although the caching prevents remote calls, at high volumes (eg thousands requests/sec, each requiring JWT verification) this amount of object allocation creates a measureable overhead.
The text was updated successfully, but these errors were encountered: