libre.sh/keycloak-scim
2
0
Fork 0
Code Issues 14 Pull requests Releases Packages Activity Actions 6

Add null checks after retrieving user from LDAP for validation to prevent NPE when user is removed in LDAP.

Browse source 
Closes #28523

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
This commit is contained in:
Stefan Guilhen 2024-04-10 14:15:12 -03:00 committed by Pedro Igor
parent d31f128ca2
commit e6b9d287af
2 changed files with 37 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
View file

@ -790,6 +790,10 @@ public class LDAPStorageProvider implements UserStorageProvider,
} else { } else {
// Use Naming LDAP API // Use Naming LDAP API
LDAPObject ldapUser = loadAndValidateUser(realm, user); LDAPObject ldapUser = loadAndValidateUser(realm, user);
if (ldapUser == null) {
// user was removed from ldap - password verification must fail.
return false;
}
try { try {
ldapIdentityStore.validatePassword(ldapUser, password); ldapIdentityStore.validatePassword(ldapUser, password);
@ -821,6 +825,10 @@ public class LDAPStorageProvider implements UserStorageProvider,
LDAPIdentityStore ldapIdentityStore = getLdapIdentityStore(); LDAPIdentityStore ldapIdentityStore = getLdapIdentityStore();
String password = input.getChallengeResponse(); String password = input.getChallengeResponse();
LDAPObject ldapUser = loadAndValidateUser(realm, user); LDAPObject ldapUser = loadAndValidateUser(realm, user);
if (ldapUser == null) {
logger.warnf("User '%s' can't be updated in LDAP as it doesn't exist there", user.getUsername());
return false;
}
if (ldapIdentityStore.getConfig().isValidatePasswordPolicy()) { if (ldapIdentityStore.getConfig().isValidatePasswordPolicy()) {
PolicyError error = session.getProvider(PasswordPolicyManagerProvider.class).validate(realm, user, password); PolicyError error = session.getProvider(PasswordPolicyManagerProvider.class).validate(realm, user, password);
if (error != null) throw new ModelException(error.getMessage(), error.getParameters()); if (error != null) throw new ModelException(error.getMessage(), error.getParameters());
@ -967,7 +975,8 @@ public class LDAPStorageProvider implements UserStorageProvider,
return null; return null;
} else { } else {
LDAPObject ldapObject = loadAndValidateUser(realm, user); LDAPObject ldapObject = loadAndValidateUser(realm, user);
if (kerberosPrincipalAttrName != null && !kerberosPrincipal.toString().equalsIgnoreCase(ldapObject.getAttributeAsString(kerberosPrincipalAttrName))) { if (kerberosPrincipalAttrName != null && ldapObject != null &&
!kerberosPrincipal.toString().equalsIgnoreCase(ldapObject.getAttributeAsString(kerberosPrincipalAttrName))) {
logger.warnf("User with username [%s] aready exists and is linked to provider [%s] but is not valid. Authenticated kerberos principal is [%s], but LDAP user has different kerberos principal [%s]", logger.warnf("User with username [%s] aready exists and is linked to provider [%s] but is not valid. Authenticated kerberos principal is [%s], but LDAP user has different kerberos principal [%s]",
user.getUsername(), model.getName(), kerberosPrincipal, ldapObject.getAttributeAsString(kerberosPrincipalAttrName)); user.getUsername(), model.getName(), kerberosPrincipal, ldapObject.getAttributeAsString(kerberosPrincipalAttrName));
ldapObject = null; ldapObject = null;

27
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPUserLoginTest.java
View file

@ -324,4 +324,31 @@ public class LDAPUserLoginTest extends AbstractLDAPTest {
verifyConnectionUrlProtocolPrefix("ldap://"); verifyConnectionUrlProtocolPrefix("ldap://");
runLDAPLoginTest(); runLDAPLoginTest();
} }
// Check that login fails as expected when an LDAP user that has already authenticated is removed from LDAP and attempts to authenticate again.
// See https://github.com/keycloak/keycloak/issues/28523
@Test
@LDAPConnectionParameters(bindType=LDAPConnectionParameters.BindType.SIMPLE, encryption=LDAPConnectionParameters.Encryption.NONE)
public void loginLDAPUserAuthenticationSimpleDeleteLDAPUser() {
// create another user for this test.
getTestingClient().server().run(session -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
RealmModel appRealm = ctx.getRealm();
LDAPObject jane = LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "janedoe", "Jane",
"Doe", "janedoe@keycloak.org", "2nd Avenue", "09283");
LDAPTestUtils.updateLDAPPassword(ctx.getLdapProvider(), jane, DEFAULT_TEST_USERS.get("VALID_USER_PASSWORD"));
});
// login with the new user, then logout - user is now cached in Keycloak.
this.verifyLoginSucceededAndLogout("janedoe", DEFAULT_TEST_USERS.get("VALID_USER_PASSWORD"));
// now remove the user directly in LDAP.
getTestingClient().server().run(session -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
RealmModel appRealm = ctx.getRealm();
LDAPTestUtils.removeLDAPUserByUsername(ctx.getLdapProvider(), appRealm, ctx.getLdapProvider().getLdapIdentityStore().getConfig(), "janedoe");
});
// attempt to login again with the deleted user should fail with the proper message.
this.verifyLoginFailed("janedoe", DEFAULT_TEST_USERS.get("VALID_USER_PASSWORD"));
}
} }