address review comments

Author: richard-dennehy

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java

@@ -95,6 +95,8 @@ public class SamlObjectHandler {
         }
     });
 
+    private static final int ISSUER_VALUE_MAX_LENGTH = 512;
+
     protected final Logger logger = LogManager.getLogger(getClass());
 
     @Nullable
@@ -268,13 +270,13 @@ private ElasticsearchSecurityException samlSignatureException(Issuer issuer, Lis
     }
 
     // package private for testing
-    String describeIssuer(@Nullable Issuer issuer) {
+    static String describeIssuer(@Nullable Issuer issuer) {
         if (issuer == null || issuer.getValue() == null) {
             return "";
         }
         final String msg = " The issuer included in the SAML message was [%s]";
-        if (issuer.getValue().length() > 64) {
-            return Strings.format(msg + "...", Strings.cleanTruncate(issuer.getValue(), 64));
+        if (issuer.getValue().length() > ISSUER_VALUE_MAX_LENGTH) {
+            return Strings.format(msg + "...", Strings.cleanTruncate(issuer.getValue(), ISSUER_VALUE_MAX_LENGTH));
         }
         return Strings.format(msg, issuer.getValue());
     }

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java

@@ -1374,29 +1374,25 @@ public void testFailureWhenIdPCredentialsAreNull() throws Exception {
     }
 
     public void testDescribeNullIssuer() {
-        assertThat(authenticator.describeIssuer(null), equalTo(""));
-    }
-
-    public void testDescribeNullIssuerValue() {
-        final Issuer issuer = new IssuerBuilder().buildObject();
-        assertThat(authenticator.describeIssuer(issuer), equalTo(""));
+        final Issuer issuer = randomFrom(new IssuerBuilder().buildObject(), null);
+        assertThat(SamlAuthenticator.describeIssuer(issuer), equalTo(""));
     }
 
     public void testDescribeIssuer() {
         final Issuer issuer = new IssuerBuilder().buildObject();
         issuer.setValue("https://idp.saml.elastic.test/");
         assertThat(
-            authenticator.describeIssuer(issuer),
+            SamlAuthenticator.describeIssuer(issuer),
             equalTo(" The issuer included in the SAML message was [https://idp.saml.elastic.test/]")
         );
     }
 
     public void testDescribeVeryLongIssuer() {
         final Issuer issuer = new IssuerBuilder().buildObject();
-        issuer.setValue("https://idp.saml.elastic.test/" + "a".repeat(128));
+        issuer.setValue("https://idp.saml.elastic.test/" + randomAlphaOfLength(512));
 
-        final String description = authenticator.describeIssuer(issuer);
-        assertThat(description, hasLength(114));
+        final String description = SamlAuthenticator.describeIssuer(issuer);
+        assertThat(description, hasLength(562));
         assertThat(description, endsWith("..."));
     }