keycloak-aplcache

KEYCLOAK-3509

12/6/2016 9:52:37 PM

Details

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 e8f5344..9f12420 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 b979063..467f7d1 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<String, String> 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 0a4803c..8d52282 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 25e046f..404d24d 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;
@@ -99,6 +101,11 @@ public class AdapterTest {
     }
 
     @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 cd15bdd..1e45612 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 77e85e4..fef4c21 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("<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();
 
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 68070ea..4876b3b 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
@@ -101,6 +101,12 @@ public class Jetty9Test {
     }
 
     @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 00603de..bd53150 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
@@ -96,6 +96,12 @@ public class Jetty9Test {
     }
 
     @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 00603de..bd53150 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
@@ -96,6 +96,12 @@ public class Jetty9Test {
     }
 
     @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 2dca1cb..b72aa68 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 eb97549..608ab1e 100755
--- a/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/Tomcat7Test.java
+++ b/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/Tomcat7Test.java
@@ -84,6 +84,12 @@ public class Tomcat7Test {
     }
 
     @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 11e798a..bd801d1 100755
--- a/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatTest.java
+++ b/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatTest.java
@@ -84,6 +84,12 @@ public class TomcatTest {
     }
 
     @Test
+    public void testLoginEncodedRedirectUri() throws Exception {
+        testStrategy.testLoginEncodedRedirectUri();
+    }
+
+
+    @Test
     public void testSavedPostRequest() throws Exception {
         testStrategy.testSavedPostRequest();
     }