Details
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 b178df9..b69cea8 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<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);
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 2bc755e..b40e10e 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<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;
+ }
+ }
+
}
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 017aca5..f358882 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
@@ -98,31 +98,6 @@ public class AuthorizationCodeTest {
}
@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 0000000..5762c9d
--- /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 <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="));
+ }
+
+}
