fix(login): prevent panic with --ignore-ssl-errors on subsequent runs

pkg/apiclient/ssl.go

@@ -0,0 +1,47 @@
+package apiclient
+
+import (
+	"crypto/tls"
+	"net/http"
+)
+
+// ApplySSLIgnoreConfiguration configures the HTTP client to ignore SSL errors
+// by setting InsecureSkipVerify on the underlying transport. This function
+// handles multiple transport types:
+// - Direct *http.Transport
+// - *SpinnerRoundTripper wrapping *http.Transport
+// - Any other transport type (fallback replacement)
+func ApplySSLIgnoreConfiguration(httpClient *http.Client) {
+	if httpClient.Transport == nil {
+		httpClient.Transport = &http.Transport{}
+	}
+
+	// Handle both direct http.Transport and SpinnerRoundTripper wrapping http.Transport
+	switch transport := httpClient.Transport.(type) {
+	case *http.Transport:
+		if transport.TLSClientConfig != nil {
+			transport.TLSClientConfig.InsecureSkipVerify = true
+		} else {
+			transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+		}
+	case *SpinnerRoundTripper:
+		// If the SpinnerRoundTripper's Next is an http.Transport, configure it
+		if httpTransport, ok := transport.Next.(*http.Transport); ok {
+			if httpTransport.TLSClientConfig != nil {
+				httpTransport.TLSClientConfig.InsecureSkipVerify = true
+			} else {
+				httpTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+			}
+		} else {
+			// If Next is not an http.Transport, replace it with one that has SSL verification disabled
+			transport.Next = &http.Transport{
+				TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+			}
+		}
+	default:
+		// Fallback: replace the transport entirely with one that ignores SSL errors
+		httpClient.Transport = &http.Transport{
+			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+		}
+	}
+}

pkg/apiclient/ssl_test.go

