Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix new SAML. Re-establish SAML setup for legacy #3164

Merged
merged 6 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,18 @@ public RelyingPartyRegistration findByRegistrationId(String registrationId) {
if (idpDefinition == null) {
idpDefinition = configurator.getIdentityProviderDefinitionsForIssuer(currentZone, registrationId);
}
if (idpDefinition instanceof SamlIdentityProviderDefinition foundSamlIdentityProviderDefinition) {
return createRelyingPartyRegistration(foundSamlIdentityProviderDefinition.getIdpEntityAlias(), foundSamlIdentityProviderDefinition, currentZone);
}
try {
if (idpDefinition instanceof SamlIdentityProviderDefinition foundSamlIdentityProviderDefinition) {
return createRelyingPartyRegistration(foundSamlIdentityProviderDefinition.getIdpEntityAlias(), foundSamlIdentityProviderDefinition, currentZone);
}

for (SamlIdentityProviderDefinition identityProviderDefinition : configurator.getIdentityProviderDefinitionsForZone(currentZone)) {
if (registrationId.equals(identityProviderDefinition.getIdpEntityAlias()) || registrationId.equals(identityProviderDefinition.getIdpEntityId())) {
return createRelyingPartyRegistration(identityProviderDefinition.getIdpEntityAlias(), identityProviderDefinition, currentZone);
for (SamlIdentityProviderDefinition identityProviderDefinition : configurator.getIdentityProviderDefinitionsForZone(currentZone)) {
if (registrationId.equals(identityProviderDefinition.getIdpEntityAlias()) || registrationId.equals(identityProviderDefinition.getIdpEntityId())) {
return createRelyingPartyRegistration(identityProviderDefinition.getIdpEntityAlias(), identityProviderDefinition, currentZone);
}
}
} catch (Exception e) {
// ignore
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import lombok.Data;
import lombok.extern.slf4j.Slf4j;
import org.cloudfoundry.identity.uaa.saml.SamlKey;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.lang.Nullable;

Expand Down Expand Up @@ -52,6 +53,7 @@ public class SamlConfigProps {

/**
* Map of key IDs to SamlKey objects
* This replaces the deprecated settings from login.* and login.saml.*
*/
private Map<String, SamlKey> keys = new HashMap<>();

Expand Down Expand Up @@ -87,6 +89,39 @@ public class SamlConfigProps {
*/
private Boolean disableInResponseToCheck = false;

/**
* Legacy setting: login.saml.serviceProviderKey
*/
private String serviceProviderKey;

/**
* Legacy setting: login.saml.serviceProviderKeyPassword
*/
private String serviceProviderKeyPassword;

/**
* Legacy setting: login.saml.serviceProviderCertificate
*/
private String serviceProviderCertificate;

/**
* Deprecated but sill working: login.serviceProviderKey
*/
@Value("${login.serviceProviderKey:null}")
private String legacyServiceProviderKey;

/**
* Deprecated but sill working: login.serviceProviderKeyPassword
*/
@Value("${login.serviceProviderKeyPassword:null}")
private String legacyServiceProviderKeyPassword;

/**
* Deprecated but sill working: login.serviceProviderCertificate
*/
@Value("${login.serviceProviderCertificate:null}")
private String legacyServiceProviderCertificate;

/**
* Get the active key
* @return the active SamlKey, if available or null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,21 @@ public List<KeyWithCert> getAvailableCredentials() {
.map(Map.Entry::getValue)
.toList());

if (keyList.isEmpty()) {
SamlKey legacyKey = null;
if (samlConfigProps.getServiceProviderKey() != null && samlConfigProps.getServiceProviderCertificate() != null) {
legacyKey = new SamlKey(samlConfigProps.getServiceProviderKey(), samlConfigProps.getServiceProviderKeyPassword(),
samlConfigProps.getServiceProviderCertificate());
log.warn("SAML should be setup with key map structure.");
} else if (samlConfigProps.getLegacyServiceProviderKey() != null && samlConfigProps.getLegacyServiceProviderCertificate() != null) {
legacyKey = new SamlKey(samlConfigProps.getLegacyServiceProviderKey(), samlConfigProps.getLegacyServiceProviderKeyPassword(),
samlConfigProps.getLegacyServiceProviderCertificate());
log.error("This section is deprecated and will not work in near future anymore.");
}
if (legacyKey != null) {
keyList.add(legacyKey);
}
}
return convertList(keyList);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.saml2.Saml2Exception;
import org.springframework.security.saml2.provider.service.registration.InMemoryRelyingPartyRegistrationRepository;
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration;
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository;
Expand Down Expand Up @@ -84,7 +85,11 @@ RelyingPartyRegistrationRepository relyingPartyRegistrationRepository(SamlIdenti
.requestSigned(samlConfigProps.getSignRequest())
.signatureAlgorithms(signatureAlgorithms)
.build();
relyingPartyRegistrations.add(RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration(params));
try {
relyingPartyRegistrations.add(RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration(params));
} catch (Saml2Exception e) {
// ignore
}
}

InMemoryRelyingPartyRegistrationRepository bootstrapRepo = new InMemoryRelyingPartyRegistrationRepository(relyingPartyRegistrations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.springframework.core.io.DefaultResourceLoader;
import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader;
import org.springframework.security.saml2.Saml2Exception;
import org.springframework.security.saml2.core.Saml2X509Credential;
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration;
import org.springframework.util.FileCopyUtils;
Expand All @@ -29,6 +28,7 @@

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.cloudfoundry.identity.uaa.provider.saml.TestCredentialObjects.keyName1;
import static org.cloudfoundry.identity.uaa.provider.saml.TestCredentialObjects.keyName2;
Expand Down Expand Up @@ -315,9 +315,7 @@ void failsWhenInvalidMetadataLocationIsStored() {
when(definition.getMetaDataLocation()).thenReturn("not_found_metadata.xml");
when(configurator.getIdentityProviderDefinitionsForZone(identityZone)).thenReturn(List.of(definition));

assertThatThrownBy(() -> repository.findByRegistrationId(REGISTRATION_ID))
.isInstanceOf(Saml2Exception.class)
.hasMessageContaining("not_found_metadata.xml");
assertThatNoException().isThrownBy(() -> repository.findByRegistrationId(REGISTRATION_ID));
}

@Test
Expand All @@ -331,9 +329,7 @@ void failsWhenInvalidMetadataXmlIsStored() {
when(definition.getMetaDataLocation()).thenReturn("<?xml version=\"1.0\"?>\n<xml>invalid xml</xml>");
when(configurator.getIdentityProviderDefinitionsForZone(identityZone)).thenReturn(List.of(definition));

assertThatThrownBy(() -> repository.findByRegistrationId(REGISTRATION_ID))
.isInstanceOf(Saml2Exception.class)
.hasMessageContaining("Unsupported element");
assertThatNoException().isThrownBy(() -> repository.findByRegistrationId(REGISTRATION_ID));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.cloudfoundry.identity.uaa.provider.saml;

import org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider;
import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition;
import org.cloudfoundry.identity.uaa.saml.SamlKey;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand All @@ -12,8 +14,10 @@

import java.security.Security;
import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
class SamlRelyingPartyRegistrationRepositoryConfigTest {
Expand All @@ -36,16 +40,31 @@ public static void beforeAll() {

@Test
void relyingPartyRegistrationRepository() {
when(bootstrapSamlIdentityProviderData.getIdentityProviderDefinitions()).thenReturn(List.of(new SamlIdentityProviderDefinition()));
SamlConfigProps localSamlConfigProps = new SamlConfigProps();
localSamlConfigProps.setActiveKeyId(samlConfigProps.getActiveKeyId());
localSamlConfigProps.setKeys(samlConfigProps.getKeys());
Map<String, SamlKey> samlKeys = localSamlConfigProps.getKeys();
localSamlConfigProps.setKeys(Map.of());
localSamlConfigProps.setLegacyServiceProviderKey(samlKeys.entrySet().stream().findFirst().map(e -> e.getValue().getKey()).orElse(null));
localSamlConfigProps.setLegacyServiceProviderCertificate(samlKeys.entrySet().stream().findFirst().map(e -> e.getValue().getCertificate()).orElse(null));
SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID,
samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, List.of());
localSamlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, List.of());
RelyingPartyRegistrationRepository repository = config.relyingPartyRegistrationRepository(samlIdentityProviderConfigurator);
assertThat(repository).isNotNull();
}

@Test
void relyingPartyRegistrationResolver() {
SamlConfigProps localSamlConfigProps = new SamlConfigProps();
localSamlConfigProps.setActiveKeyId(samlConfigProps.getActiveKeyId());
localSamlConfigProps.setKeys(samlConfigProps.getKeys());
Map<String, SamlKey> samlKeys = localSamlConfigProps.getKeys();
localSamlConfigProps.setKeys(Map.of());
localSamlConfigProps.setServiceProviderKey(samlKeys.entrySet().stream().findFirst().map(e -> e.getValue().getKey()).orElse(null));
localSamlConfigProps.setServiceProviderCertificate(samlKeys.entrySet().stream().findFirst().map(e -> e.getValue().getCertificate()).orElse(null));
SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID,
samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, List.of());
localSamlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, List.of());
RelyingPartyRegistrationRepository repository = config.relyingPartyRegistrationRepository(samlIdentityProviderConfigurator);
RelyingPartyRegistrationResolver resolver = config.relyingPartyRegistrationResolver(repository, ENTITY_ID);

Expand Down
Loading