Fix expiration logic for ldap internal credential cache (#11395)
* Fix expiration logic for ldap internal credential cache
* Removed sleeps from tests
* Make method package scoped so it can be used in unit tests
* Removed unused thrown exceptions
diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java
index cac753d..d185b02 100644
--- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java
+++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java
@@ -19,6 +19,7 @@
package org.apache.druid.security.basic.authentication;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.logger.Logger;
import org.apache.druid.security.basic.BasicAuthUtils;
@@ -50,7 +51,8 @@
this(name, credentials, searchResult, Instant.now());
}
- private LdapUserPrincipal(
+ @VisibleForTesting
+ public LdapUserPrincipal(
String name,
BasicAuthenticatorCredentials credentials,
SearchResult searchResult,
@@ -106,21 +108,26 @@
}
}
- public boolean isExpired(int duration, int maxDuration)
+ public boolean isExpired(int durationSeconds, int maxDurationSeconds)
{
- long now = System.currentTimeMillis();
+ return isExpired(durationSeconds, maxDurationSeconds, System.currentTimeMillis());
+ }
- long maxCutoff = now - (maxDuration * 1000L);
- if (this.createdAt.toEpochMilli() < maxCutoff) {
- long cutoff = now - (duration * 1000L);
- if (this.lastVerified.get().toEpochMilli() < cutoff) {
- return true;
- } else {
- return false;
- }
+ @VisibleForTesting
+ boolean isExpired(int durationSeconds, int maxDurationSeconds, long nowMillis)
+ {
+ long maxCutoffMillis = nowMillis - (maxDurationSeconds * 1000L);
+ if (this.createdAt.toEpochMilli() < maxCutoffMillis) {
+ // max cutoff is up...so expired
+ return true;
} else {
- return false;
+ long cutoffMillis = nowMillis - (durationSeconds * 1000L);
+ if (this.lastVerified.get().toEpochMilli() < cutoffMillis) {
+ // max cutoff not reached yet but cutoff since verified is up, so expired
+ return true;
+ }
}
+ return false;
}
@Override
diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/LdapUserPrincipalTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/LdapUserPrincipalTest.java
new file mode 100644
index 0000000..aaff67b
--- /dev/null
+++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/LdapUserPrincipalTest.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.security.basic.authentication;
+
+import junit.framework.TestCase;
+import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate;
+import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentials;
+import org.junit.Assert;
+
+import javax.naming.directory.SearchResult;
+import java.time.Instant;
+
+public class LdapUserPrincipalTest extends TestCase
+{
+
+ private static final BasicAuthenticatorCredentials USER_CREDENTIALS = new BasicAuthenticatorCredentials(
+ new BasicAuthenticatorCredentialUpdate("helloworld", 20)
+ );
+
+ private static final long CREATED_MILLIS = 10000;
+
+ // this will create a cache with createdAt now():
+ private static final LdapUserPrincipal PRINCIPAL = new LdapUserPrincipal(
+ "foo",
+ USER_CREDENTIALS,
+ new SearchResult("foo", null, null),
+ Instant.ofEpochMilli(CREATED_MILLIS)
+ );
+
+ public void testIsNotExpired()
+ {
+ Assert.assertFalse(PRINCIPAL.isExpired(1, 10, CREATED_MILLIS + 500));
+ }
+
+ public void testIsObviouslyExpired()
+ {
+ // real clock now should be so much bigger than CREATED_MILLIS....so it must have expired...
+ Assert.assertTrue(PRINCIPAL.isExpired(100, 1000));
+ }
+
+ public void testIsExpiredWhenMaxDurationIsSmall()
+ {
+ Assert.assertTrue(PRINCIPAL.isExpired(10, 1, CREATED_MILLIS + 1001));
+ }
+
+ public void testIsExpiredWhenDurationIsSmall()
+ {
+ Assert.assertTrue(PRINCIPAL.isExpired(1, 10, CREATED_MILLIS + 1001));
+ }
+
+ public void testIsExpiredWhenDurationsAreSmall()
+ {
+ Assert.assertTrue(PRINCIPAL.isExpired(1, 1, CREATED_MILLIS + 1001));
+ }
+}