@@ -0,0 +1,109 @@
+package apiclient
+
+import (
+	"crypto/tls"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestApplySSLIgnoreConfiguration tests the ApplySSLIgnoreConfiguration function
+func TestApplySSLIgnoreConfiguration(t *testing.T) {
+	tests := []struct {
+		name      string
+		setupFunc func() *http.Client
+	}{
+		{
+			name: "nil transport",
+			setupFunc: func() *http.Client {
+				return &http.Client{}
+			},
+		},
+		{
+			name: "direct http.Transport with nil TLSClientConfig",
+			setupFunc: func() *http.Client {
+				return &http.Client{
+					Transport: &http.Transport{},
+				}
+			},
+		},
+		{
+			name: "direct http.Transport with existing TLSClientConfig",
+			setupFunc: func() *http.Client {
+				return &http.Client{
+					Transport: &http.Transport{
+						TLSClientConfig: &tls.Config{
+							ServerName: "example.com",
+						},
+					},
+				}
+			},
+		},
+		{
+			name: "SpinnerRoundTripper with nil TLSClientConfig",
+			setupFunc: func() *http.Client {
+				return &http.Client{
+					Transport: &SpinnerRoundTripper{
+						Next: &http.Transport{},
+					},
+				}
+			},
+		},
+		{
+			name: "SpinnerRoundTripper with existing TLSClientConfig",
+			setupFunc: func() *http.Client {
+				return &http.Client{
+					Transport: &SpinnerRoundTripper{
+						Next: &http.Transport{
+							TLSClientConfig: &tls.Config{
+								ServerName: "example.com",
+							},
+						},
+					},
+				}
+			},
+		},
+		{
+			name: "SpinnerRoundTripper with default transport",
+			setupFunc: func() *http.Client {
+				return &http.Client{
+					Transport: NewSpinnerRoundTripper(),
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := tt.setupFunc()
+
+			// Apply the SSL ignore configuration
+			ApplySSLIgnoreConfiguration(client)
+
+			// Verify the configuration was applied correctly
+			verifySSLIgnored(t, client)
+		})
+	}
+}
+
+func verifySSLIgnored(t *testing.T, client *http.Client) {
+	assert.NotNil(t, client.Transport, "Transport should not be nil")
+
+	switch transport := client.Transport.(type) {
+	case *http.Transport:
+		assert.NotNil(t, transport.TLSClientConfig, "TLS config should be set")
+		assert.True(t, transport.TLSClientConfig.InsecureSkipVerify, "InsecureSkipVerify should be true")
+
+	case *SpinnerRoundTripper:
+		assert.NotNil(t, transport.Next, "SpinnerRoundTripper.Next should not be nil")
+
+		httpTransport, ok := transport.Next.(*http.Transport)
+		assert.True(t, ok, "SpinnerRoundTripper.Next should be *http.Transport")
+		assert.NotNil(t, httpTransport.TLSClientConfig, "Underlying TLS config should be set")
+		assert.True(t, httpTransport.TLSClientConfig.InsecureSkipVerify, "Underlying InsecureSkipVerify should be true")
+
+	default:
+		t.Errorf("Unexpected transport type: %T", transport)
+	}
+}

pkg/cmd/login/login.go

@@ -2,7 +2,6 @@ package login
 
 import (
 	"bytes"
-	"crypto/tls"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -126,11 +125,7 @@ func loginRun(cmd *cobra.Command, f factory.Factory, isPromptEnabled bool, ask q
 	}
 
 	if inputs.ignoreSslErrors {
-		if httpClient.Transport == nil {
-			httpClient.Transport = &http.Transport{}
-		}
-
-		httpClient.Transport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+		apiclient.ApplySSLIgnoreConfiguration(httpClient)
 	}
 
 	if inputs.apiKey != "" {

pkg/cmd/login/login_ssl_test.go

@@ -0,0 +1,85 @@
+package login_test
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/OctopusDeploy/cli/pkg/apiclient"
+	"github.com/stretchr/testify/assert"
+)
+
+// TestSSLIgnoreHandling tests that our SSL ignore logic works with both 
+// direct http.Transport and SpinnerRoundTripper scenarios
+func TestSSLIgnoreHandling(t *testing.T) {
+	tests := []struct {
+		name        string
+		transport   http.RoundTripper
+		expectPanic bool
+	}{
+		{
+			name:        "Direct http.Transport should work",
+			transport:   &http.Transport{},
+			expectPanic: false,
+		},
+		{
+			name:        "SpinnerRoundTripper with http.Transport should work",
+			transport:   &apiclient.SpinnerRoundTripper{Next: &http.Transport{}},
+			expectPanic: false,
+		},
+		{
+			name:        "SpinnerRoundTripper with default transport should work",
+			transport:   apiclient.NewSpinnerRoundTripper(),
+			expectPanic: false,
+		},
+		{
+			name:        "nil transport should work",
+			transport:   nil,
+			expectPanic: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := &http.Client{Transport: tt.transport}
+
+			// This simulates the SSL ignore logic from loginRun function
+			defer func() {
+				if r := recover(); r != nil {
+					if !tt.expectPanic {
+						t.Errorf("Unexpected panic: %v", r)
+					}
+				}
+			}()
+
+			// Apply the SSL ignore logic using the shared utility
+			apiclient.ApplySSLIgnoreConfiguration(client)
+
+			// Verify the SSL configuration was applied correctly
+			verifySSLConfig(t, client)
+		})
+	}
+}
+
+// verifySSLConfig checks that the SSL configuration was applied correctly
+func verifySSLConfig(t *testing.T, httpClient *http.Client) {
+	assert.NotNil(t, httpClient.Transport, "Transport should not be nil")
+
+	switch transport := httpClient.Transport.(type) {
+	case *http.Transport:
+		assert.NotNil(t, transport.TLSClientConfig, "TLS config should be set")
+		assert.True(t, transport.TLSClientConfig.InsecureSkipVerify, "InsecureSkipVerify should be true")
+
+	case *apiclient.SpinnerRoundTripper:
+		assert.NotNil(t, transport.Next, "SpinnerRoundTripper.Next should not be nil")
+
+		if httpTransport, ok := transport.Next.(*http.Transport); ok {
+			assert.NotNil(t, httpTransport.TLSClientConfig, "Underlying TLS config should be set")
+			assert.True(t, httpTransport.TLSClientConfig.InsecureSkipVerify, "Underlying InsecureSkipVerify should be true")
+		} else {
+			t.Errorf("SpinnerRoundTripper.Next should be *http.Transport, got %T", transport.Next)
+		}
+
+	default:
+		t.Errorf("Unexpected transport type: %T", transport)
+	}
+}