Friendlier handling of DeploymentHandshakeException from CLI in `-w…

…ebSocket` mode (jenkinsci#9591)
krisstern · Aug 30, 2024 · 15e045f · 15e045f
1 parent fc73c26
commit 15e045f
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 1 deletion.
diff --git a/cli/src/main/java/hudson/cli/CLI.java b/cli/src/main/java/hudson/cli/CLI.java
@@ -32,6 +32,7 @@
 import jakarta.websocket.ClientEndpointConfig;
 import jakarta.websocket.Endpoint;
 import jakarta.websocket.EndpointConfig;
+import jakarta.websocket.HandshakeResponse;
 import jakarta.websocket.Session;
 import java.io.DataInputStream;
 import java.io.File;
@@ -64,6 +65,7 @@
 import org.glassfish.tyrus.client.ClientManager;
 import org.glassfish.tyrus.client.ClientProperties;
 import org.glassfish.tyrus.client.SslEngineConfigurator;
+import org.glassfish.tyrus.client.exception.DeploymentHandshakeException;
 import org.glassfish.tyrus.container.jdk.client.JdkClientContainer;
 
 /**
@@ -340,13 +342,19 @@ public void onOpen(Session session, EndpointConfig config) {}
         }
 
         class Authenticator extends ClientEndpointConfig.Configurator {
+            HandshakeResponse hr;
             @Override
             public void beforeRequest(Map<String, List<String>> headers) {
                 if (factory.authorization != null) {
                     headers.put("Authorization", List.of(factory.authorization));
                 }
             }
+            @Override
+            public void afterResponse(HandshakeResponse hr) {
+                this.hr = hr;
+            }
         }
+        var authenticator = new Authenticator();
 
         ClientManager client = ClientManager.createClient(JdkClientContainer.class.getName()); // ~ ContainerProvider.getWebSocketContainer()
         client.getProperties().put(ClientProperties.REDIRECT_ENABLED, true); // https://tyrus-project.github.io/documentation/1.13.1/index/tyrus-proprietary-config.html#d0e1775
@@ -357,7 +365,21 @@ public void beforeRequest(Map<String, List<String>> headers) {
             sslEngineConfigurator.setHostnameVerifier((s, sslSession) -> true);
             client.getProperties().put(ClientProperties.SSL_ENGINE_CONFIGURATOR, sslEngineConfigurator);
         }
-        Session session = client.connectToServer(new CLIEndpoint(), ClientEndpointConfig.Builder.create().configurator(new Authenticator()).build(), URI.create(url.replaceFirst("^http", "ws") + "cli/ws"));
+        Session session;
+        try {
+            session = client.connectToServer(new CLIEndpoint(), ClientEndpointConfig.Builder.create().configurator(authenticator).build(), URI.create(url.replaceFirst("^http", "ws") + "cli/ws"));
+        } catch (DeploymentHandshakeException x) {
+            System.err.println("CLI handshake failed with status code " + x.getHttpStatusCode());
+            if (authenticator.hr != null) {
+                for (var entry : authenticator.hr.getHeaders().entrySet()) {
+                    // org.glassfish.tyrus.core.Utils.parseHeaderValue improperly splits values like Date at commas, so undo that:
+                    System.err.println(entry.getKey() + ": " + String.join(", ", entry.getValue()));
+                }
+                // UpgradeResponse.getReasonPhrase is useless since Jetty generates it from the code,
+                // and the body is not accessible at all.
+            }
+            return 15; // compare CLICommand.main
+        }
         PlainCLIProtocol.Output out = new PlainCLIProtocol.Output() {
             @Override
             public void send(byte[] data) throws IOException {

diff --git a/test/src/test/java/hudson/cli/CLIActionTest.java b/test/src/test/java/hudson/cli/CLIActionTest.java
@@ -1,5 +1,9 @@
 package hudson.cli;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
 
 import hudson.Functions;
@@ -131,6 +135,19 @@ private void assertExitCode(int code, boolean useApiToken, File jar, String... a
         assertEquals(code, proc.join());
     }
 
+    @Test public void authenticationFailed() throws Exception {
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.ADMINISTER).everywhere().toAuthenticated());
+        var jar = tmp.newFile("jenkins-cli.jar");
+        FileUtils.copyURLToFile(j.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar);
+        var baos = new ByteArrayOutputStream();
+        var exitStatus = new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
+            "java", "-jar", jar.getAbsolutePath(), "-s", j.getURL().toString(), "-auth", "user:bogustoken", "who-am-i"
+        ).stdout(baos).start().join();
+        assertThat(baos.toString(), allOf(containsString("status code 401"), containsString("Server: Jetty")));
+        assertThat(exitStatus, is(15));
+    }
+
     @Issue("JENKINS-41745")
     @Test
     public void encodingAndLocale() throws Exception {