Merge pull request #16060 from sberyozkin/oidc_request_retry

Update OIDC client code to retry in case of the connection errors and have OIDC starting without the complete config
quarkusio · Mar 30, 2021 · 25a0c42 · 25a0c42
2 parents aa4b5c5 + 7a630dd
commit 25a0c42
Show file tree

Hide file tree

Showing 12 changed files with 183 additions and 12 deletions.
diff --git a/...r/deployment/src/test/java/io/quarkus/oidc/client/filter/OidcClientFilterDevModeTest.java b/...r/deployment/src/test/java/io/quarkus/oidc/client/filter/OidcClientFilterDevModeTest.java
@@ -45,7 +45,16 @@ public void testGetUserName() {
                 .then()
                 .statusCode(401)
                 .body(equalTo("ProtectedResourceService requires a token"));
-        test.modifyResourceFile("application.properties", s -> s.replace("#", ""));
+        test.modifyResourceFile("application.properties", s -> s.replace("#quarkus.oidc-client-filter.register-filter",
+                "quarkus.oidc-client-filter.register-filter"));
+
+        // OidcClient configuration is not complete - Quarkus should start - but 500 returned
+        RestAssured.when().get("/frontend/user-before-registering-provider")
+                .then()
+                .statusCode(500);
+        test.modifyResourceFile("application.properties", s -> s.replace("#quarkus.oidc-client.auth-server-url",
+                "quarkus.oidc-client.auth-server-url"));
+
         // token lifespan (3 secs) is less than the auto-refresh interval so the token should be refreshed immediately 
         RestAssured.when().get("/frontend/user-after-registering-provider")
                 .then()

diff --git a/...idc-client-filter/deployment/src/test/resources/application-oidc-client-filter.properties b/...idc-client-filter/deployment/src/test/resources/application-oidc-client-filter.properties
@@ -2,7 +2,7 @@ quarkus.oidc.auth-server-url=${keycloak.url}/realms/quarkus/
 quarkus.oidc.client-id=quarkus-app
 quarkus.oidc.credentials.secret=secret
 
-quarkus.oidc-client.auth-server-url=${quarkus.oidc.auth-server-url}
+#quarkus.oidc-client.auth-server-url=${quarkus.oidc.auth-server-url}
 quarkus.oidc-client.client-id=${quarkus.oidc.client-id}
 quarkus.oidc-client.credentials.client-secret.value=${quarkus.oidc.credentials.secret}
 quarkus.oidc-client.credentials.client-secret.method=POST

diff --git a/...t-filter/runtime/src/main/java/io/quarkus/oidc/client/filter/OidcClientRequestFilter.java b/...t-filter/runtime/src/main/java/io/quarkus/oidc/client/filter/OidcClientRequestFilter.java
@@ -14,6 +14,7 @@
 import org.jboss.logging.Logger;
 
 import io.quarkus.oidc.client.runtime.AbstractTokensProducer;
+import io.quarkus.oidc.client.runtime.DisabledOidcClientException;
 import io.quarkus.oidc.common.runtime.OidcConstants;
 
 @Provider
@@ -28,6 +29,8 @@ public void filter(ClientRequestContext requestContext) throws IOException {
         try {
             final String accessToken = getAccessToken();
             requestContext.getHeaders().add(HttpHeaders.AUTHORIZATION, BEARER_SCHEME_WITH_SPACE + accessToken);
+        } catch (DisabledOidcClientException ex) {
+            requestContext.abortWith(Response.status(500).build());
         } catch (Exception ex) {
             LOG.debugf("Access token is not available, aborting the request with HTTP 401 error: %s", ex.getMessage());
             requestContext.abortWith(Response.status(401).build());

diff --git a/...ent/runtime/src/main/java/io/quarkus/oidc/client/runtime/DisabledOidcClientException.java b/...ent/runtime/src/main/java/io/quarkus/oidc/client/runtime/DisabledOidcClientException.java
@@ -0,0 +1,20 @@
+package io.quarkus.oidc.client.runtime;
+
+@SuppressWarnings("serial")
+public class DisabledOidcClientException extends RuntimeException {
+    public DisabledOidcClientException() {
+
+    }
+
+    public DisabledOidcClientException(String errorMessage) {
+        this(errorMessage, null);
+    }
+
+    public DisabledOidcClientException(Throwable cause) {
+        this(null, cause);
+    }
+
+    public DisabledOidcClientException(String errorMessage, Throwable cause) {
+        super(errorMessage, cause);
+    }
+}
diff --git a/...ions/oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientImpl.java b/...ions/oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientImpl.java
@@ -1,8 +1,10 @@
 package io.quarkus.oidc.client.runtime;
 
 import java.io.IOException;
+import java.net.ConnectException;
 import java.nio.charset.StandardCharsets;
 import java.security.Key;
+import java.time.Duration;
 import java.time.Instant;
 import java.util.Base64;
 import java.util.function.Supplier;
@@ -29,6 +31,7 @@ public class OidcClientImpl implements OidcClient {
 
     private static final Logger LOG = Logger.getLogger(OidcClientImpl.class);
 
+    private static final Duration REQUEST_RETRY_BACKOFF_DURATION = Duration.ofSeconds(1);
     private static final String AUTHORIZATION_HEADER = String.valueOf(HttpHeaders.AUTHORIZATION);
 
     private final WebClient client;
@@ -84,7 +87,12 @@ public Uni<Tokens> get() {
                     body.add(OidcConstants.CLIENT_ASSERTION_TYPE, OidcConstants.JWT_BEARER_CLIENT_ASSERTION_TYPE);
                     body.add(OidcConstants.CLIENT_ASSERTION, OidcCommonUtils.signJwtWithKey(oidcConfig, clientJwtKey));
                 }
-                return request.sendForm(body).onItem()
+                // Retry up to three times with a one second delay between the retries if the connection is closed
+                Uni<HttpResponse<Buffer>> response = request.sendForm(body)
+                        .onFailure(ConnectException.class)
+                        .retry()
+                        .atMost(3);
+                return response.onItem()
                         .transform(resp -> emitGrantTokens(resp, refresh));
             }
         });

diff --git a/.../oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientRecorder.java b/.../oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientRecorder.java
@@ -97,7 +97,13 @@ protected static Uni<OidcClient> createOidcClientUni(OidcClientConfig oidcConfig
             oidcConfig.setId(oidcClientId);
         }
 
-        OidcCommonUtils.verifyCommonConfiguration(oidcConfig, false);
+        try {
+            OidcCommonUtils.verifyCommonConfiguration(oidcConfig, false);
+        } catch (Throwable t) {
+            String message = String.format("'%s' client configuration is not initialized", oidcClientId);
+            LOG.debug(message);
+            return Uni.createFrom().item(new DisabledOidcClient(message));
+        }
 
         String authServerUriString = OidcCommonUtils.getAuthServerUrl(oidcConfig);
 
@@ -200,17 +206,17 @@ private static class DisabledOidcClient implements OidcClient {
 
         @Override
         public Uni<Tokens> getTokens() {
-            throw new OidcClientException(message);
+            throw new DisabledOidcClientException(message);
         }
 
         @Override
         public Uni<Tokens> refreshTokens(String refreshToken) {
-            throw new OidcClientException(message);
+            throw new DisabledOidcClientException(message);
         }
 
         @Override
         public void close() throws IOException {
-            throw new OidcClientException(message);
+            throw new DisabledOidcClientException(message);
         }
     }
 }
