From be32b111d4e75d85f3db39efa85f4e47b4d5e718 Mon Sep 17 00:00:00 2001 From: Mike Roda Date: Thu, 26 Sep 2024 03:06:57 -0400 Subject: [PATCH] fix: add external saml groups with empty whitelist (#3061) To be consistent with external OIDC logins, we should add external SAML groups if the whitelist is empty. Change-Id: I0981440b1986c6ff107ea39d7e141b6eb6b683e5 Co-authored-by: d036670 --- .../saml/LoginSamlAuthenticationProvider.java | 3 +++ .../saml/LoginSamlAuthenticationProviderTests.java | 14 +++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProvider.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProvider.java index 5092ee78d05..376db3356e1 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProvider.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProvider.java @@ -215,6 +215,9 @@ protected void publish(ApplicationEvent event) { protected Set filterSamlAuthorities(SamlIdentityProviderDefinition definition, Collection samlAuthorities) { List whiteList = of(definition.getExternalGroupsWhitelist()).orElse(Collections.EMPTY_LIST); Set authorities = samlAuthorities.stream().map(s -> s.getAuthority()).collect(Collectors.toSet()); + if (whiteList.isEmpty()) { + return authorities; + } Set result = retainAllMatches(authorities, whiteList); logger.debug(String.format("White listed external SAML groups:'%s'", result)); return result; diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProviderTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProviderTests.java index 2e0ae56e9eb..2f94c9a08ae 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProviderTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/LoginSamlAuthenticationProviderTests.java @@ -139,6 +139,7 @@ class LoginSamlAuthenticationProviderTests { private static final String SAML_ADMIN = "saml.admin"; private static final String SAML_TEST = "saml.test"; private static final String SAML_NOT_MAPPED = "saml.unmapped"; + private static final String SAML_NOT_ASSERTED = "saml.unasserted"; private static final String UAA_USER = "uaa.user"; private static final String UAA_SAML_USER = "uaa.saml.user"; private static final String UAA_SAML_ADMIN = "uaa.saml.admin"; @@ -428,14 +429,25 @@ void test_group_attribute_not_set() { } @Test - void dontAdd_external_groups_to_authentication_without_whitelist() { + void dontAdd_external_groups_to_authentication_without_matching_whitelist() { providerDefinition.addAttributeMapping(GROUP_ATTRIBUTE_NAME, "groups"); + providerDefinition.addWhiteListedGroup(SAML_NOT_ASSERTED); provider.setConfig(providerDefinition); providerProvisioning.update(provider, identityZoneManager.getCurrentIdentityZone().getId()); UaaAuthentication authentication = getAuthentication(authprovider); assertEquals(Collections.EMPTY_SET, authentication.getExternalGroups()); } + + @Test + void add_external_groups_to_authentication_with_empty_whitelist() { + providerDefinition.addAttributeMapping(GROUP_ATTRIBUTE_NAME, "groups"); + provider.setConfig(providerDefinition); + providerProvisioning.update(provider, identityZoneManager.getCurrentIdentityZone().getId()); + + UaaAuthentication authentication = getAuthentication(authprovider); + assertThat(authentication.getExternalGroups(), containsInAnyOrder(SAML_USER, SAML_ADMIN, SAML_NOT_MAPPED)); + } @Test void add_external_groups_to_authentication_with_whitelist() {