From 67f8622d13bad3e618c1ad0dcbf0adf13c87db9a Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Thu, 18 Jul 2019 23:48:59 +0200 Subject: [PATCH] KEYCLOAK-8318 Workaround Elytron's double encoding of the query parameters Co-Authored-By: mhajas --- .../keycloak/adapters/elytron/ElytronHttpFacade.java | 11 ++++++----- .../adapters/saml/elytron/ElytronHttpFacade.java | 8 ++++++-- .../keycloak/testsuite/auth/page/login/LoginForm.java | 4 ++++ .../adapter/servlet/DemoServletsAdapterTest.java | 8 ++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/adapters/oidc/wildfly-elytron/src/main/java/org/keycloak/adapters/elytron/ElytronHttpFacade.java b/adapters/oidc/wildfly-elytron/src/main/java/org/keycloak/adapters/elytron/ElytronHttpFacade.java index 9004bda266..6f1d19327e 100644 --- a/adapters/oidc/wildfly-elytron/src/main/java/org/keycloak/adapters/elytron/ElytronHttpFacade.java +++ b/adapters/oidc/wildfly-elytron/src/main/java/org/keycloak/adapters/elytron/ElytronHttpFacade.java @@ -19,7 +19,6 @@ package org.keycloak.adapters.elytron; import io.undertow.server.handlers.CookieImpl; -import org.bouncycastle.asn1.cmp.Challenge; import org.keycloak.KeycloakSecurityContext; import org.keycloak.adapters.AdapterDeploymentContext; import org.keycloak.adapters.AdapterTokenStore; @@ -31,10 +30,8 @@ import org.keycloak.adapters.spi.AuthenticationError; import org.keycloak.adapters.spi.LogoutError; import org.keycloak.enums.TokenStore; import org.wildfly.security.auth.server.SecurityIdentity; -import org.wildfly.security.http.HttpAuthenticationException; import org.wildfly.security.http.HttpScope; import org.wildfly.security.http.HttpServerCookie; -import org.wildfly.security.http.HttpServerMechanismsResponder; import org.wildfly.security.http.HttpServerRequest; import org.wildfly.security.http.HttpServerResponse; import org.wildfly.security.http.Scope; @@ -201,9 +198,13 @@ class ElytronHttpFacade implements OIDCHttpFacade { if (query != null) { String[] parameters = query.split("&"); for (String parameter : parameters) { - String[] keyValue = parameter.split("="); + String[] keyValue = parameter.split("=", 2); if (keyValue[0].equals(param)) { - return keyValue[1]; + try { + return URLDecoder.decode(keyValue[1], "UTF-8"); + } catch (IOException e) { + throw new RuntimeException("Failed to decode request URI", e); + } } } } diff --git a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronHttpFacade.java b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronHttpFacade.java index a017bdd519..eb08d29d41 100644 --- a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronHttpFacade.java +++ b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronHttpFacade.java @@ -186,9 +186,13 @@ class ElytronHttpFacade implements HttpFacade { if (query != null) { String[] parameters = query.split("&"); for (String parameter : parameters) { - String[] keyValue = parameter.split("="); + String[] keyValue = parameter.split("=", 2); if (keyValue[0].equals(param)) { - return keyValue[1]; + try { + return URLDecoder.decode(keyValue[1], "UTF-8"); + } catch (IOException e) { + throw new RuntimeException("Failed to decode request URI", e); + } } } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/login/LoginForm.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/login/LoginForm.java index 39eb739d86..c0b30c0d7b 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/login/LoginForm.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/login/LoginForm.java @@ -145,6 +145,10 @@ public class LoginForm extends Form { return accountFields.getUsernameLabel(); } + public String getUsername() { + return accountFields.getUsername(); + } + public String getPasswordLabel() { return passwordFields.getPasswordLabel(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java index 17291a562e..fa707e0fc5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java @@ -1409,4 +1409,12 @@ public class DemoServletsAdapterTest extends AbstractServletsAdapterTest { .clearDetails() .assertEvent(); } + + @Test + public void testLoginHintFromClientRequest() { + driver.navigate().to(customerPortal + "?login_hint=blah%3d"); + waitForPageToLoad(); + assertCurrentUrlStartsWithLoginUrlOf(testRealmPage); + assertThat(testRealmLoginPage.form().getUsername(), is("blah=")); + } }