command: Swappable implementation of launching web browsers

For unit testing in particular we can't launch a real browser for testing,
so this indirection is primarily to allow us to substitute a mock when
testing a command that can launch a browser.

This includes a simple mock implementation that expects to interact with
a running web server directly.

command/login.go

@@ -21,7 +21,6 @@ import (
 	"github.com/hashicorp/terraform/tfdiags"
 
 	uuid "github.com/hashicorp/go-uuid"
-	"github.com/pkg/browser"
 	"golang.org/x/oauth2"
 )
 
@@ -375,12 +374,22 @@ func (c *LoginCommand) interactiveGetTokenByCode(hostname svchost.Hostname, cred
 		oauth2.SetAuthURLParam("code_challenge", proofKeyChallenge),
 		oauth2.SetAuthURLParam("code_challenge_method", "S256"),
 	)
-	err = browser.OpenURL(authCodeURL)
+
-	if err == nil {
+	launchBrowserManually := false
-		c.Ui.Output(fmt.Sprintf("Terraform must now open a web browser to the login page for %s.\n", hostname.ForDisplay()))
+	if c.BrowserLauncher != nil {
-		c.Ui.Output(fmt.Sprintf("If a browser does not open this automatically, open the following URL to proceed:\n    %s\n", authCodeURL))
+		err = c.BrowserLauncher.OpenURL(authCodeURL)
+		if err == nil {
+			c.Ui.Output(fmt.Sprintf("Terraform must now open a web browser to the login page for %s.\n", hostname.ForDisplay()))
+			c.Ui.Output(fmt.Sprintf("If a browser does not open this automatically, open the following URL to proceed:\n    %s\n", authCodeURL))
+		} else {
+			// Assume we're on a platform where opening a browser isn't possible.
+			launchBrowserManually = true
+		}
 	} else {
-		// Assume we're on a platform where opening a browser isn't possible.
+		launchBrowserManually = true
+	}
+
+	if launchBrowserManually {
 		c.Ui.Output(fmt.Sprintf("Open the following URL to access the login page for %s:\n    %s\n", hostname.ForDisplay(), authCodeURL))
 	}
 

command/meta.go

@@ -18,6 +18,7 @@ import (
 	"github.com/hashicorp/terraform/backend"
 	"github.com/hashicorp/terraform/backend/local"
 	"github.com/hashicorp/terraform/command/format"
+	"github.com/hashicorp/terraform/command/webbrowser"
 	"github.com/hashicorp/terraform/configs/configload"
 	"github.com/hashicorp/terraform/helper/experiment"
 	"github.com/hashicorp/terraform/helper/wrappedstreams"
@@ -78,6 +79,10 @@ type Meta struct {
 	// is not suitable, e.g. because of a read-only filesystem.
 	OverrideDataDir string
 
+	// BrowserLauncher is used by commands that need to open a URL in a
+	// web browser.
+	BrowserLauncher webbrowser.Launcher
+
 	// When this channel is closed, the command will be cancelled.
 	ShutdownCh <-chan struct{}
 

command/webbrowser/mock.go

@@ -0,0 +1,155 @@
+package webbrowser
+
+import (
+	"context"
+	"fmt"
+	"log"
+	"net/http"
+	"net/url"
+	"sync"
+
+	"github.com/hashicorp/terraform/httpclient"
+)
+
+// NewMockLauncher creates and returns a mock implementation of Launcher,
+// with some special behavior designed for use in unit tests.
+//
+// See the documentation of MockLauncher itself for more information.
+func NewMockLauncher(ctx context.Context) *MockLauncher {
+	client := httpclient.New()
+	return &MockLauncher{
+		Client:  client,
+		Context: ctx,
+	}
+}
+
+// MockLauncher is a mock implementation of Launcher that has some special
+// behavior designed for use in unit tests.
+//
+// When OpenURL is called, MockLauncher will make an HTTP request to the given
+// URL rather than interacting with a "real" browser.
+//
+// In normal situations it will then return with no further action, but if
+// the response to the given URL is either a standard HTTP redirect response
+// or includes the custom HTTP header X-Redirect-To then MockLauncher will
+// send a follow-up request to that target URL, and continue in this manner
+// until it reaches a URL that is not a redirect. (The X-Redirect-To header
+// is there so that a server can potentially offer a normal HTML page to
+// an actual browser while also giving a next-hop hint for MockLauncher.)
+//
+// Since MockLauncher is not a full programmable user-agent implementation
+// it can't be used for testing of real-world web applications, but it can
+// be used for testing against specialized test servers that are written
+// with MockLauncher in mind and know how to drive the request flow through
+// whatever steps are required to complete the desired test.
+//
+// All of the actions taken by MockLauncher happen asynchronously in the
+// background, to simulate the concurrency of a separate web browser.
+// Test code using MockLauncher should provide a context which is cancelled
+// when the test completes, to help avoid leaking MockLaunchers.
+type MockLauncher struct {
+	// Client is the HTTP client that MockLauncher will use to make requests.
+	// By default (if you use NewMockLauncher) this is a new client created
+	// via httpclient.New, but callers may override it if they need customized
+	// behavior for a particular test.
+	//
+	// Do not use a client that is shared with any other subsystem, because
+	// MockLauncher will customize the settings of the given client.
+	Client *http.Client
+
+	// Context can be cancelled in order to abort an OpenURL call before it
+	// would naturally complete.
+	Context context.Context
+
+	// Responses is a log of all of the responses recieved from the launcher's
+	// requests, in the order requested.
+	Responses []*http.Response
+
+	// done is a waitgroup used internally to signal when the async work is
+	// complete, in order to make this mock more convenient to use in tests.
+	done sync.WaitGroup
+}
+
+var _ Launcher = (*MockLauncher)(nil)
+
+// OpenURL is the mock implementation of Launcher, which has the special
+// behavior described for type MockLauncher.
+func (l *MockLauncher) OpenURL(u string) error {
+	// We run our operation in the background because it's supposed to be
+	// behaving like a web browser running in a separate process.
+	log.Printf("[TRACE] webbrowser.MockLauncher: OpenURL(%q) starting in the background", u)
+	l.done.Add(1)
+	go func() {
+		err := l.openURL(u)
+		if err != nil {
+			// Can't really do anything with this asynchronously, so we'll
+			// just log it so that someone debugging will be able to see it.
+			log.Printf("[ERROR] webbrowser.MockLauncher: OpenURL(%q): %s", u, err)
+		} else {
+			log.Printf("[TRACE] webbrowser.MockLauncher: OpenURL(%q) has concluded", u)
+		}
+		l.done.Done()
+	}()
+	return nil
+}
+
+func (l *MockLauncher) openURL(u string) error {
+	// We need to disable automatic redirect following so that we can implement
+	// it ourselves below, and thus be able to see the redirects in our
+	// responses log.
+	l.Client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+		return http.ErrUseLastResponse
+	}
+
+	// We'll keep looping as long as the server keeps giving us new URLs to
+	// request.
+	for u != "" {
+		log.Printf("[DEBUG] webbrowser.MockLauncher: requesting %s", u)
+		req, err := http.NewRequest("GET", u, nil)
+		if err != nil {
+			return fmt.Errorf("failed to construct HTTP request for %s: %s", u, err)
+		}
+		resp, err := l.Client.Do(req)
+		if err != nil {
+			log.Printf("[DEBUG] webbrowser.MockLauncher: request failed: %s", err)
+			return fmt.Errorf("error requesting %s: %s", u, err)
+		}
+		l.Responses = append(l.Responses, resp)
+		if resp.StatusCode >= 400 {
+			log.Printf("[DEBUG] webbrowser.MockLauncher: request failed: %s", resp.Status)
+			return fmt.Errorf("error requesting %s: %s", u, resp.Status)
+		}
+		log.Printf("[DEBUG] webbrowser.MockLauncher: request succeeded: %s", resp.Status)
+
+		u = "" // unless it's a redirect, we'll stop after this
+		if location := resp.Header.Get("Location"); location != "" {
+			u = location
+		} else if redirectTo := resp.Header.Get("X-Redirect-To"); redirectTo != "" {
+			u = redirectTo
+		}
+
+		if u != "" {
+			// HTTP technically doesn't permit relative URLs in Location, but
+			// browsers tolerate it and so real-world servers do it, and thus
+			// we'll allow it here too.
+			oldURL := resp.Request.URL
+			givenURL, err := url.Parse(u)
+			if err != nil {
+				return fmt.Errorf("invalid redirect URL %s: %s", u, err)
+			}
+			u = oldURL.ResolveReference(givenURL).String()
+			log.Printf("[DEBUG] webbrowser.MockLauncher: redirected to %s", u)
+		}
+	}
+
+	log.Printf("[DEBUG] webbrowser.MockLauncher: all done")
+	return nil
+}
+
+// Wait blocks until the MockLauncher has finished its asynchronous work of
+// making HTTP requests and following redirects, at which point it will have
+// reached a request that didn't redirect anywhere and stopped iterating.
+func (l *MockLauncher) Wait() {
+	log.Printf("[TRACE] webbrowser.MockLauncher: Wait() for current work to complete")
+	l.done.Wait()
+}

command/webbrowser/mock_test.go

@@ -0,0 +1,95 @@
+package webbrowser
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestMockLauncher(t *testing.T) {
+	s := httptest.NewServer(http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+		resp.Header().Set("Content-Length", "0")
+		switch req.URL.Path {
+		case "/standard-redirect-source":
+			resp.Header().Set("Location", "/standard-redirect-target")
+			resp.WriteHeader(302)
+		case "/custom-redirect-source":
+			resp.Header().Set("X-Redirect-To", "/custom-redirect-target")
+			resp.WriteHeader(200)
+		case "/error":
+			resp.WriteHeader(500)
+		default:
+			resp.WriteHeader(200)
+		}
+	}))
+	defer s.Close()
+
+	t.Run("no redirects", func(t *testing.T) {
+		l := NewMockLauncher(context.Background())
+		err := l.OpenURL(s.URL)
+		if err != nil {
+			t.Fatal(err)
+		}
+		l.Wait() // Let the async work complete
+		if got, want := len(l.Responses), 1; got != want {
+			t.Fatalf("wrong number of responses %d; want %d", got, want)
+		}
+		if got, want := l.Responses[0].Request.URL.Path, ""; got != want {
+			t.Fatalf("wrong request URL %q; want %q", got, want)
+		}
+	})
+	t.Run("error", func(t *testing.T) {
+		l := NewMockLauncher(context.Background())
+		err := l.OpenURL(s.URL + "/error")
+		if err != nil {
+			// Th is kind of error is supposed to happen asynchronously, so we
+			// should not see it here.
+			t.Fatal(err)
+		}
+		l.Wait() // Let the async work complete
+		if got, want := len(l.Responses), 1; got != want {
+			t.Fatalf("wrong number of responses %d; want %d", got, want)
+		}
+		if got, want := l.Responses[0].Request.URL.Path, "/error"; got != want {
+			t.Fatalf("wrong request URL %q; want %q", got, want)
+		}
+		if got, want := l.Responses[0].StatusCode, 500; got != want {
+			t.Fatalf("wrong response status %d; want %d", got, want)
+		}
+	})
+	t.Run("standard redirect", func(t *testing.T) {
+		l := NewMockLauncher(context.Background())
+		err := l.OpenURL(s.URL + "/standard-redirect-source")
+		if err != nil {
+			t.Fatal(err)
+		}
+		l.Wait() // Let the async work complete
+		if got, want := len(l.Responses), 2; got != want {
+			t.Fatalf("wrong number of responses %d; want %d", got, want)
+		}
+		if got, want := l.Responses[0].Request.URL.Path, "/standard-redirect-source"; got != want {
+			t.Fatalf("wrong request 0 URL %q; want %q", got, want)
+		}
+		if got, want := l.Responses[1].Request.URL.Path, "/standard-redirect-target"; got != want {
+			t.Fatalf("wrong request 1 URL %q; want %q", got, want)
+		}
+	})
+	t.Run("custom redirect", func(t *testing.T) {
+		l := NewMockLauncher(context.Background())
+		err := l.OpenURL(s.URL + "/custom-redirect-source")
+		if err != nil {
+			t.Fatal(err)
+		}
+		l.Wait() // Let the async work complete
+		if got, want := len(l.Responses), 2; got != want {
+			t.Fatalf("wrong number of responses %d; want %d", got, want)
+		}
+		if got, want := l.Responses[0].Request.URL.Path, "/custom-redirect-source"; got != want {
+			t.Fatalf("wrong request 0 URL %q; want %q", got, want)
+		}
+		if got, want := l.Responses[1].Request.URL.Path, "/custom-redirect-target"; got != want {
+			t.Fatalf("wrong request 1 URL %q; want %q", got, want)
+		}
+	})
+}

