[java-idp-plugin-duo] branch main updated: JDUO-85 - Admin API needs to implement rate limiting
Scott Cantor
cantor.2 at osu.edu
Thu Mar 28 17:11: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=fc263012e1d5ceb3c4b4d31e9fb9cb655277d349
The following commit(s) were added to refs/heads/main by this push:
new fc263012 JDUO-85 - Admin API needs to implement rate limiting
fc263012 is described below
commit fc263012e1d5ceb3c4b4d31e9fb9cb655277d349
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Mar 28 13:11:10 2024 -0400
JDUO-85 - Admin API needs to implement rate limiting
https://shibboleth.atlassian.net/browse/JDUO-85
Simple support for detecting 429 error and scaling backoff/retry.
---
.../authn/duo/impl/DefaultDuoAdminClient.java | 95 ++++++++++++++++++++--
.../META-INF/net.shibboleth.idp/postconfig.xml | 5 +-
.../authn/duo/impl/DefaultDuoAdminClientTest.java | 19 +++++
3 files changed, 110 insertions(+), 9 deletions(-)
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 728f100f..c0762f51 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
@@ -22,6 +22,7 @@ import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.util.List;
import java.util.Map;
+import java.util.Random;
import java.util.function.Function;
import javax.annotation.Nonnull;
@@ -71,9 +72,33 @@ import net.shibboleth.shared.primitive.LoggerFactory;
@ThreadSafeAfterInit
public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComponent implements DuoAdminClient {
+ /** Default rate limiting backoff multiplier. */
+ private static final int DEFAULT_BACKOFF_FACTOR = 2;
+
+ /** Default initial backoff delay. */
+ private static final int DEFAULT_INITIAL_BACKOFF_MS = 1000;
+
+ /** Default maximum backoff. */
+ private static final int DEFAULT_MAX_BACKOFF_MS = 16000;
+
+ /** The error returned for rate limiting rejection. */
+ private static final int RATE_LIMIT_ERROR_CODE = HttpStatus.SC_TOO_MANY_REQUESTS;
+
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DefaultDuoAdminClient.class);
+ /** Rate limiting backoff multiplier. */
+ private int backoffFactor;
+
+ /** Initial rate limiting delay. */
+ private int initialBackoff;
+
+ /** Maximum backoff delay. */
+ private int maxBackoff;
+
+ /** Generates random backoff delay. */
+ @Nonnull private final Random random;
+
/** HttpClient for contacting Duo. */
@NonnullAfterInit private HttpClient httpClient;
@@ -91,10 +116,45 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
/** Constructor.*/
public DefaultDuoAdminClient() {
+ random = new Random();
+ backoffFactor = DEFAULT_BACKOFF_FACTOR;
+ initialBackoff = DEFAULT_INITIAL_BACKOFF_MS;
+ maxBackoff = DEFAULT_MAX_BACKOFF_MS;
+
adminDuoIntegrationLookupStrategy = FunctionSupport.constant(null);
usersAdminEndpoint = "/admin/v1/users";
}
+ /**
+ * Set the rate limiting multipler factor.
+ *
+ * @param factor multipler
+ */
+ public void setBackoffFactor(final int factor) {
+ checkSetterPreconditions();
+ backoffFactor = factor;
+ }
+
+ /**
+ * Set the initial backoff delay.
+ *
+ * @param backoff initial backoff
+ */
+ public void setInitialBackoff(final int backoff) {
+ checkSetterPreconditions();
+ initialBackoff = backoff;
+ }
+
+ /**
+ * Set the maximum backoff delay.
+ *
+ * @param backoff maximum backoff
+ */
+ public void setMaxBackoff(final int backoff) {
+ checkSetterPreconditions();
+ maxBackoff = backoff;
+ }
+
/**
* Set the {@link HttpClient} to use for contacting Duo.
*
@@ -170,8 +230,8 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
buildRequest(context, usersAdminEndpoint, Map.of(DuoAuthAPI.DUO_USERNAME, username));
// execute the request
- final List<User> response =
- doAPIRequest(request, new TypeReference<DuoAdminResponseWrapper<List<User>>>() {}).getResponse();
+ 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");
}
@@ -193,7 +253,7 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
throws DuoException {
try {
final ClassicHttpRequest request = buildRequest(context, path, parameters);
- return doAPIRequest(request, wrapperTypeRef);
+ return doAPIRequest(request, wrapperTypeRef, initialBackoff);
} catch (final DuoException | IOException e) {
//Wrap the exception
throw new DuoException(e);
@@ -206,7 +266,7 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
throws DuoException {
try {
final ClassicHttpRequest request = buildRequest(context, path, parameters);
- return doAPIRequest(request, new TypeReference<DuoAdminListMapResponseWrapper>() {});
+ return doAPIRequest(request, new TypeReference<DuoAdminListMapResponseWrapper>() {}, initialBackoff);
} catch (final DuoException | IOException e) {
//Wrap the exception
throw new DuoException(e);
@@ -276,14 +336,15 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
}
-// Checkstyle: CyclomaticComplexity OFF
+// Checkstyle: CyclomaticComplexity|MethodLength OFF
/**
* Performs a call to the Duo AdminAPI. Upon a successful call, the JSON response is mapped into the appropriate
* type of {@link DuoAdminResponseWrapper}.
*
+ * @param <T> the DuoResponse type being wrapped
* @param request the prepared HTTP request
* @param wrapperTypeRef the type of {@link DuoResponseWrapper} to use
- * @param <T> the DuoResponse type being wrapped
+ * @param backoff the backoff delay to apply if needed
*
* @return a {@link DuoResponseWrapper}
*
@@ -291,7 +352,7 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
* @throws DuoException on a Duo-related error
*/
@Nonnull private <T extends DuoAdminResponseWrapper<?>> T doAPIRequest(@Nonnull final ClassicHttpRequest request,
- @Nonnull final TypeReference<T> wrapperTypeRef)
+ @Nonnull final TypeReference<T> wrapperTypeRef, final int backoff)
throws DuoException, IOException {
// Make the request.
@@ -323,6 +384,24 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
}
}
}
+
+ if (httpStatusCode == RATE_LIMIT_ERROR_CODE) {
+ if (backoff <= maxBackoff) {
+ log.warn("Duo admin API request rejected due to rate limiting, waiting {} seconds for next attempt",
+ backoff / 1000);
+ try {
+ Thread.sleep(backoff + random.nextInt(1000));
+ } catch (final InterruptedException e) {
+ log.info("Sleeping Duo admin API thread interrupted");
+ throw new IOException(e);
+ }
+ final int newBackoff = backoff * backoffFactor;
+ return doAPIRequest(request, wrapperTypeRef, newBackoff);
+ } else {
+ log.warn("Duo admin API request rejected due to rate limiting, exhausted backoff attemots");
+ }
+ }
+
if (httpStatusCode != HttpStatus.SC_OK) {
throw new IOException("Non-ok status code (" + httpStatusCode + ") returned from Duo: "
+ (httpResponse.getReasonPhrase() != null ? httpResponse.getReasonPhrase() : "none"));
@@ -348,6 +427,6 @@ public class DefaultDuoAdminClient extends AbstractIdentifiableInitializableComp
}
}
}
-// Checkstyle: CyclomaticComplexity ON
+// Checkstyle: CyclomaticComplexity|MethodLength ON
}
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 e42c17fb..99a45fb1 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
@@ -66,7 +66,10 @@
p:objectMapper-ref="shibboleth.JSONObjectMapper"
p:httpClient="#{getObject('shibboleth.authn.DuoOIDC.Admin.HttpClient') ?: getObject('shibboleth.InternalHttpClient')}"
p:httpClientSecurityParameters="#{getObject('shibboleth.authn.DuoOIDC.Admin.HttpClientSecurityParameters')}"
- p:adminDuoIntegrationLookupStrategy-ref="shibboleth.authn.DuoOIDC.Admin.DuoIntegrationStrategy"/>
+ p:adminDuoIntegrationLookupStrategy-ref="shibboleth.authn.DuoOIDC.Admin.DuoIntegrationStrategy"
+ p:backoffFactor="%{idp.duo.oidc.admin.backoffFactor:2}"
+ p:initialBackoff="%{idp.duo.oidc.admin.initialBackoff:1000}"
+ p:maxBackoff="%{idp.duo.oidc.admin.maxBackoff:16000}" />
<!-- Default passwordless condition that uses the admin API -->
<bean id="shibboleth.authn.DuoOIDC.Passwordless.DefaultCondition" lazy-init="true"
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 75b7d194..1537f480 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
@@ -26,6 +26,7 @@ import org.apache.hc.client5.http.classic.HttpClient;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.mockito.Mockito;
@@ -272,6 +273,7 @@ public class DefaultDuoAdminClientTest {
}
return integration;
});
+ client.setMaxBackoff(8000);
}
@@ -343,6 +345,23 @@ public class DefaultDuoAdminClientTest {
client.getUser(new ProfileRequestContext(), "jdoe");
}
+ @Test(expectedExceptions = DuoException.class)
+ public void testClientGetUsers_Backoff() throws Exception {
+ final HttpClient httpClient = Mockito.mock(HttpClient.class);
+ final ClassicHttpResponse httpResponse = Mockito.mock(ClassicHttpResponse.class);
+
+ Mockito.when(httpResponse.getCode()).thenReturn(HttpStatus.SC_TOO_MANY_REQUESTS);
+ Mockito.when(httpResponse.getEntity()).thenReturn(new StringEntity(""));
+ Mockito.when(httpClient.executeOpen((HttpHost) Mockito.any(), (ClassicHttpRequest) Mockito.any(),
+ (HttpContext) Mockito.any())).thenReturn(httpResponse);
+
+ client.setHttpClient(httpClient);
+ client.setObjectMapper(new ObjectMapper());
+ client.initialize();
+
+ client.getUser(new ProfileRequestContext(), "jdoe");
+ }
+
@Test
public void testClientGetUsers_OK() throws Exception {
final HttpClient httpClient = Mockito.mock(HttpClient.class);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list