From 7ccd6204c4663014e9b2ba91dc0078ac879adfb4 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Thu, 29 Aug 2019 15:50:03 -0700 Subject: [PATCH] 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 +++-- command/meta.go | 5 + command/webbrowser/mock.go | 155 +++++++++++++++++++++++++++++++ command/webbrowser/mock_test.go | 95 +++++++++++++++++++ command/webbrowser/native.go | 18 ++++ command/webbrowser/webbrowser.go | 19 ++++ commands.go | 4 +- 7 files changed, 310 insertions(+), 7 deletions(-) create mode 100644 command/webbrowser/mock.go create mode 100644 command/webbrowser/mock_test.go create mode 100644 command/webbrowser/native.go create mode 100644 command/webbrowser/webbrowser.go diff --git a/command/login.go b/command/login.go index 3bac99d1c..de7d0afbf 100644 --- a/command/login.go +++ b/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 { - 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)) + + launchBrowserManually := false + if c.BrowserLauncher != nil { + 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)) } diff --git a/command/meta.go b/command/meta.go index 9e1d8c4cb..d55c9f8a5 100644 --- a/command/meta.go +++ b/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{} diff --git a/command/webbrowser/mock.go b/command/webbrowser/mock.go new file mode 100644 index 000000000..ef411ba1e --- /dev/null +++ b/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() +} diff --git a/command/webbrowser/mock_test.go b/command/webbrowser/mock_test.go new file mode 100644 index 000000000..610f83d87 --- /dev/null +++ b/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) + } + }) +} diff --git a/command/webbrowser/native.go b/command/webbrowser/native.go new file mode 100644 index 000000000..4e8281ce1 --- /dev/null +++ b/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) +} diff --git a/command/webbrowser/webbrowser.go b/command/webbrowser/webbrowser.go new file mode 100644 index 000000000..8931ec517 --- /dev/null +++ b/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 +} diff --git a/commands.go b/commands.go index 1aefd5eb2..9ea02c49c 100644 --- a/commands.go +++ b/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,