diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java index e8f534492a..9f12420683 100755 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java @@ -25,6 +25,7 @@ import org.keycloak.adapters.spi.AuthChallenge; import org.keycloak.adapters.spi.AuthOutcome; import org.keycloak.adapters.spi.HttpFacade; import org.keycloak.common.VerificationException; +import org.keycloak.common.util.Encode; import org.keycloak.common.util.KeycloakUriBuilder; import org.keycloak.common.util.UriUtils; import org.keycloak.constants.AdapterConstants; @@ -169,7 +170,7 @@ public class OAuthRequestAuthenticator { KeycloakUriBuilder redirectUriBuilder = deployment.getAuthUrl().clone() .queryParam(OAuth2Constants.RESPONSE_TYPE, OAuth2Constants.CODE) .queryParam(OAuth2Constants.CLIENT_ID, deployment.getResourceName()) - .queryParam(OAuth2Constants.REDIRECT_URI, url) + .queryParam(OAuth2Constants.REDIRECT_URI, Encode.encodeQueryParamAsIs(url)) // Need to encode uri ourselves as queryParam() will not encode % characters. .queryParam(OAuth2Constants.STATE, state) .queryParam("login", "true"); if(loginHint != null && loginHint.length() > 0){ @@ -203,7 +204,7 @@ public class OAuthRequestAuthenticator { protected AuthChallenge loginRedirect() { final String state = getStateCode(); - final String redirect = getRedirectUri(state); + final String redirect = getRedirectUri(state); if (redirect == null) { return challenge(403, OIDCAuthenticationError.Reason.NO_REDIRECT_URI, null); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java index b979063d81..467f7d1c58 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java @@ -89,7 +89,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { @GET public Response build() { MultivaluedMap params = uriInfo.getQueryParameters(); - + String requestUri = uriInfo.getRequestUri().toString(); String clientId = params.getFirst(OIDCLoginProtocol.CLIENT_ID_PARAM); checkSsl(); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index 0a4803c92b..8d5228203d 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java @@ -245,7 +245,8 @@ public class TokenEndpoint { event.session(userSession.getId()); String redirectUri = clientSession.getNote(OIDCLoginProtocol.REDIRECT_URI_PARAM); - if (redirectUri != null && !redirectUri.equals(formParams.getFirst(OAuth2Constants.REDIRECT_URI))) { + String formParam = formParams.getFirst(OAuth2Constants.REDIRECT_URI); + if (redirectUri != null && !redirectUri.equals(formParam)) { event.error(Errors.INVALID_CODE); throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Incorrect redirect_uri", Response.Status.BAD_REQUEST); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTest.java index 25e046f1ea..404d24dd67 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTest.java @@ -19,6 +19,8 @@ package org.keycloak.testsuite.adapter; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; +import org.keycloak.common.util.Encode; +import org.keycloak.common.util.KeycloakUriBuilder; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.services.managers.RealmManager; @@ -98,6 +100,11 @@ public class AdapterTest { testStrategy.testLoginSSOAndLogout(); } + @Test + public void testLoginEncodedRedirectUri() throws Exception { + testStrategy.testLoginEncodedRedirectUri(); + } + @Test public void testSavedPostRequest() throws Exception { testStrategy.testSavedPostRequest(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java index cd15bdde5e..1e456128ab 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java @@ -216,6 +216,36 @@ public class AdapterTestStrategy extends ExternalResource { Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); } + /** + * KEYCLOAK-3509 + * + * @throws Exception + */ + public void testLoginEncodedRedirectUri() throws Exception { + // test login to customer-portal which does a bearer request to customer-db + driver.navigate().to(APP_SERVER_BASE_URL + "/product-portal?encodeTest=a%3Cb"); + System.out.println("Current url: " + driver.getCurrentUrl()); + Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); + loginPage.login("bburke@redhat.com", "password"); + System.out.println("Current url: " + driver.getCurrentUrl()); + Assert.assertEquals(driver.getCurrentUrl(), APP_SERVER_BASE_URL + "/product-portal?encodeTest=a%3Cb"); + String pageSource = driver.getPageSource(); + System.out.println(pageSource); + Assert.assertTrue(pageSource.contains("iPhone")); + Assert.assertTrue(pageSource.contains("uriEncodeTest=true")); + + // test logout + String logoutUri = OIDCLoginProtocolService.logoutUrl(UriBuilder.fromUri(AUTH_SERVER_URL)) + .queryParam(OAuth2Constants.REDIRECT_URI, APP_SERVER_BASE_URL + "/product-portal").build("demo").toString(); + driver.navigate().to(logoutUri); + Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); + driver.navigate().to(APP_SERVER_BASE_URL + "/product-portal"); + Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); + driver.navigate().to(APP_SERVER_BASE_URL + "/customer-portal"); + Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); + } + + public void testServletRequestLogout() throws Exception { // test login to customer-portal which does a bearer request to customer-db driver.navigate().to(APP_SERVER_BASE_URL + "/customer-portal"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/ProductServlet.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/ProductServlet.java index 77e85e4ba0..fef4c21369 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/ProductServlet.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/ProductServlet.java @@ -38,6 +38,9 @@ public class ProductServlet extends HttpServlet { pw.printf("%s", "Product Portal"); pw.println("iPhone"); pw.println("iPad"); + String x = req.getParameter("encodeTest"); + String encodeTest= Boolean.toString("a"); pw.flush(); diff --git a/testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/Jetty9Test.java b/testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/Jetty9Test.java index 68070ea4a7..4876b3bee2 100755 --- a/testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/Jetty9Test.java +++ b/testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/Jetty9Test.java @@ -100,6 +100,12 @@ public class Jetty9Test { testStrategy.testLoginSSOAndLogout(); } + @Test + public void testLoginEncodedRedirectUri() throws Exception { + testStrategy.testLoginEncodedRedirectUri(); + } + + @Test public void testServletRequestLogout() throws Exception { testStrategy.testServletRequestLogout(); diff --git a/testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/Jetty9Test.java b/testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/Jetty9Test.java index 00603de4e5..bd53150b1d 100755 --- a/testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/Jetty9Test.java +++ b/testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/Jetty9Test.java @@ -95,6 +95,12 @@ public class Jetty9Test { testStrategy.testLoginSSOAndLogout(); } + @Test + public void testLoginEncodedRedirectUri() throws Exception { + testStrategy.testLoginEncodedRedirectUri(); + } + + @Test public void testSavedPostRequest() throws Exception { testStrategy.testSavedPostRequest(); diff --git a/testsuite/jetty/jetty93/src/test/java/org/keycloak/testsuite/Jetty9Test.java b/testsuite/jetty/jetty93/src/test/java/org/keycloak/testsuite/Jetty9Test.java index 00603de4e5..bd53150b1d 100644 --- a/testsuite/jetty/jetty93/src/test/java/org/keycloak/testsuite/Jetty9Test.java +++ b/testsuite/jetty/jetty93/src/test/java/org/keycloak/testsuite/Jetty9Test.java @@ -95,6 +95,12 @@ public class Jetty9Test { testStrategy.testLoginSSOAndLogout(); } + @Test + public void testLoginEncodedRedirectUri() throws Exception { + testStrategy.testLoginEncodedRedirectUri(); + } + + @Test public void testSavedPostRequest() throws Exception { testStrategy.testSavedPostRequest(); diff --git a/testsuite/tomcat6/src/test/java/org/keycloak/testsuite/TomcatTest.java b/testsuite/tomcat6/src/test/java/org/keycloak/testsuite/TomcatTest.java index 2dca1cb1fd..b72aa68eb6 100755 --- a/testsuite/tomcat6/src/test/java/org/keycloak/testsuite/TomcatTest.java +++ b/testsuite/tomcat6/src/test/java/org/keycloak/testsuite/TomcatTest.java @@ -79,6 +79,10 @@ public class TomcatTest { public void testLoginSSOAndLogout() throws Exception { testStrategy.testLoginSSOAndLogout(); } + @Test + public void testLoginEncodedRedirectUri() throws Exception { + testStrategy.testLoginEncodedRedirectUri(); + } @Test public void testSavedPostRequest() throws Exception { diff --git a/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/Tomcat7Test.java b/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/Tomcat7Test.java index eb97549da7..608ab1ed86 100755 --- a/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/Tomcat7Test.java +++ b/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/Tomcat7Test.java @@ -83,6 +83,12 @@ public class Tomcat7Test { testStrategy.testLoginSSOAndLogout(); } + @Test + public void testLoginEncodedRedirectUri() throws Exception { + testStrategy.testLoginEncodedRedirectUri(); + } + + @Test public void testSavedPostRequest() throws Exception { testStrategy.testSavedPostRequest(); diff --git a/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatTest.java b/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatTest.java index 11e798a74b..bd801d15e4 100755 --- a/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatTest.java +++ b/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatTest.java @@ -83,6 +83,12 @@ public class TomcatTest { testStrategy.testLoginSSOAndLogout(); } + @Test + public void testLoginEncodedRedirectUri() throws Exception { + testStrategy.testLoginEncodedRedirectUri(); + } + + @Test public void testSavedPostRequest() throws Exception { testStrategy.testSavedPostRequest();