[Fix] Fail fast when authenticating if host is not configured (#1033)

mgyucht · web-flow · commit 4886afe31293 · 2024-08-30T12:42:07.000Z
## Changes
The SDK currently hangs when making API requests if the Host is set to a
non-empty but invalid endpoint, such as `https://:443`. This PR adds a
check during authentication to ensure that the hostname is defined,
failing early if not. The underlying error is exported so other clients,
such as the CLI or Terraform Provider, can provide specific advice when
this happens.

## Tests
Added a unit test for this case.

- [ ] `make test` passing
- [ ] `make fmt` applied
- [ ] relevant integration tests applied
diff --git a/config/config.go b/config/config.go
@@ -2,6 +2,7 @@ package config
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -336,7 +337,8 @@ func (c *Config) WithTesting() *Config {
 }
 
 func (c *Config) CanonicalHostName() string {
-	c.fixHostIfNeeded()
+	// Missing host is tolerated here.
+	_ = c.fixHostIfNeeded()
 	return c.Host
 }
 
@@ -361,7 +363,9 @@ func (c *Config) authenticateIfNeeded() error {
 	if c.Credentials == nil {
 		c.Credentials = &DefaultCredentials{}
 	}
-	c.fixHostIfNeeded()
+	if err := c.fixHostIfNeeded(); err != nil && !errors.Is(err, ErrNoHostConfigured) {
+		return err
+	}
 	ctx := c.refreshClient.InContextForOAuth2(c.refreshCtx)
 	credentialsProvider, err := c.Credentials.Configure(ctx, c)
 	if err != nil {
@@ -372,7 +376,9 @@ func (c *Config) authenticateIfNeeded() error {
 	}
 	c.credentialsProvider = credentialsProvider
 	c.AuthType = c.Credentials.Name()
-	c.fixHostIfNeeded()
+	if err := c.fixHostIfNeeded(); err != nil {
+		return err
+	}
 	// TODO: error customization
 	return nil
 }
@@ -393,6 +399,9 @@ func (c *Config) fixHostIfNeeded() error {
 			return err
 		}
 	}
+	if parsedHost.Hostname() == "" {
+		return ErrNoHostConfigured
+	}
 	// Create new instance to ensure other fields are initialized as empty.
 	parsedHost = &url.URL{
 		Scheme: parsedHost.Scheme,
@@ -403,6 +412,11 @@ func (c *Config) fixHostIfNeeded() error {
 	return nil
 }
 
+// ErrNoHostConfigured is the error returned when a user tries to authenticate
+// without a host configured. Applications can check for this error to provide
+// more user-friendly error messages.
+var ErrNoHostConfigured = fmt.Errorf("no host configured")
+
 func (c *Config) refreshTokenErrorMapper(ctx context.Context, resp common.ResponseWrapper) error {
 	defaultErr := httpclient.DefaultErrorMapper(ctx, resp)
 	if defaultErr == nil {
diff --git a/config/config_test.go b/config/config_test.go
@@ -1,9 +1,12 @@
 package config
 
 import (
+	"context"
+	"net/http"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestIsAccountClient_AwsAccount(t *testing.T) {
@@ -52,3 +55,14 @@ func TestNewWithWorkspaceHost(t *testing.T) {
 	// The new config will not be automatically resolved.
 	assert.False(t, c2.resolved)
 }
+
+func TestAuthenticate_InvalidHostSet(t *testing.T) {
+	c := &Config{
+		Host:  "https://:443",
+		Token: "abcdefg",
+	}
+	req, err := http.NewRequestWithContext(context.Background(), "GET", c.Host, nil)
+	require.NoError(t, err)
+	err = c.Authenticate(req)
+	assert.ErrorIs(t, err, ErrNoHostConfigured)
+}
