From bbef37a2223feca4153c4a63f9ce77b12dfb6cf6 Mon Sep 17 00:00:00 2001 From: "Brian M. Long" Date: Mon, 9 Aug 2021 15:08:46 -0400 Subject: [PATCH 1/2] refactored --- ...entScanner.java => KeycloakExtSpringComponentScanner.java} | 4 ++-- .../java/com/inteligr8/activiti/{ais => }/Authenticator.java | 2 +- .../ais/AbstractIdentityServiceActivitiAuthenticator.java | 1 + .../activiti/ais/IdentityServiceActivitiAppAuthenticator.java | 1 + .../ais/IdentityServiceActivitiEngineAuthenticator.java | 2 ++ .../ais/IdentityServiceAuthenticationProviderAdapter.java | 1 + .../ais/IdentityServiceSecurityConfigurationAdapter.java | 1 + .../InterceptingIdentityServiceAuthenticationProvider.java | 1 + 8 files changed, 10 insertions(+), 3 deletions(-) rename src/main/java/com/activiti/extension/conf/{OidcExtSpringComponentScanner.java => KeycloakExtSpringComponentScanner.java} (59%) rename src/main/java/com/inteligr8/activiti/{ais => }/Authenticator.java (91%) diff --git a/src/main/java/com/activiti/extension/conf/OidcExtSpringComponentScanner.java b/src/main/java/com/activiti/extension/conf/KeycloakExtSpringComponentScanner.java similarity index 59% rename from src/main/java/com/activiti/extension/conf/OidcExtSpringComponentScanner.java rename to src/main/java/com/activiti/extension/conf/KeycloakExtSpringComponentScanner.java index b53a694..2ee46b1 100644 --- a/src/main/java/com/activiti/extension/conf/OidcExtSpringComponentScanner.java +++ b/src/main/java/com/activiti/extension/conf/KeycloakExtSpringComponentScanner.java @@ -4,7 +4,7 @@ import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; @Configuration -@ComponentScan(basePackages = {"com.inteligr8.activiti.oidc"}) -public class OidcExtSpringComponentScanner { +@ComponentScan(basePackages = {"com.inteligr8.activiti.ais"}) +public class KeycloakExtSpringComponentScanner { } diff --git a/src/main/java/com/inteligr8/activiti/ais/Authenticator.java b/src/main/java/com/inteligr8/activiti/Authenticator.java similarity index 91% rename from src/main/java/com/inteligr8/activiti/ais/Authenticator.java rename to src/main/java/com/inteligr8/activiti/Authenticator.java index cfa145f..6ef403d 100644 --- a/src/main/java/com/inteligr8/activiti/ais/Authenticator.java +++ b/src/main/java/com/inteligr8/activiti/Authenticator.java @@ -1,4 +1,4 @@ -package com.inteligr8.activiti.ais; +package com.inteligr8.activiti; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; diff --git a/src/main/java/com/inteligr8/activiti/ais/AbstractIdentityServiceActivitiAuthenticator.java b/src/main/java/com/inteligr8/activiti/ais/AbstractIdentityServiceActivitiAuthenticator.java index dc8426e..f73d569 100644 --- a/src/main/java/com/inteligr8/activiti/ais/AbstractIdentityServiceActivitiAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/ais/AbstractIdentityServiceActivitiAuthenticator.java @@ -15,6 +15,7 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import com.activiti.security.identity.service.authentication.provider.IdentityServiceAuthenticationToken; +import com.inteligr8.activiti.Authenticator; public abstract class AbstractIdentityServiceActivitiAuthenticator implements Authenticator { diff --git a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiAppAuthenticator.java b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiAppAuthenticator.java index ccde6b3..a8e99ba 100644 --- a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiAppAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiAppAuthenticator.java @@ -24,6 +24,7 @@ import com.activiti.service.api.GroupService; import com.activiti.service.api.UserService; import com.activiti.service.idm.TenantService; import com.activiti.service.license.LicenseService; +import com.inteligr8.activiti.Authenticator; /** * This class/bean implements an Open ID Connect authenticator for Alfresco diff --git a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiEngineAuthenticator.java b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiEngineAuthenticator.java index f9c6f3f..ad02ad8 100644 --- a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiEngineAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiEngineAuthenticator.java @@ -15,6 +15,8 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.stereotype.Component; +import com.inteligr8.activiti.Authenticator; + /** * This is an unused implementation for non-APS installation. It is not tested * and probably pointless. diff --git a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceAuthenticationProviderAdapter.java b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceAuthenticationProviderAdapter.java index ad6c2b9..f74ef19 100644 --- a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceAuthenticationProviderAdapter.java +++ b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceAuthenticationProviderAdapter.java @@ -7,6 +7,7 @@ import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.security.authentication.AuthenticationProvider; import com.activiti.api.security.AlfrescoAuthenticationProviderOverride; +import com.inteligr8.activiti.Authenticator; /** * FIXME This would be nice, but with AIS enabled, it is never called. The use diff --git a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceSecurityConfigurationAdapter.java b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceSecurityConfigurationAdapter.java index eb89481..f729886 100644 --- a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceSecurityConfigurationAdapter.java +++ b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceSecurityConfigurationAdapter.java @@ -13,6 +13,7 @@ import org.springframework.stereotype.Component; import com.activiti.api.msmt.MsmtTenantResolver; import com.activiti.api.security.AlfrescoSecurityConfigOverride; import com.activiti.conf.MsmtProperties; +import com.inteligr8.activiti.Authenticator; /** * This class/bean overrides the AIS authentication provider, enabling a more diff --git a/src/main/java/com/inteligr8/activiti/ais/InterceptingIdentityServiceAuthenticationProvider.java b/src/main/java/com/inteligr8/activiti/ais/InterceptingIdentityServiceAuthenticationProvider.java index 0244023..bde2786 100644 --- a/src/main/java/com/inteligr8/activiti/ais/InterceptingIdentityServiceAuthenticationProvider.java +++ b/src/main/java/com/inteligr8/activiti/ais/InterceptingIdentityServiceAuthenticationProvider.java @@ -6,6 +6,7 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import com.activiti.security.identity.service.authentication.provider.IdentityServiceAuthenticationProvider; +import com.inteligr8.activiti.Authenticator; /** * This class/bean extends the APS AIS OOTB authentication provider. It uses From 916f371297d23af5847df02f9c5da028e8bdb786 Mon Sep 17 00:00:00 2001 From: Brian Long Date: Wed, 11 Aug 2021 09:08:01 -0400 Subject: [PATCH 2/2] refactored a bit with fixes --- ...tIdentityServiceActivitiAuthenticator.java | 82 ++++++++++++++----- ...entityServiceActivitiAppAuthenticator.java | 11 ++- ...ityServiceActivitiEngineAuthenticator.java | 7 +- 3 files changed, 75 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/inteligr8/activiti/ais/AbstractIdentityServiceActivitiAuthenticator.java b/src/main/java/com/inteligr8/activiti/ais/AbstractIdentityServiceActivitiAuthenticator.java index f73d569..ccb3d0f 100644 --- a/src/main/java/com/inteligr8/activiti/ais/AbstractIdentityServiceActivitiAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/ais/AbstractIdentityServiceActivitiAuthenticator.java @@ -2,19 +2,20 @@ package com.inteligr8.activiti.ais; import java.util.Collection; import java.util.HashSet; +import java.util.Map.Entry; import java.util.Set; +import org.apache.commons.lang3.StringUtils; import org.keycloak.KeycloakPrincipal; import org.keycloak.KeycloakSecurityContext; -import org.keycloak.adapters.OidcKeycloakAccount; import org.keycloak.adapters.springsecurity.token.KeycloakAuthenticationToken; import org.keycloak.representations.AccessToken; +import org.keycloak.representations.AccessToken.Access; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; -import com.activiti.security.identity.service.authentication.provider.IdentityServiceAuthenticationToken; import com.inteligr8.activiti.Authenticator; public abstract class AbstractIdentityServiceActivitiAuthenticator implements Authenticator { @@ -23,33 +24,76 @@ public abstract class AbstractIdentityServiceActivitiAuthenticator implements Au - protected AccessToken getOidcAccessToken(Authentication auth) { + protected Set getRoles(Authentication auth) { + Set authorities = this.toSet(auth.getAuthorities()); + this.logger.debug("Auto-parsed authorities: {}", authorities); + + if (authorities.isEmpty()) { + AccessToken atoken = this.getKeycloakAccessToken(auth); + if (atoken == null) { + this.logger.debug("Access token not available"); + return null; + } else if (atoken.getRealmAccess() == null && atoken.getResourceAccess().isEmpty()) { + this.logger.debug("Access token has no role information"); + return null; + } else { + if (atoken.getRealmAccess() != null) { + this.logger.debug("Access token realm roles: {}", atoken.getRealmAccess().getRoles()); + authorities.addAll(atoken.getRealmAccess().getRoles()); + } + + for (Entry resourceAccess : atoken.getResourceAccess().entrySet()) { + this.logger.debug("Access token resources '{}' roles: {}", resourceAccess.getKey(), resourceAccess.getValue().getRoles()); + authorities.addAll(resourceAccess.getValue().getRoles()); + } + + this.logger.debug("Access token authorities: {}", authorities); + } + } + + return authorities; + } + + protected AccessToken getKeycloakAccessToken(Authentication auth) { KeycloakSecurityContext ksc = this.getKeycloakSecurityContext(auth); - return ksc.getToken(); + return ksc == null ? null : ksc.getToken(); } @SuppressWarnings("unchecked") protected KeycloakSecurityContext getKeycloakSecurityContext(Authentication auth) { - if (auth instanceof KeycloakAuthenticationToken) { - this.logger.debug("Fetching KeycloakSecurityContext from KeycloakAuthenticationToken"); - if (auth.getPrincipal() instanceof KeycloakPrincipal) { - return ((KeycloakPrincipal)auth.getPrincipal()).getKeycloakSecurityContext(); - } else { - return null; - } - } else if (auth instanceof IdentityServiceAuthenticationToken) { - this.logger.debug("Fetching KeycloakSecurityContext from IdentityServiceAuthenticationToken"); - OidcKeycloakAccount account = ((IdentityServiceAuthenticationToken)auth).getAccount(); - return account == null ? null : account.getKeycloakSecurityContext(); - } else { + if (auth.getCredentials() instanceof KeycloakSecurityContext) { + this.logger.debug("Found keycloak context in credentials"); + return (KeycloakSecurityContext)auth.getCredentials(); + } else if (auth.getPrincipal() instanceof KeycloakPrincipal) { + this.logger.debug("Found keycloak context in principal: {}", auth.getPrincipal()); + return ((KeycloakPrincipal)auth.getPrincipal()).getKeycloakSecurityContext(); + } else if (!(auth instanceof KeycloakAuthenticationToken)) { + this.logger.warn("Unexpected token: {}", auth.getClass()); return null; } + + KeycloakAuthenticationToken ktoken = (KeycloakAuthenticationToken)auth; + if (ktoken.getAccount() != null) { + this.logger.debug("Found keycloak context in account: {}", ktoken.getAccount().getPrincipal() == null ? null : ktoken.getAccount().getPrincipal().getName()); + return ktoken.getAccount().getKeycloakSecurityContext(); + } else { + this.logger.warn("Unable to find keycloak security context"); + this.logger.debug("Principal: {}", auth.getPrincipal()); + this.logger.debug("Account: {}", ktoken.getAccount()); + if (auth.getPrincipal() != null) + this.logger.debug("Principal type: {}", auth.getPrincipal().getClass()); + return null; + } } protected Set toSet(Collection grantedAuthorities) { - Set authorities = new HashSet<>(grantedAuthorities.size()); - for (GrantedAuthority grantedAuthority : grantedAuthorities) - authorities.add(grantedAuthority.getAuthority()); + Set authorities = new HashSet<>(Math.max(grantedAuthorities.size(), 16)); + for (GrantedAuthority grantedAuthority : grantedAuthorities) { + String authority = StringUtils.trimToNull(grantedAuthority.getAuthority()); + if (authority == null) + this.logger.warn("The granted authorities include an empty authority!?: '{}'", grantedAuthority.getAuthority()); + authorities.add(authority); + } return authorities; } } diff --git a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiAppAuthenticator.java b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiAppAuthenticator.java index a8e99ba..3c250ad 100644 --- a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiAppAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiAppAuthenticator.java @@ -148,9 +148,9 @@ public class IdentityServiceActivitiAppAuthenticator extends AbstractIdentitySer } private User createUser(Authentication auth, Long tenantId) { - AccessToken atoken = this.getOidcAccessToken(auth); + AccessToken atoken = this.getKeycloakAccessToken(auth); if (atoken == null) { - this.logger.debug("The OIDC access token could not be found; using email to determine names: {}", auth.getName()); + this.logger.debug("The keycloak access token could not be found; using email to determine names: {}", auth.getName()); Matcher emailNamesMatcher = this.emailNamesPattern.matcher(auth.getName()); if (!emailNamesMatcher.matches()) { this.logger.warn("The email address could not be parsed for names: {}", auth.getName()); @@ -166,8 +166,11 @@ public class IdentityServiceActivitiAppAuthenticator extends AbstractIdentitySer } private void syncUserAuthorities(User user, Authentication auth, Long tenantId) { - Set authorities = this.toSet(auth.getAuthorities()); - this.logger.debug("OIDC authorities: {}", authorities); + Set authorities = this.getRoles(auth); + if (authorities == null) { + this.logger.debug("The user authorities could not be determined; skipping sync: {}", user.getEmail()); + return; + } // check Activiti groups User userWithGroups = this.userService.findUserByEmailFetchGroups(user.getEmail()); diff --git a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiEngineAuthenticator.java b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiEngineAuthenticator.java index ad02ad8..fcfffdd 100644 --- a/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiEngineAuthenticator.java +++ b/src/main/java/com/inteligr8/activiti/ais/IdentityServiceActivitiEngineAuthenticator.java @@ -106,8 +106,11 @@ public class IdentityServiceActivitiEngineAuthenticator extends AbstractIdentity } private void syncUserAuthorities(User user, Authentication auth) { - Set authorities = this.toSet(auth.getAuthorities()); - this.logger.debug("OIDC authorities: {}", authorities); + Set authorities = this.getRoles(auth); + if (authorities == null) { + this.logger.debug("The user authorities could not be determined; skipping sync: {}", user.getEmail()); + return; + } // check Activiti groups List groups = this.identityService.createGroupQuery()