Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion pkg/cmd/login/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ 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}
if err := skipTlsVerification(httpClient.Transport); err != nil {
return err
}
}

if inputs.apiKey != "" {
Expand Down Expand Up @@ -421,3 +423,19 @@ func testLogin(cmd *cobra.Command, httpClient *http.Client, server string, crede

return nil
}

// The client factory wraps the transport in a spinner, so the setting has to be
// applied to the transport underneath rather than to the wrapper.
func skipTlsVerification(roundTripper http.RoundTripper) error {
switch transport := roundTripper.(type) {
case *http.Transport:
transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
return nil
case *apiclient.SpinnerRoundTripper:
return skipTlsVerification(transport.Next)
default:
// Better to say so than to quietly leave verification on after the
// caller asked for the opposite.
return fmt.Errorf("cannot ignore SSL errors: unsupported HTTP transport %T", roundTripper)
}
}
38 changes: 38 additions & 0 deletions pkg/cmd/login/login_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package login

import (
"net/http"
"testing"

"github.com/OctopusDeploy/cli/pkg/apiclient"
"github.com/stretchr/testify/assert"
)

func TestSkipTlsVerification_OnATransport(t *testing.T) {
transport := &http.Transport{}

assert.NoError(t, skipTlsVerification(transport))
assert.True(t, transport.TLSClientConfig.InsecureSkipVerify)
}

// The transport the client factory hands out is always spinner-wrapped, which is
// what the old type assertion tripped over.
func TestSkipTlsVerification_ThroughTheSpinner(t *testing.T) {
spinner := apiclient.NewSpinnerRoundTripper(nil)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@flin-8 with the session I had going with Claude that spun up context from when we first started to fix the original driver for the changes in this space, on this it brings up:

sets Next: http.DefaultTransport, the process-wide *http.Transport every client without its own uses.

spinner.Next = &http.Transport{}

assert.NoError(t, skipTlsVerification(spinner))
assert.True(t, spinner.Next.(*http.Transport).TLSClientConfig.InsecureSkipVerify)
}

func TestSkipTlsVerification_ReportsAnUnknownTransport(t *testing.T) {
err := skipTlsVerification(unknownTransport{})

assert.ErrorContains(t, err, "unsupported HTTP transport login.unknownTransport")
}

type unknownTransport struct{}

func (unknownTransport) RoundTrip(*http.Request) (*http.Response, error) {
return nil, nil
}
16 changes: 16 additions & 0 deletions pkg/cmd/login/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,22 @@ func TestLogin_ApiKey(t *testing.T) {
assert.Empty(t, fac.ConfigProvider.Get(constants.ConfigAccessToken))
}},

{"non-interactive: ignoring ssl errors no longer panics on a wrapped transport", func(t *testing.T, fac *testutil.MockFactory, api *testutil.MockHttpServer, qa *testutil.AskMocker, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) {
currentHost := fac.GetCurrentHost()

cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) {
defer api.Close()
rootCmd.SetArgs([]string{"login", "--server", currentHost, "--api-key", "API-APIKEY01", "--ignore-ssl-errors", "--no-prompt"})
return rootCmd.ExecuteC()
})

_, err := testutil.ReceivePair(cmdReceiver)

// The mock factory's transport is not one we can reach into, so the
// flag is refused; before the fix this panicked instead.
assert.ErrorContains(t, err, "unsupported HTTP transport")
}},

{"non-interactive: if server parameter not supplied returns error", func(t *testing.T, fac *testutil.MockFactory, api *testutil.MockHttpServer, qa *testutil.AskMocker, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) {
apiKey := "API-APIKEY01"

Expand Down