KEYCLOAK-190 KEYCLOAK-191 Fixed redirect uri's

services/src/main/java/org/keycloak/services/resources/TokenService.java

@@ -183,7 +183,7 @@ public class TokenService {
     @POST
     @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
     public Response processLogin(@QueryParam("client_id") final String clientId, @QueryParam("scope") final String scopeParam,
-            @QueryParam("state") final String state, @QueryParam("redirect_uri") final String redirect,
+            @QueryParam("state") final String state, @QueryParam("redirect_uri") String redirect,
             final MultivaluedMap<String, String> formData) {
         logger.debug("TokenService.processLogin");
         OAuthFlows oauth = Flows.oauth(realm, request, uriInfo, authManager, tokenManager);
@@ -199,6 +199,11 @@ public class TokenService {
             return oauth.forwardToSecurityFailure("Login requester not enabled.");
         }
 
+        redirect = verifyRedirectUri(redirect, client);
+        if (redirect == null) {
+            return oauth.forwardToSecurityFailure("Invalid redirect_uri.");
+        }
+
         if (formData.containsKey("cancel")) {
             return oauth.redirectError(client, "access_denied", state, redirect);
         }
@@ -290,6 +295,11 @@ public class TokenService {
             return oauth.forwardToSecurityFailure("Login requester not enabled.");
         }
 
+        redirect = verifyRedirectUri(redirect, client);
+        if (redirect == null) {
+            return oauth.forwardToSecurityFailure("Invalid redirect_uri.");
+        }
+
         if (!realm.isRegistrationAllowed()) {
             logger.warn("Registration not allowed");
             return oauth.forwardToSecurityFailure("Registration not allowed");
@@ -472,7 +482,7 @@ public class TokenService {
     @Path("login")
     @GET
     public Response loginPage(final @QueryParam("response_type") String responseType,
-            final @QueryParam("redirect_uri") String redirect, final @QueryParam("client_id") String clientId,
+            @QueryParam("redirect_uri") String redirect, final @QueryParam("client_id") String clientId,
             final @QueryParam("scope") String scopeParam, final @QueryParam("state") String state, final @QueryParam("prompt") String prompt) {
         logger.info("TokenService.loginPage");
         OAuthFlows oauth = Flows.oauth(realm, request, uriInfo, authManager, tokenManager);
@@ -497,6 +507,11 @@ public class TokenService {
             session.close();
             return null;
         }
+        redirect = verifyRedirectUri(redirect, client);
+        if (redirect == null) {
+            return oauth.forwardToSecurityFailure("Invalid redirect_uri.");
+        }
+
         logger.info("Checking roles...");
         RoleModel resourceRole = realm.getRole(Constants.APPLICATION_ROLE);
         RoleModel identityRequestRole = realm.getRole(Constants.IDENTITY_REQUESTER_ROLE);
@@ -525,7 +540,7 @@ public class TokenService {
     @Path("registrations")
     @GET
     public Response registerPage(final @QueryParam("response_type") String responseType,
-            final @QueryParam("redirect_uri") String redirect, final @QueryParam("client_id") String clientId,
+            @QueryParam("redirect_uri") String redirect, final @QueryParam("client_id") String clientId,
             final @QueryParam("scope") String scopeParam, final @QueryParam("state") String state) {
         logger.info("**********registerPage()");
         OAuthFlows oauth = Flows.oauth(realm, request, uriInfo, authManager, tokenManager);
@@ -545,6 +560,11 @@ public class TokenService {
             return oauth.forwardToSecurityFailure("Login requester not enabled.");
         }
 
+        redirect = verifyRedirectUri(redirect, client);
+        if (redirect == null) {
+            return oauth.forwardToSecurityFailure("Invalid redirect_uri.");
+        }
+
         if (!realm.isRegistrationAllowed()) {
             logger.warn("Registration not allowed");
             return oauth.forwardToSecurityFailure("Registration not allowed");
@@ -613,4 +633,15 @@ public class TokenService {
         return location.build();
     }
 
+    protected String verifyRedirectUri(String redirectUri, UserModel client) {
+        if (redirectUri == null) {
+            return client.getRedirectUris().size() == 1 ? client.getRedirectUris().iterator().next() : null;
+        } else if (client.getRedirectUris().isEmpty()) {
+            return redirectUri;
+        } else {
+            String r = redirectUri.indexOf('?') != -1 ? redirectUri.substring(0, redirectUri.indexOf('?')) : redirectUri;
+            return client.getRedirectUris().contains(r) ? redirectUri : null;
+        }
+    }
+
 }

services/src/main/java/org/keycloak/services/resources/flows/OAuthFlows.java

@@ -67,12 +67,6 @@ public class OAuthFlows {
     }
 
     public Response redirectAccessCode(AccessCodeEntry accessCode, String state, String redirect) {
-        UserModel client = realm.getUser(accessCode.getClient().getLoginName());
-        Set<String> redirectUris = client.getRedirectUris();
-        if (!redirectUris.isEmpty() && !redirectUris.contains(redirect)) {
-            return forwardToSecurityFailure("Invalid redirect_uri " + redirect);
-        }
-
         String code = accessCode.getCode();
         UriBuilder redirectUri = UriBuilder.fromUri(redirect).queryParam("code", code);
         log.debug("redirectAccessCode: state: {0}", state);
@@ -86,11 +80,6 @@ public class OAuthFlows {
     }
 
     public Response redirectError(UserModel client, String error, String state, String redirect) {
-        Set<String> redirectUris = client.getRedirectUris();
-        if (!redirectUris.isEmpty() && !redirectUris.contains(redirect)) {
-            return forwardToSecurityFailure("Invalid redirect_uri " + redirect);
-        }
-
         UriBuilder redirectUri = UriBuilder.fromUri(redirect).queryParam("error", error);
         if (state != null) {
             redirectUri.queryParam("state", state);

testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java

@@ -97,31 +97,6 @@ public class AuthorizationCodeTest {
         Assert.assertNotNull(response.getCode());
     }
 
-    @Test
-    public void authorizationRequestInvalidRedirectUri() throws IOException {
-        keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
-            @Override
-            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
-                for (ApplicationModel app : appRealm.getApplications()) {
-                    if (app.getName().equals("test-app")) {
-                        UserModel client = app.getApplicationUser();
-                        client.addRedirectUri(oauth.getRedirectUri());
-                    }
-                }
-            }
-        });
-
-        oauth.redirectUri("http://invalid");
-        oauth.state("mystate");
-
-        AuthorizationCodeResponse response = oauth.doLogin("test-user@localhost", "password");
-
-        Assert.assertFalse(response.isRedirected());
-
-        Assert.assertTrue(errorPage.isCurrent());
-        Assert.assertEquals("Invalid redirect_uri http://invalid", errorPage.getError());
-    }
-
     @Test
     public void authorizationRequestNoState() throws IOException {
         AuthorizationCodeResponse response = oauth.doLogin("test-user@localhost", "password");

testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java

@@ -0,0 +1,174 @@
+/*
+ * JBoss, Home of Professional Open Source.
+ * Copyright 2012, Red Hat, Inc., and individual contributors
+ * as indicated by the @author tags. See the copyright.txt file in the
+ * distribution for a full listing of individual contributors.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package org.keycloak.testsuite.oauth;
+
+import org.junit.Assert;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.keycloak.models.ApplicationModel;
+import org.keycloak.models.RealmModel;
+import org.keycloak.representations.SkeletonKeyToken;
+import org.keycloak.services.managers.RealmManager;
+import org.keycloak.testsuite.OAuthClient;
+import org.keycloak.testsuite.pages.ErrorPage;
+import org.keycloak.testsuite.pages.LoginPage;
+import org.keycloak.testsuite.pages.OAuthGrantPage;
+import org.keycloak.testsuite.rule.KeycloakRule;
+import org.keycloak.testsuite.rule.WebResource;
+import org.keycloak.testsuite.rule.WebRule;
+import org.openqa.selenium.WebDriver;
+
+import java.io.IOException;
+import java.util.Map;
+
+/**
+ * @author <a href="mailto:vrockai@redhat.com">Viliam Rockai</a>
+ */
+public class OAuthRedirectUriTest {
+
+    @ClassRule
+    public static KeycloakRule keycloakRule = new KeycloakRule(new KeycloakRule.KeycloakSetup() {
+        @Override
+        public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+            ApplicationModel app = appRealm.getApplicationNameMap().get("test-app");
+            app.getApplicationUser().addRedirectUri("http://localhost:8081/app");
+        }
+    });
+
+    @Rule
+    public WebRule webRule = new WebRule(this);
+
+    @WebResource
+    protected WebDriver driver;
+
+    @WebResource
+    protected OAuthClient oauth;
+
+    @WebResource
+    protected LoginPage loginPage;
+
+    @WebResource
+    protected ErrorPage errorPage;
+
+    @Test
+    public void testNoParam() throws IOException {
+        oauth.redirectUri(null);
+        OAuthClient.AuthorizationCodeResponse response = oauth.doLogin("test-user@localhost", "password");
+
+        Assert.assertNotNull(response.getCode());
+        Assert.assertEquals(oauth.getCurrentRequest(), "http://localhost:8081/app");
+    }
+
+    @Test
+    public void testNoParamMultipleValidUris() throws IOException {
+        keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                appRealm.getApplicationNameMap().get("test-app").getApplicationUser().addRedirectUri("http://localhost:8081/app2");
+            }
+        });
+
+        try {
+            oauth.redirectUri(null);
+            oauth.openLoginForm();
+
+            Assert.assertTrue(errorPage.isCurrent());
+            Assert.assertEquals("Invalid redirect_uri.", errorPage.getError());
+        } finally {
+            keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
+                @Override
+                public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                    appRealm.getApplicationNameMap().get("test-app").getApplicationUser().removeRedirectUri("http://localhost:8081/app2");
+                }
+            });
+        }
+    }
+
+    @Test
+    public void testNoParamNoValidUris() throws IOException {
+        keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                appRealm.getApplicationNameMap().get("test-app").getApplicationUser().removeRedirectUri("http://localhost:8081/app");
+            }
+        });
+
+        try {
+            oauth.redirectUri(null);
+            oauth.openLoginForm();
+
+            Assert.assertTrue(errorPage.isCurrent());
+            Assert.assertEquals("Invalid redirect_uri.", errorPage.getError());
+        } finally {
+            keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
+                @Override
+                public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                    appRealm.getApplicationNameMap().get("test-app").getApplicationUser().addRedirectUri("http://localhost:8081/app");
+                }
+            });
+        }
+    }
+
+    @Test
+    public void testNoValidUris() throws IOException {
+        keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                appRealm.getApplicationNameMap().get("test-app").getApplicationUser().removeRedirectUri("http://localhost:8081/app");
+            }
+        });
+
+        try {
+            OAuthClient.AuthorizationCodeResponse response = oauth.doLogin("test-user@localhost", "password");
+
+            Assert.assertNotNull(response.getCode());
+            Assert.assertEquals(oauth.getCurrentRequest(), "http://localhost:8081/app/auth");
+        } finally {
+            keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
+                @Override
+                public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                    appRealm.getApplicationNameMap().get("test-app").getApplicationUser().addRedirectUri("http://localhost:8081/app");
+                }
+            });
+        }
+    }
+
+    @Test
+    public void testInvalid() throws IOException {
+        oauth.redirectUri("http://localhost:8081/app2");
+        oauth.openLoginForm();
+
+        Assert.assertTrue(errorPage.isCurrent());
+        Assert.assertEquals("Invalid redirect_uri.", errorPage.getError());
+    }
+
+    @Test
+    public void testWithParams() throws IOException {
+        oauth.redirectUri("http://localhost:8081/app?key=value");
+        OAuthClient.AuthorizationCodeResponse response = oauth.doLogin("test-user@localhost", "password");
+
+        Assert.assertNotNull(response.getCode());
+        Assert.assertTrue(driver.getCurrentUrl().startsWith("http://localhost:8081/app?key=value&code="));
+    }
+
+}