From 214dc88141cc23d48a951fd22eec28d638ad9b5f Mon Sep 17 00:00:00 2001 From: D023954 Date: Tue, 10 Oct 2023 15:32:50 +0200 Subject: [PATCH 01/36] fix sonar issue: Utility classes should not have public constructors --- .../org/cloudfoundry/identity/uaa/metrics/MetricsUtil.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/metrics-data/src/main/java/org/cloudfoundry/identity/uaa/metrics/MetricsUtil.java b/metrics-data/src/main/java/org/cloudfoundry/identity/uaa/metrics/MetricsUtil.java index c88b8ffcbe6..0e1016a514a 100644 --- a/metrics-data/src/main/java/org/cloudfoundry/identity/uaa/metrics/MetricsUtil.java +++ b/metrics-data/src/main/java/org/cloudfoundry/identity/uaa/metrics/MetricsUtil.java @@ -18,6 +18,11 @@ public class MetricsUtil { public static final String GLOBAL_GROUP = "uaa.global.metrics"; + // Utility classes should not have public constructors + private MetricsUtil() { + throw new IllegalStateException("Utility class"); + } + public static double addAverages(double oldCount, double oldAverage, double newCount, From 931f880cdb63d5a595c4732e911d499ea3aa2a24 Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 18 Oct 2023 10:08:16 +0200 Subject: [PATCH 02/36] fix: possibly unnecessary method call --- .../uaa/scim/jdbc/JdbcScimGroupMembershipManager.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java index 63db5bdfa6d..32affc32d1a 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java @@ -118,10 +118,9 @@ private Set getDefaultUserGroups(String zoneId) { return emptySet(); } IdentityZone currentZone = IdentityZoneHolder.get(); - List zoneDefaultGroups = currentZone.getConfig().getUserConfig().getDefaultGroups(); - if (!zoneId.equals(currentZone.getId())) { - zoneDefaultGroups = zoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().getDefaultGroups(); - } + List zoneDefaultGroups = (zoneId.equals(currentZone.getId())) ? + currentZone.getConfig().getUserConfig().getDefaultGroups() : + zoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().getDefaultGroups(); return zoneDefaultGroups .stream() .map(groupName -> createOrGetGroup(groupName, zoneId)) From 57bc4722bbcb54fc95d7f7a2299b48220f5400f3 Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 18 Oct 2023 10:52:10 +0200 Subject: [PATCH 03/36] fix: define Java version --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 3f958cafd8a..53db391735e 100644 --- a/build.gradle +++ b/build.gradle @@ -81,8 +81,8 @@ subprojects { [compileJava, compileTestJava]*.options*.compilerArgs = ["-Xlint:none", "-nowarn"] - sourceCompatibility = 1.11 - targetCompatibility = 1.11 + sourceCompatibility = JavaVersion.VERSION_11 + targetCompatibility = JavaVersion.VERSION_11 test { maxParallelForks = 1 From 0352bb666943d9a0550af21619416c2d6873ab10 Mon Sep 17 00:00:00 2001 From: Klaus Kiefer Date: Fri, 20 Oct 2023 10:56:20 +0200 Subject: [PATCH 04/36] handle in separate PR --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 53db391735e..3f958cafd8a 100644 --- a/build.gradle +++ b/build.gradle @@ -81,8 +81,8 @@ subprojects { [compileJava, compileTestJava]*.options*.compilerArgs = ["-Xlint:none", "-nowarn"] - sourceCompatibility = JavaVersion.VERSION_11 - targetCompatibility = JavaVersion.VERSION_11 + sourceCompatibility = 1.11 + targetCompatibility = 1.11 test { maxParallelForks = 1 From 9a34f0acc140ab43c2ff4439d539c3be951841d4 Mon Sep 17 00:00:00 2001 From: D023954 Date: Mon, 30 Oct 2023 15:36:56 +0100 Subject: [PATCH 05/36] feature: add attribute userConfig.allowedGroups to IdZ --- .../identity/uaa/zone/UserConfig.java | 10 +++ .../identity/uaa/zone/IdentityZoneTest.java | 16 +++- .../identity/uaa/zone/SampleIdentityZone.json | 76 ++++++++----------- .../IdentityZoneConfigurationBootstrap.java | 9 +++ .../oauth/UaaAuthorizationRequestManager.java | 4 + .../scim/jdbc/JdbcScimGroupProvisioning.java | 31 ++++++++ .../uaa/user/JdbcUaaUserDatabase.java | 4 + ...ralIdentityZoneConfigurationValidator.java | 2 + .../uaa/zone/UserConfigValidator.java | 27 +++++++ ...entityZoneConfigurationBootstrapTests.java | 10 +++ .../IdentityZoneConfigurationTests.java | 2 + .../identity/uaa/oauth/TokenTestSupport.java | 3 + .../UaaAuthorizationRequestManagerTests.java | 13 ++++ .../LoginSamlAuthenticationProviderTests.java | 7 +- .../jdbc/JdbcScimGroupProvisioningTests.java | 25 ++++++ .../uaa/user/JdbcUaaUserDatabaseTests.java | 1 + .../uaa/zone/UserConfigValidatorTest.java | 35 +++++++++ .../mock/zones/IdentityZoneEndpointDocs.java | 5 ++ .../IdentityZoneEndpointsMockMvcTests.java | 20 +++++ 19 files changed, 251 insertions(+), 49 deletions(-) create mode 100644 server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java create mode 100644 server/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidatorTest.java diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java index f3fdb393105..b157b0c41ef 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java @@ -27,4 +27,14 @@ public List getDefaultGroups() { public void setDefaultGroups(List defaultGroups) { this.defaultGroups = defaultGroups; } + + private List allowedGroups = null; + + public List getAllowedGroups() { + return allowedGroups; + } + + public void setAllowedGroups(List allowedGroups) { + this.allowedGroups = allowedGroups; + } } diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java index 8b4db207c7c..151135f0254 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java @@ -10,11 +10,13 @@ import java.util.Calendar; import java.util.Date; +import java.util.List; import java.util.stream.Stream; import static org.cloudfoundry.identity.uaa.test.ModelTestUtils.getResourceAsString; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; class IdentityZoneTest { @@ -124,8 +126,16 @@ void hashCode_usesOnlyId(IdentityZone zone, int expectedHashCode) { @Test void deserialize() { - final String sampleIdentityZone = getResourceAsString(getClass(), "SampleIdentityZone.json"); - - JsonUtils.readValue(sampleIdentityZone, IdentityZone.class); + final String sampleIdentityZoneJson = getResourceAsString(getClass(), "SampleIdentityZone.json"); + IdentityZone sampleIdentityZone = JsonUtils.readValue(sampleIdentityZoneJson, IdentityZone.class); + assertEquals("f7758816-ab47-48d9-9d24-25b10b92d4cc", sampleIdentityZone.getId()); + assertEquals("demo", sampleIdentityZone.getSubdomain()); + assertEquals(List.of("openid", "password.write", "uaa.user", "approvals.me", + "profile", "roles", "user_attributes", "uaa.offline_token"), + sampleIdentityZone.getConfig().getUserConfig().getDefaultGroups()); + assertEquals(List.of("openid", "password.write", "uaa.user", "approvals.me", + "profile", "roles", "user_attributes", "uaa.offline_token", + "scim.me", "cloud_controller.user"), + sampleIdentityZone.getConfig().getUserConfig().getAllowedGroups()); } } \ No newline at end of file diff --git a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json index 0aa5600263c..4181fd7205d 100644 --- a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json +++ b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json @@ -17,11 +17,12 @@ "refreshTokenUnique": false, "refreshTokenFormat": "jwt", "activeKeyId": "key-id-1", - "keys" : { - "key-id-1":{ - "signingKey":"some-signing-key-1", - "signingCert":"some-cert", - "signingAlg":"RS256"} + "keys": { + "key-id-1": { + "signingKey": "some-signing-key-1", + "signingCert": "some-cert", + "signingAlg": "RS256" + } } }, "samlConfig": { @@ -30,52 +31,27 @@ "wantAssertionSigned": true, "wantAuthnRequestSigned": false, "assertionTimeToLiveSeconds": 600, - "keys": { - }, + "keys": {}, "disableInResponseToCheck": true }, "corsPolicy": { "xhrConfiguration": { - "allowedOrigins": [ - ".*" - ], - "allowedOriginPatterns": [ - ], - "allowedUris": [ - ".*" - ], - "allowedUriPatterns": [ - ], - "allowedHeaders": [ - "Accept", - "Authorization", - "Content-Type" - ], - "allowedMethods": [ - "GET" - ], + "allowedOrigins": [".*"], + "allowedOriginPatterns": [], + "allowedUris": [".*"], + "allowedUriPatterns": [], + "allowedHeaders": ["Accept", "Authorization", "Content-Type"], + "allowedMethods": ["GET"], "allowedCredentials": false, "maxAge": 1728000 }, "defaultConfiguration": { - "allowedOrigins": [ - ".*" - ], - "allowedOriginPatterns": [ - ], - "allowedUris": [ - ".*" - ], - "allowedUriPatterns": [ - ], - "allowedHeaders": [ - "Accept", - "Authorization", - "Content-Type" - ], - "allowedMethods": [ - "GET" - ], + "allowedOrigins": [".*"], + "allowedOriginPatterns": [], + "allowedUris": [".*"], + "allowedUriPatterns": [], + "allowedHeaders": ["Accept", "Authorization", "Content-Type"], + "allowedMethods": ["GET"], "allowedCredentials": false, "maxAge": 1728000 } @@ -122,6 +98,18 @@ "roles", "user_attributes", "uaa.offline_token" + ], + "allowedGroups": [ + "openid", + "password.write", + "uaa.user", + "approvals.me", + "profile", + "roles", + "user_attributes", + "uaa.offline_token", + "scim.me", + "cloud_controller.user" ] } }, @@ -130,4 +118,4 @@ "description": "{\"plan_display_name\":\"Demo\",\"plan_description\":\"Demo SSO Plan\"}", "created": 1503504273000, "last_modified": 1504898224000 -} \ No newline at end of file +} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java index 645aefed2b0..dc7a96b73ee 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java @@ -67,6 +67,8 @@ public class IdentityZoneConfigurationBootstrap implements InitializingBean { private Collection defaultUserGroups; + private Collection allowedUserGroups; + private IdentityZoneValidator validator = (config, mode) -> config; private Map branding; @@ -133,6 +135,9 @@ public void afterPropertiesSet() throws InvalidIdentityZoneDetailsException { definition.getUserConfig().setDefaultGroups(new LinkedList<>(defaultUserGroups)); } + if (allowedUserGroups!=null) { + definition.getUserConfig().setAllowedGroups(new LinkedList<>(allowedUserGroups)); + } identityZone.setConfig(definition); @@ -254,6 +259,10 @@ public void setDefaultUserGroups(Collection defaultUserGroups) { this.defaultUserGroups = defaultUserGroups; } + public void setAllowedUserGroups(Collection allowedUserGroups) { + this.allowedUserGroups = allowedUserGroups; + } + public boolean isDisableSamlInResponseToCheck() { return disableSamlInResponseToCheck; } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java index 0859400ca64..81a782dca58 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java @@ -249,7 +249,11 @@ private Set checkUserScopes(Set requestedScopes, Set allowed = new LinkedHashSet<>(AuthorityUtils.authorityListToSet(authorities)); // Add in all default requestedScopes Collection defaultScopes = IdentityZoneHolder.get().getConfig().getUserConfig().getDefaultGroups(); + Collection allowedScopes = IdentityZoneHolder.get().getConfig().getUserConfig().getAllowedGroups(); allowed.addAll(defaultScopes); + if (allowedScopes != null) { + allowed.retainAll(allowedScopes); + } // Find intersection of user authorities, default requestedScopes and client requestedScopes: Set result = intersectScopes(new LinkedHashSet<>(requestedScopes), clientDetails.getScope(), allowed); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index 7bdd806fa7d..f052d4a9874 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -13,6 +13,9 @@ import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceNotFoundException; import org.cloudfoundry.identity.uaa.util.beans.DbUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; +import org.cloudfoundry.identity.uaa.zone.JdbcIdentityZoneProvisioning; +import org.cloudfoundry.identity.uaa.zone.ZoneDoesNotExistsException; import org.cloudfoundry.identity.uaa.zone.event.IdentityZoneModifiedEvent; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -66,6 +69,7 @@ public Logger getLogger() { private JdbcScimGroupExternalMembershipManager jdbcScimGroupExternalMembershipManager; private JdbcScimGroupMembershipManager jdbcScimGroupMembershipManager; + private JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning; public JdbcScimGroupProvisioning( final JdbcTemplate jdbcTemplate, @@ -74,6 +78,7 @@ public JdbcScimGroupProvisioning( super(jdbcTemplate, pagingListFactory, new ScimGroupRowMapper()); this.jdbcTemplate = jdbcTemplate; + jdbcIdentityZoneProvisioning = new JdbcIdentityZoneProvisioning(jdbcTemplate); // TODO inject final String quotedGroupsTableName = dbUtils.getQuotedIdentifier(GROUP_TABLE, jdbcTemplate); updateGroupSql = String.format( @@ -222,8 +227,29 @@ public ScimGroup retrieve(String id, final String zoneId) throws ScimResourceNot } } + private List getAllowedUserGroups(String zoneId) { + List zoneAllowedGroups = null; // default: all groups allowed + if (!hasText(zoneId)) { + return zoneAllowedGroups; + } + IdentityZone currentZone = IdentityZoneHolder.get(); + try { + zoneAllowedGroups = (zoneId.equals(currentZone.getId())) ? + currentZone.getConfig().getUserConfig().getAllowedGroups() : + jdbcIdentityZoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().getAllowedGroups(); + } catch (ZoneDoesNotExistsException e) { + logger.debug("could not retrieve identity zone with id: " + zoneId); + } + return zoneAllowedGroups; + } + @Override public ScimGroup create(final ScimGroup group, final String zoneId) throws InvalidScimResourceException { + List allowedGroups = getAllowedUserGroups(zoneId); + if ((allowedGroups != null) && (!allowedGroups.contains(group.getDisplayName()))) { + throw new InvalidScimResourceException("The group with displayName: " + group.getDisplayName() + + " is not allowed in Identity Zone " + zoneId); + } final String id = UUID.randomUUID().toString(); logger.debug("creating new group with id: " + id); try { @@ -248,6 +274,11 @@ public ScimGroup create(final ScimGroup group, final String zoneId) throws Inval @Override public ScimGroup update(final String id, final ScimGroup group, final String zoneId) throws InvalidScimResourceException, ScimResourceNotFoundException { + List allowedGroups = getAllowedUserGroups(zoneId); + if ((allowedGroups != null) && (!allowedGroups.contains(group.getDisplayName()))) { + throw new InvalidScimResourceException("The group with displayName: " + group.getDisplayName() + + " is not allowed in Identity Zone " + zoneId); + } try { validateGroup(group); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabase.java b/server/src/main/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabase.java index 28e0caba27c..82f49699d84 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabase.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabase.java @@ -265,6 +265,10 @@ private String getAuthorities(final String userId) throws SQLException { Set authorities = new HashSet<>(); getAuthorities(authorities, Collections.singletonList(userId)); authorities.addAll(identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().getDefaultGroups()); + List allowedGroups = identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().getAllowedGroups(); + if (allowedGroups != null) { + authorities.retainAll(allowedGroups); + } return StringUtils.collectionToCommaDelimitedString(new HashSet<>(authorities)); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/GeneralIdentityZoneConfigurationValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/GeneralIdentityZoneConfigurationValidator.java index 70f430b7e7f..12535ae4295 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/GeneralIdentityZoneConfigurationValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/GeneralIdentityZoneConfigurationValidator.java @@ -74,6 +74,8 @@ public IdentityZoneConfiguration validate(IdentityZone zone, IdentityZoneValidat validateRegexStrings(config.getCorsPolicy().getXhrConfiguration().getAllowedOrigins(), "config.corsPolicy.xhrConfiguration.allowedOrigins"); validateRegexStrings(config.getCorsPolicy().getDefaultConfiguration().getAllowedUris(), "config.corsPolicy.defaultConfiguration.allowedUris"); validateRegexStrings(config.getCorsPolicy().getDefaultConfiguration().getAllowedOrigins(), "config.corsPolicy.defaultConfiguration.allowedOrigins"); + + UserConfigValidator.validate(config.getUserConfig()); } if (config.getBranding() != null && config.getBranding().getConsent() != null) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java new file mode 100644 index 00000000000..5095cbde733 --- /dev/null +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java @@ -0,0 +1,27 @@ +package org.cloudfoundry.identity.uaa.zone; + +import java.util.List; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class UserConfigValidator { + private static Logger logger = LoggerFactory.getLogger(UserConfigValidator.class); + + public static void validate(UserConfig config) throws InvalidIdentityZoneConfigurationException { + List defaultGroups = (config == null) ? null : config.getDefaultGroups(); + List allowedGroups = (config == null) ? null : config.getAllowedGroups(); + if (allowedGroups != null) { + if (allowedGroups.isEmpty()) { + String message = "At least one group must be allowed"; + logger.error(message); + throw new InvalidIdentityZoneConfigurationException(message); + } + if ((defaultGroups == null) || (!allowedGroups.containsAll(defaultGroups))) { + String message = "All default groups must be allowed"; + logger.error(message); + throw new InvalidIdentityZoneConfigurationException(message); + } + } + } +} diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java index 4d67bdfe45b..98e9119f3d9 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java @@ -192,6 +192,16 @@ public void testDefaultGroups() throws Exception { assertThat(uaa.getConfig().getUserConfig().getDefaultGroups(), containsInAnyOrder(groups)); } + @Test + public void testAllowedGroups() throws Exception { + String[] groups = {"group1", "group2", "group3"}; + bootstrap.setDefaultUserGroups(Arrays.asList(groups)); + bootstrap.setAllowedUserGroups(Arrays.asList(groups)); + bootstrap.afterPropertiesSet(); + IdentityZone uaa = provisioning.retrieve(IdentityZone.getUaaZoneId()); + assertThat(uaa.getConfig().getUserConfig().getAllowedGroups(), containsInAnyOrder(groups)); + } + @Test public void tokenPolicy_configured_fromValuesInYaml() throws Exception { TokenPolicy tokenPolicy = new TokenPolicy(); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationTests.java index 323dcb54bed..cd425874727 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationTests.java @@ -40,6 +40,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.springframework.http.HttpHeaders.ACCEPT; @@ -74,6 +75,7 @@ public void default_user_groups_when_json_is_deserialized() { "user_attributes", "uaa.offline_token" )); + assertNull(definition.getUserConfig().getAllowedGroups()); s = JsonUtils.writeValueAsString(definition); assertThat(s, containsString("userConfig")); assertThat(s, containsString("uaa.offline_token")); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/TokenTestSupport.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/TokenTestSupport.java index d69c3614584..950aaf2f29a 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/TokenTestSupport.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/TokenTestSupport.java @@ -306,6 +306,9 @@ public TokenTestSupport(UaaTokenEnhancer tokenEnhancer, KeyInfoService keyInfo) IdentityZoneHolder.get().getConfig().getUserConfig().setDefaultGroups( new LinkedList<>(AuthorityUtils.authorityListToSet(USER_AUTHORITIES)) ); + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups( + new LinkedList<>(AuthorityUtils.authorityListToSet(USER_AUTHORITIES)) + ); } public UaaTokenServices getUaaTokenServices() { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java index 5a8b76c69ed..3159c5fd0cc 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java @@ -148,6 +148,7 @@ void test_user_token_request() { parameters.put("expires_in", "44000"); parameters.put(OAuth2Utils.GRANT_TYPE, TokenConstants.GRANT_TYPE_USER_TOKEN); IdentityZoneHolder.get().getConfig().getUserConfig().setDefaultGroups(Collections.singletonList("uaa.user")); + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); // all groups allowed client.setScope(StringUtils.commaDelimitedListToSet("aud1.test,aud2.test,uaa.user")); when(clientDetailsService.loadClientByClientId(recipient.getClientId(), "uaa")).thenReturn(recipient); ReflectionTestUtils.setField(factory, "uaaUserDatabase", null); @@ -185,10 +186,22 @@ void testWildcardScopesIncludesAuthoritiesForUser() { factory.validateParameters(request.getRequestParameters(), client); } + @Test + void testWildcardScopesIncludesAllowedAuthoritiesForUser() { + when(mockSecurityContextAccessor.isUser()).thenReturn(true); + when(mockSecurityContextAccessor.getAuthorities()).thenReturn((Collection)AuthorityUtils.commaSeparatedStringToAuthorityList("space.1.developer,space.2.developer,space.1.admin")); + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(Arrays.asList("openid","space.1.developer")); + client.setScope(StringUtils.commaDelimitedListToSet("space.*.developer")); + AuthorizationRequest request = factory.createAuthorizationRequest(parameters); + assertEquals(StringUtils.commaDelimitedListToSet("space.1.developer"), new TreeSet(request.getScope())); + factory.validateParameters(request.getRequestParameters(), client); + } + @Test void testOpenidScopeIncludeIsAResourceId() { parameters.put("scope", "openid foo.bar"); IdentityZoneHolder.get().getConfig().getUserConfig().setDefaultGroups(Collections.singletonList("openid")); + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(Arrays.asList("openid","foo.bar")); client.setScope(StringUtils.commaDelimitedListToSet("openid,foo.bar")); AuthorizationRequest request = factory.createAuthorizationRequest(parameters); assertEquals(StringUtils.commaDelimitedListToSet("openid,foo.bar"), new TreeSet(request.getScope())); 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 b93ae0eb322..c4c196227ef 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 @@ -134,6 +134,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 UAA_USER = "uaa.user"; private static final String UAA_SAML_USER = "uaa.saml.user"; private static final String UAA_SAML_ADMIN = "uaa.saml.admin"; private static final String UAA_SAML_TEST = "uaa.saml.test"; @@ -178,8 +179,10 @@ void configureProvider() throws SAMLException, SecurityException, DecryptionExce ScimGroupProvisioning groupProvisioning = new JdbcScimGroupProvisioning( jdbcTemplate, new JdbcPagingListFactory(jdbcTemplate, limitSqlAdapter), dbUtils); - identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().setDefaultGroups(Collections.singletonList("uaa.user")); - groupProvisioning.createOrGet(new ScimGroup(null, "uaa.user", identityZoneManager.getCurrentIdentityZone().getId()), identityZoneManager.getCurrentIdentityZone().getId()); + identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().setDefaultGroups(Collections.singletonList(UAA_USER)); + identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().setAllowedGroups(Arrays.asList(UAA_USER, SAML_USER, + SAML_ADMIN,SAML_TEST,SAML_NOT_MAPPED, UAA_SAML_USER,UAA_SAML_ADMIN,UAA_SAML_TEST)); + groupProvisioning.createOrGet(new ScimGroup(null, UAA_USER, identityZoneManager.getCurrentIdentityZone().getId()), identityZoneManager.getCurrentIdentityZone().getId()); providerDefinition = new SamlIdentityProviderDefinition(); userProvisioning = new JdbcScimUserProvisioning(jdbcTemplate, new JdbcPagingListFactory(jdbcTemplate, limitSqlAdapter), passwordEncoder); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java index 63b860c47a7..442145ed9ef 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java @@ -7,6 +7,7 @@ import org.cloudfoundry.identity.uaa.scim.ScimGroupMember; import org.cloudfoundry.identity.uaa.scim.ScimUser; import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning; +import org.cloudfoundry.identity.uaa.scim.exception.InvalidScimResourceException; import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceNotFoundException; import org.cloudfoundry.identity.uaa.scim.test.TestUtils; import org.cloudfoundry.identity.uaa.util.beans.DbUtils; @@ -85,6 +86,7 @@ void initJdbcScimGroupProvisioningTests() throws SQLException { zone.setId(zoneId); IdentityZoneHolder.set(zone); IdentityZoneHolder.get().getConfig().getUserConfig().setDefaultGroups(new ArrayList<>()); + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); validateGroupCountInZone(0, zoneId); @@ -292,6 +294,29 @@ void canCreateAndGetGroupWithQuotes() { assertEquals(g.getId(), same.getId()); } + @Test + void cannotCreateNotAllowedGroup() { + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(Arrays.asList("allowedGroup")); + assertThrowsWithMessageThat( + InvalidScimResourceException.class, + () -> internalCreateGroup("notAllowedGroup"), + containsString("is not allowed") + ); + } + + @Test + void cannotUpdateNotAllowedGroup() { + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(Arrays.asList("allowedGroup")); + ScimGroup g = dao.retrieve(g1Id, zoneId); + g.setDisplayName("notAllowedGroup"); + g.setDescription("description-update"); + try { + dao.update(g1Id, g, zoneId); + fail(); + } catch(InvalidScimResourceException e) { + } + } + @Test void canUpdateGroup() { ScimGroup g = dao.retrieve(g1Id, zoneId); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java index 11a07316da2..618a1a83f75 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java @@ -116,6 +116,7 @@ private static void setUpIdentityZone(IdentityZoneManager mockIdentityZoneManage when(mockIdentityZone.getConfig()).thenReturn(mockIdentityZoneConfiguration); when(mockIdentityZoneConfiguration.getUserConfig()).thenReturn(mockUserConfig); when(mockUserConfig.getDefaultGroups()).thenReturn(UserConfig.DEFAULT_ZONE_GROUPS); + when(mockUserConfig.getAllowedGroups()).thenReturn(null); // allow all groups } @AfterEach diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidatorTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidatorTest.java new file mode 100644 index 00000000000..f3711c5a0d5 --- /dev/null +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidatorTest.java @@ -0,0 +1,35 @@ +package org.cloudfoundry.identity.uaa.zone; + +import org.junit.Test; + +import java.util.Collections; +import java.util.List; + + +public class UserConfigValidatorTest { + + @Test + public void testDefaultConfig() throws InvalidIdentityZoneConfigurationException { + UserConfigValidator.validate(new UserConfig()); + } + + @Test + public void testNullConfig() throws InvalidIdentityZoneConfigurationException { + UserConfigValidator.validate(null); + } + + @Test(expected = InvalidIdentityZoneConfigurationException.class) + public void testAllowedGroupsEmpty() throws InvalidIdentityZoneConfigurationException { + UserConfig userConfig = new UserConfig(); + userConfig.setAllowedGroups(Collections.emptyList()); + UserConfigValidator.validate(userConfig); + } + + @Test(expected = InvalidIdentityZoneConfigurationException.class) + public void testDefaultGroupsNotAllowed() throws InvalidIdentityZoneConfigurationException { + UserConfig userConfig = new UserConfig(); + userConfig.setDefaultGroups(List.of("openid","uaa.user")); + userConfig.setAllowedGroups(List.of("uaa.user")); // openid not allowed + UserConfigValidator.validate(userConfig); + } +} \ No newline at end of file diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java index f58d568267f..dce2f5b991f 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java @@ -156,6 +156,7 @@ class IdentityZoneEndpointDocs extends EndpointDocs { private static final String SAML_ACTIVE_KEY_ID_DESC = "The ID of the key that should be used for signing metadata and assertions."; private static final String DEFAULT_ZONE_GROUPS_DESC = "Default groups each user in the zone inherits."; + private static final String ALLOWED_ZONE_GROUPS_DESC = "Allowed groups in the zone. Defaults to null (all groups allowed)"; private static final String SERVICE_PROVIDER_ID = "cloudfoundry-saml-login"; private static final String MFA_CONFIG_ENABLED_DESC = "Set `true` to enable Multi-factor Authentication (MFA) for the current zone. Defaults to `false`"; private static final String MFA_CONFIG_PROVIDER_NAME_DESC = "The unique `name` of the MFA provider to use for this zone."; @@ -307,6 +308,7 @@ void createIdentityZone() throws Exception { fieldWithPath("config.corsPolicy.defaultConfiguration.maxAge").description(CORS_XHR_MAXAGE_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.userConfig.defaultGroups").description(DEFAULT_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), + fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.mfaConfig.enabled").description(MFA_CONFIG_ENABLED_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.mfaConfig.providerName").description(MFA_CONFIG_PROVIDER_NAME_DESC).attributes(key("constraints").value("Required when `config.mfaConfig.enabled` is `true`")).optional().type(STRING), @@ -470,6 +472,7 @@ void getAllIdentityZones() throws Exception { fieldWithPath("[].config.corsPolicy.defaultConfiguration.maxAge").optional().description(CORS_XHR_MAXAGE_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("[].config.userConfig.defaultGroups").description(DEFAULT_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), + fieldWithPath("[].config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("[].config.mfaConfig.enabled").optional().description(MFA_CONFIG_ENABLED_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("[].config.mfaConfig.providerName").optional().description(MFA_CONFIG_PROVIDER_NAME_DESC).attributes(key("constraints").value("Required when `config.mfaConfig.enabled` is `true`")).optional().type(STRING), @@ -617,6 +620,7 @@ void updateIdentityZone() throws Exception { fieldWithPath("config.corsPolicy.defaultConfiguration.maxAge").description(CORS_XHR_MAXAGE_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.userConfig.defaultGroups").description(DEFAULT_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), + fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.mfaConfig.enabled").description(MFA_CONFIG_ENABLED_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.mfaConfig.providerName").description(MFA_CONFIG_PROVIDER_NAME_DESC).attributes(key("constraints").value("Required when `config.mfaConfig.enabled` is `true`")).optional().type(STRING), @@ -801,6 +805,7 @@ private Snippet getResponseFields() { fieldWithPath("config.corsPolicy.xhrConfiguration.maxAge").description(CORS_XHR_MAXAGE_DESC), fieldWithPath("config.userConfig.defaultGroups").description(DEFAULT_ZONE_GROUPS_DESC), + fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC), fieldWithPath("config.mfaConfig.enabled").description(MFA_CONFIG_ENABLED_DESC), fieldWithPath("config.mfaConfig.providerName").description(MFA_CONFIG_PROVIDER_NAME_DESC).optional().type(STRING), diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java index 6ef61f3a73e..d2850a0b041 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java @@ -507,6 +507,26 @@ void createZoneWithNoSubdomainFailsWithUnprocessableEntity() throws Exception { assertEquals(0, zoneModifiedEventListener.getEventCount()); } + @Test + void createZoneWithNotAllowedGroupsFailsWithUnprocessableEntity() throws Exception { + String id = generator.generate(); + IdentityZone zone = this.createSimpleIdentityZone(id); + zone.getConfig().getUserConfig().setDefaultGroups(List.of("uaa.user", "openid")); + zone.getConfig().getUserConfig().setAllowedGroups(List.of("uaa.user")); // openid not allowed + + mockMvc.perform( + post("/identity-zones") + .header("Authorization", "Bearer " + identityClientToken) + .contentType(APPLICATION_JSON) + .content(JsonUtils.writeValueAsString(zone))) + .andExpect(status().isUnprocessableEntity()) + .andExpect(jsonPath("$.error").value("invalid_identity_zone")) + .andExpect(jsonPath("$.error_description").value("The identity zone details are invalid. " + + "The zone configuration is invalid. All default groups must be allowed")); + + assertEquals(0, zoneModifiedEventListener.getEventCount()); + } + @Test void testCreateZoneInsufficientScope() throws Exception { String id = new RandomValueStringGenerator().generate(); From 7d2fb2dd6b5740cfdcecf11f3c4d98a4698edf7b Mon Sep 17 00:00:00 2001 From: Markus Strehle <11627201+strehle@users.noreply.github.com> Date: Sat, 18 Nov 2023 19:04:48 +0100 Subject: [PATCH 06/36] revert format changes --- .../identity/uaa/zone/SampleIdentityZone.json | 62 +++++++++++++------ 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json index 4181fd7205d..ff4789e1c0b 100644 --- a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json +++ b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json @@ -17,12 +17,11 @@ "refreshTokenUnique": false, "refreshTokenFormat": "jwt", "activeKeyId": "key-id-1", - "keys": { - "key-id-1": { - "signingKey": "some-signing-key-1", - "signingCert": "some-cert", - "signingAlg": "RS256" - } + "keys" : { + "key-id-1":{ + "signingKey":"some-signing-key-1", + "signingCert":"some-cert", + "signingAlg":"RS256"} } }, "samlConfig": { @@ -31,27 +30,52 @@ "wantAssertionSigned": true, "wantAuthnRequestSigned": false, "assertionTimeToLiveSeconds": 600, - "keys": {}, + "keys": { + }, "disableInResponseToCheck": true }, "corsPolicy": { "xhrConfiguration": { - "allowedOrigins": [".*"], - "allowedOriginPatterns": [], - "allowedUris": [".*"], - "allowedUriPatterns": [], - "allowedHeaders": ["Accept", "Authorization", "Content-Type"], - "allowedMethods": ["GET"], + "allowedOrigins": [ + ".*" + ], + "allowedOriginPatterns": [ + ], + "allowedUris": [ + ".*" + ], + "allowedUriPatterns": [ + ], + "allowedHeaders": [ + "Accept", + "Authorization", + "Content-Type" + ], + "allowedMethods": [ + "GET" + ], "allowedCredentials": false, "maxAge": 1728000 }, "defaultConfiguration": { - "allowedOrigins": [".*"], - "allowedOriginPatterns": [], - "allowedUris": [".*"], - "allowedUriPatterns": [], - "allowedHeaders": ["Accept", "Authorization", "Content-Type"], - "allowedMethods": ["GET"], + "allowedOrigins": [ + ".*" + ], + "allowedOriginPatterns": [ + ], + "allowedUris": [ + ".*" + ], + "allowedUriPatterns": [ + ], + "allowedHeaders": [ + "Accept", + "Authorization", + "Content-Type" + ], + "allowedMethods": [ + "GET" + ], "allowedCredentials": false, "maxAge": 1728000 } From d4c52f4d60030b5c7fc390c4001278265a8a3018 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 18 Nov 2023 19:03:23 +0100 Subject: [PATCH 07/36] revert format changes --- .../org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json index ff4789e1c0b..8f7e5dd7bed 100644 --- a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json +++ b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json @@ -142,4 +142,4 @@ "description": "{\"plan_display_name\":\"Demo\",\"plan_description\":\"Demo SSO Plan\"}", "created": 1503504273000, "last_modified": 1504898224000 -} +} \ No newline at end of file From ec56ea66ddd24ef014b8d8d727692ebb2c9c3c73 Mon Sep 17 00:00:00 2001 From: D023954 Date: Mon, 20 Nov 2023 10:27:32 +0100 Subject: [PATCH 08/36] fix Sonar warnings --- .../scim/jdbc/JdbcScimGroupProvisioning.java | 4 +- .../uaa/zone/UserConfigValidator.java | 4 ++ ...entityZoneConfigurationBootstrapTests.java | 42 +++++++++---------- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index f052d4a9874..66ec1c1bf79 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -238,7 +238,7 @@ private List getAllowedUserGroups(String zoneId) { currentZone.getConfig().getUserConfig().getAllowedGroups() : jdbcIdentityZoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().getAllowedGroups(); } catch (ZoneDoesNotExistsException e) { - logger.debug("could not retrieve identity zone with id: " + zoneId); + logger.debug("could not retrieve identity zone with id: %s", zoneId); } return zoneAllowedGroups; } @@ -251,7 +251,7 @@ public ScimGroup create(final ScimGroup group, final String zoneId) throws Inval + " is not allowed in Identity Zone " + zoneId); } final String id = UUID.randomUUID().toString(); - logger.debug("creating new group with id: " + id); + logger.debug("creating new group with id: %s", id); try { validateGroup(group); jdbcTemplate.update(addGroupSql, ps -> { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java index 5095cbde733..c52284bea18 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java @@ -8,6 +8,10 @@ public class UserConfigValidator { private static Logger logger = LoggerFactory.getLogger(UserConfigValidator.class); + // add a private constructor to hide the implicit public one + private UserConfigValidator() { + } + public static void validate(UserConfig config) throws InvalidIdentityZoneConfigurationException { List defaultGroups = (config == null) ? null : config.getDefaultGroups(); List allowedGroups = (config == null) ? null : config.getAllowedGroups(); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java index 98e9119f3d9..babb22ff9af 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java @@ -64,14 +64,14 @@ public class IdentityZoneConfigurationBootstrapTests { "wTKZHjWybPHsW2q8Z6Moz5dvE+XMd11c5NtIG2/L97I=\n" + "-----END RSA PRIVATE KEY-----"; - public static final String ID = "id"; + private static final String ID = "id"; private IdentityZoneProvisioning provisioning; private IdentityZoneConfigurationBootstrap bootstrap; private Map links = new HashMap<>(); private GeneralIdentityZoneValidator validator; @BeforeEach - public void configureProvisioning(@Autowired JdbcTemplate jdbcTemplate) throws SQLException { + void configureProvisioning(@Autowired JdbcTemplate jdbcTemplate) throws SQLException { TestUtils.cleanAndSeedDb(jdbcTemplate); provisioning = new JdbcIdentityZoneProvisioning(jdbcTemplate); bootstrap = new IdentityZoneConfigurationBootstrap(provisioning); @@ -98,7 +98,7 @@ public void configureProvisioning(@Autowired JdbcTemplate jdbcTemplate) throws S } @Test - public void testClientSecretPolicy() throws Exception { + void testClientSecretPolicy() throws Exception { bootstrap.setClientSecretPolicy(new ClientSecretPolicy(0, 255, 0, 1, 1, 1, 6)); bootstrap.afterPropertiesSet(); IdentityZone uaa = provisioning.retrieve(IdentityZone.getUaaZoneId()); @@ -112,7 +112,7 @@ public void testClientSecretPolicy() throws Exception { } @Test - public void test_multiple_keys() throws InvalidIdentityZoneDetailsException { + void test_multiple_keys() throws InvalidIdentityZoneDetailsException { bootstrap.setSamlSpPrivateKey(SamlTestUtils.PROVIDER_PRIVATE_KEY); bootstrap.setSamlSpCertificate(SamlTestUtils.PROVIDER_CERTIFICATE); bootstrap.setSamlSpPrivateKeyPassphrase(SamlTestUtils.PROVIDER_PRIVATE_KEY_PASSWORD); @@ -156,7 +156,7 @@ void test_keyId_null_exception() { } @Test - public void testDefaultSamlKeys() throws Exception { + void testDefaultSamlKeys() throws Exception { bootstrap.setSamlSpPrivateKey(SamlTestUtils.PROVIDER_PRIVATE_KEY); bootstrap.setSamlSpCertificate(SamlTestUtils.PROVIDER_CERTIFICATE); bootstrap.setSamlSpPrivateKeyPassphrase(SamlTestUtils.PROVIDER_PRIVATE_KEY_PASSWORD); @@ -168,7 +168,7 @@ public void testDefaultSamlKeys() throws Exception { } @Test - public void enable_in_response_to() throws Exception { + void enable_in_response_to() throws Exception { bootstrap.setDisableSamlInResponseToCheck(false); bootstrap.afterPropertiesSet(); IdentityZone uaa = provisioning.retrieve(IdentityZone.getUaaZoneId()); @@ -176,7 +176,7 @@ public void enable_in_response_to() throws Exception { } @Test - public void saml_disable_in_response_to() throws Exception { + void saml_disable_in_response_to() throws Exception { bootstrap.setDisableSamlInResponseToCheck(true); bootstrap.afterPropertiesSet(); IdentityZone uaa = provisioning.retrieve(IdentityZone.getUaaZoneId()); @@ -184,7 +184,7 @@ public void saml_disable_in_response_to() throws Exception { } @Test - public void testDefaultGroups() throws Exception { + void testDefaultGroups() throws Exception { String[] groups = {"group1", "group2", "group3"}; bootstrap.setDefaultUserGroups(Arrays.asList(groups)); bootstrap.afterPropertiesSet(); @@ -193,7 +193,7 @@ public void testDefaultGroups() throws Exception { } @Test - public void testAllowedGroups() throws Exception { + void testAllowedGroups() throws Exception { String[] groups = {"group1", "group2", "group3"}; bootstrap.setDefaultUserGroups(Arrays.asList(groups)); bootstrap.setAllowedUserGroups(Arrays.asList(groups)); @@ -203,7 +203,7 @@ public void testAllowedGroups() throws Exception { } @Test - public void tokenPolicy_configured_fromValuesInYaml() throws Exception { + void tokenPolicy_configured_fromValuesInYaml() throws Exception { TokenPolicy tokenPolicy = new TokenPolicy(); Map keys = new HashMap<>(); keys.put(ID, PRIVATE_KEY); @@ -224,7 +224,7 @@ public void tokenPolicy_configured_fromValuesInYaml() throws Exception { } @Test - public void disable_self_service_links() throws Exception { + void disable_self_service_links() throws Exception { bootstrap.setSelfServiceLinksEnabled(false); bootstrap.afterPropertiesSet(); @@ -233,7 +233,7 @@ public void disable_self_service_links() throws Exception { } @Test - public void set_home_redirect() throws Exception { + void set_home_redirect() throws Exception { bootstrap.setHomeRedirect("http://some.redirect.com/redirect"); bootstrap.afterPropertiesSet(); @@ -242,7 +242,7 @@ public void set_home_redirect() throws Exception { } @Test - public void signup_link_configured() throws Exception { + void signup_link_configured() throws Exception { links.put("signup", "/configured_signup"); bootstrap.setSelfServiceLinks(links); bootstrap.afterPropertiesSet(); @@ -253,7 +253,7 @@ public void signup_link_configured() throws Exception { } @Test - public void passwd_link_configured() throws Exception { + void passwd_link_configured() throws Exception { links.put("passwd", "/configured_passwd"); bootstrap.setSelfServiceLinks(links); bootstrap.afterPropertiesSet(); @@ -264,7 +264,7 @@ public void passwd_link_configured() throws Exception { } @Test - public void test_logout_redirect() throws Exception { + void test_logout_redirect() throws Exception { bootstrap.setLogoutDefaultRedirectUrl("/configured_login"); bootstrap.setLogoutDisableRedirectParameter(false); bootstrap.setLogoutRedirectParameterName("test"); @@ -279,7 +279,7 @@ public void test_logout_redirect() throws Exception { @Test - public void test_prompts() throws Exception { + void test_prompts() throws Exception { List prompts = Arrays.asList( new Prompt("name1", "type1", "text1"), new Prompt("name2", "type2", "text2") @@ -291,7 +291,7 @@ public void test_prompts() throws Exception { } @Test - public void idpDiscoveryEnabled() throws Exception { + void idpDiscoveryEnabled() throws Exception { bootstrap.setIdpDiscoveryEnabled(true); bootstrap.afterPropertiesSet(); IdentityZoneConfiguration config = provisioning.retrieve(IdentityZone.getUaaZoneId()).getConfig(); @@ -299,18 +299,18 @@ public void idpDiscoveryEnabled() throws Exception { } @Test - public void testMfaDisabledByDefault() { + void testMfaDisabledByDefault() { assertFalse(bootstrap.isMfaEnabled()); } @Test - public void testMfaDisabledWithInvalidName() throws Exception { + void testMfaDisabledWithInvalidName() throws Exception { bootstrap.setMfaProviderName("NotExistingProvider"); assertThrows(InvalidIdentityZoneDetailsException.class, () -> bootstrap.afterPropertiesSet()); } @Test - public void testMfaEnabledValidName() throws Exception { + void testMfaEnabledValidName() throws Exception { bootstrap.setMfaProviderName("testProvider"); bootstrap.setMfaEnabled(true); bootstrap.afterPropertiesSet(); @@ -320,7 +320,7 @@ public void testMfaEnabledValidName() throws Exception { } @Test - public void testMfaEnabledInvalidName() throws Exception { + void testMfaEnabledInvalidName() throws Exception { bootstrap.setMfaProviderName("InvalidProvider"); bootstrap.setMfaEnabled(true); assertThrows(InvalidIdentityZoneDetailsException.class, () -> bootstrap.afterPropertiesSet()); From ef8f6ca5aa6a89fdbc501506dec3c09fc582e800 Mon Sep 17 00:00:00 2001 From: D023954 Date: Mon, 20 Nov 2023 14:03:33 +0100 Subject: [PATCH 09/36] remove TODO --- .../identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index 66ec1c1bf79..91cb7f25d28 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -69,7 +69,6 @@ public Logger getLogger() { private JdbcScimGroupExternalMembershipManager jdbcScimGroupExternalMembershipManager; private JdbcScimGroupMembershipManager jdbcScimGroupMembershipManager; - private JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning; public JdbcScimGroupProvisioning( final JdbcTemplate jdbcTemplate, @@ -78,7 +77,6 @@ public JdbcScimGroupProvisioning( super(jdbcTemplate, pagingListFactory, new ScimGroupRowMapper()); this.jdbcTemplate = jdbcTemplate; - jdbcIdentityZoneProvisioning = new JdbcIdentityZoneProvisioning(jdbcTemplate); // TODO inject final String quotedGroupsTableName = dbUtils.getQuotedIdentifier(GROUP_TABLE, jdbcTemplate); updateGroupSql = String.format( @@ -233,6 +231,7 @@ private List getAllowedUserGroups(String zoneId) { return zoneAllowedGroups; } IdentityZone currentZone = IdentityZoneHolder.get(); + JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning = new JdbcIdentityZoneProvisioning(jdbcTemplate); try { zoneAllowedGroups = (zoneId.equals(currentZone.getId())) ? currentZone.getConfig().getUserConfig().getAllowedGroups() : From 9dd1e380baa97b1ca8333e014937b6764d4eb336 Mon Sep 17 00:00:00 2001 From: d036670 Date: Mon, 20 Nov 2023 16:43:54 +0100 Subject: [PATCH 10/36] create new needed bean similar than the other beans created --- .../identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java | 6 +++++- .../uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java | 2 ++ uaa/src/main/webapp/WEB-INF/spring/scim-endpoints.xml | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index 91cb7f25d28..2308885fd85 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -69,6 +69,7 @@ public Logger getLogger() { private JdbcScimGroupExternalMembershipManager jdbcScimGroupExternalMembershipManager; private JdbcScimGroupMembershipManager jdbcScimGroupMembershipManager; + private JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning; public JdbcScimGroupProvisioning( final JdbcTemplate jdbcTemplate, @@ -156,6 +157,10 @@ public void setJdbcScimGroupMembershipManager(final JdbcScimGroupMembershipManag this.jdbcScimGroupMembershipManager = jdbcScimGroupMembershipManager; } + public void setJdbcIdentityZoneProvisioning(JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning) { + this.jdbcIdentityZoneProvisioning = jdbcIdentityZoneProvisioning; + } + void createAndIgnoreDuplicate(final String name, final String zoneId) { try { create(new ScimGroup(null, name, zoneId), zoneId); @@ -231,7 +236,6 @@ private List getAllowedUserGroups(String zoneId) { return zoneAllowedGroups; } IdentityZone currentZone = IdentityZoneHolder.get(); - JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning = new JdbcIdentityZoneProvisioning(jdbcTemplate); try { zoneAllowedGroups = (zoneId.equals(currentZone.getId())) ? currentZone.getConfig().getUserConfig().getAllowedGroups() : diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java index 442145ed9ef..316bd82e5b0 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java @@ -14,6 +14,7 @@ import org.cloudfoundry.identity.uaa.util.TimeServiceImpl; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; +import org.cloudfoundry.identity.uaa.zone.JdbcIdentityZoneProvisioning; import org.cloudfoundry.identity.uaa.zone.MultitenancyFixture; import org.cloudfoundry.identity.uaa.zone.ZoneManagementScopes; import org.cloudfoundry.identity.uaa.zone.event.IdentityZoneModifiedEvent; @@ -106,6 +107,7 @@ void initJdbcScimGroupProvisioningTests() throws SQLException { new JdbcScimGroupExternalMembershipManager(jdbcTemplate, dbUtils); jdbcScimGroupExternalMembershipManager.setScimGroupProvisioning(dao); dao.setJdbcScimGroupExternalMembershipManager(jdbcScimGroupExternalMembershipManager); + dao.setJdbcIdentityZoneProvisioning(new JdbcIdentityZoneProvisioning(jdbcTemplate)); g1Id = "g1"; g2Id = "g2"; diff --git a/uaa/src/main/webapp/WEB-INF/spring/scim-endpoints.xml b/uaa/src/main/webapp/WEB-INF/spring/scim-endpoints.xml index 7014e090cea..90626f42280 100644 --- a/uaa/src/main/webapp/WEB-INF/spring/scim-endpoints.xml +++ b/uaa/src/main/webapp/WEB-INF/spring/scim-endpoints.xml @@ -112,6 +112,7 @@ + Date: Mon, 20 Nov 2023 16:44:12 +0100 Subject: [PATCH 11/36] is an optional entry in identity zone configuration --- .../identity/uaa/mock/zones/IdentityZoneEndpointDocs.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java index dce2f5b991f..7b512fd4e0d 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointDocs.java @@ -308,7 +308,7 @@ void createIdentityZone() throws Exception { fieldWithPath("config.corsPolicy.defaultConfiguration.maxAge").description(CORS_XHR_MAXAGE_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.userConfig.defaultGroups").description(DEFAULT_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), - fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), + fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("config.mfaConfig.enabled").description(MFA_CONFIG_ENABLED_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.mfaConfig.providerName").description(MFA_CONFIG_PROVIDER_NAME_DESC).attributes(key("constraints").value("Required when `config.mfaConfig.enabled` is `true`")).optional().type(STRING), @@ -472,7 +472,7 @@ void getAllIdentityZones() throws Exception { fieldWithPath("[].config.corsPolicy.defaultConfiguration.maxAge").optional().description(CORS_XHR_MAXAGE_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("[].config.userConfig.defaultGroups").description(DEFAULT_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), - fieldWithPath("[].config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), + fieldWithPath("[].config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("[].config.mfaConfig.enabled").optional().description(MFA_CONFIG_ENABLED_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("[].config.mfaConfig.providerName").optional().description(MFA_CONFIG_PROVIDER_NAME_DESC).attributes(key("constraints").value("Required when `config.mfaConfig.enabled` is `true`")).optional().type(STRING), @@ -620,7 +620,7 @@ void updateIdentityZone() throws Exception { fieldWithPath("config.corsPolicy.defaultConfiguration.maxAge").description(CORS_XHR_MAXAGE_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.userConfig.defaultGroups").description(DEFAULT_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), - fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")), + fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).attributes(key("constraints").value("Optional")).optional().type(ARRAY), fieldWithPath("config.mfaConfig.enabled").description(MFA_CONFIG_ENABLED_DESC).attributes(key("constraints").value("Optional")), fieldWithPath("config.mfaConfig.providerName").description(MFA_CONFIG_PROVIDER_NAME_DESC).attributes(key("constraints").value("Required when `config.mfaConfig.enabled` is `true`")).optional().type(STRING), @@ -805,7 +805,7 @@ private Snippet getResponseFields() { fieldWithPath("config.corsPolicy.xhrConfiguration.maxAge").description(CORS_XHR_MAXAGE_DESC), fieldWithPath("config.userConfig.defaultGroups").description(DEFAULT_ZONE_GROUPS_DESC), - fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC), + fieldWithPath("config.userConfig.allowedGroups").description(ALLOWED_ZONE_GROUPS_DESC).optional().type(ARRAY), fieldWithPath("config.mfaConfig.enabled").description(MFA_CONFIG_ENABLED_DESC), fieldWithPath("config.mfaConfig.providerName").description(MFA_CONFIG_PROVIDER_NAME_DESC).optional().type(STRING), From c7eb6d746b20ef1cbeb55615efcbd6ff3461245f Mon Sep 17 00:00:00 2001 From: d036670 Date: Mon, 20 Nov 2023 17:01:25 +0100 Subject: [PATCH 12/36] fix sonar smells --- .../identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index 2308885fd85..25a7033a9ad 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -230,6 +230,7 @@ public ScimGroup retrieve(String id, final String zoneId) throws ScimResourceNot } } + @SuppressWarnings("java:S1874") private List getAllowedUserGroups(String zoneId) { List zoneAllowedGroups = null; // default: all groups allowed if (!hasText(zoneId)) { @@ -241,7 +242,7 @@ private List getAllowedUserGroups(String zoneId) { currentZone.getConfig().getUserConfig().getAllowedGroups() : jdbcIdentityZoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().getAllowedGroups(); } catch (ZoneDoesNotExistsException e) { - logger.debug("could not retrieve identity zone with id: %s", zoneId); + logger.debug("could not retrieve identity zone with id: {}", zoneId); } return zoneAllowedGroups; } From 6e6ab189b2be5f60f2b3200ede998a04c5253aa8 Mon Sep 17 00:00:00 2001 From: D023954 Date: Tue, 21 Nov 2023 12:49:02 +0100 Subject: [PATCH 13/36] combine default and allowed groups --- .../identity/uaa/zone/UserConfig.java | 14 +++++++++++++ .../identity/uaa/zone/IdentityZoneTest.java | 5 +++-- .../identity/uaa/zone/SampleIdentityZone.json | 10 +--------- .../oauth/UaaAuthorizationRequestManager.java | 2 +- .../scim/jdbc/JdbcScimGroupProvisioning.java | 13 ++++++------ .../uaa/user/JdbcUaaUserDatabase.java | 2 +- .../uaa/zone/UserConfigValidator.java | 20 ++++++------------- ...entityZoneConfigurationBootstrapTests.java | 2 +- .../IdentityZoneConfigurationTests.java | 2 +- .../uaa/user/JdbcUaaUserDatabaseTests.java | 2 +- .../uaa/zone/UserConfigValidatorTest.java | 11 +++++----- .../IdentityZoneEndpointsMockMvcTests.java | 8 ++++---- 12 files changed, 45 insertions(+), 46 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java index b157b0c41ef..44d2cacc8d8 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java @@ -3,7 +3,9 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; +import java.util.HashSet; import java.util.List; +import java.util.Set; @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) @@ -28,6 +30,7 @@ public void setDefaultGroups(List defaultGroups) { this.defaultGroups = defaultGroups; } + // in addition to defaultGroups, which are implicitely allowed private List allowedGroups = null; public List getAllowedGroups() { @@ -37,4 +40,15 @@ public List getAllowedGroups() { public void setAllowedGroups(List allowedGroups) { this.allowedGroups = allowedGroups; } + + // return defaultGroups plus allowedGroups + public Set resultingAllowedGroups() { + if (allowedGroups == null) { + return null; // all groups allowed + } else { + HashSet allAllowedGroups = new HashSet(allowedGroups); + if (defaultGroups != null) allAllowedGroups.addAll(defaultGroups); + return allAllowedGroups; + } + } } diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java index 151135f0254..59fa780794f 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java @@ -11,6 +11,7 @@ import java.util.Calendar; import java.util.Date; import java.util.List; +import java.util.Set; import java.util.stream.Stream; import static org.cloudfoundry.identity.uaa.test.ModelTestUtils.getResourceAsString; @@ -133,9 +134,9 @@ void deserialize() { assertEquals(List.of("openid", "password.write", "uaa.user", "approvals.me", "profile", "roles", "user_attributes", "uaa.offline_token"), sampleIdentityZone.getConfig().getUserConfig().getDefaultGroups()); - assertEquals(List.of("openid", "password.write", "uaa.user", "approvals.me", + assertEquals(Set.of("openid", "password.write", "uaa.user", "approvals.me", "profile", "roles", "user_attributes", "uaa.offline_token", "scim.me", "cloud_controller.user"), - sampleIdentityZone.getConfig().getUserConfig().getAllowedGroups()); + sampleIdentityZone.getConfig().getUserConfig().resultingAllowedGroups()); } } \ No newline at end of file diff --git a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json index 8f7e5dd7bed..36dea4ff4bc 100644 --- a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json +++ b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json @@ -124,14 +124,6 @@ "uaa.offline_token" ], "allowedGroups": [ - "openid", - "password.write", - "uaa.user", - "approvals.me", - "profile", - "roles", - "user_attributes", - "uaa.offline_token", "scim.me", "cloud_controller.user" ] @@ -142,4 +134,4 @@ "description": "{\"plan_display_name\":\"Demo\",\"plan_description\":\"Demo SSO Plan\"}", "created": 1503504273000, "last_modified": 1504898224000 -} \ No newline at end of file +} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java index 81a782dca58..928226bfc24 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java @@ -249,7 +249,7 @@ private Set checkUserScopes(Set requestedScopes, Set allowed = new LinkedHashSet<>(AuthorityUtils.authorityListToSet(authorities)); // Add in all default requestedScopes Collection defaultScopes = IdentityZoneHolder.get().getConfig().getUserConfig().getDefaultGroups(); - Collection allowedScopes = IdentityZoneHolder.get().getConfig().getUserConfig().getAllowedGroups(); + Collection allowedScopes = IdentityZoneHolder.get().getConfig().getUserConfig().resultingAllowedGroups(); allowed.addAll(defaultScopes); if (allowedScopes != null) { allowed.retainAll(allowedScopes); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index 25a7033a9ad..41f4543a76e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -28,6 +28,7 @@ import java.sql.Timestamp; import java.util.Date; import java.util.List; +import java.util.Set; import java.util.UUID; import static org.cloudfoundry.identity.uaa.zone.ZoneManagementScopes.getSystemScopes; @@ -231,16 +232,16 @@ public ScimGroup retrieve(String id, final String zoneId) throws ScimResourceNot } @SuppressWarnings("java:S1874") - private List getAllowedUserGroups(String zoneId) { - List zoneAllowedGroups = null; // default: all groups allowed + private Set getAllowedUserGroups(String zoneId) { + Set zoneAllowedGroups = null; // default: all groups allowed if (!hasText(zoneId)) { return zoneAllowedGroups; } IdentityZone currentZone = IdentityZoneHolder.get(); try { zoneAllowedGroups = (zoneId.equals(currentZone.getId())) ? - currentZone.getConfig().getUserConfig().getAllowedGroups() : - jdbcIdentityZoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().getAllowedGroups(); + currentZone.getConfig().getUserConfig().resultingAllowedGroups() : + jdbcIdentityZoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().resultingAllowedGroups(); } catch (ZoneDoesNotExistsException e) { logger.debug("could not retrieve identity zone with id: {}", zoneId); } @@ -249,7 +250,7 @@ private List getAllowedUserGroups(String zoneId) { @Override public ScimGroup create(final ScimGroup group, final String zoneId) throws InvalidScimResourceException { - List allowedGroups = getAllowedUserGroups(zoneId); + Set allowedGroups = getAllowedUserGroups(zoneId); if ((allowedGroups != null) && (!allowedGroups.contains(group.getDisplayName()))) { throw new InvalidScimResourceException("The group with displayName: " + group.getDisplayName() + " is not allowed in Identity Zone " + zoneId); @@ -278,7 +279,7 @@ public ScimGroup create(final ScimGroup group, final String zoneId) throws Inval @Override public ScimGroup update(final String id, final ScimGroup group, final String zoneId) throws InvalidScimResourceException, ScimResourceNotFoundException { - List allowedGroups = getAllowedUserGroups(zoneId); + Set allowedGroups = getAllowedUserGroups(zoneId); if ((allowedGroups != null) && (!allowedGroups.contains(group.getDisplayName()))) { throw new InvalidScimResourceException("The group with displayName: " + group.getDisplayName() + " is not allowed in Identity Zone " + zoneId); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabase.java b/server/src/main/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabase.java index 82f49699d84..77ffc79ab9f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabase.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabase.java @@ -265,7 +265,7 @@ private String getAuthorities(final String userId) throws SQLException { Set authorities = new HashSet<>(); getAuthorities(authorities, Collections.singletonList(userId)); authorities.addAll(identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().getDefaultGroups()); - List allowedGroups = identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().getAllowedGroups(); + Set allowedGroups = identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().resultingAllowedGroups(); if (allowedGroups != null) { authorities.retainAll(allowedGroups); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java index c52284bea18..02703c3383f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java @@ -1,6 +1,6 @@ package org.cloudfoundry.identity.uaa.zone; -import java.util.List; +import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -13,19 +13,11 @@ private UserConfigValidator() { } public static void validate(UserConfig config) throws InvalidIdentityZoneConfigurationException { - List defaultGroups = (config == null) ? null : config.getDefaultGroups(); - List allowedGroups = (config == null) ? null : config.getAllowedGroups(); - if (allowedGroups != null) { - if (allowedGroups.isEmpty()) { - String message = "At least one group must be allowed"; - logger.error(message); - throw new InvalidIdentityZoneConfigurationException(message); - } - if ((defaultGroups == null) || (!allowedGroups.containsAll(defaultGroups))) { - String message = "All default groups must be allowed"; - logger.error(message); - throw new InvalidIdentityZoneConfigurationException(message); - } + Set allowedGroups = (config == null) ? null : config.resultingAllowedGroups(); + if ((allowedGroups != null) && (allowedGroups.isEmpty())) { + String message = "At least one group must be allowed"; + logger.error(message); + throw new InvalidIdentityZoneConfigurationException(message); } } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java index babb22ff9af..dc846518e1a 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java @@ -199,7 +199,7 @@ void testAllowedGroups() throws Exception { bootstrap.setAllowedUserGroups(Arrays.asList(groups)); bootstrap.afterPropertiesSet(); IdentityZone uaa = provisioning.retrieve(IdentityZone.getUaaZoneId()); - assertThat(uaa.getConfig().getUserConfig().getAllowedGroups(), containsInAnyOrder(groups)); + assertThat(uaa.getConfig().getUserConfig().resultingAllowedGroups(), containsInAnyOrder(groups)); } @Test diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationTests.java index cd425874727..fda80e8b5f7 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationTests.java @@ -75,7 +75,7 @@ public void default_user_groups_when_json_is_deserialized() { "user_attributes", "uaa.offline_token" )); - assertNull(definition.getUserConfig().getAllowedGroups()); + assertNull(definition.getUserConfig().resultingAllowedGroups()); s = JsonUtils.writeValueAsString(definition); assertThat(s, containsString("userConfig")); assertThat(s, containsString("uaa.offline_token")); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java index 618a1a83f75..f270caf291e 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/user/JdbcUaaUserDatabaseTests.java @@ -116,7 +116,7 @@ private static void setUpIdentityZone(IdentityZoneManager mockIdentityZoneManage when(mockIdentityZone.getConfig()).thenReturn(mockIdentityZoneConfiguration); when(mockIdentityZoneConfiguration.getUserConfig()).thenReturn(mockUserConfig); when(mockUserConfig.getDefaultGroups()).thenReturn(UserConfig.DEFAULT_ZONE_GROUPS); - when(mockUserConfig.getAllowedGroups()).thenReturn(null); // allow all groups + when(mockUserConfig.resultingAllowedGroups()).thenReturn(null); // allow all groups } @AfterEach diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidatorTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidatorTest.java index f3711c5a0d5..8cc53698d3a 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidatorTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidatorTest.java @@ -3,14 +3,13 @@ import org.junit.Test; import java.util.Collections; -import java.util.List; public class UserConfigValidatorTest { @Test public void testDefaultConfig() throws InvalidIdentityZoneConfigurationException { - UserConfigValidator.validate(new UserConfig()); + UserConfigValidator.validate(new UserConfig()); // defaultGroups not empty, allowedGroups is null } @Test @@ -18,7 +17,7 @@ public void testNullConfig() throws InvalidIdentityZoneConfigurationException { UserConfigValidator.validate(null); } - @Test(expected = InvalidIdentityZoneConfigurationException.class) + @Test public void testAllowedGroupsEmpty() throws InvalidIdentityZoneConfigurationException { UserConfig userConfig = new UserConfig(); userConfig.setAllowedGroups(Collections.emptyList()); @@ -26,10 +25,10 @@ public void testAllowedGroupsEmpty() throws InvalidIdentityZoneConfigurationExce } @Test(expected = InvalidIdentityZoneConfigurationException.class) - public void testDefaultGroupsNotAllowed() throws InvalidIdentityZoneConfigurationException { + public void testNoGroupsAllowed() throws InvalidIdentityZoneConfigurationException { UserConfig userConfig = new UserConfig(); - userConfig.setDefaultGroups(List.of("openid","uaa.user")); - userConfig.setAllowedGroups(List.of("uaa.user")); // openid not allowed + userConfig.setDefaultGroups(Collections.emptyList()); + userConfig.setAllowedGroups(Collections.emptyList()); // no groups allowed UserConfigValidator.validate(userConfig); } } \ No newline at end of file diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java index d2850a0b041..c29d1be61bc 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java @@ -508,11 +508,11 @@ void createZoneWithNoSubdomainFailsWithUnprocessableEntity() throws Exception { } @Test - void createZoneWithNotAllowedGroupsFailsWithUnprocessableEntity() throws Exception { + void createZoneWithNoAllowedGroupsFailsWithUnprocessableEntity() throws Exception { String id = generator.generate(); IdentityZone zone = this.createSimpleIdentityZone(id); - zone.getConfig().getUserConfig().setDefaultGroups(List.of("uaa.user", "openid")); - zone.getConfig().getUserConfig().setAllowedGroups(List.of("uaa.user")); // openid not allowed + zone.getConfig().getUserConfig().setDefaultGroups(Collections.emptyList()); + zone.getConfig().getUserConfig().setAllowedGroups(Collections.emptyList()); // no groups allowed mockMvc.perform( post("/identity-zones") @@ -522,7 +522,7 @@ void createZoneWithNotAllowedGroupsFailsWithUnprocessableEntity() throws Excepti .andExpect(status().isUnprocessableEntity()) .andExpect(jsonPath("$.error").value("invalid_identity_zone")) .andExpect(jsonPath("$.error_description").value("The identity zone details are invalid. " + - "The zone configuration is invalid. All default groups must be allowed")); + "The zone configuration is invalid. At least one group must be allowed")); assertEquals(0, zoneModifiedEventListener.getEventCount()); } From 979f93348e22e55a201bb4d9f63b912046131280 Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 22 Nov 2023 11:25:27 +0100 Subject: [PATCH 14/36] fix Sonar warnings --- .../java/org/cloudfoundry/identity/uaa/zone/UserConfig.java | 5 +++-- .../identity/uaa/oauth/UaaAuthorizationRequestManager.java | 1 + .../identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java index 44d2cacc8d8..9fbff518d0e 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java @@ -42,11 +42,12 @@ public void setAllowedGroups(List allowedGroups) { } // return defaultGroups plus allowedGroups + @SuppressWarnings("java:S1168") public Set resultingAllowedGroups() { if (allowedGroups == null) { - return null; // all groups allowed + return null; // null = all groups allowed } else { - HashSet allAllowedGroups = new HashSet(allowedGroups); + HashSet allAllowedGroups = new HashSet<>(allowedGroups); if (defaultGroups != null) allAllowedGroups.addAll(defaultGroups); return allAllowedGroups; } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java index 928226bfc24..10e761dc95f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManager.java @@ -243,6 +243,7 @@ protected void checkClientIdpAuthorization(BaseClientDetails client, UaaUser use * @param authorities the users authorities * @return modified requested scopes adapted according to the rules specified */ + @SuppressWarnings("java:S1874") private Set checkUserScopes(Set requestedScopes, Collection authorities, ClientDetails clientDetails) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index 41f4543a76e..ad0c520c6a6 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -256,7 +256,7 @@ public ScimGroup create(final ScimGroup group, final String zoneId) throws Inval + " is not allowed in Identity Zone " + zoneId); } final String id = UUID.randomUUID().toString(); - logger.debug("creating new group with id: %s", id); + logger.debug("creating new group with id: {}", id); try { validateGroup(group); jdbcTemplate.update(addGroupSql, ps -> { From 8cde93e0abb522f5a23552c547eb9d7228715a85 Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 22 Nov 2023 11:53:07 +0100 Subject: [PATCH 15/36] add test class --- .../identity/uaa/zone/UserConfigTest.java | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java new file mode 100644 index 00000000000..18eae7cab18 --- /dev/null +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java @@ -0,0 +1,41 @@ +package org.cloudfoundry.identity.uaa.zone; + +import static org.junit.Assert.assertTrue; + +import java.util.List; + +import org.junit.Test; + +public class UserConfigTest { + + @Test + public void testDefaultConfig() { + UserConfig userConfig = new UserConfig(); + assertTrue(userConfig.getDefaultGroups() != null); + assertTrue(userConfig.getDefaultGroups().contains("uaa.user")); + assertTrue(userConfig.getAllowedGroups() == null); // all groups allowed + assertTrue(userConfig.resultingAllowedGroups() == null); // all groups allowed + } + + @Test + public void testNoDefaultGroups() { + UserConfig userConfig = new UserConfig(); + userConfig.setDefaultGroups(null); + userConfig.setAllowedGroups(List.of("uaa.user")); + assertTrue(userConfig.getDefaultGroups() == null); + assertTrue(userConfig.getAllowedGroups() != null); + assertTrue(userConfig.getAllowedGroups().contains("uaa.user")); + assertTrue(userConfig.resultingAllowedGroups() != null); + assertTrue(userConfig.resultingAllowedGroups().contains("uaa.user")); + } + + @Test + public void testNoDefaultAndNoAllowedGroups() { + UserConfig userConfig = new UserConfig(); + userConfig.setDefaultGroups(null); + userConfig.setAllowedGroups(null); + assertTrue(userConfig.getDefaultGroups() == null); + assertTrue(userConfig.getAllowedGroups() == null); // all groups allowed + assertTrue(userConfig.resultingAllowedGroups() == null); // all groups allowed + } +} From b7addddca85e00c65b1e0c5ff81a731d2e1b4dcf Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 22 Nov 2023 12:14:47 +0100 Subject: [PATCH 16/36] add testResultingAllowedGroups --- .../identity/uaa/zone/UserConfigTest.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java index 18eae7cab18..82cb852cc8d 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertTrue; import java.util.List; +import java.util.Set; import org.junit.Test; @@ -11,22 +12,29 @@ public class UserConfigTest { @Test public void testDefaultConfig() { UserConfig userConfig = new UserConfig(); - assertTrue(userConfig.getDefaultGroups() != null); - assertTrue(userConfig.getDefaultGroups().contains("uaa.user")); + assertTrue(userConfig.getDefaultGroups().contains("openid")); assertTrue(userConfig.getAllowedGroups() == null); // all groups allowed assertTrue(userConfig.resultingAllowedGroups() == null); // all groups allowed } + @Test + public void testResultingAllowedGroups() { + UserConfig userConfig = new UserConfig(); + userConfig.setDefaultGroups(List.of("openid")); + userConfig.setAllowedGroups(List.of("uaa.user")); + assertTrue(userConfig.getDefaultGroups().equals(List.of("openid"))); + assertTrue(userConfig.getAllowedGroups().equals(List.of("uaa.user"))); + assertTrue(userConfig.resultingAllowedGroups().equals(Set.of("openid", "uaa.user"))); + } + @Test public void testNoDefaultGroups() { UserConfig userConfig = new UserConfig(); userConfig.setDefaultGroups(null); userConfig.setAllowedGroups(List.of("uaa.user")); assertTrue(userConfig.getDefaultGroups() == null); - assertTrue(userConfig.getAllowedGroups() != null); - assertTrue(userConfig.getAllowedGroups().contains("uaa.user")); - assertTrue(userConfig.resultingAllowedGroups() != null); - assertTrue(userConfig.resultingAllowedGroups().contains("uaa.user")); + assertTrue(userConfig.getAllowedGroups().equals(List.of("uaa.user"))); + assertTrue(userConfig.resultingAllowedGroups().equals(Set.of("uaa.user"))); } @Test From a464babd7399d39fa3eaa6778b9b12dec00ecee7 Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 22 Nov 2023 15:10:28 +0100 Subject: [PATCH 17/36] add testScopesIncludesAllowedAuthoritiesForUser --- .../oauth/UaaAuthorizationRequestManagerTests.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java index 3159c5fd0cc..b663fb7aa51 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaAuthorizationRequestManagerTests.java @@ -176,6 +176,17 @@ void testScopeIncludesAuthoritiesForUser() { factory.validateParameters(request.getRequestParameters(), client); } + @Test + void testScopesIncludesAllowedAuthoritiesForUser() { + when(mockSecurityContextAccessor.isUser()).thenReturn(true); + when(mockSecurityContextAccessor.getAuthorities()).thenReturn((Collection)AuthorityUtils.commaSeparatedStringToAuthorityList("foo.bar,spam.baz,space.1.developer")); + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(Arrays.asList("openid","foo.bar")); + client.setScope(StringUtils.commaDelimitedListToSet("foo.bar,spam.baz,space.1.developer")); + AuthorizationRequest request = factory.createAuthorizationRequest(parameters); + assertEquals(StringUtils.commaDelimitedListToSet("foo.bar"), new TreeSet(request.getScope())); + factory.validateParameters(request.getRequestParameters(), client); + } + @Test void testWildcardScopesIncludesAuthoritiesForUser() { when(mockSecurityContextAccessor.isUser()).thenReturn(true); From 370be522393608696adab86ba3888269b8ed235d Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 22 Nov 2023 16:09:17 +0100 Subject: [PATCH 18/36] zoneId must not be null --- .../identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java | 8 ++++---- .../uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index ad0c520c6a6..7b554ea9999 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -233,12 +233,12 @@ public ScimGroup retrieve(String id, final String zoneId) throws ScimResourceNot @SuppressWarnings("java:S1874") private Set getAllowedUserGroups(String zoneId) { - Set zoneAllowedGroups = null; // default: all groups allowed - if (!hasText(zoneId)) { - return zoneAllowedGroups; + if (zoneId == null) { + throw new IllegalArgumentException("zoneId is null"); } - IdentityZone currentZone = IdentityZoneHolder.get(); + Set zoneAllowedGroups = null; // default: all groups allowed try { + IdentityZone currentZone = IdentityZoneHolder.get(); zoneAllowedGroups = (zoneId.equals(currentZone.getId())) ? currentZone.getConfig().getUserConfig().resultingAllowedGroups() : jdbcIdentityZoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().resultingAllowedGroups(); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java index 316bd82e5b0..8e627fd637c 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioningTests.java @@ -316,6 +316,7 @@ void cannotUpdateNotAllowedGroup() { dao.update(g1Id, g, zoneId); fail(); } catch(InvalidScimResourceException e) { + assertTrue(e.getMessage().contains("is not allowed")); } } From a6722b50e4fe1ff4c23983d7b11a0799d913b77e Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 22 Nov 2023 16:32:17 +0100 Subject: [PATCH 19/36] use right asserts --- .../identity/uaa/zone/UserConfigTest.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java index 82cb852cc8d..560f2129b55 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/UserConfigTest.java @@ -1,5 +1,7 @@ package org.cloudfoundry.identity.uaa.zone; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.List; @@ -13,8 +15,8 @@ public class UserConfigTest { public void testDefaultConfig() { UserConfig userConfig = new UserConfig(); assertTrue(userConfig.getDefaultGroups().contains("openid")); - assertTrue(userConfig.getAllowedGroups() == null); // all groups allowed - assertTrue(userConfig.resultingAllowedGroups() == null); // all groups allowed + assertNull(userConfig.getAllowedGroups()); // all groups allowed + assertNull(userConfig.resultingAllowedGroups()); // all groups allowed } @Test @@ -22,9 +24,9 @@ public void testResultingAllowedGroups() { UserConfig userConfig = new UserConfig(); userConfig.setDefaultGroups(List.of("openid")); userConfig.setAllowedGroups(List.of("uaa.user")); - assertTrue(userConfig.getDefaultGroups().equals(List.of("openid"))); - assertTrue(userConfig.getAllowedGroups().equals(List.of("uaa.user"))); - assertTrue(userConfig.resultingAllowedGroups().equals(Set.of("openid", "uaa.user"))); + assertEquals(List.of("openid"), userConfig.getDefaultGroups()); + assertEquals(List.of("uaa.user"), userConfig.getAllowedGroups()); + assertEquals(Set.of("openid", "uaa.user"), userConfig.resultingAllowedGroups()); } @Test @@ -32,9 +34,9 @@ public void testNoDefaultGroups() { UserConfig userConfig = new UserConfig(); userConfig.setDefaultGroups(null); userConfig.setAllowedGroups(List.of("uaa.user")); - assertTrue(userConfig.getDefaultGroups() == null); - assertTrue(userConfig.getAllowedGroups().equals(List.of("uaa.user"))); - assertTrue(userConfig.resultingAllowedGroups().equals(Set.of("uaa.user"))); + assertNull(userConfig.getDefaultGroups()); + assertEquals(List.of("uaa.user"), userConfig.getAllowedGroups()); + assertEquals(Set.of("uaa.user"), userConfig.resultingAllowedGroups()); } @Test @@ -42,8 +44,8 @@ public void testNoDefaultAndNoAllowedGroups() { UserConfig userConfig = new UserConfig(); userConfig.setDefaultGroups(null); userConfig.setAllowedGroups(null); - assertTrue(userConfig.getDefaultGroups() == null); - assertTrue(userConfig.getAllowedGroups() == null); // all groups allowed - assertTrue(userConfig.resultingAllowedGroups() == null); // all groups allowed + assertNull(userConfig.getDefaultGroups()); + assertNull(userConfig.getAllowedGroups()); // all groups allowed + assertNull(userConfig.resultingAllowedGroups()); // all groups allowed } } From ab2d5cf74037ce00effc502395c8b2578be87c2e Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 23 Nov 2023 11:16:19 +0100 Subject: [PATCH 20/36] remove null-check --- .../identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index 7b554ea9999..2bd9d535fbf 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -233,13 +233,10 @@ public ScimGroup retrieve(String id, final String zoneId) throws ScimResourceNot @SuppressWarnings("java:S1874") private Set getAllowedUserGroups(String zoneId) { - if (zoneId == null) { - throw new IllegalArgumentException("zoneId is null"); - } Set zoneAllowedGroups = null; // default: all groups allowed try { IdentityZone currentZone = IdentityZoneHolder.get(); - zoneAllowedGroups = (zoneId.equals(currentZone.getId())) ? + zoneAllowedGroups = (currentZone.getId().equals(zoneId)) ? currentZone.getConfig().getUserConfig().resultingAllowedGroups() : jdbcIdentityZoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().resultingAllowedGroups(); } catch (ZoneDoesNotExistsException e) { From bd87bed95e23f1b1af3b47eeb61c58975b4618b6 Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 23 Nov 2023 14:00:22 +0100 Subject: [PATCH 21/36] add integration tests --- .../ScimGroupEndpointsIntegrationTests.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index e6a73309153..2967ce11fcb 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -39,6 +39,7 @@ import org.springframework.security.oauth2.provider.client.BaseClientDetails; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; import java.net.URI; @@ -207,6 +208,34 @@ public void createGroupSucceeds() { assertEquals(g1, g2); } + @Test + public void createAllowedGroupSucceeds() { + ScimGroup g1 = null; + try { + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(List.of(CFID)); // allow CFID group + g1 = createGroup(CFID); + } finally { + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); // restore default + } + // Check we can GET the group + ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); + assertEquals(g1, g2); + } + + @Test + public void createNotAllowedGroupFails() { + fail("createNotAllowedGroupFails"); + try { + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(List.of("notallowed")); // dont allow CFID group + createGroup(CFID); + fail("Could create not allowed group " + CFID); + } catch (Exception e) { + assertTrue(e.getMessage().contains("not allowed")); + } finally { + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); // restore default + } + } + @Test public void createGroupWithMembersSucceeds() { ScimGroup g1 = createGroup(CFID, JOEL, DALE, VIDYA); From fdc6b02e489943909f4486d7ea69e2b3b9b65579 Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 23 Nov 2023 15:27:53 +0100 Subject: [PATCH 22/36] fix createNotAllowedGroupFails --- .../uaa/integration/ScimGroupEndpointsIntegrationTests.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index 2967ce11fcb..25bba3f040c 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -224,13 +224,12 @@ public void createAllowedGroupSucceeds() { @Test public void createNotAllowedGroupFails() { - fail("createNotAllowedGroupFails"); try { IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(List.of("notallowed")); // dont allow CFID group createGroup(CFID); fail("Could create not allowed group " + CFID); - } catch (Exception e) { - assertTrue(e.getMessage().contains("not allowed")); + } catch (RestClientException e) { + assertFalse(e.getMessage().isEmpty()); } finally { IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); // restore default } From f4005987708c5b476eca3199ad28c2957af581a2 Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 23 Nov 2023 16:02:20 +0100 Subject: [PATCH 23/36] introduce list allowedGroups --- .../ScimGroupEndpointsIntegrationTests.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index 25bba3f040c..314f4ea8bf5 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -69,6 +69,8 @@ public class ScimGroupEndpointsIntegrationTests { private final String CFID = "cfid_" + new RandomValueStringGenerator().generate().toLowerCase(); + private final List allowedGroups = List.of(DELETE_ME, CF_DEV, CF_MGR, CFID); + private final String groupEndpoint = "/Groups"; private final String userEndpoint = "/Users"; @@ -210,26 +212,26 @@ public void createGroupSucceeds() { @Test public void createAllowedGroupSucceeds() { - ScimGroup g1 = null; try { - IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(List.of(CFID)); // allow CFID group - g1 = createGroup(CFID); + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(allowedGroups); + ScimGroup g1 = createGroup(CFID); + // Check we can GET the group + ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); + assertEquals(g1, g2); } finally { IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); // restore default } - // Check we can GET the group - ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); - assertEquals(g1, g2); } @Test - public void createNotAllowedGroupFails() { + public void createNotAllowedGroupFailsCorrectly() { + final String NOT_ALLOWED = "not_allowed_" + new RandomValueStringGenerator().generate().toLowerCase(); try { - IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(List.of("notallowed")); // dont allow CFID group - createGroup(CFID); - fail("Could create not allowed group " + CFID); - } catch (RestClientException e) { - assertFalse(e.getMessage().isEmpty()); + IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(allowedGroups); + ScimGroup g1 = createGroup(NOT_ALLOWED); + // Check we cannot GET the group + ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); + assertNull(g2); } finally { IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); // restore default } From 163194cbcdfdea61d3c873ff13094b2f72dc5c9f Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 23 Nov 2023 17:40:41 +0100 Subject: [PATCH 24/36] update allowedGroups on server --- .../ScimGroupEndpointsIntegrationTests.java | 9 ++++----- .../uaa/integration/util/IntegrationTestUtils.java | 10 ++++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index 314f4ea8bf5..0263d2a5f54 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -39,7 +39,6 @@ import org.springframework.security.oauth2.provider.client.BaseClientDetails; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; import java.net.URI; @@ -213,13 +212,13 @@ public void createGroupSucceeds() { @Test public void createAllowedGroupSucceeds() { try { - IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(allowedGroups); + IntegrationTestUtils.updateDefaultZoneAllowedGroups(client, serverRunning.getBaseUrl(), allowedGroups); ScimGroup g1 = createGroup(CFID); // Check we can GET the group ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); assertEquals(g1, g2); } finally { - IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); // restore default + IntegrationTestUtils.updateDefaultZoneAllowedGroups(client, serverRunning.getBaseUrl(), null); // restore default } } @@ -227,13 +226,13 @@ public void createAllowedGroupSucceeds() { public void createNotAllowedGroupFailsCorrectly() { final String NOT_ALLOWED = "not_allowed_" + new RandomValueStringGenerator().generate().toLowerCase(); try { - IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(allowedGroups); + IntegrationTestUtils.updateDefaultZoneAllowedGroups(client, serverRunning.getBaseUrl(), allowedGroups); ScimGroup g1 = createGroup(NOT_ALLOWED); // Check we cannot GET the group ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); assertNull(g2); } finally { - IdentityZoneHolder.get().getConfig().getUserConfig().setAllowedGroups(null); // restore default + IntegrationTestUtils.updateDefaultZoneAllowedGroups(client, serverRunning.getBaseUrl(), null); // restore default } } diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java index 048cfffb0b7..59143aaeefe 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java @@ -676,6 +676,16 @@ public static IdentityZone createZoneOrUpdateSubdomain(RestTemplate client, return createZoneOrUpdateSubdomain(client, url, id, subdomain, config, true); } + public static IdentityZone updateDefaultZoneAllowedGroups(RestTemplate client, String url, List allowedGroups) { + ResponseEntity zoneGet = client.getForEntity(url + "/identity-zones/uaa", IdentityZone.class); + if (!(zoneGet.getStatusCode() == HttpStatus.OK)) { + throw new RuntimeException("Could not get default zone."); + } + IdentityZone idz = zoneGet.getBody(); + idz.getConfig().getUserConfig().setAllowedGroups(allowedGroups); + return createZoneOrUpdateSubdomain(client, url, idz.getId(), idz.getSubdomain(), idz.getConfig(), idz.isActive()); + } + public static void addMemberToGroup(RestTemplate client, String url, String userId, From 1e02b3886df8447970ebccd705b8bf68c3026d43 Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 23 Nov 2023 19:40:57 +0100 Subject: [PATCH 25/36] pass IdZ id --- .../integration/ScimGroupEndpointsIntegrationTests.java | 8 ++++---- .../uaa/integration/util/IntegrationTestUtils.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index 0263d2a5f54..1943f8918e2 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -212,13 +212,13 @@ public void createGroupSucceeds() { @Test public void createAllowedGroupSucceeds() { try { - IntegrationTestUtils.updateDefaultZoneAllowedGroups(client, serverRunning.getBaseUrl(), allowedGroups); + IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), IdentityZoneHolder.get().getId(), allowedGroups); ScimGroup g1 = createGroup(CFID); // Check we can GET the group ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); assertEquals(g1, g2); } finally { - IntegrationTestUtils.updateDefaultZoneAllowedGroups(client, serverRunning.getBaseUrl(), null); // restore default + IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), IdentityZoneHolder.get().getId(), null); // restore default } } @@ -226,13 +226,13 @@ public void createAllowedGroupSucceeds() { public void createNotAllowedGroupFailsCorrectly() { final String NOT_ALLOWED = "not_allowed_" + new RandomValueStringGenerator().generate().toLowerCase(); try { - IntegrationTestUtils.updateDefaultZoneAllowedGroups(client, serverRunning.getBaseUrl(), allowedGroups); + IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), IdentityZoneHolder.get().getId(), allowedGroups); ScimGroup g1 = createGroup(NOT_ALLOWED); // Check we cannot GET the group ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); assertNull(g2); } finally { - IntegrationTestUtils.updateDefaultZoneAllowedGroups(client, serverRunning.getBaseUrl(), null); // restore default + IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), IdentityZoneHolder.get().getId(), null); // restore default } } diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java index 59143aaeefe..01171de880a 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java @@ -676,8 +676,8 @@ public static IdentityZone createZoneOrUpdateSubdomain(RestTemplate client, return createZoneOrUpdateSubdomain(client, url, id, subdomain, config, true); } - public static IdentityZone updateDefaultZoneAllowedGroups(RestTemplate client, String url, List allowedGroups) { - ResponseEntity zoneGet = client.getForEntity(url + "/identity-zones/uaa", IdentityZone.class); + public static IdentityZone updateIdentityZoneAllowedGroups(RestTemplate client, String url, String id, List allowedGroups) { + ResponseEntity zoneGet = client.getForEntity(url + "/identity-zones/{id}", IdentityZone.class); if (!(zoneGet.getStatusCode() == HttpStatus.OK)) { throw new RuntimeException("Could not get default zone."); } From c9810f7cc17faf143db284bfb0c445e788bce299 Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 23 Nov 2023 21:25:05 +0100 Subject: [PATCH 26/36] determine zone id --- .../ScimGroupEndpointsIntegrationTests.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index 1943f8918e2..b8123199186 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -93,6 +93,7 @@ public class ScimGroupEndpointsIntegrationTests { private RestTemplate client; private List scimGroups; + private String zoneId; @Before public void createRestTemplate() { @@ -108,8 +109,9 @@ public boolean hasError(ClientHttpResponse response) { public void handleError(ClientHttpResponse response) { } }); - - JOEL = new ScimGroupMember(createUser("joel_" + new RandomValueStringGenerator().generate().toLowerCase(), "Passwo3d").getId()); + ScimUser joelScimUser = createUser("joel_" + new RandomValueStringGenerator().generate().toLowerCase(), "Passwo3d"); + zoneId = joelScimUser.getZoneId(); + JOEL = new ScimGroupMember(joelScimUser.getId()); DALE = new ScimGroupMember(createUser("dale_" + new RandomValueStringGenerator().generate().toLowerCase(), "Passwo3d").getId()); VIDYA = new ScimGroupMember(createUser("vidya_" + new RandomValueStringGenerator().generate().toLowerCase(), "Passwo3d").getId()); } @@ -212,13 +214,13 @@ public void createGroupSucceeds() { @Test public void createAllowedGroupSucceeds() { try { - IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), IdentityZoneHolder.get().getId(), allowedGroups); + IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), zoneId, allowedGroups); ScimGroup g1 = createGroup(CFID); // Check we can GET the group ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); assertEquals(g1, g2); } finally { - IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), IdentityZoneHolder.get().getId(), null); // restore default + IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), zoneId, null); // restore default } } @@ -226,13 +228,13 @@ public void createAllowedGroupSucceeds() { public void createNotAllowedGroupFailsCorrectly() { final String NOT_ALLOWED = "not_allowed_" + new RandomValueStringGenerator().generate().toLowerCase(); try { - IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), IdentityZoneHolder.get().getId(), allowedGroups); + IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), zoneId, allowedGroups); ScimGroup g1 = createGroup(NOT_ALLOWED); // Check we cannot GET the group ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); assertNull(g2); } finally { - IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), IdentityZoneHolder.get().getId(), null); // restore default + IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), zoneId, null); // restore default } } From 12912288d1a412f9fe347ca4621b2e1c3aa62d04 Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 23 Nov 2023 23:41:39 +0100 Subject: [PATCH 27/36] update integration tests We have 4 tests 2 positive and 2 negative 2 using allowedGroups 2 relying on defaultGroups because allowedGroups is empty --- .../identity/uaa/zone/SampleIdentityZone.json | 2 +- .../ScimGroupEndpointsIntegrationTests.java | 102 ++++++++++++++---- .../util/IntegrationTestUtils.java | 30 ++++-- 3 files changed, 104 insertions(+), 30 deletions(-) diff --git a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json index 36dea4ff4bc..3cbc9812f5e 100644 --- a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json +++ b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json @@ -134,4 +134,4 @@ "description": "{\"plan_display_name\":\"Demo\",\"plan_description\":\"Demo SSO Plan\"}", "created": 1503504273000, "last_modified": 1504898224000 -} +} \ No newline at end of file diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index b8123199186..8f43b861c6c 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -22,6 +22,8 @@ import org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository; import org.cloudfoundry.identity.uaa.test.TestAccountSetup; import org.cloudfoundry.identity.uaa.test.UaaTestAccounts; +import org.cloudfoundry.identity.uaa.util.JsonUtils; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.junit.After; import org.junit.Before; @@ -39,16 +41,20 @@ import org.springframework.security.oauth2.provider.client.BaseClientDetails; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; import java.net.URI; import java.net.URISyntaxException; import java.util.*; +import java.util.stream.Collectors; +import static org.cloudfoundry.identity.uaa.integration.util.IntegrationTestUtils.doesSupportZoneDNS; import static org.cloudfoundry.identity.uaa.integration.util.IntegrationTestUtils.getHeaders; import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_AUTHORIZATION_CODE; import static org.cloudfoundry.identity.uaa.security.web.CookieBasedCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.collection.IsCollectionWithSize.hasSize; import static org.hamcrest.core.Is.is; @@ -93,7 +99,6 @@ public class ScimGroupEndpointsIntegrationTests { private RestTemplate client; private List scimGroups; - private String zoneId; @Before public void createRestTemplate() { @@ -109,9 +114,8 @@ public boolean hasError(ClientHttpResponse response) { public void handleError(ClientHttpResponse response) { } }); - ScimUser joelScimUser = createUser("joel_" + new RandomValueStringGenerator().generate().toLowerCase(), "Passwo3d"); - zoneId = joelScimUser.getZoneId(); - JOEL = new ScimGroupMember(joelScimUser.getId()); + + JOEL = new ScimGroupMember(createUser("joel_" + new RandomValueStringGenerator().generate().toLowerCase(), "Passwo3d").getId()); DALE = new ScimGroupMember(createUser("dale_" + new RandomValueStringGenerator().generate().toLowerCase(), "Passwo3d").getId()); VIDYA = new ScimGroupMember(createUser("vidya_" + new RandomValueStringGenerator().generate().toLowerCase(), "Passwo3d").getId()); } @@ -212,29 +216,89 @@ public void createGroupSucceeds() { } @Test - public void createAllowedGroupSucceeds() { + public void createAllowedGroupSucceeds() throws URISyntaxException { + String testZoneId = "testzone1"; + assertTrue("Expected testzone1.localhost and testzone2.localhost to resolve to 127.0.0.1", doesSupportZoneDNS()); + String adminToken = IntegrationTestUtils.getClientCredentialsToken(serverRunning.getBaseUrl(), "admin", "adminsecret"); + IdentityZoneConfiguration config = new IdentityZoneConfiguration(); + config.getUserConfig().setAllowedGroups(allowedGroups); + String zoneUrl = serverRunning.getBaseUrl().replace("localhost", testZoneId + ".localhost"); + String inZoneAdminToken = IntegrationTestUtils.createClientAdminTokenInZone(serverRunning.getBaseUrl(), adminToken, testZoneId, config); + ScimGroup g1 = new ScimGroup(null, CFID, testZoneId); + // Check we can GET the group + ScimGroup g2 = IntegrationTestUtils.createOrUpdateGroup(inZoneAdminToken, null, zoneUrl, g1); + assertEquals(g1.getDisplayName(), g2.getDisplayName()); + assertEquals(g1.getDisplayName(), IntegrationTestUtils.getGroup(inZoneAdminToken, null, zoneUrl, g1.getDisplayName()).getDisplayName()); + } + + @Test + public void createNotAllowedGroupFailsCorrectly() throws URISyntaxException { + String testZoneId = "testzone1"; + assertTrue("Expected testzone1.localhost and testzone2.localhost to resolve to 127.0.0.1", doesSupportZoneDNS()); + final String NOT_ALLOWED = "not_allowed_" + new RandomValueStringGenerator().generate().toLowerCase(); + String adminToken = IntegrationTestUtils.getClientCredentialsToken(serverRunning.getBaseUrl(), "admin", "adminsecret"); + ScimGroup g1 = new ScimGroup(null, NOT_ALLOWED, testZoneId); + IdentityZoneConfiguration config = new IdentityZoneConfiguration(); + config.getUserConfig().setAllowedGroups(allowedGroups); + String zoneUrl = serverRunning.getBaseUrl().replace("localhost", testZoneId + ".localhost"); + String inZoneAdminToken = IntegrationTestUtils.createClientAdminTokenInZone(serverRunning.getBaseUrl(), adminToken, testZoneId, config); + RestTemplate template = new RestTemplate(); + HttpEntity entity = new HttpEntity<>(JsonUtils.writeValueAsBytes(g1), IntegrationTestUtils.getAuthenticatedHeaders(inZoneAdminToken)); try { - IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), zoneId, allowedGroups); - ScimGroup g1 = createGroup(CFID); - // Check we can GET the group - ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); - assertEquals(g1, g2); + template.exchange(zoneUrl + "/Groups", HttpMethod.POST, entity, HashMap.class); + fail("must fail"); + } catch (HttpClientErrorException e) { + assertTrue(e.getStatusCode().is4xxClientError()); + assertEquals(400, e.getRawStatusCode()); + assertThat(e.getMessage(), + containsString("The group with displayName: "+ g1.getDisplayName() +" is not allowed in Identity Zone " + testZoneId)); } finally { - IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), zoneId, null); // restore default + IntegrationTestUtils.deleteZone(serverRunning.getBaseUrl(), testZoneId, adminToken); } } @Test - public void createNotAllowedGroupFailsCorrectly() { - final String NOT_ALLOWED = "not_allowed_" + new RandomValueStringGenerator().generate().toLowerCase(); + public void relyOnDefaultGroupsShouldAllowedGroupSucceed() throws URISyntaxException { + String testZoneId = "testzone1"; + assertTrue("Expected testzone1.localhost and testzone2.localhost to resolve to 127.0.0.1", doesSupportZoneDNS()); + String adminToken = IntegrationTestUtils.getClientCredentialsToken(serverRunning.getBaseUrl(), "admin", "adminsecret"); + IdentityZoneConfiguration config = new IdentityZoneConfiguration(); + config.getUserConfig().setAllowedGroups(List.of()); + config.getUserConfig().setDefaultGroups(defaultGroups); + String zoneUrl = serverRunning.getBaseUrl().replace("localhost", testZoneId + ".localhost"); + String inZoneAdminToken = IntegrationTestUtils.createClientAdminTokenInZone(serverRunning.getBaseUrl(), adminToken, testZoneId, config); + ScimGroup ccRead = new ScimGroup(null, "cloud_controller_service_permissions.read", testZoneId); + ScimGroup g1 = IntegrationTestUtils.createGroup(inZoneAdminToken, null, zoneUrl, ccRead); + // Check we can GET the group + ScimGroup g2 = IntegrationTestUtils.createOrUpdateGroup(inZoneAdminToken, null, zoneUrl, g1); + assertEquals("cloud_controller_service_permissions.read", g2.getDisplayName()); + assertEquals("cloud_controller_service_permissions.read", IntegrationTestUtils.getGroup(inZoneAdminToken, null, zoneUrl, g1.getDisplayName()).getDisplayName()); + } + + @Test + public void changeDefaultGroupsAllowedGroupsUsageShouldFail() throws URISyntaxException { + String testZoneId = "testzone1"; + assertTrue("Expected testzone1.localhost and testzone2.localhost to resolve to 127.0.0.1", doesSupportZoneDNS()); + String adminToken = IntegrationTestUtils.getClientCredentialsToken(serverRunning.getBaseUrl(), "admin", "adminsecret"); + IdentityZoneConfiguration config = new IdentityZoneConfiguration(); + config.getUserConfig().setAllowedGroups(List.of()); + config.getUserConfig().setDefaultGroups(defaultGroups.stream().filter(g -> !g.equals("cloud_controller_service_permissions.read")).collect( + Collectors.toList())); + String zoneUrl = serverRunning.getBaseUrl().replace("localhost", testZoneId + ".localhost"); + String inZoneAdminToken = IntegrationTestUtils.createClientAdminTokenInZone(serverRunning.getBaseUrl(), adminToken, testZoneId, config); + RestTemplate template = new RestTemplate(); + ScimGroup g1 = new ScimGroup(null,"cloud_controller_service_permissions.read", testZoneId); + HttpEntity entity = new HttpEntity<>(JsonUtils.writeValueAsBytes(g1), IntegrationTestUtils.getAuthenticatedHeaders(inZoneAdminToken)); try { - IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), zoneId, allowedGroups); - ScimGroup g1 = createGroup(NOT_ALLOWED); - // Check we cannot GET the group - ScimGroup g2 = client.getForObject(serverRunning.getUrl(groupEndpoint + "/{id}"), ScimGroup.class, g1.getId()); - assertNull(g2); + template.exchange(zoneUrl + "/Groups", HttpMethod.POST, entity, HashMap.class); + fail("must fail"); + } catch (HttpClientErrorException e) { + assertTrue(e.getStatusCode().is4xxClientError()); + assertEquals(400, e.getRawStatusCode()); + assertThat(e.getMessage(), + containsString("The group with displayName: "+ g1.getDisplayName() +" is not allowed in Identity Zone " + testZoneId)); } finally { - IntegrationTestUtils.updateIdentityZoneAllowedGroups(client, serverRunning.getBaseUrl(), zoneId, null); // restore default + IntegrationTestUtils.deleteZone(serverRunning.getBaseUrl(), testZoneId, adminToken); } } diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java index 01171de880a..ab05a136118 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java @@ -676,16 +676,6 @@ public static IdentityZone createZoneOrUpdateSubdomain(RestTemplate client, return createZoneOrUpdateSubdomain(client, url, id, subdomain, config, true); } - public static IdentityZone updateIdentityZoneAllowedGroups(RestTemplate client, String url, String id, List allowedGroups) { - ResponseEntity zoneGet = client.getForEntity(url + "/identity-zones/{id}", IdentityZone.class); - if (!(zoneGet.getStatusCode() == HttpStatus.OK)) { - throw new RuntimeException("Could not get default zone."); - } - IdentityZone idz = zoneGet.getBody(); - idz.getConfig().getUserConfig().setAllowedGroups(allowedGroups); - return createZoneOrUpdateSubdomain(client, url, idz.getId(), idz.getSubdomain(), idz.getConfig(), idz.isActive()); - } - public static void addMemberToGroup(RestTemplate client, String url, String userId, @@ -1579,4 +1569,24 @@ public StatelessRequestFactory() { } } + public static HttpHeaders getAuthenticatedHeaders(String token) { + HttpHeaders headers = new HttpHeaders(); + headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON)); + headers.setContentType(MediaType.APPLICATION_JSON); + headers.set("Authorization", "Bearer " + token); + return headers; + } + + public static String createClientAdminTokenInZone(String baseUrl, String uaaAdminToken, String zoneId, IdentityZoneConfiguration config) { + RestTemplate identityClient = getClientCredentialsTemplate(getClientCredentialsResource(baseUrl, + new String[] { "zones.write", "zones.read", "scim.zones" }, "identity", "identitysecret")); + createZoneOrUpdateSubdomain(identityClient, baseUrl, zoneId, zoneId, config); + String zoneUrl = baseUrl.replace("localhost", zoneId + ".localhost"); + BaseClientDetails zoneClient = new BaseClientDetails("admin-client-in-zone", null, "openid", + "authorization_code,client_credentials", "uaa.admin,scim.read,scim.write,zones.testzone1.admin ", zoneUrl); + zoneClient.setClientSecret("admin-secret-in-zone"); + createOrUpdateClient(uaaAdminToken, baseUrl, zoneId, zoneClient); + return getClientCredentialsToken(zoneUrl, "admin-client-in-zone", "admin-secret-in-zone"); + } + } From 1658023e685076789ec5b8816837142c76c0cb39 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 24 Nov 2023 10:59:41 +0100 Subject: [PATCH 28/36] cleanup in integration tests, fixes mftop refactor to omit dublicates --- .../scim/jdbc/JdbcScimGroupProvisioning.java | 20 +++++++++---------- .../ScimGroupEndpointsIntegrationTests.java | 2 ++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java index 2bd9d535fbf..508a74821a5 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java @@ -247,11 +247,7 @@ private Set getAllowedUserGroups(String zoneId) { @Override public ScimGroup create(final ScimGroup group, final String zoneId) throws InvalidScimResourceException { - Set allowedGroups = getAllowedUserGroups(zoneId); - if ((allowedGroups != null) && (!allowedGroups.contains(group.getDisplayName()))) { - throw new InvalidScimResourceException("The group with displayName: " + group.getDisplayName() - + " is not allowed in Identity Zone " + zoneId); - } + validateAllowedUserGroups(zoneId, group); final String id = UUID.randomUUID().toString(); logger.debug("creating new group with id: {}", id); try { @@ -276,11 +272,7 @@ public ScimGroup create(final ScimGroup group, final String zoneId) throws Inval @Override public ScimGroup update(final String id, final ScimGroup group, final String zoneId) throws InvalidScimResourceException, ScimResourceNotFoundException { - Set allowedGroups = getAllowedUserGroups(zoneId); - if ((allowedGroups != null) && (!allowedGroups.contains(group.getDisplayName()))) { - throw new InvalidScimResourceException("The group with displayName: " + group.getDisplayName() - + " is not allowed in Identity Zone " + zoneId); - } + validateAllowedUserGroups(zoneId, group); try { validateGroup(group); @@ -350,4 +342,12 @@ protected void validateOrderBy(String orderBy) throws IllegalArgumentException { super.validateOrderBy(orderBy, GROUP_FIELDS); } + private void validateAllowedUserGroups(String zoneId, ScimGroup group) { + Set allowedGroups = getAllowedUserGroups(zoneId); + if ((allowedGroups != null) && (!allowedGroups.contains(group.getDisplayName()))) { + throw new InvalidScimResourceException("The group with displayName: " + group.getDisplayName() + + " is not allowed in Identity Zone " + zoneId); + } + } + } diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index 8f43b861c6c..816fe594505 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -229,6 +229,7 @@ public void createAllowedGroupSucceeds() throws URISyntaxException { ScimGroup g2 = IntegrationTestUtils.createOrUpdateGroup(inZoneAdminToken, null, zoneUrl, g1); assertEquals(g1.getDisplayName(), g2.getDisplayName()); assertEquals(g1.getDisplayName(), IntegrationTestUtils.getGroup(inZoneAdminToken, null, zoneUrl, g1.getDisplayName()).getDisplayName()); + IntegrationTestUtils.deleteZone(serverRunning.getBaseUrl(), testZoneId, adminToken); } @Test @@ -273,6 +274,7 @@ public void relyOnDefaultGroupsShouldAllowedGroupSucceed() throws URISyntaxExcep ScimGroup g2 = IntegrationTestUtils.createOrUpdateGroup(inZoneAdminToken, null, zoneUrl, g1); assertEquals("cloud_controller_service_permissions.read", g2.getDisplayName()); assertEquals("cloud_controller_service_permissions.read", IntegrationTestUtils.getGroup(inZoneAdminToken, null, zoneUrl, g1.getDisplayName()).getDisplayName()); + IntegrationTestUtils.deleteZone(serverRunning.getBaseUrl(), testZoneId, adminToken); } @Test From cc27f6661a541d1462b07af49c245803869d4bac Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 14 Dec 2023 17:22:58 +0100 Subject: [PATCH 29/36] change log level to info --- .../org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java index 02703c3383f..6f697d8f94f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java @@ -16,7 +16,7 @@ public static void validate(UserConfig config) throws InvalidIdentityZoneConfigu Set allowedGroups = (config == null) ? null : config.resultingAllowedGroups(); if ((allowedGroups != null) && (allowedGroups.isEmpty())) { String message = "At least one group must be allowed"; - logger.error(message); + logger.info(message); throw new InvalidIdentityZoneConfigurationException(message); } } From ef94dea12c7af9305d06317acb0daf34fd5c7c6f Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 14 Dec 2023 17:27:08 +0100 Subject: [PATCH 30/36] remove log message --- .../cloudfoundry/identity/uaa/zone/UserConfigValidator.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java index 6f697d8f94f..e16cb667926 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfigValidator.java @@ -2,11 +2,7 @@ import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - public class UserConfigValidator { - private static Logger logger = LoggerFactory.getLogger(UserConfigValidator.class); // add a private constructor to hide the implicit public one private UserConfigValidator() { @@ -16,7 +12,6 @@ public static void validate(UserConfig config) throws InvalidIdentityZoneConfigu Set allowedGroups = (config == null) ? null : config.resultingAllowedGroups(); if ((allowedGroups != null) && (allowedGroups.isEmpty())) { String message = "At least one group must be allowed"; - logger.info(message); throw new InvalidIdentityZoneConfigurationException(message); } } From 56adc44dbf4b0281bf8044e852fce5ce3d60253a Mon Sep 17 00:00:00 2001 From: D023954 Date: Fri, 15 Dec 2023 17:17:19 +0100 Subject: [PATCH 31/36] check for groups which would be not allowed after the update --- .../cloudfoundry/identity/uaa/zone/UserConfig.java | 6 +++++- .../identity/uaa/zone/IdentityZoneEndpoints.java | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java index 9fbff518d0e..1ed0e5b474a 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java @@ -41,10 +41,14 @@ public void setAllowedGroups(List allowedGroups) { this.allowedGroups = allowedGroups; } + public boolean allGroupsAllowed() { + return (allowedGroups == null); + } + // return defaultGroups plus allowedGroups @SuppressWarnings("java:S1168") public Set resultingAllowedGroups() { - if (allowedGroups == null) { + if (allGroupsAllowed()) { return null; // null = all groups allowed } else { HashSet allAllowedGroups = new HashSet<>(allowedGroups); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java index 1389890c106..778dabe37e6 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java @@ -47,6 +47,7 @@ import java.util.Locale; import java.util.Map; import java.util.UUID; +import java.util.stream.Collectors; import static java.util.Optional.ofNullable; import static org.springframework.http.HttpStatus.CONFLICT; @@ -267,6 +268,18 @@ public ResponseEntity updateIdentityZone( body.setId(id); body = validator.validate(body, IdentityZoneValidator.Mode.MODIFY); + // check for groups which would be not allowed after the update + UserConfig userConfig = body.getConfig().getUserConfig(); + if (!userConfig.allGroupsAllowed()) { + List existingGroupNames = groupProvisioning.retrieveAll(body.getId()) + .stream() + .map(ScimGroup::getDescription) + .collect(Collectors.toList()); + if (!userConfig.resultingAllowedGroups().containsAll(existingGroupNames)) { + throw new UnprocessableEntityException("The identity zone details contains not-allowed groups."); + } + } + logger.debug("Zone - updating id[{}] subdomain[{}]", UaaStringUtils.getCleanedUserControlString(id), UaaStringUtils.getCleanedUserControlString(body.getSubdomain()) From 86e7f0312b306ec27667d63d23a7dd60bfac54c9 Mon Sep 17 00:00:00 2001 From: D023954 Date: Mon, 18 Dec 2023 12:34:14 +0100 Subject: [PATCH 32/36] must not remove default group from configuration --- .../ScimGroupEndpointsIntegrationTests.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java index 816fe594505..12b8170d24d 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ScimGroupEndpointsIntegrationTests.java @@ -278,27 +278,25 @@ public void relyOnDefaultGroupsShouldAllowedGroupSucceed() throws URISyntaxExcep } @Test - public void changeDefaultGroupsAllowedGroupsUsageShouldFail() throws URISyntaxException { + public void changeDefaultGroupsAllowedGroupsUsageShouldSucceed() throws URISyntaxException { String testZoneId = "testzone1"; assertTrue("Expected testzone1.localhost and testzone2.localhost to resolve to 127.0.0.1", doesSupportZoneDNS()); String adminToken = IntegrationTestUtils.getClientCredentialsToken(serverRunning.getBaseUrl(), "admin", "adminsecret"); IdentityZoneConfiguration config = new IdentityZoneConfiguration(); + final String ALLOWED = "allowed_" + new RandomValueStringGenerator().generate().toLowerCase(); + List newDefaultGroups = new ArrayList(defaultGroups); + newDefaultGroups.add(ALLOWED); config.getUserConfig().setAllowedGroups(List.of()); - config.getUserConfig().setDefaultGroups(defaultGroups.stream().filter(g -> !g.equals("cloud_controller_service_permissions.read")).collect( - Collectors.toList())); + config.getUserConfig().setDefaultGroups(newDefaultGroups); String zoneUrl = serverRunning.getBaseUrl().replace("localhost", testZoneId + ".localhost"); String inZoneAdminToken = IntegrationTestUtils.createClientAdminTokenInZone(serverRunning.getBaseUrl(), adminToken, testZoneId, config); RestTemplate template = new RestTemplate(); - ScimGroup g1 = new ScimGroup(null,"cloud_controller_service_permissions.read", testZoneId); + ScimGroup g1 = new ScimGroup(null,ALLOWED, testZoneId); HttpEntity entity = new HttpEntity<>(JsonUtils.writeValueAsBytes(g1), IntegrationTestUtils.getAuthenticatedHeaders(inZoneAdminToken)); try { template.exchange(zoneUrl + "/Groups", HttpMethod.POST, entity, HashMap.class); - fail("must fail"); - } catch (HttpClientErrorException e) { - assertTrue(e.getStatusCode().is4xxClientError()); - assertEquals(400, e.getRawStatusCode()); - assertThat(e.getMessage(), - containsString("The group with displayName: "+ g1.getDisplayName() +" is not allowed in Identity Zone " + testZoneId)); + } catch (Exception e) { + fail("must not fail"); } finally { IntegrationTestUtils.deleteZone(serverRunning.getBaseUrl(), testZoneId, adminToken); } From 5ddf0caedfcf12d4fd910159196b78f78aabca9f Mon Sep 17 00:00:00 2001 From: Markus Strehle <11627201+strehle@users.noreply.github.com> Date: Tue, 19 Dec 2023 11:49:44 +0100 Subject: [PATCH 33/36] remove sonar comment --- .../main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java | 1 - 1 file changed, 1 deletion(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java index 1ed0e5b474a..702cde9b7b8 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/zone/UserConfig.java @@ -46,7 +46,6 @@ public boolean allGroupsAllowed() { } // return defaultGroups plus allowedGroups - @SuppressWarnings("java:S1168") public Set resultingAllowedGroups() { if (allGroupsAllowed()) { return null; // null = all groups allowed From ab81926a15d71ab91fc4f2fb6da9df53429456c6 Mon Sep 17 00:00:00 2001 From: D023954 Date: Thu, 21 Dec 2023 10:59:14 +0100 Subject: [PATCH 34/36] fix: check for existing groups --- .../uaa/zone/IdentityZoneEndpoints.java | 14 +++---- ...IdentityZoneEndpointsIntegrationTests.java | 37 +++++++++++++++++++ 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java index 778dabe37e6..aeaffe7e3c9 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java @@ -46,8 +46,8 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.UUID; -import java.util.stream.Collectors; import static java.util.Optional.ofNullable; import static org.springframework.http.HttpStatus.CONFLICT; @@ -271,12 +271,12 @@ public ResponseEntity updateIdentityZone( // check for groups which would be not allowed after the update UserConfig userConfig = body.getConfig().getUserConfig(); if (!userConfig.allGroupsAllowed()) { - List existingGroupNames = groupProvisioning.retrieveAll(body.getId()) - .stream() - .map(ScimGroup::getDescription) - .collect(Collectors.toList()); - if (!userConfig.resultingAllowedGroups().containsAll(existingGroupNames)) { - throw new UnprocessableEntityException("The identity zone details contains not-allowed groups."); + List existingGroups = groupProvisioning.retrieveAll(body.getId()); + Set allowedGroups = userConfig.resultingAllowedGroups(); + for (ScimGroup g: existingGroups) { + if (!allowedGroups.contains(g.getDisplayName())) { + throw new UnprocessableEntityException("The identity zone contains a not-allowed group: "+g.getDisplayName()); + } } } diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/IdentityZoneEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/IdentityZoneEndpointsIntegrationTests.java index 34931c0980d..4cc3a45c8c8 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/IdentityZoneEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/IdentityZoneEndpointsIntegrationTests.java @@ -144,6 +144,43 @@ public void testCreateZone() { assertNull(ObjectUtils.castInstance(identityProvider.getConfig(),UaaIdentityProviderDefinition.class).getPasswordPolicy()); } + @Test + public void testUpdateZoneAllowedGroups() { + IdentityZone idZone = new IdentityZone(); + String id = UUID.randomUUID().toString(); + idZone.setId(id); + idZone.setSubdomain(id); + idZone.setName("testUpdateZone-"+id); + ResponseEntity response = client.exchange( + serverRunning.getUrl("/identity-zones"), + HttpMethod.POST, + new HttpEntity<>(idZone), + new ParameterizedTypeReference() {}, + id); + assertEquals(response.getBody(), HttpStatus.CREATED, response.getStatusCode()); + + List existingGroups = List.of("sps.write", "sps.read", "idps.write", "idps.read", "clients.admin", "clients.write", "clients.read", + "clients.secret", "scim.write", "scim.read", "scim.create", "scim.userids", "scim.zones", "groups.update", "password.write", "oauth.login", "uaa.admin"); + idZone.getConfig().getUserConfig().setAllowedGroups(existingGroups); + response = client.exchange( + serverRunning.getUrl("/identity-zones/"+id), + HttpMethod.PUT, + new HttpEntity<>(idZone), + new ParameterizedTypeReference() {}, + id); + assertEquals(response.getBody() , HttpStatus.OK, response.getStatusCode()); + + List notAllExistingGroups = List.of("clients.admin", "clients.write", "clients.read", "clients.secret"); + idZone.getConfig().getUserConfig().setAllowedGroups(notAllExistingGroups); + response = client.exchange( + serverRunning.getUrl("/identity-zones/"+id), + HttpMethod.PUT, + new HttpEntity<>(idZone), + new ParameterizedTypeReference() {}, + id); + assertEquals(response.getBody() , HttpStatus.UNPROCESSABLE_ENTITY, response.getStatusCode()); + } + @Test public void testCreateZoneWithClient() { IdentityZone idZone = new IdentityZone(); From cc600f57cd2e1e75395f705fc831595d19c3adf0 Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 17 Jan 2024 15:01:35 +0100 Subject: [PATCH 35/36] add maxUsers to sample configuration --- .../org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java | 1 + .../cloudfoundry/identity/uaa/zone/SampleIdentityZone.json | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java index 59fa780794f..b751f21af07 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java @@ -138,5 +138,6 @@ void deserialize() { "profile", "roles", "user_attributes", "uaa.offline_token", "scim.me", "cloud_controller.user"), sampleIdentityZone.getConfig().getUserConfig().resultingAllowedGroups()); + assertEquals(16, sampleIdentityZone.getConfig().getUserConfig().getMaxUsers()); } } \ No newline at end of file diff --git a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json index 3cbc9812f5e..71544a6bbd9 100644 --- a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json +++ b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json @@ -126,7 +126,8 @@ "allowedGroups": [ "scim.me", "cloud_controller.user" - ] + ], + "maxUsers": 16 } }, "name": "Demo Login Page", @@ -134,4 +135,4 @@ "description": "{\"plan_display_name\":\"Demo\",\"plan_description\":\"Demo SSO Plan\"}", "created": 1503504273000, "last_modified": 1504898224000 -} \ No newline at end of file +} From a6d0383ad9d8eb1b7eeaabc3bdaf653383771b5b Mon Sep 17 00:00:00 2001 From: D023954 Date: Wed, 17 Jan 2024 17:14:33 +0100 Subject: [PATCH 36/36] increase maxUsers --- .../org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java | 2 +- .../org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java index b751f21af07..04f2ca17de3 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneTest.java @@ -138,6 +138,6 @@ void deserialize() { "profile", "roles", "user_attributes", "uaa.offline_token", "scim.me", "cloud_controller.user"), sampleIdentityZone.getConfig().getUserConfig().resultingAllowedGroups()); - assertEquals(16, sampleIdentityZone.getConfig().getUserConfig().getMaxUsers()); + assertEquals(1000, sampleIdentityZone.getConfig().getUserConfig().getMaxUsers()); } } \ No newline at end of file diff --git a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json index 71544a6bbd9..e5b23d89c8b 100644 --- a/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json +++ b/model/src/test/resources/org/cloudfoundry/identity/uaa/zone/SampleIdentityZone.json @@ -127,7 +127,7 @@ "scim.me", "cloud_controller.user" ], - "maxUsers": 16 + "maxUsers": 1000 } }, "name": "Demo Login Page",