diff --git a/common/src/main/java/org/keycloak/common/util/HttpPostRedirect.java b/common/src/main/java/org/keycloak/common/util/HttpPostRedirect.java index c931ac469d..134fd3c57f 100644 --- a/common/src/main/java/org/keycloak/common/util/HttpPostRedirect.java +++ b/common/src/main/java/org/keycloak/common/util/HttpPostRedirect.java @@ -72,9 +72,12 @@ public class HttpPostRedirect { builder.append("") .append("") - .append("
"); + .append(""); for (Map.Entry param : params.entrySet()) { - builder.append(""); + builder.append(""); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java index e09e4ba789..87bf9f3dcb 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java @@ -263,7 +263,9 @@ public abstract class OIDCRedirectUriBuilder { builder.append(" "); builder.append(" "); - builder.append(" "); + builder.append(" "); builder.append(" set = Stream.of( + "https://keycloak.org/test/*" + ).collect(Collectors.toSet()); + + Assert.assertEquals("https://keycloak.org/test/index.html", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/index.html", set, false)); + Assert.assertEquals("https://keycloak.org/test?encodeTest=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test?encodeTest=a%3Cb", set, false)); + Assert.assertEquals("https://keycloak.org/test?encodeTest=a%3Cb#encode2=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test?encodeTest=a%3Cb#encode2=a%3Cb", set, false)); + Assert.assertEquals("https://keycloak.org/test/#encode2=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/#encode2=a%3Cb", set, false)); + + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/../", set, false)); // direct + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2E%2E/", set, false)); // encoded + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%2F%2E%2E%2F", set, false)); // encoded + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/", set, false)); // double-encoded + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/?some_query_param=some_value", set, false)); // double-encoded + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/#encodeTest=a%3Cb", set, false)); // double-encoded + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%25252E%25252E/", set, false)); // triple-encoded + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2525252525252E%2525252525252E/", set, false)); // seventh-encoded + + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak%2Eorg/test/", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org%2Ftest%2F%40sample.com", set, false)); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java index eb5efbb8b0..af51bc31ee 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java @@ -16,8 +16,6 @@ */ 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; @@ -29,7 +27,10 @@ import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventType; import org.keycloak.models.Constants; +import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.utils.OIDCResponseMode; +import org.keycloak.representations.AuthorizationResponseToken; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AssertEvents; @@ -260,17 +261,57 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest { .update()) { oauth.responseMode(OIDCResponseMode.FORM_POST.value()); oauth.responseType(OAuth2Constants.CODE); - oauth.redirectUri("/test?p=>"); // set HTML entity > + final String redirectUri = oauth.getRedirectUri() + "?p=>"; // set HTML entity > + oauth.redirectUri(redirectUri); + oauth.stateParamHardcoded(KeycloakModelUtils.generateId()); 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=>")); + Assert.assertEquals("Redirect page was not encoded", redirectUri, oauth.getDriver().getCurrentUrl()); + String state = driver.findElement(By.id("state")).getText(); + Assert.assertEquals(oauth.getState(), state); + Assert.assertNotNull(driver.findElement(By.id("code")).getText()); events.expect(EventType.LOGIN) .user(AssertEvents.isUUID()) .session(AssertEvents.isUUID()) .detail(Details.USERNAME, "test-user@localhost") + .detail(OIDCLoginProtocol.RESPONSE_MODE_PARAM, OIDCResponseMode.FORM_POST.name().toLowerCase()) + .detail(OAuth2Constants.REDIRECT_URI, redirectUri) + .assertEvent(); + } + } + + @Test + public void authorizationRequestFormPostJwtResponseModeHTMLEntitiesRedirectUri() throws IOException { + try (var c = ClientAttributeUpdater.forClient(adminClient, "test", "test-app") + .setRedirectUris(Collections.singletonList("*")) + .update()) { + oauth.responseMode(OIDCResponseMode.FORM_POST_JWT.value()); + oauth.responseType(OAuth2Constants.CODE); + final String redirectUri = oauth.getRedirectUri() + "?p=>"; // set HTML entity > + oauth.redirectUri(redirectUri); + oauth.stateParamHardcoded(KeycloakModelUtils.generateId()); + oauth.doLogin("test-user@localhost", "password"); + + WaitUtils.waitForPageToLoad(); + // if not properly encoded %3E would be received instead of > + Assert.assertEquals("Redirect page was not encoded", redirectUri, oauth.getDriver().getCurrentUrl()); + String responseTokenEncoded = driver.findElement(By.id("response")).getText(); + AuthorizationResponseToken responseToken = oauth.verifyAuthorizationResponseToken(responseTokenEncoded); + assertEquals("test-app", responseToken.getAudience()[0]); + Assert.assertNotNull(responseToken.getOtherClaims().get("code")); + Assert.assertNull(responseToken.getOtherClaims().get("error")); + Assert.assertEquals(oauth.getState(), responseToken.getOtherClaims().get("state")); + Assert.assertNotNull(responseToken.getOtherClaims().get("code")); + + events.expect(EventType.LOGIN) + .user(AssertEvents.isUUID()) + .session((String) responseToken.getOtherClaims().get("session_state")) + .detail(Details.USERNAME, "test-user@localhost") + .detail(OIDCLoginProtocol.RESPONSE_MODE_PARAM, OIDCResponseMode.FORM_POST_JWT.name().toLowerCase()) + .detail(OAuth2Constants.REDIRECT_URI, redirectUri) .assertEvent(); } }