You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ns...@apache.org on 2026/08/27 12:01:15 UTC

(sling-org-apache-sling-auth-oauth-client) branch master updated: SLING-13315: Make OIDC JWK set retrieval HTTP limits configurable (#51)

This is an automated email from the ASF dual-hosted git repository.

nscendoni pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-oauth-client.git


The following commit(s) were added to refs/heads/master by this push:
     new 248ea57  SLING-13315: Make OIDC JWK set retrieval HTTP limits configurable (#51)
248ea57 is described below

commit 248ea579e9851deefd9219d0d57d83965d70656c
Author: Nicola Scendoni <sc...@gmail.com>
AuthorDate: Thu Aug 27 14:01:10 2026 +0200

    SLING-13315: Make OIDC JWK set retrieval HTTP limits configurable (#51)
    
    * SLING-13315: Make OIDC JWK set retrieval HTTP limits configurable
    
    The OidcAuthenticationHandler built the IDTokenValidator with the
    constructor that uses Nimbus' default DefaultResourceRetriever, whose
    HTTP entity size limit is fixed at 50 KB. IdPs that publish a large JWK
    set cause token validation to fail with:
    
      Couldn't retrieve remote JWK set: Exceeded configured input limit of
      51200 bytes
    
    Add three OSGi config attributes (jwkSetHttpSizeLimit,
    jwkSetHttpConnectTimeout, jwkSetHttpReadTimeout) and pass an explicitly
    configured DefaultResourceRetriever to IDTokenValidator. Defaults match
    the previous Nimbus defaults (51200 bytes, 500 ms, 500 ms) so behaviour
    is unchanged unless overridden.
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
    
    * SLING-13315: Fix SonarCloud S5778 in JWK size limit test
    
    Hoist createIdToken() and createMockCookies() out of the assertThrows
    lambda so it contains a single invocation that may throw a runtime
    exception.
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
    
    * SLING-13315: Use actual IdP server URL in JWK size limit tests
    
    Replace the hardcoded http://localhost:4567 base URL with the real
    idpServer address so the tests do not rely on a magic port value.
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
    
    * SLING-13315: Validate JWK set retrieval limits and forbid unbounded values
    
    Address review findings on the new JWK set HTTP settings:
    
    - Reject unbounded values: Nimbus treats 0 as infinite for the size
      limit and timeouts, which removes DoS protection on the authentication
      path. Require positive values and cap them at defensible upper bounds
      (10 MB size, 60 s timeouts).
    - Fail fast on bad configuration: values are now validated at activation
      (requireInRange) so a misconfiguration throws immediately with a clear,
      property-named message instead of activating successfully and failing
      on every later OIDC callback. Added metatype min/max constraints.
    
    Adds unit tests for zero, negative and above-max configurations.
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
    
    ---------
    
    Co-authored-by: Claude Opus 4.8 <no...@anthropic.com>
---
 .../impl/OidcAuthenticationHandler.java            | 83 +++++++++++++++++++++-
 .../impl/OidcAuthenticationHandlerTest.java        | 65 +++++++++++++++++
 2 files changed, 146 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java b/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java
index 91ac2b7..4ea79dd 100644
--- a/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java
+++ b/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java
@@ -37,6 +37,7 @@ import java.util.stream.Stream;
 import com.nimbusds.jose.JOSEException;
 import com.nimbusds.jose.JWSAlgorithm;
 import com.nimbusds.jose.proc.BadJOSEException;
+import com.nimbusds.jose.util.DefaultResourceRetriever;
 import com.nimbusds.oauth2.sdk.AuthorizationCode;
 import com.nimbusds.oauth2.sdk.AuthorizationCodeGrant;
 import com.nimbusds.oauth2.sdk.AuthorizationResponse;
@@ -93,6 +94,17 @@ public class OidcAuthenticationHandler extends DefaultAuthenticationFeedbackHand
     public static final String REDIRECT_ATTRIBUTE_NAME = "sling.redirect";
 
     private static final Logger logger = LoggerFactory.getLogger(OidcAuthenticationHandler.class);
+
+    // Nimbus RemoteJWKSet defaults, kept in sync so the default behaviour is unchanged.
+    static final int DEFAULT_JWK_SET_HTTP_SIZE_LIMIT = 50 * 1024;
+    static final int DEFAULT_JWK_SET_HTTP_CONNECT_TIMEOUT = 500;
+    static final int DEFAULT_JWK_SET_HTTP_READ_TIMEOUT = 500;
+
+    // Defensible bounds for the JWK set retrieval settings. These guard the authentication path against a
+    // compromised or malfunctioning IdP: an unbounded ("0") size/timeout is not allowed (Nimbus treats 0 as
+    // infinite), and the values are capped so no single value can be misconfigured to an abusive amount.
+    static final int MAX_JWK_SET_HTTP_SIZE_LIMIT = 10 * 1024 * 1024; // 10 MB
+    static final int MAX_JWK_SET_HTTP_TIMEOUT = 60_000; // 60 s
     private static final String AUTH_TYPE = "oidc";
 
     private final Map<String, ClientConnection> connections;
@@ -127,6 +139,12 @@ public class OidcAuthenticationHandler extends DefaultAuthenticationFeedbackHand
 
     private final OidcLogoutHandler logoutHandler;
 
+    private final int jwkSetHttpSizeLimit;
+
+    private final int jwkSetHttpConnectTimeout;
+
+    private final int jwkSetHttpReadTimeout;
+
     @ObjectClassDefinition(
             name = "Apache Sling Oidc Authentication Handler",
             description = "Apache Sling Oidc Authentication Handler Service")
@@ -196,6 +214,35 @@ public class OidcAuthenticationHandler extends DefaultAuthenticationFeedbackHand
                         + "logout is enabled and this is empty, the service will fail to activate with an exception.",
                 cardinality = Integer.MAX_VALUE)
         String[] logoutRedirectAllowedHosts() default {};
+
+        @AttributeDefinition(
+                name = "JWK set HTTP size limit (bytes)",
+                description = "Maximum size, in bytes, of the JWK set document fetched from the IdP's jwks_uri "
+                        + "when validating the ID token signature. Increase this if the IdP publishes a large key "
+                        + "set and token validation fails with 'Exceeded configured input limit'. Must be between "
+                        + "1 and 10485760 (10 MB); an unbounded value is not allowed on the authentication path. "
+                        + "Default is 51200 (50 KB), the Nimbus library default.",
+                min = "1",
+                max = "" + MAX_JWK_SET_HTTP_SIZE_LIMIT)
+        int jwkSetHttpSizeLimit() default DEFAULT_JWK_SET_HTTP_SIZE_LIMIT;
+
+        @AttributeDefinition(
+                name = "JWK set HTTP connect timeout (ms)",
+                description = "Connect timeout, in milliseconds, for retrieving the JWK set from the IdP's "
+                        + "jwks_uri. Must be between 1 and 60000; an unbounded value is not allowed on the "
+                        + "authentication path. Default is 500.",
+                min = "1",
+                max = "" + MAX_JWK_SET_HTTP_TIMEOUT)
+        int jwkSetHttpConnectTimeout() default DEFAULT_JWK_SET_HTTP_CONNECT_TIMEOUT;
+
+        @AttributeDefinition(
+                name = "JWK set HTTP read timeout (ms)",
+                description = "Read timeout, in milliseconds, for retrieving the JWK set from the IdP's "
+                        + "jwks_uri. Must be between 1 and 60000; an unbounded value is not allowed on the "
+                        + "authentication path. Default is 500.",
+                min = "1",
+                max = "" + MAX_JWK_SET_HTTP_TIMEOUT)
+        int jwkSetHttpReadTimeout() default DEFAULT_JWK_SET_HTTP_READ_TIMEOUT;
     }
 
     @Activate
@@ -230,6 +277,14 @@ public class OidcAuthenticationHandler extends DefaultAuthenticationFeedbackHand
                 : Set.of();
         this.cryptoService = cryptoService;
         this.logoutHandler = logoutHandler;
+        // Validate the JWK set retrieval settings at activation so a bad configuration fails immediately
+        // with a clear message, rather than silently activating and failing on every later OIDC callback.
+        this.jwkSetHttpSizeLimit =
+                requireInRange(config.jwkSetHttpSizeLimit(), "jwkSetHttpSizeLimit", MAX_JWK_SET_HTTP_SIZE_LIMIT);
+        this.jwkSetHttpConnectTimeout =
+                requireInRange(config.jwkSetHttpConnectTimeout(), "jwkSetHttpConnectTimeout", MAX_JWK_SET_HTTP_TIMEOUT);
+        this.jwkSetHttpReadTimeout =
+                requireInRange(config.jwkSetHttpReadTimeout(), "jwkSetHttpReadTimeout", MAX_JWK_SET_HTTP_TIMEOUT);
 
         // Security validation: enforce allowed hosts when SP-initiated single logout is enabled
         if (this.enableSPInitiatedSingleLogout && this.logoutRedirectAllowedHosts.isEmpty()) {
@@ -249,6 +304,27 @@ public class OidcAuthenticationHandler extends DefaultAuthenticationFeedbackHand
         logger.info("OidcAuthenticationHandler successfully activated");
     }
 
+    /**
+     * Validates that a JWK set retrieval setting is a positive value within the allowed range. A value of 0
+     * (which Nimbus interprets as "unlimited"/"no timeout") or any negative value is rejected, as an unbounded
+     * network fetch on the authentication path is a denial-of-service risk.
+     *
+     * @param value the configured value
+     * @param name  the configuration property name, used in the error message
+     * @param max   the inclusive upper bound
+     * @return the validated value
+     * @throws IllegalArgumentException if the value is outside {@code [1, max]}
+     */
+    private static int requireInRange(int value, @NotNull String name, int max) {
+        if (value < 1 || value > max) {
+            throw new IllegalArgumentException(String.format(
+                    "SECURITY: '%s' must be between 1 and %d but was %d. An unbounded (0 or negative) value is "
+                            + "not allowed for JWK set retrieval on the authentication path.",
+                    name, max, value));
+        }
+        return value;
+    }
+
     @Override
     public AuthenticationInfo extractCredentials(
             @NotNull HttpServletRequest request, @NotNull HttpServletResponse response) {
@@ -511,7 +587,7 @@ public class OidcAuthenticationHandler extends DefaultAuthenticationFeedbackHand
      * @param conn         The resolved OIDC connection.
      * @return The validated ID token claims set.
      */
-    private static @NotNull IDTokenClaimsSet validateIdToken(
+    private @NotNull IDTokenClaimsSet validateIdToken(
             @NotNull TokenResponse tokenResponse, @NotNull ResolvedOidcConnection conn, Nonce nonce) {
         Issuer issuer = new Issuer(conn.issuer());
         ClientID clientID = new ClientID(conn.clientId());
@@ -519,7 +595,10 @@ public class OidcAuthenticationHandler extends DefaultAuthenticationFeedbackHand
             JWSAlgorithm jwsAlg = JWSAlgorithm.RS256; // TODO: Read from config
             URL jwkSetURL = conn.jwkSetURL().toURL();
 
-            IDTokenValidator validator = new IDTokenValidator(issuer, clientID, jwsAlg, jwkSetURL);
+            // Use a resource retriever with configurable HTTP limits so large JWK sets can be fetched.
+            DefaultResourceRetriever resourceRetriever =
+                    new DefaultResourceRetriever(jwkSetHttpConnectTimeout, jwkSetHttpReadTimeout, jwkSetHttpSizeLimit);
+            IDTokenValidator validator = new IDTokenValidator(issuer, clientID, jwsAlg, jwkSetURL, resourceRetriever);
             return validator.validate(
                     tokenResponse.toSuccessResponse().getTokens().toOIDCTokens().getIDToken(), nonce);
         } catch (BadJOSEException | JOSEException | MalformedURLException e) {
diff --git a/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java b/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java
index ea99ac0..f07d87d 100644
--- a/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java
+++ b/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java
@@ -463,6 +463,71 @@ class OidcAuthenticationHandlerTest {
                 "testUser", ((OidcAuthCredentials) authInfo.get("user.jcr.credentials")).getAttribute("profile/name"));
     }
 
+    @Test
+    void extractCredentials_WithJwkSetHttpSizeLimitTooSmall_FailsToRetrieveJwkSet() throws JOSEException {
+        // A very small JWK set size limit must cause the JWK set retrieval to fail during token validation.
+        config = createConfig(Map.of("userInfoEnabled", true, "jwkSetHttpSizeLimit", 10));
+
+        RSAKey rsaJWK = new RSAKeyGenerator(2048).keyID("123").generate();
+        String idToken = createIdToken(rsaJWK, "client-id", ISSUER);
+        Cookie[] cookies = createMockCookies();
+        String baseUrl = "http://localhost:" + idpServer.getAddress().getPort();
+        RuntimeException exception = assertThrows(
+                RuntimeException.class,
+                () -> extractCredentials_WithMatchingState_WithValidConnection_WithIdToken(
+                        idToken, rsaJWK, baseUrl, cookies, false, true));
+        assertTrue(
+                exception.getMessage().contains("Exceeded configured input limit"),
+                "Expected an input-limit-exceeded error but got: " + exception.getMessage());
+    }
+
+    @Test
+    void extractCredentials_WithJwkSetHttpSizeLimitRaised_ValidatesToken() throws JOSEException {
+        // A raised JWK set size limit must still allow the JWK set to be retrieved and the token validated.
+        config = createConfig(Map.of("userInfoEnabled", true, "jwkSetHttpSizeLimit", 102400));
+
+        RSAKey rsaJWK = new RSAKeyGenerator(2048).keyID("123").generate();
+        AuthenticationInfo authInfo = extractCredentials_WithMatchingState_WithValidConnection_WithIdToken(
+                createIdToken(rsaJWK, "client-id", ISSUER),
+                rsaJWK,
+                "http://localhost:" + idpServer.getAddress().getPort(),
+                createMockCookies(),
+                false,
+                true);
+        assertEquals("1234567890", authInfo.get("user.name"));
+    }
+
+    @Test
+    void activation_WithZeroJwkSetHttpSizeLimit_FailsFast() {
+        // 0 means "unlimited" to Nimbus and must be rejected at activation, not silently accepted.
+        config = createConfig(Map.of("jwkSetHttpSizeLimit", 0));
+        IllegalArgumentException exception =
+                assertThrows(IllegalArgumentException.class, this::createOidcAuthenticationHandler);
+        assertTrue(
+                exception.getMessage().contains("jwkSetHttpSizeLimit"),
+                "Expected a validation error naming the property but got: " + exception.getMessage());
+    }
+
+    @Test
+    void activation_WithNegativeJwkSetHttpConnectTimeout_FailsFast() {
+        config = createConfig(Map.of("jwkSetHttpConnectTimeout", -1));
+        IllegalArgumentException exception =
+                assertThrows(IllegalArgumentException.class, this::createOidcAuthenticationHandler);
+        assertTrue(
+                exception.getMessage().contains("jwkSetHttpConnectTimeout"),
+                "Expected a validation error naming the property but got: " + exception.getMessage());
+    }
+
+    @Test
+    void activation_WithJwkSetHttpReadTimeoutAboveMax_FailsFast() {
+        config = createConfig(Map.of("jwkSetHttpReadTimeout", 60_001));
+        IllegalArgumentException exception =
+                assertThrows(IllegalArgumentException.class, this::createOidcAuthenticationHandler);
+        assertTrue(
+                exception.getMessage().contains("jwkSetHttpReadTimeout"),
+                "Expected a validation error naming the property but got: " + exception.getMessage());
+    }
+
     @Test
     void extractCredentials_WithMatchingState_WithValidConnection_WithValidIdToken_WithMissingUserInfo()
             throws JOSEException {