libre.sh/keycloak-scim
2
0
Fork 0
Code Issues 14 Pull requests Releases Packages Activity Actions 6

Switch Trusted Host policy redirect verification to URI

Browse source 
Switch parsing of the redirect URIs for the Trusted Host Client Registration Policy from URL to URI.
The java URL class tries to instantiate a handler for the scheme, which fails when a "custom" scheme, such as those used in phone apps is used.
In contrast, the URI class simply parses the string, ensuring the format is valid.
The other URLs (baseUrl, rootUrl, adminUrl) are still parsed as URLs.
See https://openid.net/specs/openid-connect-registration-1_0.html#ClientMetadata for the Client Registration parameter documentation.

Closes #22309
This commit is contained in:
t0xicCode 2023-08-08 08:51:18 -04:00 committed by Marek Posolda
parent baac060eb1
commit 822c13ff6f
3 changed files with 60 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
services/src/main/java/org/keycloak/services/ServicesLogger.java
View file

@ -471,4 +471,8 @@ public interface ServicesLogger extends BasicLogger {
@LogMessage(level = DEBUG)
@Message(id=107, value="Skipping create admin user. Admin already exists in realm '%s'.")
void addAdminUserFailedAdminExists(String realm);
@LogMessage(level = WARN)
@Message(id=108, value="URI '%s' doesn't match any trustedHost or trustedDomain")
void uriDoesntMatch(String uri);
}

52
services/src/main/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicy.java
View file

@ -19,6 +19,8 @@ package org.keycloak.services.clientregistration.policy.impl;
import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.UnknownHostException;
import java.util.LinkedList;
@ -64,7 +66,6 @@ public class TrustedHostClientRegistrationPolicy implements ClientRegistrationPo
}
@Override
public void beforeUpdate(ClientRegistrationContext context, ClientModel clientModel) throws ClientRegistrationPolicyException {
verifyHost();
@ -214,26 +215,17 @@ public class TrustedHostClientRegistrationPolicy implements ClientRegistrationPo
checkURLTrusted(adminUrl, trustedHosts, trustedDomains);
}
for (String redirect : resolvedRedirects) {
checkURLTrusted(redirect, trustedHosts, trustedDomains);
checkURITrusted(redirect, trustedHosts, trustedDomains);
}
}
protected void checkURLTrusted(String url, List<String> trustedHosts, List<String> trustedDomains) throws ClientRegistrationPolicyException {
try {
String host = new URL(url).getHost();
for (String trustedHost : trustedHosts) {
if (host.equals(trustedHost)) {
return;
}
}
for (String trustedDomain : trustedDomains) {
if (host.endsWith(trustedDomain)) {
return;
}
if (checkHostTrusted(host, trustedHosts, trustedDomains)) {
return;
}
} catch (MalformedURLException mfe) {
logger.debugf(mfe, "URL '%s' is malformed", url);
@ -244,6 +236,38 @@ public class TrustedHostClientRegistrationPolicy implements ClientRegistrationPo
throw new ClientRegistrationPolicyException("URL doesn't match any trusted host or trusted domain");
}
protected void checkURITrusted(String uri, List<String> trustedHosts, List<String> trustedDomains) throws ClientRegistrationPolicyException {
try {
String host = new URI(uri).getHost();
if (checkHostTrusted(host, trustedHosts, trustedDomains)) {
return;
}
} catch (URISyntaxException use) {
logger.debugf(use, "URI '%s' is malformed", uri);
throw new ClientRegistrationPolicyException("URI is malformed");
}
ServicesLogger.LOGGER.uriDoesntMatch(uri);
throw new ClientRegistrationPolicyException("URI doesn't match any trusted host or trusted domain");
}
private boolean checkHostTrusted(String host, List<String> trustedHosts, List<String> trustedDomains) {
for (String trustedHost : trustedHosts) {
if (host.equals(trustedHost)) {
return true;
}
}
for (String trustedDomain : trustedDomains) {
if (host.endsWith(trustedDomain)) {
return true;
}
}
return false;
}
private static String relativeToAbsoluteURI(String rootUrl, String relative) {
if (relative == null) {
@ -270,6 +294,6 @@ public class TrustedHostClientRegistrationPolicy implements ClientRegistrationPo
// True by default
private boolean parseBoolean(String propertyKey) {
String val = componentModel.getConfig().getFirst(propertyKey);
return val==null || Boolean.parseBoolean(val);
return val == null || Boolean.parseBoolean(val);
}
}

20
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationPoliciesTest.java
View file

@ -194,6 +194,21 @@ public class ClientRegistrationPoliciesTest extends AbstractClientRegistrationTe
}
@Test
public void testAnonCreateWithInvalidRedirectScheme() throws Exception {
setTrustedHost("localhost");
OIDCClientRepresentation client = createRepOidc();
client.setRedirectUris(Collections.singletonList("invalid://bad.host"));
assertOidcFail(ClientRegOp.CREATE, client, 403, "URI doesn't match");
client.setRedirectUris(Collections.singletonList("invalid://localhost"));
OIDCClientRepresentation oidcClientRep = reg.oidc().create(client);
assertRegAccessToken(oidcClientRep.getRegistrationAccessToken(), RegistrationAuth.ANONYMOUS);
}
@Test
public void testAnonUpdateWithTrustedHost() throws Exception {
setTrustedHost("localhost");
@ -201,7 +216,7 @@ public class ClientRegistrationPoliciesTest extends AbstractClientRegistrationTe
// Fail update client
client.setRedirectUris(Collections.singletonList("http://bad:8080/foo"));
assertOidcFail(ClientRegOp.UPDATE, client, 403, "URL doesn't match");
assertOidcFail(ClientRegOp.UPDATE, client, 403, "URI doesn't match");
// Should be fine now
client.setRedirectUris(Collections.singletonList("http://localhost:8080/foo"));
@ -209,6 +224,7 @@ public class ClientRegistrationPoliciesTest extends AbstractClientRegistrationTe
}
@Test
public void testRedirectUriWithDomain() throws Exception {
// Change the policy to avoid checking hosts
@ -229,7 +245,7 @@ public class ClientRegistrationPoliciesTest extends AbstractClientRegistrationTe
// Check new client can't be created anymore
oidcClientRep = createRepOidc("http://www.host.com", "http://www.example.com");
assertOidcFail(ClientRegOp.CREATE, oidcClientRep, 403, "URL doesn't match");
assertOidcFail(ClientRegOp.CREATE, oidcClientRep, 403, "URI doesn't match");
}