keycloak-aplcache

Kerberos fixes for jboss-modules refactoring. Fix environment

2/25/2015 11:40:00 AM

Details

diff --git a/distribution/modules/build.xml b/distribution/modules/build.xml
index 208e061..0cd950a 100755
--- a/distribution/modules/build.xml
+++ b/distribution/modules/build.xml
@@ -228,6 +228,10 @@
             <maven-resource group="org.keycloak" artifact="keycloak-social-facebook"/>
         </module-def>
 
+        <module-def name="org.keycloak.keycloak-kerberos-federation">
+            <maven-resource group="org.keycloak" artifact="keycloak-kerberos-federation"/>
+        </module-def>
+
         <module-def name="org.keycloak.keycloak-ldap-federation">
             <maven-resource group="org.keycloak" artifact="keycloak-ldap-federation"/>
         </module-def>
diff --git a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-kerberos-federation/main/module.xml b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-kerberos-federation/main/module.xml
new file mode 100644
index 0000000..a238b9c
--- /dev/null
+++ b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-kerberos-federation/main/module.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+
+
+<module xmlns="urn:jboss:module:1.1" name="org.keycloak.keycloak-kerberos-federation">
+    <resources>
+        <!-- Insert resources here -->
+    </resources>
+    <dependencies>
+        <module name="org.keycloak.keycloak-core"/>
+        <module name="org.keycloak.keycloak-model-api"/>
+        <module name="net.iharder.base64"/>
+        <module name="javax.ws.rs.api"/>
+        <module name="org.jboss.resteasy.resteasy-jaxrs"/>
+        <module name="org.jboss.logging"/>
+        <module name="javax.api"/>
+    </dependencies>
+
+</module>
\ No newline at end of file
diff --git a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-ldap-federation/main/module.xml b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-ldap-federation/main/module.xml
index 5c81548..29dfd9c 100755
--- a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-ldap-federation/main/module.xml
+++ b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-ldap-federation/main/module.xml
@@ -9,6 +9,7 @@
     <dependencies>
         <module name="org.keycloak.keycloak-core"/>
         <module name="org.keycloak.keycloak-model-api"/>
+        <module name="org.keycloak.keycloak-kerberos-federation"/>
         <module name="org.keycloak.keycloak-picketlink-api"/>
         <module name="javax.ws.rs.api"/>
         <module name="org.jboss.resteasy.resteasy-jaxrs"/>
diff --git a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-server/main/module.xml b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-server/main/module.xml
index a2efd4a..c0e942c 100755
--- a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-server/main/module.xml
+++ b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-server/main/module.xml
@@ -35,6 +35,7 @@
         <module name="org.keycloak.keycloak-invalidation-cache-model" services="import"/>
         <module name="org.keycloak.keycloak-jboss-adapter-core" services="import"/>
         <module name="org.keycloak.keycloak-js-adapter" services="import"/>
+        <module name="org.keycloak.keycloak-kerberos-federation" services="import"/>
         <module name="org.keycloak.keycloak-ldap-federation" services="import"/>
         <module name="org.keycloak.keycloak-login-api" services="import"/>
         <module name="org.keycloak.keycloak-login-freemarker" services="import"/>
diff --git a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-services/main/module.xml b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-services/main/module.xml
index e98c558..4cc0d13 100755
--- a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-services/main/module.xml
+++ b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-services/main/module.xml
@@ -35,6 +35,7 @@
         <module name="org.keycloak.keycloak-invalidation-cache-model" services="import"/>
         <module name="org.keycloak.keycloak-jboss-adapter-core" services="import"/>
         <module name="org.keycloak.keycloak-js-adapter" services="import"/>
+        <module name="org.keycloak.keycloak-kerberos-federation" services="import"/>
         <module name="org.keycloak.keycloak-ldap-federation" services="import"/>
         <module name="org.keycloak.keycloak-login-api" services="import"/>
         <module name="org.keycloak.keycloak-login-freemarker" services="import"/>
diff --git a/distribution/subsystem-war/src/main/webapp/WEB-INF/jboss-deployment-structure.xml b/distribution/subsystem-war/src/main/webapp/WEB-INF/jboss-deployment-structure.xml
index 356b953..9818d93 100755
--- a/distribution/subsystem-war/src/main/webapp/WEB-INF/jboss-deployment-structure.xml
+++ b/distribution/subsystem-war/src/main/webapp/WEB-INF/jboss-deployment-structure.xml
@@ -26,6 +26,7 @@
             <module name="org.keycloak.keycloak-invalidation-cache-model" services="import"/>
             <module name="org.keycloak.keycloak-jboss-adapter-core" services="import"/>
             <module name="org.keycloak.keycloak-js-adapter" services="import"/>
+            <module name="org.keycloak.keycloak-kerberos-federation" services="import"/>
             <module name="org.keycloak.keycloak-ldap-federation" services="import"/>
             <module name="org.keycloak.keycloak-login-api" services="import"/>
             <module name="org.keycloak.keycloak-login-freemarker" services="import"/>
