From a0cc13dc02e67be3436408cc0fc0853b47386972 Mon Sep 17 00:00:00 2001 From: Brian Long Date: Mon, 23 Aug 2021 16:06:44 -0400 Subject: [PATCH] allowing org/cap group types; other fixes --- ...nteligr8SecurityConfigurationRegistry.java | 45 ++++++-- ...AbstractKeycloakActivitiAuthenticator.java | 28 ++++- .../KeycloakActivitiAppAuthenticator.java | 107 +++++++++++------- .../KeycloakActivitiEngineAuthenticator.java | 22 ++-- 4 files changed, 145 insertions(+), 57 deletions(-) diff --git a/src/main/java/com/inteligr8/activiti/Inteligr8SecurityConfigurationRegistry.java b/src/main/java/com/inteligr8/activiti/Inteligr8SecurityConfigurationRegistry.java index cd3b0bf..735fd1f 100644 --- a/src/main/java/com/inteligr8/activiti/Inteligr8SecurityConfigurationRegistry.java +++ b/src/main/java/com/inteligr8/activiti/Inteligr8SecurityConfigurationRegistry.java @@ -5,7 +5,9 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -17,6 +19,7 @@ import org.springframework.stereotype.Component; import com.activiti.api.security.AlfrescoSecurityConfigOverride; import com.activiti.domain.idm.Group; +import com.activiti.domain.idm.GroupCapability; import com.activiti.domain.idm.Tenant; import com.activiti.domain.idm.User; import com.activiti.service.api.GroupService; @@ -40,6 +43,15 @@ public class Inteligr8SecurityConfigurationRegistry implements AlfrescoSecurityC private final Logger logger = LoggerFactory.getLogger(this.getClass()); + private final List adminCapabilities = Arrays.asList( + "access-all-models-in-tenant", + "access-editor", + "access-reports", + "publish-app-to-dashboard", + "tenant-admin", + "tenant-admin-api", + "upload-license"); + @Autowired private List adapters; @@ -61,7 +73,7 @@ public class Inteligr8SecurityConfigurationRegistry implements AlfrescoSecurityC @Value("${keycloak-ext.group.admins.name:admins}") private String adminGroupName; - @Value("${keycloak-ext.group.admins.externalId:aps-admin}") + @Value("${keycloak-ext.group.admins.externalId:#{null}}") private String adminGroupExternalId; @Value("${keycloak-ext.group.admins.validate:false}") @@ -108,15 +120,32 @@ public class Inteligr8SecurityConfigurationRegistry implements AlfrescoSecurityC return; Long tenantId = this.findDefaultTenantId(); - Group group = this.groupService.getGroupByExternalId(this.adminGroupExternalId); + Group group = this.groupService.getGroupByExternalIdAndTenantId(this.adminGroupExternalId, tenantId); if (group == null) { - this.logger.info("Creating '{}' group ...", this.adminGroupName); - group = this.groupService.createGroupFromExternalStore( - this.adminGroupExternalId, tenantId, Group.TYPE_SYSTEM_GROUP, null, this.adminGroupName, new Date()); + List groups = this.groupService.getGroupByNameAndTenantId(this.adminGroupName, tenantId); + if (!groups.isEmpty()) + group = groups.iterator().next(); + } + + if (group == null) { + this.logger.info("Creating group: {} ({})", this.adminGroupName, this.adminGroupExternalId); + if (this.adminGroupExternalId != null) { + group = this.groupService.createGroupFromExternalStore( + this.adminGroupExternalId, tenantId, Group.TYPE_SYSTEM_GROUP, null, this.adminGroupName, new Date()); + } else { + group = this.groupService.createGroup(this.adminGroupName, tenantId, Group.TYPE_SYSTEM_GROUP, null); + } + } + + this.logger.debug("Checking group capabilities: {}", group.getName()); + Group groupWithCaps = this.groupService.getGroup(group.getId(), false, true, false, false); + Set adminCaps = new HashSet<>(this.adminCapabilities); + for (GroupCapability cap : groupWithCaps.getCapabilities()) + adminCaps.remove(cap.getName()); + if (!adminCaps.isEmpty()) { + this.logger.info("Granting group '{}' capabilities: {}", group.getName(), adminCaps); + this.groupService.addCapabilitiesToGroup(group.getId(), new ArrayList<>(adminCaps)); } - - this.logger.info("Granting '{}' group all capabilities ...", group.getName()); - this.groupService.addCapabilitiesToGroup(group.getId(), Arrays.asList("access-all-models-in-tenant", "access-editor", "access-reports", "publish-app-to-dashboard", "tenant-admin", "tenant-admin-api", "upload-license")); } private void associateAdmins() { diff --git a/src/main/java/com/inteligr8/activiti/keycloak/AbstractKeycloakActivitiAuthenticator.java b/src/main/java/com/inteligr8/activiti/keycloak/AbstractKeycloakActivitiAuthenticator.java index 1667d3d..9610660 100644 --- a/src/main/java/com/inteligr8/activiti/keycloak/AbstractKeycloakActivitiAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/keycloak/AbstractKeycloakActivitiAuthenticator.java @@ -3,6 +3,7 @@ package com.inteligr8.activiti.keycloak; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -34,8 +35,8 @@ public abstract class AbstractKeycloakActivitiAuthenticator implements Authentic @Value("${keycloak-ext.createMissingUser:true}") protected boolean createMissingUser; - @Value("${keycloak-ext.clearNewUserGroups:true}") - protected boolean clearNewUserGroups; + @Value("${keycloak-ext.clearNewUserDefaultGroups:true}") + protected boolean clearNewUserDefaultGroups; @Value("${keycloak-ext.createMissingGroup:true}") protected boolean createMissingGroup; @@ -45,6 +46,9 @@ public abstract class AbstractKeycloakActivitiAuthenticator implements Authentic @Value("${keycloak-ext.syncGroupRemove:true}") protected boolean syncGroupRemove; + + @Value("${keycloak-ext.syncInternalGroups:false}") + protected boolean syncInternalGroups; @Value("${keycloak-ext.resource.include.regex.patterns:#{null}}") protected String resourceRegexIncludes; @@ -99,7 +103,7 @@ public abstract class AbstractKeycloakActivitiAuthenticator implements Authentic - protected Map getRoles(Authentication auth) { + protected Map getKeycloakRoles(Authentication auth) { Map authorities = new HashMap<>(); AccessToken atoken = this.getKeycloakAccessToken(auth); @@ -234,6 +238,24 @@ public abstract class AbstractKeycloakActivitiAuthenticator implements Authentic } } + protected boolean removeMapEntriesByValue(Map map, V value) { + if (value == null) + throw new IllegalArgumentException(); + + int found = 0; + + Iterator> i = map.entrySet().iterator(); + while (i.hasNext()) { + Entry entry = i.next(); + if (entry.getValue() != null && value.equals(entry.getValue())) { + i.remove(); + found++; + } + } + + return found > 0; + } + protected Set toSet(Collection grantedAuthorities) { Set authorities = new HashSet<>(Math.max(grantedAuthorities.size(), 16)); for (GrantedAuthority grantedAuthority : grantedAuthorities) { diff --git a/src/main/java/com/inteligr8/activiti/keycloak/KeycloakActivitiAppAuthenticator.java b/src/main/java/com/inteligr8/activiti/keycloak/KeycloakActivitiAppAuthenticator.java index 772a653..7b2207d 100644 --- a/src/main/java/com/inteligr8/activiti/keycloak/KeycloakActivitiAppAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/keycloak/KeycloakActivitiAppAuthenticator.java @@ -1,7 +1,6 @@ package com.inteligr8.activiti.keycloak; import java.util.Date; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -15,6 +14,7 @@ import org.keycloak.representations.AccessToken; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Lazy; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; @@ -61,6 +61,17 @@ public class KeycloakActivitiAppAuthenticator extends AbstractKeycloakActivitiAu @Autowired private GroupService groupService; + + @Value("${keycloak-ext.syncGroupAs:organization}") + protected String syncGroupAs; + + protected boolean syncGroupAsOrganization() { + return !this.syncGroupAsCapability(); + } + + protected boolean syncGroupAsCapability() { + return this.syncGroupAs != null && this.syncGroupAs.toLowerCase().startsWith("cap"); + } /** * This method validates that the user exists, if not, it creates the @@ -79,16 +90,26 @@ public class KeycloakActivitiAppAuthenticator extends AbstractKeycloakActivitiAu user = this.createUser(auth, tenantId); this.logger.debug("Created user: {} => {}", user.getId(), user.getExternalId()); - if (this.clearNewUserGroups) { + if (this.clearNewUserDefaultGroups) { this.logger.debug("Clearing groups: {}", user.getId()); // fetch and remove default groups - user = this.userService.findUserByEmailFetchGroups(user.getEmail()); + user = this.userService.getUser(user.getId(), true); for (Group group : user.getGroups()) this.groupService.deleteUserFromGroup(group, user); } } else { this.logger.info("User does not exist; user creation is disabled: {}", auth.getName()); } + } else if (user.getExternalOriginalSrc() == null || user.getExternalOriginalSrc().length() == 0) { + this.logger.debug("User exists, but not created by an external source: {}", auth.getName()); + this.logger.info("Linking user '{}' with external IDM '{}'", auth.getName(), this.externalIdmSource); + user.setExternalId(auth.getName()); + user.setExternalOriginalSrc(this.externalIdmSource); + this.userService.save(user); + } else if (!this.externalIdmSource.equals(user.getExternalOriginalSrc())) { + this.logger.debug("User '{}' exists, but created by another source: {}", auth.getName(), user.getExternalOriginalSrc()); + } else { + this.logger.trace("User already exists: {}", auth.getName()); } } @@ -141,7 +162,7 @@ public class KeycloakActivitiAppAuthenticator extends AbstractKeycloakActivitiAu Matcher emailNamesMatcher = this.emailNamesPattern.matcher(auth.getName()); if (!emailNamesMatcher.matches()) { this.logger.warn("The email address could not be parsed for names: {}", auth.getName()); - return this.userService.createNewUserFromExternalStore(auth.getName(), "Unknown", "User", tenantId, auth.getName(), this.externalIdmSource, new Date()); + return this.userService.createNewUserFromExternalStore(auth.getName(), "Unknown", "Person", tenantId, auth.getName(), this.externalIdmSource, new Date()); } else { String firstName = StringUtils.capitalize(emailNamesMatcher.group(1)); String lastName = StringUtils.capitalize(emailNamesMatcher.group(2)); @@ -153,22 +174,28 @@ public class KeycloakActivitiAppAuthenticator extends AbstractKeycloakActivitiAu } private void syncUserRoles(User user, Authentication auth, Long tenantId) { - Map roles = this.getRoles(auth); + Map roles = this.getKeycloakRoles(auth); if (roles == null) { this.logger.debug("The user roles could not be determined; skipping sync: {}", user.getEmail()); return; } + boolean syncAsOrg = this.syncGroupAsOrganization(); + // check Activiti groups - User userWithGroups = this.userService.findUserByEmailFetchGroups(user.getEmail()); + User userWithGroups = this.userService.getUser(user.getId(), true); for (Group group : userWithGroups.getGroups()) { + if (group.getExternalId() == null && !this.syncInternalGroups) + continue; + this.logger.trace("Inspecting group: {} => {} ({})", group.getId(), group.getName(), group.getExternalId()); - if (group.getExternalId() == null) { - // skip APS system groups - } else if (roles.remove(group.getExternalId()) != null) { - // all good + if (group.getExternalId() != null && this.removeMapEntriesByValue(roles, this.apsGroupExternalIdToKeycloakRole(group.getExternalId()))) { + // role already existed and the user is already a member + } else if (group.getExternalId() == null && roles.remove(this.apsGroupNameToKeycloakRole(group.getName())) != null) { + // internal role already existed and the user is already a member } else { + // at this point, we have a group that the user does not have a corresponding role for if (this.syncGroupRemove) { this.logger.trace("Removing user '{}' from group '{}'", user.getExternalId(), group.getName()); this.groupService.deleteUserFromGroup(group, userWithGroups); @@ -184,20 +211,29 @@ public class KeycloakActivitiAppAuthenticator extends AbstractKeycloakActivitiAu Group group; try { - group = this.groupService.getGroupByExternalId(role.getKey()); + group = this.groupService.getGroupByExternalIdAndTenantId(this.keycloakRoleToApsGroupExternalId(role.getKey()), tenantId); } catch (NonUniqueResultException nure) { - if (this.logger.isDebugEnabled()) { - // FIXME only added to address a former bug - group = this.fixMultipleGroups(role.getKey(), tenantId); - } else { - throw nure; + this.logger.warn("There are multiple groups with the external ID; not adding user to group: {}", role.getKey()); + continue; + } + + if (group == null) { + List groups = this.groupService.getGroupByNameAndTenantId(this.keycloakRoleToApsGroupName(role.getValue()), tenantId); + if (groups.size() > 1) { + this.logger.warn("There are multiple groups with the same name; not adding user to group: {}", role.getValue()); + continue; + } else if (groups.size() == 1) { + group = groups.iterator().next(); } } if (group == null) { if (this.createMissingGroup) { this.logger.trace("Creating new group: {}", role); - group = this.groupService.createGroupFromExternalStore(role.getValue(), tenantId, Group.TYPE_SYSTEM_GROUP, null, role.getKey(), new Date()); + String name = this.keycloakRoleToApsGroupName(role.getValue()); + String externalId = this.keycloakRoleToApsGroupExternalId(role.getKey()); + int type = syncAsOrg ? Group.TYPE_FUNCTIONAL_GROUP : Group.TYPE_SYSTEM_GROUP; + group = this.groupService.createGroupFromExternalStore(name, tenantId, type, null, externalId, new Date()); } else { this.logger.debug("Group does not exist; group creation is disabled: {}", role); } @@ -212,28 +248,21 @@ public class KeycloakActivitiAppAuthenticator extends AbstractKeycloakActivitiAu } } - private Group fixMultipleGroups(String externalId, Long tenantId) { - List groupsToDelete = new LinkedList<>(); - Date earliestDate = new Date(); - Group earliestGroup = null; - - for (Group group : this.groupService.getSystemGroups(tenantId)) { - if (externalId.equals(group.getExternalId())) { - if (group.getLastUpdate().before(earliestDate)) { - if (earliestGroup != null) - groupsToDelete.add(earliestGroup); - earliestDate = group.getLastUpdate(); - earliestGroup = group; - } else { - groupsToDelete.add(group); - } - } - } - - for (Group group : groupsToDelete) - this.groupService.deleteGroup(group.getId()); - - return earliestGroup; + private String keycloakRoleToApsGroupExternalId(String role) { + return this.externalIdmSource + "_" + role; + } + + private String apsGroupExternalIdToKeycloakRole(String externalId) { + int underscorePos = externalId.indexOf('_'); + return underscorePos < 0 ? externalId : externalId.substring(underscorePos + 1); + } + + private String keycloakRoleToApsGroupName(String role) { + return role; + } + + private String apsGroupNameToKeycloakRole(String externalId) { + return externalId; } } diff --git a/src/main/java/com/inteligr8/activiti/keycloak/KeycloakActivitiEngineAuthenticator.java b/src/main/java/com/inteligr8/activiti/keycloak/KeycloakActivitiEngineAuthenticator.java index b58cf2a..576ee35 100644 --- a/src/main/java/com/inteligr8/activiti/keycloak/KeycloakActivitiEngineAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/keycloak/KeycloakActivitiEngineAuthenticator.java @@ -48,7 +48,7 @@ public class KeycloakActivitiEngineAuthenticator extends AbstractKeycloakActivit user = this.createUser(auth); this.logger.debug("Created user: {} => {}", user.getId(), user.getEmail()); - if (this.clearNewUserGroups) { + if (this.clearNewUserDefaultGroups) { this.logger.debug("Clearing groups: {}", user.getId()); List groups = this.identityService.createGroupQuery() .groupMember(user.getId()) @@ -93,7 +93,7 @@ public class KeycloakActivitiEngineAuthenticator extends AbstractKeycloakActivit } private void syncUserRoles(User user, Authentication auth) { - Map roles = this.getRoles(auth); + Map roles = this.getKeycloakRoles(auth); if (roles == null) { this.logger.debug("The user roles could not be determined; skipping sync: {}", user.getEmail()); return; @@ -105,11 +105,11 @@ public class KeycloakActivitiEngineAuthenticator extends AbstractKeycloakActivit .list(); this.logger.debug("User is currently a member of {} groups", groups.size()); for (Group group : groups) { - if (!group.getId().startsWith(this.groupPrefix)) + if (!group.getId().startsWith(this.groupPrefix) && this.syncInternalGroups) continue; this.logger.trace("Inspecting group: {} => {} ({})", group.getId(), group.getName(), group.getType()); - if (roles.remove(group.getId().substring(this.groupPrefix.length())) != null) { + if (roles.remove(this.activitiGroupIdToKeycloakRole(group.getId())) != null) { this.logger.trace("Group and membership already exist: {} => {}", user.getEmail(), group.getName()); // already a member of the group } else { @@ -129,16 +129,16 @@ public class KeycloakActivitiEngineAuthenticator extends AbstractKeycloakActivit this.logger.trace("Inspecting role: {}", role); Group group = this.identityService.createGroupQuery() - .groupId(this.groupPrefix + role.getKey()) + .groupId(this.keycloakRoleToActivitiGroupId(role.getKey())) .singleResult(); if (group == null) { if (this.createMissingGroup) { this.logger.trace("Group does not exist; creating one"); - group = this.identityService.newGroup(this.groupPrefix + role.getKey()); + group = this.identityService.newGroup(this.keycloakRoleToActivitiGroupId(role.getKey())); group.setName(role.getValue()); this.identityService.saveGroup(group); } else { - this.logger.info("User does not exist; user creation is disabled: {}", auth.getName()); + this.logger.info("Group does not exist; group creation is disabled: {}", role.getKey()); } } @@ -151,4 +151,12 @@ public class KeycloakActivitiEngineAuthenticator extends AbstractKeycloakActivit } } + private String keycloakRoleToActivitiGroupId(String role) { + return this.groupPrefix + role; + } + + private String activitiGroupIdToKeycloakRole(String groupId) { + return groupId.startsWith(this.groupPrefix) ? groupId.substring(this.groupPrefix.length()) : groupId; + } + }