command/webbrowser/native.go

@@ -0,0 +1,18 @@
+package webbrowser
+
+import (
+	"github.com/pkg/browser"
+)
+
+// NewNativeLauncher creates and returns a Launcher that will attempt to interact
+// with the browser-launching mechanisms of the operating system where the
+// program is currently running.
+func NewNativeLauncher() Launcher {
+	return nativeLauncher{}
+}
+
+type nativeLauncher struct{}
+
+func (l nativeLauncher) OpenURL(url string) error {
+	return browser.OpenURL(url)
+}

command/webbrowser/webbrowser.go

@@ -0,0 +1,19 @@
+package webbrowser
+
+// Launcher is an object that knows how to open a given URL in a new tab in
+// some suitable browser on the current system.
+//
+// Launching of browsers is a very target-platform-sensitive activity, so
+// this interface serves as an abstraction over many possible implementations
+// which can be selected based on what is appropriate for a specific situation.
+type Launcher interface {
+	// OpenURL opens the given URL in a web browser.
+	//
+	// Depending on the circumstances and on the target platform, this may or
+	// may not cause the browser to take input focus. Because of this
+	// uncertainty, any caller of this method must be sure to include some
+	// language in its UI output to let the user know that a browser tab has
+	// opened somewhere, so that they can go and find it if the focus didn't
+	// switch automatically.
+	OpenURL(url string) error
+}

commands.go

@@ -5,6 +5,7 @@ import (
 	"os/signal"
 
 	"github.com/hashicorp/terraform/command"
+	"github.com/hashicorp/terraform/command/webbrowser"
 	pluginDiscovery "github.com/hashicorp/terraform/plugin/discovery"
 	"github.com/hashicorp/terraform/svchost"
 	"github.com/hashicorp/terraform/svchost/auth"
@@ -63,7 +64,8 @@ func initCommands(config *Config, services *disco.Disco) {
 		PluginOverrides:  &PluginOverrides,
 		Ui:               Ui,
 
-		Services: services,
+		Services:        services,
+		BrowserLauncher: webbrowser.NewNativeLauncher(),
 
 		RunningInAutomation: inAutomation,
 		CLIConfigDir:        configDir,