[KEYCLOAK-13386] - SslRequired.EXTERNAL doesn't work for identity broker validations

This commit is contained in:
Pedro Igor 2020-03-23 11:14:47 -03:00
parent 4279f5b54f
commit ec63245ac8
3 changed files with 52 additions and 55 deletions

View file

@ -108,7 +108,7 @@ public class UriUtils {
throw new IllegalArgumentException("Invalid protocol/scheme for url [" + name + "]"); throw new IllegalArgumentException("Invalid protocol/scheme for url [" + name + "]");
} }
if (!"https".equals(protocol) && sslRequired.isRequired(url)) { if (!"https".equals(protocol) && sslRequired.isRequired(parsed.getHost())) {
throw new IllegalArgumentException("The url [" + name + "] requires secure connections"); throw new IllegalArgumentException("The url [" + name + "] requires secure connections");
} }
} }

View file

@ -9,7 +9,7 @@ import java.util.List;
* Updater for realm attributes. See {@link ServerResourceUpdater} for further details. * Updater for realm attributes. See {@link ServerResourceUpdater} for further details.
* @author hmlnarik * @author hmlnarik
*/ */
public class RealmAttributeUpdater extends ServerResourceUpdater<ServerResourceUpdater, RealmResource, RealmRepresentation> { public class RealmAttributeUpdater extends ServerResourceUpdater<RealmAttributeUpdater, RealmResource, RealmRepresentation> {
public RealmAttributeUpdater(RealmResource resource) { public RealmAttributeUpdater(RealmResource resource) {
super(resource, resource::toRepresentation, resource::update); super(resource, resource::toRepresentation, resource::update);

View file

@ -44,6 +44,7 @@ import org.keycloak.saml.common.exceptions.ParsingException;
import org.keycloak.saml.processing.core.parsers.saml.SAMLParser; import org.keycloak.saml.processing.core.parsers.saml.SAMLParser;
import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.Assert;
import org.keycloak.testsuite.broker.OIDCIdentityProviderConfigRep; import org.keycloak.testsuite.broker.OIDCIdentityProviderConfigRep;
import org.keycloak.testsuite.updaters.RealmAttributeUpdater;
import org.keycloak.testsuite.util.AdminEventPaths; import org.keycloak.testsuite.util.AdminEventPaths;
import org.w3c.dom.NodeList; import org.w3c.dom.NodeList;
@ -83,6 +84,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.keycloak.testsuite.arquillian.AuthServerTestEnricher.AUTH_SERVER_SSL_REQUIRED; import static org.keycloak.testsuite.arquillian.AuthServerTestEnricher.AUTH_SERVER_SSL_REQUIRED;
/** /**
@ -111,9 +113,6 @@ public class IdentityProviderTest extends AbstractAdminTest {
+ "vOU8TyqfZF5jpv0IcrviLl/DoFrbjByeHR+pu/vClcAOjL/u7oQELuuTfNsBI4tpexUj5G8q/YbEz0gk7idf" + "vOU8TyqfZF5jpv0IcrviLl/DoFrbjByeHR+pu/vClcAOjL/u7oQELuuTfNsBI4tpexUj5G8q/YbEz0gk7idf"
+ "LXrAUVcsR73oTngrhRfwUSmPrjjK0kjcRb6HL9V/+wh3R/6mEd59U08ExT8N38rhmn0CI3ehMdebReprP7U8="; + "LXrAUVcsR73oTngrhRfwUSmPrjjK0kjcRb6HL9V/+wh3R/6mEd59U08ExT8N38rhmn0CI3ehMdebReprP7U8=";
@Rule
public ExpectedException expectedException = ExpectedException.none();
@Test @Test
public void testFindAll() { public void testFindAll() {
create(createRep("google", "google")); create(createRep("google", "google"));
@ -167,14 +166,11 @@ public class IdentityProviderTest extends AbstractAdminTest {
} }
@Test @Test
public void failCreateInvalidUrl() { public void failCreateInvalidUrl() throws Exception {
RealmRepresentation realmRep = realm.toRepresentation(); try (AutoCloseable c = new RealmAttributeUpdater(realmsResouce().realm("test"))
.updateWith(r -> r.setSslRequired(SslRequired.ALL.name()))
realmRep.setSslRequired(SslRequired.ALL.name()); .update()
) {
try {
realm.update(realmRep);
IdentityProviderRepresentation newIdentityProvider = createRep("new-identity-provider", "oidc"); IdentityProviderRepresentation newIdentityProvider = createRep("new-identity-provider", "oidc");
newIdentityProvider.getConfig().put("clientId", "clientId"); newIdentityProvider.getConfig().put("clientId", "clientId");
@ -226,9 +222,6 @@ public class IdentityProviderTest extends AbstractAdminTest {
assertEquals(AUTH_SERVER_SSL_REQUIRED ? Response.Status.BAD_REQUEST.getStatusCode() : assertEquals(AUTH_SERVER_SSL_REQUIRED ? Response.Status.BAD_REQUEST.getStatusCode() :
Response.Status.CREATED.getStatusCode(), response.getStatus()); Response.Status.CREATED.getStatusCode(), response.getStatus());
} }
} finally {
realmRep.setSslRequired(SslRequired.NONE.name());
realm.update(realmRep);
} }
} }
@ -347,14 +340,11 @@ public class IdentityProviderTest extends AbstractAdminTest {
} }
@Test @Test
public void failUpdateInvalidUrl() { public void failUpdateInvalidUrl() throws Exception {
RealmRepresentation realmRep = realm.toRepresentation(); try (RealmAttributeUpdater rau = new RealmAttributeUpdater(realm)
.updateWith(r -> r.setSslRequired(SslRequired.ALL.name()))
realmRep.setSslRequired(SslRequired.ALL.name()); .update()
) {
try {
realm.update(realmRep);
IdentityProviderRepresentation representation = createRep(UUID.randomUUID().toString(), "oidc"); IdentityProviderRepresentation representation = createRep(UUID.randomUUID().toString(), "oidc");
representation.getConfig().put("clientId", "clientId"); representation.getConfig().put("clientId", "clientId");
@ -370,57 +360,64 @@ public class IdentityProviderTest extends AbstractAdminTest {
OIDCIdentityProviderConfigRep oidcConfig = new OIDCIdentityProviderConfigRep(representation); OIDCIdentityProviderConfigRep oidcConfig = new OIDCIdentityProviderConfigRep(representation);
oidcConfig.setAuthorizationUrl("invalid://test"); oidcConfig.setAuthorizationUrl("invalid://test");
try {
this.expectedException.expect( resource.update(representation);
Matchers.allOf(Matchers.instanceOf(ClientErrorException.class), Matchers.hasProperty("response", fail("Invalid URL");
Matchers.hasProperty("status", Matchers.is( } catch (Exception e) {
Response.Status.BAD_REQUEST.getStatusCode()))))); assertTrue(e instanceof ClientErrorException);
resource.update(representation); assertEquals( Response.Status.BAD_REQUEST.getStatusCode(), ClientErrorException.class.cast(e).getResponse().getStatus());
}
oidcConfig.setAuthorizationUrl(null); oidcConfig.setAuthorizationUrl(null);
oidcConfig.setTokenUrl("http://test"); oidcConfig.setTokenUrl("http://test");
this.expectedException.expect( try {
Matchers.allOf(Matchers.instanceOf(ClientErrorException.class), Matchers.hasProperty("response", resource.update(representation);
Matchers.hasProperty("status", Matchers.is( fail("Invalid URL");
Response.Status.BAD_REQUEST.getStatusCode()))))); } catch (Exception e) {
resource.update(representation); assertTrue(e instanceof ClientErrorException);
assertEquals( Response.Status.BAD_REQUEST.getStatusCode(), ClientErrorException.class.cast(e).getResponse().getStatus());
}
oidcConfig.setAuthorizationUrl(null); oidcConfig.setAuthorizationUrl(null);
oidcConfig.setTokenUrl(null); oidcConfig.setTokenUrl(null);
oidcConfig.setJwksUrl("http://test"); oidcConfig.setJwksUrl("http://test");
try {
this.expectedException.expect( resource.update(representation);
Matchers.allOf(Matchers.instanceOf(ClientErrorException.class), Matchers.hasProperty("response", fail("Invalid URL");
Matchers.hasProperty("status", Matchers.is( } catch (Exception e) {
Response.Status.BAD_REQUEST.getStatusCode()))))); assertTrue(e instanceof ClientErrorException);
resource.update(representation); assertEquals( Response.Status.BAD_REQUEST.getStatusCode(), ClientErrorException.class.cast(e).getResponse().getStatus());
}
oidcConfig.setAuthorizationUrl(null); oidcConfig.setAuthorizationUrl(null);
oidcConfig.setTokenUrl(null); oidcConfig.setTokenUrl(null);
oidcConfig.setJwksUrl(null); oidcConfig.setJwksUrl(null);
oidcConfig.setLogoutUrl("http://test"); oidcConfig.setLogoutUrl("http://test");
try {
this.expectedException.expect( resource.update(representation);
Matchers.allOf(Matchers.instanceOf(ClientErrorException.class), Matchers.hasProperty("response", fail("Invalid URL");
Matchers.hasProperty("status", Matchers.is( } catch (Exception e) {
Response.Status.BAD_REQUEST.getStatusCode()))))); assertTrue(e instanceof ClientErrorException);
resource.update(representation); assertEquals( Response.Status.BAD_REQUEST.getStatusCode(), ClientErrorException.class.cast(e).getResponse().getStatus());
}
oidcConfig.setAuthorizationUrl(null); oidcConfig.setAuthorizationUrl(null);
oidcConfig.setTokenUrl(null); oidcConfig.setTokenUrl(null);
oidcConfig.setJwksUrl(null); oidcConfig.setJwksUrl(null);
oidcConfig.setLogoutUrl(null); oidcConfig.setLogoutUrl(null);
oidcConfig.setUserInfoUrl("http://test"); oidcConfig.setUserInfoUrl("http://localhost");
this.expectedException.expect( try {
Matchers.allOf(Matchers.instanceOf(ClientErrorException.class), Matchers.hasProperty("response", resource.update(representation);
Matchers.hasProperty("status", Matchers.is( fail("Invalid URL");
Response.Status.BAD_REQUEST.getStatusCode()))))); } catch (Exception e) {
assertTrue(e instanceof ClientErrorException);
assertEquals( Response.Status.BAD_REQUEST.getStatusCode(), ClientErrorException.class.cast(e).getResponse().getStatus());
}
rau.updateWith(r -> r.setSslRequired(SslRequired.EXTERNAL.name())).update();
resource.update(representation); resource.update(representation);
} finally {
realmRep.setSslRequired(SslRequired.NONE.name());
realm.update(realmRep);
} }
} }