Browse Source

Add brackets where necessary in error messages (#48140)

This commit attempts to help error readability by adding brackets
where applicable/missing in saml errors.
Ioannis Kakavas 6 years ago
parent
commit
f0e3711ffb

+ 8 - 8
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java

@@ -94,9 +94,9 @@ class SamlAuthenticator extends SamlRequestHandler {
         }
 
         if (Strings.hasText(response.getInResponseTo()) && allowedSamlRequestIds.contains(response.getInResponseTo()) == false) {
-            logger.debug("The SAML Response with ID {} is unsolicited. A user might have used a stale URL or the Identity Provider " +
+            logger.debug("The SAML Response with ID [{}] is unsolicited. A user might have used a stale URL or the Identity Provider " +
                     "incorrectly populates the InResponseTo attribute", response.getID());
-            throw samlException("SAML content is in-response-to {} but expected one of {} ",
+            throw samlException("SAML content is in-response-to [{}] but expected one of {} ",
                     response.getInResponseTo(), allowedSamlRequestIds);
         }
 
@@ -126,8 +126,8 @@ class SamlAuthenticator extends SamlRequestHandler {
             logger.trace(sb.toString());
         }
         if (attributes.isEmpty() && nameId == null) {
-            logger.debug("The Attribute Statements of SAML Response with ID {} contained no attributes and the SAML Assertion Subject did" +
-                    "not contain a SAML NameID. Please verify that the Identity Provider configuration with regards to attribute " +
+            logger.debug("The Attribute Statements of SAML Response with ID [{}] contained no attributes and the SAML Assertion Subject " +
+                "did not contain a SAML NameID. Please verify that the Identity Provider configuration with regards to attribute " +
                     "release is correct. ", response.getID());
             throw samlException("Could not process any SAML attributes in {}", response.getElementQName());
         }
@@ -262,7 +262,7 @@ class SamlAuthenticator extends SamlRequestHandler {
 
     private void checkAuthnStatement(List<AuthnStatement> authnStatements) {
         if (authnStatements.size() != 1) {
-            throw samlException("SAML Assertion subject contains {} Authn Statements while exactly one was expected.",
+            throw samlException("SAML Assertion subject contains [{}] Authn Statements while exactly one was expected.",
                 authnStatements.size());
         }
         final AuthnStatement authnStatement = authnStatements.get(0);
@@ -322,7 +322,7 @@ class SamlAuthenticator extends SamlRequestHandler {
                 .filter(data -> data.getMethod().equals(METHOD_BEARER))
                 .map(SubjectConfirmation::getSubjectConfirmationData).filter(Objects::nonNull).collect(Collectors.toList());
         if (confirmationData.size() != 1) {
-            throw samlException("SAML Assertion subject contains {} bearer SubjectConfirmation, while exactly one was expected.",
+            throw samlException("SAML Assertion subject contains [{}] bearer SubjectConfirmation, while exactly one was expected.",
                     confirmationData.size());
         }
         if (logger.isTraceEnabled()) {
@@ -338,7 +338,7 @@ class SamlAuthenticator extends SamlRequestHandler {
     private void checkRecipient(SubjectConfirmationData subjectConfirmationData) {
         final SpConfiguration sp = getSpConfiguration();
         if (sp.getAscUrl().equals(subjectConfirmationData.getRecipient()) == false) {
-            throw samlException("SAML Assertion SubjectConfirmationData Recipient {} does not match expected value {}",
+            throw samlException("SAML Assertion SubjectConfirmationData Recipient [{}] does not match expected value [{}]",
                     subjectConfirmationData.getRecipient(), sp.getAscUrl());
         }
     }
@@ -347,7 +347,7 @@ class SamlAuthenticator extends SamlRequestHandler {
         // Allow for IdP initiated SSO where InResponseTo MUST be missing
         if (Strings.hasText(subjectConfirmationData.getInResponseTo())
                 && allowedSamlRequestIds.contains(subjectConfirmationData.getInResponseTo()) == false) {
-            throw samlException("SAML Assertion SubjectConfirmationData is in-response-to {} but expected one of {} ",
+            throw samlException("SAML Assertion SubjectConfirmationData is in-response-to [{}] but expected one of [{}]",
                     subjectConfirmationData.getInResponseTo(), allowedSamlRequestIds);
         }
     }

+ 1 - 1
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java

@@ -729,7 +729,7 @@ public final class SamlRealm extends Realm implements Releasable {
             try {
                 onChange.run();
             } catch (Exception e) {
-                logger.warn(new ParameterizedMessage("An error occurred while reloading file {}", file), e);
+                logger.warn(new ParameterizedMessage("An error occurred while reloading file [{}]", file), e);
             }
         }
     }

+ 3 - 3
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRequestHandler.java

@@ -218,11 +218,11 @@ public class SamlRequestHandler {
 
     protected void checkIssuer(Issuer issuer, XMLObject parent) {
         if (issuer == null) {
-            throw samlException("Element {} ({}) has no issuer, but expected {}",
+            throw samlException("Element {} ({}) has no issuer, but expected [{}]",
                     parent.getElementQName(), text(parent, 16), idp.getEntityId());
         }
         if (idp.getEntityId().equals(issuer.getValue()) == false) {
-            throw samlException("SAML Issuer {} does not match expected value {}", issuer.getValue(), idp.getEntityId());
+            throw samlException("SAML Issuer [{}] does not match expected value [{}]", issuer.getValue(), idp.getEntityId());
         }
     }
 
@@ -259,7 +259,7 @@ public class SamlRequestHandler {
             Object[] args = new Object[] { element.getTagName(), type.getName(), object == null ? "<null>" : object.getClass().getName() };
             throw samlException("SAML object [{}] is incorrect type. Expected [{}] but was [{}]", args);
         } catch (UnmarshallingException e) {
-            throw samlException("Failed to unmarshall SAML content [{}", e, element.getTagName());
+            throw samlException("Failed to unmarshall SAML content [{}]", e, element.getTagName());
         }
     }
 

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

@@ -854,7 +854,7 @@ public class SamlAuthenticatorTests extends SamlTestCase {
                 "</proto:Response>";
         SamlToken token = token(signDoc(xml));
         final ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token));
-        assertThat(exception.getMessage(), containsString("SAML Assertion subject contains 0 bearer SubjectConfirmation"));
+        assertThat(exception.getMessage(), containsString("SAML Assertion subject contains [0] bearer SubjectConfirmation"));
         assertThat(exception.getCause(), nullValue());
         assertThat(SamlUtils.isSamlException(exception), is(true));
     }