diff --git a/server-spi-private/src/main/java/org/keycloak/models/Constants.java b/server-spi-private/src/main/java/org/keycloak/models/Constants.java index 27ae10a866a..54409c33a10 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/Constants.java +++ b/server-spi-private/src/main/java/org/keycloak/models/Constants.java @@ -43,8 +43,6 @@ public final class Constants { public static final Collection defaultClients = Arrays.asList(ACCOUNT_MANAGEMENT_CLIENT_ID, ADMIN_CLI_CLIENT_ID, BROKER_SERVICE_CLIENT_ID, REALM_MANAGEMENT_CLIENT_ID, ADMIN_CONSOLE_CLIENT_ID); public static final String INSTALLED_APP_URN = "urn:ietf:wg:oauth:2.0:oob"; - public static final String INSTALLED_APP_URL = "http://localhost"; - public static final String INSTALLED_APP_LOOPBACK = "http://127.0.0.1"; public static final String READ_TOKEN_ROLE = "read-token"; public static final String[] BROKER_SERVICE_ROLES = {READ_TOKEN_ROLE}; diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java index d7dff200d22..75439671b02 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java @@ -18,6 +18,7 @@ package org.keycloak.protocol.oidc.utils; import org.jboss.logging.Logger; +import org.keycloak.common.util.KeycloakUriBuilder; import org.keycloak.common.util.UriUtils; import org.keycloak.models.ClientModel; import org.keycloak.models.Constants; @@ -29,7 +30,9 @@ import org.keycloak.services.Urls; import org.keycloak.services.util.ResolveRelative; import java.net.URI; +import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.Set; import java.util.TreeSet; import java.util.regex.Pattern; @@ -40,6 +43,8 @@ import java.util.stream.Collectors; */ public class RedirectUtils { + public static final Set LOOPBACK_INTERFACES = new HashSet<>(Arrays.asList("localhost", "127.0.0.1", "[::1]")); + private static final Logger logger = Logger.getLogger(RedirectUtils.class); /** @@ -118,20 +123,9 @@ public class RedirectUtils { String valid = matchesRedirects(resolveValidRedirects, r, allowWildcards); - if (valid == null && (r.startsWith(Constants.INSTALLED_APP_URL) || r.startsWith(Constants.INSTALLED_APP_LOOPBACK)) && r.indexOf(':', Constants.INSTALLED_APP_URL.length()) >= 0) { - int i = r.indexOf(':', Constants.INSTALLED_APP_URL.length()); - - StringBuilder sb = new StringBuilder(); - sb.append(r.substring(0, i)); - - i = r.indexOf('/', i); - if (i >= 0) { - sb.append(r.substring(i)); - } - - r = sb.toString(); - - valid = matchesRedirects(resolveValidRedirects, r, allowWildcards); + if (valid == null && "http".equals(originalRedirect.getScheme()) && LOOPBACK_INTERFACES.contains(originalRedirect.getHost())) { + String redirectWithDefaultPort = KeycloakUriBuilder.fromUri(originalRedirect).port(80).buildAsString(); + valid = matchesRedirects(resolveValidRedirects, redirectWithDefaultPort, allowWildcards); } if (valid != null && !originalRedirect.isAbsolute()) { diff --git a/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java b/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java index fcc501934d3..a9210c4015b 100644 --- a/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java +++ b/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java @@ -72,6 +72,45 @@ public class RedirectUtilsTest { Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.com/test", set, false)); } + @Test + public void testVerifyRedirectUriNative() { + Set set = Stream.of( + "http://127.0.0.1", + "http://127.0.0.1/callback", + "http://[::1]", + "http://[::1]/callback", + "http://127.0.0.1/callback", + "http://localhost", + "http://localhost/callback" + ).collect(Collectors.toSet()); + + Assert.assertEquals("http://127.0.0.1", RedirectUtils.verifyRedirectUri(session, null, "http://127.0.0.1", set, false)); + Assert.assertEquals("http://127.0.0.1:12324", RedirectUtils.verifyRedirectUri(session, null, "http://127.0.0.1:12324", set, false)); + Assert.assertEquals("http://127.0.0.1/callback", RedirectUtils.verifyRedirectUri(session, null, "http://127.0.0.1/callback", set, false)); + Assert.assertEquals("http://127.0.0.1:12324/callback", RedirectUtils.verifyRedirectUri(session, null, "http://127.0.0.1:12324/callback", set, false)); + Assert.assertEquals("http://[::1]", RedirectUtils.verifyRedirectUri(session, null, "http://[::1]", set, false)); + Assert.assertEquals("http://[::1]:12324", RedirectUtils.verifyRedirectUri(session, null, "http://[::1]:12324", set, false)); + Assert.assertEquals("http://[::1]/callback", RedirectUtils.verifyRedirectUri(session, null, "http://[::1]/callback", set, false)); + Assert.assertEquals("http://[::1]:12324/callback", RedirectUtils.verifyRedirectUri(session, null, "http://[::1]:12324/callback", set, false)); + Assert.assertEquals("http://localhost", RedirectUtils.verifyRedirectUri(session, null, "http://localhost", set, false)); + Assert.assertEquals("http://localhost:12324", RedirectUtils.verifyRedirectUri(session, null, "http://localhost:12324", set, false)); + Assert.assertEquals("http://localhost/callback", RedirectUtils.verifyRedirectUri(session, null, "http://localhost/callback", set, false)); + Assert.assertEquals("http://localhost:12324/callback", RedirectUtils.verifyRedirectUri(session, null, "http://localhost:12324/callback", set, false)); + + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://127.0.0.1@invalid.com", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://127.0.0.1:12324@invalid.com", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://127.0.0.1@invalid.com/callback", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://127.0.0.1:12324@invalid.com/callback", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://[::1]@invalid.com", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://[::1]:12324@invalid.com", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://[::1]@invalid.com/callback", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://[::1]:12324@invalid.com/callback", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://localhost@invalid.com", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://localhost:12324@invalid.com", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://localhost@invalid.com/callback", set, false)); + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "http://localhost:12324@invalid.com/callback", set, false)); + } + @Test public void testverifyRedirectUriMixedSchemes() { Set set = Stream.of( diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java index 52c361c8f26..1c2061692c3 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java @@ -105,12 +105,12 @@ public class OAuthRedirectUriTest extends AbstractKeycloakTest { RealmBuilder realm = RealmBuilder.edit(realmRepresentation).testEventListener(); ClientBuilder installedApp = ClientBuilder.create().clientId("test-installed").name("test-installed") - .redirectUris(Constants.INSTALLED_APP_URN, Constants.INSTALLED_APP_URL) + .redirectUris(Constants.INSTALLED_APP_URN, "http://localhost") .secret("password"); realm.client(installedApp); ClientBuilder installedApp2 = ClientBuilder.create().clientId("test-installed2").name("test-installed2") - .redirectUris(Constants.INSTALLED_APP_URL + "/myapp") + .redirectUris("http://localhost/myapp") .secret("password"); realm.client(installedApp2); @@ -157,12 +157,12 @@ public class OAuthRedirectUriTest extends AbstractKeycloakTest { realm.client(installedAppCustomScheme); ClientBuilder installedAppLoopback = ClientBuilder.create().clientId("test-installed-loopback").name("test-installed-loopback") - .redirectUris(Constants.INSTALLED_APP_LOOPBACK) + .redirectUris("http://127.0.0.1") .secret("password"); realm.client(installedAppLoopback); ClientBuilder installedAppLoopback2 = ClientBuilder.create().clientId("test-installed-loopback2").name("test-installed-loopback2") - .redirectUris(Constants.INSTALLED_APP_LOOPBACK + "/myapp") + .redirectUris("http://127.0.0.1/myapp") .secret("password"); realm.client(installedAppLoopback2);