feat(auth): apply auth map fn on every request

src/main/java/io/cryostat/agent/AuthorizationType.java

@@ -24,46 +24,56 @@
 
 import io.cryostat.agent.util.StringUtils;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 public enum AuthorizationType implements Function<String, String> {
-    NONE(v -> null),
-    BEARER(v -> String.format("Bearer %s", v)),
+    NONE(false, v -> null),
+    BEARER(false, v -> String.format("Bearer %s", v)),
     BASIC(
+            false,
             v ->
                     String.format(
                             "Basic %s",
                             Base64.getEncoder()
                                     .encodeToString(v.getBytes(StandardCharsets.UTF_8)))),
     KUBERNETES(
+            true,
             v -> {
                 try {
                     File file = new File(v);
                     String token = Files.readString(file.toPath()).strip();
                     return String.format("Bearer %s", token);
                 } catch (IOException ioe) {
-                    Logger log = LoggerFactory.getLogger(AuthorizationType.class);
-                    log.warn(String.format("Failed to read serviceaccount token from %s", v), ioe);
-                    return null;
+                    throw new RuntimeException(
+                            String.format("Failed to read serviceaccount token from %s", v), ioe);
                 }
             }),
     AUTO(
+            true,
             v -> {
-                String k8s = KUBERNETES.fn.apply(v);
-                if (StringUtils.isNotBlank(k8s)) {
-                    return k8s;
+                try {
+                    String k8s = KUBERNETES.fn.apply(v);
+                    if (StringUtils.isNotBlank(k8s)) {
+                        return k8s;
+                    }
+                } catch (Exception e) {
+                    // ignore
                 }
                 return NONE.fn.apply(v);
             }),
     ;
 
+    private final boolean dynamic;
     private final Function<String, String> fn;
 
-    private AuthorizationType(Function<String, String> fn) {
+    private AuthorizationType(boolean dynamic, Function<String, String> fn) {
+        this.dynamic = dynamic;
         this.fn = fn;
     }
 
+    public boolean isDynamic() {
+        // if the authorization value may change between invocations
+        return this.dynamic;
+    }
+
     @Override
     public String apply(String in) {
         return fn.apply(in);

src/main/java/io/cryostat/agent/ConfigModule.java

@@ -33,6 +33,7 @@
 import java.util.Optional;
 import java.util.UUID;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -81,6 +82,8 @@ public abstract class ConfigModule {
     public static final Pattern CRYOSTAT_AGENT_TRUSTSTORE_PATTERN =
             Pattern.compile(
                     "^(?:cryostat\\.agent\\.webclient\\.tls\\.truststore\\.cert)\\[(?<index>\\d+)\\]\\.(?<property>.*)$");
+    public static final String CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT =
+            "cryostat.agent.webclient.response.retry-count";
 
     public static final String CRYOSTAT_AGENT_WEBSERVER_HOST = "cryostat.agent.webserver.host";
     public static final String CRYOSTAT_AGENT_WEBSERVER_PORT = "cryostat.agent.webserver.port";
@@ -201,12 +204,12 @@ public static String provideCryostatAgentRealm(
     @Provides
     @Singleton
     @Named(CRYOSTAT_AGENT_AUTHORIZATION)
-    public static Optional<String> provideCryostatAgentAuthorization(
+    public static Supplier<Optional<String>> provideCryostatAgentAuthorization(
             Config config,
             AuthorizationType authorizationType,
             @Named(CRYOSTAT_AGENT_AUTHORIZATION_VALUE) Optional<String> authorizationValue) {
         Optional<String> opt = config.getOptionalValue(CRYOSTAT_AGENT_AUTHORIZATION, String.class);
-        return opt.or(() -> authorizationValue.map(authorizationType::apply));
+        return () -> opt.or(() -> authorizationValue.map(authorizationType::apply));
     }
 
     @Provides
@@ -310,6 +313,13 @@ public static List<TruststoreConfig> provideCryostatAgentWecblientTlsTruststoreC
         return truststoreConfigs;
     }
 
+    @Provides
+    @Singleton
+    @Named(CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT)
+    public static int provideCryostatAgentWebclientResponseRetryCount(Config config) {
+        return config.getValue(CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT, int.class);
+    }
+
     @Provides
     @Singleton
     @Named(CRYOSTAT_AGENT_WEBSERVER_HOST)

src/main/java/io/cryostat/agent/CryostatClient.java

@@ -35,6 +35,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.function.Function;
+import java.util.function.Supplier;
 
 import io.cryostat.agent.FlightRecorderHelper.ConfigurationInfo;
 import io.cryostat.agent.FlightRecorderHelper.TemplatedRecording;
@@ -80,6 +81,7 @@ public class CryostatClient {
     private final Executor executor;
     private final ObjectMapper mapper;
     private final HttpClient http;
+    private final Supplier<Optional<String>> authorizationSupplier;
 
     private final String appName;
     private final String instanceId;
@@ -91,6 +93,7 @@ public class CryostatClient {
             Executor executor,
             ObjectMapper mapper,
             HttpClient http,
+            Supplier<Optional<String>> authorizationSupplier,
             String instanceId,
             String jvmId,
             String appName,
@@ -99,6 +102,7 @@ public class CryostatClient {
         this.executor = executor;
         this.mapper = mapper;
         this.http = http;
+        this.authorizationSupplier = authorizationSupplier;
         this.instanceId = instanceId;
         this.jvmId = jvmId;
         this.appName = appName;
@@ -450,6 +454,12 @@ private HttpResponse logResponse(HttpRequestBase req, HttpResponse res) {
     }
 
     private <T> CompletableFuture<T> supply(HttpRequestBase req, Function<HttpResponse, T> fn) {
+        // FIXME Apache httpclient 4 does not support Bearer token auth easily, so we explicitly set
+        // the header here. This is a form of preemptive auth - the token is always sent with the
+        // request. It would be better to attempt to send the request to the server first and see if
+        // it responds with an auth challenge, and then send the auth information we have, and use
+        // the client auth cache. This flow is supported for Bearer tokens in httpclient 5.
+        authorizationSupplier.get().ifPresent(v -> req.addHeader(HttpHeaders.AUTHORIZATION, v));
         return CompletableFuture.supplyAsync(() -> fn.apply(executeQuiet(req)), executor)
                 .whenComplete((v, t) -> req.reset());
     }

src/main/java/io/cryostat/agent/MainModule.java

@@ -31,14 +31,13 @@
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
 
 import javax.inject.Named;
 import javax.inject.Singleton;
@@ -67,12 +66,14 @@
 import dagger.Provides;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.exception.ExceptionUtils;
-import org.apache.http.Header;
+import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.protocol.HttpClientContext;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.client.HttpClients;
-import org.apache.http.message.BasicHeader;
+import org.apache.http.impl.client.StandardHttpRequestRetryHandler;
+import org.apache.http.protocol.HttpContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -332,29 +333,56 @@ public static Optional<SSLContext> provideServerSslContext(
     @Provides
     @Singleton
     public static HttpClient provideHttpClient(
-            @Named(HTTP_CLIENT_SSL_CTX) SSLContext sslContext,
             AuthorizationType authorizationType,
-            @Named(ConfigModule.CRYOSTAT_AGENT_AUTHORIZATION) Optional<String> authorization,
+            @Named(HTTP_CLIENT_SSL_CTX) SSLContext sslContext,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_VERIFY_HOSTNAME)
                     boolean verifyHostname,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_CONNECT_TIMEOUT_MS) int connectTimeout,
-            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_TIMEOUT_MS) int responseTimeout) {
-        Set<Header> headers = new HashSet<>();
-        authorization
-                .filter(Objects::nonNull)
-                .map(v -> new BasicHeader("Authorization", v))
-                .ifPresent(headers::add);
+            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_TIMEOUT_MS) int responseTimeout,
+            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT) int retryCount) {
         HttpClientBuilder builder =
                 HttpClients.custom()
-                        .setDefaultHeaders(headers)
                         .setSSLContext(sslContext)
                         .setDefaultRequestConfig(
                                 RequestConfig.custom()
                                         .setAuthenticationEnabled(true)
                                         .setExpectContinueEnabled(true)
                                         .setConnectTimeout(connectTimeout)
                                         .setSocketTimeout(responseTimeout)
-                                        .build());
+                                        .setRedirectsEnabled(true)
+                                        .build())
+                        .setRetryHandler(
+                                new StandardHttpRequestRetryHandler(retryCount, true) {
+                                    @Override
+                                    public boolean retryRequest(
+                                            IOException exception,
+                                            int executionCount,
+                                            HttpContext context) {
+                                        // if the Authorization header we should send may change
+                                        // over time, ex. we read a Bearer token from a file, then
+                                        // it is possible that we get a 401 or 403 response because
+                                        // the token expired in between the time that we read it
+                                        // from our filesystem and when it was received by the
+                                        // authenticator. So, in this set of conditions, we should
+                                        // refresh our header value and try again right away
+                                        if (authorizationType.isDynamic()) {
+                                            HttpClientContext clientCtx =
+                                                    HttpClientContext.adapt(context);
+                                            if (clientCtx.isRequestSent()) {
+                                                HttpResponse resp = clientCtx.getResponse();
+                                                if (resp != null && resp.getStatusLine() != null) {
+                                                    int sc = resp.getStatusLine().getStatusCode();
+                                                    if (executionCount < 2
+                                                            && (sc == 401 || sc == 403)) {
+                                                        return true;
+                                                    }
+                                                }
+                                            }
+                                        }
+                                        return super.retryRequest(
+                                                exception, executionCount, context);
+                                    }
+                                });
 
         if (!verifyHostname) {
             builder = builder.setSSLHostnameVerifier((hostname, session) -> true);
@@ -412,13 +440,23 @@ public static CryostatClient provideCryostatClient(
             ScheduledExecutorService executor,
             ObjectMapper objectMapper,
             HttpClient http,
+            @Named(ConfigModule.CRYOSTAT_AGENT_AUTHORIZATION)
+                    Supplier<Optional<String>> authorizationSupplier,
             @Named(ConfigModule.CRYOSTAT_AGENT_INSTANCE_ID) String instanceId,
             @Named(JVM_ID) String jvmId,
             @Named(ConfigModule.CRYOSTAT_AGENT_APP_NAME) String appName,
             @Named(ConfigModule.CRYOSTAT_AGENT_BASEURI) URI baseUri,
             @Named(ConfigModule.CRYOSTAT_AGENT_REALM) String realm) {
         return new CryostatClient(
-                executor, objectMapper, http, instanceId, jvmId, appName, baseUri, realm);
+                executor,
+                objectMapper,
+                http,
+                authorizationSupplier,
+                instanceId,
+                jvmId,
+                appName,
+                baseUri,
+                realm);
     }
 
     @Provides

src/main/resources/META-INF/microprofile-config.properties

@@ -9,6 +9,7 @@ cryostat.agent.webclient.tls.trust-all=false
 cryostat.agent.webclient.tls.verify-hostname=true
 cryostat.agent.webclient.connect.timeout-ms=1000
 cryostat.agent.webclient.response.timeout-ms=1000
+cryostat.agent.webclient.response.retry-count=3
 cryostat.agent.webserver.host=0.0.0.0
 cryostat.agent.webserver.port=9977
 cryostat.agent.webserver.tls.version=${cryostat.agent.webclient.tls.version}