Skip to content

Commit

Permalink
feat: Handle Multiple SAML keys
Browse files Browse the repository at this point in the history 
- Rotation Tests working
- Uses keys from SamlConfig for each zone
- Fall back to default keys if none set

[#187994938]

Signed-off-by: Duane May <duane.may@broadcom.com>
  • Loading branch information
@duanemay
duanemay committed Jul 25, 2024
1 parent 5ee57e1 commit ae14c2f
Show file tree
Hide file tree
Showing 26 changed files with 976 additions and 528 deletions.
40 changes: 6 additions & 34 deletions model/src/main/java/org/cloudfoundry/identity/uaa/saml/SamlKey.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,45 +17,17 @@

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;

@Data
@AllArgsConstructor
@NoArgsConstructor
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
public class SamlKey {

private String key;
private String passphrase;
private String certificate;

public SamlKey() {
}

public SamlKey(String key, String passphrase, String certificate) {
this.key = key;
this.passphrase = passphrase;
this.certificate = certificate;
}

public String getKey() {
return key;
}

public void setKey(String key) {
this.key = key;
}

public String getPassphrase() {
return passphrase;
}

public void setPassphrase(String passphrase) {
this.passphrase = passphrase;
}

public String getCertificate() {
return certificate;
}

public void setCertificate(String certificate) {
this.certificate = certificate;
}
}
90 changes: 54 additions & 36 deletions model/src/main/java/org/cloudfoundry/identity/uaa/zone/SamlConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
import lombok.Data;
import org.cloudfoundry.identity.uaa.saml.SamlKey;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import static org.springframework.util.StringUtils.hasText;

Expand Down Expand Up @@ -55,81 +58,96 @@ public void setEntityID(String entityID) {

@JsonProperty("certificate")
public void setCertificate(String certificate) {
SamlKey legacyKey = keys.get(LEGACY_KEY_ID);
if (hasText(certificate) && null == legacyKey) {
legacyKey = new SamlKey();
}
if (legacyKey != null) {
legacyKey.setCertificate(certificate);
keys.put(LEGACY_KEY_ID, legacyKey);
if (hasText(certificate)) {
keys.computeIfAbsent(LEGACY_KEY_ID, k -> new SamlKey());
}
keys.computeIfPresent(LEGACY_KEY_ID, (k, v) -> {
v.setCertificate(certificate);
return v;
});
}

@JsonProperty("privateKey")
public void setPrivateKey(String privateKey) {
SamlKey legacyKey = keys.get(LEGACY_KEY_ID);
if (hasText(privateKey) && null == legacyKey) {
legacyKey = new SamlKey();
}
if (legacyKey != null) {
legacyKey.setKey(privateKey);
keys.put(LEGACY_KEY_ID, legacyKey);
if (hasText(privateKey)) {
keys.computeIfAbsent(LEGACY_KEY_ID, k -> new SamlKey());
}
keys.computeIfPresent(LEGACY_KEY_ID, (k, v) -> {
v.setKey(privateKey);
return v;
});
}

@JsonProperty("privateKeyPassword")
public void setPrivateKeyPassword(String privateKeyPassword) {
SamlKey legacyKey = keys.get(LEGACY_KEY_ID);
if (hasText(privateKeyPassword) && null == legacyKey) {
legacyKey = new SamlKey();
}
if (legacyKey != null) {
legacyKey.setPassphrase(privateKeyPassword);
keys.put(LEGACY_KEY_ID, legacyKey);
if (hasText(privateKeyPassword)) {
keys.computeIfAbsent(LEGACY_KEY_ID, k -> new SamlKey());
}
keys.computeIfPresent(LEGACY_KEY_ID, (k, v) -> {
v.setPassphrase(privateKeyPassword);
return v;
});
}

@JsonProperty("certificate")
public String getCertificate() {
SamlKey legacyKey = keys.get(LEGACY_KEY_ID);
if (null != legacyKey) {
return legacyKey.getCertificate();
}
return null;
return Optional.ofNullable(keys.get(LEGACY_KEY_ID))
.map(SamlKey::getCertificate)
.orElse(null);
}

@JsonProperty
public String getPrivateKey() {
SamlKey legacyKey = keys.get(LEGACY_KEY_ID);
if (null != legacyKey) {
return legacyKey.getKey();
}
return null;
return Optional.ofNullable(keys.get(LEGACY_KEY_ID))
.map(SamlKey::getKey)
.orElse(null);
}

@JsonProperty
public String getPrivateKeyPassword() {
SamlKey legacyKey = keys.get(LEGACY_KEY_ID);
if (null != legacyKey) {
return legacyKey.getPassphrase();
}
return null;
return Optional.ofNullable(keys.get(LEGACY_KEY_ID))
.map(SamlKey::getPassphrase)
.orElse(null);
}

public String getActiveKeyId() {
return hasText(activeKeyId) ? activeKeyId : hasLegacyKey() ? LEGACY_KEY_ID : null;
}

@JsonIgnore
public SamlKey getActiveKey() {
String keyId = getActiveKeyId();
return keyId != null ? keys.get(keyId) : null;
}

public void setActiveKeyId(String activeKeyId) {
if (!LEGACY_KEY_ID.equals(activeKeyId)) {
this.activeKeyId = activeKeyId;
}
}

/**
* @return a map of all keys by keyName
*/
public Map<String, SamlKey> getKeys() {
return Collections.unmodifiableMap(keys);
}

/**
* @return the list of keys, with the active key first.
*/
@JsonIgnore
public List<SamlKey> getKeyList() {
List<SamlKey> keyList = new ArrayList<>();
String activeKeyId = getActiveKeyId();
Optional.ofNullable(getActiveKey()).ifPresent(keyList::add);
keyList.addAll(keys.entrySet().stream()
.filter(e -> !e.getKey().equals(activeKeyId))
.map(Map.Entry::getValue)
.toList());
return Collections.unmodifiableList(keyList);
}

public void setKeys(Map<String, SamlKey> keys) {
this.keys = new HashMap<>(keys);
}
Expand Down
112 changes: 93 additions & 19 deletions model/src/test/java/org/cloudfoundry/identity/uaa/zone/SamlConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.junit.jupiter.api.Test;

import java.util.Collections;
import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -104,7 +105,7 @@ void legacy_key_is_part_of_map() {
config.setPrivateKeyPassword(passphrase);
config.setCertificate(certificate);
Map<String, SamlKey> keys = config.getKeys();
assertThat(keys).hasSize(1).containsKey(LEGACY_KEY_ID);
assertThat(keys).containsOnlyKeys(LEGACY_KEY_ID);
assertThat(keys.get(LEGACY_KEY_ID).getKey()).isEqualTo(privateKey);
assertThat(keys.get(LEGACY_KEY_ID).getPassphrase()).isEqualTo(passphrase);
assertThat(keys.get(LEGACY_KEY_ID).getCertificate()).isEqualTo(certificate);
Expand All @@ -116,12 +117,14 @@ void addActiveKey() {
String keyId = "testKeyId";
config.addAndActivateKey(keyId, key);
Map<String, SamlKey> keys = config.getKeys();
assertThat(keys).hasSize(1);
assertThat(keys).hasSize(1)
.containsKey(keyId);
assertThat(config.getActiveKeyId()).isEqualTo(keyId);
assertThat(keys).containsKey(keyId);
assertThat(keys.get(keyId).getKey()).isEqualTo(privateKey);
assertThat(keys.get(keyId).getPassphrase()).isEqualTo(passphrase);
assertThat(keys.get(keyId).getCertificate()).isEqualTo(certificate);
assertThat(keys.get(keyId)).returns(privateKey, SamlKey::getKey)
.returns(passphrase, SamlKey::getPassphrase)
.returns(certificate, SamlKey::getCertificate);
assertThat(config.getActiveKey()).isSameAs(keys.get(keyId));
assertThat(config.getKeyList()).hasSize(1).containsExactly(key);
}

@Test
Expand All @@ -131,12 +134,44 @@ void addNonActive() {
String keyId = "nonActiveKeyId";
config.addKey(keyId, key);
Map<String, SamlKey> keys = config.getKeys();
assertThat(keys).hasSize(2);
assertThat(keys).hasSize(2)
.containsKey(keyId);
assertThat(config.getActiveKeyId()).isNotEqualTo(keyId);
assertThat(keys).containsKey(keyId);
assertThat(keys.get(keyId).getKey()).isEqualTo(privateKey);
assertThat(keys.get(keyId).getPassphrase()).isEqualTo(passphrase);
assertThat(keys.get(keyId).getCertificate()).isEqualTo(certificate);
assertThat(keys.get(keyId)).returns(privateKey, SamlKey::getKey)
.returns(passphrase, SamlKey::getPassphrase)
.returns(certificate, SamlKey::getCertificate);
}

@Test
void getKeyList() {
// Default is empty
assertThat(config.getKeyList()).isEmpty();

// Add active key, should only have that key
addActiveKey();
SamlKey activeKey = config.getActiveKey();
assertThat(config.getKeyList()).containsExactly(activeKey);

// Add another key, should have both keys
SamlKey nonActiveKey = new SamlKey(privateKey, passphrase, certificate);
String nonActiveKeyId = "nonActiveKeyId";
config.addKey(nonActiveKeyId, nonActiveKey);
assertThat(config.getKeyList()).containsExactly(activeKey, nonActiveKey);

// add another active key, should have the new key first
SamlKey otherActiveKey = new SamlKey(privateKey, passphrase, certificate);
config.addAndActivateKey("anotherActiveKeyId", otherActiveKey);
assertThat(config.getKeyList()).hasSize(3).first().isSameAs(otherActiveKey);

// remove the non-active key, should have other 2 keys
config.removeKey(nonActiveKeyId);
assertThat(config.getKeyList()).containsExactly(otherActiveKey, activeKey);

// drop the current active key, should have only the remaining key... even though it is not active
config.removeKey("anotherActiveKeyId");
assertThat(config.getActiveKey()).isNull();
assertThat(config.getKeys()).hasSize(1);
assertThat(config.getKeyList()).containsExactly(activeKey);
}

@Test
Expand All @@ -153,19 +188,56 @@ void testIsWantAssertionSigned() {

@Test
void testSetKeyAndCert() {
// Default values are null
assertThat(config).returns(null, SamlConfig::getPrivateKey)
.returns(null, SamlConfig::getPrivateKeyPassword)
.returns(null, SamlConfig::getCertificate)
.extracting(SamlConfig::getActiveKey)
.isNull();

// Set values to null, does not create a key
config.setPrivateKey(null);
config.setPrivateKeyPassword(null);
config.setCertificate(null);
assertThat(config).returns(null, SamlConfig::getPrivateKey)
.returns(null, SamlConfig::getPrivateKeyPassword)
.returns(null, SamlConfig::getCertificate)
.extracting(SamlConfig::getActiveKey)
.isNull();

// Set values to non-null, creates a key object
config.setPrivateKey(privateKey);
config.setPrivateKeyPassword(passphrase);
config.setCertificate(certificate);
assertThat(config.getPrivateKey()).isEqualTo(privateKey);
assertThat(config.getPrivateKeyPassword()).isEqualTo(passphrase);
assertThat(config).returns(privateKey, SamlConfig::getPrivateKey)
.returns(passphrase, SamlConfig::getPrivateKeyPassword)
.returns(certificate, SamlConfig::getCertificate)
.extracting(SamlConfig::getActiveKey)
.isNotNull()
.returns(privateKey, SamlKey::getKey)
.returns(certificate, SamlKey::getCertificate)
.returns(passphrase, SamlKey::getPassphrase);

// Set values to null, retains the key object with nulls
config.setPrivateKey(null);
config.setPrivateKeyPassword(null);
config.setCertificate(null);
assertThat(config).returns(null, SamlConfig::getPrivateKey)
.returns(null, SamlConfig::getPrivateKeyPassword)
.returns(null, SamlConfig::getCertificate)
.extracting(SamlConfig::getActiveKey)
.isNotNull()
.returns(null, SamlKey::getKey)
.returns(null, SamlKey::getCertificate)
.returns(null, SamlKey::getPassphrase);
}

@Test
void read_old_json_works() {
read_json(oldJson);
assertThat(config.getPrivateKey()).isEqualTo(privateKey);
assertThat(config.getPrivateKeyPassword()).isEqualTo(passphrase);
assertThat(config.getCertificate()).isEqualTo(certificate);
assertThat(config).returns(privateKey, SamlConfig::getPrivateKey)
.returns(passphrase, SamlConfig::getPrivateKeyPassword)
.returns(certificate, SamlConfig::getCertificate);
}

public void read_json(String json) {
Expand All @@ -177,9 +249,9 @@ void to_json_ignores_legacy_values() {
read_json(oldJson);
String json = JsonUtils.writeValueAsString(config);
read_json(json);
assertThat(config.getPrivateKey()).isEqualTo(privateKey);
assertThat(config.getPrivateKeyPassword()).isEqualTo(passphrase);
assertThat(config.getCertificate()).isEqualTo(certificate);
assertThat(config).returns(privateKey, SamlConfig::getPrivateKey)
.returns(passphrase, SamlConfig::getPrivateKeyPassword)
.returns(certificate, SamlConfig::getCertificate);
}

@Test
Expand All @@ -193,8 +265,10 @@ void can_clear_keys() {
read_json(oldJson);
assertThat(config.getKeys()).hasSize(1);
assertThat(config.getActiveKeyId()).isNotNull();
assertThat(config.getActiveKey()).isNotNull();
config.setKeys(Collections.emptyMap());
assertThat(config.getKeys()).isEmpty();
assertThat(config.getActiveKeyId()).isNull();
assertThat(config.getActiveKey()).isNull();
}
}
Loading

0 comments on commit ae14c2f

Please sign in to comment.