Escape action in the form_post response mode (#60)
Closes keycloak/keycloak-private#31 Closes https://issues.redhat.com/browse/RHBK-652 Signed-off-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
parent
ba8c22eaf0
commit
2ba7a51da6
6 changed files with 72 additions and 2 deletions
|
@ -381,7 +381,7 @@ public class BaseSAML2BindingBuilder<T extends BaseSAML2BindingBuilder> {
|
|||
.append("</HEAD>")
|
||||
.append("<BODY Onload=\"document.forms[0].submit()\">")
|
||||
|
||||
.append("<FORM METHOD=\"POST\" ACTION=\"").append(actionUrl).append("\">");
|
||||
.append("<FORM METHOD=\"POST\" ACTION=\"").append(escapeAttribute(actionUrl)).append("\">");
|
||||
|
||||
builder.append("<p>Redirecting, please wait.</p>");
|
||||
|
||||
|
|
|
@ -160,7 +160,9 @@ public abstract class OIDCRedirectUriBuilder {
|
|||
builder.append(" </HEAD>");
|
||||
builder.append(" <BODY Onload=\"document.forms[0].submit()\">");
|
||||
|
||||
builder.append(" <FORM METHOD=\"POST\" ACTION=\"" + redirectUri.toString() + "\">");
|
||||
builder.append(" <FORM METHOD=\"POST\" ACTION=\"")
|
||||
.append(HtmlUtils.escapeAttribute(redirectUri.toString()))
|
||||
.append("\">");
|
||||
|
||||
for (Map.Entry<String, String> param : params.entrySet()) {
|
||||
builder.append(" <INPUT TYPE=\"HIDDEN\" NAME=\"")
|
||||
|
|
|
@ -1398,6 +1398,9 @@ public class OAuthClient {
|
|||
int index = driver.getCurrentUrl().indexOf('?');
|
||||
if (index == -1) {
|
||||
index = driver.getCurrentUrl().indexOf('#');
|
||||
if (index == -1) {
|
||||
index = driver.getCurrentUrl().length();
|
||||
}
|
||||
}
|
||||
return driver.getCurrentUrl().substring(0, index);
|
||||
}
|
||||
|
|
|
@ -639,6 +639,20 @@ public class SamlClient {
|
|||
return SAMLRequestParser.parseResponsePostBinding(respElement.val());
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the form element from a Post binding.
|
||||
*
|
||||
* @param responsePage HTML code in the page
|
||||
* @return The element that is the form
|
||||
*/
|
||||
public static Element extractFormFromPostResponse(String responsePage) {
|
||||
org.jsoup.nodes.Document theResponsePage = Jsoup.parse(responsePage);
|
||||
Elements form = theResponsePage.select("form");
|
||||
assertThat("Checking uniqueness of SAMLResponse/SAMLRequest form in Post binding", form.size(), is(1));
|
||||
|
||||
return form.first();
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts and parses value of RelayState input field of a form present in the given page.
|
||||
*
|
||||
|
|
|
@ -16,6 +16,8 @@
|
|||
*/
|
||||
package org.keycloak.testsuite.oauth;
|
||||
|
||||
import org.hamcrest.MatcherAssert;
|
||||
import org.hamcrest.Matchers;
|
||||
import org.jboss.arquillian.graphene.page.Page;
|
||||
import org.junit.Assert;
|
||||
import org.junit.Before;
|
||||
|
@ -25,6 +27,7 @@ import org.keycloak.OAuth2Constants;
|
|||
import org.keycloak.OAuthErrorException;
|
||||
import org.keycloak.events.Details;
|
||||
import org.keycloak.events.Errors;
|
||||
import org.keycloak.events.EventType;
|
||||
import org.keycloak.models.Constants;
|
||||
import org.keycloak.protocol.oidc.utils.OIDCResponseMode;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
|
@ -35,6 +38,7 @@ import org.keycloak.testsuite.pages.InstalledAppRedirectPage;
|
|||
import org.keycloak.testsuite.updaters.ClientAttributeUpdater;
|
||||
import org.keycloak.testsuite.util.ClientManager;
|
||||
import org.keycloak.testsuite.util.OAuthClient;
|
||||
import org.keycloak.testsuite.util.WaitUtils;
|
||||
import org.openqa.selenium.By;
|
||||
|
||||
import jakarta.ws.rs.core.UriBuilder;
|
||||
|
@ -249,6 +253,28 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void authorizationRequestFormPostResponseModeHTMLEntitiesRedirectUri() throws IOException {
|
||||
try (var c = ClientAttributeUpdater.forClient(adminClient, "test", "test-app")
|
||||
.setRedirectUris(Collections.singletonList("*"))
|
||||
.update()) {
|
||||
oauth.responseMode(OIDCResponseMode.FORM_POST.value());
|
||||
oauth.responseType(OAuth2Constants.CODE);
|
||||
oauth.redirectUri("/test?p=>"); // set HTML entity >
|
||||
oauth.doLogin("test-user@localhost", "password");
|
||||
|
||||
WaitUtils.waitForPageToLoad();
|
||||
// if not properly encoded %3E would be received instead of >
|
||||
MatcherAssert.assertThat("Redirect page was not encoded", oauth.getDriver().getCurrentUrl(), Matchers.endsWith("/test?p=>"));
|
||||
|
||||
events.expect(EventType.LOGIN)
|
||||
.user(AssertEvents.isUUID())
|
||||
.session(AssertEvents.isUUID())
|
||||
.detail(Details.USERNAME, "test-user@localhost")
|
||||
.assertEvent();
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void authorizationRequestFormPostResponseModeWithCustomState() throws IOException {
|
||||
oauth.responseMode(OIDCResponseMode.FORM_POST.value());
|
||||
|
|
|
@ -50,6 +50,7 @@ import static org.hamcrest.Matchers.containsString;
|
|||
import static org.hamcrest.Matchers.is;
|
||||
import static org.hamcrest.Matchers.nullValue;
|
||||
import static org.hamcrest.Matchers.notNullValue;
|
||||
import static org.hamcrest.Matchers.endsWith;
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.hamcrest.Matchers.matchesRegex;
|
||||
import static org.keycloak.saml.common.constants.JBossSAMLURIConstants.NAMEID_FORMAT_TRANSIENT;
|
||||
|
@ -345,4 +346,28 @@ public class BasicSamlTest extends AbstractSamlTest {
|
|||
assertThat(page, containsString("Invalid redirect uri"));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testConsumerServiceURLHtmlEntities() throws IOException {
|
||||
try (var c = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_SALES_POST)
|
||||
.setRedirectUris(Collections.singletonList("*"))
|
||||
.update()) {
|
||||
|
||||
String action = new SamlClientBuilder()
|
||||
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, "javascript:alert('xss');", Binding.POST)
|
||||
.build()
|
||||
.login().user(bburkeUser).build()
|
||||
.executeAndTransform(response -> {
|
||||
assertThat(response, statusCodeIsHC(Response.Status.OK));
|
||||
String responsePage = EntityUtils.toString(response.getEntity(), "UTF-8");
|
||||
return SamlClient.extractFormFromPostResponse(responsePage)
|
||||
.attributes().asList().stream()
|
||||
.filter(a -> "action".equalsIgnoreCase(a.getKey()))
|
||||
.map(org.jsoup.nodes.Attribute::getValue)
|
||||
.findAny().orElse(null);
|
||||
});
|
||||
// if not encoded properly jsoup returns ":" instead of ":"
|
||||
assertThat(action, endsWith("javascript:alert('xss');"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue