fix(discovery): respond 400, not 500, on validation failures

src/main/java/io/cryostat/discovery/Discovery.java

@@ -39,7 +39,6 @@
 import io.cryostat.util.URIUtil;
 
 import com.fasterxml.jackson.annotation.JsonView;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.nimbusds.jose.JOSEException;
 import com.nimbusds.jwt.proc.BadJWTException;
@@ -163,15 +162,18 @@ public DiscoveryNode get() {
     @Path("/api/v4/discovery/{id}")
     @RolesAllowed("read")
     public void checkRegistration(
-            @Context RoutingContext ctx, @RestPath UUID id, @RestQuery String token)
-            throws SocketException,
-                    UnknownHostException,
-                    MalformedURLException,
-                    ParseException,
-                    JOSEException,
-                    URISyntaxException {
+            @Context RoutingContext ctx, @RestPath UUID id, @RestQuery String token) {
         DiscoveryPlugin plugin = DiscoveryPlugin.find("id", id).singleResult();
-        jwtValidator.validateJwt(ctx, plugin, token, true);
+        try {
+            jwtValidator.validateJwt(ctx, plugin, token, true);
+        } catch (MalformedURLException
+                | URISyntaxException
+                | UnknownHostException
+                | SocketException
+                | JOSEException
+                | ParseException e) {
+            throw new BadRequestException(e);
+        }
     }
 
     @Transactional
@@ -182,39 +184,54 @@ public void checkRegistration(
     @RolesAllowed("write")
     @SuppressFBWarnings("DLS_DEAD_LOCAL_STORE")
     public PluginRegistration register(@Context RoutingContext ctx, JsonObject body)
-            throws URISyntaxException,
-                    MalformedURLException,
-                    JOSEException,
-                    UnknownHostException,
-                    SocketException,
-                    ParseException,
-                    BadJWTException,
-                    SchedulerException {
+            throws SchedulerException {
         String pluginId = body.getString("id");
         String priorToken = body.getString("token");
         String realmName = body.getString("realm");
-        URI callbackUri = new URI(body.getString("callback"));
+        URI callbackUri;
+        try {
+            String callback = body.getString("callback");
+            if (StringUtils.isBlank(callback)) {
+                throw new BadRequestException("callback cannot be blank");
+            }
+            callbackUri = new URI(callback);
+        } catch (URISyntaxException e) {
+            throw new BadRequestException(e);
+        }
         URI unauthCallback = UriBuilder.fromUri(callbackUri).userInfo(null).build();
 
-        // URI range validation
-        if (!uriUtil.validateUri(callbackUri)) {
-            throw new BadRequestException(
-                    String.format(
-                            "cryostat.target.callback of \"%s\" is unacceptable with the"
-                                    + " current URI range settings",
-                            callbackUri));
+        InetAddress remoteAddress = getRemoteAddress(ctx);
+        URI remoteURI;
+        try {
+            remoteURI = new URI(remoteAddress.getHostAddress());
+        } catch (URISyntaxException e) {
+            throw new BadRequestException(e);
         }
 
-        InetAddress remoteAddress = getRemoteAddress(ctx);
-        URI remoteURI = new URI(remoteAddress.getHostAddress());
-        if (!uriUtil.validateUri(remoteURI)) {
-            throw new BadRequestException(
-                    String.format(
-                            "Remote Address of \"%s\" is unacceptable with the"
-                                    + " current URI range settings",
-                            remoteURI));
+        try {
+            if (!uriUtil.validateUri(callbackUri)) {
+                throw new BadRequestException(
+                        String.format(
+                                "cryostat.target.callback of \"%s\" is unacceptable with the"
+                                        + " current URI range settings",
+                                callbackUri));
+            }
+            if (!uriUtil.validateUri(remoteURI)) {
+                throw new BadRequestException(
+                        String.format(
+                                "Remote Address of \"%s\" is unacceptable with the"
+                                        + " current URI range settings",
+                                remoteURI));
+            }
+        } catch (MalformedURLException e) {
+            throw new BadRequestException(e);
         }
 
+        for (var e : new String[] {callbackUri.getScheme(), callbackUri.getHost()}) {
+            if (StringUtils.isBlank(e)) {
+                throw new BadRequestException("callback must contain scheme and host");
+            }
+        }
         if (agentTlsRequired && !callbackUri.getScheme().equals("https")) {
             throw new BadRequestException(
                     String.format(
@@ -233,10 +250,20 @@ public PluginRegistration register(@Context RoutingContext ctx, JsonObject body)
                 throw new ForbiddenException();
             }
             if (!Objects.equals(plugin.callback, unauthCallback)) {
-                throw new BadRequestException();
+                throw new BadRequestException("plugin callback mismatch");
+            }
+            try {
+                location = jwtFactory.getPluginLocation(plugin);
+                jwtFactory.parseDiscoveryPluginJwt(
+                        plugin, priorToken, location, remoteAddress, false);
+            } catch (URISyntaxException
+                    | UnknownHostException
+                    | SocketException
+                    | BadJWTException
+                    | JOSEException
+                    | ParseException e) {
+                throw new BadRequestException(e);
             }
-            location = jwtFactory.getPluginLocation(plugin);
-            jwtFactory.parseDiscoveryPluginJwt(plugin, priorToken, location, remoteAddress, false);
         } else {
             // check if a plugin record with the same callback already exists. If it does,
             // ping it:
@@ -274,7 +301,11 @@ public PluginRegistration register(@Context RoutingContext ctx, JsonObject body)
             universe.children.add(plugin.realm);
             universe.persist();
 
-            location = jwtFactory.getPluginLocation(plugin);
+            try {
+                location = jwtFactory.getPluginLocation(plugin);
+            } catch (URISyntaxException e) {
+                throw new BadRequestException(e);
+            }
 
             var dataMap = new JobDataMap();
             dataMap.put(PLUGIN_ID_MAP_KEY, plugin.id);
@@ -297,7 +328,12 @@ public PluginRegistration register(@Context RoutingContext ctx, JsonObject body)
             scheduler.scheduleJob(jobDetail, trigger);
         }
 
-        String token = jwtFactory.createDiscoveryPluginJwt(plugin, remoteAddress, location);
+        String token;
+        try {
+            token = jwtFactory.createDiscoveryPluginJwt(plugin, remoteAddress, location);
+        } catch (URISyntaxException | JOSEException | UnknownHostException | SocketException e) {
+            throw new BadRequestException(e);
+        }
 
         // TODO implement more generic env map passing by some platform detection
         // strategy or generalized config properties
@@ -318,26 +354,32 @@ public void publish(
             @Context RoutingContext ctx,
             @RestPath UUID id,
             @RestQuery String token,
-            List<DiscoveryNode> body)
-            throws SocketException,
-                    UnknownHostException,
-                    MalformedURLException,
-                    ParseException,
-                    JOSEException,
-                    URISyntaxException {
+            List<DiscoveryNode> body) {
         DiscoveryPlugin plugin = DiscoveryPlugin.find("id", id).singleResult();
-        jwtValidator.validateJwt(ctx, plugin, token, true);
+        try {
+            jwtValidator.validateJwt(ctx, plugin, token, true);
+        } catch (MalformedURLException
+                | URISyntaxException
+                | UnknownHostException
+                | SocketException
+                | JOSEException
+                | ParseException e) {
+            throw new BadRequestException(e);
+        }
         plugin.realm.children.clear();
         plugin.realm.children.addAll(body);
         for (var b : body) {
             if (b.target != null) {
-                // URI range validation
-                if (!uriUtil.validateUri(b.target.connectUrl)) {
-                    throw new BadRequestException(
-                            String.format(
-                                    "Connect URL of \"%s\" is unacceptable with the"
-                                            + " current URI range settings",
-                                    b.target.connectUrl));
+                try {
+                    if (!uriUtil.validateUri(b.target.connectUrl)) {
+                        throw new BadRequestException(
+                                String.format(
+                                        "Connect URL of \"%s\" is unacceptable with the"
+                                                + " current URI range settings",
+                                        b.target.connectUrl));
+                    }
+                } catch (MalformedURLException e) {
+                    throw new BadRequestException(e);
                 }
                 if (!uriUtil.isJmxUrl(b.target.connectUrl)) {
                     if (agentTlsRequired && !b.target.connectUrl.getScheme().equals("https")) {
@@ -370,15 +412,18 @@ public void publish(
     @Path("/api/v4/discovery/{id}")
     @PermitAll
     public void deregister(@Context RoutingContext ctx, @RestPath UUID id, @RestQuery String token)
-            throws SocketException,
-                    UnknownHostException,
-                    MalformedURLException,
-                    ParseException,
-                    JOSEException,
-                    URISyntaxException,
-                    SchedulerException {
+            throws SchedulerException {
         DiscoveryPlugin plugin = DiscoveryPlugin.find("id", id).singleResult();
-        jwtValidator.validateJwt(ctx, plugin, token, false);
+        try {
+            jwtValidator.validateJwt(ctx, plugin, token, false);
+        } catch (MalformedURLException
+                | URISyntaxException
+                | UnknownHostException
+                | SocketException
+                | JOSEException
+                | ParseException e) {
+            throw new BadRequestException(e);
+        }
         if (plugin.builtin) {
             throw new ForbiddenException();
         }
@@ -396,8 +441,7 @@ public void deregister(@Context RoutingContext ctx, @RestPath UUID id, @RestQuer
     @JsonView(DiscoveryNode.Views.Flat.class)
     @Path("/api/v4/discovery_plugins")
     @RolesAllowed("read")
-    public List<DiscoveryPlugin> getPlugins(@RestQuery String realm)
-            throws JsonProcessingException {
+    public List<DiscoveryPlugin> getPlugins(@RestQuery String realm) {
         // TODO filter for the matching realm name within the DB query
         return DiscoveryPlugin.findAll().<DiscoveryPlugin>list().stream()
                 .filter(p -> StringUtils.isBlank(realm) || p.realm.name.equals(realm))
@@ -407,7 +451,7 @@ public List<DiscoveryPlugin> getPlugins(@RestQuery String realm)
     @GET
     @Path("/api/v4/discovery_plugins/{id}")
     @RolesAllowed("read")
-    public DiscoveryPlugin getPlugin(@RestPath UUID id) throws JsonProcessingException {
+    public DiscoveryPlugin getPlugin(@RestPath UUID id) {
         return DiscoveryPlugin.find("id", id).singleResult();
     }
 

src/main/resources/application-test.properties

@@ -8,6 +8,8 @@ cryostat.discovery.kubernetes.enabled=true
 quarkus.test.env.JAVA_OPTS_APPEND=-Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.port=9091 -Dcom.sun.management.jmxremote.rmi.port=9091 -Djava.rmi.server.hostname=127.0.0.1 -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.local.only=false
 quarkus.http.test-timeout=60s
 
+cryostat.agent.tls.required=false
+
 quarkus.datasource.devservices.enabled=true
 quarkus.datasource.devservices.image-name=quay.io/cryostat/cryostat-db
 quarkus.hibernate-orm.log.sql=true

src/test/java/io/cryostat/AbstractTestBase.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright The Cryostat Authors.
+ *
+ * Licensed 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 io.cryostat;
+
+import static io.restassured.RestAssured.given;
+
+import java.time.Duration;
+
+import io.cryostat.util.HttpStatusCodeIdentifier;
+
+import io.restassured.http.ContentType;
+import jakarta.inject.Inject;
+import org.apache.http.client.utils.URLEncodedUtils;
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+import org.jboss.logging.Logger;
+import org.junit.jupiter.api.BeforeEach;
+import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.model.HeadBucketRequest;
+
+public abstract class AbstractTestBase {
+
+    public static final String SELF_JMX_URL = "service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi";
+    public static String SELF_JMX_URL_ENCODED =
+            URLEncodedUtils.formatSegments(SELF_JMX_URL).substring(1);
+    public static final String SELFTEST_ALIAS = "selftest";
+
+    @ConfigProperty(name = "storage.buckets.archives.name")
+    String archivesBucket;
+
+    @ConfigProperty(name = "test.storage.timeout", defaultValue = "5m")
+    Duration storageTimeout;
+
+    @ConfigProperty(name = "test.storage.retry", defaultValue = "5s")
+    Duration storageRetry;
+
+    @Inject Logger logger;
+    @Inject S3Client storage;
+
+    @BeforeEach
+    void waitForStorage() throws InterruptedException {
+        long totalTime = 0;
+        while (!bucketExists(archivesBucket)) {
+            long start = System.nanoTime();
+            Thread.sleep(storageRetry.toMillis());
+            long elapsed = System.nanoTime() - start;
+            totalTime += elapsed;
+            if (Duration.ofNanos(totalTime).compareTo(storageTimeout) > 0) {
+                throw new IllegalStateException("Storage took too long to become ready");
+            }
+        }
+    }
+
+    private boolean bucketExists(String bucket) {
+        boolean exists = false;
+        try {
+            exists =
+                    HttpStatusCodeIdentifier.isSuccessCode(
+                            storage.headBucket(HeadBucketRequest.builder().bucket(bucket).build())
+                                    .sdkHttpResponse()
+                                    .statusCode());
+            logger.debugv("Storage bucket \"{0}\" exists? {1}", bucket, exists);
+        } catch (Exception e) {
+            logger.warn(e);
+        }
+        return exists;
+    }
+
+    protected int defineSelfCustomTarget() {
+        return given().basePath("/")
+                .log()
+                .all()
+                .contentType(ContentType.URLENC)
+                .formParam("connectUrl", SELF_JMX_URL)
+                .formParam("alias", SELFTEST_ALIAS)
+                .when()
+                .post("/api/v4/targets")
+                .then()
+                .log()
+                .all()
+                .extract()
+                .jsonPath()
+                .getInt("id");
+    }
+}