From f754b34c0c76718270f6b2265fc459289c35deb6 Mon Sep 17 00:00:00 2001 From: Phillip Schichtel Date: Wed, 13 Jan 2021 22:09:33 +0100 Subject: [PATCH] KEYCLOAK-13633 Generalize GenericPrincipalFactory to PrincipleFactory This allows to replace java.security.acl.Group usage only where necessary while keeping legacy adapter unchanged. Signed-off-by: Phillip Schichtel --- .../adapters/jbossweb/JBossWebPrincipalFactory.java | 9 ++------- .../adapters/jbossweb/KeycloakAuthenticatorValve.java | 4 ++-- .../tomcat/AbstractKeycloakAuthenticatorValve.java | 2 +- .../adapters/tomcat/CatalinaCookieTokenStore.java | 4 ++-- .../adapters/tomcat/CatalinaRequestAuthenticator.java | 4 ++-- .../adapters/tomcat/CatalinaSessionTokenStore.java | 4 ++-- .../saml/jbossweb/SamlAuthenticatorValve.java | 4 ++-- .../adapters/saml/AbstractSamlAuthenticatorValve.java | 4 ++-- .../adapters/saml/CatalinaSamlSessionStore.java | 6 +++--- .../adapters/saml/tomcat/TomcatSamlSessionStore.java | 4 ++-- .../keycloak/adapters/jboss/KeycloakLoginModule.java | 5 ++--- .../adapters/tomcat/GenericPrincipalFactory.java | 5 +++-- .../keycloak/adapters/tomcat/PrincipalFactory.java | 11 +++++++++++ 13 files changed, 36 insertions(+), 30 deletions(-) create mode 100644 adapters/spi/tomcat-adapter-spi/src/main/java/org/keycloak/adapters/tomcat/PrincipalFactory.java diff --git a/adapters/oidc/as7-eap6/as7-adapter-spi/src/main/java/org/keycloak/adapters/jbossweb/JBossWebPrincipalFactory.java b/adapters/oidc/as7-eap6/as7-adapter-spi/src/main/java/org/keycloak/adapters/jbossweb/JBossWebPrincipalFactory.java index 5ae712cb26..b7916f20de 100755 --- a/adapters/oidc/as7-eap6/as7-adapter-spi/src/main/java/org/keycloak/adapters/jbossweb/JBossWebPrincipalFactory.java +++ b/adapters/oidc/as7-eap6/as7-adapter-spi/src/main/java/org/keycloak/adapters/jbossweb/JBossWebPrincipalFactory.java @@ -27,7 +27,7 @@ import org.jboss.security.SecurityContextAssociation; import org.jboss.security.SimpleGroup; import org.jboss.security.SimplePrincipal; import org.keycloak.adapters.spi.KeycloakAccount; -import org.keycloak.adapters.tomcat.GenericPrincipalFactory; +import org.keycloak.adapters.tomcat.PrincipalFactory; import javax.security.auth.Subject; import java.lang.reflect.Constructor; @@ -44,15 +44,10 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class JBossWebPrincipalFactory extends GenericPrincipalFactory { +public class JBossWebPrincipalFactory implements PrincipalFactory { private static Constructor jbossWebPrincipalConstructor = findJBossGenericPrincipalConstructor(); - @Override - protected GenericPrincipal createPrincipal(Principal userPrincipal, List roles) { - return null; - } - @Override public GenericPrincipal createPrincipal(Realm realm, final Principal identity, final Set roleSet) { KeycloakAccount account = new KeycloakAccount() { diff --git a/adapters/oidc/as7-eap6/as7-adapter/src/main/java/org/keycloak/adapters/jbossweb/KeycloakAuthenticatorValve.java b/adapters/oidc/as7-eap6/as7-adapter/src/main/java/org/keycloak/adapters/jbossweb/KeycloakAuthenticatorValve.java index 6a79f0b45c..f21b75a9f4 100755 --- a/adapters/oidc/as7-eap6/as7-adapter/src/main/java/org/keycloak/adapters/jbossweb/KeycloakAuthenticatorValve.java +++ b/adapters/oidc/as7-eap6/as7-adapter/src/main/java/org/keycloak/adapters/jbossweb/KeycloakAuthenticatorValve.java @@ -27,7 +27,7 @@ import org.apache.catalina.deploy.LoginConfig; import org.keycloak.adapters.AdapterDeploymentContext; import org.keycloak.adapters.tomcat.AbstractAuthenticatedActionsValve; import org.keycloak.adapters.tomcat.AbstractKeycloakAuthenticatorValve; -import org.keycloak.adapters.tomcat.GenericPrincipalFactory; +import org.keycloak.adapters.tomcat.PrincipalFactory; import javax.servlet.http.HttpServletResponse; import java.io.IOException; @@ -65,7 +65,7 @@ public class KeycloakAuthenticatorValve extends AbstractKeycloakAuthenticatorVal } @Override - protected GenericPrincipalFactory createPrincipalFactory() { + protected PrincipalFactory createPrincipalFactory() { return new JBossWebPrincipalFactory(); } diff --git a/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/AbstractKeycloakAuthenticatorValve.java b/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/AbstractKeycloakAuthenticatorValve.java index 6a46e33d93..9758853982 100755 --- a/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/AbstractKeycloakAuthenticatorValve.java +++ b/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/AbstractKeycloakAuthenticatorValve.java @@ -183,7 +183,7 @@ public abstract class AbstractKeycloakAuthenticatorValve extends FormAuthenticat } } - protected abstract GenericPrincipalFactory createPrincipalFactory(); + protected abstract PrincipalFactory createPrincipalFactory(); protected abstract boolean forwardToErrorPageInternal(Request request, HttpServletResponse response, Object loginConfig) throws IOException; protected abstract AbstractAuthenticatedActionsValve createAuthenticatedActionsValve(AdapterDeploymentContext deploymentContext, Valve next, Container container); diff --git a/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaCookieTokenStore.java b/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaCookieTokenStore.java index 768a4ddfad..864711d90f 100755 --- a/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaCookieTokenStore.java +++ b/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaCookieTokenStore.java @@ -43,11 +43,11 @@ public class CatalinaCookieTokenStore implements AdapterTokenStore { private Request request; private HttpFacade facade; private KeycloakDeployment deployment; - private GenericPrincipalFactory principalFactory; + private PrincipalFactory principalFactory; private KeycloakPrincipal authenticatedPrincipal; - public CatalinaCookieTokenStore(Request request, HttpFacade facade, KeycloakDeployment deployment, GenericPrincipalFactory principalFactory) { + public CatalinaCookieTokenStore(Request request, HttpFacade facade, KeycloakDeployment deployment, PrincipalFactory principalFactory) { this.request = request; this.facade = facade; this.deployment = deployment; diff --git a/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaRequestAuthenticator.java b/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaRequestAuthenticator.java index cbb69bd8c3..84c39a7655 100755 --- a/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaRequestAuthenticator.java +++ b/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaRequestAuthenticator.java @@ -41,13 +41,13 @@ import java.util.logging.Logger; public class CatalinaRequestAuthenticator extends RequestAuthenticator { private static final Logger log = Logger.getLogger(""+CatalinaRequestAuthenticator.class); protected Request request; - protected GenericPrincipalFactory principalFactory; + protected PrincipalFactory principalFactory; public CatalinaRequestAuthenticator(KeycloakDeployment deployment, AdapterTokenStore tokenStore, CatalinaHttpFacade facade, Request request, - GenericPrincipalFactory principalFactory) { + PrincipalFactory principalFactory) { super(facade, deployment, tokenStore, request.getConnector().getRedirectPort()); this.request = request; this.principalFactory = principalFactory; diff --git a/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaSessionTokenStore.java b/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaSessionTokenStore.java index ded41e3de9..5f2ba3edf3 100755 --- a/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaSessionTokenStore.java +++ b/adapters/oidc/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/tomcat/CatalinaSessionTokenStore.java @@ -45,12 +45,12 @@ public class CatalinaSessionTokenStore extends CatalinaAdapterSessionStore imple private KeycloakDeployment deployment; private CatalinaUserSessionManagement sessionManagement; - protected GenericPrincipalFactory principalFactory; + protected PrincipalFactory principalFactory; public CatalinaSessionTokenStore(Request request, KeycloakDeployment deployment, CatalinaUserSessionManagement sessionManagement, - GenericPrincipalFactory principalFactory, + PrincipalFactory principalFactory, AbstractKeycloakAuthenticatorValve valve) { super(request, valve); this.deployment = deployment; diff --git a/adapters/saml/as7-eap6/adapter/src/main/java/org/keycloak/adapters/saml/jbossweb/SamlAuthenticatorValve.java b/adapters/saml/as7-eap6/adapter/src/main/java/org/keycloak/adapters/saml/jbossweb/SamlAuthenticatorValve.java index 99d539387e..e5399d90e8 100755 --- a/adapters/saml/as7-eap6/adapter/src/main/java/org/keycloak/adapters/saml/jbossweb/SamlAuthenticatorValve.java +++ b/adapters/saml/as7-eap6/adapter/src/main/java/org/keycloak/adapters/saml/jbossweb/SamlAuthenticatorValve.java @@ -26,7 +26,7 @@ import org.apache.catalina.deploy.LoginConfig; import org.keycloak.adapters.jbossweb.JBossWebPrincipalFactory; import org.keycloak.adapters.saml.*; import org.keycloak.adapters.spi.SessionIdMapperUpdater; -import org.keycloak.adapters.tomcat.GenericPrincipalFactory; +import org.keycloak.adapters.tomcat.PrincipalFactory; import javax.servlet.http.HttpServletResponse; import java.io.IOException; @@ -69,7 +69,7 @@ public class SamlAuthenticatorValve extends AbstractSamlAuthenticatorValve { } @Override - protected GenericPrincipalFactory createPrincipalFactory() { + protected PrincipalFactory createPrincipalFactory() { return new JBossWebPrincipalFactory(); } diff --git a/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/AbstractSamlAuthenticatorValve.java b/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/AbstractSamlAuthenticatorValve.java index f8968c8070..a7b8f41fd0 100755 --- a/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/AbstractSamlAuthenticatorValve.java +++ b/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/AbstractSamlAuthenticatorValve.java @@ -31,7 +31,7 @@ import org.keycloak.adapters.saml.config.parsers.ResourceLoader; import org.keycloak.adapters.spi.*; import org.keycloak.adapters.tomcat.CatalinaHttpFacade; import org.keycloak.adapters.tomcat.CatalinaUserSessionManagement; -import org.keycloak.adapters.tomcat.GenericPrincipalFactory; +import org.keycloak.adapters.tomcat.PrincipalFactory; import org.keycloak.saml.common.exceptions.ParsingException; import javax.servlet.RequestDispatcher; @@ -186,7 +186,7 @@ public abstract class AbstractSamlAuthenticatorValve extends FormAuthenticator i } - protected abstract GenericPrincipalFactory createPrincipalFactory(); + protected abstract PrincipalFactory createPrincipalFactory(); protected abstract boolean forwardToErrorPageInternal(Request request, HttpServletResponse response, Object loginConfig) throws IOException; private static final Pattern PROTOCOL_PATTERN = Pattern.compile("^[a-zA-Z][a-zA-Z0-9+.-]*:"); diff --git a/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java b/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java index 98d236965a..ff122ac5a6 100755 --- a/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java +++ b/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java @@ -26,7 +26,7 @@ import org.keycloak.adapters.spi.HttpFacade; import org.keycloak.adapters.spi.SessionIdMapper; import org.keycloak.adapters.spi.SessionIdMapperUpdater; import org.keycloak.adapters.tomcat.CatalinaUserSessionManagement; -import org.keycloak.adapters.tomcat.GenericPrincipalFactory; +import org.keycloak.adapters.tomcat.PrincipalFactory; import org.keycloak.common.util.KeycloakUriBuilder; import javax.servlet.http.HttpSession; @@ -44,7 +44,7 @@ public class CatalinaSamlSessionStore implements SamlSessionStore { public static final String SAML_REDIRECT_URI = "SAML_REDIRECT_URI"; private final CatalinaUserSessionManagement sessionManagement; - protected final GenericPrincipalFactory principalFactory; + protected final PrincipalFactory principalFactory; private final SessionIdMapper idMapper; private final SessionIdMapperUpdater idMapperUpdater; protected final Request request; @@ -52,7 +52,7 @@ public class CatalinaSamlSessionStore implements SamlSessionStore { protected final HttpFacade facade; protected final SamlDeployment deployment; - public CatalinaSamlSessionStore(CatalinaUserSessionManagement sessionManagement, GenericPrincipalFactory principalFactory, + public CatalinaSamlSessionStore(CatalinaUserSessionManagement sessionManagement, PrincipalFactory principalFactory, SessionIdMapper idMapper, SessionIdMapperUpdater idMapperUpdater, Request request, AbstractSamlAuthenticatorValve valve, HttpFacade facade, SamlDeployment deployment) { diff --git a/adapters/saml/tomcat/tomcat/src/main/java/org/keycloak/adapters/saml/tomcat/TomcatSamlSessionStore.java b/adapters/saml/tomcat/tomcat/src/main/java/org/keycloak/adapters/saml/tomcat/TomcatSamlSessionStore.java index f3d852ece1..c1e04dd048 100755 --- a/adapters/saml/tomcat/tomcat/src/main/java/org/keycloak/adapters/saml/tomcat/TomcatSamlSessionStore.java +++ b/adapters/saml/tomcat/tomcat/src/main/java/org/keycloak/adapters/saml/tomcat/TomcatSamlSessionStore.java @@ -26,14 +26,14 @@ import org.keycloak.adapters.spi.HttpFacade; import org.keycloak.adapters.spi.SessionIdMapper; import org.keycloak.adapters.spi.SessionIdMapperUpdater; import org.keycloak.adapters.tomcat.CatalinaUserSessionManagement; -import org.keycloak.adapters.tomcat.GenericPrincipalFactory; +import org.keycloak.adapters.tomcat.PrincipalFactory; /** * @author Bill Burke * @version $Revision: 1 $ */ public class TomcatSamlSessionStore extends CatalinaSamlSessionStore { - public TomcatSamlSessionStore(CatalinaUserSessionManagement sessionManagement, GenericPrincipalFactory principalFactory, SessionIdMapper idMapper, Request request, AbstractSamlAuthenticatorValve valve, HttpFacade facade, SamlDeployment deployment) { + public TomcatSamlSessionStore(CatalinaUserSessionManagement sessionManagement, PrincipalFactory principalFactory, SessionIdMapper idMapper, Request request, AbstractSamlAuthenticatorValve valve, HttpFacade facade, SamlDeployment deployment) { super(sessionManagement, principalFactory, idMapper, SessionIdMapperUpdater.DIRECT, request, valve, facade, deployment); } diff --git a/adapters/spi/jboss-adapter-core/src/main/java/org/keycloak/adapters/jboss/KeycloakLoginModule.java b/adapters/spi/jboss-adapter-core/src/main/java/org/keycloak/adapters/jboss/KeycloakLoginModule.java index bd24c1ffc8..ac2ba35ed0 100755 --- a/adapters/spi/jboss-adapter-core/src/main/java/org/keycloak/adapters/jboss/KeycloakLoginModule.java +++ b/adapters/spi/jboss-adapter-core/src/main/java/org/keycloak/adapters/jboss/KeycloakLoginModule.java @@ -30,7 +30,6 @@ import javax.security.auth.callback.UnsupportedCallbackException; import javax.security.auth.login.LoginException; import java.io.IOException; import java.security.Principal; -import java.security.acl.Group; import java.util.Set; /** @@ -83,10 +82,10 @@ public class KeycloakLoginModule extends AbstractServerLoginModule { */ @Override - protected Group[] getRoleSets() throws LoginException { + protected SimpleGroup[] getRoleSets() throws LoginException { //log.info("getRoleSets"); SimpleGroup roles = new SimpleGroup("Roles"); - Group[] roleSets = {roles}; + SimpleGroup[] roleSets = {roles}; for (String role : roleSet) { //log.info(" adding role: " + role); roles.addMember(new SimplePrincipal(role)); diff --git a/adapters/spi/tomcat-adapter-spi/src/main/java/org/keycloak/adapters/tomcat/GenericPrincipalFactory.java b/adapters/spi/tomcat-adapter-spi/src/main/java/org/keycloak/adapters/tomcat/GenericPrincipalFactory.java index 3405d2546e..4042f4367f 100755 --- a/adapters/spi/tomcat-adapter-spi/src/main/java/org/keycloak/adapters/tomcat/GenericPrincipalFactory.java +++ b/adapters/spi/tomcat-adapter-spi/src/main/java/org/keycloak/adapters/tomcat/GenericPrincipalFactory.java @@ -24,16 +24,17 @@ import javax.security.auth.Subject; import java.security.Principal; import java.util.ArrayList; import java.util.Enumeration; -import java.util.List; import java.util.Collection; +import java.util.List; import java.util.Set; /** * @author Davide Ungari * @version $Revision: 1 $ */ -public abstract class GenericPrincipalFactory { +public abstract class GenericPrincipalFactory implements PrincipalFactory { + @Override public GenericPrincipal createPrincipal(Realm realm, final Principal identity, final Set roleSet) { Subject subject = new Subject(); Set principals = subject.getPrincipals(); diff --git a/adapters/spi/tomcat-adapter-spi/src/main/java/org/keycloak/adapters/tomcat/PrincipalFactory.java b/adapters/spi/tomcat-adapter-spi/src/main/java/org/keycloak/adapters/tomcat/PrincipalFactory.java new file mode 100644 index 0000000000..7fc301ab6f --- /dev/null +++ b/adapters/spi/tomcat-adapter-spi/src/main/java/org/keycloak/adapters/tomcat/PrincipalFactory.java @@ -0,0 +1,11 @@ +package org.keycloak.adapters.tomcat; + +import org.apache.catalina.Realm; +import org.apache.catalina.realm.GenericPrincipal; + +import java.security.Principal; +import java.util.Set; + +public interface PrincipalFactory { + GenericPrincipal createPrincipal(Realm realm, final Principal identity, final Set roleSet); +}