Skip to content

Commit 744700c

Browse files
strehlestrehle
authored
Fix new SAML. Re-establish SAML setup for legacy (#3164)
* Fix: NEW SAML. Re-establish SAML setup for legacy * Test change, ensure no exception during setup In confuration endpoints we have checks * test coverage * Add doc * Add warning and error for deprecated and legacy settings * test coverage
1 parent 208a340 commit 744700c

File tree

6 files changed

+90
-16
lines changed

6 files changed

+90
-16
lines changed

‎server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,18 @@ public RelyingPartyRegistration findByRegistrationId(String registrationId) {
4040
if (idpDefinition == null) {
4141
idpDefinition = configurator.getIdentityProviderDefinitionsForIssuer(currentZone, registrationId);
4242
}
43-
if (idpDefinition instanceof SamlIdentityProviderDefinition foundSamlIdentityProviderDefinition) {
44-
return createRelyingPartyRegistration(foundSamlIdentityProviderDefinition.getIdpEntityAlias(), foundSamlIdentityProviderDefinition, currentZone);
45-
}
43+
try {
44+
if (idpDefinition instanceof SamlIdentityProviderDefinition foundSamlIdentityProviderDefinition) {
45+
return createRelyingPartyRegistration(foundSamlIdentityProviderDefinition.getIdpEntityAlias(), foundSamlIdentityProviderDefinition, currentZone);
46+
}
4647

47-
for (SamlIdentityProviderDefinition identityProviderDefinition : configurator.getIdentityProviderDefinitionsForZone(currentZone)) {
48-
if (registrationId.equals(identityProviderDefinition.getIdpEntityAlias()) || registrationId.equals(identityProviderDefinition.getIdpEntityId())) {
49-
return createRelyingPartyRegistration(identityProviderDefinition.getIdpEntityAlias(), identityProviderDefinition, currentZone);
48+
for (SamlIdentityProviderDefinition identityProviderDefinition : configurator.getIdentityProviderDefinitionsForZone(currentZone)) {
49+
if (registrationId.equals(identityProviderDefinition.getIdpEntityAlias()) || registrationId.equals(identityProviderDefinition.getIdpEntityId())) {
50+
return createRelyingPartyRegistration(identityProviderDefinition.getIdpEntityAlias(), identityProviderDefinition, currentZone);
51+
}
5052
}
53+
} catch (Exception e) {
54+
// ignore
5155
}
5256

5357
return null;

‎server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlConfigProps.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import lombok.Data;
44
import lombok.extern.slf4j.Slf4j;
55
import org.cloudfoundry.identity.uaa.saml.SamlKey;
6+
import org.springframework.beans.factory.annotation.Value;
67
import org.springframework.boot.context.properties.ConfigurationProperties;
78
import org.springframework.lang.Nullable;
89

@@ -52,6 +53,7 @@ public class SamlConfigProps {
5253

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

@@ -87,6 +89,39 @@ public class SamlConfigProps {
8789
*/
8890
private Boolean disableInResponseToCheck = false;
8991

92+
/**
93+
* Legacy setting: login.saml.serviceProviderKey
94+
*/
95+
private String serviceProviderKey;
96+
97+
/**
98+
* Legacy setting: login.saml.serviceProviderKeyPassword
99+
*/
100+
private String serviceProviderKeyPassword;
101+
102+
/**
103+
* Legacy setting: login.saml.serviceProviderCertificate
104+
*/
105+
private String serviceProviderCertificate;
106+
107+
/**
108+
* Deprecated but sill working: login.serviceProviderKey
109+
*/
110+
@Value("${login.serviceProviderKey:null}")
111+
private String legacyServiceProviderKey;
112+
113+
/**
114+
* Deprecated but sill working: login.serviceProviderKeyPassword
115+
*/
116+
@Value("${login.serviceProviderKeyPassword:null}")
117+
private String legacyServiceProviderKeyPassword;
118+
119+
/**
120+
* Deprecated but sill working: login.serviceProviderCertificate
121+
*/
122+
@Value("${login.serviceProviderCertificate:null}")
123+
private String legacyServiceProviderCertificate;
124+
90125
/**
91126
* Get the active key
92127
* @return the active SamlKey, if available or null

‎server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlKeyManagerFactory.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,21 @@ public List<KeyWithCert> getAvailableCredentials() {
155155
.map(Map.Entry::getValue)
156156
.toList());
157157

158+
if (keyList.isEmpty()) {
159+
SamlKey legacyKey = null;
160+
if (samlConfigProps.getServiceProviderKey() != null && samlConfigProps.getServiceProviderCertificate() != null) {
161+
legacyKey = new SamlKey(samlConfigProps.getServiceProviderKey(), samlConfigProps.getServiceProviderKeyPassword(),
162+
samlConfigProps.getServiceProviderCertificate());
163+
log.warn("SAML should be setup with key map structure.");
164+
} else if (samlConfigProps.getLegacyServiceProviderKey() != null && samlConfigProps.getLegacyServiceProviderCertificate() != null) {
165+
legacyKey = new SamlKey(samlConfigProps.getLegacyServiceProviderKey(), samlConfigProps.getLegacyServiceProviderKeyPassword(),
166+
samlConfigProps.getLegacyServiceProviderCertificate());
167+
log.error("This section is deprecated and will not work in near future anymore.");
168+
}
169+
if (legacyKey != null) {
170+
keyList.add(legacyKey);
171+
}
172+
}
158173
return convertList(keyList);
159174
}
160175

‎server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfig.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import org.springframework.beans.factory.annotation.Value;
99
import org.springframework.context.annotation.Bean;
1010
import org.springframework.context.annotation.Configuration;
11+
import org.springframework.security.saml2.Saml2Exception;
1112
import org.springframework.security.saml2.provider.service.registration.InMemoryRelyingPartyRegistrationRepository;
1213
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration;
1314
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository;
@@ -84,7 +85,11 @@ RelyingPartyRegistrationRepository relyingPartyRegistrationRepository(SamlIdenti
8485
.requestSigned(samlConfigProps.getSignRequest())
8586
.signatureAlgorithms(signatureAlgorithms)
8687
.build();
87-
relyingPartyRegistrations.add(RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration(params));
88+
try {
89+
relyingPartyRegistrations.add(RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration(params));
90+
} catch (Saml2Exception e) {
91+
// ignore
92+
}
8893
}
8994

9095
InMemoryRelyingPartyRegistrationRepository bootstrapRepo = new InMemoryRelyingPartyRegistrationRepository(relyingPartyRegistrations);

‎server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepositoryTest.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import org.springframework.core.io.DefaultResourceLoader;
1515
import org.springframework.core.io.Resource;
1616
import org.springframework.core.io.ResourceLoader;
17-
import org.springframework.security.saml2.Saml2Exception;
1817
import org.springframework.security.saml2.core.Saml2X509Credential;
1918
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration;
2019
import org.springframework.util.FileCopyUtils;
@@ -29,6 +28,7 @@
2928

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

318-
assertThatThrownBy(() -> repository.findByRegistrationId(REGISTRATION_ID))
319-
.isInstanceOf(Saml2Exception.class)
320-
.hasMessageContaining("not_found_metadata.xml");
318+
assertThatNoException().isThrownBy(() -> repository.findByRegistrationId(REGISTRATION_ID));
321319
}
322320

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

334-
assertThatThrownBy(() -> repository.findByRegistrationId(REGISTRATION_ID))
335-
.isInstanceOf(Saml2Exception.class)
336-
.hasMessageContaining("Unsupported element");
332+
assertThatNoException().isThrownBy(() -> repository.findByRegistrationId(REGISTRATION_ID));
337333
}
338334

339335
@Test

‎server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfigTest.java‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package org.cloudfoundry.identity.uaa.provider.saml;
22

33
import org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider;
4+
import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition;
5+
import org.cloudfoundry.identity.uaa.saml.SamlKey;
46
import org.junit.jupiter.api.BeforeAll;
57
import org.junit.jupiter.api.Test;
68
import org.junit.jupiter.api.extension.ExtendWith;
@@ -12,8 +14,10 @@
1214

1315
import java.security.Security;
1416
import java.util.List;
17+
import java.util.Map;
1518

1619
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.mockito.Mockito.when;
1721

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

3741
@Test
3842
void relyingPartyRegistrationRepository() {
43+
when(bootstrapSamlIdentityProviderData.getIdentityProviderDefinitions()).thenReturn(List.of(new SamlIdentityProviderDefinition()));
44+
SamlConfigProps localSamlConfigProps = new SamlConfigProps();
45+
localSamlConfigProps.setActiveKeyId(samlConfigProps.getActiveKeyId());
46+
localSamlConfigProps.setKeys(samlConfigProps.getKeys());
47+
Map<String, SamlKey> samlKeys = localSamlConfigProps.getKeys();
48+
localSamlConfigProps.setKeys(Map.of());
49+
localSamlConfigProps.setLegacyServiceProviderKey(samlKeys.entrySet().stream().findFirst().map(e -> e.getValue().getKey()).orElse(null));
50+
localSamlConfigProps.setLegacyServiceProviderCertificate(samlKeys.entrySet().stream().findFirst().map(e -> e.getValue().getCertificate()).orElse(null));
3951
SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID,
40-
samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, List.of());
52+
localSamlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, List.of());
4153
RelyingPartyRegistrationRepository repository = config.relyingPartyRegistrationRepository(samlIdentityProviderConfigurator);
4254
assertThat(repository).isNotNull();
4355
}
4456

4557
@Test
4658
void relyingPartyRegistrationResolver() {
59+
SamlConfigProps localSamlConfigProps = new SamlConfigProps();
60+
localSamlConfigProps.setActiveKeyId(samlConfigProps.getActiveKeyId());
61+
localSamlConfigProps.setKeys(samlConfigProps.getKeys());
62+
Map<String, SamlKey> samlKeys = localSamlConfigProps.getKeys();
63+
localSamlConfigProps.setKeys(Map.of());
64+
localSamlConfigProps.setServiceProviderKey(samlKeys.entrySet().stream().findFirst().map(e -> e.getValue().getKey()).orElse(null));
65+
localSamlConfigProps.setServiceProviderCertificate(samlKeys.entrySet().stream().findFirst().map(e -> e.getValue().getCertificate()).orElse(null));
4766
SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID,
48-
samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, List.of());
67+
localSamlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, List.of());
4968
RelyingPartyRegistrationRepository repository = config.relyingPartyRegistrationRepository(samlIdentityProviderConfigurator);
5069
RelyingPartyRegistrationResolver resolver = config.relyingPartyRegistrationResolver(repository, ENTITY_ID);
5170

0 commit comments

Comments
 (0)