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 2bc755e8cc..b40e10e0f7 100755 --- a/services/src/main/java/org/keycloak/services/resources/TokenService.java +++ b/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 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; + } + } + } diff --git a/services/src/main/java/org/keycloak/services/resources/flows/OAuthFlows.java b/services/src/main/java/org/keycloak/services/resources/flows/OAuthFlows.java index b178df9716..b69cea8069 100755 --- a/services/src/main/java/org/keycloak/services/resources/flows/OAuthFlows.java +++ b/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 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 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); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java index 017aca52b3..f35888228f 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java +++ b/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"); 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 new file mode 100755 index 0000000000..5762c9d635 --- /dev/null +++ b/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 Viliam Rockai + */ +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=")); + } + +}