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

Escape action in the form_post.jwt and only decode path in RedirectUtils (#93) (#25995)

Browse source 
Closes #90

Signed-off-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
Ricardo Martin 2024-01-09 08:20:14 +01:00 committed by GitHub
parent 92d6da437b
commit 097d68c86b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 88 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
common/src/main/java/org/keycloak/common/util/HttpPostRedirect.java
View file

@ -72,9 +72,12 @@ public class HttpPostRedirect {
builder.append("</HEAD>")
.append("<BODY Onload=\"document.forms[0].submit()\">")
.append("<FORM METHOD=\"POST\" ACTION=\"").append(actionUrl).append("\">");
.append("<FORM METHOD=\"POST\" ACTION=\"")
.append(HtmlUtils.escapeAttribute(actionUrl))
.append("\">");
for (Map.Entry<String, String> param : params.entrySet()) {
builder.append("<INPUT TYPE=\"HIDDEN\" NAME=\"").append(param.getKey()).append("\"").append(" VALUE=\"").append(param.getValue()).append("\"/>");
builder.append("<INPUT TYPE=\"HIDDEN\" NAME=\"").append(param.getKey()).append("\"").append(" VALUE=\"")
.append(HtmlUtils.escapeAttribute(param.getValue())).append("\"/>");
}

4
services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java
View file

@ -263,7 +263,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("\">");
builder.append(" <INPUT TYPE=\"HIDDEN\" NAME=\"response\" VALUE=\"")
.append(HtmlUtils.escapeAttribute(session.tokens().encodeAndEncrypt(responseJWT)))

30
services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java
View file

@ -195,7 +195,7 @@ public class RedirectUtils {
return redirectUri;
}
// Decode redirectUri. We don't decode query and fragment as those can be encoded in the original URL.
// Decode redirectUri. Only path is decoded as other elements can be encoded in the original URL or cannot be encoded at all.
// URL can be decoded multiple times (in case it was encoded multiple times, or some of it's parts were encoded multiple times)
private static String decodeRedirectUri(String redirectUri) {
if (redirectUri == null) return null;
@ -203,28 +203,20 @@ public class RedirectUtils {
try {
KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(redirectUri, false).preserveDefaultPort();
String origQuery = uriBuilder.getQuery();
String origFragment = uriBuilder.getFragment();
String origUserInfo = uriBuilder.getUserInfo();
String encodedRedirectUri = uriBuilder
.replaceQuery(null)
.fragment(null)
.userInfo(null)
.buildAsString();
String decodedRedirectUri = null;
if (uriBuilder.getPath() == null) {
return redirectUri;
}
String encodedPath = uriBuilder.getPath();
String decodedPath;
for (int i = 0; i < MAX_DECODING_COUNT; i++) {
decodedRedirectUri = Encode.decode(encodedRedirectUri);
if (decodedRedirectUri.equals(encodedRedirectUri)) {
// URL is decoded. We can return it (after attach original query and fragment)
return KeycloakUriBuilder.fromUri(decodedRedirectUri, false).preserveDefaultPort()
.replaceQuery(origQuery)
.fragment(origFragment)
.userInfo(origUserInfo)
.buildAsString();
decodedPath = Encode.decode(encodedPath);
if (decodedPath.equals(encodedPath)) {
// URL path is decoded. We can return it in the original redirect URI
return uriBuilder.replacePath(decodedPath, false).buildAsString();
} else {
// Next attempt
encodedRedirectUri = decodedRedirectUri;
encodedPath = decodedPath;
}
}
} catch (IllegalArgumentException iae) {

24
services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java
View file

@ -194,4 +194,28 @@ public class RedirectUtilsTest {
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://test@other.com", set, false));
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://test.com@other.com", set, false));
}
@Test
public void testEncodedRedirectUri() {
Set<String> 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));
}
}

49
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java
View file

@ -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=&gt;"); // set HTML entity &gt;
final String redirectUri = oauth.getRedirectUri() + "?p=&gt;"; // set HTML entity &gt;
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 &gt;
MatcherAssert.assertThat("Redirect page was not encoded", oauth.getDriver().getCurrentUrl(), Matchers.endsWith("/test?p=&gt;"));
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=&gt;"; // set HTML entity &gt;
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 &gt;
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();
}
}