[java-idp-plugin-duo] branch main updated: JDUO-85 - Admin API needs to implement rate limiting
Scott Cantor
cantor.2 at osu.edu
Mon Apr 1 20:13:13 UTC 2024
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository java-idp-plugin-duo.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-duo.git;a=commit;h=97f3311ed51f3674d56fb3c3b3f43d8b39673763
The following commit(s) were added to refs/heads/main by this push:
new 97f3311e JDUO-85 - Admin API needs to implement rate limiting
97f3311e is described below
commit 97f3311ed51f3674d56fb3c3b3f43d8b39673763
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Apr 1 16:13:10 2024 -0400
JDUO-85 - Admin API needs to implement rate limiting
https://shibboleth.atlassian.net/browse/JDUO-85
Adjust getUser API to return nulls in place of exceptions.
Wire in lockout manager to meter enrollment condition.
---
idp-duo-api/pom.xml | 7 ++
...=> DefaultPasswordlessEnrollmentCondition.java} | 91 +++++++++++++++++++++-
.../idp/plugin/authn/duo/DuoAdminClient.java | 4 +-
.../authn/duo/impl/DefaultDuoAdminClient.java | 15 ++--
.../META-INF/net.shibboleth.idp/postconfig.xml | 18 ++++-
.../authn/duo/impl/DefaultDuoAdminClientTest.java | 2 +-
6 files changed, 119 insertions(+), 18 deletions(-)
diff --git a/idp-duo-api/pom.xml b/idp-duo-api/pom.xml
index 47f272c0..a26038c2 100644
--- a/idp-duo-api/pom.xml
+++ b/idp-duo-api/pom.xml
@@ -70,6 +70,13 @@
<artifactId>jackson-databind</artifactId>
<scope>provided</scope>
</dependency>
+
+ <dependency>
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
+ <scope>provided</scope>
+ </dependency>
+
<!-- Test dependencies -->
</dependencies>
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/PasswordlessEnrollmentCondition.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultPasswordlessEnrollmentCondition.java
similarity index 62%
rename from idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/PasswordlessEnrollmentCondition.java
rename to idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultPasswordlessEnrollmentCondition.java
index 95a5b673..84138249 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/PasswordlessEnrollmentCondition.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DefaultPasswordlessEnrollmentCondition.java
@@ -18,6 +18,7 @@ import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.function.BiPredicate;
+import java.util.function.Function;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
@@ -26,6 +27,8 @@ import javax.annotation.Nullable;
import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
+import jakarta.servlet.http.HttpServletRequest;
+import net.shibboleth.idp.authn.AccountLockoutManager;
import net.shibboleth.idp.plugin.authn.duo.model.User;
import net.shibboleth.idp.plugin.authn.duo.model.WebAuthnCredential;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
@@ -33,8 +36,11 @@ import net.shibboleth.shared.annotation.constraint.NonnullElements;
import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.AbstractInitializableComponent;
import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.primitive.NonnullSupplier;
import net.shibboleth.shared.primitive.StringSupport;
+import net.shibboleth.shared.servlet.HttpServletSupport;
/**
* A BiPredicate which checks the enrollment status of a user against the Duo Admin APIs.
@@ -43,11 +49,11 @@ import net.shibboleth.shared.primitive.StringSupport;
*
* @since 2.1.0
*/
-public class PasswordlessEnrollmentCondition extends AbstractInitializableComponent
+public class DefaultPasswordlessEnrollmentCondition extends AbstractInitializableComponent
implements BiPredicate<ProfileRequestContext,String> {
/** Class logger. */
- @Nonnull private final Logger log = LoggerFactory.getLogger(PasswordlessEnrollmentCondition.class);
+ @Nonnull private final Logger log = LoggerFactory.getLogger(DefaultPasswordlessEnrollmentCondition.class);
/** The admin client used to access the DuoAdmin API.*/
@NonnullAfterInit private DuoAdminClient adminClient;
@@ -55,8 +61,11 @@ public class PasswordlessEnrollmentCondition extends AbstractInitializableCompon
/** Allowed WebAuthnCredential labels that qualify. */
@Nonnull @NonnullElements private Set<String> allowedLabels;
+ /** Optional lockout tracker for rate limiting. */
+ @Nullable private AccountLockoutManager lockoutManager;
+
/** Constructor. */
- public PasswordlessEnrollmentCondition() {
+ public DefaultPasswordlessEnrollmentCondition() {
allowedLabels = CollectionSupport.emptySet();
}
@@ -80,6 +89,16 @@ public class PasswordlessEnrollmentCondition extends AbstractInitializableCompon
allowedLabels = CollectionSupport.copyToSet(StringSupport.normalizeStringCollection(labels));
}
+ /**
+ * Sets the optional {@link AccountLockoutManager} to use for rate limiting.
+ *
+ * @param manager lockout manager
+ */
+ public void setLockoutManager(@Nullable final AccountLockoutManager manager) {
+ checkSetterPreconditions();
+ lockoutManager = manager;
+ }
+
/** {@inheritDoc} */
@Override
public void doInitialize() throws ComponentInitializationException {
@@ -90,6 +109,7 @@ public class PasswordlessEnrollmentCondition extends AbstractInitializableCompon
}
}
+// Checkstyle: CyclomaticComplexity|ReturnCount OFF
/** {@inheritDoc} */
public boolean test(@Nullable final ProfileRequestContext profileRequestContext, @Nullable final String username) {
checkComponentActive();
@@ -101,9 +121,24 @@ public class PasswordlessEnrollmentCondition extends AbstractInitializableCompon
log.trace("Checking passwordless enrollment status for '{}'", username);
try {
+
+ if (lockoutManager != null) {
+ if (lockoutManager.check(profileRequestContext)) {
+ log.warn("Lockout manager precludes enrollment check for '{}'", username);
+ return false;
+ }
+
+ assert lockoutManager != null;
+ lockoutManager.increment(profileRequestContext);
+ }
+
final User response = adminClient.getUser(profileRequestContext, username);
- final Boolean enrolled = response.isEnrolled();
+ if (response == null) {
+ log.info("User '{}' not found in Duo", username);
+ return false;
+ }
+ final Boolean enrolled = response.isEnrolled();
if (enrolled == null || !enrolled) {
log.info("User '{}' not enrolled", response.getUsername());
return false;
@@ -140,5 +175,53 @@ public class PasswordlessEnrollmentCondition extends AbstractInitializableCompon
return false;
}
}
+// Checkstyle: CyclomaticComplexity|ReturnCount ON
+
+ /**
+ * A function to generate a key for lockout storage, just the client address.
+ */
+ public static class IPLockoutKeyStrategy implements Function<ProfileRequestContext,String> {
+
+ /** Supplier for the Servlet request to pull client ip from. **/
+ @Nullable private NonnullSupplier<HttpServletRequest> httpRequestSupplier;
+
+ /**
+ * Set the Supplier for the servlet request to read from.
+ *
+ * @param requestSupplier servlet request Supplier
+ */
+ public void setHttpServletRequestSupplier(@Nonnull final NonnullSupplier<HttpServletRequest> requestSupplier) {
+ httpRequestSupplier = Constraint.isNotNull(requestSupplier, "HttpServletRequest cannot be null");
+ }
+
+ /**
+ * Get the current HTTP request if available.
+ *
+ * @return current HTTP request
+ */
+ @Nullable private HttpServletRequest getHttpServletRequest() {
+ if (httpRequestSupplier == null) {
+ return null;
+ }
+ assert httpRequestSupplier != null;
+ return httpRequestSupplier.get();
+ }
+
+ /** {@inheritDoc} */
+ @Nullable public String apply(@Nullable final ProfileRequestContext profileRequestContext) {
+
+ final HttpServletRequest request = getHttpServletRequest();
+ if (request == null) {
+ return null;
+ }
+
+ final String ipAddr = HttpServletSupport.getRemoteAddr(request);
+ if (ipAddr == null || ipAddr.isEmpty()) {
+ return null;
+ }
+
+ return ipAddr;
+ }
+ }
}
\ No newline at end of file
diff --git a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoAdminClient.java b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoAdminClient.java
index b1985f70..c8f009cf 100644
--- a/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoAdminClient.java
+++ b/idp-duo-api/src/main/java/net/shibboleth/idp/plugin/authn/duo/DuoAdminClient.java
@@ -43,11 +43,11 @@ public interface DuoAdminClient {
* @param context context the profile request context, typically used in locating the {@link DuoIntegration} to use
* @param username a user name (or username alias) to look up a single user
*
- * @return the User iff found
+ * @return the User iff found, or null
*
* @throws DuoException on error retrieving a response from the API
*/
- @Nonnull User getUser(@Nonnull final ProfileRequestContext context, @Nonnull @NotEmpty final String username)
+ @Nullable User getUser(@Nonnull final ProfileRequestContext context, @Nonnull @NotEmpty final String username)
throws DuoException;
/**
diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoAdminClient.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoAdminClient.java
index c0762f51..90a72deb 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoAdminClient.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoAdminClient.java
@@ -222,7 +222,7 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
}
/** {@inheritDoc} */
- @Nonnull public User getUser(@Nonnull final ProfileRequestContext context, @Nonnull final String username)
+ @Nullable public User getUser(@Nonnull final ProfileRequestContext context, @Nonnull final String username)
throws DuoException {
try {
// prepare the request
@@ -232,15 +232,12 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
// execute the request
final List<User> response = doAPIRequest(request,
new TypeReference<DuoAdminResponseWrapper<List<User>>>() {}, initialBackoff).getResponse();
- if (response.size() != 1) {
- throw new DuoException("User API response contained either no user record, or too many");
+ if (response.isEmpty()) {
+ return null;
+ } else if (response.size() > 1) {
+ throw new DuoException("User API response contained more than one record");
}
- final User user = response.get(0);
- if (user == null) {
- throw new DuoException("User API response did not contain a user");
- }
- return user;
-
+ return response.get(0);
} catch (final Exception ex) {
throw new DuoException("Unable to to get User '"+username+"' from Duo's Admin API", ex);
}
diff --git a/idp-duo-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml b/idp-duo-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
index 99a45fb1..a207f33f 100644
--- a/idp-duo-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
+++ b/idp-duo-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
@@ -73,12 +73,26 @@
<!-- Default passwordless condition that uses the admin API -->
<bean id="shibboleth.authn.DuoOIDC.Passwordless.DefaultCondition" lazy-init="true"
- class="net.shibboleth.idp.plugin.authn.duo.PasswordlessEnrollmentCondition"
- p:duoAdminClient="#{getObject('shibboleth.authn.DuoOIDC.AdminClient') ?: getObject('shibboleth.authn.DuoOIDC.DefaultAdminClient')}">
+ class="net.shibboleth.idp.plugin.authn.duo.DefaultPasswordlessEnrollmentCondition"
+ p:duoAdminClient="#{getObject('shibboleth.authn.DuoOIDC.AdminClient') ?: getObject('shibboleth.authn.DuoOIDC.DefaultAdminClient')}"
+ p:lockoutManager="#{%{idp.duo.oidc.passwordless.limitEnrollmentChecking:true} ? getObject('shibboleth.authn.DuoOIDC.Passwordless.LockoutManager') : null}">
<property name="allowedLabels">
<bean parent="shibboleth.CommaDelimStringArray"
c:_0="#{'%{idp.duo.oidc.passwordless.allowedLabels:}'.trim()}" />
</property>
</bean>
+ <bean id="shibboleth.authn.DuoOIDC.Passwordless.LockoutManager" lazy-init="true"
+ parent="shibboleth.StorageBackedAccountLockoutManager"
+ p:storageService-ref="#{'%{idp.duo.oidc.passwordless.enrollmentLockoutStorageService:shibboleth.StorageService}'.trim()}"
+ p:maxAttempts="%{idp.duo.oidc.passwordless.limitEnrollmentChecking.maxAttempts:30}"
+ p:counterInterval="%{idp.duo.oidc.passwordless.limitEnrollmentChecking.counterInterval:PT1M}"
+ p:lockoutDuration="%{idp.duo.oidc.passwordless.limitEnrollmentChecking.lockoutDuration:PT2M}"
+ p:extendLockoutDuration="false">
+ <property name="lockoutKeyStrategy">
+ <bean class="net.shibboleth.idp.plugin.authn.duo.DefaultPasswordlessEnrollmentCondition.IPLockoutKeyStrategy"
+ p:httpServletRequestSupplier-ref="shibboleth.HttpServletRequestSupplier" />
+ </property>
+ </bean>
+
</beans>
\ No newline at end of file
diff --git a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoAdminClientTest.java b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoAdminClientTest.java
index 1537f480..8c4939e1 100644
--- a/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoAdminClientTest.java
+++ b/idp-duo-impl/src/test/java/net/shibboleth/idp/plugin/authn/duo/impl/DefaultDuoAdminClientTest.java
@@ -377,7 +377,7 @@ public class DefaultDuoAdminClientTest {
client.initialize();
final User user = client.getUser(new ProfileRequestContext(), "jdoe");
- assertNotNull(user);
+ assert user != null;
assertEquals(user.getUsername(), "jdoe");
assertEquals(user.getWebAuthnCredentials().size(), 3);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list