From 7d6108d4b901f6d6909e02a7eefcdfe93c9328af Mon Sep 17 00:00:00 2001 From: Marie Daly Date: Tue, 10 Feb 2026 12:00:06 +0000 Subject: [PATCH] Redirect Wildcard changes and more https checks to secure-client-executor (#46082) Closes #45587 Signed-off-by: Marie Daly --- .../topics/changes/changes-26_6_0.adoc | 12 ++ .../java/org/keycloak/models/Constants.java | 2 + .../protocol/oidc/OIDCConfigAttributes.java | 4 +- .../protocol/oidc/utils/RedirectUtils.java | 27 +++ .../protocol/oidc/utils/WebOriginsUtils.java | 7 +- .../executor/SecureClientUrisExecutor.java | 67 +++++-- .../services/managers/RealmManager.java | 4 +- .../policies/ClientPoliciesExecutorTest.java | 172 +++++++++++++++++- 8 files changed, 270 insertions(+), 25 deletions(-) diff --git a/docs/documentation/upgrading/topics/changes/changes-26_6_0.adoc b/docs/documentation/upgrading/topics/changes/changes-26_6_0.adoc index 46308cb72b4..7a0f0ba039d 100644 --- a/docs/documentation/upgrading/topics/changes/changes-26_6_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-26_6_0.adoc @@ -14,6 +14,18 @@ It was also not aligned with how other script-based features work in {project_na If you have existing JavaScript-based policies, make sure to enable the `scripts` feature when starting {project_name}. +=== Stricter Validation for Client URIs (secure-client-uris) +The 'secure-client-uris' client policy executor has been updated to enforce stricter validation on Client URIs. This aligns with FAPI 2.0 security requirements by mandating TLS (HTTPS) for client endpoints. + +HTTPS Enforcement: The following URIs must now use the https scheme. Existing clients configured with http in these fields will fail validation during updates: +'Post logout redirect URIs', 'Logo URL', 'Policy URL', and 'Terms of Service URL'. + +Expanded Wildcard Support: The + wildcard character is now allowed in 'Valid post logout redirect URIs' and 'Web origins'. +Configuring + defers validation to the client's standard 'Valid redirect URIs'. Since redirect URIs are already checked for HTTPS, using the wildcard maintains FAPI 2.0 compliance while reducing configuration duplication. + +Administrators must ensure that all configured URIs for the fields listed above use https. Clients attempting to update or register with http in these fields will fail validation when using the 'secure-client-uris' executor. + + // ------------------------ Notable changes ------------------------ // == Notable changes 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 2e33cfc8224..c331335b2c5 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 @@ -135,6 +135,8 @@ public final class Constants { // multiple values into single string public static final String CFG_DELIMITER = "##"; + public static final String INCLUDE_REDIRECTS = "+"; + // Better performance to use this instead of String.split public static final Pattern CFG_DELIMITER_PATTERN = Pattern.compile("\\s*" + CFG_DELIMITER + "\\s*"); diff --git a/server-spi-private/src/main/java/org/keycloak/protocol/oidc/OIDCConfigAttributes.java b/server-spi-private/src/main/java/org/keycloak/protocol/oidc/OIDCConfigAttributes.java index 0f069c50a7e..d2929d7936b 100644 --- a/server-spi-private/src/main/java/org/keycloak/protocol/oidc/OIDCConfigAttributes.java +++ b/server-spi-private/src/main/java/org/keycloak/protocol/oidc/OIDCConfigAttributes.java @@ -98,7 +98,9 @@ public final class OIDCConfigAttributes { public static final String JWT_AUTHORIZATION_GRANT_ENABLED = "oauth2.jwt.authorization.grant.enabled"; public static final String JWT_AUTHORIZATION_GRANT_IDP = "oauth2.jwt.authorization.grant.idp"; - + public static final String LOGO_URI = "logoUri"; + public static final String TOS_URI = "tosUri"; + public static final String POLICY_URI = "policyUri"; private OIDCConfigAttributes() { } 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 889d884d0c5..db582ca5414 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 @@ -20,7 +20,9 @@ package org.keycloak.protocol.oidc.utils; import java.net.URI; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.TreeSet; import java.util.regex.Pattern; @@ -214,4 +216,29 @@ public class RedirectUtils { } return redirectUri; } + + public static Set resolveUrlsWithRedirects(KeycloakSession session, List origUrls, + String rootUrl, List redirectUris, boolean returnAsOrigins) { + + Set refactoredUrls = (origUrls != null) ? new HashSet<>(origUrls) : new HashSet<>(); + if (refactoredUrls.contains(Constants.INCLUDE_REDIRECTS)) { + refactoredUrls.remove(Constants.INCLUDE_REDIRECTS); + + Set redirectsToProcess = (redirectUris != null) ? new HashSet<>(redirectUris) : Collections.emptySet(); + for (String redirectUri : resolveValidRedirects(session, rootUrl, redirectsToProcess)) { + if (isValidScheme(redirectUri)) { + if (returnAsOrigins) { + refactoredUrls.add(UriUtils.getOrigin(redirectUri)); + } else { + refactoredUrls.add(redirectUri); + } + } + } + } + return refactoredUrls; + } + + private static boolean isValidScheme(String url) { + return url != null && (url.startsWith("http://") || url.startsWith("https://")); + } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/WebOriginsUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/WebOriginsUtils.java index 6ebc8c86671..d29decfb822 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/WebOriginsUtils.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/WebOriginsUtils.java @@ -22,6 +22,7 @@ import java.util.Set; import org.keycloak.common.util.UriUtils; import org.keycloak.models.ClientModel; +import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; /** @@ -29,15 +30,13 @@ import org.keycloak.models.KeycloakSession; */ public class WebOriginsUtils { - public static final String INCLUDE_REDIRECTS = "+"; - public static Set resolveValidWebOrigins(KeycloakSession session, ClientModel client) { Set origins = new HashSet<>(); if (client.getWebOrigins() != null) { origins.addAll(client.getWebOrigins()); } - if (origins.contains(INCLUDE_REDIRECTS)) { - origins.remove(INCLUDE_REDIRECTS); + if (origins.contains(Constants.INCLUDE_REDIRECTS)) { + origins.remove(Constants.INCLUDE_REDIRECTS); for (String redirectUri : RedirectUtils.resolveValidRedirects(session, client.getRootUrl(), client.getRedirectUris())) { if (redirectUri.startsWith("http://") || redirectUri.startsWith("https://")) { origins.add(UriUtils.getOrigin(redirectUri)); diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureClientUrisExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureClientUrisExecutor.java index d320a70b950..1e9c2c28d05 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureClientUrisExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureClientUrisExecutor.java @@ -17,16 +17,19 @@ package org.keycloak.services.clientpolicy.executor; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import org.keycloak.OAuthErrorException; import org.keycloak.models.CibaConfig; import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; import org.keycloak.protocol.oidc.OIDCConfigAttributes; +import org.keycloak.protocol.oidc.utils.RedirectUtils; import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.services.clientpolicy.ClientPolicyContext; @@ -86,38 +89,40 @@ public class SecureClientUrisExecutor implements ClientPolicyExecutorProvider webOrigins = clientRep.getWebOrigins(); - if (webOrigins != null) confirmSecureUris(webOrigins, "webOrigins"); + if (baseUrl != null) confirmSecureUris(List.of(baseUrl), "baseUrl"); // backchannel logout URL String logoutUrl = Optional.ofNullable(clientRep.getAttributes()).orElse(Collections.emptyMap()).get(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL); - if (logoutUrl != null) confirmSecureUris(Arrays.asList(logoutUrl), "logoutUrl"); + if (logoutUrl != null) confirmSecureUris(List.of(logoutUrl), "logoutUrl"); // OAuth2 : redirectUris List redirectUris = clientRep.getRedirectUris(); if (redirectUris != null) confirmSecureUris(redirectUris, "redirectUris"); + // web origins + List webOrigins = clientRep.getWebOrigins(); + if (webOrigins != null) { + List resolvedWebOriginUrls = resolveUrlWithRedirects(webOrigins, redirectUris, rootUrl, true); + confirmSecureUris(resolvedWebOriginUrls, "webOrigins"); + } + // OAuth2 : jwks_uri String jwksUri = Optional.ofNullable(clientRep.getAttributes()).orElse(Collections.emptyMap()).get(OIDCConfigAttributes.JWKS_URL); - if (jwksUri != null) confirmSecureUris(Arrays.asList(jwksUri), "jwksUri"); + if (jwksUri != null) confirmSecureUris(List.of(jwksUri), "jwksUri"); // OIDD : requestUris List requestUris = getAttributeMultivalued(clientRep, OIDCConfigAttributes.REQUEST_URIS); @@ -125,7 +130,27 @@ public class SecureClientUrisExecutor implements ClientPolicyExecutorProvider postLogoutRedirectUris = getAttributeMultivalued(clientRep, OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS); + if (postLogoutRedirectUris != null && !postLogoutRedirectUris.isEmpty()) { + List validRedirects = clientRep.getRedirectUris() != null ? clientRep.getRedirectUris() : Collections.emptyList(); + List resolvedPostLogoutUrls = resolveUrlWithRedirects(postLogoutRedirectUris, validRedirects, clientRep.getRootUrl(), false); + confirmSecureUris(resolvedPostLogoutUrls, "postLogoutUris"); + } + + // logoUri + String logoUri = Optional.ofNullable(clientRep.getAttributes()).orElse(Collections.emptyMap()).get(OIDCConfigAttributes.LOGO_URI); + if (logoUri != null) confirmSecureUris(List.of(logoUri), "logoUri"); + + // termsOfServiceUri + String termsOfServiceUri = Optional.ofNullable(clientRep.getAttributes()).orElse(Collections.emptyMap()).get(OIDCConfigAttributes.TOS_URI); + if (termsOfServiceUri != null) confirmSecureUris(List.of(termsOfServiceUri), "tosUri"); + + // policyUri + String policyUri = Optional.ofNullable(clientRep.getAttributes()).orElse(Collections.emptyMap()).get(OIDCConfigAttributes.POLICY_URI); + if (policyUri != null) confirmSecureUris(List.of(policyUri), "policyUri"); } private List getAttributeMultivalued(ClientRepresentation clientRep, String attrKey) { @@ -140,9 +165,11 @@ public class SecureClientUrisExecutor implements ClientPolicyExecutorProvider resolveUrlWithRedirects(List originalUrls, List redirectUris, + String rootUrl, boolean returnAsOrigins) { + if (originalUrls == null || originalUrls.isEmpty()) { + return Collections.emptyList(); + } + Set resolvedUrls = RedirectUtils.resolveUrlsWithRedirects(session, originalUrls, rootUrl, redirectUris, returnAsOrigins); + return new ArrayList<>(resolvedUrls); + } +} diff --git a/services/src/main/java/org/keycloak/services/managers/RealmManager.java b/services/src/main/java/org/keycloak/services/managers/RealmManager.java index eb73f05b9fa..695b147f7bb 100755 --- a/services/src/main/java/org/keycloak/services/managers/RealmManager.java +++ b/services/src/main/java/org/keycloak/services/managers/RealmManager.java @@ -197,8 +197,8 @@ public class RealmManager { String baseUrl = "/admin/" + Encode.encodePathAsIs(realm.getName()) + "/console/"; adminConsole.setBaseUrl(baseUrl); adminConsole.addRedirectUri(baseUrl + "*"); - adminConsole.setAttribute(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, "+"); - adminConsole.setWebOrigins(Collections.singleton("+")); + adminConsole.setAttribute(OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, Constants.INCLUDE_REDIRECTS); + adminConsole.setWebOrigins(Collections.singleton(Constants.INCLUDE_REDIRECTS)); adminConsole.setEnabled(true); adminConsole.setAlwaysDisplayInConsole(false); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExecutorTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExecutorTest.java index c08f9340f60..5ca41c5e174 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExecutorTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesExecutorTest.java @@ -1216,14 +1216,14 @@ public class ClientPoliciesExecutorTest extends AbstractClientPoliciesTest { clientRep.setAdminUrl("https://client.example.com/admin/"); // baseUrl clientRep.setBaseUrl("https://client.example.com/base/"); + // OAuth2 : redirectUris + clientRep.setRedirectUris(Arrays.asList("https://client.example.com/redirect/", "https://client.example.com/callback/")); // web origins clientRep.setWebOrigins(Arrays.asList("https://valid.other.client.example.com/", "https://valid.another.client.example.com/")); // backchannel logout URL Map attributes = Optional.ofNullable(clientRep.getAttributes()).orElse(new HashMap<>()); attributes.put(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL, "https://client.example.com/logout/"); clientRep.setAttributes(attributes); - // OAuth2 : redirectUris - clientRep.setRedirectUris(Arrays.asList("https://client.example.com/redirect/", "https://client.example.com/callback/")); // OAuth2 : jwks_uri attributes.put(OIDCConfigAttributes.JWKS_URL, "https://client.example.com/jwks/"); clientRep.setAttributes(attributes); @@ -1341,6 +1341,45 @@ public class ClientPoliciesExecutorTest extends AbstractClientPoliciesTest { assertEquals(OAuthErrorException.INVALID_CLIENT_METADATA, e.getError()); assertEquals("Invalid cibaClientNotificationEndpoint", e.getErrorDetail()); } + + try { + updateClientByAdmin(cid, (ClientRepresentation clientRep) -> { + // OIDC: logo_uri + Map attributes = Optional.ofNullable(clientRep.getAttributes()).orElse(new HashMap<>()); + attributes.put("logoUri", "http://client.example.com/logo/"); + clientRep.setAttributes(attributes); + }); + fail(); + } catch (ClientPolicyException e) { + assertEquals(OAuthErrorException.INVALID_CLIENT_METADATA, e.getError()); + assertEquals("Invalid logoUri", e.getErrorDetail()); + } + + try { + updateClientByAdmin(cid, (ClientRepresentation clientRep) -> { + // OIDC: tos_uri (Terms of Service) + Map attributes = Optional.ofNullable(clientRep.getAttributes()).orElse(new HashMap<>()); + attributes.put("tosUri", "http://client.example.com/tos/"); + clientRep.setAttributes(attributes); + }); + fail(); + } catch (ClientPolicyException e) { + assertEquals(OAuthErrorException.INVALID_CLIENT_METADATA, e.getError()); + assertEquals("Invalid tosUri", e.getErrorDetail()); + } + + try { + updateClientByAdmin(cid, (ClientRepresentation clientRep) -> { + // OIDC: policy_uri + Map attributes = Optional.ofNullable(clientRep.getAttributes()).orElse(new HashMap<>()); + attributes.put("policyUri", "http://client.example.com/policy/"); + clientRep.setAttributes(attributes); + }); + fail(); + } catch (ClientPolicyException e) { + assertEquals(OAuthErrorException.INVALID_CLIENT_METADATA, e.getError()); + assertEquals("Invalid policyUri", e.getErrorDetail()); + } } @Test @@ -1830,4 +1869,133 @@ public class ClientPoliciesExecutorTest extends AbstractClientPoliciesTest { return client.execute(post); } } + + @Test + public void testSecureClientRegisteringUriEnforceExecutorWithRedirectWildcard() throws Exception { + // register profiles + String json = (new ClientProfilesBuilder()).addProfile( + (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Ensimmainen Profiili") + .addExecutor(SecureClientUrisExecutorFactory.PROVIDER_ID, null) + .toRepresentation() + ).toString(); + updateProfiles(json); + + // register policies + json = (new ClientPoliciesBuilder()).addPolicy( + (new ClientPolicyBuilder()).createPolicy(POLICY_NAME, "Ensimmainen Politiikka", Boolean.TRUE) + .addCondition(ClientUpdaterContextConditionFactory.PROVIDER_ID, + createClientUpdateContextConditionConfig(Arrays.asList( + ClientUpdaterContextConditionFactory.BY_AUTHENTICATED_USER, + ClientUpdaterContextConditionFactory.BY_INITIAL_ACCESS_TOKEN, + ClientUpdaterContextConditionFactory.BY_REGISTRATION_ACCESS_TOKEN))) + .addProfile(PROFILE_NAME) + .toRepresentation() + ).toString(); + updatePolicies(json); + + + String cid = null; + String clientId = generateSuffixedName(CLIENT_NAME); + try { + cid = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> { + clientRep.setServiceAccountsEnabled(Boolean.TRUE); + clientRep.setRedirectUris(List.of("https://client.example.com/redirect/")); + }); + } catch (Exception e) { + fail(); + } + + try { // Redirect relative wildcard - '+' as single value + updateClientByAdmin(cid, (ClientRepresentation clientRep) -> { + // rootUrl + clientRep.setRootUrl("https://client.example.com/"); + // adminUrl + clientRep.setAdminUrl("https://client.example.com/admin/"); + // baseUrl + clientRep.setBaseUrl("https://client.example.com/base/"); + // OAuth2 : redirectUris + clientRep.setRedirectUris(Arrays.asList("https://client.example.com/redirect/", "https://client.example.com/callback/")); + // web origins + clientRep.setWebOrigins(List.of("+")); + // backchannel logout URL + Map attributes = Optional.ofNullable(clientRep.getAttributes()).orElse(new HashMap<>()); + attributes.put(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL, "https://client.example.com/logout/"); + clientRep.setAttributes(attributes); + // OIDC: postLogoutUris + setAttributeMultivalued(clientRep, OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, List.of("+")); + // OAuth2 : jwks_uri + attributes.put(OIDCConfigAttributes.JWKS_URL, "https://client.example.com/jwks/"); + clientRep.setAttributes(attributes); + // OIDC : requestUris + setAttributeMultivalued(clientRep, OIDCConfigAttributes.REQUEST_URIS, Arrays.asList("https://client.example.com/request/", "https://client.example.com/reqobj/")); + // CIBA Client Notification Endpoint + attributes.put(CibaConfig.CIBA_BACKCHANNEL_CLIENT_NOTIFICATION_ENDPOINT, "https://client.example.com/client-notification/"); + clientRep.setAttributes(attributes); + }); + } catch (Exception e) { + fail(); + } + + try { // Redirect relative wildcard - '+' with multiple values + updateClientByAdmin(cid, (ClientRepresentation clientRep) -> { + // rootUrl + clientRep.setRootUrl("https://client.example.com/"); + // adminUrl + clientRep.setAdminUrl("https://client.example.com/admin/"); + // baseUrl + clientRep.setBaseUrl("https://client.example.com/base/"); + // OAuth2 : redirectUris + clientRep.setRedirectUris(List.of("https://client.example.com/redirect/")); + // web origins + clientRep.setWebOrigins(Arrays.asList("https://valid.other.client.example.com/", "https://valid.another.client.example.com/", "+")); + // backchannel logout URL + Map attributes = Optional.ofNullable(clientRep.getAttributes()).orElse(new HashMap<>()); + attributes.put(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL, "https://client.example.com/logout/"); + clientRep.setAttributes(attributes); + // OIDC: postLogoutUris + setAttributeMultivalued(clientRep, OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, Arrays.asList("https://client.example.com/postlogout/", "+")); + // OAuth2 : jwks_uri + attributes.put(OIDCConfigAttributes.JWKS_URL, "https://client.example.com/jwks/"); + clientRep.setAttributes(attributes); + // OIDC : requestUris + setAttributeMultivalued(clientRep, OIDCConfigAttributes.REQUEST_URIS, Arrays.asList("https://client.example.com/request/", "https://client.example.com/reqobj/")); + // CIBA Client Notification Endpoint + attributes.put(CibaConfig.CIBA_BACKCHANNEL_CLIENT_NOTIFICATION_ENDPOINT, "https://client.example.com/client-notification/"); + clientRep.setAttributes(attributes); + }); + } catch (Exception e) { + fail(); + } + + try { // Redirect relative wildcard - '+' with no redirect value added + updateClientByAdmin(cid, (ClientRepresentation clientRep) -> { + // rootUrl + clientRep.setRootUrl("https://client.example.com/"); + // adminUrl + clientRep.setAdminUrl("https://client.example.com/admin/"); + // baseUrl + clientRep.setBaseUrl("https://client.example.com/base/"); + // OAuth2 : redirectUris + clientRep.setRedirectUris(null); + // web origins + clientRep.setWebOrigins(Arrays.asList("+", "https://valid.other.client.example.com/")); + // backchannel logout URL + Map attributes = Optional.ofNullable(clientRep.getAttributes()).orElse(new HashMap<>()); + attributes.put(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL, "https://client.example.com/logout/"); + clientRep.setAttributes(attributes); + // OIDC: postLogoutUris + setAttributeMultivalued(clientRep, OIDCConfigAttributes.POST_LOGOUT_REDIRECT_URIS, Arrays.asList("https://client.example.com/postlogout/", "+")); + // OAuth2 : jwks_uri + attributes.put(OIDCConfigAttributes.JWKS_URL, "https://client.example.com/jwks/"); + clientRep.setAttributes(attributes); + // OIDC : requestUris + setAttributeMultivalued(clientRep, OIDCConfigAttributes.REQUEST_URIS, Arrays.asList("https://client.example.com/request/", "https://client.example.com/reqobj/")); + // CIBA Client Notification Endpoint + attributes.put(CibaConfig.CIBA_BACKCHANNEL_CLIENT_NOTIFICATION_ENDPOINT, "https://client.example.com/client-notification/"); + clientRep.setAttributes(attributes); + }); + } catch (Exception e) { + fail(); + } + } }