From 8e19b6c654b0f714c734ffa437f7cbaec29d613f Mon Sep 17 00:00:00 2001 From: Matthew Buckett Date: Fri, 2 Jan 2026 14:51:40 +0000 Subject: [PATCH 1/2] Cache on URI not registration The cache should be based on the URI that we retrieve the JWK set from and not the client registration. That way if the JWK set URI changes we will start looking for a different value in the cache and the change will correctly be picked up. This also makes the cache much more efficient as if multiple tools (clientRegistrations) share the same JWK set then they will share the same JwtDecoder. --- .../OidcLaunchFlowAuthenticationProvider.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/authentication/OidcLaunchFlowAuthenticationProvider.java b/src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/authentication/OidcLaunchFlowAuthenticationProvider.java index 7d0bee1..7964850 100644 --- a/src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/authentication/OidcLaunchFlowAuthenticationProvider.java +++ b/src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/authentication/OidcLaunchFlowAuthenticationProvider.java @@ -163,26 +163,26 @@ private OidcIdToken createOidcToken(ClientRegistration clientRegistration, Strin } private JwtDecoder getJwtDecoder(ClientRegistration clientRegistration) { - JwtDecoder jwtDecoder = this.jwtDecoders.get(clientRegistration.getRegistrationId()); + final String jwkSetUri = clientRegistration.getProviderDetails().getJwkSetUri(); + if (!StringUtils.hasText(jwkSetUri)) { + OAuth2Error oauth2Error = new OAuth2Error( + MISSING_SIGNATURE_VERIFIER_ERROR_CODE, + "Failed to find a Signature Verifier for Client Registration: '" + + clientRegistration.getRegistrationId() + "'. Check to ensure you have configured the JwkSet URI.", + null + ); + throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); + } + JwtDecoder jwtDecoder = this.jwtDecoders.get(jwkSetUri); if (jwtDecoder == null) { - if (!StringUtils.hasText(clientRegistration.getProviderDetails().getJwkSetUri())) { - OAuth2Error oauth2Error = new OAuth2Error( - MISSING_SIGNATURE_VERIFIER_ERROR_CODE, - "Failed to find a Signature Verifier for Client Registration: '" + - clientRegistration.getRegistrationId() + "'. Check to ensure you have configured the JwkSet URI.", - null - ); - throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); - } // TODO This should look at the Cache-Control header so to expire old jwtDecoders. // Canvas looks to rotate it's keys monthly. - String jwkSetUri = clientRegistration.getProviderDetails().getJwkSetUri(); NimbusJwtDecoder.JwkSetUriJwtDecoderBuilder decoderBuilder = NimbusJwtDecoder.withJwkSetUri(jwkSetUri).jwsAlgorithm(SignatureAlgorithm.from(JwsAlgorithms.RS256)); if (restOperations != null) { decoderBuilder.restOperations(restOperations); } jwtDecoder = decoderBuilder.build(); - this.jwtDecoders.put(clientRegistration.getRegistrationId(), jwtDecoder); + this.jwtDecoders.put(jwkSetUri, jwtDecoder); } return jwtDecoder; } From 237658b66fb34909b5473705862929da6235cd5d Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 2 Jan 2026 15:21:25 +0000 Subject: [PATCH 2/2] Fix race condition in JwtDecoder caching with computeIfAbsent (#57) * Initial plan * Fix race condition using computeIfAbsent for atomic JwtDecoder creation Co-authored-by: buckett <5921+buckett@users.noreply.github.com> * Fix grammar in TODO comment (it's -> its) Co-authored-by: buckett <5921+buckett@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: buckett <5921+buckett@users.noreply.github.com> --- .../OidcLaunchFlowAuthenticationProvider.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/authentication/OidcLaunchFlowAuthenticationProvider.java b/src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/authentication/OidcLaunchFlowAuthenticationProvider.java index 7964850..c8b5923 100644 --- a/src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/authentication/OidcLaunchFlowAuthenticationProvider.java +++ b/src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/authentication/OidcLaunchFlowAuthenticationProvider.java @@ -173,17 +173,14 @@ private JwtDecoder getJwtDecoder(ClientRegistration clientRegistration) { ); throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); } - JwtDecoder jwtDecoder = this.jwtDecoders.get(jwkSetUri); - if (jwtDecoder == null) { - // TODO This should look at the Cache-Control header so to expire old jwtDecoders. - // Canvas looks to rotate it's keys monthly. - NimbusJwtDecoder.JwkSetUriJwtDecoderBuilder decoderBuilder = NimbusJwtDecoder.withJwkSetUri(jwkSetUri).jwsAlgorithm(SignatureAlgorithm.from(JwsAlgorithms.RS256)); + // TODO This should look at the Cache-Control header so to expire old jwtDecoders. + // Canvas looks to rotate its keys monthly. + return this.jwtDecoders.computeIfAbsent(jwkSetUri, uri -> { + NimbusJwtDecoder.JwkSetUriJwtDecoderBuilder decoderBuilder = NimbusJwtDecoder.withJwkSetUri(uri).jwsAlgorithm(SignatureAlgorithm.from(JwsAlgorithms.RS256)); if (restOperations != null) { decoderBuilder.restOperations(restOperations); } - jwtDecoder = decoderBuilder.build(); - this.jwtDecoders.put(jwkSetUri, jwtDecoder); - } - return jwtDecoder; + return decoderBuilder.build(); + }); } }