From 6b3fa8b7a77166d34b5a88a4cee94306ae339fdb Mon Sep 17 00:00:00 2001 From: Pedro Hos Date: Mon, 19 Feb 2024 10:40:42 -0300 Subject: [PATCH] Invalid redirect uri when identity provider alias has spaces (#22840) closes #22836 Co-authored-by: Marek Posolda --- .../keycloak/utils/ReservedCharValidator.java | 17 ++- .../admin/IdentityProvidersResource.java | 8 +- .../testsuite/admin/IdentityProviderTest.java | 116 +++++++++++------- 3 files changed, 95 insertions(+), 46 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/utils/ReservedCharValidator.java b/server-spi-private/src/main/java/org/keycloak/utils/ReservedCharValidator.java index 738afb6437..219dbd081f 100644 --- a/server-spi-private/src/main/java/org/keycloak/utils/ReservedCharValidator.java +++ b/server-spi-private/src/main/java/org/keycloak/utils/ReservedCharValidator.java @@ -48,7 +48,22 @@ public class ReservedCharValidator { throw new ReservedCharException(message); } } - + + public static void validateNoSpace(String str) { + if (str == null) return; + + Pattern pattern = Pattern.compile("\\s"); + Matcher matcher = pattern.matcher(str); + + if (matcher.find()) { + String message = "Empty Space not allowed."; + logger.warn(message); + throw new ReservedCharException(message); + } + + validate(str, RESERVED_CHARS_PATTERN); + } + public static void validate(String str) { validate(str, RESERVED_CHARS_PATTERN); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java index 8d962a86a8..4a7a071d9d 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java @@ -149,9 +149,9 @@ public class IdentityProvidersResource { if (data == null || !(data.containsKey("providerId") && data.containsKey("fromUrl"))) { throw new BadRequestException(); } - - ReservedCharValidator.validate((String)data.get("alias")); - + + ReservedCharValidator.validateNoSpace((String)data.get("alias")); + String providerId = data.get("providerId").toString(); String from = data.get("fromUrl").toString(); InputStream inputStream = session.getProvider(HttpClientProvider.class).get(from); @@ -239,7 +239,7 @@ public class IdentityProvidersResource { public Response create(@Parameter(description = "JSON body") IdentityProviderRepresentation representation) { this.auth.realm().requireManageIdentityProviders(); - ReservedCharValidator.validate(representation.getAlias()); + ReservedCharValidator.validateNoSpace(representation.getAlias()); try { IdentityProviderModel identityProvider = RepresentationToModel.toModel(realm, representation, session); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java index dd875da236..c97b939358 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java @@ -17,6 +17,48 @@ package org.keycloak.testsuite.admin; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.keycloak.saml.common.constants.JBossSAMLURIConstants.XMLDSIG_NSURI; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.security.PublicKey; +import java.security.cert.CertificateEncodingException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Base64; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import javax.xml.crypto.dsig.XMLSignature; + import org.jboss.resteasy.plugins.providers.multipart.MultipartFormDataOutput; import org.junit.Test; import org.keycloak.admin.client.resource.IdentityProviderResource; @@ -47,6 +89,7 @@ import org.keycloak.saml.common.exceptions.ConfigurationException; import org.keycloak.saml.common.exceptions.ParsingException; import org.keycloak.saml.common.exceptions.ProcessingException; import org.keycloak.saml.common.util.DocumentUtil; +import org.keycloak.saml.common.util.XmlKeyInfoKeyNameTransformer; import org.keycloak.saml.processing.api.saml.v2.sig.SAML2Signature; import org.keycloak.saml.processing.core.parsers.saml.SAMLParser; import org.keycloak.saml.processing.core.util.XMLSignatureUtil; @@ -56,55 +99,16 @@ import org.keycloak.testsuite.broker.oidc.OverwrittenMappersTestIdentityProvider import org.keycloak.testsuite.updaters.RealmAttributeUpdater; import org.keycloak.testsuite.util.AdminEventPaths; import org.keycloak.testsuite.util.KeyUtils; +import org.keycloak.utils.ReservedCharValidator.ReservedCharException; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NodeList; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.ClientErrorException; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; -import javax.xml.crypto.dsig.XMLSignature; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; -import java.nio.charset.Charset; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.security.PublicKey; -import java.security.cert.CertificateEncodingException; -import java.security.cert.X509Certificate; -import java.util.Arrays; -import java.util.Base64; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.UUID; - -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasEntry; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.notNullValue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertNotNull; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.keycloak.saml.common.constants.JBossSAMLURIConstants.XMLDSIG_NSURI; - -import org.keycloak.saml.common.util.XmlKeyInfoKeyNameTransformer; /** * @author Stian Thorgersen @@ -271,6 +275,22 @@ public class IdentityProviderTest extends AbstractAdminTest { } } } + + @Test + public void shouldFailWhenAliasHasSpaceDuringCreation() { + IdentityProviderRepresentation newIdentityProvider = createRep("New Identity Provider", "oidc"); + + newIdentityProvider.getConfig().put(IdentityProviderModel.SYNC_MODE, "IMPORT"); + newIdentityProvider.getConfig().put("clientId", "clientId"); + newIdentityProvider.getConfig().put("clientSecret", "some secret value"); + newIdentityProvider.getConfig().put("clientAuthMethod",OIDCLoginProtocol.CLIENT_SECRET_BASIC); + + try (Response response = this.realm.identityProviders().create(newIdentityProvider)) { + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + String error = response.readEntity(String.class); + assertTrue(error.contains("Empty Space not allowed.")); + } + } @Test public void testCreateWithBasicAuth() { @@ -681,6 +701,20 @@ public class IdentityProviderTest extends AbstractAdminTest { response.close(); } + @Test + public void importShouldFailDueAliasWithSpace() { + + Map data = new HashMap<>(); + data.put("providerId", "saml"); + data.put("alias", "Alias With Space"); + data.put("fromUrl", "http://"); + + assertThrows(BadRequestException.class, () -> { + realm.identityProviders().importFrom(data); + }); + + } + @Test public void testSamlImportAndExport() throws URISyntaxException, IOException, ParsingException {