keycloak-developers

Kerberos/LDAP fixes

2/23/2015 9:12:41 AM

Details

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 5998363..a320469 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,14 +47,7 @@ public class SPNEGOAuthenticator {
             Subject serverSubject = kerberosSubjectAuthenticator.authenticateServerSubject();
             authenticated = Subject.doAs(serverSubject, new AcceptSecContext());
         } catch (Exception e) {
-            String message = e.getMessage();
-            if (e instanceof PrivilegedActionException && e.getCause() != null) {
-                message = e.getCause().getMessage();
-            }
-            log.warn("SPNEGO login failed: " + message);
-            if (log.isDebugEnabled()) {
-                log.debug("SPNEGO login failed: " + message, e);
-            }
+            log.warn("SPNEGO login failed: " + e.getMessage(), 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 b0d23dc..f1ea251 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
@@ -210,15 +210,17 @@ public class KerberosFederationProvider 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)) {
-                throw new IllegalStateException("User with username " + username + " already exists, but is not linked to provider [" + model.getDisplayName() +
+            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));
+                session.userStorage().removeUser(realm, user);
             }
-
-            return proxy(user);
-        } else {
-            return importUserToKeycloak(realm, username);
         }
+
+        logger.debug("Kerberos authenticated user " + username + " not in Keycloak storage. Creating him");
+        return importUserToKeycloak(realm, username);
     }
 
     protected UserModel importUserToKeycloak(RealmModel realm, String username) {
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 3e2237d..9f927b4 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
@@ -353,6 +353,11 @@ public class LDAPFederationProvider implements UserFederationProvider {
                     String username = spnegoAuthenticator.getAuthenticatedUsername();
                     UserModel user = findOrCreateAuthenticatedUser(realm, username);
 
+                    if (user == null) {
+                        logger.warn("Kerberos/SPNEGO authentication succeeded with username [" + username + "], but couldn't find or create user with federation provider [" + model.getDisplayName() + "]");
+                        return CredentialValidationOutput.failed();
+                    }
+
                     return new CredentialValidationOutput(user, CredentialValidationOutput.Status.AUTHENTICATED, state);
                 }  else {
                     Map<String, Object> state = new HashMap<String, Object>();
@@ -404,15 +409,17 @@ 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)) {
-                throw new IllegalStateException("User with username " + username + " already exists, but is not linked to provider [" + model.getDisplayName() +
-                        "] or LDAP_ID is not correct. LDAP_ID on user is: " + user.getAttribute(LDAP_ID));
+            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));
+                session.userStorage().removeUser(realm, user);
             }
-
-            return proxy(user);
-        } else {
-            // Creating user to local storage
-            return getUserByUsername(realm, username);
         }
+
+        // Creating user to local storage
+        logger.debug("Kerberos authenticated user " + username + " not in Keycloak storage. Creating him");
+        return getUserByUsername(realm, username);
     }
 }
diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/users.js b/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/users.js
index fb0ce1b..4603153 100755
--- a/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/users.js
+++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/users.js
@@ -547,10 +547,6 @@ module.controller('LDAPCtrl', function($scope, $location, Notifications, Dialog,
         { "id": "other", "name": "Other" }
     ];
 
-    $scope.usernameLDAPAttributes = [
-        "uid", "cn", "sAMAccountName", "entryDN"
-    ];
-
     $scope.realm = realm;
 
     $scope.$watch('fullSyncEnabled', function(newVal, oldVal) {
@@ -581,7 +577,7 @@ module.controller('LDAPCtrl', function($scope, $location, Notifications, Dialog,
             $scope.lastVendor = $scope.instance.config.vendor;
 
             if ($scope.lastVendor === "ad") {
-                $scope.instance.config.usernameLDAPAttribute = "cn";
+                $scope.instance.config.usernameLDAPAttribute = "sAMAccountName";
                 $scope.instance.config.userObjectClasses = "person, organizationalPerson, user";
             } else {
                 $scope.instance.config.usernameLDAPAttribute = "uid";
diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/federated-ldap.html b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/federated-ldap.html
index 66ff5c2..35323a6 100755
--- a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/federated-ldap.html
+++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/federated-ldap.html
@@ -78,15 +78,9 @@
                 <div class="form-group clearfix">
                     <label class="col-sm-2 control-label" for="usernameLDAPAttribute">Username LDAP attribute<span class="required">*</span></label>
                     <div class="col-sm-4">
-                        <div class="select-kc">
-                            <select id="usernameLDAPAttribute"
-                                    ng-model="instance.config.usernameLDAPAttribute"
-                                    ng-options="usernameLDAPAttribute for usernameLDAPAttribute in usernameLDAPAttributes"
-                                    required>
-                            </select>
-                        </div>
+                        <input class="form-control" id="usernameLDAPAttribute" type="text" ng-model="instance.config.usernameLDAPAttribute" placeholder="LDAP attribute for uid" required>
                     </div>
-                    <span tooltip-placement="right" tooltip="Name of LDAP attribute, which is mapped as Keycloak username" class="fa fa-info-circle"></span>
+                    <span tooltip-placement="right" tooltip="Name of LDAP attribute, which is mapped as Keycloak username. For many LDAP server vendors it's 'uid'. For Active directory it's usually 'sAMAccountName' or 'cn'" class="fa fa-info-circle"></span>
                 </div>
                 <div class="form-group clearfix">
                     <label class="col-sm-2 control-label" for="userObjectClasses">User Object Classes<span class="required">*</span></label>
diff --git a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java
index 7d3ecf4..a986185 100755
--- a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java
+++ b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java
@@ -384,6 +384,7 @@ public class UserFederationManager implements UserProvider {
                 return CredentialValidationOutput.failed();
             }
 
+            logger.debug("Found provider [" + providerSupportingCreds + "] supporting credentials of type " + cred.getType());
             CredentialValidationOutput currentResult = providerSupportingCreds.validCredentials(realm, cred);
             result = (result == null) ? currentResult : result.merge(currentResult);
         }
diff --git a/services/src/main/java/org/keycloak/services/managers/HttpAuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/HttpAuthenticationManager.java
index 34ed900..9791de3 100644
--- a/services/src/main/java/org/keycloak/services/managers/HttpAuthenticationManager.java
+++ b/services/src/main/java/org/keycloak/services/managers/HttpAuthenticationManager.java
@@ -140,7 +140,6 @@ public class HttpAuthenticationManager {
 
                 loginFormsProvider.setStatus(Response.Status.UNAUTHORIZED);
                 loginFormsProvider.setResponseHeader(HttpHeaders.WWW_AUTHENTICATE, negotiateHeader);
-                loginFormsProvider.setWarning("errorKerberosLogin");
             }
 
         });
diff --git a/testsuite/integration/src/main/resources/kerberos/test-krb5.conf b/testsuite/integration/src/main/resources/kerberos/test-krb5.conf
index 2ba050d..350c086 100644
--- a/testsuite/integration/src/main/resources/kerberos/test-krb5.conf
+++ b/testsuite/integration/src/main/resources/kerberos/test-krb5.conf
@@ -2,6 +2,7 @@
     default_realm = KEYCLOAK.ORG
     default_tgs_enctypes = des3-cbc-sha1-kd rc4-hmac
     default_tkt_enctypes = des3-cbc-sha1-kd rc4-hmac
+    permitted_enctypes = des3-cbc-sha1-kd rc4-hmac
     kdc_timeout = 30000
     dns_lookup_realm = false
     dns_lookup_kdc = false