Add allowedGroups to UserConfig in IdZ configuration (#2606)

* fix sonar issue: Utility classes should not have public constructors * fix: possibly unnecessary method call * fix: define Java version * handle in separate PR * feature: add attribute userConfig.allowedGroups to IdZ * revert format changes * revert format changes * fix Sonar warnings * remove TODO * create new needed bean similar than the other beans created * is an optional entry in identity zone configuration * fix sonar smells * combine default and allowed groups * fix Sonar warnings * add test class * add testResultingAllowedGroups * add testScopesIncludesAllowedAuthoritiesForUser * zoneId must not be null * use right asserts * remove null-check * add integration tests * fix createNotAllowedGroupFails * introduce list allowedGroups * update allowedGroups on server * pass IdZ id * determine zone id * update integration tests We have 4 tests 2 positive and 2 negative 2 using allowedGroups 2 relying on defaultGroups because allowedGroups is empty * cleanup in integration tests, fixes mftop refactor to omit dublicates * change log level to info * remove log message * check for groups which would be not allowed after the update * must not remove default group from configuration * remove sonar comment --------- Co-authored-by: Markus Strehle <11627201+strehle@users.noreply.github.com> Co-authored-by: d036670 <markus.strehle@sap.com>
cloudfoundry · Dec 20, 2023 · 63ad5f3 · 63ad5f3
1 parent ee33241
commit 63ad5f3
Show file tree

Hide file tree

Showing 24 changed files with 449 additions and 25 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
@@ -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)
@@ -27,4 +29,30 @@ public List<String> getDefaultGroups() {
     public void setDefaultGroups(List<String> defaultGroups) {
         this.defaultGroups = defaultGroups;
     }
+
+    // in addition to defaultGroups, which are implicitely allowed
+    private List<String> allowedGroups = null;
+
+    public List<String> getAllowedGroups() {
+        return allowedGroups;
+    }
+
+    public void setAllowedGroups(List<String> allowedGroups) {
+        this.allowedGroups = allowedGroups;
+    }
+
+    public boolean allGroupsAllowed() {
+        return (allowedGroups == null);
+    }
+
+    // return defaultGroups plus allowedGroups
+    public Set<String> resultingAllowedGroups() {
+        if (allGroupsAllowed()) {
+            return null; // null = all groups allowed
+        } else {
+            HashSet<String> 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
@@ -10,11 +10,14 @@
 
 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;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
 
 class IdentityZoneTest {
 
@@ -124,8 +127,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(Set.of("openid", "password.write", "uaa.user", "approvals.me",
+                            "profile", "roles", "user_attributes", "uaa.offline_token",
+                            "scim.me", "cloud_controller.user"),
+                            sampleIdentityZone.getConfig().getUserConfig().resultingAllowedGroups());
     }
 }
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
@@ -0,0 +1,51 @@
+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;
+import java.util.Set;
+
+import org.junit.Test;
+
+public class UserConfigTest {
+
+    @Test
+    public void testDefaultConfig() {
+        UserConfig userConfig = new UserConfig();
+        assertTrue(userConfig.getDefaultGroups().contains("openid"));
+        assertNull(userConfig.getAllowedGroups());       // all groups allowed
+        assertNull(userConfig.resultingAllowedGroups()); // all groups allowed
+    }
+
+    @Test
+    public void testResultingAllowedGroups() {
+        UserConfig userConfig = new UserConfig();
+        userConfig.setDefaultGroups(List.of("openid"));
+        userConfig.setAllowedGroups(List.of("uaa.user"));
+        assertEquals(List.of("openid"), userConfig.getDefaultGroups());
+        assertEquals(List.of("uaa.user"), userConfig.getAllowedGroups());
+        assertEquals(Set.of("openid", "uaa.user"), userConfig.resultingAllowedGroups());
+    }
+
+    @Test
+    public void testNoDefaultGroups() {
+        UserConfig userConfig = new UserConfig();
+        userConfig.setDefaultGroups(null);
+        userConfig.setAllowedGroups(List.of("uaa.user"));
+        assertNull(userConfig.getDefaultGroups());
+        assertEquals(List.of("uaa.user"), userConfig.getAllowedGroups());
+        assertEquals(Set.of("uaa.user"), userConfig.resultingAllowedGroups());
+    }
+
+    @Test
+    public void testNoDefaultAndNoAllowedGroups() {
+        UserConfig userConfig = new UserConfig();
+        userConfig.setDefaultGroups(null);
+        userConfig.setAllowedGroups(null);
+        assertNull(userConfig.getDefaultGroups());
+        assertNull(userConfig.getAllowedGroups());       // all groups allowed
+        assertNull(userConfig.resultingAllowedGroups()); // all groups allowed
+    }
+}
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
@@ -122,6 +122,10 @@
         "roles",
         "user_attributes",
         "uaa.offline_token"
+      ],
+      "allowedGroups": [
+        "scim.me",
+        "cloud_controller.user"
       ]
     }
   },

