Make SAML relayState max length configurable (#6381)

Motivation:
The default `SsoHandler` currently omits `relayState` when its length exceeds 80 characters. Although the SAML spec defines an 80-character limit, many implementations do not strictly enforce it, and this restriction is often too tight in practice.

References:
- https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf
- https://help.salesforce.com/s/articleView?id=000387925&type=1

Modifications:
- Introduced `SamlServiceProviderBuilder.relayStateMaxLength()` to configure the maximum length.

Result:
- You can override the default 80-character restriction for SAML relayState.

Author: minwoox

saml/src/main/java/com/linecorp/armeria/server/saml/HttpRedirectBindingUtil.java

@@ -106,13 +106,6 @@ static String toRedirectionUrl(SAMLObject msg,
         params.add(messageParamName, toDeflatedBase64(msg));
 
         if (relayState != null) {
-            // RelayState data MAY be included with a SAML protocol message transmitted with this binding.
-            // The value MUST NOT exceed 80 bytes in length and SHOULD be integrity protected by the entity
-            // creating the message independent of any other protections that may or may not exist
-            // during message transmission.
-            if (relayState.length() > 80) {
-                throw new IllegalArgumentException("too long relayState string: " + relayState.length());
-            }
             params.add(RELAY_STATE, relayState);
         }
 

saml/src/main/java/com/linecorp/armeria/server/saml/SamlServiceProviderBuilder.java

@@ -16,6 +16,8 @@
 package com.linecorp.armeria.server.saml;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.linecorp.armeria.server.saml.HttpRedirectBindingUtil.responseWithLocation;
@@ -68,8 +70,11 @@
  * A builder which builds a {@link SamlServiceProvider}.
  */
 public final class SamlServiceProviderBuilder {
+
     private static final Logger logger = LoggerFactory.getLogger(SamlServiceProviderBuilder.class);
 
+    private static final int DEFAULT_MIN_RELAY_STATE_MAX_LENGTH = 80;
+
     private final List<SamlIdentityProviderConfigBuilder> idpConfigBuilders = new ArrayList<>();
     private final List<SamlAssertionConsumerConfigBuilder> acsConfigBuilders = new ArrayList<>();
 
@@ -101,35 +106,11 @@ public final class SamlServiceProviderBuilder {
 
     private boolean signatureRequired = true;
 
-    private SamlSingleSignOnHandler ssoHandler = new SamlSingleSignOnHandler() {
-        @Override
-        public CompletionStage<Void> beforeInitiatingSso(ServiceRequestContext ctx, HttpRequest req,
-                                                         MessageContext<AuthnRequest> message,
-                                                         SamlIdentityProviderConfig idpConfig) {
-            final String requestedPath = req.path();
-            if (requestedPath.length() <= 80) {
-                // Relay the requested path by default.
-                final SAMLBindingContext sub = message.getSubcontext(SAMLBindingContext.class, true);
-                assert sub != null : "SAMLBindingContext";
-                sub.setRelayState(requestedPath);
-            }
-            return UnmodifiableFuture.completedFuture(null);
-        }
-
-        @Override
-        public HttpResponse loginSucceeded(ServiceRequestContext ctx, AggregatedHttpRequest req,
-                                           MessageContext<Response> message, @Nullable String sessionIndex,
-                                           @Nullable String relayState) {
-            return responseWithLocation(firstNonNull(relayState, "/"));
-        }
+    @Nullable
+    private Integer relayStateMaxLength;
 
-        @Override
-        public HttpResponse loginFailed(ServiceRequestContext ctx, AggregatedHttpRequest req,
-                                        @Nullable MessageContext<Response> message, Throwable cause) {
-            logger.warn("{} SAML SSO failed", ctx, cause);
-            return responseWithLocation("/error");
-        }
-    };
+    @Nullable
+    private SamlSingleSignOnHandler ssoHandler;
 
     private SamlSingleLogoutHandler sloHandler = new SamlSingleLogoutHandler() {
         @Override
@@ -263,13 +244,33 @@ public SamlServiceProviderBuilder requestIdManager(SamlRequestIdManager requestI
     }
 
     /**
-     * Sets a {@link SamlSingleSignOnHandler} which handles SAML messages for a single sign-on.
+     * Sets a {@link SamlSingleSignOnHandler} which handles SAML messages for a single sign-on. If this is set,
+     * {@link #relayStateMaxLength(int)} will be ignored so that the
+     * {@link SamlSingleSignOnHandler#beforeInitiatingSso(ServiceRequestContext, HttpRequest,
+     * MessageContext, SamlIdentityProviderConfig)}
+     * is responsible for handling the {@code RelayState} parameter.
      */
     public SamlServiceProviderBuilder ssoHandler(SamlSingleSignOnHandler ssoHandler) {
+        checkState(relayStateMaxLength == null,
+                   "relayStateMaxLength() and ssoHandler() are mutually exclusive.");
         this.ssoHandler = requireNonNull(ssoHandler, "ssoHandler");
         return this;
     }
 
+    /**
+     * Sets the maximum length of the {@code RelayState} parameter which is sent to an identity provider
+     * and is returned with the {@code SAMLResponse} parameter. If the length of the {@code RelayState}
+     * exceeds the specified value, the {@code RelayState} parameter will be ignored.
+     * The value must be equal to or greater than {@value #DEFAULT_MIN_RELAY_STATE_MAX_LENGTH}.
+     */
+    public SamlServiceProviderBuilder relayStateMaxLength(int maxLength) {
+        checkState(ssoHandler == null, "relayStateMaxLength() and ssoHandler() are mutually exclusive.");
+        checkArgument(maxLength >= DEFAULT_MIN_RELAY_STATE_MAX_LENGTH,
+                      "maxLength: %s (expected: >= %s)", maxLength, DEFAULT_MIN_RELAY_STATE_MAX_LENGTH);
+        relayStateMaxLength = maxLength;
+        return this;
+    }
+
     /**
      * Sets a {@link SamlSingleLogoutHandler} which handles SAML messages for a single sign-on.
      */
@@ -447,6 +448,15 @@ public SamlServiceProvider build() {
                                             e);
         }
 
+        final SamlSingleSignOnHandler ssoHandler;
+        if (this.ssoHandler != null) {
+            ssoHandler = this.ssoHandler;
+        } else {
+            final int relayStateMaxLength = firstNonNull(this.relayStateMaxLength,
+                                                         DEFAULT_MIN_RELAY_STATE_MAX_LENGTH);
+            ssoHandler = newDefaultSsoHandler(relayStateMaxLength);
+        }
+
         return new SamlServiceProvider(authorizer,
                                        entityId,
                                        hostname,
@@ -487,6 +497,41 @@ private static void validateSignatureAlgorithm(String signatureAlgorithm, Creden
         }
     }
 
+    private static SamlSingleSignOnHandler newDefaultSsoHandler(int relayStateMaxLength) {
+        return new SamlSingleSignOnHandler() {
+            @Override
+            public CompletionStage<Void> beforeInitiatingSso(ServiceRequestContext ctx, HttpRequest req,
+                                                             MessageContext<AuthnRequest> message,
+                                                             SamlIdentityProviderConfig idpConfig) {
+                final String requestedPath = req.path();
+                if (requestedPath.length() <= relayStateMaxLength) {
+                    // Relay the requested path by default.
+                    final SAMLBindingContext sub = message.getSubcontext(SAMLBindingContext.class, true);
+                    assert sub != null : "SAMLBindingContext";
+                    sub.setRelayState(requestedPath);
+                } else {
+                    logger.debug("requested path length ({}) exceeds the configured maximum length ({}).",
+                                 requestedPath.length(), relayStateMaxLength);
+                }
+                return UnmodifiableFuture.completedFuture(null);
+            }
+
+            @Override
+            public HttpResponse loginSucceeded(ServiceRequestContext ctx, AggregatedHttpRequest req,
+                                               MessageContext<Response> message, @Nullable String sessionIndex,
+                                               @Nullable String relayState) {
+                return responseWithLocation(firstNonNull(relayState, "/"));
+            }
+
+            @Override
+            public HttpResponse loginFailed(ServiceRequestContext ctx, AggregatedHttpRequest req,
+                                            @Nullable MessageContext<Response> message, Throwable cause) {
+                logger.warn("{} SAML SSO failed", ctx, cause);
+                return responseWithLocation("/error");
+            }
+        };
+    }
+
     /**
      * An adapter for {@link CredentialResolver} which helps to resolve a {@link Credential} from
      * the specified {@code keyName}.

saml/src/test/java/com/linecorp/armeria/server/saml/SamlServiceProviderTest.java

@@ -89,6 +89,7 @@
 
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 
 import com.linecorp.armeria.client.WebClient;
@@ -276,8 +277,11 @@ static class CookieBasedSsoHandler implements SamlSingleSignOnHandler {
         public CompletionStage<Void> beforeInitiatingSso(ServiceRequestContext ctx, HttpRequest req,
                                                          MessageContext<AuthnRequest> message,
                                                          SamlIdentityProviderConfig idpConfig) {
-            message.getSubcontext(SAMLBindingContext.class, true)
-                   .setRelayState(req.path());
+            final SAMLBindingContext subcontext = message.getSubcontext(SAMLBindingContext.class, true);
+            assert subcontext != null;
+            if (req.path().length() <= 80) {
+                subcontext.setRelayState(req.path());
+            }
             return UnmodifiableFuture.completedFuture(null);
         }
 
@@ -343,6 +347,21 @@ void shouldRespondAuthnRequest_HttpRedirect() throws Exception {
                               .get(SIGNATURE_ALGORITHM)).isEqualTo(signatureAlgorithm);
     }
 
+    @Test
+    void relayStateExceeding80BytesShouldNotBeSent() throws Exception {
+        final String meaninglessQuery = Strings.repeat("12345678910", 10);
+
+        final AggregatedHttpResponse resp = client.get("/redirect?a=" + meaninglessQuery).aggregate().join();
+        assertThat(resp.status()).isEqualTo(HttpStatus.FOUND);
+
+        final String location = resp.headers().get(HttpHeaderNames.LOCATION);
+        final Pattern p = Pattern.compile(
+                "http://idp\\.example\\.com/saml/sso/redirect\\?" +
+                "SAMLRequest=([^&]+)&SigAlg=([^&]+)&Signature=(.+)$");
+        assertThat(location).isNotNull();
+        assertThat(p.matcher(location).matches()).isTrue();
+    }
+
     @Test
     void shouldRespondAuthnRequest_HttpPost() throws Exception {
         final AggregatedHttpResponse resp = client.get("/post").aggregate().join();