Skip to content

Use cache for password hash while validating LDAP password #15993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kfaraz merged 3 commits into from
Feb 28, 2024
Merged

Use cache for password hash while validating LDAP password #15993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa37ecd
Use cache for password hash while validating LDAP password
kfaraz Feb 28, 2024
51ec32c
Fix tests
kfaraz Feb 28, 2024
6f53397
Fix test coverage
kfaraz Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...urity/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ public Instant getLastVerified()
return lastVerified.get();
}

public boolean hasSameCredentials(char[] password)
public boolean hasSameCredentials(char[] password, PasswordHashGenerator hashGenerator)
{
byte[] recalculatedHash = PasswordHashGenerator.computePasswordHash(
byte[] recalculatedHash = hashGenerator.getOrComputePasswordHash(
password,
this.credentials.getSalt(),
this.credentials.getIterations()
Expand Down
13 changes: 6 additions & 7 deletions ...va/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,13 @@ public AuthenticationResult validateCredentials(
char[] password
)
{
SearchResult userResult;
LdapName userDn;
Map<String, Object> contextMap = new HashMap<>();
final SearchResult userResult;
final LdapName userDn;
final Map<String, Object> contextMap = new HashMap<>();
final LdapUserPrincipal principal = this.cache.getOrExpire(username);

LdapUserPrincipal principal = this.cache.getOrExpire(username);
if (principal != null && principal.hasSameCredentials(password)) {
if (principal != null && principal.hasSameCredentials(password, hashGenerator)) {
contextMap.put(BasicAuthUtils.SEARCH_RESULT_CONTEXT_KEY, principal.getSearchResult());
return new AuthenticationResult(username, authorizerName, authenticatorName, contextMap);
} else {
ClassLoader currentClassLoader = Thread.currentThread().getContextClassLoader();
try {
Expand Down Expand Up @@ -208,8 +207,8 @@ public AuthenticationResult validateCredentials(

this.cache.put(username, newPrincipal);
contextMap.put(BasicAuthUtils.SEARCH_RESULT_CONTEXT_KEY, userResult);
return new AuthenticationResult(username, authorizerName, authenticatorName, contextMap);
}
return new AuthenticationResult(username, authorizerName, authenticatorName, contextMap);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion ...java/org/apache/druid/security/authentication/validator/LDAPCredentialsValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import javax.naming.directory.SearchResult;
import javax.naming.ldap.LdapContext;
import javax.naming.spi.InitialContextFactory;

import java.util.Collections;
import java.util.Hashtable;
import java.util.Iterator;
Expand Down Expand Up @@ -94,6 +93,7 @@ public void testValidateCredentials()
properties
);
validator.validateCredentials("ldap", "ldap", "validUser", "password".toCharArray());
validator.validateCredentials("ldap", "ldap", "validUser", "password".toCharArray());
}

public static class MockContextFactory implements InitialContextFactory
Expand Down
42 changes: 19 additions & 23 deletions ...validator/DBCredentialsValidatorTest.java → ...etadataStoreCredentialsValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

package org.apache.druid.security.authentication.validator;
package org.apache.druid.security.basic.authentication.validator;

import com.google.common.collect.ImmutableMap;
import com.google.inject.Provider;
Expand All @@ -28,28 +28,21 @@
import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate;
import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentials;
import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorUser;
import org.apache.druid.security.basic.authentication.validator.MetadataStoreCredentialsValidator;
import org.apache.druid.server.security.Access;
import org.apache.druid.server.security.AuthenticationResult;
import org.easymock.EasyMock;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.util.Map;

public class DBCredentialsValidatorTest
public class MetadataStoreCredentialsValidatorTest
{

@Rule
public ExpectedException expectedException = ExpectedException.none();

private static BasicAuthenticatorCredentials USER_A_CREDENTIALS = new BasicAuthenticatorCredentials(
private static final BasicAuthenticatorCredentials USER_A_CREDENTIALS = new BasicAuthenticatorCredentials(
new BasicAuthenticatorCredentialUpdate("helloworld", 20)
);

private static Provider<BasicAuthenticatorCacheManager> CACHE_MANAGER_PROVIDER = Providers.of(
private static final Provider<BasicAuthenticatorCacheManager> CACHE_MANAGER_PROVIDER = Providers.of(
new BasicAuthenticatorCacheManager()
{
@Override
Expand All @@ -69,9 +62,8 @@ public Map<String, BasicAuthenticatorUser> getUserMap(String authenticatorPrefix
}
);

private static MetadataStoreCredentialsValidator validator = new MetadataStoreCredentialsValidator(CACHE_MANAGER_PROVIDER);


private static final MetadataStoreCredentialsValidator VALIDATOR
= new MetadataStoreCredentialsValidator(CACHE_MANAGER_PROVIDER);

@Test
public void validateBadAuthenticator()
Expand All @@ -87,9 +79,11 @@ public void validateBadAuthenticator()

MetadataStoreCredentialsValidator validator = new MetadataStoreCredentialsValidator(Providers.of(cacheManager));

expectedException.expect(IAE.class);
expectedException.expectMessage("No userMap is available for authenticator with prefix: [notbasic]");
validator.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
IAE exception = Assert.assertThrows(
IAE.class,
() -> validator.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray())
);
Assert.assertEquals("No userMap is available for authenticator with prefix: [notbasic]", exception.getMessage());

EasyMock.verify(cacheManager);
}
Expand All @@ -102,7 +96,7 @@ public void validateMissingCredentials()
String username = "userB";
String password = "helloworld";

AuthenticationResult result = validator.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
AuthenticationResult result = VALIDATOR.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
Assert.assertNull(result);
}

Expand All @@ -114,7 +108,7 @@ public void validateMissingUser()
String username = "userC";
String password = "helloworld";

AuthenticationResult result = validator.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
AuthenticationResult result = VALIDATOR.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
Assert.assertNull(result);
}

Expand All @@ -126,7 +120,7 @@ public void validateGoodCredentials()
String username = "userA";
String password = "helloworld";

AuthenticationResult result = validator.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
AuthenticationResult result = VALIDATOR.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());

Assert.assertNotNull(result);
Assert.assertEquals(username, result.getIdentity());
Expand All @@ -143,8 +137,10 @@ public void validateBadCredentials()
String username = "userA";
String password = "badpassword";

expectedException.expect(BasicSecurityAuthenticationException.class);
expectedException.expectMessage(Access.DEFAULT_ERROR_MESSAGE);
validator.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
Exception exception = Assert.assertThrows(
BasicSecurityAuthenticationException.class,
() -> VALIDATOR.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray())
);
Assert.assertEquals(Access.DEFAULT_ERROR_MESSAGE, exception.getMessage());
}
}