diff --git a/...c/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowDevModeDefaultTenantTestCase.java b/...c/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowDevModeDefaultTenantTestCase.java
@@ -0,0 +1,74 @@
+package io.quarkus.oidc.test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import java.io.IOException;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.SilentCssErrorHandler;
+import com.gargoylesoftware.htmlunit.WebClient;
+import com.gargoylesoftware.htmlunit.html.HtmlForm;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+
+import io.quarkus.test.QuarkusDevModeTest;
+import io.quarkus.test.common.QuarkusTestResource;
+
+@QuarkusTestResource(KeycloakDevModeRealmResourceManager.class)
+public class CodeFlowDevModeDefaultTenantTestCase {
+
+    private static Class<?>[] testClasses = {
+            ProtectedResource.class,
+            UnprotectedResource.class
+    };
+
+    @RegisterExtension
+    static final QuarkusDevModeTest test = new QuarkusDevModeTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(testClasses)
+                    .addAsResource("application-dev-mode-default-tenant.properties", "application.properties"));
+
+    @Test
+    public void testAccessAndRefreshTokenInjectionDevMode() throws IOException, InterruptedException {
+        try (final WebClient webClient = createWebClient()) {
+
+            try {
+                webClient.getPage("http://localhost:8080/protected");
+                fail("Exception is expected because auth-server-url is not available and the authentication can not be completed");
+            } catch (FailingHttpStatusCodeException ex) {
+                // Reported by Quarkus
+                assertEquals(500, ex.getStatusCode());
+            }
+
+            // Enable auth-server-url
+            test.modifyResourceFile("application.properties",
+                    s -> s.replace("#quarkus.oidc.auth-server-url", "quarkus.oidc.auth-server-url"));
+
+            HtmlPage page = webClient.getPage("http://localhost:8080/protected");
+
+            assertEquals("Sign in to devmode", page.getTitleText());
+
+            HtmlForm loginForm = page.getForms().get(0);
+
+            loginForm.getInputByName("username").setValueAttribute("alice-dev-mode");
+            loginForm.getInputByName("password").setValueAttribute("alice-dev-mode");
+
+            page = loginForm.getInputByName("login").click();
+
+            assertEquals("alice-dev-mode", page.getBody().asText());
+
+            webClient.getCookieManager().clearCookies();
+        }
+    }
+
+    private WebClient createWebClient() {
+        WebClient webClient = new WebClient();
+        webClient.setCssErrorHandler(new SilentCssErrorHandler());
+        return webClient;
+    }
+}
diff --git a/extensions/oidc/deployment/src/test/resources/application-dev-mode-default-tenant.properties b/extensions/oidc/deployment/src/test/resources/application-dev-mode-default-tenant.properties
@@ -0,0 +1,6 @@
+#quarkus.oidc.auth-server-url=${keycloak.url}/realms/devmode
+quarkus.oidc.client-id=client-dev-mode
+quarkus.oidc.application-type=web-app
+
+quarkus.log.category."com.gargoylesoftware.htmlunit.javascript.host.css.CSSStyleSheet".level=FATAL
+
diff --git a/...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTenantConfigResolver.java b/...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTenantConfigResolver.java
@@ -102,10 +102,17 @@ private TenantConfigContext getStaticTenantContext(RoutingContext context) {
         TenantConfigContext configContext = tenantId != null ? tenantConfigBean.getStaticTenantsConfig().get(tenantId) : null;
         if (configContext == null) {
             if (tenantId != null && !tenantId.isEmpty()) {
-                LOG.debugf("No configuration with a tenant id '%s' has been found, using the default configuration");
+                LOG.debugf(
+                        "Registered TenantResolver has not provided the configuration for tenant '%s', using the default tenant",
+                        tenantId);
             }
             configContext = tenantConfigBean.getDefaultTenant();
         }
+        if (!configContext.ready) {
+            LOG.debugf("Tenant '%s' is not initialized", tenantId);
+            configContext = null;
+        }
+
         return configContext;
     }
 

diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProviderClient.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProviderClient.java
@@ -1,7 +1,9 @@
 package io.quarkus.oidc.runtime;
 
+import java.net.ConnectException;
 import java.nio.charset.StandardCharsets;
 import java.security.Key;
+import java.time.Duration;
 
 import org.jboss.logging.Logger;
 
@@ -23,6 +25,8 @@
 
 public class OidcProviderClient {
     private static final Logger LOG = Logger.getLogger(OidcProviderClient.class);
+
+    private static final Duration REQUEST_RETRY_BACKOFF_DURATION = Duration.ofSeconds(1);
     private static final String AUTHORIZATION_HEADER = String.valueOf(HttpHeaders.AUTHORIZATION);
 
     private final WebClient client;
@@ -104,7 +108,12 @@ private UniOnItem<HttpResponse<Buffer>> getHttpResponse(String uri, MultiMap req
         } else {
             reqBody.add(OidcConstants.CLIENT_ID, oidcConfig.clientId.get());
         }
-        return request.sendForm(reqBody).onItem();
+        // Retry up to three times with a one second delay between the retries if the connection is closed.
+        Uni<HttpResponse<Buffer>> response = request.sendForm(reqBody)
+                .onFailure(ConnectException.class)
+                .retry()
+                .atMost(3);
+        return response.onItem();
     }
 
     private AuthorizationCodeTokens getAuthorizationCodeTokens(HttpResponse<Buffer> resp) {

diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcRecorder.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcRecorder.java
@@ -81,7 +81,9 @@ private Uni<TenantConfigContext> createDynamicTenantContext(Vertx vertx,
             OidcTenantConfig oidcConfig, TlsConfig tlsConfig, String tenantId) {
 
         if (!dynamicTenantsConfig.containsKey(tenantId)) {
-            return createTenantContext(vertx, oidcConfig, tlsConfig, tenantId).onItem().transform(
+            Uni<TenantConfigContext> uniContext = createTenantContext(vertx, oidcConfig, tlsConfig, tenantId);
+            uniContext.onFailure().transform(t -> logTenantConfigContextFailure(t, tenantId));
+            return uniContext.onItem().transform(
                     new Function<TenantConfigContext, TenantConfigContext>() {
                         @Override
                         public TenantConfigContext apply(TenantConfigContext t) {
@@ -97,7 +99,23 @@ public TenantConfigContext apply(TenantConfigContext t) {
     private TenantConfigContext createStaticTenantContext(Vertx vertx,
             OidcTenantConfig oidcConfig, TlsConfig tlsConfig, String tenantId) {
 
-        return createTenantContext(vertx, oidcConfig, tlsConfig, tenantId).await().indefinitely();
+        Uni<TenantConfigContext> uniContext = createTenantContext(vertx, oidcConfig, tlsConfig, tenantId);
+        return uniContext.onFailure()
+                .recoverWithItem(new Function<Throwable, TenantConfigContext>() {
+                    @Override
+                    public TenantConfigContext apply(Throwable t) {
+                        logTenantConfigContextFailure(t, tenantId);
+                        return new TenantConfigContext(null, oidcConfig, false);
+                    }
+                })
+                .await().indefinitely();
+    }
+
+    private static Throwable logTenantConfigContextFailure(Throwable t, String tenantId) {
+        LOG.debugf(
+                "'%s' tenant initialization has failed: '%s'. Access to resources protected by this tenant will fail with HTTP 401.",
+                tenantId, t.getMessage());
+        return t;
     }
 
     private Uni<TenantConfigContext> createTenantContext(Vertx vertx, OidcTenantConfig oidcConfig, TlsConfig tlsConfig,
@@ -114,7 +132,11 @@ private Uni<TenantConfigContext> createTenantContext(Vertx vertx, OidcTenantConf
             return Uni.createFrom().item(createTenantContextFromPublicKey(oidcConfig));
         }
 
-        OidcCommonUtils.verifyCommonConfiguration(oidcConfig, true);
+        try {
+            OidcCommonUtils.verifyCommonConfiguration(oidcConfig, true);
+        } catch (Throwable t) {
+            return Uni.createFrom().failure(t);
+        }
 
         if (!oidcConfig.discoveryEnabled) {
             if (oidcConfig.applicationType != ApplicationType.SERVICE) {

diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java
@@ -14,8 +14,15 @@ class TenantConfigContext {
      */
     final OidcTenantConfig oidcConfig;
 
+    final boolean ready;
+
     public TenantConfigContext(OidcProvider client, OidcTenantConfig config) {
+        this(client, config, true);
+    }
+
+    public TenantConfigContext(OidcProvider client, OidcTenantConfig config, boolean ready) {
         this.provider = client;
         this.oidcConfig = config;
+        this.ready = ready;
     }
 }