From 9f8b59dfb6bb28d6f5128337427635dd5ae4a23d Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 23 Feb 2015 13:12:41 +0100 Subject: [PATCH] Kerberos/LDAP fixes --- .../kerberos/KerberosFederationProvider.java | 14 ++++++----- .../kerberos/impl/SPNEGOAuthenticator.java | 9 +------- .../ldap/LDAPFederationProvider.java | 23 ++++++++++++------- .../base/resources/js/controllers/users.js | 6 +---- .../resources/partials/federated-ldap.html | 10 ++------ .../models/UserFederationManager.java | 1 + .../managers/HttpAuthenticationManager.java | 1 - .../main/resources/kerberos/test-krb5.conf | 1 + 8 files changed, 29 insertions(+), 36 deletions(-) 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 b0d23dcffa..f1ea251547 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/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 59983637ab..a320469967 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/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java index 3e2237df53..9f927b4aef 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 state = new HashMap(); @@ -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 fb0ce1bef1..4603153e1c 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 66ff5c2d07..35323a64d1 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 @@
-
- -
+
- +
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 7d3ecf470b..a98618559f 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 34ed9008d1..9791de321d 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 2ba050d9b5..350c086af7 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