Refactor redirect flow and fix URLs for --bind-addr

lib/client/weblogin.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2015 Gravitational, Inc.
+Copyright 2015-2019 Gravitational, Inc.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@ import (
 	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/lib/auth"
 	"github.com/gravitational/teleport/lib/defaults"
+	"github.com/gravitational/teleport/lib/utils"
 
 	"github.com/gravitational/roundtrip"
 	"github.com/gravitational/trace"
@@ -225,6 +226,17 @@ func SSHAgentSSOLogin(login SSHLogin) (*auth.SSHLoginResponse, error) {
 		return re, nil
 	})
 
+	redirPath := "/" + uuid.New()
+	// longURL will be set based on the response from the webserver
+	var longURL utils.SyncString
+	mux := http.NewServeMux()
+	mux.Handle("/callback", handler)
+	mux.HandleFunc(redirPath, func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, longURL.Value(), http.StatusFound)
+	})
+	redir := httptest.NewServer(mux)
+	defer redir.Close()
+
 	var server *httptest.Server
 	if login.BindAddr != "" {
 		log.Debugf("Binding to %v.", login.BindAddr)
@@ -234,14 +246,17 @@ func SSHAgentSSOLogin(login SSHLogin) (*auth.SSHLoginResponse, error) {
 		}
 		server = &httptest.Server{
 			Listener: listener,
-			Config:   &http.Server{Handler: handler},
+			Config:   &http.Server{Handler: mux},
 		}
 		server.Start()
 	} else {
-		server = httptest.NewServer(handler)
+		server = httptest.NewServer(mux)
 	}
 	defer server.Close()
 
+	// redirURL is the short URL presented to the user
+	redirURL := server.URL + redirPath
+
 	u, err := url.Parse(server.URL + "/callback")
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -266,18 +281,7 @@ func SSHAgentSSOLogin(login SSHLogin) (*auth.SSHLoginResponse, error) {
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-
-	// Start a HTTP server on the client that re-directs to the SAML provider.
-	// This creates nice short URLs and also works around some platforms (like
-	// Windows) that truncate long URLs before passing them to the default browser.
-	redirPath := "/" + uuid.New()
-	redirMux := http.NewServeMux()
-	redirMux.HandleFunc(redirPath, func(w http.ResponseWriter, r *http.Request) {
-		http.Redirect(w, r, re.RedirectURL, http.StatusFound)
-	})
-	redir := httptest.NewServer(redirMux)
-	defer redir.Close()
-	redirURL := redir.URL + redirPath
+	longURL.Set(re.RedirectURL)
 
 	// If a command was found to launch the browser, create and start it.
 	var execCmd *exec.Cmd
@@ -307,7 +311,7 @@ func SSHAgentSSOLogin(login SSHLogin) (*auth.SSHLoginResponse, error) {
 
 	// Print to screen in-case the command that launches the browser did not run.
 	fmt.Printf("If browser window does not open automatically, open it by ")
-	fmt.Printf("clicking on the link:\n %v\n", redirURL)
+	fmt.Printf("clicking on the link:\n %v\n", utils.ClickableURL(redirURL))
 
 	log.Infof("Waiting for response at: %v.", server.URL)
 

lib/utils/utils.go

@@ -18,13 +18,16 @@ package utils
 
 import (
 	"encoding/json"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/gravitational/teleport"
@@ -33,6 +36,49 @@ import (
 	"github.com/pborman/uuid"
 )
 
+// SyncString is a string value
+// that can be concurrently accessed
+type SyncString struct {
+	sync.Mutex
+	string
+}
+
+// Value returns value of the string
+func (s *SyncString) Value() string {
+	s.Lock()
+	defer s.Unlock()
+	return s.string
+}
+
+// Set sets the value of the string
+func (s *SyncString) Set(v string) {
+	s.Lock()
+	defer s.Unlock()
+	s.string = v
+}
+
+// ClickableURL fixes address in url to make sure
+// it's clickable, e.g. it replaces "undefined" address like
+// 0.0.0.0 used in network listeners format with loopback 127.0.0.1
+func ClickableURL(in string) string {
+	out, err := url.Parse(in)
+	if err != nil {
+		return in
+	}
+	host, port, err := net.SplitHostPort(out.Host)
+	if err != nil {
+		return in
+	}
+	ip := net.ParseIP(host)
+	// if address is not an IP, unspecified, e.g. all interfaces 0.0.0.0 or multicast,
+	// replace with localhost that is clickable
+	if len(ip) == 0 || ip.IsUnspecified() || ip.IsMulticast() {
+		out.Host = fmt.Sprintf("127.0.0.1:%v", port)
+		return out.String()
+	}
+	return out.String()
+}
+
 // AsBool converts string to bool, in case of the value is empty
 // or unknown, defaults to false
 func AsBool(v string) bool {

lib/utils/utils_test.go

@@ -124,6 +124,26 @@ func (s *UtilsSuite) TestVersions(c *check.C) {
 	}
 }
 
+// TestClickableURL tests clickable URL conversions
+func (s *UtilsSuite) TestClickableURL(c *check.C) {
+	testCases := []struct {
+		info string
+		in   string
+		out  string
+	}{
+		{info: "original URL is OK", in: "http://127.0.0.1:3000/hello", out: "http://127.0.0.1:3000/hello"},
+		{info: "unspecified IPV6", in: "http://[::]:5050/howdy", out: "http://127.0.0.1:5050/howdy"},
+		{info: "unspecified IPV4", in: "http://0.0.0.0:5050/howdy", out: "http://127.0.0.1:5050/howdy"},
+		{info: "specified IPV4", in: "http://192.168.1.1:5050/howdy", out: "http://192.168.1.1:5050/howdy"},
+		{info: "specified IPV6", in: "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:5050/howdy", out: "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:5050/howdy"},
+	}
+	for i, testCase := range testCases {
+		comment := check.Commentf("test case %v %q", i, testCase.info)
+		out := ClickableURL(testCase.in)
+		c.Assert(out, check.Equals, testCase.out, comment)
+	}
+}
+
 // TestParseSessionsURI parses sessions URI
 func (s *UtilsSuite) TestParseSessionsURI(c *check.C) {
 	testCases := []struct {