Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allowedGroups to UserConfig in IdZ configuration #2606

Merged
merged 40 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
214dc88
fix sonar issue: Utility classes should not have public constructors
klaus-sap Oct 10, 2023
d02780e
Merge branch 'cloudfoundry:develop' into develop
klaus-sap Oct 12, 2023
931f880
fix: possibly unnecessary method call
klaus-sap Oct 18, 2023
57bc472
fix: define Java version
klaus-sap Oct 18, 2023
0352bb6
handle in separate PR
klaus-sap Oct 20, 2023
12d1a44
Merge branch 'cloudfoundry:develop' into develop
klaus-sap Oct 23, 2023
006e134
Merge branch 'cloudfoundry:develop' into develop
klaus-sap Oct 30, 2023
9a34f0a
feature: add attribute userConfig.allowedGroups to IdZ
klaus-sap Oct 30, 2023
71b7abb
Merge branch 'cloudfoundry:develop' into develop
klaus-sap Nov 17, 2023
7d2fb2d
revert format changes
strehle Nov 18, 2023
d4c52f4
revert format changes
strehle Nov 18, 2023
ec56ea6
fix Sonar warnings
klaus-sap Nov 20, 2023
ef8f6ca
remove TODO
klaus-sap Nov 20, 2023
9dd1e38
create new needed bean similar than the other beans created
strehle Nov 20, 2023
244110c
is an optional entry in identity zone configuration
strehle Nov 20, 2023
85891ed
Merge branch 'develop' of github.com:cloudfoundry/uaa into klaus-develop
strehle Nov 20, 2023
c7eb6d7
fix sonar smells
strehle Nov 20, 2023
6e6ab18
combine default and allowed groups
klaus-sap Nov 21, 2023
979f933
fix Sonar warnings
klaus-sap Nov 22, 2023
8cde93e
add test class
klaus-sap Nov 22, 2023
b7adddd
add testResultingAllowedGroups
klaus-sap Nov 22, 2023
a464bab
add testScopesIncludesAllowedAuthoritiesForUser
klaus-sap Nov 22, 2023
370be52
zoneId must not be null
klaus-sap Nov 22, 2023
a6722b5
use right asserts
klaus-sap Nov 22, 2023
ab2d5cf
remove null-check
klaus-sap Nov 23, 2023
bd87bed
add integration tests
klaus-sap Nov 23, 2023
fdc6b02
fix createNotAllowedGroupFails
klaus-sap Nov 23, 2023
f400598
introduce list allowedGroups
klaus-sap Nov 23, 2023
163194c
update allowedGroups on server
klaus-sap Nov 23, 2023
1e02b38
pass IdZ id
klaus-sap Nov 23, 2023
c9810f7
determine zone id
klaus-sap Nov 23, 2023
7102503
Merge branch 'develop' of github.com:cloudfoundry/uaa into integratio…
strehle Nov 24, 2023
1291228
update integration tests
strehle Nov 23, 2023
1658023
cleanup in integration tests, fixes mftop
strehle Nov 24, 2023
d1a2cac
Merge remote-tracking branch 'origin/develop' into klaus-rebase
strehle Dec 11, 2023
cc27f66
change log level to info
klaus-sap Dec 14, 2023
ef94dea
remove log message
klaus-sap Dec 14, 2023
56adc44
check for groups which would be not allowed after the update
klaus-sap Dec 15, 2023
86e7f03
must not remove default group from configuration
klaus-sap Dec 18, 2023
5ddf0ca
remove sonar comment
strehle Dec 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;

klaus-sap marked this conversation as resolved.
Show resolved Hide resolved
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
hsinn0 marked this conversation as resolved.
Show resolved Hide resolved
} else {
HashSet<String> allAllowedGroups = new HashSet<>(allowedGroups);
if (defaultGroups != null) allAllowedGroups.addAll(defaultGroups);
return allAllowedGroups;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@
"roles",
"user_attributes",
"uaa.offline_token"
],
"allowedGroups": [
"scim.me",
"cloud_controller.user"
]
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
klaus-sap marked this conversation as resolved.
Show resolved Hide resolved
}

// Find intersection of user authorities, default requestedScopes and client requestedScopes:
Set<String> result = intersectScopes(new LinkedHashSet<>(requestedScopes), clientDetails.getScope(), allowed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -66,6 +70,7 @@ public Logger getLogger() {

private JdbcScimGroupExternalMembershipManager jdbcScimGroupExternalMembershipManager;
private JdbcScimGroupMembershipManager jdbcScimGroupMembershipManager;
private JdbcIdentityZoneProvisioning jdbcIdentityZoneProvisioning;

public JdbcScimGroupProvisioning(
final JdbcTemplate jdbcTemplate,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -220,9 +229,24 @@ public ScimGroup retrieve(String id, final String zoneId) throws ScimResourceNot
}
}

@SuppressWarnings("java:S1874")
strehle marked this conversation as resolved.
Show resolved Hide resolved
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();
hsinn0 marked this conversation as resolved.
Show resolved Hide resolved
} 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 {
Expand All @@ -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);
Expand Down Expand Up @@ -324,4 +349,12 @@ protected void validateOrderBy(String orderBy) throws IllegalArgumentException {
super.validateOrderBy(orderBy, GROUP_FIELDS);
}

private void validateAllowedUserGroups(String zoneId, ScimGroup group) {
hsinn0 marked this conversation as resolved.
Show resolved Hide resolved
Set<String> allowedGroups = getAllowedUserGroups(zoneId);
if ((allowedGroups != null) && (!allowedGroups.contains(group.getDisplayName()))) {
hsinn0 marked this conversation as resolved.
Show resolved Hide resolved
throw new InvalidScimResourceException("The group with displayName: " + group.getDisplayName()
+ " is not allowed in Identity Zone " + zoneId);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
klaus-sap marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Loading