allowing org/cap group types; other fixes

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<String> 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<ActivitiSecurityConfigAdapter> 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);
+			List<Group> groups = this.groupService.getGroupByNameAndTenantId(this.adminGroupName, tenantId);
-			group = this.groupService.createGroupFromExternalStore(
+			if (!groups.isEmpty())
-					this.adminGroupExternalId, tenantId, Group.TYPE_SYSTEM_GROUP, null, this.adminGroupName, new Date());
+				group = groups.iterator().next();
 		}
 
-		this.logger.info("Granting '{}' group all capabilities ...", group.getName());
+		if (group == null) {
-		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"));
+			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<String> 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));
+		}
 	}
 	
 	private void associateAdmins() {

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}")
+    @Value("${keycloak-ext.clearNewUserDefaultGroups:true}")
-    protected boolean clearNewUserGroups;
+    protected boolean clearNewUserDefaultGroups;
 
     @Value("${keycloak-ext.createMissingGroup:true}")
     protected boolean createMissingGroup;
@@ -46,6 +47,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<String, String> getRoles(Authentication auth) {
+    protected Map<String, String> getKeycloakRoles(Authentication auth) {
     	Map<String, String> authorities = new HashMap<>();
 		
 		AccessToken atoken = this.getKeycloakAccessToken(auth);
@@ -234,6 +238,24 @@ public abstract class AbstractKeycloakActivitiAuthenticator implements Authentic
 		}
     }
     
+    protected <K, V> boolean removeMapEntriesByValue(Map<K, V> map, V value) {
+    	if (value == null)
+    		throw new IllegalArgumentException();
+    	
+    	int found = 0;
+    	
+    	Iterator<Entry<K, V>> i = map.entrySet().iterator();
+    	while (i.hasNext()) {
+    		Entry<K, V> entry = i.next();
+    		if (entry.getValue() != null && value.equals(entry.getValue())) {
+    			i.remove();
+    			found++;
+    		}
+    	}
+    	
+    	return found > 0;
+    }
+    
     protected Set<String> toSet(Collection<? extends GrantedAuthority> grantedAuthorities) {
     	Set<String> authorities = new HashSet<>(Math.max(grantedAuthorities.size(), 16));
     	for (GrantedAuthority grantedAuthority : grantedAuthorities) {

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;
@@ -62,6 +62,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
      * missing user.  Without this functionality, SSO straight up fails in APS.
@@ -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<String, String> roles = this.getRoles(auth);
+    	Map<String, String> 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) {
+			if (group.getExternalId() != null && this.removeMapEntriesByValue(roles, this.apsGroupExternalIdToKeycloakRole(group.getExternalId()))) {
-				// skip APS system groups
+				// role already existed and the user is already a member
-			} else if (roles.remove(group.getExternalId()) != null) {
+			} else if (group.getExternalId() == null && roles.remove(this.apsGroupNameToKeycloakRole(group.getName())) != null) {
-				// all good
+				// 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()) {
+				this.logger.warn("There are multiple groups with the external ID; not adding user to group: {}", role.getKey());
-					// FIXME only added to address a former bug
+				continue;
-					group = this.fixMultipleGroups(role.getKey(), tenantId);
+			}
-				} else {
+
-					throw nure;
+			if (group == null) {
+				List<Group> 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) {
+    private String keycloakRoleToApsGroupExternalId(String role) {
-    	List<Group> groupsToDelete = new LinkedList<>();
+    	return this.externalIdmSource + "_" + role;
-    	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)
+    private String apsGroupExternalIdToKeycloakRole(String externalId) {
-    		this.groupService.deleteGroup(group.getId());
+    	int underscorePos = externalId.indexOf('_');
+    	return underscorePos < 0 ? externalId : externalId.substring(underscorePos + 1);
+    }
     
-    	return earliestGroup;
+    private String keycloakRoleToApsGroupName(String role) {
+    	return role;
+    }
+    
+    private String apsGroupNameToKeycloakRole(String externalId) {
+    	return externalId;
     }
     
 }

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<Group> groups = this.identityService.createGroupQuery()
 	        				.groupMember(user.getId())
@@ -93,7 +93,7 @@ public class KeycloakActivitiEngineAuthenticator extends AbstractKeycloakActivit
     }
 
     private void syncUserRoles(User user, Authentication auth) {
-    	Map<String, String> roles = this.getRoles(auth);
+    	Map<String, String> 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;
+    }
+    
 }