From 0fc648c9a8662ed99acb86401b1300038eaebf9f Mon Sep 17 00:00:00 2001 From: "Johnson, Conrad" Date: Sat, 4 Oct 2025 19:55:59 -0400 Subject: [PATCH 1/6] fix: handle SpinnerRoundTripper in login --ignore-ssl-errors --- pkg/cmd/login/login.go | 21 +++++- pkg/cmd/login/login_ssl_test.go | 114 ++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 pkg/cmd/login/login_ssl_test.go diff --git a/pkg/cmd/login/login.go b/pkg/cmd/login/login.go index 5844a1d7..0a9a3b7b 100644 --- a/pkg/cmd/login/login.go +++ b/pkg/cmd/login/login.go @@ -130,7 +130,26 @@ func loginRun(cmd *cobra.Command, f factory.Factory, isPromptEnabled bool, ask q httpClient.Transport = &http.Transport{} } - httpClient.Transport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + // Handle both direct http.Transport and SpinnerRoundTripper wrapping http.Transport + switch transport := httpClient.Transport.(type) { + case *http.Transport: + transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + case *apiclient.SpinnerRoundTripper: + // If the SpinnerRoundTripper's Next is an http.Transport, configure it + if httpTransport, ok := transport.Next.(*http.Transport); ok { + 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}, + } + } } if inputs.apiKey != "" { diff --git a/pkg/cmd/login/login_ssl_test.go b/pkg/cmd/login/login_ssl_test.go new file mode 100644 index 00000000..068360e6 --- /dev/null +++ b/pkg/cmd/login/login_ssl_test.go @@ -0,0 +1,114 @@ +package login_test + +import ( + "crypto/tls" + "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 (extracted from loginRun) + applySSLIgnoreLogic(client) + + // Verify the SSL configuration was applied correctly + verifySSLConfig(t, client) + }) + } +} + +// applySSLIgnoreLogic contains the same logic as in loginRun for handling SSL +func applySSLIgnoreLogic(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: + transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + case *apiclient.SpinnerRoundTripper: + // If the SpinnerRoundTripper's Next is an http.Transport, configure it + if httpTransport, ok := transport.Next.(*http.Transport); ok { + 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}, + } + } +} + +// 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) + } +} \ No newline at end of file From 958d2716f64780b4dba07de496c63a6b73e80756 Mon Sep 17 00:00:00 2001 From: "Johnson, Conrad" Date: Sat, 4 Oct 2025 20:13:30 -0400 Subject: [PATCH 2/6] Add ssl.go for shared functionality --- pkg/apiclient/ssl.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 pkg/apiclient/ssl.go diff --git a/pkg/apiclient/ssl.go b/pkg/apiclient/ssl.go new file mode 100644 index 00000000..cf7d29a0 --- /dev/null +++ b/pkg/apiclient/ssl.go @@ -0,0 +1,39 @@ +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: + 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 { + 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}, + } + } +} \ No newline at end of file From 2b51718f60a6e0eb9b689d51f716ab51f471a7bb Mon Sep 17 00:00:00 2001 From: "Johnson, Conrad" Date: Sat, 4 Oct 2025 20:14:34 -0400 Subject: [PATCH 3/6] Replace inline ssl logic with call to apiclient.ApplySSLIgnoreCconfiguration --- pkg/cmd/login/login.go | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/pkg/cmd/login/login.go b/pkg/cmd/login/login.go index 0a9a3b7b..b01de1f0 100644 --- a/pkg/cmd/login/login.go +++ b/pkg/cmd/login/login.go @@ -2,7 +2,6 @@ package login import ( "bytes" - "crypto/tls" "encoding/json" "errors" "fmt" @@ -126,30 +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{} - } - - // Handle both direct http.Transport and SpinnerRoundTripper wrapping http.Transport - switch transport := httpClient.Transport.(type) { - case *http.Transport: - transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} - case *apiclient.SpinnerRoundTripper: - // If the SpinnerRoundTripper's Next is an http.Transport, configure it - if httpTransport, ok := transport.Next.(*http.Transport); ok { - 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}, - } - } + apiclient.ApplySSLIgnoreConfiguration(httpClient) } if inputs.apiKey != "" { From 316f0b26cc68f9d8f9e2fcd414e94a19c2d13b84 Mon Sep 17 00:00:00 2001 From: "Johnson, Conrad" Date: Sat, 4 Oct 2025 20:15:29 -0400 Subject: [PATCH 4/6] Rm dup ssl function, to use shared apiclient.ApplySSLIgnoreConfiguration --- pkg/cmd/login/login_ssl_test.go | 33 ++------------------------------- 1 file changed, 2 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/login/login_ssl_test.go b/pkg/cmd/login/login_ssl_test.go index 068360e6..b84bf2ca 100644 --- a/pkg/cmd/login/login_ssl_test.go +++ b/pkg/cmd/login/login_ssl_test.go @@ -1,7 +1,6 @@ package login_test import ( - "crypto/tls" "net/http" "testing" @@ -52,8 +51,8 @@ func TestSSLIgnoreHandling(t *testing.T) { } }() - // Apply the SSL ignore logic (extracted from loginRun) - applySSLIgnoreLogic(client) + // Apply the SSL ignore logic using the shared utility + apiclient.ApplySSLIgnoreConfiguration(client) // Verify the SSL configuration was applied correctly verifySSLConfig(t, client) @@ -61,34 +60,6 @@ func TestSSLIgnoreHandling(t *testing.T) { } } -// applySSLIgnoreLogic contains the same logic as in loginRun for handling SSL -func applySSLIgnoreLogic(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: - transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} - case *apiclient.SpinnerRoundTripper: - // If the SpinnerRoundTripper's Next is an http.Transport, configure it - if httpTransport, ok := transport.Next.(*http.Transport); ok { - 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}, - } - } -} - // 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") From dc91df97db38d6d6f266d05ebcae7f00a481f314 Mon Sep 17 00:00:00 2001 From: "Conrad J." Date: Sat, 4 Oct 2025 20:20:22 -0400 Subject: [PATCH 5/6] Update pkg/apiclient/ssl.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/apiclient/ssl.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/apiclient/ssl.go b/pkg/apiclient/ssl.go index cf7d29a0..8b960e8f 100644 --- a/pkg/apiclient/ssl.go +++ b/pkg/apiclient/ssl.go @@ -19,11 +19,19 @@ func ApplySSLIgnoreConfiguration(httpClient *http.Client) { // Handle both direct http.Transport and SpinnerRoundTripper wrapping http.Transport switch transport := httpClient.Transport.(type) { case *http.Transport: - transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + 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 { - httpTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + 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{ From 4847d32628b42b9ae5fc85698ba2ff5f70fb7e49 Mon Sep 17 00:00:00 2001 From: "Johnson, Conrad" Date: Sat, 4 Oct 2025 20:23:36 -0400 Subject: [PATCH 6/6] Update ssl.go --- pkg/apiclient/ssl_test.go | 109 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 pkg/apiclient/ssl_test.go diff --git a/pkg/apiclient/ssl_test.go b/pkg/apiclient/ssl_test.go new file mode 100644 index 00000000..0e5430f5 --- /dev/null +++ b/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) + } +} \ No newline at end of file