Merge pull request #407 from patriot1burke/master

check redirect uri exists in token service

docbook/reference/en/en-US/modules/MigrationFromOlderVersions.xml

@@ -1,5 +1,14 @@
 <chapter id="Migration_from_older_versions">
     <title>Migration from older versions</title>
+    <sect1>
+        <title>Migrating from 1.0 Alpha 4 to Beta 1</title>
+        <itemizedlist>
+            <listitem>
+                For all clients except bearer-only applications, you must specify at least one redirect uri.  Keycloak
+                will not allow you to log in unless you have specified a valid redirect uri for that application.
+            </listitem>
+        </itemizedlist>
+    </sect1>
     <sect1>
         <title>Migrating from 1.0 Alpha 2 to Alpha 3</title>
         <itemizedlist>

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

@@ -945,6 +945,7 @@ public class TokenService {
     }
 
     public static boolean matchesRedirects(Set<String> validRedirects, String redirect) {
+        if (Constants.INSTALLED_APP_URN.equals(redirect)) return true;
         for (String validRedirect : validRedirects) {
             if (validRedirect.endsWith("*")) {
                 // strip off *
@@ -963,13 +964,16 @@ public class TokenService {
     public static String verifyRedirectUri(UriInfo uriInfo, String redirectUri, ClientModel client) {
         Set<String> validRedirects = client.getRedirectUris();
         if (redirectUri == null) {
-            return validRedirects.size() == 1 ? validRedirects.iterator().next() : null;
+            if (validRedirects.size() != 1) return null;
-        } else if (validRedirects.isEmpty()) {
+            String validRedirect = validRedirects.iterator().next();
-            if (client.isPublicClient()) {
+            int idx = validRedirect.indexOf("/*");
-                logger.error("Client redirect uri must be registered for public client");
+            if (idx > -1) {
-                return null;
+                validRedirect = validRedirect.substring(0, idx);
             }
-            return redirectUri;
+            return validRedirect;
+        } else if (validRedirects.isEmpty()) {
+            logger.error("Redirect URI is required for client: " + client.getClientId());
+            return null;
         } else {
             String r = redirectUri.indexOf('?') != -1 ? redirectUri.substring(0, redirectUri.indexOf('?')) : redirectUri;
             Set<String> resolveValidRedirects = resolveValidRedirects(uriInfo, validRedirects);

testsuite/integration/src/test/java/org/keycloak/testsuite/account/ProfileTest.java

@@ -68,7 +68,7 @@ public class ProfileTest {
 
             ApplicationModel app = appRealm.getApplicationNameMap().get("test-app");
             appRealm.addScopeMapping(app, accountApp.getRole(AccountRoles.VIEW_PROFILE));
-
+            app.addRedirectUri("http://localhost:8081/app/*");
             app.addWebOrigin("http://localtest.me:8081");
 
             ClientModel thirdParty = appRealm.findClient("third-party");

testsuite/integration/src/test/java/org/keycloak/testsuite/composites/CompositeRoleTest.java

@@ -88,6 +88,7 @@ public class CompositeRoleTest {
             final ApplicationModel realmComposite1Application = new ApplicationManager(manager).createApplication(realm, "REALM_COMPOSITE_1_APPLICATION");
             realmComposite1Application.setEnabled(true);
             realm.addScopeMapping(realmComposite1Application, realmComposite1);
+            realmComposite1Application.addRedirectUri("http://localhost:8081/app/*");
             realmComposite1Application.setBaseUrl("http://localhost:8081/app");
             realmComposite1Application.setManagementUrl("http://localhost:8081/app/logout");
             realmComposite1Application.setSecret("password");
@@ -95,6 +96,7 @@ public class CompositeRoleTest {
             final ApplicationModel realmRole1Application = new ApplicationManager(manager).createApplication(realm, "REALM_ROLE_1_APPLICATION");
             realmRole1Application.setEnabled(true);
             realm.addScopeMapping(realmRole1Application, realmRole1);
+            realmRole1Application.addRedirectUri("http://localhost:8081/app/*");
             realmRole1Application.setBaseUrl("http://localhost:8081/app");
             realmRole1Application.setManagementUrl("http://localhost:8081/app/logout");
             realmRole1Application.setSecret("password");
@@ -102,6 +104,7 @@ public class CompositeRoleTest {
 
             final ApplicationModel appRoleApplication = new ApplicationManager(manager).createApplication(realm, "APP_ROLE_APPLICATION");
             appRoleApplication.setEnabled(true);
+            appRoleApplication.addRedirectUri("http://localhost:8081/app/*");
             appRoleApplication.setBaseUrl("http://localhost:8081/app");
             appRoleApplication.setManagementUrl("http://localhost:8081/app/logout");
             appRoleApplication.setSecret("password");
@@ -123,6 +126,7 @@ public class CompositeRoleTest {
 
             final ApplicationModel appCompositeApplication = new ApplicationManager(manager).createApplication(realm, "APP_COMPOSITE_APPLICATION");
             appCompositeApplication.setEnabled(true);
+            appCompositeApplication.addRedirectUri("http://localhost:8081/app/*");
             appCompositeApplication.setBaseUrl("http://localhost:8081/app");
             appCompositeApplication.setManagementUrl("http://localhost:8081/app/logout");
             appCompositeApplication.setSecret("password");

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

@@ -49,7 +49,6 @@ public class OAuthRedirectUriTest {
         @Override
         public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
             ApplicationModel app = appRealm.getApplicationNameMap().get("test-app");
-            app.addRedirectUri("http://localhost:8081/app");
 
             ApplicationModel installedApp = appRealm.addApplication("test-installed");
             installedApp.setEnabled(true);
@@ -119,7 +118,7 @@ public class OAuthRedirectUriTest {
         keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
             @Override
             public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
-                appRealm.getApplicationNameMap().get("test-app").removeRedirectUri("http://localhost:8081/app");
+                appRealm.getApplicationNameMap().get("test-app").removeRedirectUri("http://localhost:8081/app/*");
             }
         });
 
@@ -133,7 +132,7 @@ public class OAuthRedirectUriTest {
             keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
                 @Override
                 public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
-                    appRealm.getApplicationNameMap().get("test-app").addRedirectUri("http://localhost:8081/app");
+                    appRealm.getApplicationNameMap().get("test-app").addRedirectUri("http://localhost:8081/app/*");
                 }
             });
         }
@@ -144,20 +143,21 @@ public class OAuthRedirectUriTest {
         keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
             @Override
             public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
-                appRealm.getApplicationNameMap().get("test-app").removeRedirectUri("http://localhost:8081/app");
+                appRealm.getApplicationNameMap().get("test-app").removeRedirectUri("http://localhost:8081/app/*");
             }
         });
 
         try {
-            OAuthClient.AuthorizationCodeResponse response = oauth.doLogin("test-user@localhost", "password");
+            oauth.redirectUri(null);
+            oauth.openLoginForm();
 
-            Assert.assertNotNull(response.getCode());
+            Assert.assertTrue(errorPage.isCurrent());
-            Assert.assertEquals(oauth.getCurrentRequest(), "http://localhost:8081/app/auth");
+            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").addRedirectUri("http://localhost:8081/app");
+                    appRealm.getApplicationNameMap().get("test-app").addRedirectUri("http://localhost:8081/app/*");
                 }
             });
         }

testsuite/integration/src/test/resources/testcomposite.json

@@ -102,6 +102,9 @@
             "enabled": true,
             "baseUrl": "http://localhost:8081/app",
             "adminUrl": "http://localhost:8081/app/logout",
+            "redirectUris": [
+                "http://localhost:8081/app/*"
+            ],
             "secret": "password"
          },
         {
@@ -109,6 +112,9 @@
             "enabled": true,
             "baseUrl": "http://localhost:8081/app",
             "adminUrl": "http://localhost:8081/app/logout",
+            "redirectUris": [
+                "http://localhost:8081/app/*"
+            ],
             "secret": "password"
         },
         {
@@ -116,6 +122,9 @@
             "enabled": true,
             "baseUrl": "http://localhost:8081/app",
             "adminUrl": "http://localhost:8081/app/logout",
+            "redirectUris": [
+                "http://localhost:8081/app/*"
+            ],
             "secret": "password"
         },
         {
@@ -123,6 +132,9 @@
             "enabled": true,
             "baseUrl": "http://localhost:8081/app",
             "adminUrl": "http://localhost:8081/app/logout",
+            "redirectUris": [
+                "http://localhost:8081/app/*"
+            ],
             "secret": "password"
         }
     ],

testsuite/integration/src/test/resources/testrealm.json

@@ -29,6 +29,9 @@
         {
             "name" : "third-party",
             "enabled": true,
+            "redirectUris": [
+                "http://localhost:8081/app/*"
+            ],
             "secret": "password"
         }
     ],
@@ -53,6 +56,9 @@
             "name": "test-app",
             "enabled": true,
             "baseUrl": "http://localhost:8081/app",
+            "redirectUris": [
+                "http://localhost:8081/app/*"
+            ],
             "adminUrl": "http://localhost:8081/app/logout",
             "secret": "password"
          }