libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

[KEYCLOAK-5912] Add better improper SAML assertion error handling

Browse source
This commit is contained in:
James Stapleton 2017-11-26 10:58:22 -05:00 committed by Hynek Mlnařík
parent 916ba573bc
commit 92cce7a6d4
2 changed files with 63 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/SAMLAssertionParser.java
View file

@ -24,6 +24,8 @@ import org.keycloak.dom.saml.v2.assertion.EncryptedAssertionType;
import org.keycloak.dom.saml.v2.assertion.NameIDType; import org.keycloak.dom.saml.v2.assertion.NameIDType;
import org.keycloak.dom.saml.v2.assertion.SubjectType; import org.keycloak.dom.saml.v2.assertion.SubjectType;
import org.keycloak.saml.common.ErrorCodes; import org.keycloak.saml.common.ErrorCodes;
import org.keycloak.saml.common.PicketLinkLogger;
import org.keycloak.saml.common.PicketLinkLoggerFactory;
import org.keycloak.saml.common.constants.JBossSAMLConstants; import org.keycloak.saml.common.constants.JBossSAMLConstants;
import org.keycloak.saml.common.constants.JBossSAMLURIConstants; import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
import org.keycloak.saml.common.exceptions.ConfigurationException; import org.keycloak.saml.common.exceptions.ConfigurationException;
@ -52,6 +54,7 @@ import javax.xml.stream.events.XMLEvent;
* @since Oct 12, 2010 * @since Oct 12, 2010
*/ */
public class SAMLAssertionParser implements ParserNamespaceSupport { public class SAMLAssertionParser implements ParserNamespaceSupport {
private static final PicketLinkLogger logger = PicketLinkLoggerFactory.getLogger();
private final String ASSERTION = JBossSAMLConstants.ASSERTION.get(); private final String ASSERTION = JBossSAMLConstants.ASSERTION.get();
@ -157,15 +160,22 @@ public class SAMLAssertionParser implements ParserNamespaceSupport {
} }
private AssertionType parseBaseAttributes(StartElement nextElement) throws ParsingException { private AssertionType parseBaseAttributes(StartElement nextElement) throws ParsingException {
Attribute idAttribute = nextElement.getAttributeByName(new QName(JBossSAMLConstants.ID.get())); String id = StaxParserUtil.getAttributeValue(nextElement, JBossSAMLConstants.ID.get());
String id = StaxParserUtil.getAttributeValue(idAttribute); if (id == null) {
throw logger.parserRequiredAttribute(JBossSAMLConstants.ID.get());
}
Attribute versionAttribute = nextElement.getAttributeByName(new QName(JBossSAMLConstants.VERSION.get())); String version = StaxParserUtil.getAttributeValue(nextElement, JBossSAMLConstants.VERSION.get());
String version = StaxParserUtil.getAttributeValue(versionAttribute); if (!JBossSAMLConstants.VERSION_2_0.get().equals(version)) {
StringUtil.match(JBossSAMLConstants.VERSION_2_0.get(), version); throw logger.parserException(new RuntimeException(
String.format("Assertion %s required to be \"%s\"", JBossSAMLConstants.VERSION.get(), JBossSAMLConstants.VERSION_2_0.get())));
}
Attribute issueInstantAttribute = nextElement.getAttributeByName(new QName(JBossSAMLConstants.ISSUE_INSTANT.get())); String issueInstantString = StaxParserUtil.getAttributeValue(nextElement, JBossSAMLConstants.ISSUE_INSTANT.get());
XMLGregorianCalendar issueInstant = XMLTimeUtil.parse(StaxParserUtil.getAttributeValue(issueInstantAttribute)); if (issueInstantString == null) {
throw logger.parserRequiredAttribute(JBossSAMLConstants.ISSUE_INSTANT.get());
}
XMLGregorianCalendar issueInstant = XMLTimeUtil.parse(issueInstantString);
return new AssertionType(id, issueInstant); return new AssertionType(id, issueInstant);
} }

51
saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java
View file

@ -16,25 +16,28 @@
*/ */
package org.keycloak.saml.processing.core.parsers.saml; package org.keycloak.saml.processing.core.parsers.saml;
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import java.io.ByteArrayInputStream;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.security.PrivateKey; import java.security.PrivateKey;
import org.junit.Before; import org.junit.Before;
import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.hamcrest.CustomMatcher;
import org.keycloak.common.util.Base64; import org.keycloak.common.util.Base64;
import org.keycloak.common.util.DerUtils; import org.keycloak.common.util.DerUtils;
import org.keycloak.common.util.StreamUtil;
import org.keycloak.dom.saml.v2.assertion.AssertionType; import org.keycloak.dom.saml.v2.assertion.AssertionType;
import org.keycloak.dom.saml.v2.assertion.AttributeStatementType; import org.keycloak.dom.saml.v2.assertion.AttributeStatementType;
import org.keycloak.dom.saml.v2.assertion.AttributeType; import org.keycloak.dom.saml.v2.assertion.AttributeType;
@ -67,6 +70,9 @@ public class SAMLParserTest {
private SAMLParser parser; private SAMLParser parser;
@Rule
public ExpectedException thrown = ExpectedException.none();
@Before @Before
public void initParser() { public void initParser() {
this.parser = new SAMLParser(); this.parser = new SAMLParser();
@ -326,5 +332,40 @@ public class SAMLParserTest {
} }
} }
@Test
public void testSaml20AssertionsMissingId() throws IOException, ParsingException {
try (InputStream st = removeAttribute("saml20-assertion-example.xml", "ID")) {
thrown.expect(ParsingException.class);
thrown.expectMessage(endsWith("Required attribute missing: ID"));
parser.parse(st);
}
}
@Test
public void testSaml20AssertionsMissingVersion() throws IOException, ParsingException {
try (InputStream st = removeAttribute("saml20-assertion-example.xml", "Version")) {
thrown.expect(ParsingException.class);
thrown.expectMessage(endsWith("Assertion Version required to be \"2.0\""));
parser.parse(st);
}
}
@Test
public void testSaml20AssertionsMissingIssueInstance() throws IOException, ParsingException {
try (InputStream st = removeAttribute("saml20-assertion-example.xml", "IssueInstant")) {
thrown.expect(ParsingException.class);
thrown.expectMessage(endsWith("Required attribute missing: IssueInstant"));
parser.parse(st);
}
}
private InputStream removeAttribute(String resourceName, String attribute) throws IOException {
try (InputStream st = SAMLParserTest.class.getResourceAsStream(resourceName)) {
String str = StreamUtil.readString(st, StandardCharsets.UTF_8);
String processed = str.replaceAll(attribute + "=\"[^\"]+\"", "");
return new ByteArrayInputStream(processed.getBytes());
}
}
} }