External-Mirrors/keycloak
2
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-01-09 23:12:06 -03:30
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
Closes #90

Signed-off-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
Ricardo Martin 2024-01-04 13:46:43 +01:00 committed by GitHub
parent c46920bfdd
commit 4188bc33ae
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>") builder.append("</HEAD>")
.append("<BODY Onload=\"document.forms[0].submit()\">") .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()) { 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(" </HEAD>");
builder.append(" <BODY Onload=\"document.forms[0].submit()\">"); 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=\"") builder.append(" <INPUT TYPE=\"HIDDEN\" NAME=\"response\" VALUE=\"")
.append(HtmlUtils.escapeAttribute(session.tokens().encodeAndEncrypt(responseJWT))) .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; 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) // 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) { private static String decodeRedirectUri(String redirectUri) {
if (redirectUri == null) return null; if (redirectUri == null) return null;
@ -203,28 +203,20 @@ public class RedirectUtils {
try { try {
KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(redirectUri, false).preserveDefaultPort(); KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(redirectUri, false).preserveDefaultPort();
String origQuery = uriBuilder.getQuery(); if (uriBuilder.getPath() == null) {
String origFragment = uriBuilder.getFragment(); return redirectUri;
String origUserInfo = uriBuilder.getUserInfo(); }
String encodedRedirectUri = uriBuilder String encodedPath = uriBuilder.getPath();
.replaceQuery(null) String decodedPath;
.fragment(null)
.userInfo(null)
.buildAsString();
String decodedRedirectUri = null;
for (int i = 0; i < MAX_DECODING_COUNT; i++) { for (int i = 0; i < MAX_DECODING_COUNT; i++) {
decodedRedirectUri = Encode.decode(encodedRedirectUri); decodedPath = Encode.decode(encodedPath);
if (decodedRedirectUri.equals(encodedRedirectUri)) { if (decodedPath.equals(encodedPath)) {
// URL is decoded. We can return it (after attach original query and fragment) // URL path is decoded. We can return it in the original redirect URI
return KeycloakUriBuilder.fromUri(decodedRedirectUri, false).preserveDefaultPort() return uriBuilder.replacePath(decodedPath, false).buildAsString();
.replaceQuery(origQuery)
.fragment(origFragment)
.userInfo(origUserInfo)
.buildAsString();
} else { } else {
// Next attempt // Next attempt
encodedRedirectUri = decodedRedirectUri; encodedPath = decodedPath;
} }
} }
} catch (IllegalArgumentException iae) { } 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@other.com", set, false));
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://test.com@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; package org.keycloak.testsuite.oauth;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.jboss.arquillian.graphene.page.Page; import org.jboss.arquillian.graphene.page.Page;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
@ -29,7 +27,10 @@ import org.keycloak.events.Details;
import org.keycloak.events.Errors; import org.keycloak.events.Errors;
import org.keycloak.events.EventType; import org.keycloak.events.EventType;
import org.keycloak.models.Constants; 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.protocol.oidc.utils.OIDCResponseMode;
import org.keycloak.representations.AuthorizationResponseToken;
import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AbstractKeycloakTest;
import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.AssertEvents;
@ -260,17 +261,57 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest {
.update()) { .update()) {
oauth.responseMode(OIDCResponseMode.FORM_POST.value()); oauth.responseMode(OIDCResponseMode.FORM_POST.value());
oauth.responseType(OAuth2Constants.CODE); 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"); oauth.doLogin("test-user@localhost", "password");
WaitUtils.waitForPageToLoad(); WaitUtils.waitForPageToLoad();
// if not properly encoded %3E would be received instead of &gt; // 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) events.expect(EventType.LOGIN)
.user(AssertEvents.isUUID()) .user(AssertEvents.isUUID())
.session(AssertEvents.isUUID()) .session(AssertEvents.isUUID())
.detail(Details.USERNAME, "test-user@localhost") .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(); .assertEvent();
} }
} }