diff --git a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/impl/KerberosUsernamePasswordAuthenticator.java b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/impl/KerberosUsernamePasswordAuthenticator.java
index c8e6fa0..ddebe66 100644
--- a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/impl/KerberosUsernamePasswordAuthenticator.java
+++ b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/impl/KerberosUsernamePasswordAuthenticator.java
@@ -36,14 +36,13 @@ public class KerberosUsernamePasswordAuthenticator {
     /**
      * Returns true if user with given username exists in kerberos database
      *
-     * @param username username without Kerberos realm attached
+     * @param username username without Kerberos realm attached or with correct realm attached
      * @return true if user available
      */
     public boolean isUserAvailable(String username) {
-        String principal = getKerberosPrincipal(username);
-
-        logger.debug("Checking existence of principal: " + principal);
+        logger.debug("Checking existence of user: " + username);
         try {
+            String principal = getKerberosPrincipal(username);
             loginContext = new LoginContext("does-not-matter", null,
                     createJaasCallbackHandler(principal, "fake-password-which-nobody-has"),
                     createJaasConfiguration());
@@ -65,7 +64,7 @@ public class KerberosUsernamePasswordAuthenticator {
     /**
      * Returns true if user was successfully authenticated against Kerberos
      *
-     * @param username username without Kerberos realm attached
+     * @param username username without Kerberos realm attached or with correct realm attached
      * @param password kerberos password
      * @return  true if user was successfully authenticated
      */
@@ -113,7 +112,17 @@ public class KerberosUsernamePasswordAuthenticator {
 
 
 
-    protected String getKerberosPrincipal(String username) {
+    protected String getKerberosPrincipal(String username) throws LoginException {
+        if (username.contains("@")) {
+            String[] tokens = username.split("@");
+            username = tokens[0];
+            String kerberosRealm = tokens[1];
+            if (kerberosRealm.toUpperCase().equals(config.getKerberosRealm())) {
+                logger.warn("Invalid kerberos realm. Expected realm: " + config.getKerberosRealm() + ", username: " + username);
+                throw new LoginException("Invalid kerberos realm");
+            }
+        }
+
         return username + "@" + config.getKerberosRealm();
     }
 
diff --git a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/impl/SPNEGOAuthenticator.java b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/impl/SPNEGOAuthenticator.java
index a320469..1132351 100644
--- a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/impl/SPNEGOAuthenticator.java
+++ b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/impl/SPNEGOAuthenticator.java
@@ -47,7 +47,7 @@ public class SPNEGOAuthenticator {
             Subject serverSubject = kerberosSubjectAuthenticator.authenticateServerSubject();
             authenticated = Subject.doAs(serverSubject, new AcceptSecContext());
         } catch (Exception e) {
-            log.warn("SPNEGO login failed: " + e.getMessage(), e);
+            log.warn("SPNEGO login failed", e);
         } finally {
             kerberosSubjectAuthenticator.logoutServerSubject();
         }
diff --git a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java
index f1ea251..603f3bd 100644
--- a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java
+++ b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java
@@ -69,12 +69,13 @@ public class KerberosFederationProvider implements UserFederationProvider {
 
     @Override
     public UserModel getUserByUsername(RealmModel realm, String username) {
-        if (username.contains("@")) {
-            username = username.split("@")[0];
-        }
-
         KerberosUsernamePasswordAuthenticator authenticator = factory.createKerberosUsernamePasswordAuthenticator(kerberosConfig);
         if (authenticator.isUserAvailable(username)) {
+            // Case when method was called with username including kerberos realm like john@REALM.ORG . Authenticator already checked that kerberos realm was correct
+            if (username.contains("@")) {
+                username = username.split("@")[0];
+            }
+
             return findOrCreateAuthenticatedUser(realm, username);
         } else {
             return null;
@@ -106,7 +107,7 @@ public class KerberosFederationProvider implements UserFederationProvider {
         // KerberosUsernamePasswordAuthenticator.isUserAvailable is an overhead, so avoid it for now
 
         String kerberosPrincipal = local.getUsername() + "@" + kerberosConfig.getKerberosRealm();
-        return model.getId().equals(local.getFederationLink()) && kerberosPrincipal.equals(local.getAttribute(KERBEROS_PRINCIPAL));
+        return kerberosPrincipal.equals(local.getAttribute(KERBEROS_PRINCIPAL));
     }
 
     @Override
@@ -181,8 +182,11 @@ public class KerberosFederationProvider implements UserFederationProvider {
 
                 String username = spnegoAuthenticator.getAuthenticatedUsername();
                 UserModel user = findOrCreateAuthenticatedUser(realm, username);
-
-                return new CredentialValidationOutput(user, CredentialValidationOutput.Status.AUTHENTICATED, state);
+                if (user == null) {
+                    return CredentialValidationOutput.failed();
+                } else {
+                    return new CredentialValidationOutput(user, CredentialValidationOutput.Status.AUTHENTICATED, state);
+                }
             }  else {
                 Map<String, Object> state = new HashMap<String, Object>();
                 state.put(KerberosConstants.RESPONSE_TOKEN, spnegoAuthenticator.getResponseToken());
@@ -202,19 +206,24 @@ public class KerberosFederationProvider implements UserFederationProvider {
     /**
      * Called after successful authentication
      *
-     * @param realm
+     * @param realm realm
      * @param username username without realm prefix
-     * @return
+     * @return user if found or successfully created. Null if user with same username already exists, but is not linked to this provider
      */
     protected UserModel findOrCreateAuthenticatedUser(RealmModel realm, String username) {
         UserModel user = session.userStorage().getUserByUsername(username, realm);
         if (user != null) {
             logger.debug("Kerberos authenticated user " + username + " found in Keycloak storage");
-            if (isValid(user)) {
+
+            if (!model.getId().equals(user.getFederationLink())) {
+                logger.warn("User with username " + username + " already exists, but is not linked to provider [" + model.getDisplayName() + "]");
+                return null;
+            } else if (isValid(user)) {
                 return proxy(user);
             } else {
-                logger.warn("User with username " + username + " already exists, but is not linked to provider [" + model.getDisplayName() +
-                        "] or kerberos principal is not correct. Kerberos principal on user is: " + user.getAttribute(KERBEROS_PRINCIPAL));
+                logger.warn("User with username " + username + " already exists and is linked to provider [" + model.getDisplayName() +
+                        "] but kerberos principal is not correct. Kerberos principal on user is: " + user.getAttribute(KERBEROS_PRINCIPAL));
+                logger.warn("Will re-create user");
                 session.userStorage().removeUser(realm, user);
             }
         }
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
index 9f927b4..7aabe26 100755
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
@@ -409,11 +409,15 @@ public class LDAPFederationProvider implements UserFederationProvider {
         UserModel user = session.userStorage().getUserByUsername(username, realm);
         if (user != null) {
             logger.debug("Kerberos authenticated user " + username + " found in Keycloak storage");
-            if (isValid(user)) {
+            if (!model.getId().equals(user.getFederationLink())) {
+                logger.warn("User with username " + username + " already exists, but is not linked to provider [" + model.getDisplayName() + "]");
+                return null;
+            } else if (isValid(user)) {
                 return proxy(user);
             } else {
-                logger.warn("User with username " + username + " already exists, but is not linked to provider [" + model.getDisplayName() +
-                        "] or LDAP_ID is not correct. Stale LDAP_ID on local user is: " + user.getAttribute(LDAP_ID));
+                logger.warn("User with username " + username + " already exists and is linked to provider [" + model.getDisplayName() +
+                        "] but is not valid. Stale LDAP_ID on local user is: " + user.getAttribute(LDAP_ID));
+                logger.warn("Will re-create user");
                 session.userStorage().removeUser(realm, user);
             }
         }
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/KerberosLdapTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/KerberosLdapTest.java
index 1f1092a..4123508 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/KerberosLdapTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/KerberosLdapTest.java
@@ -103,7 +103,7 @@ public class KerberosLdapTest extends AbstractKerberosTest {
         KeycloakRule keycloakRule = getKeycloakRule();
         AssertEvents events = getAssertEvents();
 
-        // Change editMode to READ_ONLY
+        // Change editMode to WRITABLE
         updateProviderEditMode(UserFederationProvider.EditMode.WRITABLE);
 
         // Login with username/password from kerberos
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/KeycloakSPNegoSchemeFactory.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/KeycloakSPNegoSchemeFactory.java
index 3403263..00c9954 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/KeycloakSPNegoSchemeFactory.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/KeycloakSPNegoSchemeFactory.java
@@ -8,7 +8,10 @@ import org.apache.http.auth.AuthScheme;
 import org.apache.http.impl.auth.SPNegoScheme;
 import org.apache.http.impl.auth.SPNegoSchemeFactory;
 import org.apache.http.params.HttpParams;
+import org.ietf.jgss.GSSContext;
 import org.ietf.jgss.GSSException;
+import org.ietf.jgss.GSSManager;
+import org.ietf.jgss.GSSName;
 import org.ietf.jgss.Oid;
 import org.keycloak.federation.kerberos.CommonKerberosConfig;
 import org.keycloak.federation.kerberos.impl.KerberosUsernamePasswordAuthenticator;
@@ -83,7 +86,17 @@ public class KeycloakSPNegoSchemeFactory extends SPNegoSchemeFactory {
 
             @Override
             public ByteArrayHolder run() throws Exception {
-                byte[] outputToken = KeycloakSPNegoScheme.super.generateGSSToken(input, oid, authServer);
+                byte[] token = input;
+                if (token == null) {
+                    token = new byte[0];
+                }
+                GSSManager manager = getManager();
+                GSSName serverName = manager.createName("HTTP/" + authServer + "@" + kerberosConfig.getKerberosRealm(), null);
+                GSSContext gssContext = manager.createContext(
+                        serverName.canonicalize(oid), oid, null, GSSContext.DEFAULT_LIFETIME);
+                gssContext.requestMutualAuth(true);
+                gssContext.requestCredDeleg(true);
+                byte[] outputToken = gssContext.initSecContext(token, 0, token.length);
 
                 ByteArrayHolder result = new ByteArrayHolder();
                 result.bytes = outputToken;