diff --git a/...in/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java b/...in/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java
@@ -67,6 +67,8 @@ public class IdentityZoneConfigurationBootstrap implements InitializingBean {
 
     private Collection<String> defaultUserGroups;
 
+    private Collection<String> allowedUserGroups;
+
     private IdentityZoneValidator validator = (config, mode) -> config;
     private Map<String, Object> 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<String> defaultUserGroups) {
         this.defaultUserGroups = defaultUserGroups;
     }
 
+    public void setAllowedUserGroups(Collection<String> 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
@@ -243,13 +243,18 @@ 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<String> checkUserScopes(Set<String> requestedScopes,
                                         Collection<? extends GrantedAuthority> authorities,
                                         ClientDetails clientDetails) {
         Set<String> allowed = new LinkedHashSet<>(AuthorityUtils.authorityListToSet(authorities));
         // Add in all default requestedScopes
         Collection<String> defaultScopes = IdentityZoneHolder.get().getConfig().getUserConfig().getDefaultGroups();
+        Collection<String> allowedScopes = IdentityZoneHolder.get().getConfig().getUserConfig().resultingAllowedGroups();
         allowed.addAll(defaultScopes);
+        if (allowedScopes != null) {
+            allowed.retainAll(allowedScopes);
+        }
 
         // Find intersection of user authorities, default requestedScopes and client requestedScopes:
         Set<String> 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
@@ -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;
@@ -25,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;
@@ -66,6 +70,7 @@ public Logger getLogger() {
 
     private JdbcScimGroupExternalMembershipManager jdbcScimGroupExternalMembershipManager;
     private JdbcScimGroupMembershipManager jdbcScimGroupMembershipManager;
+    private JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning;
 
     public JdbcScimGroupProvisioning(
             final JdbcTemplate jdbcTemplate,
@@ -153,6 +158,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);
@@ -220,9 +229,24 @@ public ScimGroup retrieve(String id, final String zoneId) throws ScimResourceNot
         }
     }
 
+    @SuppressWarnings("java:S1874")
+    private Set<String> getAllowedUserGroups(String zoneId) {
+        Set<String> zoneAllowedGroups = null; // default: all groups allowed
+        try {
+            IdentityZone currentZone = IdentityZoneHolder.get();
+            zoneAllowedGroups = (currentZone.getId().equals(zoneId)) ?
+                currentZone.getConfig().getUserConfig().resultingAllowedGroups() :
+                jdbcIdentityZoneProvisioning.retrieve(zoneId).getConfig().getUserConfig().resultingAllowedGroups();
+        } 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 {
         validateZoneId(zoneId);
+        validateAllowedUserGroups(zoneId, group);
         final String id = UUID.randomUUID().toString();
         logger.debug("creating new group with id: {}", id);
         try {
@@ -247,6 +271,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 {
+        validateAllowedUserGroups(zoneId, group);
         try {
             validateZoneId(zoneId);
             validateGroup(group);
@@ -324,4 +349,12 @@ protected void validateOrderBy(String orderBy) throws IllegalArgumentException {
         super.validateOrderBy(orderBy, GROUP_FIELDS);
     }
 
+    private void validateAllowedUserGroups(String zoneId, ScimGroup group) {
+        Set<String> 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
@@ -265,6 +265,10 @@ private String getAuthorities(final String userId) throws SQLException {
             Set<String> authorities = new HashSet<>();
             getAuthorities(authorities, Collections.singletonList(userId));
             authorities.addAll(identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().getDefaultGroups());
+            Set<String> allowedGroups = identityZoneManager.getCurrentIdentityZone().getConfig().getUserConfig().resultingAllowedGroups();
+            if (allowedGroups != null) {
+                authorities.retainAll(allowedGroups);
+            }
             return StringUtils.collectionToCommaDelimitedString(new HashSet<>(authorities));
         }
 

diff --git a/...in/java/org/cloudfoundry/identity/uaa/zone/GeneralIdentityZoneConfigurationValidator.java b/...in/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/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<IdentityZone> 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<String> 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())

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
@@ -0,0 +1,18 @@
+package org.cloudfoundry.identity.uaa.zone;
+
+import java.util.Set;
+
+public class UserConfigValidator {
+
+    // add a private constructor to hide the implicit public one
+    private UserConfigValidator() {
+    }
+
+    public static void validate(UserConfig config) throws InvalidIdentityZoneConfigurationException {
+        Set<String> allowedGroups = (config == null) ? null : config.resultingAllowedGroups();
+        if ((allowedGroups != null) && (allowedGroups.isEmpty())) {
+            String message = "At least one group must be allowed";
+            throw new InvalidIdentityZoneConfigurationException(message);
+        }
+    }
+}