From 1c906c834beb8721790ee06fc427dbca946d8477 Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Mon, 18 Mar 2019 13:17:20 +0100 Subject: [PATCH] KEYCLOAK-3373 Remove SAML IdP descriptor from client installation and publicize it in realm endpoint instead --- .../keycloak/protocol/saml/SamlService.java | 24 ++- .../ModAuthMellonClientInstallation.java | 3 +- .../SamlIDPDescriptorClientInstallation.java | 171 ------------------ ...ycloak.protocol.ClientInstallationProvider | 1 - .../admin/client/InstallationTest.java | 9 +- .../messages/admin-messages_en.properties | 4 +- .../resources/partials/realm-detail.html | 6 +- 7 files changed, 29 insertions(+), 189 deletions(-) delete mode 100755 services/src/main/java/org/keycloak/protocol/saml/installation/SamlIDPDescriptorClientInstallation.java diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java index a9e189a49f..ab0ad8c0b1 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java @@ -37,7 +37,6 @@ import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; -import org.keycloak.keys.RsaKeyMetadata; import org.keycloak.models.AuthenticatedClientSessionModel; import org.keycloak.models.ClientModel; import org.keycloak.models.KeyManager; @@ -80,6 +79,9 @@ import java.util.Properties; import java.util.Set; import java.util.TreeSet; import org.keycloak.common.util.StringPropertyReplacer; +import org.keycloak.crypto.Algorithm; +import org.keycloak.crypto.KeyUse; +import org.keycloak.crypto.KeyWrapper; import org.keycloak.dom.saml.v2.metadata.KeyTypes; import org.keycloak.rotation.HardcodedKeyLocator; import org.keycloak.rotation.KeyLocator; @@ -87,6 +89,8 @@ import org.keycloak.saml.SPMetadataDescriptor; import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator; import org.keycloak.saml.validators.DestinationValidator; import org.keycloak.sessions.AuthenticationSessionModel; +import java.nio.charset.StandardCharsets; +import java.util.logging.Level; /** * Resource class for the saml connect token service @@ -590,27 +594,33 @@ public class SamlService extends AuthorizationEndpointBase { } - public static String getIDPMetadataDescriptor(UriInfo uriInfo, KeycloakSession session, RealmModel realm) throws IOException { + public static String getIDPMetadataDescriptor(UriInfo uriInfo, KeycloakSession session, RealmModel realm) { InputStream is = SamlService.class.getResourceAsStream("/idp-metadata-template.xml"); - String template = StreamUtil.readString(is); + String template; + try { + template = StreamUtil.readString(is, StandardCharsets.UTF_8); + } catch (IOException ex) { + logger.error("Cannot generate IdP metadata", ex); + return ""; + } Properties props = new Properties(); props.put("idp.entityID", RealmsResource.realmBaseUrl(uriInfo).build(realm.getName()).toString()); props.put("idp.sso.HTTP-POST", RealmsResource.protocolUrl(uriInfo).build(realm.getName(), SamlProtocol.LOGIN_PROTOCOL).toString()); props.put("idp.sso.HTTP-Redirect", RealmsResource.protocolUrl(uriInfo).build(realm.getName(), SamlProtocol.LOGIN_PROTOCOL).toString()); props.put("idp.sls.HTTP-POST", RealmsResource.protocolUrl(uriInfo).build(realm.getName(), SamlProtocol.LOGIN_PROTOCOL).toString()); StringBuilder keysString = new StringBuilder(); - Set keys = new TreeSet<>((o1, o2) -> o1.getStatus() == o2.getStatus() // Status can be only PASSIVE OR ACTIVE, push PASSIVE to end of list + Set keys = new TreeSet<>((o1, o2) -> o1.getStatus() == o2.getStatus() // Status can be only PASSIVE OR ACTIVE, push PASSIVE to end of list ? (int) (o2.getProviderPriority() - o1.getProviderPriority()) : (o1.getStatus() == KeyStatus.PASSIVE ? 1 : -1)); - keys.addAll(session.keys().getRsaKeys(realm)); - for (RsaKeyMetadata key : keys) { + keys.addAll(session.keys().getKeys(realm, KeyUse.SIG, Algorithm.RS256)); + for (KeyWrapper key : keys) { addKeyInfo(keysString, key, KeyTypes.SIGNING.value()); } props.put("idp.signing.certificates", keysString.toString()); return StringPropertyReplacer.replaceProperties(template, props); } - private static void addKeyInfo(StringBuilder target, RsaKeyMetadata key, String purpose) { + private static void addKeyInfo(StringBuilder target, KeyWrapper key, String purpose) { if (key == null) { return; } diff --git a/services/src/main/java/org/keycloak/protocol/saml/installation/ModAuthMellonClientInstallation.java b/services/src/main/java/org/keycloak/protocol/saml/installation/ModAuthMellonClientInstallation.java index 39bfda056e..9ae8b65789 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/installation/ModAuthMellonClientInstallation.java +++ b/services/src/main/java/org/keycloak/protocol/saml/installation/ModAuthMellonClientInstallation.java @@ -26,6 +26,7 @@ import org.keycloak.protocol.ClientInstallationProvider; import org.keycloak.protocol.saml.SamlClient; import org.keycloak.protocol.saml.SamlProtocol; +import org.keycloak.protocol.saml.SamlService; import javax.ws.rs.core.Response; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -43,7 +44,7 @@ public class ModAuthMellonClientInstallation implements ClientInstallationProvid SamlClient samlClient = new SamlClient(client); ByteArrayOutputStream baos = new ByteArrayOutputStream(); ZipOutputStream zip = new ZipOutputStream(baos); - String idpDescriptor = SamlIDPDescriptorClientInstallation.getIDPDescriptorForClient(session, realm, client, serverBaseUri); + String idpDescriptor = SamlService.getIDPMetadataDescriptor(session.getContext().getUri(), session, realm); String spDescriptor = SamlSPDescriptorClientInstallation.getSPDescriptorForClient(client); String clientDirName = client.getClientId() .replace('/', '_') diff --git a/services/src/main/java/org/keycloak/protocol/saml/installation/SamlIDPDescriptorClientInstallation.java b/services/src/main/java/org/keycloak/protocol/saml/installation/SamlIDPDescriptorClientInstallation.java deleted file mode 100755 index b3b6d6ade7..0000000000 --- a/services/src/main/java/org/keycloak/protocol/saml/installation/SamlIDPDescriptorClientInstallation.java +++ /dev/null @@ -1,171 +0,0 @@ -/* - * Copyright 2016 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.keycloak.protocol.saml.installation; - -import org.keycloak.Config; -import org.keycloak.common.util.PemUtils; -import org.keycloak.crypto.KeyStatus; -import org.keycloak.dom.saml.v2.metadata.KeyTypes; -import org.keycloak.keys.RsaKeyMetadata; -import org.keycloak.models.ClientModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.models.RealmModel; -import org.keycloak.protocol.ClientInstallationProvider; -import org.keycloak.protocol.saml.SamlClient; -import org.keycloak.protocol.saml.SamlProtocol; -import org.keycloak.saml.SPMetadataDescriptor; -import org.keycloak.services.resources.RealmsResource; - -import javax.ws.rs.core.MediaType; -import javax.ws.rs.core.Response; -import javax.ws.rs.core.UriBuilder; -import java.net.URI; -import java.util.Set; -import java.util.TreeSet; - -/** - * @author Bill Burke - * @version $Revision: 1 $ - */ -public class SamlIDPDescriptorClientInstallation implements ClientInstallationProvider { - public static String getIDPDescriptorForClient(KeycloakSession session, RealmModel realm, ClientModel client, URI serverBaseUri) { - SamlClient samlClient = new SamlClient(client); - String idpEntityId = RealmsResource.realmBaseUrl(UriBuilder.fromUri(serverBaseUri)).build(realm.getName()).toString(); - String bindUrl = RealmsResource.protocolUrl(UriBuilder.fromUri(serverBaseUri)).build(realm.getName(), SamlProtocol.LOGIN_PROTOCOL).toString(); - StringBuilder sb = new StringBuilder(); - sb.append("\n" - + "\n" - + " \n"); - - // logout service - sb.append(" \n"); - if (! samlClient.forcePostBinding()) { - sb.append(" \n"); - } - // nameid format - if (samlClient.forceNameIDFormat() && samlClient.getNameIDFormat() != null) { - sb.append(" ").append(samlClient.getNameIDFormat()).append("\n"); - } else { - sb.append(" urn:oasis:names:tc:SAML:2.0:nameid-format:persistent\n" - + " urn:oasis:names:tc:SAML:2.0:nameid-format:transient\n" - + " urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified\n" - + " urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress\n"); - } - // sign on service - sb.append("\n" - + " \n"); - if (! samlClient.forcePostBinding()) { - sb.append(" \n"); - - } - - // keys - Set keys = new TreeSet<>((o1, o2) -> o1.getStatus() == o2.getStatus() // Status can be only PASSIVE OR ACTIVE, push PASSIVE to end of list - ? (int) (o2.getProviderPriority() - o1.getProviderPriority()) - : (o1.getStatus() == KeyStatus.PASSIVE ? 1 : -1)); - keys.addAll(session.keys().getRsaKeys(realm)); - for (RsaKeyMetadata key : keys) { - addKeyInfo(sb, key, KeyTypes.SIGNING.value()); - } - - sb.append(" \n" - + "\n"); - return sb.toString(); - } - - private static void addKeyInfo(StringBuilder target, RsaKeyMetadata key, String purpose) { - if (key == null) { - return; - } - - target.append(SPMetadataDescriptor.xmlKeyInfo(" ", key.getKid(), PemUtils.encodeCertificate(key.getCertificate()), purpose, false)); - } - - @Override - public Response generateInstallation(KeycloakSession session, RealmModel realm, ClientModel client, URI serverBaseUri) { - String descriptor = getIDPDescriptorForClient(session, realm, client, serverBaseUri); - return Response.ok(descriptor, MediaType.TEXT_PLAIN_TYPE).build(); - } - - @Override - public String getProtocol() { - return SamlProtocol.LOGIN_PROTOCOL; - } - - @Override - public String getDisplayType() { - return "SAML Metadata IDPSSODescriptor"; - } - - @Override - public String getHelpText() { - return "SAML Metadata IDPSSODescriptor tailored for the client. This is special because not every client may require things like digital signatures"; - } - - @Override - public String getFilename() { - return "client-tailored-saml-idp-metadata.xml"; - } - - public String getMediaType() { - return MediaType.APPLICATION_XML; - } - - @Override - public boolean isDownloadOnly() { - return false; - } - - @Override - public void close() { - - } - - @Override - public ClientInstallationProvider create(KeycloakSession session) { - return this; - } - - @Override - public void init(Config.Scope config) { - - } - - @Override - public void postInit(KeycloakSessionFactory factory) { - - } - - @Override - public String getId() { - return "saml-idp-descriptor"; - } -} diff --git a/services/src/main/resources/META-INF/services/org.keycloak.protocol.ClientInstallationProvider b/services/src/main/resources/META-INF/services/org.keycloak.protocol.ClientInstallationProvider index f38a5c2366..361c9f2c5f 100755 --- a/services/src/main/resources/META-INF/services/org.keycloak.protocol.ClientInstallationProvider +++ b/services/src/main/resources/META-INF/services/org.keycloak.protocol.ClientInstallationProvider @@ -19,7 +19,6 @@ org.keycloak.protocol.oidc.installation.KeycloakOIDCClientInstallation org.keycloak.protocol.oidc.installation.KeycloakOIDCJbossSubsystemClientInstallation org.keycloak.protocol.saml.installation.KeycloakSamlClientInstallation org.keycloak.protocol.saml.installation.SamlSPDescriptorClientInstallation -org.keycloak.protocol.saml.installation.SamlIDPDescriptorClientInstallation org.keycloak.protocol.saml.installation.ModAuthMellonClientInstallation org.keycloak.protocol.saml.installation.KeycloakSamlSubsystemInstallation org.keycloak.protocol.docker.installation.DockerVariableOverrideInstallationProvider diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java index 5885dd9f11..1caaf4e52c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java @@ -27,6 +27,7 @@ import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.arquillian.AuthServerTestEnricher; import org.keycloak.testsuite.util.AdminEventPaths; +import javax.ws.rs.NotFoundException; import static org.junit.Assert.assertThat; import static org.hamcrest.Matchers.*; @@ -139,13 +140,9 @@ public class InstallationTest extends AbstractClientTest { assertThat(config, containsString(authServerUrl())); } - @Test + @Test(expected = NotFoundException.class) public void testSamlMetadataIdpDescriptor() { - String xml = samlClient.getInstallationProvider("saml-idp-descriptor"); - assertThat(xml, containsString("
- OpenID Endpoint Configuration + {{:: 'realm-detail.protocol-endpoints.oidc' | translate}} + + {{:: 'realm-detail.protocol-endpoints.saml' | translate}}
- {{:: 'realm-detail.oidc-endpoints.tooltip' | translate}} + {{:: 'realm-detail.protocol-endpoints.tooltip' | translate}}