From b3dd349342a2346cbd5239bba329963038da7133 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Tue, 20 May 2014 11:38:35 -0400 Subject: [PATCH] check redirect uri exists in token service --- .../en-US/modules/MigrationFromOlderVersions.xml | 9 +++++++++ .../services/resources/TokenService.java | 16 ++++++++++------ .../keycloak/testsuite/account/ProfileTest.java | 2 +- .../testsuite/composites/CompositeRoleTest.java | 4 ++++ .../testsuite/oauth/OAuthRedirectUriTest.java | 16 ++++++++-------- .../src/test/resources/testcomposite.json | 12 ++++++++++++ .../src/test/resources/testrealm.json | 6 ++++++ 7 files changed, 50 insertions(+), 15 deletions(-) diff --git a/docbook/reference/en/en-US/modules/MigrationFromOlderVersions.xml b/docbook/reference/en/en-US/modules/MigrationFromOlderVersions.xml index 7977348e0b..b0d33201dd 100755 --- a/docbook/reference/en/en-US/modules/MigrationFromOlderVersions.xml +++ b/docbook/reference/en/en-US/modules/MigrationFromOlderVersions.xml @@ -1,5 +1,14 @@ Migration from older versions + + Migrating from 1.0 Alpha 4 to Beta 1 + + + 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. + + + Migrating from 1.0 Alpha 2 to Alpha 3 diff --git a/services/src/main/java/org/keycloak/services/resources/TokenService.java b/services/src/main/java/org/keycloak/services/resources/TokenService.java index 77e470ae02..5c337b7556 100755 --- a/services/src/main/java/org/keycloak/services/resources/TokenService.java +++ b/services/src/main/java/org/keycloak/services/resources/TokenService.java @@ -945,6 +945,7 @@ public class TokenService { } public static boolean matchesRedirects(Set 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 validRedirects = client.getRedirectUris(); if (redirectUri == null) { - return validRedirects.size() == 1 ? validRedirects.iterator().next() : null; - } else if (validRedirects.isEmpty()) { - if (client.isPublicClient()) { - logger.error("Client redirect uri must be registered for public client"); - return null; + if (validRedirects.size() != 1) return null; + String validRedirect = validRedirects.iterator().next(); + int idx = validRedirect.indexOf("/*"); + if (idx > -1) { + 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 resolveValidRedirects = resolveValidRedirects(uriInfo, validRedirects); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/ProfileTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/ProfileTest.java index 73de4a7e7e..eee56e0c6e 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/ProfileTest.java +++ b/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"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/composites/CompositeRoleTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/composites/CompositeRoleTest.java index aa5004b04a..c354f0dd8b 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/composites/CompositeRoleTest.java +++ b/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"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java index 73516b8140..6d7d6ef7c9 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java +++ b/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.assertEquals(oauth.getCurrentRequest(), "http://localhost:8081/app/auth"); + 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").addRedirectUri("http://localhost:8081/app"); + appRealm.getApplicationNameMap().get("test-app").addRedirectUri("http://localhost:8081/app/*"); } }); } diff --git a/testsuite/integration/src/test/resources/testcomposite.json b/testsuite/integration/src/test/resources/testcomposite.json index 9b0878447e..6e01de3dd0 100755 --- a/testsuite/integration/src/test/resources/testcomposite.json +++ b/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" } ], diff --git a/testsuite/integration/src/test/resources/testrealm.json b/testsuite/integration/src/test/resources/testrealm.json index a7cf0d49e1..8c889e0d9b 100755 --- a/testsuite/integration/src/test/resources/testrealm.json +++ b/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" }