Merge pull request #3620 from patriot1burke/master

KEYCLOAK-3509

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){

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<String, String> params = uriInfo.getQueryParameters();
-
+        String requestUri = uriInfo.getRequestUri().toString();
         String clientId = params.getFirst(OIDCLoginProtocol.CLIENT_ID_PARAM);
 
         checkSsl();

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);
         }

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();

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");

testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/ProductServlet.java

@@ -38,6 +38,9 @@ public class ProductServlet extends HttpServlet {
         pw.printf("<html><head><title>%s</title></head><body>", "Product Portal");
         pw.println("iPhone");
         pw.println("iPad");
+        String x = req.getParameter("encodeTest");
+        String encodeTest= Boolean.toString("a<b".equals(x));
+        pw.println("uriEncodeTest=" + encodeTest);
         pw.print("</body></html>");
         pw.flush();
 

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();

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();

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();

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 {

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();

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();